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

Support ES256 JWK Algo #627

Closed
joshuarubin opened this issue Oct 24, 2017 · 7 comments
Closed

Support ES256 JWK Algo #627

joshuarubin opened this issue Oct 24, 2017 · 7 comments

Comments

@joshuarubin
Copy link

It looks like support for ES256 is available (jwk/generator_ecdsa256.go), but that the JWK handler doesn't support it (https://github.com/ory/hydra/blob/master/jwk/handler.go#L27-L38).

Go's P-256 is implementation is constant-time (which prevents certain types of attacks) while its P-384 and P-521 are not (https://github.com/gtank/cryptopasta).

@joshuarubin
Copy link
Author

closed in favor of #628

@aeneasr
Copy link
Member

aeneasr commented Oct 25, 2017

I think we should disable the P521 generator for now, as it's not safe to use them for repeated use in private keys.

@aeneasr aeneasr reopened this Oct 25, 2017
@joshuarubin
Copy link
Author

joshuarubin commented Oct 25, 2017

RFC7518 lists ES256 as "Recommended+" meaning:

The use of "+" in the Implementation Requirements column indicates
that the requirement strength is likely to be increased in a future
version of the specification.

I think that the stronger algorithm should continue to be supported.

Also, the webcrypto spec has renamed ES521 to ES512:

If the "alg" field is equal to the string "ES512":
Let algNamedCurve be the string "P-521".

@aeneasr
Copy link
Member

aeneasr commented Oct 25, 2017

Ok, let's keep them then. As long as JWK/JWT spec talk of ES521 I think we should keep it that way. Definitely doesn't help though that there are naming inconsistencies now.

@aeneasr aeneasr closed this as completed Oct 25, 2017
@joshuarubin
Copy link
Author

Can you link to the spec which still references ES521?

@aeneasr
Copy link
Member

aeneasr commented Oct 25, 2017

My bad, looks like I got mixed up, it's definitely ES512

@aeneasr
Copy link
Member

aeneasr commented Nov 13, 2017

I closed this because ES512 is now tracked in #651

@aeneasr aeneasr closed this as completed Nov 13, 2017
@mitar mitar mentioned this issue Apr 9, 2021
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

No branches or pull requests

2 participants