IdentityModel / IdentityModel.OidcClient.Old

OpenID Connect Client Library for Native Applications
Other
38 stars 17 forks source link

Wrong key type used for HmacUsingSha.Verify #36

Closed nielsenko closed 7 years ago

nielsenko commented 8 years ago

I'm trying to get OidcClient to work with Auth0.com. They don't support the Hybrid flow, and they use HS256 instead of RS256 for signature algorithm. EDIT: You can setup auth0.com to use RS256 in the dashboard under "advanced settings"

This has exposed a bug in OidcClient.. or its use of jose-pcl.

Looking at DefaultIdentityTokenValidator I see:

line 30-34:

        var e = Base64Url.Decode(providerInformation.KeySet.Keys.First().E);
        var n = Base64Url.Decode(providerInformation.KeySet.Keys.First().N);
        var pubKey = PublicKey.New(e, n);

        var json = JosePCL.Jwt.Decode(identityToken, pubKey);

But HmacUsingSha.Verify expects key to be a byte[] array, not a ICryptographicKey, so the JosePCL.Jwt.Decode will eventually fail in HmacUsingSha.Verify:

public bool Verify ([ReadOnlyArray] byte[] signature, [ReadOnlyArray] byte[] securedInput, object key)
{
    byte[] array = Ensure.Type<byte[]> (key, "HmacUsingSha expects key to be byte[] array.", new object[0]);
    ICryptographicKey cryptographicKey = this.AlgProvider.CreateKey (array);
    return WinRTCrypto.get_CryptographicEngine ().VerifySignature (cryptographicKey, securedInput, signature);
}

Here is the interesting of the stacktrace.

Stacktrace: at JosePCL.Util.Ensure.Type[T](System.Object obj, System.String msg, System.Object[] args) [0x0000f] in :0 at JosePCL.Jws.HmacUsingSha.Verify (System.Byte[] signature, System.Byte[] securedInput, System.Object key) [0x00000] in :0 at JosePCL.Jwt.Verify (JosePCL.Serialization.Part[] parts, System.Object key) [0x00090] in :0 at JosePCL.Jwt.Decode (System.String token, System.Object key) [0x0001e] in :0 at IdentityModel.OidcClient.IdentityTokenValidation.DefaultIdentityTokenValidator.ValidateAsync (System.String identityToken, System.String clientId, IdentityModel.OidcClient.ProviderInformation providerInformation) [0x00076] in C:\local\identity\model\OidcClient\src\Shared\IdentityTokenValidation\DefaultIdentityTokenValidator.cs:34 ...

Message: HmacUsingSha expects key to be byte[] array.

I don't feel confident solving this myself, as I'm unfamiliar with the code base, but I hope it will be clear to you what to do.

leastprivilege commented 8 years ago

You can switch for authorization code flow on the OidcOptions.

HS256 does not make sense - this would require to embed the symmetric key into your client.

nielsenko commented 8 years ago

Yes, I already switched to the authorization code flow using the OidcOptions class.

By default Auth0.com uses HS256 for signing, and yes that means you would need to know the private key to validate. I assume the idea is that the client doesn't validate (just forwards) or validate on another endpoint.

Anyway, I can enable RS256 in the "advanced settings" on Auth0.com to avoid the issue, and that is better all around, but I still think something fishy is going on in OidcClient and its use of jose-pcl, so I wanted to report it as good net citizen :-)

Also, as you know, using RS256 via PCLCrypto on iOS10, currently comes with its own problem :-/ https://github.com/AArnott/PCLCrypto/issues/109

leastprivilege commented 8 years ago

what do you mean with "fishy" ?

And yes - iOS10 is a problem - I hope with netstandard there is soon a better x-plat crypto story than relying on someone doing the p/invokes by hand.

nielsenko commented 8 years ago

I totally agree, regarding netstandard. It can't happen fast enough :-)

Regarding the "fishy" part, and pardon the expression - the thing I think is a bug, is that:

IdentityModel.OidcClient.IdentityTokenValidation.DefaultIdentityTokenValidator.ValidateAsync 

calls

JosePCL.Jwt.Decode

with a key of type ICryptographicKeywhen further down the stack

JosePCL.Jws.HmacUsingSha.Verify

Expects a byte-array. I don't like the opaque interface design of jose-pcl, where a key is just an object, but given that the api is as-it-is, it is up to user to ensure the key object is valid.

leastprivilege commented 8 years ago

Agreed - I don't see an alternative right now though.