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

Change JWE Algorithm from RSA-OAEP to RSA-OAEP-256 #288

Open
rdebusscher opened this issue May 3, 2022 · 12 comments
Open

Change JWE Algorithm from RSA-OAEP to RSA-OAEP-256 #288

rdebusscher opened this issue May 3, 2022 · 12 comments
Milestone

Comments

@rdebusscher
Copy link
Member

The specification says

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

But RSA-OAEP uses SHA-1 which is considered unsafe. The spec (and the tests) should be updated to make use of RSA-OAEP-256 (makes use of SHA-256)

@sberyozkin
Copy link
Contributor

@rdebusscher I think we may need a better justification as the message digest is not what in itself is determining the overall security of the RSA-OAEP encryption. There are a few references on the web like

although the collision resistance of SHA-1 has succumbed to a series recent attacks, such as [WYY05], the security of
RSA-OAEP instantiated with SHA-1 may not be correspondingly affected as a consequence.

which you can find from https://en.wikipedia.org/wiki/Optimal_asymmetric_encryption_padding

The reason RSA-OAEP was selected by default was because it has a Recommended+ rating at https://datatracker.ietf.org/doc/html/rfc7518#section-4.3%5BRSA-OAEP%5D, where + implies that

The use of "+" in the Implementation Requirements column indicates
   that the requirement strength is likely to be increased in a future version of the specification.

while RSA-OAEP-256 is optional - I think it may be because it is not well supported yet.

The same JWA specification recommends RSA keys with 2048+ bits - so I believe the experts who wrote this spec did not consider an SHA1 element in RSA-OAEP as a critical factor.

It will be worth asking a question on the JOSE list.

I don't mind too much about RSA-OAEP-256 but I wonder if a better option is to start with adding
mp.jwt.decryption.algorithm to let users choose between RSA-OAEP and RSA-OAEP-256 similarly to mp.jwt.verify.publickey.algorithm which can be used to select between RS256 and ES256. We have such a smallrye-jwt specific property.

@sberyozkin sberyozkin added this to the MPJWT-3.0 milestone May 5, 2022
@sberyozkin
Copy link
Contributor

@rdebusscher Yes, IMHO, adding mp.jwt.decryption.algorithm and requiring to support both RSA-OAEP and RSA-OAEP-256 would be not a bad approach. We can keep RSA-OAEP a default for now - it may let me add this property for 2.1, if time will allow, as I've just released 2.1.RC1

@sberyozkin
Copy link
Contributor

For later major versions we can consider making mp.jwt.decryption.algorithm=RSA-OAEP-256 be a default

@sberyozkin
Copy link
Contributor

CC @teddyjtorres

@teddyjtorres
Copy link
Contributor

I agree with adding a new property for the key management key algorithm.

@sberyozkin
Copy link
Contributor

sberyozkin commented May 5, 2022

@rdebusscher @teddyjtorres I'll keep this issue open to discuss mp.jwt.decryption.key.algorithm=RSA-OAEP-256 as a default for MP JWT 3.0 as it is a dedicated requirement which also breaks compatibility, I'll open a new issue specifically for MP JWT 2.1 to add mp.jwt.decryption.key.algorithm, with RSA-OAEP as default, and with an extra RSA-OAEP-256 support requirement.

@rdebusscher
Copy link
Member Author

@sberyozkin thank you for the references, but some remarks

with SHA-1 may not be correspondingly affected as a consequence

The word may suggest it is not to be excluded that a problem can arise.

The RFC 7518 you linked is dated May 2015, thus before the announcement in 2017 that SHA-1 is to be considered unsafe (https://security.googleblog.com/2017/02/announcing-first-sha1-collision.html)

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

@rdebusscher
Copy link
Member Author

The reason why I opened this is the following.

I'm currently writing a new implementation of JWT-Auth spec and can't pass the current TCK since it is mandatory to support RSA-OAEP (the RolesAllowedSignEncryptTest TCK test makes use of it also). It will be used in an environment where RSA-OAEP-256 is used. Users are also making the remark why an 'old' algorithm is required and you can't make use of newer algorithms (RSA-OAEP-256 already dates from 2017)

Since Log4Shell, people avoid anything that is associated with a potential security vulnerability (a valid one or an assumed one)

@sberyozkin
Copy link
Contributor

sberyozkin commented May 6, 2022

@rdebusscher I've added a comment at #289 to explain why depending on the headers alone is not a good idea.

can't pass the current TCK since it is mandatory to support RSA-OAEP

Can you explain why you can't pass it ? Any JOSE library supports it.

Users are also making the remark why an 'old' algorithm is required and you can't make use of newer algorithms (RSA-OAEP-256 already dates from 2017)

We'll add this property and your users will be able to enforce RSA-OAEP-256, no problems

@sberyozkin
Copy link
Contributor

@rdebusscher

Since Log4Shell, people avoid anything that is associated with a potential security vulnerability (a valid one or an assumed one)

I appreciate it and I don't take it lightly. But we can't just dismiss RSA-OAEP because of some use of SHA-1 in the bigger encryption process, just because SHA-1 on its own is unsafe. RSA-OAEP is not SHA-1. We'll need a better justification to drop it, with some reference to at least some public expert analysis why it is unsafe - I don't see any such analysis.

We can't drop it in 2.1 anyway - recall our conversation related to RSA keys < 2048 bits - they are still supported despite them explicitly discouraged in the specs - because of your concern removing them would break a backward compatibility - I'm only referring to it because it is the same situation as far as the backward compatibility is concerned.

@sberyozkin
Copy link
Contributor

sberyozkin commented May 6, 2022

@rdebusscher If the TCK issue is related to the fact that you'd like to certify MP JWT 2.0 implementation in the environment where implementing RSA-OAEP is technically impossible then I guess we should be open to updating TCK itself to use RSA-OAEP-256 in MP JWT 2.1, by adding mp.jwt.decrypt.key.algorithm=RSA-OAEP-256 - while as I said in MP JWT 3.0 it can become a default. But please confirm it is the case

@sberyozkin
Copy link
Contributor

@rdebusscher I've asked a question about RSA-OAEP in the JOSE forum, I've got an answer from a well-known expert saying that he was not aware of any SHA-1 weaknesses in scope of RSA-OAEP but also assumed it could be because no one is looking into trying to find such a weakness - which is a fair comment but obviously it is just a thought.
This is only FYI, I support your request to allow RSA-OAEP-256 and we'll make it a default for 3.0

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

3 participants