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 names should be written in lower case #264

Closed
srosenda opened this issue Dec 17, 2024 · 3 comments · Fixed by #266
Closed

Hash algorithm names should be written in lower case #264

srosenda opened this issue Dec 17, 2024 · 3 comments · Fixed by #266
Assignees

Comments

@srosenda
Copy link

This repository seems to use mixed case characters when defining hash algorithm names, see search results https://github.com/search?q=repo%3Aopenwallet-foundation%2Fsd-jwt-js+%22SHA-%22&type=code. Specifically the online tool https://www.sdjwt.co/issue seems to be fixed to use sha-256 algorithm to calculate the SD-JWT Disclosure hashes, but it outputs the algorithm name to the SD-JWT payload in upper case as "_sd_alg": "SHA-256". This is incorrect. The hash algorithm names are defined in IANA Named Information Hash Algorithm Registry and they should be treated as case-sensitive. See also https://www.ietf.org/archive/id/draft-ietf-oauth-selective-disclosure-jwt-14.html#section-4.1.1-2.

@lukasjhan lukasjhan self-assigned this Dec 17, 2024
@lukasjhan
Copy link
Member

Oh Thank you @srosenda
I'll update the online debugger

FYI: online debugger repo is here: https://github.com/lukasjhan/sd-jwt-io

@srosenda
Copy link
Author

Oh Thank you @srosenda I'll update the online debugger

Great and thank you for providing the tool, it has been very useful!

FYI: online debugger repo is here: https://github.com/lukasjhan/sd-jwt-io

OK, thanks. I followed the "Library" link on the debugger top bar to this repo. Should we close this and open a new issue in the debugger's repo?

@cre8
Copy link
Contributor

cre8 commented Dec 28, 2024

Hi @srosenda,

I think we should keep this issue open and fix it in this library. A quick search showed that we have set it to SHA-256 in the tests, but to be compliant we should even update them.

We could also set the type from string to a fixed set of strings to help developers with the lower case situation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants