Merge branch '2.7'

intgr · Dec 9, 2019 · 2e6d0a4 · 2e6d0a4
2 parents 961c0e6 + 573f03f
commit 2e6d0a4
Show file tree

Hide file tree

Showing 7 changed files with 100 additions and 38 deletions.
diff --git 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
@@ -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 <https://ed25519.cr.yp.to/>`_ 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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-----