Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mp.jwt.verify.publickey format clarification #199

Open
vsmid opened this issue Jul 12, 2020 · 12 comments
Open

mp.jwt.verify.publickey format clarification #199

vsmid opened this issue Jul 12, 2020 · 12 comments
Assignees
Milestone

Comments

@vsmid
Copy link
Contributor

vsmid commented Jul 12, 2020

Hi,

I noticed that OpenLiberty requires -----BEGIN/END PUBLIC KEY----- header/footer headers when setting mp.jwt.verify.publickey value. For example, I tried the same thing on Payara and Quarkus and it didn't work. They both work without headers.

A while ago I did ask on OpenLiberty GitHub about this and they referred to the MicroProfile JWT docs part where it says that MicroProfile JWT implementations must inspect the supplied Public Key body for the -----BEGIN PUBLIC KEY----- header and parse the key as PCKS#8 if found.

Is this a case of a less strict public key format (Payara, Quarkus) when specifying mp.jwt.verify.publickey and should it be allowed?

@sberyozkin
Copy link
Contributor

sberyozkin commented Jul 13, 2020

@vsmid In general, the implementations are allowed to support things which have not been explicitly allowed for in the spec.
Re this specific issue, from a pure practical point of view, putting the Base64 encoded value directly is something a user can do easily, but it appears that dealing with -----BEGIN PUBLIC KEY----- and -----END PUBLIC KEY----- was really meant for the mp.jwt.verify.publickey.location.
Question, the spec example shows how a Base64URL encoded JWK is passed as a system property. If we have a PEM file, is it even possible to pass its content as a mp.jwt.verify.publickey system property ?
I.e, one would most likely have to wrap that into " double quotes - but that adds the extra processing requirements (strip the double quotes which is not needed for processing PEM)

@sberyozkin
Copy link
Contributor

@vsmid
Copy link
Contributor Author

vsmid commented Jul 13, 2020

@sberyozkin Thanks. Perhaps both formats(with and without headers) can be acceptable? I mean, it is a bit confusing when switching servers the same thing acts very differently. Maybe docs could say a little bit more about this.

@rdebusscher
Copy link
Member

Seems the headers are more or less standard these days (as also defined in an RFC), so probably we want to indicate this in the spec that headers are required (2.0 as this is breaking change)

@sberyozkin sberyozkin added this to the JWT-2.0 milestone Jul 14, 2020
@sberyozkin
Copy link
Contributor

sberyozkin commented Jul 14, 2020

@vsmid Thanks; I'm pretty sure the PEM key without the headers (as mp.jwt.verify.publickey) should also work with Open Liberty, I think we have a TCK test for it :-).
We can add a new test in 2.0 with the headers included...

@sberyozkin
Copy link
Contributor

sberyozkin commented Mar 18, 2021

@rdebusscher Hi Rudy, I misinterpreted your comment,

Seems the headers are more or less standard these days (as also defined in an RFC), so probably we want to indicate this in the spec that headers are required (2.0 as this is breaking change)

I don't think we have to mandate in 2.0 that the users use complete PEM file content with the headers for mp.jwt.verify.publickey - this is simply much more complex to do and less readable compared to inlining the encoded key content alone which MP JWT already supports.
But what should become a requirement is that the implementations must be able to accept the the PEM headers + the encoded key if someone really wants to do it - but it will not be a breaking change as this is already required - we just don't have a test.

As such I'd consider it for a possible 1.2.1 if it were to happen

Thanks

@sberyozkin sberyozkin self-assigned this Mar 29, 2021
@sberyozkin
Copy link
Contributor

sberyozkin commented Apr 27, 2021

@vsmid Hi - I've just looked closer into this issue - MP JWT TCK already enforces a fully formatted inlined public key is accepted, see the test properties.

I've also verified it works in smallrye-jwt which is used by Quarkus (about to open a PR there) - you mentioned above it does not work in Quarkus - perhaps it did not work at some stage but I do expect it to work now.

IMHO we should certainly not require the users to start using the fully formatted PEM key content for mp.jwt.verify.public.key - for the inlined key value it makes sense to continue making it easy for the users and support the encoded key value alone as opposed to having to add the whole PEM resource content.

I'll open a PR to clarify it in the spec

@sberyozkin
Copy link
Contributor

sberyozkin commented Apr 27, 2021

I'll open a PR to clarify it in the spec

OK, the spec does not explicitly allow the encoded keys without PEM headers - but in that case there is nothing to clarify for MP JWT 1.2.1 - as I said I'd be against starting failing the applications which have for example

mp.jwt.verify.public.key=MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA0440JtmhlywtkMvR6tTMs0U6e9Ja4xXj5+q+joWdT2xCHt91Ck9+5C5WOaRTco4CPFMBxoUPi1jktW5c+OyknOIACXu6grXexarFQLjsREE+dkDVrMu75f7Gb9/lC7mrVM73118wnMP2u5MOQIoXOqqC1y1gaoJaLpOjTiJGCm4uxzubzUPN5IDAFaTfK+QErhtcGeBDwWjvikGfUfX+WVq74DOoggLiGbB4jsT8iVXEm53JcoEY8nVr2ygr92TuU1+xLAGisjRSYJVe7V1tpdRG1CiyCIkqhDFfFBGhFnWlu4gKMiT0KToA9GJfOuCz67XZEAhQYizcXbn1uxaOQIDAQAB

as opposed to

mp.jwt.verify.publickey=------BEGIN PUBLIC KEY------\n\
MIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEAtL6HShqY5H4y56rsCo7VdhT9/eLQwsJpKWg66j98XsB/qc5ZxkJ25GXCzpjR0ZvzAxMNlj1hrMORaKVzz2/5axZgF1eZfzgrNyQ9rtGaBtMNAB20jLsoYp5psRTaYxKeOiLHPr3956ukSRUF9YfJGSamrvGOwC8h6zbq6uaydv+FVJXijlMD/iCggUfoirtVOWK/X1IzV7covxcGzT0X019/4RbtjLdnvqZnGqmpHQpBEItI+4gNvaKR8NDWUxAjO/v+oOKR5nEUnDWcQSCxKmyQrVJtHr9PBwWrHzTSx4k1L1hLf+AWXAdy/r6c0Lzgt5knmZTyWDG2+n8SlrXxHHxFO1Wz8H/OKBzTAf8zIuj2lkXYo+M6aoJM7qQmTys80dtYvnaHGSl+jpe2plMbS9RS4XcHH7vCqJc9acBnp9CvLgjOmA0b5Rc0WyN4sn1SDFYe6HZcVo4YGTbtTTlwgu/ozQ1x+xpTAaU0mWkHMwT0CO79rPORjhDXokEuduvtp6VUiAaoFF6Y3QQLf6O3P9p8yghpBBLb460lEQqOHQQGP0EK46cU81dlcD5lYE0TayDzb9pZZWUyjIE4ElzyW7wgI4xw7czdBalN+IhXKfGUCqIDVh7X7JpmskZMaRixf424yBcZLntEejZy59yLDSssHMc/bqnBraXuo8JBEPkCAwEAAQ==\n\
------END PUBLIC KEY------

the latter should work - but I don't want to require our users to add those headers in application.properties - it is already clear it is a public key from the name of the property.

But what I'd like to enhance - for 2.0 - is to require that implementations accept

mp.jwt.verify.public.key=MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA0440JtmhlywtkMvR6tTMs0U6e9Ja4xXj5+q+joWdT2xCHt91Ck9+5C5WOaRTco4CPFMBxoUPi1jktW5c+OyknOIACXu6grXexarFQLjsREE+dkDVrMu75f7Gb9/lC7mrVM73118wnMP2u5MOQIoXOqqC1y1gaoJaLpOjTiJGCm4uxzubzUPN5IDAFaTfK+QErhtcGeBDwWjvikGfUfX+WVq74DOoggLiGbB4jsT8iVXEm53JcoEY8nVr2ygr92TuU1+xLAGisjRSYJVe7V1tpdRG1CiyCIkqhDFfFBGhFnWlu4gKMiT0KToA9GJfOuCz67XZEAhQYizcXbn1uxaOQIDAQAB

as it is simpler for the users to do but which would indeed be a 2.0 level enhancement since 1.2 does not require it

@teddyjtorres
Copy link
Contributor

I agree with making this a 2.0 requirement.

@sberyozkin
Copy link
Contributor

@teddyjtorres Hi Teddy, I was about to start catching up on this one - but I wonder if we are here in the same boat as we were after 1.2 was out. MP JWT 2.0 does not require it so therefore we can't enforce this requirement for 2.1. I propose to delay it till 3.0

@teddyjtorres
Copy link
Contributor

@sberyozkin Hi Sergey. I agree with delaying it until 3.0.

@Kiiv
Copy link

Kiiv commented Mar 22, 2024

Hi,

I've just passed a few hours understanding why Open Liberty throw me the following error before I find this issue :

[22/03/2024 16:57:56:836 CET] 0000002b com.ibm.ws.security.mp.jwt.tai.MicroProfileJwtTAI            E CWWKS5523E: La fonction MicroProfile JWT ne peut pas authentifier la demande car le jeton qui est inclus dans la demande ne peut pas être validé. CWWKS6029E: Le jeton JWT (JSON Web Token) ne peut pas être validé car une clé de signature est introuvable. L'algorithme de signature configuré [RS256] a besoin d'une clé pour valider le jeton.

Does Open Liberty must throw an "DeploymentException" as stated in the specification instead of telling that no public key was found ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants