diff --git a/paramiko/ecdsakey.py b/paramiko/ecdsakey.py index 28d1222b3..3d3d09beb 100644 --- a/paramiko/ecdsakey.py +++ b/paramiko/ecdsakey.py @@ -292,10 +292,21 @@ def _decode_key(self, data): except (ValueError, AssertionError) as e: raise SSHException(str(e)) elif pkformat == self._PRIVATE_KEY_FORMAT_OPENSSH: - curve, verkey, sigkey = self._uint32_cstruct_unpack(data, "sss") try: - key = ec.derive_private_key(sigkey, curve, default_backend()) - except (AttributeError, TypeError) as e: + msg = Message(data) + curve_name = msg.get_text() + verkey = msg.get_binary() # noqa: F841 + sigkey = msg.get_mpint() + name = "ecdsa-sha2-" + curve_name + curve = self._ECDSA_CURVES.get_by_key_format_identifier(name) + if not curve: + raise SSHException("Invalid key curve identifier") + key = ec.derive_private_key( + sigkey, curve.curve_class(), default_backend() + ) + except Exception as e: + # PKey._read_private_key_openssh() should check or return + # keytype - parsing could fail for any reason due to wrong type raise SSHException(str(e)) else: self._got_bad_key_format_id(pkformat) diff --git a/paramiko/ed25519key.py b/paramiko/ed25519key.py index 96cff7d03..b584f5218 100644 --- a/paramiko/ed25519key.py +++ b/paramiko/ed25519key.py @@ -21,32 +21,12 @@ import nacl.signing -import six - from paramiko.message import Message -from paramiko.pkey import PKey +from paramiko.pkey import PKey, OPENSSH_AUTH_MAGIC, _unpad_openssh from paramiko.py3compat import b from paramiko.ssh_exception import SSHException, PasswordRequiredException -OPENSSH_AUTH_MAGIC = b"openssh-key-v1\x00" - - -def unpad(data): - # At the moment, this is only used for unpadding private keys on disk. This - # really ought to be made constant time (possibly by upstreaming this logic - # into pyca/cryptography). - padding_length = six.indexbytes(data, -1) - if 0x20 <= padding_length < 0x7f: - return data # no padding, last byte part comment (printable ascii) - if padding_length > 15: - raise SSHException("Invalid key") - for i in range(padding_length): - if six.indexbytes(data, i - padding_length) != i + 1: - raise SSHException("Invalid key") - return data[:-padding_length] - - class Ed25519Key(PKey): """ Representation of an `Ed25519 `_ key. @@ -155,7 +135,7 @@ def _parse_signing_key_data(self, data, password): decryptor.update(private_ciphertext) + decryptor.finalize() ) - message = Message(unpad(private_data)) + message = Message(_unpad_openssh(private_data)) if message.get_int() != message.get_int(): raise SSHException("Invalid key") diff --git a/paramiko/pkey.py b/paramiko/pkey.py index c6beef516..3a07426f1 100644 --- a/paramiko/pkey.py +++ b/paramiko/pkey.py @@ -27,6 +27,7 @@ import re import struct +import six import bcrypt from cryptography.hazmat.backends import default_backend @@ -35,18 +36,29 @@ from paramiko import util from paramiko.common import o600 -from paramiko.py3compat import ( - u, - encodebytes, - decodebytes, - b, - string_types, - byte_ord, -) +from paramiko.py3compat import u, b, encodebytes, decodebytes, string_types from paramiko.ssh_exception import SSHException, PasswordRequiredException from paramiko.message import Message +OPENSSH_AUTH_MAGIC = b"openssh-key-v1\x00" + + +def _unpad_openssh(data): + # At the moment, this is only used for unpadding private keys on disk. This + # really ought to be made constant time (possibly by upstreaming this logic + # into pyca/cryptography). + padding_length = six.indexbytes(data, -1) + if 0x20 <= padding_length < 0x7f: + return data # no padding, last byte part comment (printable ascii) + if padding_length > 15: + raise SSHException("Invalid key") + for i in range(padding_length): + if six.indexbytes(data, i - padding_length) != i + 1: + raise SSHException("Invalid key") + return data[:-padding_length] + + class PKey(object): """ Base class for public keys. @@ -395,8 +407,8 @@ def _read_private_key_openssh(self, lines, password): raise SSHException("base64 decoding error: {}".format(e)) # read data struct - auth_magic = data[:14] - if auth_magic != b("openssh-key-v1"): + auth_magic = data[:15] + if auth_magic != OPENSSH_AUTH_MAGIC: raise SSHException("unexpected OpenSSH key header encountered") cstruct = self._uint32_cstruct_unpack(data[15:], "sssur") @@ -466,9 +478,7 @@ def _read_private_key_openssh(self, lines, password): "OpenSSH private key file checkints do not match" ) - # Remove padding - padlen = byte_ord(keydata[len(keydata) - 1]) - return keydata[: len(keydata) - padlen] + return _unpad_openssh(keydata) def _uint32_cstruct_unpack(self, data, strformat): """ diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 0d68c26b5..8863f32d1 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,12 @@ Changelog ========= +- :bug:`1567` The new-style private key format (added in 2.7) suffered from an + unpadding bug which had been fixed earlier for Ed25519 (as that key type has + always used the newer format). That fix has been refactored and applied to + the base key class, courtesy of Pierce Lopez. +- :bug:`1565` (via :issue:`1566`) Fix a bug in support for ECDSA keys under the + newly supported OpenSSH key format. Thanks to Pierce Lopez for the patch. - :release:`2.7.0 <2019-12-03>` - :feature:`602` (via :issue:`1343`, :issue:`1313`, :issue:`618`) Implement support for OpenSSH 6.5-style private key files (typically denoted as having diff --git a/tests/test_ecdsa_384_openssh.key b/tests/test_ecdsa_384_openssh.key new file mode 100644 index 000000000..8a160ce28 --- /dev/null +++ b/tests/test_ecdsa_384_openssh.key @@ -0,0 +1,11 @@ +-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAACmFlczI1Ni1jdHIAAAAGYmNyeXB0AAAAGAAAABDwIHkBEZ +75XuqQS6/7daAIAAAAEAAAAAEAAACIAAAAE2VjZHNhLXNoYTItbmlzdHAzODQAAAAIbmlz +dHAzODQAAABhBIch5LXTq/L/TWsTGG6dIktxD8DIMh7EfvoRmWsks6CuNDTvFvbQNtY4QO +1mn5OXegHbS0M5DPIS++wpKGFP3suDEH08O35vZQasLNrL0tO2jyyEnzB2ZEx3PPYci811 +ygAAAOBKGxFl+JcMHjldOdTA9iwv88gxoelCwln/NATglUuyzHMLJwx53n8NLqrnHALvbz +RHjyTmjU4dbSM9o9Vjhcvq+1aipjAQg2qx825f7T4BMoKyhLBS/qTg7RfyW/h0Sbequ1wl +PhBfwhv0LUphRFsGdnOgrXWfZqWqxOP1WhJWIh1p+ja5va/Ii/+hD6RORQjvzbHTPJA53c +OguISImkx0vdqPuFTLyclaC3eO4Px68Ki0b8cdyivExbAWLkNOtBdIAgeO7Egbruu4O5Sn +I6bn1Kc+kZlWtO02IkwSA5DaKw== +-----END OPENSSH PRIVATE KEY----- diff --git a/tests/test_pkey.py b/tests/test_pkey.py index c949a6767..8d88545ad 100644 --- a/tests/test_pkey.py +++ b/tests/test_pkey.py @@ -40,6 +40,7 @@ PUB_ECDSA_521 = "ecdsa-sha2-nistp521 AAAAE2VjZHNhLXNoYTItbmlzdHA1MjEAAAAIbmlzdHA1MjEAAACFBACaOaFLZGuxa5AW16qj6VLypFbLrEWrt9AZUloCMefxO8bNLjK/O5g0rAVasar1TnyHE9qj4NwzANZASWjQNbc4MAG8vzqezFwLIn/kNyNTsXNfqEko9OgHZknlj2Z79dwTJcRAL4QLcT5aND0EHZLB2fAUDXiWIb2j4rg1mwPlBMiBXA==" # noqa PUB_RSA_2K_OPENSSH = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDF+Dpr54DX0WdeTDpNAMdkCWEkl3OXtNgf58qlN1gX572OLBqLf0zT4bHstUEpU3piazph/rSWcUMuBoD46tZ6jiH7H9b9Pem2eYQWaELDDkM+v9BMbEy5rMbFRLol5OtEvPFqneyEAanPOgvd8t3yyhSev9QVusakzJ8j8LGgrA8huYZ+Srnw0shEWLG70KUKCh3rG0QIvA8nfhtUOisr2Gp+F0YxMGb5gwBlQYAYE5l6u1SjZ7hNjyNosjK+wRBFgFFBYVpkZKJgWoK9w4ijFyzMZTucnZMqKOKAjIJvHfKBf2/cEfYxSq1EndqTqjYsd9T7/s2vcn1OH5a0wkER" # noqa PUB_DSS_1K_OPENSSH = "ssh-dss AAAAB3NzaC1kc3MAAACBAL8XEx7F9xuwBNles+vWpNF+YcofrBhjX1r5QhpBe0eoYWLHRcroN6lxwCdGYRfgOoRjTncBiixQX/uUxAY96zDh3ir492s2BcJt4ihvNn/AY0I0OTuX/2IwGk9CGzafjaeZNVYxMa8lcVt0hSOTjkPQ7gVuk6bJzMInvie+VWKLAAAAFQDUgYdY+rhR0SkKbC09BS/SIHcB+wAAAIB44+4zpCNcd0CGvZlowH99zyPX8uxQtmTLQFuR2O8O0FgVVuCdDgD0D9W8CLOp32oatpM0jyyN89EdvSWzjHzZJ+L6H1FtZps7uhpDFWHdva1R25vyGecLMUuXjo5t/D7oCDih+HwHoSAxoi0QvsPd8/qqHQVznNJKtR6thUpXEwAAAIAG4DCBjbgTTgpBw0egRkJwBSz0oTt+1IcapNU2jA6N8urMSk9YXHEQHKN68BAF3YJ59q2Ujv3LOXmBqGd1T+kzwUszfMlgzq8MMu19Yfzse6AIK1Agn1Vj6F7YXLsXDN+T4KszX5+FJa7t/Zsp3nALWy6l0f4WKivEF5Y2QpEFcQ==" # noqa +PUB_EC_384_OPENSSH = "ecdsa-sha2-nistp384 AAAAE2VjZHNhLXNoYTItbmlzdHAzODQAAAAIbmlzdHAzODQAAABhBIch5LXTq/L/TWsTGG6dIktxD8DIMh7EfvoRmWsks6CuNDTvFvbQNtY4QO1mn5OXegHbS0M5DPIS++wpKGFP3suDEH08O35vZQasLNrL0tO2jyyEnzB2ZEx3PPYci811yg==" # noqa FINGER_RSA = "1024 60:73:38:44:cb:51:86:65:7f:de:da:a2:2b:5a:57:d5" FINGER_DSS = "1024 44:78:f0:b9:a2:3c:c5:18:20:09:ff:75:5b:c1:d2:6c" @@ -49,6 +50,7 @@ SIGNED_RSA = "20:d7:8a:31:21:cb:f7:92:12:f2:a4:89:37:f5:78:af:e6:16:b6:25:b9:97:3d:a2:cd:5f:ca:20:21:73:4c:ad:34:73:8f:20:77:28:e2:94:15:08:d8:91:40:7a:85:83:bf:18:37:95:dc:54:1a:9b:88:29:6c:73:ca:38:b4:04:f1:56:b9:f2:42:9d:52:1b:29:29:b4:4f:fd:c9:2d:af:47:d2:40:76:30:f3:63:45:0c:d9:1d:43:86:0f:1c:70:e2:93:12:34:f3:ac:c5:0a:2f:14:50:66:59:f1:88:ee:c1:4a:e9:d1:9c:4e:46:f0:0e:47:6f:38:74:f1:44:a8" # noqa FINGER_RSA_2K_OPENSSH = "2048 68:d1:72:01:bf:c0:0c:66:97:78:df:ce:75:74:46:d6" FINGER_DSS_1K_OPENSSH = "1024 cf:1d:eb:d7:61:d3:12:94:c6:c0:c6:54:35:35:b0:82" +FINGER_EC_384_OPENSSH = "384 72:14:df:c1:9a:c3:e6:0e:11:29:d6:32:18:7b:ea:9b" RSA_PRIVATE_OUT = """\ -----BEGIN RSA PRIVATE KEY----- @@ -463,6 +465,17 @@ def test_load_openssh_format_DSS_key(self): my_rsa = hexlify(key.get_fingerprint()) self.assertEqual(exp_rsa, my_rsa) + def test_load_openssh_format_EC_key(self): + key = ECDSAKey.from_private_key_file( + _support("test_ecdsa_384_openssh.key"), b"television" + ) + self.assertEqual("ecdsa-sha2-nistp384", key.get_name()) + self.assertEqual(PUB_EC_384_OPENSSH.split()[1], key.get_base64()) + self.assertEqual(384, key.get_bits()) + exp_fp = b(FINGER_EC_384_OPENSSH.split()[1].replace(":", "")) + my_fp = hexlify(key.get_fingerprint()) + self.assertEqual(exp_fp, my_fp) + def test_salt_size(self): # Read an existing encrypted private key file_ = _support("test_rsa_password.key") @@ -481,6 +494,10 @@ def test_salt_size(self): finally: os.remove(newfile) + def test_load_openssh_format_RSA_nopad(self): + # check just not exploding with 'Invalid key' + RSAKey.from_private_key_file(_support("test_rsa_openssh_nopad.key")) + def test_stringification(self): key = RSAKey.from_private_key_file(_support("test_rsa.key")) comparable = TEST_KEY_BYTESTR_2 if PY2 else TEST_KEY_BYTESTR_3 diff --git a/tests/test_rsa_openssh_nopad.key b/tests/test_rsa_openssh_nopad.key new file mode 100644 index 000000000..61ac1b199 --- /dev/null +++ b/tests/test_rsa_openssh_nopad.key @@ -0,0 +1,27 @@ +-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAABFwAAAAdzc2gtcn +NhAAAAAwEAAQAAAQEAnyMwWSwrbJxxQZWMJO5xR6eAA9De4t3GViqDRaQt/BgsvzZ14SUz +aOL/A370fKxhx/JLIOOGA0o5B0/ct+CL7XFqMi5r5+iA9VcIeYKKtoAkrEvRnagNW0WVWx +thTnE01g8Pb7fDqzI2cBuBNZ2vGNm2m4UTGC8/kl/0ES1V3KqA7lPlTrkTYg9L/ornvVHc +c8gEbMwx9XXVRzbWiuDE176ojrudY9CZduVSOgW+HK3rKkqLBs/91jv0zUK0oqTQBLR7E2 +V2GWPDU4BjlHTtYr0jpKOGDr1DLu4+NiD/mX+tGMdH6ehbDii0kXmOUaZjs4OxuK3XA/gi +KZLdj1jQQwAAA7iNnvAVjZ7wFQAAAAdzc2gtcnNhAAABAQCfIzBZLCtsnHFBlYwk7nFHp4 +AD0N7i3cZWKoNFpC38GCy/NnXhJTNo4v8DfvR8rGHH8ksg44YDSjkHT9y34IvtcWoyLmvn +6ID1Vwh5goq2gCSsS9GdqA1bRZVbG2FOcTTWDw9vt8OrMjZwG4E1na8Y2babhRMYLz+SX/ +QRLVXcqoDuU+VOuRNiD0v+iue9UdxzyARszDH1ddVHNtaK4MTXvqiOu51j0Jl25VI6Bb4c +resqSosGz/3WO/TNQrSipNAEtHsTZXYZY8NTgGOUdO1ivSOko4YOvUMu7j42IP+Zf60Yx0 +fp6FsOKLSReY5RpmOzg7G4rdcD+CIpkt2PWNBDAAAAAwEAAQAAAQEAnmMbn+VCYxth7fC2 +R5u6y6J+201sSUiKOwCdHxdFXX+CKd4+fRPVkzM6tXQKSnwX5jXVaKqLm4KoOArYl3q6Sl +1zYParF2plz8oL+URgYzwvQ/1CaDP29zzOZptdwgESoWrj5kF0UlPrsrDtbTvAJm+qPCe6 +1XtRPpKaDO6eYr0PM2QTElZy3mDBUBvu816LdG/ZtnB9g5UsocT5mmhpHTHdjrpwNu5TBe +ACVodDn5Fu66OlrrnQi4IPCAWKJ1YuzEkZqLhs1L3oMHACsmzrLjzW74SjY4kWTTvGiC6i +tDoycycThk9EGLGNso99Q1fe84/OZUff7aI3yK9KvLL7oQAAAIEAh2+XrJXSBx/v9E3aJH +ncgQH1snXr7LcSRqcWicHdbm8JsOTT3TkyXHGlSZ2rr/Y0u5V1ZSO6roJLrAHsDJzx0x0U +xE/5mpzhD+yIKQwnWkZFLzYEnYDFdXDMzmghUIik9AW7n9dtS8UtVFGaL6Vs2YCOuLqeT9 +nZUkm3UUZ+7QIAAACBAM23DFjQ0/Op2ri7fJA2qFBdXqoJdNHuyYEIrKbB6XaaSUz52+IB +MbccxEz3vPsHh69tZoJ+xZNbFJe9wdmbF+DQpoukHkJnzpk/pUq8LjQMzZfwv41X8zqaq4 +AOA7g27Rk8aKewhCXjhkr0hHEaSiuqIIindFaFti5sQMi2mtkXAAAAgQDGCXkpuKZK61p9 +L6G5yZSQBCgVtm0iQEbyDXWHjy/GqLtxJjqdyaRK57hXGjbzgJJraSy+sNP9uv2QOvyZvB +3XaPWwUYVQ34WyibCqqUaPiHxX7T1lZV+asbwgbmSqYtH5dUEJ8zT572mCwxnRjX63PwDo +5vBbR/qAW5lvRYsltQAAAAFh +-----END OPENSSH PRIVATE KEY-----