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

20240116 ms extensions #18

Merged
merged 7 commits into from
Jan 25, 2024
Merged

20240116 ms extensions #18

merged 7 commits into from
Jan 25, 2024

Conversation

Firstyear
Copy link
Member

Adds ms oaxpbc extensions. This is based on reading the specs and related docs https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-oapxbc/d3f8893c-a715-412a-97d1-5ffed2c3b3d5

@dmulder it would be great to be able to embed some test vectors here from real azure ad, so we probably need to have a "throw away" rsa key that we can put the private key into a test, and some captured parameters. That way we can test against how MS encode and expect things.

The big question here is how they handle the CEK in dir mode because it's not clear in the specification of JWE.

  • [ x ] cargo fmt has been run
  • [ x ] cargo test has been run and passes
  • [ x ] documentation has been updated with relevant examples (if relevant)

@Firstyear
Copy link
Member Author

Hey @frasertweedale we're hitting a problem that I'd like your advice on.

In this pr, we are adding RSA-OAEP JWE decryption support for MS JWE extensions. We added JWE RFC RSA-OAEP with OpenSSL here https://github.com/kanidm/compact-jwt/pull/18/files#diff-d0a5e525bc53fea98466a790323bc348aba1d0230375e0e44f6a5869d64685a5R142 . We validated this with the JWE RFC verification vectors which were previously merged here https://github.com/kanidm/compact-jwt/blob/main/src/crypto/rsaes_oaep.rs#L180

However, while attempted to decrypt a content encryption key generated by microsoft, we noticed that we receive openssl errors that the input data exceeds the maximum permissible size. To ensure this was not a transient or data issue, we have collected multiple keys and CEK samples ( https://github.com/kanidm/compact-jwt/pull/18/files#diff-7235468ed73e0a2be3b55bdf61539d6df9061a715652139c6cf25bb50e693a86R204 ) and all of them fail in the same way.

The initial assumption was that my RSA-OAEP implementation was incorrect in some way, so I have also written a second using a different rust-backed cryptographic ecosystem here ( https://github.com/kanidm/compact-jwt/pull/18/files#diff-d0a5e525bc53fea98466a790323bc348aba1d0230375e0e44f6a5869d64685a5R385 ). This also passes the JWE rfc verification examples, but fails identically on all of the microsoft CEK samples.

What we have noticed is that the valid encrypted RSA-OAEP messages are all 256 bytes in length, however microsoft's are 294 bytes. After some research I have found https://crypto.stackexchange.com/questions/42097/what-is-the-maximum-size-of-the-plaintext-message-for-rsa-oaep/42100#42100 and this rfc section: https://www.rfc-editor.org/rfc/rfc8017#section-7.1.1 . The summary is that the maximum encrypted message size for a 2048 bit key will always be 256 bytes, where 42 bytes of overhead exist for padding leaving 214 bytes that can be used for plaintext.

In addition, even if the MGF and hash were altered, the encrypted message will still be 256 bytes in length - it is that the hash restricts the size of the possible plaintext that you may encrypt instead.

I have been reading the MS CSharp source for their implementation and I can't see anything "obvious" that would lead to why their outputs are longer than expected. Nor does it seem to be a problem for their cryptographic ecosystem.

My next theory was that they were accidentally encoding garbage bytes in the buffer, however truncation of the value to 256 bytes yields an RSA-OAEP MGF failure.

Is there anything obvious you can see wrong in the process that I have taken here? Any ideas on next steps? Or is this an example where perhaps something is broken in the MS cryptographic provider's implementation of RSA-OAEP?

@Firstyear
Copy link
Member Author

@frasertweedale - we've solved it - it turns out MS has a bug in azure ad where they can send you an invalid JWE. We have now correctly obtained some valid vectors and our code works. So no need to investigate. :)

@Firstyear
Copy link
Member Author

@dmulder This is good to go now :)

Copy link
Collaborator

@dmulder dmulder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dmulder dmulder merged commit 420f2ea into main Jan 25, 2024
1 check passed
@dmulder dmulder deleted the 20240116-ms-extensions branch January 25, 2024 15:11
@frasertweedale
Copy link

@frasertweedale - we've solved it - it turns out MS has a bug in azure ad where they can send you an invalid JWE. We have now correctly obtained some valid vectors and our code works. So no need to investigate. :)

Hi mate, glad you got to the bottom of it.

Yes, this is the world of 800-pound gorillas failing to implement specs correctly, and (usually) not giving a single flip about fixing their broken code. Let's get a beer some time and share war stories. I bet you've accumulated a few since we last had a good yarn.

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

Successfully merging this pull request may close these issues.

None yet

3 participants