Add `mp.jwt.decrypt.key.algorithm` property

[sberyozkin]

Add mp.jwt.decrypt.key.location property to let users choose the algorithm which should be used to decrypt the content encryption key. RSA-OAEP will remain a default value for 2.1 to maintain the backward compatibility, but RSA-OAEP-256 will also be required to be supported (might become a new default value in one of the next major releases, see #288), support for other key management algorithms will be optional.

[rdebusscher]

I don't get the reason why a new config property is needed since the JWE algorithm comes from the header of the token.

[sberyozkin]

@rdebusscher Accepting the algorithm requirements from a client is indeed considered to be a security risk. While, in general, in JOSE, one can just dynamically determine everything, whether to decrypt first, how to decrypt, how to verify, it is not what a real production should do where the concrete security policies, keys are in place. There was a known issue related to a case where a server was blindly accepting the algorithm in the token headers and was tricked into verifying the signature as a hash as opposed to RS signature or something like that...

This was a rationale for the way MP JWT 1.2 spec was written (i.e, don't decrypt just because 5 dots are in the sequence, decrypt only if the decrypt key location is used, don't do EC256 verification just because the header says so).

The idea behind such a property is to encourage the users to expect the security requirements related to the decryption. Same as it is already done for the signature verification, so the consistency is important.

Also we can't just replace RSA-OAEP with RSA-OAEP-256 and start rejecting RSA-OAEP, as I said, SHA-1 is not equal to RSA-OAEP therefore saying RSA-OAEP is not secure is not correct. So this header will let us retain the backward compatibilty and introduce RSA-OAEP-256 and other optional algorithms.

[rdebusscher]

mp.jwt.decrypt.key.algorithm must be a list of all JWE algorithms (RSA-OAEP, RSA-OAEP-256, RSA-OAEP-384, RSA-OAEP-512). Currently, all algorithms can be used and we can't exclude one with this property by default as that would be a breaking change.

So this header will let us retain the backward compatibilty and introduce RSA-OAEP-256 and other optional algorithms.

Spec already says that the other algorithms are optional but that means they are allowed and RSA-OAEP is not the only one that is allowed.

This property is fine if you as a developer want to restrict the value from the header to one or more algorithms instead of accepting all JWE algorithms.

[sberyozkin]

No, there must be an exact algorithm requirement at the spec level, you can have your implementation to accept whatever the client posts if you'd like

[sberyozkin]

@rdebusscher

Currently, all algorithms can be used and we can't exclude one with this property by default as that would be a breaking change.

Can you point to the spec text confirming it please ?

[sberyozkin]

@rdebusscher Rudy, here is a text:

Key management key algorithm which must be supported is [RSA-OAEP](https://tools.ietf.org/html/rfc7518#section-4.3) (RSAES using Optimal Asymmetric Encryption Padding) with a key length 2048 bits or higher.

The rest is up to implementations. Please note, this property will only add an extra requirement for RSA-OAEP-256, everything else will stay optional. It won't be restricting to only these 2 algorithms in a sense that the application won't be able to use some other algorithm (by the way where did you see RSA-OAEP-384/512 defined ?), this property will require that the implementations support at least these 2 algorithms.

The goal is to do it for 2.1 and to let your users set RSA-OAEP-256 by default

[sberyozkin]

It will be structured similarly to https://github.com/eclipse/microprofile-jwt-auth/blob/master/spec/src/main/asciidoc/configuration.asciidoc#mpjwtverifypublickeyalgorithm

[rdebusscher]

I did not interpret the spec as you do, and there are no TCK tests to backup the claim.

So you say that RSA-OAEP is the only allowed algorithm that can be handled but that all other algorithms, RSA-OAEP-256, -384, and -512) are forbidden and are not allowed to use (as there is no indication in the spec text that other JWE algorithms are allowed by the implementation, only a required one). But that means that, probably all of them, currently certified implementations fail this certification requirement (as they allow RSA-OAEP-256 for example)

If you say that other JWE algorithms can optionally be supported by implementations (already in version 2.0) then adding mp.jwt.decrypt.key.algorithm=RSA-OAEP by default, will break the application of the user when they upgrade and using RSA-OAEP-256 (since RSA-OAEP-256 is no longer allowed unless they specify a value for the new configuration parameter)

[sberyozkin]

@rdebusscher Can you please avoid referring to non-existent RSA-OAEP-384 and -512 ?

If you say that other JWE algorithms can optionally be supported by implementations (already in version 2.0) then adding mp.jwt.decrypt.key.algorithm=RSA-OAEP by default, will break the application of the user when they upgrade and using RSA-OAEP-256 (since RSA-OAEP-256 is no longer allowed unless they specify a value for the new configuration parameter)

How would such an implementation do it without having an implementation specific property ? By checking the JWE headers ? But that is not a sound security approach - the application should expect what is coming.

I think because the spec is not referring specifically to any other algorithms, it means it is an implementation specific decision to support some other algorithm - nothing to do with the spec requirements. So introducing this property at the spec level does not change it. This property will require that the implementations support both RSA-OAEP and RSA-OAEP-256 and will be set by default to RSA-OAEP. Your implementation can have it defaulted to RSA-OAEP-256 for the users not to worry about setting it themselves.

I think it will not be a breaking change

[sberyozkin]

@rdebusscher We can also consider supporting something like ALL as a value of this property - it basically will let users say that any algorithm set in JWE headers is supported. That should allow for more open-ended configuration. That said - I'd be worried it would add an unnecessary risk. Implementations can choose to do it with their own specific properties

[rdebusscher]

Can you please avoid referring to non-existent RSA-OAEP-384 and -512 ?

They do exists. you can use any Hash algorithm of the SHA2 algorithm. Here are some references where it is already supported:

https://docs.microsoft.com/en-us/dotnet/api/system.security.cryptography.rsaencryptionpadding.oaepsha384?view=net-6.0

https://connect2id.com/blog/connect2id-server-12-4

https://cryptography.io/en/latest/development/custom-vectors/rsa-oaep-sha2/

https://www.openscdp.org/sse4e/crypto.html

[sberyozkin]

@rdebusscher I see, yes, I'm keeping referring to the outdated JWA spec

[rdebusscher]

ALL as a value of this property - it basically will let users say that any algorithm set in JWE headers is supported.

I agree it should not just be anything, but the list should be definable and indeed at best at the implementation level. So spec should not be s restrictive that it only allows 1 or 2 of the algorithms.

So having the default mp.jwt.decrypt.key.location=RSA-OAEP, RSA-OAEP-256, RSA-OAEP-384, RSA-OAEP-512 makes a compromise that probably 99.9% is supported by default, and if needed, can be restricted to 1 single algorithm. Companies that take security seriously will limit it to 1 as you say.

[sberyozkin]

@rdebusscher RSA-OAEP-384, RSA-OAEP-512 are not supported by all JOSE libraries yet. So lets limit it to RSA-OAEP, RSA-OAEP-256 for 2.1, I'll work on a PR and ask you and @teddyjtorres to review and I'm sure we'll workout the text which works. In 3.0 we can push it further I guess.

[sberyozkin]

@rdebusscher Hi, I've thought more about it, unfortunately I don't think making this property a list property works.

The problem is, I don't believe having a single key to handle more than one algorithm is sound or secure. In MP JWT we can point to a JWK set which may have more than one key but single PEM keys won't really do. It might be possible technically but I don't think a practical scenario exists where a given issuer uses the same key to encrypt with RSA-OAEP and RSA-OAEP-256 - it makes no sense, it will only raise concerns and won't be allowed in production.

IMHO the proper solution at the spec level to cases where we have to support multiple keys, algorithms, etc has to be produced by fixing #239. Specific implementations can support more than one algorithm at the same time if they wish to.

I think what was proposed here initially works. The spec will require the implementations to support RSA-OAEP-256 in addition to RSA-OAEP. mp.jwt.decrypt.key.algorithm will be set to RSA-OAEP by default to maintain a backward compatibility with a note that a default will be RSA-OAEP-256 in one of the next major releases. MP JWT 2.1 users be able to set mp.jwt.decrypt.key.algorithm to RSA-OAEP-256.

IMHO your earlier comment that it will break those applications which already depend on RSA-OAEP-256 by default is not correct. The spec only refers to RSA-OAEP right now and says not a single word about other algorithms. So as far as the spec is concerned it is not a breaking change. Also, if a given implementation already does RSA-OAEP-256 only then it is already non compliant since it won't pass TCK. If it does accept both RSA-OAEP and RSA-OAEP-256 and selects RSA-OAEP-256 with a custom property then again it will only move to a standard property. And as we've discussed and I believe also agreed to is that taking the header value only is not safe.

Also CC @teddyjtorres

[sberyozkin]

@rdebusscher I think we will be able to transition it over time into a list property without breaking anything if it will be necessary for practical cases

[dblevins]

I agree with @rdebusscher, this should be a list.

[dblevins]

For my benefit, @rdebusscher and @sberyozkin did you two come to some agreement outside this thread, perhaps on a call, or was this simply closed despite there being disagreement?

[rdebusscher]

No discussion outside this thread and no agreement. But I'm no longer trying to make the implementation pass the TCK as I need multi-tenancy/multi-source and don't want to implement some of the unnecessary restrictions that MP JWT imposes on top of the JOSE Specs.

[sberyozkin]

@dblevins This issue was thoroughly discussed and while @rdebusscher did disagree I assured him we would be able to convert it to list should a practical requirement arise. @rdebusscher I'm always taking your comments on board so I'm finding it disappointing that your last comment presents it as if I just closed without listening to you. We have also discussed introducing this property with @teddyjtorres on 5th May on the call, where I have never seen neither @dblevins nor @rdebusscher. So, if you'd like 2 out of 3 people who participated did agree.

@dblevins , @rdebusscher Let me remind you that MP JWT is currently single tenancy as far as its configuration is concerned. Introducing adhoc multitenancy is not the right approach, @rdebusscher, I've linked you to the existing MP JWT issue which will deal with it. I can assure both of you you won't find a practical case where a single issuer uses multiple algorithms at the same time, and yet as you know MP JWT can only have a single issuer.

Proper multitenancy support is the way to go.

@dblevins I'm closing this issue again, you are welcome to challenge but I'd appreciate if, before reopening, you'd review the discussion and offer technical justification as opposed to reopening because you've noticed some sort of disagreement taking place.

@dblevins @rdebusscher Perhaps even a better idea would be to start a public discussion on the MP list, I'd encourage you to do it and I'll contribute to it.

Thanks

[rdebusscher]

There was never a discussion on a call proposed and I was not aware of any calls that are happening.

I can't find the mailing list, can we add it to the Github Readme or Wiki page?

[sberyozkin]

@rdebusscher Hi, it is all in the MP calendar; link to the call notes is in this repository's README.

We can also do Discussions.

But in any case, let me repeat, please don't be concerned about the current restrictions. MP JWT will be capable of supporting the practical requirements. IMHO we'd just need to avoid solving the multi-tenancy problem in the adhoc way starting from this property.

And you are always welcome to challenge things.

[jblazek]

Without supporting a list of algorithms, how should a consumer avoid downtime when a token issuer rotates keys and the new key uses a new algorithm?

We have this situation where the token issuer is migrating to a new algorithm and using the location property as JWKS endpoint, but with only allowing one algorithm it means the system doing JWT validation needs to be reconfigured in a coordinated manner with a new key rollout.