Merge lp:~wgrant/launchpad/getFullKeyText-crash into lp:launchpad

[William Grant]

Fix SSHKey.getFullKeyText to not crash on some corrupt keys.

It already handled cases where the base64 or netstring were bad, but could crash
if the netstring contained non-ASCII garbage.

[Colin Watson (cjwatson)]

review: Approve

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2018-10-03 09:14:06 +0000
+++ lib/lp/registry/model/person.py	2018-11-07 10:43:45 +0000
@@ -4129,7 +4129,8 @@
 
     def getFullKeyText(self):
         try:
-            ssh_keytype = getNS(base64.b64decode(self.keytext))[0]
+            ssh_keytype = getNS(base64.b64decode(self.keytext))[0].decode(
+                'ascii')
         except Exception as e:
             # We didn't always validate keys, so there might be some that
             # can't be loaded this way.

=== modified file 'lib/lp/registry/tests/test_ssh.py'
--- lib/lp/registry/tests/test_ssh.py	2018-07-02 14:08:22 +0000
+++ lib/lp/registry/tests/test_ssh.py	2018-11-07 10:43:45 +0000
@@ -7,6 +7,7 @@
 
 from testtools.matchers import StartsWith
 from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
 
 from lp.registry.interfaces.ssh import (
     ISSHKeySet,
@@ -62,6 +63,18 @@
         expected = "ecdsa-sha2-nistp521 %s %s" % (key.keytext, key.comment)
         self.assertEqual(expected, key.getFullKeyText())
 
+    def test_getFullKeyText_for_corrupt_key(self):
+        # If the key text is corrupt, the type from the database is used
+        # instead of the one decoded from the text.
+        person = self.factory.makePerson()
+        with person_logged_in(person):
+            key = self.factory.makeSSHKey(person, "ssh-rsa")
+            # The base64 has a valid netstring, but the contents are garbage so
+            # can't be a valid key type.
+            removeSecurityProxy(key).keytext = 'AAAAB3NzaC1012EAAAA='
+        expected = "ssh-rsa %s %s" % (key.keytext, key.comment)
+        self.assertEqual(expected, key.getFullKeyText())
+
     def test_destroySelf_sends_notification_by_default(self):
         person = self.factory.makePerson()
         with person_logged_in(person):