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

diffieHellman.computeSecret Seemingly Produces An Incorrect Secret #24645

Closed
cristian-bicheru opened this issue Nov 25, 2018 · 13 comments
Closed
Labels
crypto Issues and PRs related to the crypto subsystem. question Issues that look for answers.

Comments

@cristian-bicheru
Copy link

  • Version: v8.10.0
  • Platform: Linux 4.15.0-39-generic rename node.js -> io.js #42-Ubuntu SMP Tue Oct 23 15:48:01 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
  • Subsystem: Crypto

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:

prime = FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F14374FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE649286651ECE45B3DC2007CB8A163BF0598DA48361C55D39A69163FA8FD24CF5F83655D23DCA3AD961C62F356208552BB9ED529077096966D670C354E4ABC9804F1746C08CA18217C32905E462E36CE3BE39E772C180E86039B2783A2EC07A28FB5C55DF06F4C52C9DE2BCBF6955817183995497CEA956AE515D2261898FA051015728E5A8AACAA68FFFFFFFFFFFFFFFF (hex)
generator = 2 (int)
client_privateKey = 2968590119 (int)
client_publicKey = 3393198061192be5268f6eef2eaa29a19d8ab3e5810e0c6f89b3dbddd30c94536426164396b0154a997340029ca2b44612892a790027ee247f0ef1242520c37b2a9880e22056b5a7a341f43f94a82960631273a4b67e8cf0178435bc6160d04bd77c05904ef8e8449d9515f555273a33412fe70c90763f3f906b189f51380729df830fc25808f7e3d6fb2ac064dc476d3cdfa66d5ef8768a38d3b028c489a0817274dafebe529f6635ab1169a4c6ef120db7f4022410914618dd1cfb01d72933dbf231b3982a9ac8afe6a0d2ea631f2dc8fc2379de95b6cbafb54cfd0f2d18f4725f56c8f55409f6b95b94f6e6d82ae932541b6769685142ce002e41fc448b3 (hex)
server_privateKey = 721c3f360948f1417e61f0caedc5539f9abdad4578b80c95a04008359457bbba9c0a316cf43ce3cba0819faf5eac8d07130263fa8a06edfafbadd9dea37616a50b9e52f64fe851ab47239be7710b1edfde3db1a1953ebb2b1cbd9f61f6ded41dc1e2d3fdd4f84f6a97cff17e2f0e1960f170dbb3c289747c0589d69f6696f1dbec22cd5f32977984d609c3a3fac9024df76d1634d8afc7c9b669f58655aacf680c1238ca4b4fc5bc30261eaed5de2521f5884a69c6ad58d41d5c42965d24422f589b8385887fec85643b92cf64e491fae676774e9e9968c80c8b1d4204ced0239bffe277e705c1dc5923eb733bec82e90b08016080c82422e24e98c62c71679e (hex)
server_publicKey = 7b7939097eecc52613a3d2b6c30de1b9502456f6a143c830abc31ef2e30eee5761c5cf81a81e1368932ec184bc6c05f4eb5a86923557e7c60084285cd9f909305edef86b624c24531748ad1da737759dacaed537f47d48146154887a2335cdd89dd7120907d1f436ea417c95ff06cd1acdea8df4aef60c9023946caa524eb14daf9a8767d9fa3910d360d27980f334b1913644e7fed3a9119e5a743ef21b72655ab4b5b74a73aeebf69abdd1c1bbe65cfc0f6290c88a0be5f9b9cbfe5df852f90e52086d4bb8aaba0b23717854033d6db8365ccbf853711d8c224889bd31d611e4a396e280c67ceafe04ca8b42d6aae2feec7aba5864070d0052d70270c82877 (hex)

Outputs:

Correct Secret:

ba8324267fb9f4b7a7124a1692778cf9648c1b285b446eb72ac6af837e6462a027885a5fd95647598888245ffb5600777b85e1bbc5b11964ce13b3f6cb6b0fafff1403ea23a4b950be83006924acf7b96dc31a8b082e67750c6142066abb8ae003305a2c9d6210cefd9a54766495a1c0b0ddb7900ceb0861b42239a99c9974d7d2d18dace0674859d3a7f2babdf180f472fefa4c64c53820b78403c296540bb2ddbb7e8881870d710c624c8b8241e173f55554d466304a498553c96eef498f8821520efbdc6293c83abff8d39e18e36c9e2e94598e851005b06ec4c99a29f566776f832580f11cead97bde4248163fd3db64116f811195cf287d384f9f3e1b19 (hex)

Secret Computed By Node:

aad3e5e8a7b072edd765dd9ddde4e532f5abbbcc0d464493014da419a20f02f9aa4fe05808400e70964ecda99a173ca574a7d1ef72ff49c015a21479576380bfffd0bce41ba5c0eb0f267d21a890fdd88f3ac45eb43531f8a2a6a7bcd2f6b43e1b42ef241ce9cc4839b5134212b97d36e53bf628727319e334c3dd8c02f5210c8a6ede36a0d76f8cbb30612a2fe7d1717427561f28b4011939005426778b127e2483168b5b6ba60f10e9cf38e81cd15fd7ded387d4d612c3f0a37de96382cff7abb1701649db6950fd0ce0408dcc35c6272d95cefdad6457e385dd6c9fa5cc0c3c6d9109827ccffaedf3ec01453f38966de7846e2d5e9dc6f54bd926d162eef7 (hex)

My Code For Testing:

const dh_prime = Buffer.from(fs.readFileSync('./dh_prime', 'utf8'), 'hex');
let client_publicKey = '3393198061192be5268f6eef2eaa29a19d8ab3e5810e0c6f89b3dbddd30c94536426164396b0154a997340029ca2b44612892a790027ee247f0ef1242520c37b2a9880e22056b5a7a341f43f94a82960631273a4b67e8cf0178435bc6160d04bd77c05904ef8e8449d9515f555273a33412fe70c90763f3f906b189f51380729df830fc25808f7e3d6fb2ac064dc476d3cdfa66d5ef8768a38d3b028c489a0817274dafebe529f6635ab1169a4c6ef120db7f4022410914618dd1cfb01d72933dbf231b3982a9ac8afe6a0d2ea631f2dc8fc2379de95b6cbafb54cfd0f2d18f4725f56c8f55409f6b95b94f6e6d82ae932541b6769685142ce002e41fc448b3';
let generator = 2;
const server = crypto.createDiffieHellman(dh_prime, generator);
server.setPrivateKey('721c3f360948f1417e61f0caedc5539f9abdad4578b80c95a04008359457bbba9c0a316cf43ce3cba0819faf5eac8d07130263fa8a06edfafbadd9dea37616a50b9e52f64fe851ab47239be7710b1edfde3db1a1953ebb2b1cbd9f61f6ded41dc1e2d3fdd4f84f6a97cff17e2f0e1960f170dbb3c289747c0589d69f6696f1dbec22cd5f32977984d609c3a3fac9024df76d1634d8afc7c9b669f58655aacf680c1238ca4b4fc5bc30261eaed5de2521f5884a69c6ad58d41d5c42965d24422f589b8385887fec85643b92cf64e491fae676774e9e9968c80c8b1d4204ced0239bffe277e705c1dc5923eb733bec82e90b08016080c82422e24e98c62c71679e', 'hex')
const server_publicKey = server.generateKeys('hex');
console.log('server_privateKey: '+server.getPrivateKey('hex'));
console.log('server_publicKey: '+server_publicKey);
console.log('client_publicKey: '+client_publicKey);
var buffer = Buffer.from(client_publicKey, 'hex');
let secret = server.computeSecret(buffer, 'hex', 'hex');
console.log('key: '+secret);

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.

@Trott
Copy link
Member

Trott commented Nov 25, 2018

@nodejs/crypto

@bnoordhuis
Copy link
Member

Can you post the verbatim contents of the ./dh_prime file?

@bnoordhuis bnoordhuis added question Issues that look for answers. crypto Issues and PRs related to the crypto subsystem. labels Nov 25, 2018
@cristian-bicheru
Copy link
Author

cristian-bicheru commented Nov 25, 2018

@bnoordhuis sure, here are the contents:

FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F14374FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE649286651ECE45B3DC2007CB8A163BF0598DA48361C55D39A69163FA8FD24CF5F83655D23DCA3AD961C62F356208552BB9ED529077096966D670C354E4ABC9804F1746C08CA18217C32905E462E36CE3BE39E772C180E86039B2783A2EC07A28FB5C55DF06F4C52C9DE2BCBF6955817183995497CEA956AE515D2261898FA051015728E5A8AACAA68FFFFFFFFFFFFFFFF

It's the hex value for a 2048 bit prime I found here https://tools.ietf.org/html/rfc3526.

(Edited to fix the hyperlink)

@bnoordhuis
Copy link
Member

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.

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?

@cristian-bicheru
Copy link
Author

@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.

@bnoordhuis
Copy link
Member

bnoordhuis commented Nov 28, 2018

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 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.

@cristian-bicheru
Copy link
Author

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).

@cristian-bicheru
Copy link
Author

I just tried it in Java, and I got the same secret as Python.

@cristian-bicheru
Copy link
Author

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).

@bnoordhuis
Copy link
Member

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.

danbev pushed a commit that referenced this issue Dec 3, 2018
PR-URL: #24719
Refs: #24645
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BridgeAR pushed a commit that referenced this issue Dec 5, 2018
PR-URL: #24719
Refs: #24645
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@bnoordhuis
Copy link
Member

@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.

@cristian-bicheru
Copy link
Author

@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

@cristian-bicheru
Copy link
Author

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.

refack pushed a commit to refack/node that referenced this issue Jan 14, 2019
PR-URL: nodejs#24719
Refs: nodejs#24645
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this issue Feb 12, 2019
PR-URL: #24719
Refs: #24645
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
rvagg pushed a commit that referenced this issue Feb 28, 2019
PR-URL: #24719
Refs: #24645
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem. question Issues that look for answers.
Projects
None yet
Development

No branches or pull requests

3 participants