Can no longer use issuer to help locate key from 0.12.0
laurids opened this issue · 16 comments
Is your feature request related to a problem? Please describe.
From 0.12.0, in the JwtParserBuilder, the setSigningKeyResolver is deprecated, but the replacement keyLocator uses Locator and its locate method only takes a Header, compared to the old SigningKeyResolver.resolveSigningKey that took both JwsHeader and Claims. We were using the issuer ('iss') from Claims along with the 'kid' from the header to look up the signing key. We have multiple authorization servers, and the key identifiers ('kid') are not guaranteed to be unique across different authorization servers.
Describe the solution you'd like
That Locator.locate takes both Header and Claims.
Describe alternatives you've considered
We could look up the signing key only based on the key identifier and hope that it's unique or try all of them if multiple are found, but this is not desirable. The only other option that I can see, if we do not have the authorization from anywhere else in the incoming request, is to parse the JWT ourselves to find the 'iss' value before calling the JJWT library. Or demand of the incoming JWTs that they include something like certificate thumbprints which are probably unique even across authorization servers. But this is would probably be a high demand since they are optional in the specs.
It seems like the header would have been a better place for the 'iss' parameter - but we cannot change the specs:)
Additional context
You have mentioned in a comment in DefaultJwtParser that it is safer to verify the signature before reading the payload. It does seem like the correct thing to do (if it wasn't for our key location conundrum), but can I ask what your specific arguments for this is?
I have the same issue.
Hi @bdemers
Potentially every national IdP in the european union - some of which have multiple, so it could be upwards of 50.
Right, I know that de-serializing can be a dangerous operation, a lot of vulnerabilities have been discovered in that area in the past.
However, you still de-serialize the header before verifying the signature. If you are referring to the exploitation of potential de-serializing vulnerabilities, couldn't they just be exploited through the header? So is there really much "safety gained" by waiting to parse the payload until after signature verification?
Hi @laurids ! I'll try to add a little clarity if I can.
Right, I know that de-serializing can be a dangerous operation, a lot of vulnerabilities have been discovered in that area in the past.
Exactly. Reducing risk exposure is a good thing, so the less exposure, the better. No need to double the exposure area.
However, you still de-serialize the header before verifying the signature. If you are referring to the exploitation of potential de-serializing vulnerabilities, couldn't they just be exploited through the header? So is there really much "safety gained" by waiting to parse the payload until after signature verification?
Absolutely. Per the RFC specifications, JWS content is not guaranteed to be JSON, and if it is JSON, it's not even guaranteed to be identity claims. A JWS (or JWE) payload can be any arbitrary 'octet sequence' (i.e. byte[]). That octet sequence can contain a whole mess of security problems if the signature is not verified before deserializing the octets, not the least of which are DoS attacks, decompression DoS, data substitution attacks, etc.
An ancillary way of looking at this issue: what happens if you need to handle JWEs from these IdPs? The JWE payload is fully encrypted and it's impossible to even 'peek' inside of it until a key is resolved via the header. The security design case is identical for JWS, the only difference is that you can 'peek' inside the payload.
This is why the IdPs must put whatever key lookup information is required in the header. It's also why kid is and always has been a header parameter, not a claim.
It's also why the JWK Thumbprint RFC exists (which JJWT supports 😉 ) - to generate canonical guaranteed-unique IDs for keys that can then be referenced in headers by any IdP or service. X509 Certificate Thumbprints are another way.
Yet another way is to repeat necessary key lookup that may be in the claims in the header. The RFCs call this out as a 'backup' approach if necessary.
Based on all of that, the RFC committee has provided many options for JWT creators to identify keys in headers, specifically because JWT recipients are not supposed to be deserializing the payload before verifying authenticity.
Finally, I'll reference the JWS RFC on this process: they list the steps for verifying a JWS, and those steps explicitly omit parsing the payload (they only require ensuring the payload is valid Base64URL, and even then, not required to be done before the signature verification):
https://www.rfc-editor.org/rfc/rfc7515.html#section-5.2
Again, IMO, the easiest way to look at this: if the IdP sends you a JWE, what do you do? The answer should be the same for JWS.
Hi @lhazlewood, thank you for the response.
For the sake of argument, even the header of a malicious token might not follow the RFC specifications of a JWT header. So it could contain the same stuff as the payload. In theory, you could mitigate the risk by using a more restricted deserializer for parsing the header than for the payload, but the same one is currently used as far as I can read.
For JWEs, you are right, you only have the header information available. I agree that it seems logical that this should also be the case in general.
However, we are following RFC9068 in particular, and section 4 actually says to verify the 'iss' value (point 3) before verifying the signature (point 5).
Would you consider the possibility of doing "just" a JSON parsing of the payload first without deserializing other content than 'iss' and 'aud'?
We are considering our options right now in case JJWT will not be providing 'iss' for key lookup. Whether to peek in the payload, or make a database constraint requiring 'kid' to be unique across issuers.
Do you have any idea how many IdPs or authorization servers in practice use 'kid' values that can be considered globally unique? Such as the key thumbprint format.
@laurids RFC9068 Section 4 uses a a bulleted list, not a numerical/ordinal ordered list - to the best of my understanding, when order is required, ordered numerical lists must be used by RFC authors, and when order is not mandatory, bulleted lists are used.
Even so, even if they implied order, I find it interesting that the JWE use case is bullet #2, which means it's impossible to validate the iss (bullet 3) until after JWE key lookup and decryption is finished anyway.
Would you consider the possibility of doing "just" a JSON parsing of the payload first without deserializing other content than 'iss' and 'aud'?
This isn't possible within JJWT: JJWT delegates the base64url-decoding InputStream to the JSON processor (e.g. Jackson) - the entire object is deserialized by Jackson before JJWT receives it as a Map<String,?>, and then JJWT evaluates each name/value pair.
We are considering our options right now in case JJWT will not be providing 'iss' for key lookup. Whether to peek in the payload, or make a database constraint requiring 'kid' to be unique across issuers.
I'm considering options for JJWT too 😉, trying to think how to provide support for this while still being as secure as possible. One potential approach is to change the Locator from this:
public interface Locator<T> {
T locate(Header);
}to something like this:
public interface KeyLocator {
LocatedKey locate(LocateKeyRequest);
}where LocateKeyRequest could have:
- the
Header, - the
TokenizedJw(t|s|e)instance that contains all parsed Base64URL tokens - anything else that I can't think of right now that could be useful 😅
And then, if the claims are required for parsing, the locator implementation can Base64Url-decode the payload at their own risk. If they go through that effort, the resulting Map can be put in the LocatedKey result/return value so JJWT doesn't have to duplicate that effort.
This is one such idea. But then again, if all this is required, maybe JJWT can do it automatically if something on the parser builder is configured, say, Jwts.parser()...unsecuredClaims()... and then JJWT can do this work automatically.
It is just my opinion, but I must admit this feels like a a pretty distasteful workaround given the security implications. It just feels like IdP implementors are either wrong or ignorant (in the sense of lack of information, not in a derogatory sense) for not repeating this information in the header if it's important for key lookup. Or maybe they don't care because for IdP tokens - which are only a subset of JWS use cases - they claims are small and simple, just like headers, and they don't care as much because "the signature will be validated anyway.". If that is the case, they're completely ignoring the potential for abuse to use/evaluate claims that haven't yet been validated, because application developers will do this, somewhere, and likely unknowingly - it's pretty much guaranteed. It's poor design to allow that to happen IMO.
Do you have any idea how many IdPs or authorization servers in practice use 'kid' values that can be considered globally unique? Such as the key thumbprint format.
Unfortunately I don't have any statistics for that. But the JWK Thumbprint RFC was finalized in September 2015, so it's been around for 8 years now, long enough to garner adoption. Plus X509 Certificate Thumbprints have been available since the JWS RFC was finalized 4 months earlier in May 2015.
or make a database constraint requiring 'kid' to be unique across issuers.
This feels a lot safer IMO. It'd be very odd to have two IdPs use the exact same kid value. Even if they don't use JWK or X509 Certificate Thumprints, it would probably be something like a UUID.
I suppose it could be a monotonically increasing integer (e.g. database primary key), but I'd be awfully suspicious of using an IdP that exposes those kinds of internal details (doesn't give me much confidence in the safety/security/design of that particular IdP).
A colleague of mine had some good points.
First, for JWEs, the key (private key) is owned by the resource server, so in that case the resource server would presumably not have any problems with uniquely locating keys.
Secondly, what if you need to support retrieving the JWS public key from the authorization server, instead of registering it beforehand? We might need to support that in the future. There are some advantages with that approach such as supporting that the authorization server can dynamically roll keys without the overhead of a resource server administrator needing to import the new public keys.
In that case we would need the URI from 'iss' to know which endpoint to contact to retrieve the public key.
See the jwks_uri endpoint in RFC8414.
It really seems like 'iss' should have been part of the header.
In regards to your possible solution suggestions, I think passing the base64 tokens to the locate method seems like extra work for the caller and also a bit complex if the library needs to use the result of the callers own parsing implementation. Or the caller should get passed the library's deserializer method as well?
My immediate thought is that I like a simple flag, or unsecuredClaims, though not crazy about the name😅, which would then make the library parse the payload before calling locate and passing the Claims like in the previous version. There should probably be a separate method on the Locator interface to accommodate this, instead of Claims just being null if the flag was not set?
We appreciate your input.
A colleague of mine had some good points.
First, for JWEs, the key (private key) is owned by the resource server, so in that case the resource server would presumably not have any problems with uniquely locating keys.
If you mean they don't need to worry about correlating a key id with an issuer, I wouldn't be so sure - it definitely depends on how they implement their service. It is fairly common for IdPs to receive JWTs from 3rd parties (Google, Microsoft, etc login services), so they still need a way to distinguish keys from those 3rd parties vs their own.
Secondly, what if you need to support retrieving the JWS public key from the authorization server, instead of registering it beforehand?
If I understand the question correctly, that is the purpose of the jku, jwk, x5u and x5c header parameters.
But even when those header parameters are present, the URIs associated with each should be validated before executing a request, preferably against a whitelist of allowed/expected IdP URIs, i.e. you don't want to execute a request to a URI/URL found in a JWT unless, ideally, you know ahead of time you expect to send a request to that exact URL, or at least a 'base' URL. Otherwise your application could be leveraged for a coordinated DoS attack if it blindly calls out to any URL.
We might need to support that in the future. There are some advantages with that approach such as supporting that the authorization server can dynamically roll keys without the overhead of a resource server administrator needing to import the new public keys.
Exactly.
In that case we would need the URI from 'iss' to know which endpoint to contact to retrieve the public key. See the
jwks_uriendpoint in RFC8414. It really seems like 'iss' should have been part of the header.
That's what jku is for - it's technically not necessary to have another source (like jwks_uri) that accomplishes the same thing.
RFC 8414 (and jwks_uri) exists to provide additional conveniences: An application can send a request to the metadata URL a single time, get all the information it cares about dynamically, and then works automatically from there. In other words, instead of configuring all the associated metadata manually in the application's configuration, you have to configure a single metadata URL and inspect whatever comes back. Additionally, jku doesn't have to be repeated in every JWT, allowing for smaller JWTs. But this is a nice to have, external to the JWT RFC specifications.
In any case, it is definitely the responsibility of the IdP that supports the the metadata endpoint (and jwks_uri endpoint) to provide collision-resistant key identifiers (or other identifiable information) in the header.
If the kid value is not collision-resistant (e.g. a monotonically increasing integer), something else needs to be in the header to ensure it is collision-resistant. For example, having a jku value and a kid value, you know that kid always applies to keys retrieved from that URI, and those together guarantee a collision-resistant (cache) identifier.
In regards to your possible solution suggestions, I think passing the base64 tokens to the
locatemethod seems like extra work for the caller and also a bit complex if the library needs to use the result of the callers own parsing implementation. Or the caller should get passed the library's deserializer method as well?
Yeah, I was thinking out loud, that seems entirely too messy 😅 .
My immediate thought is that I like a simple flag, or
unsecuredClaims, though not crazy about the name😅, which would then make the library parse the payload before callinglocateand passing theClaimslike in the previous version. There should probably be a separate method on theLocatorinterface to accommodate this, instead ofClaimsjust being null if the flag was not set?
I'm still thinking through this, but the flag may be the solution. It just feels 🤮. IdPs should be using the tools the RFC authors gave them to uniquely identify keys (uris, thumbprints, etc) instead of forcing consumers to take on additional risk.
Good point about the 'jku' parameter!
We have not yet looked much at the requirements for retrieving keys dynamically, since we will not need it for now. Thanks for pointing us in the right direction when we get there:) Really appreciate your input.
But even when those header parameters are present, the URIs associated with each should be validated before executing a request, preferably against a whitelist of allowed/expected IdP URIs, i.e. you don't want to execute a request to a URI/URL found in a JWT unless, ideally, you know ahead of time you expect to send a request to that exact URL, or at least a 'base' URL. Otherwise your application could be leveraged for a coordinated DoS attack if it blindly calls out to any URL.
Yes, we would definitely have the issuer (jku) URI registered in advance and check it before sending a request. Also, we would use TLS as the spec requires, to prevent e.g. man-in-the-middle, so we would need to have at least the authorization servers TLS root ca certificate preregistered.
Even though we agree with you that the 'kid's should be unique, we do consider it a risk putting in a hard uniqueness constraint. The trouble is that we do not know in advance exactly which authorization servers will be used, so we cannot inquire about their practices. We have to make a solution that is generic enough to support as many as possible.
I have an idea for a "peeking" solution that might be a bit simpler: A separate locator interface, e.g. UnvalidatedPayloadLocator that extends from Locator, and then the JwtParser just needs to check the type of keyLocator to determine whether to verify the signature before or after parsing the payload. If the parse code in DefaultJwtParser got split into some shorter methods, which would be good for readability anyway, I think it would only require a relatively small change to support this.
Even though we agree with you that the 'kid's should be unique, we do consider it a risk putting in a hard uniqueness constraint. The trouble is that we do not know in advance exactly which authorization servers will be used, so we cannot inquire about their practices. We have to make a solution that is generic enough to support as many as possible.
I think this is where we might be experiencing some friction 😅.
It sounds as if you're trying to solve for a problem that might happen in the case of a potential/future poorly-behaving IdP, not what is currently happening, as opposed to, when integrating and testing with a new IdP, if you find a constraint violation, communicating with the IdP to let them know they're the cause of the problem (and allowing them to update their implementation so none of their other customers/consumers have these problems either).
Or, when you interact with said IdP, it might very well be automatically solved with additional information they already provide in the header (even just repeating iss in the header would work, but other solutions like jku and others are likely to work already).
Based on this, it doesn't feel right to me (at the moment) to relax security verification logic on a 'just in case' scenario, and without other JJWT users providing scenarios where header inspection absolutely will not work.
I have an idea for a "peeking" solution that might be a bit simpler: A separate locator interface, e.g.
UnvalidatedPayloadLocatorthat extends fromLocator, and then theJwtParserjust needs to check the type ofkeyLocatorto determine whether to verify the signature before or after parsing the payload. If theparsecode inDefaultJwtParsergot split into some shorter methods, which would be good for readability anyway, I think it would only require a relatively small change to support this.
I'd probably call it UnverifiedClaimsLocator to match crypto taxonomy (verify signatures), and only when the payload is a Claims instance, but that's beside the point. Your suggestion is a good one if the feature is mandatory; I'd like to wait to see if this is indeed mandatory: intentionally relaxing security verification really needs to be calculated and deliberate based on real/existing use cases that are not simply trying to circumvent security measures in the name of convenience.
I'm going to reach out to the RFC spec committee to see if they can give me some clarification.
I very carefully read RFC 7515, Section 5.2 again; that explicitly lists in ordinal/enumerated fashion the steps in verifying a signature.
Step 2 indicates the header must be valid Base64URL.
Step 3 indicates the header must be a valid JSON object - i.e. it's been Base64URL decoded and then actually parsed into a JSON object.
Later, Step 6 indicates the payload (what could be claims) must be valid Base64URL.
However, there is no step after 6 that indicates the payload should be parsed into a JSON object.
That they omitted this for the payload (but not for the header) seems crystal clear to me that signature verification is done before claims parsing.
I'll reach out to the RFC spec committee to see if I can get any additional clarification.
Hello again.
Good answer as always.
I think we have reached the conclusion that we will put in a database constraint to ensure uniqueness. That way, it is at least already at the time the users (admins) import the authorization server key that they will get an error, and not at the time when users actually use the system. Now, we have some good arguments to push back on poorly behaving authorization servers:)
We are still curious to hear the answer from the RFC committee to your inquiry.
If the answer is as expected that:
- You should verify the signature before deserializing the payload (as RFC 7515 says, and which makes the most sense).
- JWT issuers should use unique key identifiers.
Then you can go ahead an close the issue.
@laurids I received word back from a couple members on the working group, and the consensus is, essentially (me paraphrasing) "ideally the signature should be verified before parsing the payload due to the abuses that could occur otherwise. It's unfortunate that the RFC doesn't make this clear, and it's unlikely that the RFC will be changed - it is what it is. If possible, it may be good to update https://www.rfc-editor.org/rfc/rfc8725 to update such a recommendation"
JWT issuers should use uniquely identifying information in the header. Whether that's a (what the RFC calls a) "collision-resistent" kid value, or if non-collision-resistant, other information in the header should be non-ambiguous. For example, an IdP may not guarantee a globally-unique kid, but they may use a jku or similar, and that URI (or prefix, e.g. https://someIdP.com/...) together should ensure collision-resistant identification.
So I think it makes sense to close this for now. I'm definitely open to re-opening it in the future if it's just impossible to workaround for a decent number of JJWT users.
@laurids just an update, this is now being addressed via IETF channels as https://datatracker.ietf.org/doc/draft-tschofenig-jose-cose-guidance/
Thanks @lhazlewood that's very interesting, it will be nice to have a best practice guidance!