launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/no-explicit-keyserver-port into lp:launchpad.

Commit message:
Construct public keyserver links using HTTPS without an explicit port.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1739110 in Launchpad itself: "PGP keys link unnecessarily to keyserver with explicit port, causing SSL issues"
  https://bugs.launchpad.net/launchpad/+bug/1739110

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/no-explicit-keyserver-port/+merge/336724
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/no-explicit-keyserver-port into lp:launchpad.

=== modified file 'configs/development/launchpad-lazr.conf'
--- configs/development/launchpad-lazr.conf	2017-09-07 13:25:13 +0000
+++ configs/development/launchpad-lazr.conf	2018-01-26 22:52:25 +0000
@@ -90,6 +90,7 @@
 [gpghandler]
 host: keyserver.launchpad.dev
 public_host: keyserver.launchpad.dev
+public_https: False
 
 [launchpad]
 enable_test_openid_provider: True

=== modified file 'configs/testrunner/launchpad-lazr.conf'
--- configs/testrunner/launchpad-lazr.conf	2016-05-18 00:33:18 +0000
+++ configs/testrunner/launchpad-lazr.conf	2018-01-26 22:52:25 +0000
@@ -95,6 +95,7 @@
 upload_keys: True
 host: localhost
 public_host: keyserver.ubuntu.com
+public_https: True
 
 [karmacacheupdater]
 max_scaling: 2

=== modified file 'lib/lp/registry/stories/person/xx-person-rdf.txt'
--- lib/lp/registry/stories/person/xx-person-rdf.txt	2017-10-21 18:14:14 +0000
+++ lib/lp/registry/stories/person/xx-person-rdf.txt	2018-01-26 22:52:25 +0000
@@ -36,7 +36,7 @@
             <wot:hex_id>12345678</wot:hex_id>
             <wot:length>1024</wot:length>
             <wot:fingerprint>ABCDEF0123456789ABCDDCBA0000111112345678</wot:fingerprint>
-            <wot:pubkeyAddress rdf:resource="http://keyserver.ubuntu.com:11371/pks/lookup?fingerprint=on&amp;op=index&amp;search=0xABCDEF0123456789ABCDDCBA0000111112345678"/>
+            <wot:pubkeyAddress rdf:resource="https://keyserver.ubuntu.com/pks/lookup?fingerprint=on&amp;op=index&amp;search=0xABCDEF0123456789ABCDDCBA0000111112345678"/>
           </wot:PubKey>
         </wot:hasKey>
       </foaf:Person>

=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf	2017-09-07 13:25:13 +0000
+++ lib/lp/services/config/schema-lazr.conf	2018-01-26 22:52:25 +0000
@@ -811,6 +811,10 @@
 # datatype: ip_address_or_hostname
 public_host: keyserver.ubuntu.com
 
+# If true, construct public keyserver links using HTTPS, ignoring the value
+# of `port`.
+public_https: True
+
 # Port number on Host to access the keyserver.
 # datatype: int
 port: 11371

=== modified file 'lib/lp/services/gpg/doc/gpghandler.txt'
--- lib/lp/services/gpg/doc/gpghandler.txt	2017-07-31 11:19:23 +0000
+++ lib/lp/services/gpg/doc/gpghandler.txt	2018-01-26 22:52:25 +0000
@@ -2,7 +2,7 @@
 
 `IGPGHandler` is a utility designed to handle OpenPGP (GPG) operations.
 
-The following operation are supported:
+The following operations are supported:
 
  * Importing public and secret keys;
  * Generating a new key;
@@ -260,29 +260,6 @@
     True
 
 
-== Keyserver URLs ==
-
-The gpghandler can also provide us with convenient links to the
-keyserver web interface. By default the action is to display the index
-page. Notice that the fingerprint must be the 40-byte fingerprint,
-to avoid the retrieval of more than one key.
-
-    >>> fingerprint = "A419AE861E88BC9E04B9C26FBA2B9389DFD20543"
-    >>> gpghandler.getURLForKeyInServer(fingerprint)
-    'http://localhost:11371/pks/lookup?fingerprint=on&op=index&search=0xA419AE861E88BC9E04B9C26FBA2B9389DFD20543'
-
-But you can also specify your own action:
-
-    >>> gpghandler.getURLForKeyInServer(fingerprint, action="get")
-    'http://localhost:11371/pks/lookup?fingerprint=on&op=get&search=0xA419AE861E88BC9E04B9C26FBA2B9389DFD20543'
-
-The method accepts a flag to retrieve a link to ubuntu's public
-keyserver web interface.
-
-    >>> gpghandler.getURLForKeyInServer(fingerprint, public=True)
-    'http://keyserver.ubuntu.com:11371/pks/lookup?fingerprint=on&op=index&search=0xA419AE861E88BC9E04B9C26FBA2B9389DFD20543'
-
-
 == Keyserver uploads ==
 
 IGPGHandler also allow callsites to upload the public part of a local

=== modified file 'lib/lp/services/gpg/handler.py'
--- lib/lp/services/gpg/handler.py	2017-07-31 11:19:23 +0000
+++ lib/lp/services/gpg/handler.py	2018-01-26 22:52:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -478,9 +478,12 @@
             host = config.gpghandler.public_host
         else:
             host = config.gpghandler.host
-        return 'http://%s:%s/pks/lookup?%s' % (
-            host, config.gpghandler.port,
-            urllib.urlencode(sorted(params.items())))
+        if public and config.gpghandler.public_https:
+            base = 'https://%s' % host
+        else:
+            base = 'http://%s:%s' % (host, config.gpghandler.port)
+        return '%s/pks/lookup?%s' % (
+            base, urllib.urlencode(sorted(params.items())))
 
     def _getPubKey(self, fingerprint):
         """See IGPGHandler for further information."""

=== modified file 'lib/lp/services/gpg/tests/test_gpghandler.py'
--- lib/lp/services/gpg/tests/test_gpghandler.py	2018-01-02 10:54:31 +0000
+++ lib/lp/services/gpg/tests/test_gpghandler.py	2018-01-26 22:52:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 import base64
@@ -207,6 +207,47 @@
             GPGKeyDoesNotExistOnServer,
             removeSecurityProxy(self.gpg_handler)._getPubKey, fingerprint)
 
+    def test_getURLForKeyInServer_default(self):
+        # By default the action is to display the key's index page.  Notice
+        # that the fingerprint must be the 40-byte fingerprint, to avoid the
+        # retrieval of more than one key.
+        fingerprint = "A419AE861E88BC9E04B9C26FBA2B9389DFD20543"
+        self.assertEqual(
+            "http://localhost:11371/pks/lookup?fingerprint=on&";
+            "op=index&search=0x%s" % fingerprint,
+            self.gpg_handler.getURLForKeyInServer(fingerprint))
+
+    def test_getURLForKeyInServer_different_action(self):
+        # The caller can specify a different action.
+        fingerprint = "A419AE861E88BC9E04B9C26FBA2B9389DFD20543"
+        self.assertEqual(
+            "http://localhost:11371/pks/lookup?fingerprint=on&";
+            "op=get&search=0x%s" % fingerprint,
+            self.gpg_handler.getURLForKeyInServer(fingerprint, action="get"))
+
+    def test_getURLForKeyInServer_public_http(self):
+        # The caller can request a link to the public keyserver web
+        # interface.  If the configuration item gpghandler.public_https is
+        # false, then this uses HTTP and gpghandler.port.
+        self.pushConfig("gpghandler", public_https=False)
+        fingerprint = "A419AE861E88BC9E04B9C26FBA2B9389DFD20543"
+        self.assertEqual(
+            "http://keyserver.ubuntu.com:11371/pks/lookup?fingerprint=on&";
+            "op=index&search=0x%s" % fingerprint,
+            self.gpg_handler.getURLForKeyInServer(fingerprint, public=True))
+
+    def test_getURLForKeyInServer_public_https(self):
+        # The caller can request a link to the public keyserver web
+        # interface.  If the configuration item gpghandler.public_https is
+        # true, then this uses HTTPS and the default HTTPS port.
+        # This is the testrunner default, but let's be explicit here.
+        self.pushConfig("gpghandler", public_https=True)
+        fingerprint = "A419AE861E88BC9E04B9C26FBA2B9389DFD20543"
+        self.assertEqual(
+            "https://keyserver.ubuntu.com/pks/lookup?fingerprint=on&";
+            "op=index&search=0x%s" % fingerprint,
+            self.gpg_handler.getURLForKeyInServer(fingerprint, public=True))
+
     def test_signContent_uses_sha512_digests(self):
         secret_keys = [
             ("ppa-sample@xxxxxxxxxxxxxxxxx", ""),       # 1024R

=== modified file 'lib/lp/soyuz/stories/ppa/xx-ubuntu-ppas.txt'
--- lib/lp/soyuz/stories/ppa/xx-ubuntu-ppas.txt	2017-07-31 11:45:32 +0000
+++ lib/lp/soyuz/stories/ppa/xx-ubuntu-ppas.txt	2018-01-26 22:52:25 +0000
@@ -581,7 +581,7 @@
 
     >>> print anon_browser.getLink(
     ...     '1024D/ABCDEF0123456789ABCDDCBA0000111112345678').url
-    http://keyserver.ubuntu.com:11371/pks/lookup?fingerprint=on&op=index&search=0xABCDEF0123456789ABCDDCBA0000111112345678
+    https://keyserver.ubuntu.com/pks/lookup?fingerprint=on&op=index&search=0xABCDEF0123456789ABCDDCBA0000111112345678
 
 Using software from a PPA can be hard for novices. We offer two
 links to the same help pop-up that describes how to add a PPA and