Contents of `x5c` JWT header invalid

[onematchfox]

The x5c JWT header produced by this plugin is currently invalid. This is because the header is set at https://github.com/Optum/kong-upstream-jwt/blob/0d558c89f345b8f906499285e56ea5bb39633ad6/src/access.lua#L83 using the local method at https://github.com/Optum/kong-upstream-jwt/blob/0d558c89f345b8f906499285e56ea5bb39633ad6/src/access.lua#L39 which also performs the additional base64 url encoding. Thus, the header value itself is unnecessarily encoded twice. I.e. this substitution only needs to be performed prior during the final base64 url encoding of the header segments at https://github.com/Optum/kong-upstream-jwt/blob/0d558c89f345b8f906499285e56ea5bb39633ad6/src/access.lua#L89

Can be verified at https://jwt.io/.

Will submit PR to fix in the next day or so (along with a couple other changes/fixes).

[jeremyjpj0916]

cc @rsbrisci on the x5c logic. It's working for us internally right now 😀, but I did check our validation library we offer upstream providers internally and I see it b64 decoding the x5c element itself too post decoding of the jwt for parsing. I don't know of a way to move forward with that fix for us without potentially breaking our various api providers(some don't even use our offered library and wrote their own too). Hmm.

As for execution time, fully agree there, we don't do request body transformations internally so never been an issue for us but I can see it happening with other folks.

Also appreciate the PR refactor of schema to meet the new standard and not leverage the legacy format. I am slightly unsure the behavior of Kong when we do so as our plugin sits globally on the gateway enabled, changing the underlying schema.lua file with the plugin existing in the db. I will have to play with that in a sandbox to see how we can merge that and be safe to not hurt our environments too as its good to modernize. All our plugins currently still use the legacy schema format 💀 .

[onematchfox]

Thanks for the quick feedback!

re: x5c logic

I'll leave that up to you guys then as to how you want to handle it. Ultimately I don't want the header as I don't want the overhead of the extra data. I started out by just making the header optional but in doing so noticed that the header being generated is not according to specification so I figured I might as well fix it.

I see it b64 decoding the x5c element itself

You mean b64 URL decoding? The certificate should be b64, it's the additional URL encoding to cater for +/= that is the issue.

appreciate the PR refactor of schema

You're welcome. I thought I'd throw that in as a little thank you for the initial work that went into this.

I will have to play with that in a sandbox to see how we can merge that and be safe to not hurt our environments

I upgraded all of my own plugins back when 1.0 was released and I don't remember there being any issues with plugins that were already configured so long as the fields on the schema didn't change. I haven't added/removed any additional schema values so AFAIK it should just work out the box - Kong has been doing the conversion for you in the background until now. However, be aware that the schema is slightly different in that it is restricted to no_consumer and protocols_http

[jeremyjpj0916]

@onematchfox still trying to work in time to review all this. We do agile sprints and I have to get internal approval from our product owner for these sorts of engagements(and the changes that come along with your pr's have to be discussed among a few folk). Have not forgotten about yah!

cc @rsbrisci

[onematchfox]

All good. I'm operating off an internal fork of this for the time being. Thanks for letting me know.