launchpad-reviewers team mailing list archive

[Colin Watson <mp+415431@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:ssh-ed25519 into launchpad:master with ~cjwatson/launchpad:Twisted-20.3.0+lp5 as a prerequisite.

Commit message:
Support Ed25519 SSH keys

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #907675 in Launchpad itself: "Add support for Ed25519 SSH keys"
  https://bugs.launchpad.net/launchpad/+bug/907675
  Bug #1933722 in Launchpad itself: "Launchpad doesn't support secure SSH parameters"
  https://bugs.launchpad.net/launchpad/+bug/1933722

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/415431

The corresponding Twisted backport must be deployed to all production endpoints before this can land.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:ssh-ed25519 into launchpad:master.

diff --git a/lib/lp/registry/interfaces/ssh.py b/lib/lp/registry/interfaces/ssh.py
index a1df715..d9aef9b 100644
--- a/lib/lp/registry/interfaces/ssh.py
+++ b/lib/lp/registry/interfaces/ssh.py
@@ -35,8 +35,8 @@ from lp import _
 class SSHKeyType(DBEnumeratedType):
     """SSH key type
 
-    SSH (version 2) can use RSA, DSA, or ECDSA keys for authentication.  See
-    OpenSSH's ssh-keygen(1) man page for details.
+    SSH (version 2) can use RSA, DSA, ECDSA, or Ed25519 keys for
+    authentication.  See OpenSSH's ssh-keygen(1) man page for details.
     """
 
     RSA = DBItem(1, """
@@ -57,6 +57,12 @@ class SSHKeyType(DBEnumeratedType):
         ECDSA
         """)
 
+    ED25519 = DBItem(4, """
+        ED25519
+
+        Ed25519
+        """)
+
 
 SSH_TEXT_TO_KEY_TYPE = {
     "ssh-rsa": SSHKeyType.RSA,
@@ -64,6 +70,7 @@ SSH_TEXT_TO_KEY_TYPE = {
     "ecdsa-sha2-nistp256": SSHKeyType.ECDSA,
     "ecdsa-sha2-nistp384": SSHKeyType.ECDSA,
     "ecdsa-sha2-nistp521": SSHKeyType.ECDSA,
+    "ssh-ed25519": SSHKeyType.ED25519,
     }
 
 
diff --git a/lib/lp/registry/stories/person/xx-add-sshkey.txt b/lib/lp/registry/stories/person/xx-add-sshkey.txt
index 8c1095d..a9ab70c 100644
--- a/lib/lp/registry/stories/person/xx-add-sshkey.txt
+++ b/lib/lp/registry/stories/person/xx-add-sshkey.txt
@@ -57,9 +57,9 @@ his SSH keys. The page allows him to add a key.
     Change your SSH keys...
 
 Any key must be of the form "keytype keytext comment", where keytype must be
-one of ssh-rsa, ssh-dss, ecdsa-sha2-nistp256, ecdsa-sha2-nistp284, or
-ecdsa-sha2-nistp521.  If the key doesn't match the expected format, an error
-message will be shown.
+one of ssh-rsa, ssh-dss, ecdsa-sha2-nistp256, ecdsa-sha2-nistp284,
+ecdsa-sha2-nistp521, or ssh-ed25519.  If the key doesn't match the expected
+format, an error message will be shown.
 
     >>> sshkey = "ssh-rsa   "
     >>> browser.getControl(name='sshkey').value = sshkey
@@ -163,6 +163,16 @@ format.
     ...     print(tag.decode_contents())
     SSH public key added.
 
+    >>> sshkey = (
+    ...     "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIGAwHVarhTHSYjZiZcbrf9xM+pAh"
+    ...     "Z/MzqqxTc5Mi+oTX salgado@canario")
+    >>> browser.getControl(name='sshkey').value = sshkey
+    >>> browser.getControl('Import Public Key').click()
+    >>> soup = find_main_content(browser.contents)
+    >>> for tag in soup('p', 'informational message'):
+    ...     print(tag.decode_contents())
+    SSH public key added.
+
 Launchpad administrators are not allowed to poke at other user's ssh keys.
 
     >>> login(ANONYMOUS)
@@ -187,6 +197,7 @@ to edit his keys is on the page.
     salgado@canario
     salgado@canario
     salgado@canario
+    salgado@canario
     >>> browser.getLink('Update SSH keys').click()
     >>> print(browser.title)
     Change your SSH keys...
diff --git a/lib/lp/registry/templates/person-editsshkeys.pt b/lib/lp/registry/templates/person-editsshkeys.pt
index 6893ce7..b3f254d 100644
--- a/lib/lp/registry/templates/person-editsshkeys.pt
+++ b/lib/lp/registry/templates/person-editsshkeys.pt
@@ -47,11 +47,10 @@
         <label>Public key line</label>
         <div class="formHelp">
           Insert the contents of your public key (usually
-          <code>~/.ssh/id_rsa.pub</code>, <code>~/.ssh/id_dsa.pub</code>, or
-          <code>~/.ssh/id_ecdsa.pub</code>).
+          <code>~/.ssh/id_rsa.pub</code>, <code>~/.ssh/id_dsa.pub</code>,
+          <code>~/.ssh/id_ecdsa.pub</code>, or
+          <code>~/.ssh/id_ed25519.pub</code>).
           Only SSH v2 keys are supported.
-          Ed25519 keys are <a href="https://bugs.launchpad.net/bugs/907675";>not
-          yet supported</a>.
           <a href="https://help.launchpad.net/YourAccount/CreatingAnSSHKeyPair";>
             How do I create a public key?
           </a>
diff --git a/lib/lp/registry/tests/test_ssh.py b/lib/lp/registry/tests/test_ssh.py
index bce2795..c85baad 100644
--- a/lib/lp/registry/tests/test_ssh.py
+++ b/lib/lp/registry/tests/test_ssh.py
@@ -61,6 +61,13 @@ class TestSSHKey(TestCaseWithFactory):
         expected = "ecdsa-sha2-nistp521 %s %s" % (key.keytext, key.comment)
         self.assertEqual(expected, key.getFullKeyText())
 
+    def test_getFullKeyText_for_ed25519_key(self):
+        person = self.factory.makePerson()
+        with person_logged_in(person):
+            key = self.factory.makeSSHKey(person, "ssh-ed25519")
+        expected = "ssh-ed25519 %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.
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index b098cba..492f57b 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -4430,6 +4430,8 @@ class BareLaunchpadObjectFactory(ObjectFactory):
                        int_to_bytes(curve_data["x"], key_byte_length) +
                        int_to_bytes(curve_data["y"], key_byte_length)),
                     ]
+        elif key_type == "ssh-ed25519":
+            parameters = [NS(keydata.Ed25519Data["a"])]
         if parameters is None:
             raise AssertionError(
                 "key_type must be a member of SSH_TEXT_TO_KEY_TYPE, not %r" %