Wrong decryption key for ECDH with keywrap

[max4t]

Hello,

I believe there's an error in the implementation.

azure-activedirectory-identitymodel-extensions-for-dotnet/src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.cs

Lines 982 to 986 in 59ec62e

	// on decryption we get the public key from the EPK value see: https://datatracker.ietf.org/doc/html/rfc7518#appendix-C
	var ecdhKeyExchangeProvider = new EcdhKeyExchangeProvider(
	key as ECDsaSecurityKey,
	validationParameters.TokenDecryptionKey as ECDsaSecurityKey,
	jwtToken.Alg,

The second argument of EcdhKeyExchangeProvider should be the public key taken from the epk in the token's header (as the comment said).

[brentschmaltz]

Maxime Toumi (@max4t) thanks for reporting this issue.
We will check it out.

[GregDomzalski]

I'm also running into this issue and agree with Maxime Toumi (@max4t)

In ECDH - the TokenDecryptionKey should be your private key. The public key needs to resolve to EPK.

There are other issues in this code as well. If there are multiple keys resolved and we drop to the "brute force" approach of trying them all, the exception messages are appended exceptionStrings. But, if we find a valid key that results in unwrappedKeys.Count > 0 we still fail.

It's hard to guess the original intent here. On the one hand, brute forcing multiple keys was definitely noted in the comments. However, the if statement checking the exceptionStrings.Length also seems deliberate, albeit incorrect.

FWIW, I was able to hack around these issues and get ECDH unwrap working using the following:

var jwt = new JsonWebToken(encodedToken);

// Work around EPK not being resolved
var epk = new JsonWebKey(jwt.GetHeaderValue<string>(JwtHeaderParameterNames.Epk));

// Code expects both keys to be ECDsaSecurityKeys and not JsonWebKeys
var ecdsa = ECDsa.Create(new ECParameters()
{
    Curve = Constants.Curve,
    Q = new ECPoint()
    {
        X = WebEncoders.Base64UrlDecode(epk.X),
        Y = WebEncoders.Base64UrlDecode(epk.Y)
    }
});

var ecdsaSK = new ECDsaSecurityKey(ecdsa);

var tokenValidationParameters = new TokenValidationParameters
{
    // Setting a resolver will force the key to be the private key
    TokenDecryptionKeyResolver = (_, _, _, _) => new [] { new ECDsaSecurityKey(decryptKey) },
    // Decryption key is used as the public key
    TokenDecryptionKey = ecdsaSK
};

var result = new JsonWebTokenHandler().ValidateToken(request, tokenValidationParameters);

[GregDomzalski]

It would also really be nice if on the flip side, if KeyExchangePublicKey is set on the EncryptingCredentials that CreateToken would automatically add epk to the header. It seems that right now we need to add epk ourselves.

[inf9144]

Hey, just ran into this issue and worked around it by manually setting EPK in AdditionalHeaderClaims and reading it in TokenDecryptionKeyResolver. Will there be an update? Saw the already open pull request.
Are you planning to add functionality so that the user does not need to provide the ephemeral key hisself/herself? Most other libs i saw are just generating the ephemeral under the hood and the dev only need sto supply the ECDsa public / private key on each side.

[brentschmaltz]

Greg Domzalski (@GregDomzalski) Alexander Bülow (@inf9144) looking into this now.

[brentschmaltz]

Greg Domzalski (@GregDomzalski) the PR 2120 looks good one comments about back compat.