Skip to content
This repository has been archived by the owner on Jul 31, 2024. It is now read-only.

c_hash generated using wrong hashing algorithm acording to spec #3435

Closed
sorenhnielsen opened this issue Jul 15, 2019 · 5 comments
Closed

c_hash generated using wrong hashing algorithm acording to spec #3435

sorenhnielsen opened this issue Jul 15, 2019 · 5 comments
Labels
Milestone

Comments

@sorenhnielsen
Copy link

sorenhnielsen commented Jul 15, 2019

Hi

I'm using IdentityServer4 with OicdClient2 in a Hybrid Flow setup.
There I have problems with validation of the c_hash value.

It looks like IdentityServer4 is always hashing the code value with sha256.
This is in DefaultTokenService.HashAdditionalData

while the OicdClient2 is using sha256/384/512 based on the Algorithm from the Token.

Since we get this header token back
{
"alg": "RS512",
"typ": "JWT"
}
OicdClient2 expects to hash the code with sha512.

This looks like it's according to the specification on
https://openid.net/specs/openid-connect-core-1_0.html#rfc.section.3.2.2.9

Hash the octets of the ASCII representation of the access_token with the hash algorithm specified in JWA [JWA] for the alg Header Parameter of the ID Token's JOSE Header. For instance, if the alg is RS256, the hash algorithm used is SHA-256.

and
https://openid.net/specs/openid-connect-core-1_0.html#rfc.section.3.3.2.8

Right now this is preventing me from using the Hybrid Flow.

@leastprivilege
Copy link
Member

Where does the RS512 come from - our code only allows RS256 - have you changed it?

We have to wait til netcore3 for better crypto agility.

@sorenhnielsen
Copy link
Author

The RS512 comes from the replacement of builder.AddDeveloperSigningCredential (in Startup).

In production we dont have a Developer certificate, so the example code throw an exception.
Therefore we added our own certificate, which is based on RS512.
This certificate seems to control which algorithm the JWT token gets based on.

For now I have changed that certificate so we use RS256.

@leastprivilege
Copy link
Member

OK - yes I know there is room for improvement here. I will keep this open.

Glad you have a workaround for now.

@leastprivilege leastprivilege modified the milestones: 4.0, 3.1 Jul 18, 2019
@leastprivilege leastprivilege modified the milestones: 3.1, 3.0 Aug 3, 2019
@leastprivilege
Copy link
Member

OK - this is fixed in 3.0

@lock
Copy link

lock bot commented Jan 10, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants