launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/uefiroot-ppa into lp:launchpad.

Commit message:
Read UEFI signing keys from SIGNING_KEYS_ROOT/uefi/OWNER/NAME for PPAs.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1036597 in Launchpad itself: "No UEFI signing configuration for PPAs"
  https://bugs.launchpad.net/launchpad/+bug/1036597

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/uefiroot-ppa/+merge/125467

== Summary ==

Bug 1036597: In order to release security updates to signed UEFI images, we need to be able to sign UEFI images in selected PPAs.  Of course these would need to use a different key.

== Proposed fix ==

All the files that Launchpad currently looks at in signing_key_root are <something>.gpg, so we can safely create a uefi/ subdirectory there and put keys in uefi/OWNER/NAME/.

== LOC Rationale ==

+11.  I have 3853 lines of credit and this will be critical to Ubuntu Engineering from the 12.10 release onward.

== Tests ==

bin/test -vvct lib/lp/archivepublisher/tests/archive-signing.txt -t lp.archivepublisher.tests.test_config

== Demo and Q/A ==

Generate a test key for cjwatson/ppa on dogfood and try to publish that PPA; it currently has two uploads blocked in ACCEPTED waiting for this bug fix, which should now be able to publish.

== Lint ==

Pre-existing, unrelated to my code:

./configs/testrunner/launchpad-lazr.conf
     114: Line exceeds 80 characters.
-- 
https://code.launchpad.net/~cjwatson/launchpad/uefiroot-ppa/+merge/125467
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/uefiroot-ppa into lp:launchpad.

=== modified file 'configs/testrunner/launchpad-lazr.conf'
--- configs/testrunner/launchpad-lazr.conf	2012-08-09 04:44:13 +0000
+++ configs/testrunner/launchpad-lazr.conf	2012-09-20 12:14:26 +0000
@@ -148,6 +148,7 @@
 
 [personalpackagearchive]
 root: /var/tmp/ppa.test/
+signing_keys_root: /var/tmp/ppa-signing-keys.test
 
 [ppa_apache_log_parser]
 logs_root: lib/lp/soyuz/scripts/tests/ppa-apache-log-files

=== modified file 'lib/lp/archivepublisher/config.py'
--- lib/lp/archivepublisher/config.py	2012-06-28 16:32:27 +0000
+++ lib/lp/archivepublisher/config.py	2012-09-20 12:14:26 +0000
@@ -83,6 +83,10 @@
 
     if archive.is_main:
         pubconf.uefiroot = pubconf.archiveroot + '-uefi'
+    elif archive.is_ppa:
+        pubconf.uefiroot = os.path.join(
+            ppa_config.signing_keys_root, "uefi",
+            archive.owner.name, archive.name)
     else:
         pubconf.uefiroot = None
 

=== modified file 'lib/lp/archivepublisher/tests/archive-signing.txt'
--- lib/lp/archivepublisher/tests/archive-signing.txt	2012-08-17 11:14:22 +0000
+++ lib/lp/archivepublisher/tests/archive-signing.txt	2012-09-20 12:14:26 +0000
@@ -122,7 +122,7 @@
 
     >>> from lp.services.config import config
     >>> print config.personalpackagearchive.signing_keys_root
-    /var/tmp/ppa-signing-keys
+    /var/tmp/ppa-signing-keys.test
 
 In order to manipulate 'signing_keys' securily the target archive
 object has to be adapted to `IArchiveSigningKey`.

=== modified file 'lib/lp/archivepublisher/tests/test_config.py'
--- lib/lp/archivepublisher/tests/test_config.py	2012-07-03 12:48:38 +0000
+++ lib/lp/archivepublisher/tests/test_config.py	2012-09-20 12:14:26 +0000
@@ -148,7 +148,9 @@
         self.assertIsNone(self.ppa_config.germinateroot)
         self.assertEqual(
             "/var/tmp/archive/ubuntutest-temp", self.ppa_config.temproot)
-        self.assertIsNone(self.ppa_config.uefiroot)
+        uefiroot = "/var/tmp/ppa-signing-keys.test/uefi/%s/%s" % (
+            self.ppa.owner.name, self.ppa.name)
+        self.assertEqual(uefiroot, self.ppa_config.uefiroot)
 
     def test_private_ppa_separate_root(self):
         # Private PPAs are published to a different location.
@@ -177,4 +179,8 @@
         self.assertIsNone(p3a_config.germinateroot)
         self.assertEqual(
             "/var/tmp/archive/ubuntutest-temp", p3a_config.temproot)
-        self.assertIsNone(p3a_config.uefiroot)
+        # It's OK for the signing keys to be in the same location as for
+        # public PPAs, as the owner/name namespace is shared.
+        uefiroot = "/var/tmp/ppa-signing-keys.test/uefi/%s/%s" % (
+            p3a.owner.name, p3a.name)
+        self.assertEqual(uefiroot, p3a_config.uefiroot)