Merge lp:~cjwatson/lazr.sshserver/work-around-short-paramiko-signatures into lp:lazr.sshserver

[Colin Watson]

Work around RSA signature truncation bug in paramiko < 2.0.0.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'src/lazr/sshserver/NEWS.txt'
--- src/lazr/sshserver/NEWS.txt	2015-11-29 12:33:03 +0000
+++ src/lazr/sshserver/NEWS.txt	2017-12-21 11:41:35 +0000
@@ -2,6 +2,15 @@
 NEWS for lazr.sshserver
 =======================
 
+0.1.5
+=====
+
+- Work around a bug in paramiko < 2.0.0: if the most significant byte of an
+  RSA signature is zero, then it strips leading zero bytes rather than
+  zero-padding it to the correct length.  This previously caused sporadic
+  failures when running with Twisted >= 16.0.0, since cryptography is
+  pickier about this than PyCrypto was.
+
 0.1.4 (2015-11-29)
 ==================
 

=== modified file 'src/lazr/sshserver/auth.py'
--- src/lazr/sshserver/auth.py	2017-12-21 11:04:23 +0000
+++ src/lazr/sshserver/auth.py	2017-12-21 11:41:35 +0000
@@ -244,6 +244,16 @@
         algName, blob, rest = getNS(packet[1:], 2)
         pubKey = keys.Key.fromString(blob).keyObject
         signature = hasSig and getNS(rest)[0] or None
+        # Work around a bug in paramiko < 2.0.0: if the most significant
+        # byte of an RSA signature is zero, then it strips leading zero
+        # bytes rather than zero-padding it to the correct length.
+        if algName == 'ssh-rsa':
+            signatureType, rawSignature, rest = getNS(signature, 2)
+            pubKeyLen = (pubKey.size() + 7) // 8
+            if len(rawSignature) < pubKeyLen:
+                rawSignature = (
+                    b'\x00' * (pubKeyLen - len(rawSignature)) + rawSignature)
+                signature = NS(signatureType) + NS(rawSignature) + rest
         if hasSig:
             b = NS(self.transport.sessionID) + \
                 chr(userauth.MSG_USERAUTH_REQUEST) +  NS(self.user) + \

=== modified file 'src/lazr/sshserver/tests/test_auth.py'
--- src/lazr/sshserver/tests/test_auth.py	2017-12-20 14:40:54 +0000
+++ src/lazr/sshserver/tests/test_auth.py	2017-12-21 11:41:35 +0000
@@ -188,6 +188,35 @@
             self.user_auth.tryAuth = tryAuth
             self.user_auth._ebBadAuth = _ebBadAuth
 
+    def test_worksAroundShortParamikoSignatures(self):
+        # paramiko < 2.0.0 truncates RSA signatures whose most significant
+        # byte is zero.  auth_publickey tolerates this.
+        class MockLogin(object):
+            def __call__(self, credentials, mind, *interfaces):
+                self.credentials = credentials
+                return defer.succeed(None)
+
+        self.transport.sessionID = ''
+        self.portal.login = MockLogin()
+        keydir = sibpath(__file__, 'keys')
+        public_key = Key.fromString(
+            open(os.path.join(keydir, 'ssh_host_key_rsa.pub'), 'rb').read())
+        public_key_len = (public_key.keyObject.size() + 7) // 8
+        signature = b'\x01' * (public_key_len - 1)
+        packet = (
+            NS('user') + NS('') + NS('publickey') +
+            chr(1) + NS('ssh-rsa') + NS(public_key.blob()) +
+            NS(NS('ssh-rsa') + NS(signature)))
+        self.user_auth.serviceStarted()
+        self.user_auth.supportedAuthentications.append(b'publickey')
+        try:
+            self.user_auth.ssh_USERAUTH_REQUEST(packet)
+            self.assertEqual(
+                NS('ssh-rsa') + NS(b'\x00' + signature),
+                self.portal.login.credentials.signature)
+        finally:
+            self.user_auth.serviceStopped()
+
 
 class MockChecker(SSHPublicKeyDatabase):
     """A very simple public key checker which rejects all offered credentials.