mscdex / ssh2-streams

SSH2 and SFTP client/server protocol streams for node.js
MIT License
204 stars 145 forks source link

OpenSSH private keys not decoded correctly #163

Open Timmmm opened 4 years ago

Timmmm commented 4 years ago

We have an OpenSSH key that was generated on MacOS. ssh2-streams unfortunately decodes it incorrectly:

const s = require("ssh2-streams");
const keys = s.utils.parseKey(`-----BEGIN OPENSSH PRIVATE KEY-----
b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAABlwAAAAdzc2gtcn
NhAAAAAwEAAQAAAYEA6oDCeE6kPkmq0SUpmBz7aojSWz9jow/fqcSPnXVfANNW3A9zNi57
ZvOPyR0PuFk93O0Ybrj5DRZRFhnFzSkY5Kq0D3FLjwxM4hCKtjz+fBjBJ8HaCRtpLQa4pX
NGWuJ1cdVWr9Zzu5nO8p7SlGOTe8Rfk/JVgG0ReRnNyptF/ITvjGgXEhZwrS+9LulvSqnc
6iizmwz3r6lCiN0vRRC6F5GZOoRBcsGj0XnYq/zQqsrBCtJJgqnkxqEj3KnfvgjYSZcxZH
WNhUVSdWm7iQtzV7Thk/tiXoiAs3dPMRH05HI1wUQ7So0BhDRdaglGqHg9gQOlI6nFu/ZS
BQoAyHtQ2bNK/+f6AOIH2HRun2ss7vjM2L/zdCSlnxddlMk8IO0/v/U3aelox+CSbpReu/
xxvmO+XL/pZk3lj5MfDC+gwQXiGzgaQ7KLSM+Pj05lfx1uHRN8caLBRlm90flaAmayEIFI
GZ0f3UqFUBfNEEFoCaeGo8jVOBE5u7vIvvd3ajobAAAFkFn5hQhZ+YUIAAAAB3NzaC1yc2
EAAAGBAOqAwnhOpD5JqtElKZgc+2qI0ls/Y6MP36nEj511XwDTVtwPczYue2bzj8kdD7hZ
PdztGG64+Q0WURYZxc0pGOSqtA9xS48MTOIQirY8/nwYwSfB2gkbaS0GuKVzRlridXHVVq
/Wc7uZzvKe0pRjk3vEX5PyVYBtEXkZzcqbRfyE74xoFxIWcK0vvS7pb0qp3Ooos5sM96+p
QojdL0UQuheRmTqEQXLBo9F52Kv80KrKwQrSSYKp5MahI9yp374I2EmXMWR1jYVFUnVpu4
kLc1e04ZP7Yl6IgLN3TzER9ORyNcFEO0qNAYQ0XWoJRqh4PYEDpSOpxbv2UgUKAMh7UNmz
Sv/n+gDiB9h0bp9rLO74zNi/83QkpZ8XXZTJPCDtP7/1N2npaMfgkm6UXrv8cb5jvly/6W
ZN5Y+THwwvoMEF4hs4GkOyi0jPj49OZX8dbh0TfHGiwUZZvdH5WgJmshCBSBmdH91KhVAX
zRBBaAmnhqPI1TgRObu7yL73d2o6GwAAAAMBAAEAAAGBAK3Db1aaX1XQXITRVrribuIING
ds1zXS41b20bxJZnZEI864nBR/6OzmeZr6r9VYO2DjRwF0iwdJjBgmTM2zMEDwxK/Usbpz
0NyKiV50X9YwuZ/uItQwvYShwFzIyE88Eu6guLVe4S8Xz6M0ULGn/3bski4cmYKqTxI/Lq
B84iU1lBOkZcP/YMrT3wlN7CuVJp9UPZIcz8QYVbb4vrgcpugj+TjFG4IfPnZQJV973FtS
Fi0gmVv67AzLBrnSHnrALalswG82nQ4bt/ejc6+BUcycLcQLGVkOkt0xS/jqPFqTDI0n3r
OAVkNwPNK1YsgOtaVYvy+qrNJ11RdRno6T4wMtJcXM35HLhfs4fnDGIJDY+d9MOGXl2m57
ZgAseIoxGHvlfEIUzfX/R9HbvG/oHqU4dWKCDacO5JKlHJMJHicXgJx53y8Y+mCVCqZLSW
+btAX9rL28JoXFo1IqGr6w7KhMlWFqG+sDWL9CYyUOc572iKtqgMo43+2AdIIOFM+8QQAA
AMEAkkVJQji/v3eCdLlLQZ1pcTaCXA9KB5jsbaRGRl3xhrT10o1726J0rb9R+soXzw9JiX
uBq/2gAe3AswXQYdMHFStvj7RDNArnCc5s3IOon4OT6ATKBhnleYncOg5zxGZjP9kYBX2X
19rYljSnEZfUI4EzJvrIn3wOQFH/+9JnwLYjOXzU1u4gW3OucA8PUd4rDw1ptap4n2yleE
fQ8SvT7SNlyDmkF0v9L3qPylgAG0/NAPolI26/pAzbh+B3v45PAAAAwQD18hltshrEvHo1
2DeeNuAbY3TObHuw/LSXIL+r/vBC5Dgxf5Z7kB7G72MFx5Sq1HTE0kllcXlqhZ3xdCgUAe
cBs2313cmxwtinZcZa2qfI7MmFzIKeNTxc7jPqiUDvubWZVHAyQ2F1xq4sKAAp8QbbrBr7
bJZmBIi/j+PfSgmjc1n2017i/5da1AkeEYHEh4RwDNnXCA4UQeVM+e5PFnpGayfAl4d6BA
vjk1Bty09gyH/DKLAj4FC5NkwvKZSmP6sAAADBAPQW6I/QXL6ckhjbVPv//uyqBII0GyaV
K6+tx1A9uNTh2Gf8gKYLA83XO3hPCP8TiznFZJjgZY14TAJCUuD5kYiLfi1Fl7FAm4elcA
4jLgkOM8IWKUSFkOYxcnqSLCQQ/xoEvNhacap9LZpIR0WXvrJ57O9l/CEwaJ+W2dbt1C5G
UQBP4RXyXNDgMM2g4gbUA6qiWs7EeItMa0hALtSJaxjI+QWC/UUdcPxyl8abCUUGq9uqAG
1DEJkAueusvm8/UQAAABN6YWNockBmb3JqZWFuLmxvY2FsAQIDBAUG
-----END OPENSSH PRIVATE KEY-----`);
const privateKey = keys[0].getPrivatePEM();
console.log(privateKey);

Running the above code gives:

-----BEGIN RSA PRIVATE KEY-----
MIIG4wIBAAKCAYEA6oDCeE6kPkmq0SUpmBz7aojSWz9jow/fqcSPnXVfANNW3A9z
Ni57ZvOPyR0PuFk93O0Ybrj5DRZRFhnFzSkY5Kq0D3FLjwxM4hCKtjz+fBjBJ8Ha
CRtpLQa4pXNGWuJ1cdVWr9Zzu5nO8p7SlGOTe8Rfk/JVgG0ReRnNyptF/ITvjGgX
EhZwrS+9LulvSqnc6iizmwz3r6lCiN0vRRC6F5GZOoRBcsGj0XnYq/zQqsrBCtJJ
gqnkxqEj3KnfvgjYSZcxZHWNhUVSdWm7iQtzV7Thk/tiXoiAs3dPMRH05HI1wUQ7
So0BhDRdaglGqHg9gQOlI6nFu/ZSBQoAyHtQ2bNK/+f6AOIH2HRun2ss7vjM2L/z
dCSlnxddlMk8IO0/v/U3aelox+CSbpReu/xxvmO+XL/pZk3lj5MfDC+gwQXiGzga
Q7KLSM+Pj05lfx1uHRN8caLBRlm90flaAmayEIFIGZ0f3UqFUBfNEEFoCaeGo8jV
OBE5u7vIvvd3ajobAgMBAAECggGBAK3Db1aaX1XQXITRVrribuIINGds1zXS41b2
0bxJZnZEI864nBR/6OzmeZr6r9VYO2DjRwF0iwdJjBgmTM2zMEDwxK/Usbpz0NyK
iV50X9YwuZ/uItQwvYShwFzIyE88Eu6guLVe4S8Xz6M0ULGn/3bski4cmYKqTxI/
LqB84iU1lBOkZcP/YMrT3wlN7CuVJp9UPZIcz8QYVbb4vrgcpugj+TjFG4IfPnZQ
JV973FtSFi0gmVv67AzLBrnSHnrALalswG82nQ4bt/ejc6+BUcycLcQLGVkOkt0x
S/jqPFqTDI0n3rOAVkNwPNK1YsgOtaVYvy+qrNJ11RdRno6T4wMtJcXM35HLhfs4
fnDGIJDY+d9MOGXl2m57ZgAseIoxGHvlfEIUzfX/R9HbvG/oHqU4dWKCDacO5JKl
HJMJHicXgJx53y8Y+mCVCqZLSW+btAX9rL28JoXFo1IqGr6w7KhMlWFqG+sDWL9C
YyUOc572iKtqgMo43+2AdIIOFM+8QQKBwQD18hltshrEvHo12DeeNuAbY3TObHuw
/LSXIL+r/vBC5Dgxf5Z7kB7G72MFx5Sq1HTE0kllcXlqhZ3xdCgUAecBs2313cmx
wtinZcZa2qfI7MmFzIKeNTxc7jPqiUDvubWZVHAyQ2F1xq4sKAAp8QbbrBr7bJZm
BIi/j+PfSgmjc1n2017i/5da1AkeEYHEh4RwDNnXCA4UQeVM+e5PFnpGayfAl4d6
BAvjk1Bty09gyH/DKLAj4FC5NkwvKZSmP6sCgcEA9Bboj9BcvpySGNtU+//+7KoE
gjQbJpUrr63HUD241OHYZ/yApgsDzdc7eE8I/xOLOcVkmOBljXhMAkJS4PmRiIt+
LUWXsUCbh6VwDiMuCQ4zwhYpRIWQ5jFyepIsJBD/GgS82Fpxqn0tmkhHRZe+snns
72X8ITBon5bZ1u3ULkZRAE/hFfJc0OAwzaDiBtQDqqJazsR4i0xrSEAu1IlrGMj5
BYL9RR1w/HKXxpsJRQar26oAbUMQmQC566y+bz9RAoG/mMDFClyUQ5zMSqT/5kQu
7NPYuFyodkR95V9xrGGq1+DnBdc4n4Xjl6sW1YAf1foejCAPdfIEdySF9HEwczy7
PVMz+IDHxKA/77hGeidUDncCsxdSCPEHjLBljkWxDzNIlvLavF0dKwk7JDGz6FjK
6aT7HS1UPAiU5mV4IjbZxarfNW4SgOo+FyuafhJhhq6kkvNoWSWnmO1UfXq9iGFt
kE79YaC7hAz/VVkpmwerRXFB6PRccevgNyiCKMiqipcCgcCZ6bwZuyxQMVBahitP
f/vSZd7T72FTceqLK4wrx8+9xO99mpXQnc93Q4nHcibdGQjIk/S8BXtjuoBLSGEU
ZapkLHmSuHhPl8q6vRIsh/hmU2NFtk2tXH+i93kVWwikcWc6k9Q0pMtZ0vcnev4e
HvJrfkBuMKPMlcbTIv8X7P3HdCVtifEHsrzeJRABCbqczaHGPfv+t5q/U5+ufnJJ
KKG0jj2gQRnNNCRjuyqDXI16zhseN1NvkkNsBy41MsceRRECgcEAkkVJQji/v3eC
dLlLQZ1pcTaCXA9KB5jsbaRGRl3xhrT10o1726J0rb9R+soXzw9JiXuBq/2gAe3A
swXQYdMHFStvj7RDNArnCc5s3IOon4OT6ATKBhnleYncOg5zxGZjP9kYBX2X19rY
ljSnEZfUI4EzJvrIn3wOQFH/+9JnwLYjOXzU1u4gW3OucA8PUd4rDw1ptap4n2yl
eEfQ8SvT7SNlyDmkF0v9L3qPylgAG0/NAPolI26/pAzbh+B3v45P
-----END RSA PRIVATE KEY-----

If you use the above key in Node then it appears to work:

> require("crypto").sign("sha1", Buffer.alloc(500), privateKey);
<Buffer e1 cc 1b f1 a5 e2 89 bd a7 c7 fb 71 58 ca 5e 81 17 82 8f 15 5a 4e 23 e2 34 11 d6 b3 fc a1 61 65 bf 8c 87 7e e6 7c a2 00 cb 1d 46 0e 73 71 45 ed 2a da ... 334 more bytes>

However if you do the same thing in Electron's Renderer (i.e. Chromium) then you get this error:

"Error: Error while signing data with privateKey: error:06000066:public key routines:OPENSSL_internal:DECODE_ERROR
    at signOneShot (internal/crypto/sig.js:124:10)
    at OpenSSH_Private.sign (http://localhost:8080/js/entry_renderer.js:168552:16)
    ...

The difference is that Node uses OpenSSL whereas Chromium uses BoringSSL. BoringSSL is more strict about parsing keys, and according to them, this key is encoded incorrectly. We can get a similar error by saving the above RSA PRIVATE KEY to id_rsa and running BoringSSL on the command line:

$ ./bssl sign -key id_rsa any_file.txt
  140496764691080:error:06000066:public key routines:OPENSSL_internal:DECODE_ERROR:../crypto/evp/evp_asn1.c:164:
  140496764691080:error:0900000c:PEM routines:OPENSSL_internal:ASN.1 encoding routines:../crypto/pem/pem_pkey.c:138:

OpenSSL is more lax and outputs some data:

$ openssl dgst -sha1 -sign id_rsa any_file.txt
<it prints some binary data>

This is the issue according to David Ben in that link above:

The private key is incorrectly encoded with some negative components. (In DER, if the high bit of your positive INTEGER is set, you need a leading zero.) In particular, dmp1 and dmq1 are negative and missing a leading zero. Closing this as it's not a bug in BoringSSL (the other parsers are insufficiently strict here).

Note that I have tested this both with version 0.4.10, and with master, both of which include this recent patch that looks like it was an attempt to fix this.

In fact, looking at that fix it makes no sense to me:

      // BER/DER integers require leading zero byte to denote a positive value
      // when first byte >= 0x80
      if (sigbit === 56 || (sigbit >= 97 && sigbit <= 102))
        hex = '00' + hex;

You add 00 if hex starts with 8 or a-f. What about 9? In fact adding || sigbit === 57 fixes the issue!

I suggest using fewer magic numbers in your code to avoid this in future! It's also easy to check for !(0-7) than 8, 9, a-f, A-F (pretty risky to assume lowercase). Try this code:

      var sighex = hex.charCodeAt(0);

      const ASCII_0 = 0x30;
      const ASCII_7 = 0x38;
      // BER/DER integers require leading zero byte to denote a positive value
      // when first byte >= 0x80
      if (sighex < ASCII_0 || sighex > ASCII_7)
        hex = '00' + hex;

(Renamed sigbit because it isn't a bit - it is a nibble / hex character.)

claytongulick commented 3 years ago

This is currently preventing Mongodb Compass from connecting to a GCE instance using the gcloud compute config-ssh keys. I imagine the problem is pretty widespread, since any Electron app that uses an SSH tunnel (like MongoDB Compass) won't work on Mac.

mscdex commented 3 years ago

As long as modules/users are using modern versions of ssh2, this isn't a problem anymore (ssh2-streams is no longer used).