-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
diffieHellman.computeSecret Seemingly Produces An Incorrect Secret #24645
Comments
@nodejs/crypto |
Can you post the verbatim contents of the |
@bnoordhuis sure, here are the contents:
It's the hex value for a 2048 bit prime I found here https://tools.ietf.org/html/rfc3526. (Edited to fix the hyperlink) |
Do you also see this with the latest v8.x release, v8.13.0? I'm unable to reproduce. Also, why are you sure python generates the correct secret but node doesn't? There's no DH support in python's stdlib, is there? |
@bnoordhuis I'm not at home at the moment, but I'll check in a different language to see if the computation is correct. When you say you are unable to reproduce this in v8, are you saying that it works as expected or that the code breaks? I was pretty sure Python computed it correctly since I wrote the computation module myself and I trust Python's modular exponentiation function, but to be fair I do need to test it in a different language. Also I should mention I tried to do the same computation on Node v11 with no luck. |
I couldn't reproduce the behavior you described where it sometimes produces one key, sometimes another. In 100 runs it always printed the exact same key. |
I should clarify, It always produces the same key given the same input, otherwise, that would be really strange. It's just that the computed key is sometimes the same as my Python code, other times it has a 0 in the first digit (the key is still technically correct here since 010=10 for example), but other times it is completely off, which is why I opened this issue. The code always prints the same key given the same inputs, It's just that the key it produces isn't always correct (or perhaps my Python is incorrect, will verify later). |
I just tried it in Java, and I got the same secret as Python. |
I tried using the client's private and server's public keys, and Node outputs the correct secret. This leads me to believe that there may be an overflow in the modular exponentiation calculation in Node (the client private key is very small and probably doesn't cause an overflow, whereas when you use the server's private key and client's public key, the numbers are much larger). |
That sounds superficially plausible, but that would indicate a bug in OpenSSL. OpenSSL gets a lot of scrutiny these days, you'd think a bug like that would get caught quickly. It's possible Node.js does something wrong when converting the key from bignum to binary representation but I don't see anything glaringly wrong, not at a quick glance anyway. I opened #24719 to harden the bignum-to-binary conversions. It would be nice if you could test it and see if it makes a difference. It's against master. |
@cristian-bicheru What do you want to do with this? If your hypothesis is correct, that makes it an upstream issue; that's not something we can fix. If you still want to pursue it here, you should probably post your python code, so we can cross-check that it's doing the same thing as openssl. You can check against the example C code on the openssl DH wiki page; it's the low-level API example. |
@bnoordhuis I've been a bit clogged up with school work lately so I haven't had the time to even check out the pull request yet. I'll try it by the end of tomorrow and if it still doesn't work, I'll take a look at the openssl code. Here's my python code: client_secret = int.from_bytes(os.urandom(4), sys.byteorder)
transmission = pow(GENERATOR, client_secret, DH_PRIME)
key = hex(pow(int(server_transmission, 16), client_secret, DH_PRIME)) #server_transmission is the server's public key and is in hex |
I built and ran the code again with the changes added and still no luck. I've decided to implement RSA instead of DH so I'm going to close this issue for now. |
PR-URL: nodejs#24719 Refs: nodejs#24645 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
I am having issues computing the secret during a DH key exchange using the Crypto module. I have some Python code as the client, and the Node is the server. The Python code appears to compute the same secret given either the client-side keys or server-side keys, but the Node computes a completely different secret. Here are all the keys/variables used:
Inputs:
Outputs:
Correct Secret:
Secret Computed By Node:
My Code For Testing:
This code works as expected sometimes, other times the key has a 0 at the beginning, and other times the key seems to be completely off. I also know for sure that diffieHellman.generateKeys is working as expected, since my Python code is able to compute the correct secret given the server's keys.
I should also mention that I am new to Node and javascript as a whole, and to cryptography in general.
The text was updated successfully, but these errors were encountered: