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

Hash-Algorithm for key derivation (ECDH-ES) #580

Open
inf9144 opened this issue Dec 11, 2023 · 2 comments
Open

Hash-Algorithm for key derivation (ECDH-ES) #580

inf9144 opened this issue Dec 11, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@inf9144
Copy link

inf9144 commented Dec 11, 2023

Hey,
i tried your library and the one from Microsoft (Microsoft.IdentityModel.JsonWebTokens) and the interoperability between both.
https://datatracker.ietf.org/doc/html/rfc7518#section-4.6.2 seems to state that the Hash should be calculated with SHA256

In Microsoft code it looks like this:

// JWA's spec https://datatracker.ietf.org/doc/html/rfc7518#section-4.6.2 specifies SHA256, saml might be different
byte[] derivedKey = _ecdhPrivate.DeriveKeyFromHash(_ecdhPublic.PublicKey, HashAlgorithmName.SHA256, prepend, append);

In your code the hash algorithm is defined through it's encryption algorithm:

_hashAlgorithm = GetHashAlgorithm(encryptionAlgorithm);
...
 var hashAlgorithm = encryptionAlgorithm.SignatureAlgorithm.HashAlgorithm
...
exchangeHash = new ReadOnlySpan<byte>(ephemeralKey.DeriveKeyFromHash(otherPartyKey.PublicKey, _hashAlgorithm, _secretPreprend, secretAppend), 0, _keySizeInBytes);>

If you now use a combination of EcdhEsA128kw and Aes128CbcHmacSha256 it works because here SHA256 is used.
But if you use a combination of EcdhEsA256kw and Aes256CbcHmacSha512 the tokens from the MS-Lib and the tokens of your lib cannot be understood by the other party, because you would use SHA512 in that case.

Who is right - who is wrong? I dont have clue but hope you have an answer to this because i would like to connect two applications using different frameworks ;-)

@ycrumeyrolle
Copy link
Collaborator

https://datatracker.ietf.org/doc/html/rfc7518#section-4.6.2 states that

the Digest Method is SHA-256.

So it looks this is a bug. I will investigate on next week.

@ycrumeyrolle ycrumeyrolle added the bug Something isn't working label Dec 11, 2023
ycrumeyrolle added a commit that referenced this issue Apr 21, 2024
Force SHA256 as hash algorithm for EC key wrapping.
As a result, the encryption algorithms that require a key greater than 32 bytes have now the remaining with zero-ed bytes
@ycrumeyrolle
Copy link
Collaborator

@inf9144 PR #582 try to fix this issue, but there is a drawback.

When generating the ephemeral key, now the hash algorithm is forced to SHA2-256.
This is fine for encryption algorithms like A128CBC-HS256 which require a key of 256 bits.
For A256CBC-HS512 we generate a key with trailing zero.

@inf9144 do you know if it is the same with MSAL ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants