Merge lp:~cjwatson/lazr.sshserver/more-key-types into lp:lazr.sshserver

[Colin Watson]

Add support-in-principle for authenticating using ECDSA or Ed25519 keys.

This also requires a sufficient version of Twisted (which at the time of writing supports ECDSA but not Ed25519) and support in the Launchpad authserver.

I've confirmed this to work locally with a modified version of Launchpad. Before pushing the Launchpad changes, we need to land these lazr.sshserver changes and upgrade its various users, since otherwise it would be rather confusing that you can add ECDSA keys but not authenticate using them.

Adding Ed25519 here is anticipatory, but it's harmless and will save us a step once Twisted gains Ed25519 support.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'src/lazr/sshserver/NEWS.txt'
--- src/lazr/sshserver/NEWS.txt	2018-02-26 11:57:53 +0000
+++ src/lazr/sshserver/NEWS.txt	2018-06-27 13:56:12 +0000
@@ -2,6 +2,14 @@
 NEWS for lazr.sshserver
 =======================
 
+0.1.8
+=====
+
+- Add support-in-principle for authenticating using ECDSA or Ed25519 keys;
+  this also requires a sufficient version of Twisted (which at the time of
+  writing supports ECDSA but not Ed25519) and support in the Launchpad
+  authserver.
+
 0.1.7 (2018-02-26)
 ==================
 
 
=== modified file 'src/lazr/sshserver/auth.py'
--- src/lazr/sshserver/auth.py	2018-02-26 11:49:59 +0000
+++ src/lazr/sshserver/auth.py	2018-06-27 13:56:12 +0000
@@ -314,6 +314,10 @@
             wantKeyType = 'DSA'
         elif credentials.algName == 'ssh-rsa':
             wantKeyType = 'RSA'
+        elif credentials.algName.startswith('ecdsa-sha2-'):
+            wantKeyType = 'ECDSA'
+        elif credentials.algName == 'ssh-ed25519':
+            wantKeyType = 'ED25519'
         else:
             # unknown key type
             return False
 
=== modified file 'src/lazr/sshserver/tests/test_auth.py'
--- src/lazr/sshserver/tests/test_auth.py	2018-02-26 11:49:59 +0000
+++ src/lazr/sshserver/tests/test_auth.py	2018-06-27 13:56:12 +0000
@@ -416,7 +416,10 @@
 
         valid_user = 'valid_user'
         no_key_user = 'no_key_user'
-        valid_key = 'valid_key'
+        valid_key_rsa = 'valid_key_rsa'
+        valid_key_dsa = 'valid_key_dsa'
+        valid_key_ecdsa = 'valid_key_ecdsa'
+        valid_key_ed25519 = 'valid_key_ed25519'
 
         def __init__(self):
             self.calls = []
@@ -430,7 +433,12 @@
             if username == self.valid_user:
                 return defer.succeed({
                     'name': username,
-                    'keys': [('DSA', self.valid_key.encode('base64'))],
+                    'keys': [
+                        ('RSA', self.valid_key_rsa.encode('base64')),
+                        ('DSA', self.valid_key_dsa.encode('base64')),
+                        ('ECDSA', self.valid_key_ecdsa.encode('base64')),
+                        ('ED25519', self.valid_key_ed25519.encode('base64')),
+                        ],
                     })
             elif username == self.no_key_user:
                 return defer.succeed({
@@ -443,11 +451,11 @@
                 except auth.NoSuchPersonWithName:
                     return defer.fail()
 
-    def makeCredentials(self, username, public_key, mind=None):
+    def makeCredentials(self, username, key_type, public_key, mind=None):
         if mind is None:
             mind = auth.UserDetailsMind()
         return auth.SSHPrivateKeyWithMind(
-            username, 'ssh-dss', public_key, '', None, mind)
+            username, key_type, public_key, '', None, mind)
 
     def makeChecker(self, do_signature_checking=False):
         """Construct a PublicKeyFromLaunchpadChecker.
@@ -469,21 +477,29 @@
         super(TestPublicKeyFromLaunchpadChecker, self).setUp()
         self.authserver = self.FakeAuthenticationEndpoint()
 
+    @defer.inlineCallbacks
     def test_successful(self):
         # Attempting to log in with a username and key known to the
         # authentication end-point succeeds.
-        creds = self.makeCredentials(
-            self.authserver.valid_user, self.authserver.valid_key)
-        checker = self.makeChecker()
-        d = checker.requestAvatarId(creds)
-        return d.addCallback(self.assertEqual, self.authserver.valid_user)
+        for key_type, public_key in (
+                ('ssh-rsa', self.authserver.valid_key_rsa),
+                ('ssh-dss', self.authserver.valid_key_dsa),
+                ('ecdsa-sha2-nistp256', self.authserver.valid_key_ecdsa),
+                ('ssh-ed25519', self.authserver.valid_key_ed25519),
+                ):
+            creds = self.makeCredentials(
+                self.authserver.valid_user, key_type, public_key)
+            checker = self.makeChecker()
+            username = yield checker.requestAvatarId(creds)
+            self.assertEqual(self.authserver.valid_user, username)
 
     @suppress_stderr
     def test_invalid_signature(self):
         # The checker requests attempts to authenticate if the requests have
         # an invalid signature.
         creds = self.makeCredentials(
-            self.authserver.valid_user, self.authserver.valid_key)
+            self.authserver.valid_user, 'ssh-dss',
+            self.authserver.valid_key_dsa)
         creds.signature = 'a'
         checker = self.makeChecker(True)
         d = checker.requestAvatarId(creds)
@@ -519,7 +535,7 @@
         # Launchpad user names is public.
         checker = self.makeChecker()
         creds = self.makeCredentials(
-            'no-such-user', self.authserver.valid_key)
+            'no-such-user', 'ssh-dss', self.authserver.valid_key_dsa)
         return self.assertLoginError(
             checker, creds, 'No such Launchpad account: no-such-user')
 
@@ -528,7 +544,8 @@
         # server informs you that the account has no keys.
         checker = self.makeChecker()
         creds = self.makeCredentials(
-            self.authserver.no_key_user, self.authserver.valid_key)
+            self.authserver.no_key_user, 'ssh-dss',
+            self.authserver.valid_key_dsa)
         return self.assertLoginError(
             checker, creds,
             "Launchpad user %r doesn't have a registered SSH key"
@@ -540,7 +557,7 @@
         # tries several keys as part of normal operation.
         checker = self.makeChecker()
         creds = self.makeCredentials(
-            self.authserver.valid_user, 'invalid key')
+            self.authserver.valid_user, 'ssh-dss', 'invalid key')
         # We cannot use assertLoginError because we are checking that we fail
         # with UnauthorizedLogin and not its subclass
         # UserDisplayedUnauthorizedLogin.
@@ -555,6 +572,20 @@
         return d
 
     @defer.inlineCallbacks
+    def test_unknownKeyType(self):
+        # Authenticating using a key with an unknown type fails.
+        creds = self.makeCredentials(
+            self.authserver.valid_user, 'nonsense',
+            self.authserver.valid_key_rsa)
+        checker = self.makeChecker()
+        exception = yield assert_fails_with(
+            checker.requestAvatarId(creds),
+            UnauthorizedLogin)
+        self.assertFalse(
+            isinstance(exception, auth.UserDisplayedUnauthorizedLogin),
+            "Should not be a UserDisplayedUnauthorizedLogin")
+
+    @defer.inlineCallbacks
     def test_successful_with_second_key_calls_authserver_once(self):
         # It is normal in SSH authentication to be presented with a number of
         # keys.  When the valid key is presented after some invalid ones (a)
@@ -563,9 +594,10 @@
         checker = self.makeChecker()
         mind = auth.UserDetailsMind()
         wrong_key_creds = self.makeCredentials(
-            self.authserver.valid_user, 'invalid key', mind)
+            self.authserver.valid_user, 'ssh-dss', 'invalid key', mind)
         right_key_creds = self.makeCredentials(
-            self.authserver.valid_user, self.authserver.valid_key, mind)
+            self.authserver.valid_user, 'ssh-dss',
+            self.authserver.valid_key_dsa, mind)
         try:
             username = yield checker.requestAvatarId(wrong_key_creds)
         except UnauthorizedLogin:
@@ -579,9 +611,9 @@
         checker = self.makeChecker()
         mind = auth.UserDetailsMind()
         creds_1 = self.makeCredentials(
-            'invalid-user', 'invalid key 1', mind)
+            'invalid-user', 'ssh-dss', 'invalid key 1', mind)
         creds_2 = self.makeCredentials(
-            'invalid-user', 'invalid key 2', mind)
+            'invalid-user', 'ssh-dss', 'invalid key 2', mind)
         d = checker.requestAvatarId(creds_1)
 
         def try_second_key(failure):