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

failing on secp256r1 curve #362

Closed
m-zielinski opened this issue Jul 20, 2020 · 7 comments
Closed

failing on secp256r1 curve #362

m-zielinski opened this issue Jul 20, 2020 · 7 comments

Comments

@m-zielinski
Copy link

m-zielinski commented Jul 20, 2020

'secp256r1' curve name is not supported. Worse - it rises an ambiguous message: "payload algorithm is ES256 but signing key was provided". That's because JWT::Algos::Ecdsa::NAMED_CURVES has no 'secp256r1' key.

Question: Is it deliberate or may I make a pull request to fix this (add the curve name to NAMED_CURVES and fix the exception message for future unlisted names)?

How to reproduce:

ecdsa_key = OpenSSL::PKey::EC.new 'secp256r1'
ecdsa_key.generate_key
ecdsa_key.check_key # true
JWT.encode "", ecdsa_key, 'ES256' # raises: JWT::IncorrectAlgorithm (payload algorithm is ES256 but  signing key was provided)
@obulkin
Copy link

obulkin commented Nov 5, 2020

Running into this issue as well. We have a stored key generated using openssl ecparam -name prime256v1, but when we ingest it into OpenSSL in code, the resulting key object has the secp256r1 curve name.

My understanding is that prime256v1 and secp256r1 are two different names for the same curve, so it would be great if you could update NAMED_CURVES to support both names. We're having to work around this in very hacky ways.

@anakinj
Copy link
Member

anakinj commented Nov 6, 2020

Think you're right. openssl describes the two curves like this:

secp256k1 : SECG curve over a 256 bit prime field
prime256v1: X9.62/SECG curve over a 256 bit prime field

I might take a look at this at some point.

@anakinj
Copy link
Member

anakinj commented Nov 6, 2020

But hmm secp256r1 is not the same as secp256k1 or maybe they are very similar but with some (for this context trivial) differences.

Should all 3 curves secp256r1, secp256k1, prime256v1 be treated the equal in this context?

@obulkin
Copy link

obulkin commented Nov 6, 2020

According to https://tools.ietf.org/search/rfc4492#appendix-A, secp256k1 is a separate curve, but secp256r1 and prime256v1 are just aliases. I don't really know enough about cryptography and what this library is doing to confidently say whether secp256k1 should also be lumped in with the other two.

@anakinj
Copy link
Member

anakinj commented Nov 6, 2020

This was a pretty interesting and easy read on the subject: https://www.johndcook.com/blog/2018/08/21/a-tale-of-two-elliptic-curves/

I think you're right, lets keep the secp256k1 out of this and just treat secp256r1 and prime256v1 as the same.

@anakinj
Copy link
Member

anakinj commented Dec 12, 2020

Think #385 should fix this problem

@anakinj
Copy link
Member

anakinj commented Aug 30, 2022

Closing this. The support for secp256r1 as an alias for prime256v1 was added a while ago.

@anakinj anakinj closed this as completed Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants