launchpad-reviewers team mailing list archive

[Colin Watson <mp+456006@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:gpg-eddsa into launchpad:master.

Commit message:
Add support for EdDSA GnuPG keys

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1827369 in Launchpad itself: "Launchpad cannot handle ECC or Ed25519 OpenPGP keys"
  https://bugs.launchpad.net/launchpad/+bug/1827369

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

These should work now that we're running on focal.

I'm afraid unit testing for this slightly defeated me in the time available, but I've at least manually tested that `gpghandler.retrieveKey` now works for an EdDSA key, which I think demonstrated the underlying problems on xenial.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:gpg-eddsa into launchpad:master.

diff --git a/lib/lp/registry/model/gpgkey.py b/lib/lp/registry/model/gpgkey.py
index eb34738..f499faf 100644
--- a/lib/lp/registry/model/gpgkey.py
+++ b/lib/lp/registry/model/gpgkey.py
@@ -11,7 +11,11 @@ from lp.registry.interfaces.gpg import IGPGKey, IGPGKeySet
 from lp.services.database.enumcol import DBEnum
 from lp.services.database.interfaces import IStore
 from lp.services.database.stormbase import StormBase
-from lp.services.gpg.interfaces import GPGKeyAlgorithm, IGPGHandler
+from lp.services.gpg.interfaces import (
+    GPGKeyAlgorithm,
+    IGPGHandler,
+    gpg_algorithm_letter,
+)
 from lp.services.verification.model.logintoken import LoginToken
 
 
@@ -67,7 +71,7 @@ class GPGKey(StormBase):
     def displayname(self):
         return "%s%s/%s" % (
             self.keysize,
-            self.algorithm.title,
+            gpg_algorithm_letter(self.algorithm),
             self.fingerprint,
         )
 
diff --git a/lib/lp/registry/templates/person-editpgpkeys.pt b/lib/lp/registry/templates/person-editpgpkeys.pt
index f6dd57b..169c5f0 100644
--- a/lib/lp/registry/templates/person-editpgpkeys.pt
+++ b/lib/lp/registry/templates/person-editpgpkeys.pt
@@ -163,12 +163,6 @@
         fingerprint</a>)
       </p>
 
-      <p>
-        At present, only RSA, DSA, and some ECC keys are supported; see
-        <a href="https://bugs.launchpad.net/launchpad/+bug/1827369";>bug
-        1827369</a> for details on the state of support for other key types.
-      </p>
-
           <table class="form" id="launchpad-form-widgets">
             <tbody>
               <tr>
diff --git a/lib/lp/services/gpg/handler.py b/lib/lp/services/gpg/handler.py
index bb354e0..b994f7d 100644
--- a/lib/lp/services/gpg/handler.py
+++ b/lib/lp/services/gpg/handler.py
@@ -48,6 +48,7 @@ from lp.services.gpg.interfaces import (
     get_gpg_home_directory,
     get_gpg_path,
     get_gpgme_context,
+    gpg_algorithm_letter,
     valid_fingerprint,
 )
 from lp.services.signing.enums import SigningKeyType
@@ -672,7 +673,7 @@ class PymeKey:
     def displayname(self):
         return "%s%s/%s" % (
             self.keysize,
-            self.algorithm.title,
+            gpg_algorithm_letter(self.algorithm),
             self.fingerprint,
         )
 
diff --git a/lib/lp/services/gpg/interfaces.py b/lib/lp/services/gpg/interfaces.py
index 6dec53a..1aecb5d 100644
--- a/lib/lp/services/gpg/interfaces.py
+++ b/lib/lp/services/gpg/interfaces.py
@@ -15,6 +15,7 @@ __all__ = [
     "GPGKeyTemporarilyNotFoundError",
     "GPGUploadFailure",
     "GPGVerificationError",
+    "gpg_algorithm_letter",
     "IGPGHandler",
     "IPymeKey",
     "IPymeSignature",
@@ -119,75 +120,45 @@ def get_gpgme_context():
     return context
 
 
-# XXX: cprov 2004-10-04:
-# (gpg+dbschema) the data structure should be rearranged to support 4 field
-# needed: keynumber(1,16,17,20), keyalias(R,g,D,G), title and description
 class GPGKeyAlgorithm(DBEnumeratedType):
     """
-    GPG Compliant Key Algorithms Types:
-
-    1  : "R", # RSA
-    16 : "g", # ElGamal
-    17 : "D", # DSA
-    20 : "G", # ElGamal, compromised
-    301: "E", # ECDSA
-    302: "e", # ECDH
-
-    See `pubkey_letter` in GnuPG for the single-letter codes used here.
-
-    FIXME
-    Rewrite it according to the experimental API returning also a name
-    attribute tested on 'algorithmname' attribute
+    GPG Public Key Algorithm
 
+    The numbers must match those in `gpgme_hash_algo_t`
+    (https://git.gnupg.org/cgi-bin/gitweb.cgi?p=gpgme.git;a=blob;f=src/gpgme.h.in).
     """
 
-    R = DBItem(
-        1,
-        """
-        R
-
-        RSA""",
-    )
-
-    LITTLE_G = DBItem(
-        16,
-        """
-         g
+    R = DBItem(1, "RSA")
+    LITTLE_G = DBItem(16, "ElGamal")
+    D = DBItem(17, "DSA")
+    G = DBItem(20, "ElGamal, compromised")
+    ECDSA = DBItem(301, "ECDSA")
+    ECDH = DBItem(302, "ECDH")
+    EDDSA = DBItem(303, "EDDSA")
 
-         ElGamal""",
-    )
 
-    D = DBItem(
-        17,
-        """
-        D
+def gpg_algorithm_letter(algorithm):
+    """Return a single letter describing a GPG public key algorithm.
 
-        DSA""",
-    )
+    This can be used in display names of keys.
 
-    G = DBItem(
-        20,
-        """
-        G
-
-        ElGamal, compromised""",
-    )
-
-    ECDSA = DBItem(
-        301,
-        """
-        E
-
-        ECDSA""",
-    )
-
-    ECDH = DBItem(
-        302,
-        """
-        e
-
-        ECDH""",
-    )
+    See `pubkey_letter` in GnuPG
+    (https://git.gnupg.org/cgi-bin/gitweb.cgi?p=gnupg.git;a=blob;f=g10/keyid.c)
+    for the single-letter codes used here.  Note that they are not
+    necessarily unique.
+    """
+    if algorithm == GPGKeyAlgorithm.R:
+        return "R"
+    elif algorithm == GPGKeyAlgorithm.LITTLE_G:
+        return "g"
+    elif algorithm == GPGKeyAlgorithm.D:
+        return "D"
+    elif algorithm == GPGKeyAlgorithm.G:
+        return "G"
+    elif algorithm in (GPGKeyAlgorithm.ECDSA, GPGKeyAlgorithm.EDDSA):
+        return "E"
+    elif algorithm == GPGKeyAlgorithm.ECDH:
+        return "e"
 
 
 class MoreThanOneGPGKeyFound(Exception):
diff --git a/lib/lp/services/gpg/tests/test_gpghandler.py b/lib/lp/services/gpg/tests/test_gpghandler.py
index 1199ea0..834d91b 100644
--- a/lib/lp/services/gpg/tests/test_gpghandler.py
+++ b/lib/lp/services/gpg/tests/test_gpghandler.py
@@ -19,6 +19,7 @@ from lp.services.features.testing import FeatureFixture
 from lp.services.gpg.handler import signing_only_param
 from lp.services.gpg.interfaces import (
     GPG_INJECT,
+    GPGKeyAlgorithm,
     GPGKeyDoesNotExistOnServer,
     GPGKeyMismatchOnServer,
     GPGKeyTemporarilyNotFoundError,
@@ -407,7 +408,7 @@ class TestGPGHandler(TestCase):
             new_key,
             MatchesStructure(
                 secret=Is(True),
-                algorithm=MatchesStructure.byEquality(title="R"),
+                algorithm=Equals(GPGKeyAlgorithm.R),
                 keysize=Equals(1024),
                 can_sign=Is(True),
                 can_encrypt=Is(False),
@@ -433,7 +434,7 @@ class TestGPGHandler(TestCase):
             pub_key,
             MatchesStructure(
                 secret=Is(False),
-                algorithm=MatchesStructure.byEquality(title="R"),
+                algorithm=Equals(GPGKeyAlgorithm.R),
                 keysize=Equals(1024),
                 can_sign=Is(True),
                 can_encrypt=Is(False),
diff --git a/lib/lp/services/verification/model/logintoken.py b/lib/lp/services/verification/model/logintoken.py
index 03a5bc6..eea34dd 100644
--- a/lib/lp/services/verification/model/logintoken.py
+++ b/lib/lp/services/verification/model/logintoken.py
@@ -25,7 +25,7 @@ from lp.services.database.constants import UTC_NOW
 from lp.services.database.enumcol import DBEnum
 from lp.services.database.interfaces import IPrimaryStore, IStore
 from lp.services.database.stormbase import StormBase
-from lp.services.gpg.interfaces import IGPGHandler
+from lp.services.gpg.interfaces import IGPGHandler, gpg_algorithm_letter
 from lp.services.mail.helpers import get_email_template
 from lp.services.mail.sendmail import format_address, simple_sendmail
 from lp.services.tokens import create_token
@@ -172,7 +172,7 @@ class LoginToken(StormBase):
 
         # Here are the instructions that need to be encrypted.
         template = get_email_template("validate-gpg.txt", app=MAIL_APP)
-        key_type = "%s%s" % (key.keysize, key.algorithm.title)
+        key_type = "%s%s" % (key.keysize, gpg_algorithm_letter(key.algorithm))
         replacements = {
             "requester": self.requester.displayname,
             "requesteremail": self.requesteremail,
diff --git a/lib/lp/soyuz/browser/tests/test_personal_archive_subscription.py b/lib/lp/soyuz/browser/tests/test_personal_archive_subscription.py
index 1151c37..80f973b 100644
--- a/lib/lp/soyuz/browser/tests/test_personal_archive_subscription.py
+++ b/lib/lp/soyuz/browser/tests/test_personal_archive_subscription.py
@@ -47,8 +47,8 @@ class TestPersonArchiveSubscriptionView(TestCaseWithFactory):
             view = create_initialized_view(pas, "+index", principal=subscriber)
             signing_key = find_tag_by_id(view(), "signing-key")
             self.assertEqual(
-                "This repository is signed with\n%s%s/%s OpenPGP key."
-                % (key.keysize, key.algorithm.title, key.fingerprint),
+                "This repository is signed with\n%s OpenPGP key."
+                % key.displayname,
                 extract_text(signing_key),
             )
             self.assertEqual(
diff --git a/lib/lp/soyuz/model/archive.py b/lib/lp/soyuz/model/archive.py
index c7749ba..b86c2e9 100644
--- a/lib/lp/soyuz/model/archive.py
+++ b/lib/lp/soyuz/model/archive.py
@@ -481,11 +481,7 @@ class Archive(StormBase):
                         self.signing_key_fingerprint,
                     )
                 else:
-                    return "%s%s/%s" % (
-                        pyme_key.keysize,
-                        pyme_key.algorithm.title,
-                        pyme_key.fingerprint,
-                    )
+                    return pyme_key.displayname
 
     @property
     def is_ppa(self):