[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/ssh-ecdsa-keys into lp:launchpad

2018-07-11 Thread noreply
The proposal to merge lp:~cjwatson/launchpad/ssh-ecdsa-keys into lp:launchpad 
has been updated.

Status: Needs review => Merged

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/ssh-ecdsa-keys/+merge/348848
-- 
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp


Re: [Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/ssh-ecdsa-keys into lp:launchpad

2018-07-02 Thread William Grant
Review: Approve code


-- 
https://code.launchpad.net/~cjwatson/launchpad/ssh-ecdsa-keys/+merge/348848
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp


[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/ssh-ecdsa-keys into lp:launchpad

2018-07-02 Thread Colin Watson
Colin Watson has proposed merging lp:~cjwatson/launchpad/ssh-ecdsa-keys into 
lp:launchpad with lp:~cjwatson/launchpad/lazr.sshserver-0.1.8 as a prerequisite.

Commit message:
Add support for SSH ECDSA keys.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #907675 in Launchpad itself: "Add support for ECDSA and Ed25519 SSH keys"
  https://bugs.launchpad.net/launchpad/+bug/907675

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/ssh-ecdsa-keys/+merge/348848

The main difficulty is that there are multiple public key algorithm names for 
ECDSA, so we need to stop having data structures laid out on the assumption 
that there's a bijection between public key algorithm names and SSHKeyType 
enumeration items.  (The alternative was to have one SSHKeyType item for each 
curve size, but that seemed a bit silly.)

The prerequisite branch and the corresponding branches of txpkgupload and 
turnip must all be deployed to production before this is landed, to avoid the 
situation where people can upload keys they can't use.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
lp:~cjwatson/launchpad/ssh-ecdsa-keys into lp:launchpad.
=== modified file 'lib/lp/registry/interfaces/ssh.py'
--- lib/lp/registry/interfaces/ssh.py	2018-06-25 15:14:38 +
+++ lib/lp/registry/interfaces/ssh.py	2018-07-02 14:38:50 +
@@ -8,7 +8,6 @@
 __all__ = [
 'ISSHKey',
 'ISSHKeySet',
-'SSH_KEY_TYPE_TO_TEXT',
 'SSH_TEXT_TO_KEY_TYPE',
 'SSHKeyAdditionError',
 'SSHKeyType',
@@ -39,7 +38,7 @@
 class SSHKeyType(DBEnumeratedType):
 """SSH key type
 
-SSH (version 2) can use RSA or DSA keys for authentication. See
+SSH (version 2) can use RSA, DSA, or ECDSA keys for authentication.  See
 OpenSSH's ssh-keygen(1) man page for details.
 """
 
@@ -55,14 +54,20 @@
 DSA
 """)
 
-
-SSH_KEY_TYPE_TO_TEXT = {
-SSHKeyType.RSA: "ssh-rsa",
-SSHKeyType.DSA: "ssh-dss",
-}
-
-
-SSH_TEXT_TO_KEY_TYPE = {v: k for k, v in SSH_KEY_TYPE_TO_TEXT.items()}
+ECDSA = DBItem(3, """
+ECDSA
+
+ECDSA
+""")
+
+
+SSH_TEXT_TO_KEY_TYPE = {
+"ssh-rsa": SSHKeyType.RSA,
+"ssh-dss": SSHKeyType.DSA,
+"ecdsa-sha2-nistp256": SSHKeyType.ECDSA,
+"ecdsa-sha2-nistp384": SSHKeyType.ECDSA,
+"ecdsa-sha2-nistp521": SSHKeyType.ECDSA,
+}
 
 
 class ISSHKey(Interface):
@@ -139,6 +144,11 @@
 if 'kind' in kwargs:
 kind = kwargs.pop('kind')
 msg = "Invalid SSH key type: '%s'" % kind
+if 'type_mismatch' in kwargs:
+keytype, keydatatype = kwargs.pop('type_mismatch')
+msg = (
+"Invalid SSH key data: key type '%s' does not match key data "
+"type '%s'" % (keytype, keydatatype))
 if 'exception' in kwargs:
 exception = kwargs.pop('exception')
 try:

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2018-06-19 14:46:05 +
+++ lib/lp/registry/model/person.py	2018-07-02 14:38:50 +
@@ -29,6 +29,7 @@
 'WikiNameSet',
 ]
 
+import base64
 from datetime import (
 datetime,
 timedelta,
@@ -81,6 +82,7 @@
 Store,
 )
 import transaction
+from twisted.conch.ssh.common import getNS
 from twisted.conch.ssh.keys import Key
 from zope.component import (
 adapter,
@@ -205,7 +207,6 @@
 from lp.registry.interfaces.ssh import (
 ISSHKey,
 ISSHKeySet,
-SSH_KEY_TYPE_TO_TEXT,
 SSH_TEXT_TO_KEY_TYPE,
 SSHKeyAdditionError,
 SSHKeyType,
@@ -4122,8 +4123,23 @@
 super(SSHKey, self).destroySelf()
 
 def getFullKeyText(self):
-return "%s %s %s" % (
-SSH_KEY_TYPE_TO_TEXT[self.keytype], self.keytext, self.comment)
+try:
+ssh_keytype = getNS(base64.b64decode(self.keytext))[0]
+except Exception as e:
+# We didn't always validate keys, so there might be some that
+# can't be loaded this way.
+if self.keytype == SSHKeyType.RSA:
+ssh_keytype = "ssh-rsa"
+elif self.keytype == SSHKeyType.DSA:
+ssh_keytype = "ssh-dss"
+else:
+# There's no single ssh_keytype for ECDSA keys (it depends
+# on the curve), and we've always validated these so this
+# shouldn't happen.
+raise ValueError(
+"SSH key of type %s has invalid data '%s'" %
+(self.keytype, self.keytext))
+return "%s %s %s" % (ssh_keytype, self.keytext, self.comment)
 
 
 @implementer(ISSHKeySet)
@@ -4131,13 +4147,16 @@
 
 def new(self, person, sshkey, check_key=True, send_notification=True,
 dry_run=False):
-keytype, keytext, comment = self._extract_ssh_key_components(sshkey)
+kind, keytype, keytext, comment = self._extract_ssh_key_components(
+sshkey)
 
 i