Launchpad itself

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

  1. getFullKeyText-crash
  2. Merge into devel
Proposed by William Grant
Status: Merged
Merged at revision: 18812
Proposed branch: lp:~wgrant/launchpad/getFullKeyText-crash
Merge into: lp:launchpad
Diff against target: 44 lines (+15/-1)
2 files modified
lib/lp/registry/model/person.py (+2/-1)
lib/lp/registry/tests/test_ssh.py (+13/-0)
To merge this branch: bzr merge lp:~wgrant/launchpad/getFullKeyText-crash
Reviewer Review Type Date Requested Status
Colin Watson (community) Approve
Review via email: mp+358417@code.launchpad.net

Commit message

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

Description of the change

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

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
41294129
4130 def getFullKeyText(self):4130 def getFullKeyText(self):
4131 try:4131 try:
4132 ssh_keytype = getNS(base64.b64decode(self.keytext))[0]4132 ssh_keytype = getNS(base64.b64decode(self.keytext))[0].decode(
4133 'ascii')
4133 except Exception as e:4134 except Exception as e:
4134 # We didn't always validate keys, so there might be some that4135 # We didn't always validate keys, so there might be some that
4135 # can't be loaded this way.4136 # can't be loaded this way.
41364137
=== 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 @@
77
8from testtools.matchers import StartsWith8from testtools.matchers import StartsWith
9from zope.component import getUtility9from zope.component import getUtility
10from zope.security.proxy import removeSecurityProxy
1011
11from lp.registry.interfaces.ssh import (12from lp.registry.interfaces.ssh import (
12 ISSHKeySet,13 ISSHKeySet,
@@ -62,6 +63,18 @@
62 expected = "ecdsa-sha2-nistp521 %s %s" % (key.keytext, key.comment)63 expected = "ecdsa-sha2-nistp521 %s %s" % (key.keytext, key.comment)
63 self.assertEqual(expected, key.getFullKeyText())64 self.assertEqual(expected, key.getFullKeyText())
6465
66 def test_getFullKeyText_for_corrupt_key(self):
67 # If the key text is corrupt, the type from the database is used
68 # instead of the one decoded from the text.
69 person = self.factory.makePerson()
70 with person_logged_in(person):
71 key = self.factory.makeSSHKey(person, "ssh-rsa")
72 # The base64 has a valid netstring, but the contents are garbage so
73 # can't be a valid key type.
74 removeSecurityProxy(key).keytext = 'AAAAB3NzaC1012EAAAA='
75 expected = "ssh-rsa %s %s" % (key.keytext, key.comment)
76 self.assertEqual(expected, key.getFullKeyText())
77
65 def test_destroySelf_sends_notification_by_default(self):78 def test_destroySelf_sends_notification_by_default(self):
66 person = self.factory.makePerson()79 person = self.factory.makePerson()
67 with person_logged_in(person):80 with person_logged_in(person):