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

Using RS512 algorithm with EC512 key-pair works #23

Open
ozomer opened this issue May 11, 2018 · 5 comments
Open

Using RS512 algorithm with EC512 key-pair works #23

ozomer opened this issue May 11, 2018 · 5 comments

Comments

@ozomer
Copy link

ozomer commented May 11, 2018

Hi,
Maybe this is something that I don't understand in cryptography, but I've stumbled upon a strange behaviour.
I created a key pair for EC512 with the following commands:

openssl ecparam -genkey -name secp521r1 -noout -out ecdsa-p521-private.pem
openssl ec -in ecdsa-p521-private.pem -pubout -out ecdsa-p521-public.pem

Then, instead of using ECDSA as I should, I used RSA:

rsa = jwa('RS512');
signature = rsa.sign('hello', fs.readFileSync('ecdsa-p521-private.pem').toString());
rsa.verify('hello', signature, fs.readFileSync('ecdsa-p521-public.pem').toString());

And the result was true!
Is this an expected behaviour? Is it possible to use ECDSA keys for RSA and vice versa?

Thanks

@ozomer
Copy link
Author

ozomer commented May 11, 2018

@omsmith
Copy link
Contributor

omsmith commented May 22, 2018

Sorry I haven't gotten a response to you yet. Saw it when you posted it, but my off-hours have been busier than usual. It's on my to-do list.

@ozomer
Copy link
Author

ozomer commented May 22, 2018

No worries. It's better that you take your time and fix this correctly than make changes in a rush and accidentally break over 1M projects that depend on this package 😃 .

@omsmith
Copy link
Contributor

omsmith commented May 23, 2018

No mystery here then, just some Node.js code doing Node.js stuff (i.e. doing what it can instead of what you asked for, and hoping for the best

I appreciate the snark in that StackOverflow answser, perhaps it's deserved, perhaps it's not. Hopefully I can explain more of the details here though :).

Ref: #8
Ref: nodejs/node#15024

So, as in the two links above - OpenSSL never gave "SHA256 with ECDSA" and the like a name. There was RSA-SHA256 (and friends) and then later SHA256 (and friends) were added as a more generic name and RSA-SHA256 is there as an alias. nodejs/node#15024 explains it better. A step forward here (and for #8) is to switch to those generalized names, and is something I'll open a PR for and see what the test suite says right now. That's where that issue ended, but suggesting someone else open it. This will at least remove confusion of "why is my ecdsa going to rsa code?".

However, it still isn't going to address what you've noticed. Even though you explicitly specify the RSA algorithm ECDSA keys can be used and vice-versa (as long as you're using the same *256, *384 or *512). That isn't something I can immediately solve. Even after the former adjustment, we're still going into generic signer land (openssl dgst -sha256 -sign some.pem being the equivalent cli).

So options forward to help address this confusion...

  • Switch to JWK only (they specify the type)?
  • Switch to some similar key object like JWK?
  • Parse PEMs and verify they are the correct type?
    • Add a noVerify option to skip this step?
  • Consume some higher-level crypto API that has these abstractions?

tl;dr;

  • OpenSSL's signing API is generic to the public/private signature option, decided by the provided key
  • Node's crypto API is a thin wrapper on top of OpenSSL
  • node-jwa is a thin wrapper on top of Node's crypto API

I hope that was helpful, and hope to continue the discussion.

omsmith added a commit to omsmith/node-jwa that referenced this issue May 23, 2018
omsmith added a commit to omsmith/node-jwa that referenced this issue May 23, 2018
@ozomer
Copy link
Author

ozomer commented May 23, 2018

Hi,
I'm not an expert in cryptography, but from the stackexchange comments I understand the following:

  • Some algorithms that JWT support have been discovered to be completely flawed and should not be used.
  • Other algorithms that JWT support require random data to be included for each signature, which some packages might not implement.
  • Some implementation use the algorithm-name that is packed in the token to check the token. This is obviously silly, because a malicious user can use "none" or specify a different algorithm than expected (simpler to break?). Verifying a token should no trust the algorithm-name inside the token. When a token is verified, the user should always specify the algorithm to verify with (just like he specifies the secret/public key). Maybe JWK is the right way to do it, or some other format to pack the key together with the algorithm-name, or just force the user to specify an algorithm-name in the options.
  • As for 2018, Ed25519 is recommended for asymmetric signing.

Check out my fork of jsonwebtoken: jsonwebtoken-ed25519.

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