← Back to team overview

launchpad-reviewers team mailing list archive

  1. launchpad-reviewers team
  2. Mailing list archive
  3. Message #27458

[Merge] ~cjwatson/launchpad-buildd:sanitize-non-user-macaroons into launchpad-buildd:master

  Thread Previous • Date Previous • • Thread Next 
Colin Watson has proposed merging ~cjwatson/launchpad-buildd:sanitize-non-user-macaroons into launchpad-buildd:master.

Commit message:
Sanitize non-user-bound macaroons in build logs

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

Some of the macaroons issued by Launchpad are not bound to a user, for example if they're scoped to a particular running build.  In those cases the username part of the URL is empty.  Fix build log sanitization to remove these macaroons.

This is typically not a significant security issue since the macaroons in question cannot be used after the build has completed, and anyone who can see a build while it's in progress should normally also be able to see the resources it uses, but nevertheless it wasn't intended behaviour and is an unnecessary exposure of information.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad-buildd:sanitize-non-user-macaroons into launchpad-buildd:master.

diff --git a/debian/changelog b/debian/changelog
index 0386527..5195400 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+launchpad-buildd (201) UNRELEASED; urgency=medium
+
+  * Sanitize non-user-bound macaroons in build logs.
+
+ -- Colin Watson <cjwatson@xxxxxxxxxx>  Fri, 20 Aug 2021 17:53:12 +0100
+
 launchpad-buildd (200) bionic; urgency=medium
 
   * Add core22 to the list of supported core snap names.
diff --git a/lpbuildd/builder.py b/lpbuildd/builder.py
index d83c10f..b8362d3 100644
--- a/lpbuildd/builder.py
+++ b/lpbuildd/builder.py
@@ -48,7 +48,7 @@ def _sanitizeURLs(bytes_seq):
     """
     # This regular expression will be used to remove authentication
     # credentials from URLs.
-    password_re = re.compile(br'://([^:]+:[^@]+@)(\S+)')
+    password_re = re.compile(br'://([^:]*:[^@]+@)(\S+)')
     # Builder proxy passwords are UUIDs.
     proxy_auth_re = re.compile(br',proxyauth=[^:]+:[A-Za-z0-9-]+')
 
diff --git a/lpbuildd/tests/buildlog b/lpbuildd/tests/buildlog
index 4b64e85..2f8bff1 100644
--- a/lpbuildd/tests/buildlog
+++ b/lpbuildd/tests/buildlog
@@ -10,6 +10,7 @@ Get:1 http://buildd:secret@ftpmaster.internal gutsy Release.gpg [191B]
 Get:2 http://ftpmaster.internal gutsy Release [65.9kB]
 Get:3 http://user:blah@ftpmaster.internal gutsy/main Packages [1085kB]
 Get:4 http://ftpmaster.internal gutsy/universe Packages [3991kB]
+Get:5 http://:macaroon@private-ppa.buildd/user/archive/ubuntu Packages [11kB]
 Fetched 5142kB in 5s (1012kB/s)
 Reading package lists...
 Reading package lists...
diff --git a/lpbuildd/tests/buildlog.long b/lpbuildd/tests/buildlog.long
index 9ca5464..a4c8ebb 100644
--- a/lpbuildd/tests/buildlog.long
+++ b/lpbuildd/tests/buildlog.long
@@ -10,6 +10,7 @@ Get:1 http://buildd:secret@ftpmaster.internal gutsy Release.gpg [191B]
 Get:2 http://ftpmaster.internal gutsy Release [65.9kB]
 Get:3 http://user:blah@ftpmaster.internal gutsy/main Packages [1085kB]
 Get:4 http://ftpmaster.internal gutsy/universe Packages [3991kB]
+Get:5 http://:macaroon@private-ppa.buildd/user/archive/ubuntu Packages [11kB]
 Fetched 5142kB in 5s (1012kB/s)
 Reading package lists...
 Reading package lists...
@@ -34,6 +35,7 @@ Get:1 http://buildd:secret@ftpmaster.internal gutsy Release.gpg [191B]
 Get:2 http://ftpmaster.internal gutsy Release [65.9kB]
 Get:3 http://user:blah@ftpmaster.internal gutsy/main Packages [1085kB]
 Get:4 http://ftpmaster.internal gutsy/universe Packages [3991kB]
+Get:5 http://:macaroon@private-ppa.buildd/user/archive/ubuntu Packages [11kB]
 Fetched 5142kB in 5s (1012kB/s)
 Reading package lists...
 Reading package lists...
@@ -57,6 +59,7 @@ Get:1 http://buildd:secret@ftpmaster.internal gutsy Release.gpg [191B]
 Get:2 http://ftpmaster.internal gutsy Release [65.9kB]
 Get:3 http://user:blah@ftpmaster.internal gutsy/main Packages [1085kB]
 Get:4 http://ftpmaster.internal gutsy/universe Packages [3991kB]
+Get:5 http://:macaroon@private-ppa.buildd/user/archive/ubuntu Packages [11kB]
 Fetched 5142kB in 5s (1012kB/s)
 Reading package lists...
 Reading package lists...
@@ -80,5 +83,6 @@ Get:1 http://buildd:secret@ftpmaster.internal gutsy Release.gpg [191B]
 Get:2 http://must:go@ftpmaster.internal gutsy Release [65.9kB]
 Get:3 http://scrub:this@ftpmaster.internal gutsy/main Packages [1085kB]
 Get:4 http://ftpmaster.internal gutsy/universe Packages [3991kB]
+Get:5 http://:macaroon@private-ppa.buildd/user/archive/ubuntu Packages [11kB]
 Fetched 5142kB in 5s (1012kB/s)
 socat STDIO PROXY:builder-proxy.launchpad.dev:github.com:443,proxyport=3128,proxyauth=user:blah
diff --git a/lpbuildd/tests/test_1.diff b/lpbuildd/tests/test_1.diff
index d4f9b70..a6fe62a 100644
--- a/lpbuildd/tests/test_1.diff
+++ b/lpbuildd/tests/test_1.diff
@@ -2,7 +2,7 @@
 
 +++  
 
-@@ -6,9 +6,9 @@
+@@ -6,11 +6,11 @@
 
  Attempting OGRE for universe in build-370614-896976
  RUN: /usr/share/launchpad-buildd/bin/update-debian-chroot 370614-896976
@@ -13,9 +13,12 @@
 -Get:3 http://user:blah@ftpmaster.internal gutsy/main Packages [1085kB]
 +Get:3 http://ftpmaster.internal gutsy/main Packages [1085kB]
  Get:4 http://ftpmaster.internal gutsy/universe Packages [3991kB]
+-Get:5 http://:macaroon@private-ppa.buildd/user/archive/ubuntu Packages [11kB]
++Get:5 http://private-ppa.buildd/user/archive/ubuntu Packages [11kB]
  Fetched 5142kB in 5s (1012kB/s)
  Reading package lists...
-@@ -21,4 +21,4 @@
+ Reading package lists...
+@@ -22,4 +22,4 @@
 
  Need to get 0B/2832kB of archives.
  After unpacking 94.2kB of additional disk space will be used.
diff --git a/lpbuildd/tests/test_2.diff b/lpbuildd/tests/test_2.diff
index 77362dd..1cd3720 100644
--- a/lpbuildd/tests/test_2.diff
+++ b/lpbuildd/tests/test_2.diff
@@ -2,12 +2,9 @@
 
 +++  
 
-@@ -1,13 +1,12 @@
+@@ -1,12 +1,11 @@
 
--build 370614-896976
- RUN: /usr/share/launchpad-buildd/bin/mount-chroot 370614-896976
- Mounting chroot for build 370614-896976
- RUN: /usr/share/launchpad-buildd/bin/apply-ogre-model 370614-896976 universe
+-/bin/apply-ogre-model 370614-896976 universe
  Attempting OGRE for universe in build-370614-896976
  RUN: /usr/share/launchpad-buildd/bin/update-debian-chroot 370614-896976
  Updating debian chroot for build 370614-896976
@@ -17,9 +14,12 @@
 -Get:3 http://user:blah@ftpmaster.internal gutsy/main Packages [1085kB]
 +Get:3 http://ftpmaster.internal gutsy/main Packages [1085kB]
  Get:4 http://ftpmaster.internal gutsy/universe Packages [3991kB]
+-Get:5 http://:macaroon@private-ppa.buildd/user/archive/ubuntu Packages [11kB]
++Get:5 http://private-ppa.buildd/user/archive/ubuntu Packages [11kB]
  Fetched 5142kB in 5s (1012kB/s)
  Reading package lists...
-@@ -28,9 +27,9 @@
+ Reading package lists...
+@@ -26,10 +25,10 @@
 
  Attempting OGRE for universe in build-370614-896976
  RUN: /usr/share/launchpad-buildd/bin/update-debian-chroot 370614-896976
@@ -31,6 +31,8 @@
 +Get:2 http://ftpmaster.internal gutsy Release [65.9kB]
 +Get:3 http://ftpmaster.internal gutsy/main Packages [1085kB]
  Get:4 http://ftpmaster.internal gutsy/universe Packages [3991kB]
+-Get:5 http://:macaroon@private-ppa.buildd/user/archive/ubuntu Packages [11kB]
++Get:5 http://private-ppa.buildd/user/archive/ubuntu Packages [11kB]
  Fetched 5142kB in 5s (1012kB/s)
 -socat STDIO PROXY:builder-proxy.launchpad.dev:github.com:443,proxyport=3128,proxyauth=user:blah
 +socat STDIO PROXY:builder-proxy.launchpad.dev:github.com:443,proxyport=3128
\ No newline at end of file
  Thread Previous • Date Previous • • Thread Next