Merge pull request #1532 from reaperhulk/fix-1531

add OpenSSH DSS public key loading

3 files changed, 167 insertions, 33 deletions

diff --git a/docs/hazmat/primitives/asymmetric/serialization.rst b/docs/hazmat/primitives/asymmetric/serialization.rst
index a9392c7b..b523c342 100644
--- a/docs/hazmat/primitives/asymmetric/serialization.rst
+++ b/docs/hazmat/primitives/asymmetric/serialization.rst
@@ -201,8 +201,8 @@ OpenSSH Public Key
 
 The format used by OpenSSH to store public keys, as specified in :rfc:`4253`.
 
-Currently, only RSA public keys are supported. Any other type of key will
-result in an exception being thrown.
+Currently, only RSA and DSA public keys are supported. Any other type of key
+will result in an exception being thrown.
 
 An example RSA key in OpenSSH format (line breaks added for formatting
 purposes)::
@@ -215,6 +215,9 @@ purposes)::
     ///ImSCGHQRvhwariN2tvZ6CBNSLh3iQgeB0AkyJlng7MXB2qYq/Ci2FUOryCX
     2MzHvnbv testkey@localhost
 
+DSA keys look almost identical but begin with ``ssh-dss`` rather than
+``ssh-rsa``.
+
 .. function:: load_ssh_public_key(data, backend)
 
     .. versionadded:: 0.7
@@ -224,8 +227,10 @@ purposes)::
 
     :param bytes data: The OpenSSH encoded key data.
 
-    :param backend: An
-        :class:`~cryptography.hazmat.backends.interfaces.RSABackend` provider.
+    :param backend: A backend providing
+        :class:`~cryptography.hazmat.backends.interfaces.RSABackend` or
+        :class:`~cryptography.hazmat.backends.interfaces.DSABackend` depending
+        on key type.
 
     :returns: A new instance of a public key type.
 
diff --git a/src/cryptography/hazmat/primitives/serialization.py b/src/cryptography/hazmat/primitives/serialization.py
index 0dbbc85c..9d384fc7 100644
--- a/src/cryptography/hazmat/primitives/serialization.py
+++ b/src/cryptography/hazmat/primitives/serialization.py
@@ -10,6 +10,9 @@ import warnings
 
 from cryptography import utils
 from cryptography.exceptions import UnsupportedAlgorithm
+from cryptography.hazmat.primitives.asymmetric.dsa import (
+    DSAParameterNumbers, DSAPublicNumbers
+)
 from cryptography.hazmat.primitives.asymmetric.rsa import RSAPublicNumbers
 
 
@@ -55,19 +58,23 @@ def load_ssh_public_key(data, backend):
     key_type = key_parts[0]
     key_body = key_parts[1]
 
-    if not key_type.startswith(b'ssh-'):
-        raise ValueError('SSH-formatted keys must begin with \'ssh-\'.')
+    try:
+        decoded_data = base64.b64decode(key_body)
+    except TypeError:
+        raise ValueError('Key is not in the proper format.')
 
-    if not key_type.startswith(b'ssh-rsa'):
-        raise UnsupportedAlgorithm('Only RSA keys are currently supported.')
+    if key_type == b'ssh-rsa':
+        return _load_ssh_rsa_public_key(decoded_data, backend)
+    elif key_type == b'ssh-dss':
+        return _load_ssh_dss_public_key(decoded_data, backend)
+    else:
+        raise UnsupportedAlgorithm(
+            'Only RSA and DSA keys are currently supported.'
+        )
 
-    return _load_ssh_rsa_public_key(key_body, backend)
 
-
-def _load_ssh_rsa_public_key(key_body, backend):
-    data = base64.b64decode(key_body)
-
-    key_type, rest = _read_next_string(data)
+def _load_ssh_rsa_public_key(decoded_data, backend):
+    key_type, rest = _read_next_string(decoded_data)
     e, rest = _read_next_mpint(rest)
     n, rest = _read_next_mpint(rest)
 
@@ -81,6 +88,26 @@ def _load_ssh_rsa_public_key(key_body, backend):
     return backend.load_rsa_public_numbers(RSAPublicNumbers(e, n))
 
 
+def _load_ssh_dss_public_key(decoded_data, backend):
+    key_type, rest = _read_next_string(decoded_data)
+    p, rest = _read_next_mpint(rest)
+    q, rest = _read_next_mpint(rest)
+    g, rest = _read_next_mpint(rest)
+    y, rest = _read_next_mpint(rest)
+
+    if key_type != b'ssh-dss':
+        raise ValueError(
+            'Key header and key body contain different key type values.')
+
+    if rest:
+        raise ValueError('Key body contains extra bytes.')
+
+    parameter_numbers = DSAParameterNumbers(p, q, g)
+    public_numbers = DSAPublicNumbers(y, parameter_numbers)
+
+    return backend.load_dsa_public_numbers(public_numbers)
+
+
 def _read_next_string(data):
     """Retrieves the next RFC 4251 string value from the data."""
     str_len, = struct.unpack('>I', data[:4])
diff --git a/tests/hazmat/primitives/test_serialization.py b/tests/hazmat/primitives/test_serialization.py
index abb55751..91db318c 100644
--- a/tests/hazmat/primitives/test_serialization.py
+++ b/tests/hazmat/primitives/test_serialization.py
@@ -11,11 +11,15 @@ import pytest
 
 from cryptography.exceptions import UnsupportedAlgorithm, _Reasons
 from cryptography.hazmat.backends.interfaces import (
-    EllipticCurveBackend, PEMSerializationBackend, PKCS8SerializationBackend,
-    RSABackend, TraditionalOpenSSLSerializationBackend
+    DSABackend, EllipticCurveBackend, PEMSerializationBackend,
+    PKCS8SerializationBackend, RSABackend,
+    TraditionalOpenSSLSerializationBackend
 )
 from cryptography.hazmat.primitives import interfaces
 from cryptography.hazmat.primitives.asymmetric import ec
+from cryptography.hazmat.primitives.asymmetric.dsa import (
+    DSAParameterNumbers, DSAPublicNumbers
+)
 from cryptography.hazmat.primitives.asymmetric.rsa import RSAPublicNumbers
 from cryptography.hazmat.primitives.serialization import (
     load_pem_pkcs8_private_key, load_pem_private_key, load_pem_public_key,
@@ -684,15 +688,15 @@ class TestPKCS8Serialization(object):
 
 
 @pytest.mark.requires_backend_interface(interface=RSABackend)
-class TestSSHSerialization(object):
+class TestRSASSHSerialization(object):
     def test_load_ssh_public_key_unsupported(self, backend):
-        ssh_key = b'ssh-dss AAAAB3NzaC1kc3MAAACBAO7q0a7VsQZcdRTCqFentQt...'
+        ssh_key = b'ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTY='
 
         with pytest.raises(UnsupportedAlgorithm):
             load_ssh_public_key(ssh_key, backend)
 
     def test_load_ssh_public_key_bad_format(self, backend):
-        ssh_key = b'not-a-real-key text'
+        ssh_key = b'ssh-rsa not-a-real-key'
 
         with pytest.raises(ValueError):
             load_ssh_public_key(ssh_key, backend)
@@ -703,20 +707,6 @@ class TestSSHSerialization(object):
         with pytest.raises(ValueError):
             load_ssh_public_key(ssh_key, backend)
 
-    def test_load_ssh_public_key_rsa_key_types_dont_match(self, backend):
-        ssh_key = (
-            b"ssh-bad AAAAB3NzaC1yc2EAAAADAQABAAABAQDDu/XRP1kyK6Cgt36gts9XAk"
-            b"FiiuJLW6RU0j3KKVZSs1I7Z3UmU9/9aVh/rZV43WQG8jaR6kkcP4stOR0DEtll"
-            b"PDA7ZRBnrfiHpSQYQ874AZaAoIjgkv7DBfsE6gcDQLub0PFjWyrYQUJhtOLQEK"
-            b"vY/G0vt2iRL3juawWmCFdTK3W3XvwAdgGk71i6lHt+deOPNEPN2H58E4odrZ2f"
-            b"sxn/adpDqfb2sM0kPwQs0aWvrrKGvUaustkivQE4XWiSFnB0oJB/lKK/CKVKuy"
-            b"///ImSCGHQRvhwariN2tvZ6CBNSLh3iQgeB0AkyJlng7MXB2qYq/Ci2FUOryCX"
-            b"2MzHvnbv testkey@localhost extra"
-        )
-
-        with pytest.raises(ValueError):
-            load_ssh_public_key(ssh_key, backend)
-
     def test_load_ssh_public_key_rsa_extra_string_after_comment(self, backend):
         ssh_key = (
             b"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDDu/XRP1kyK6Cgt36gts9XAk"
@@ -796,3 +786,115 @@ class TestSSHSerialization(object):
         expected = RSAPublicNumbers(expected_e, expected_n)
 
         assert numbers == expected
+
+
+@pytest.mark.requires_backend_interface(interface=DSABackend)
+class TestDSSSSHSerialization(object):
+    def test_load_ssh_public_key_dss_too_short(self, backend):
+        ssh_key = b'ssh-dss'
+
+        with pytest.raises(ValueError):
+            load_ssh_public_key(ssh_key, backend)
+
+    def test_load_ssh_public_key_dss_extra_string_after_comment(self, backend):
+        ssh_key = (
+            b"ssh-dss AAAAB3NzaC1kc3MAAACBALmwUtfwdjAUjU2Dixd5DvT0NDcjjr69UD"
+            b"LqSD/Xt5Al7D3GXr1WOrWGpjO0NE9qzRCvMTU7zykRH6XjuNXB6Hvv48Zfm4vm"
+            b"nHQHFmmMg2bI75JbnOwdzWnnPZJrVU4rS23dFFPqs5ug+EbhVVrcwzxahjcSjJ"
+            b"7WEQSkVQWnSPbbAAAAFQDXmpD3DIkGvLSBf1GdUF4PHKtUrQAAAIB/bJFwss+2"
+            b"fngmfG/Li5OyL7A9iVoGdkUaFaxEUROTp7wkm2z49fXFAir+/U31v50Tu98YLf"
+            b"WvKlxdHcdgQYV9Ww5LIrhWwwD4UKOwC6w5S3KHVbi3pWUi7vxJFXOWfeu1mC/J"
+            b"TWqMKR91j+rmOtdppWIZRyIVIqLcMdGO3m+2VgAAAIANFDz5KQH5NvoljpoRQi"
+            b"RgyPjxWXiE7vjLElKj4v8KrpanAywBzdhIW1y/tzpGuwRwj5ihi8iNTHgSsoTa"
+            b"j5AG5HPomJf5vJElxpu/2O9pHA52wcNObIQ7j+JA5uWusxNIbl+pF6sSiP8abr"
+            b"z53N7tPF/IhHTjBHb1Ol7IFu9p9A== testkey@localhost extra"
+        )
+
+        with pytest.raises(ValueError):
+            load_ssh_public_key(ssh_key, backend)
+
+    def test_load_ssh_public_key_dss_extra_data_after_modulo(self, backend):
+        ssh_key = (
+            b"ssh-dss AAAAB3NzaC1kc3MAAACBALmwUtfwdjAUjU2Dixd5DvT0NDcjjr69UD"
+            b"LqSD/Xt5Al7D3GXr1WOrWGpjO0NE9qzRCvMTU7zykRH6XjuNXB6Hvv48Zfm4vm"
+            b"nHQHFmmMg2bI75JbnOwdzWnnPZJrVU4rS23dFFPqs5ug+EbhVVrcwzxahjcSjJ"
+            b"7WEQSkVQWnSPbbAAAAFQDXmpD3DIkGvLSBf1GdUF4PHKtUrQAAAIB/bJFwss+2"
+            b"fngmfG/Li5OyL7A9iVoGdkUaFaxEUROTp7wkm2z49fXFAir+/U31v50Tu98YLf"
+            b"WvKlxdHcdgQYV9Ww5LIrhWwwD4UKOwC6w5S3KHVbi3pWUi7vxJFXOWfeu1mC/J"
+            b"TWqMKR91j+rmOtdppWIZRyIVIqLcMdGO3m+2VgAAAIANFDz5KQH5NvoljpoRQi"
+            b"RgyPjxWXiE7vjLElKj4v8KrpanAywBzdhIW1y/tzpGuwRwj5ihi8iNTHgSsoTa"
+            b"j5AG5HPomJf5vJElxpu/2O9pHA52wcNObIQ7j+JA5uWusxNIbl+pF6sSiP8abr"
+            b"z53N7tPF/IhHTjBHb1Ol7IFu9p9AAwMD== testkey@localhost"
+        )
+
+        with pytest.raises(ValueError):
+            load_ssh_public_key(ssh_key, backend)
+
+    def test_load_ssh_public_key_dss_different_string(self, backend):
+        ssh_key = (
+            # "AAAAB3NzA" the final A is capitalized here to cause the string
+            # ssh-dss inside the base64 encoded blob to be incorrect. It should
+            # be a lower case 'a'.
+            b"ssh-dss AAAAB3NzAC1kc3MAAACBALmwUtfwdjAUjU2Dixd5DvT0NDcjjr69UD"
+            b"LqSD/Xt5Al7D3GXr1WOrWGpjO0NE9qzRCvMTU7zykRH6XjuNXB6Hvv48Zfm4vm"
+            b"nHQHFmmMg2bI75JbnOwdzWnnPZJrVU4rS23dFFPqs5ug+EbhVVrcwzxahjcSjJ"
+            b"7WEQSkVQWnSPbbAAAAFQDXmpD3DIkGvLSBf1GdUF4PHKtUrQAAAIB/bJFwss+2"
+            b"fngmfG/Li5OyL7A9iVoGdkUaFaxEUROTp7wkm2z49fXFAir+/U31v50Tu98YLf"
+            b"WvKlxdHcdgQYV9Ww5LIrhWwwD4UKOwC6w5S3KHVbi3pWUi7vxJFXOWfeu1mC/J"
+            b"TWqMKR91j+rmOtdppWIZRyIVIqLcMdGO3m+2VgAAAIANFDz5KQH5NvoljpoRQi"
+            b"RgyPjxWXiE7vjLElKj4v8KrpanAywBzdhIW1y/tzpGuwRwj5ihi8iNTHgSsoTa"
+            b"j5AG5HPomJf5vJElxpu/2O9pHA52wcNObIQ7j+JA5uWusxNIbl+pF6sSiP8abr"
+            b"z53N7tPF/IhHTjBHb1Ol7IFu9p9A== testkey@localhost"
+        )
+        with pytest.raises(ValueError):
+            load_ssh_public_key(ssh_key, backend)
+
+    def test_load_ssh_public_key_dss(self, backend):
+        ssh_key = (
+            b"ssh-dss AAAAB3NzaC1kc3MAAACBALmwUtfwdjAUjU2Dixd5DvT0NDcjjr69UD"
+            b"LqSD/Xt5Al7D3GXr1WOrWGpjO0NE9qzRCvMTU7zykRH6XjuNXB6Hvv48Zfm4vm"
+            b"nHQHFmmMg2bI75JbnOwdzWnnPZJrVU4rS23dFFPqs5ug+EbhVVrcwzxahjcSjJ"
+            b"7WEQSkVQWnSPbbAAAAFQDXmpD3DIkGvLSBf1GdUF4PHKtUrQAAAIB/bJFwss+2"
+            b"fngmfG/Li5OyL7A9iVoGdkUaFaxEUROTp7wkm2z49fXFAir+/U31v50Tu98YLf"
+            b"WvKlxdHcdgQYV9Ww5LIrhWwwD4UKOwC6w5S3KHVbi3pWUi7vxJFXOWfeu1mC/J"
+            b"TWqMKR91j+rmOtdppWIZRyIVIqLcMdGO3m+2VgAAAIANFDz5KQH5NvoljpoRQi"
+            b"RgyPjxWXiE7vjLElKj4v8KrpanAywBzdhIW1y/tzpGuwRwj5ihi8iNTHgSsoTa"
+            b"j5AG5HPomJf5vJElxpu/2O9pHA52wcNObIQ7j+JA5uWusxNIbl+pF6sSiP8abr"
+            b"z53N7tPF/IhHTjBHb1Ol7IFu9p9A== testkey@localhost"
+        )
+
+        key = load_ssh_public_key(ssh_key, backend)
+
+        assert key is not None
+        assert isinstance(key, interfaces.DSAPublicKey)
+
+        numbers = key.public_numbers()
+
+        expected_y = int(
+            "d143cf92901f936fa258e9a11422460c8f8f1597884eef8cb1252a3e2ff0aae"
+            "96a7032c01cdd8485b5cbfb73a46bb04708f98a18bc88d4c7812b284da8f900"
+            "6e473e89897f9bc9125c69bbfd8ef691c0e76c1c34e6c843b8fe240e6e5aeb3"
+            "13486e5fa917ab1288ff1a6ebcf9dcdeed3c5fc88474e30476f53a5ec816ef6"
+            "9f4", 16
+        )
+        expected_p = int(
+            "b9b052d7f07630148d4d838b17790ef4f43437238ebebd5032ea483fd7b7902"
+            "5ec3dc65ebd563ab586a633b4344f6acd10af31353bcf29111fa5e3b8d5c1e8"
+            "7befe3c65f9b8be69c740716698c8366c8ef925b9cec1dcd69e73d926b554e2"
+            "b4b6ddd1453eab39ba0f846e1555adcc33c5a8637128c9ed61104a45505a748"
+            "f6db", 16
+        )
+        expected_q = 1230879958723280233885494314531920096931919647917
+        expected_g = int(
+            "7f6c9170b2cfb67e78267c6fcb8b93b22fb03d895a0676451a15ac44511393a"
+            "7bc249b6cf8f5f5c5022afefd4df5bf9d13bbdf182df5af2a5c5d1dc7604185"
+            "7d5b0e4b22b856c300f850a3b00bac394b728755b8b7a56522eefc491573967"
+            "debb5982fc94d6a8c291f758feae63ad769a5621947221522a2dc31d18ede6f"
+            "b656", 16
+        )
+        expected = DSAPublicNumbers(
+            expected_y,
+            DSAParameterNumbers(expected_p, expected_q, expected_g)
+        )
+
+        assert numbers == expected