Overhaul PublicBlob and use it better within RSAKey.

This allows server-side Paramiko code to correctly create
cert-bearing RSAKey objects and thus verify client signatures,
and now the test suite passes again, barring the stub tests.

Re #1042

5 files changed, 102 insertions, 35 deletions

diff --git a/paramiko/client.py b/paramiko/client.py
index 3b3895b6..bff223ea 100644
--- a/paramiko/client.py
+++ b/paramiko/client.py
@@ -541,7 +541,7 @@ class SSHClient (ClosingContextManager):
         self._log(DEBUG, msg)
         # Attempt to load cert if it exists.
         if os.path.isfile(cert_path):
-            key.load_certificate(pubkey_filename=cert_path)
+            key.load_certificate(cert_path)
             self._log(DEBUG, "Adding public certificate {0}".format(cert_path))
         return key
 
diff --git a/paramiko/pkey.py b/paramiko/pkey.py
index 3a872491..948152fa 100644
--- a/paramiko/pkey.py
+++ b/paramiko/pkey.py
@@ -365,10 +365,11 @@ class PKey(object):
             encryption
         ).decode())
 
-    def load_certificate(self, **kwargs):
+    def load_certificate(self, value):
         """
-        Supplement the private key contents with data loaded from
-        an OpenSSH public key (.pub) or certificate (-cert.pub) file.
+        Supplement the private key contents with data loaded from an OpenSSH
+        public key (``.pub``) or certificate (``-cert.pub``) file, a string
+        containing such a file, or a `.Message` object.
 
         The .pub contents adds no real value, since the private key
         file includes sufficient information to derive the public
@@ -382,7 +383,13 @@ class PKey(object):
         that is for the server to decide if it is good enough to authenticate
         successfully.
         """
-        blob = PublicBlob(**kwargs)
+        if isinstance(value, Message):
+            constructor = 'from_message'
+        elif os.path.isfile(value):
+            constructor = 'from_file'
+        else:
+            constructor = 'from_string'
+        blob = getattr(PublicBlob, constructor)(value)
         if not blob.key_type.startswith(self.get_name()):
             raise ValueError('PublicBlob type %s incompatible with key type %s' %
                 (blob.key_type, self.get_name()))
@@ -396,36 +403,74 @@ class PKey(object):
 # {ssh-rsa, ssh-dss, ssh-ecdsa, ssh-ed25519}, but should
 # provide rudimentary support for {*-cert.v01}
 class PublicBlob(object):
-    '''
-    OpenSSH plain public key or OpenSSH signed public key (certificate)
-    A mostly dumb container
-    '''
-    def __init__(self, pubkey_filename=None, pubkey_string=None):
-        '''
-        Can read from a file or string.
-        '''
-        if pubkey_filename:
-            with open(pubkey_filename) as f:
-                fields = f.read().split(None, 2)
-        elif pubkey_string:
-            fields = pubkey_string.split(None, 2)
-        else:
-            raise ValueError('PublicBlob() requires either a pubkey_filename or pubkey_string')
+    """
+    OpenSSH plain public key or OpenSSH signed public key (certificate).
+
+    Tries to be as dumb as possible and barely cares about specific
+    per-key-type data.
+
+    ..note::
+        Most of the time you'll want to call `from_file`, `from_string` or
+        `from_message` for useful instantiation, the main constructor is
+        basically "I should be using ``attrs`` for this."
+    """
+    def __init__(self, type_, blob, comment=None):
+        """
+        Create a new public blob of given type and contents.
+
+        :param str type_: Type indicator, eg ``ssh-rsa``.
+        :param blob: The blob bytes themselves.
+        :param str comment: A comment, if one was given (e.g. file-based.)
+        """
+        self.key_type = type_
+        self.key_blob = blob
+        self.comment = comment
+
+    @classmethod
+    def from_file(cls, filename):
+        """
+        Create a public blob from a ``-cert.pub``-style file on disk.
+        """
+        with open(filename) as f:
+            string = f.read()
+        return cls.from_string(string)
+
+    @classmethod
+    def from_string(cls, string):
+        """
+        Create a public blob from a ``-cert.pub``-style string.
+        """
+        fields = string.split(None, 2)
         if len(fields) < 2:
-            raise ValueError('PublicBlob() not enough fields %s', fields)
-        self.key_type = fields[0]
-        self.key_blob = decodebytes(fields[1])
+            msg = "Not enough fields for public blob: {0}"
+            raise ValueError(msg.format(fields))
+        key_type = fields[0]
+        key_blob = decodebytes(fields[1])
         try:
-            self.comment = fields[2].strip()
+            comment = fields[2].strip()
         except IndexError:
-            self.comment = None
-        # Verify that the blob message first (string) field matches the key_type
-        m = Message(self.key_blob)
+            comment = None
+        # Verify that the blob message first (string) field matches the
+        # key_type
+        m = Message(key_blob)
         blob_type = m.get_string()
-        if blob_type != self.key_type:
-            raise ValueError(
-                'Invalid PublicBlob contents. Key type [%s], expected [%s]' %
-                    (blob_type, self.key_type))
+        if blob_type != key_type:
+            msg = "Invalid PublicBlob contents: key type {0!r}, expected {1!r}"
+            raise ValueError(msg.format(blob_type, key_type))
+        # All good? All good.
+        return cls(type_=key_type, blob=key_blob, comment=comment)
+
+    @classmethod
+    def from_message(cls, message):
+        """
+        Create a public blob from a network `.Message`.
+
+        Specifically, a cert-bearing pubkey auth packet, because by definition
+        OpenSSH-style certificates 'are' their own network representation."
+        """
+        type_ = message.get_text()
+        message.rewind()
+        return cls(type_=type_, blob=message.asbytes())
 
     def __str__(self):
         if self.comment:
diff --git a/paramiko/rsakey.py b/paramiko/rsakey.py
index 7abcfa28..3f28689a 100644
--- a/paramiko/rsakey.py
+++ b/paramiko/rsakey.py
@@ -54,8 +54,26 @@ class RSAKey(PKey):
         else:
             if msg is None:
                 raise SSHException('Key object may not be empty')
-            if msg.get_text() != 'ssh-rsa':
+            type_ = msg.get_text()
+            nonce = None
+            # Regular public key - nothing special to do besides the implicit
+            # type check.
+            if type_ == 'ssh-rsa':
+                pass
+            # OpenSSH-compatible certificate - store full copy as .public_blob
+            # (so signing works correctly) and then fast-forward past the
+            # nonce.
+            elif type_ == 'ssh-rsa-cert-v01@openssh.com':
+                # This seems the cleanest way to 'clone' an already-being-read
+                # message?
+                self.load_certificate(Message(msg.asbytes()))
+                # Read out nonce as it comes before the public numbers.
+                # TODO: usefully interpret it & other non-public-number fields
+                nonce = msg.get_string()
+            else:
                 raise SSHException('Invalid key')
+            # Now that we've read type and (possibly) nonce, public numbers are
+            # next in either case.
             self.key = rsa.RSAPublicNumbers(
                 e=msg.get_mpint(), n=msg.get_mpint()
             ).public_key(default_backend())
diff --git a/paramiko/transport.py b/paramiko/transport.py
index 388f60cb..0dc2e28a 100644
--- a/paramiko/transport.py
+++ b/paramiko/transport.py
@@ -204,6 +204,7 @@ class Transport(threading.Thread, ClosingContextManager):
 
     _key_info = {
         'ssh-rsa': RSAKey,
+        'ssh-rsa-cert-v01@openssh.com': RSAKey,
         'ssh-dss': DSSKey,
         'ecdsa-sha2-nistp256': ECDSAKey,
         'ecdsa-sha2-nistp384': ECDSAKey,
diff --git a/tests/test_pkey.py b/tests/test_pkey.py
index 034331a2..dac1d02b 100644
--- a/tests/test_pkey.py
+++ b/tests/test_pkey.py
@@ -485,7 +485,7 @@ class KeyTest(unittest.TestCase):
         # PKey.load_certificate
         key = RSAKey.from_private_key_file(test_path('test_rsa.key'))
         self.assertTrue(key.public_blob is None)
-        key.load_certificate(pubkey_filename=test_path('test_rsa.key-cert.pub'))
+        key.load_certificate(test_path('test_rsa.key-cert.pub'))
         self.assertTrue(key.public_blob is not None)
         self.assertEqual(key.public_blob.key_type, 'ssh-rsa-cert-v01@openssh.com')
         self.assertEqual(key.public_blob.comment, 'test_rsa.key.pub')
@@ -502,5 +502,8 @@ class KeyTest(unittest.TestCase):
 
         # Prevented from loading certificate that doesn't match
         key1 = Ed25519Key.from_private_key_file(test_path('test_ed25519.key'))
-        self.assertRaises(ValueError, key1.load_certificate,
-            pubkey_filename=test_path('test_rsa.key-cert.pub'))
+        self.assertRaises(
+            ValueError,
+            key1.load_certificate,
+            test_path('test_rsa.key-cert.pub'),
+        )