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

fix: use sha256 of nonce #96

Open
MarkLinovy opened this issue Jun 26, 2024 · 4 comments
Open

fix: use sha256 of nonce #96

MarkLinovy opened this issue Jun 26, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@MarkLinovy
Copy link

Description
I am not 100% sure if it's a bug or not. So please let me explain my undestanding and correct me if im wrong:

For Code flow we send a nonce to verify that the idtoken weve received actually belongs to the request we made. For this we usually generate a random string. Let's call it "rawNonce".
Now before we send it to the Auth Server we hash it (sha256). This allows us to hash the raw nonce again before comparing it with the nonce included in the id token.

As I could see in the source code you are not hashing the nonce. Can you extend the lib and include a rawNonce? I would happily create a PR if it makes sense to include.

@MarkLinovy MarkLinovy added the bug Something isn't working label Jun 26, 2024
@ahmednfwela
Copy link
Member

We are generating a cryptographically random nonce

    final nonce = Nonce.generate(32, Random.secure());

but we are not hashing it.

is there a reference in the spec to hashing nonce?

@MarkLinovy
Copy link
Author

There is a reference in the spec for javascript clients that mentions hashing nonce. That makes it unguessable for a potential attacker: https://openid.net/specs/openid-connect-core-1_0.html#NonceNotes

Also other libs are expecting the rawNonce and compare the hashed version with the nonce in the id token. See Firebase Auth, Apple Auth and Spring OAuth Default. If this lib doesn't support hashed nonce we can't use it with firebase identity federation e.G.

Do you want me to create a pull request?

@ahmednfwela
Copy link
Member

I see now, it would be great if you make a PR for this, yes please.

@MarkLinovy
Copy link
Author

I created the PR. Unfortunately I see no way to test it without mocking everything. Ive also remove the hashed nonce from store. Do you have any remarks?

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