Merge ~litios/ubuntu-security-tools:build-sources-list/use-auth-conf into ubuntu-security-tools:master

Proposed by David Fernandez Gonzalez
Status: Merged
Merged at revision: 086da15eec9afb6cc66ca1e055c766fee1fa4acd
Proposed branch: ~litios/ubuntu-security-tools:build-sources-list/use-auth-conf
Merge into: ubuntu-security-tools:master
Diff against target: 168 lines (+69/-37)
3 files modified
build-tools/build-sources-list (+16/-36)
build-tools/esm-auth-conf (+41/-0)
build-tools/setup-sources-list.install (+12/-1)
Reviewer Review Type Date Requested Status
Alex Murray Approve
Spyros Seimenis Approve
Review via email: mp+453802@code.launchpad.net

Description of the change

Build-sources-list currently embeds the credentials for external PPA in the sources list file. This is deprecated in APT and we should use auth conf files.

This PR addresses this issue by creating the proper auth conf file for the ESM PPAs.

Also, note that sec-buildenv will start relying on this soon: https://github.com/canonical/sec-buildenv/issues/91

Another change is added to the PR. For customer PPA / ESM purposes, we also need -updates to be added to the sources lists. See https://code.launchpad.net/~litios/ubuntu-security-tools/+git/ubuntu-security-tools/+ref/ppas-tag-migration for handling the different pockets that will now coexist (MR tbd)

Testing: https://pastebin.canonical.com/p/B6FxRCF9pd/

To post a comment you must log in.
Revision history for this message
Alex Murray (alexmurray) wrote :

This is a great improvement but I have concerns about having it silently write the the auth conf file behind the scenes via sudo - also to support umt download the auth conf file needs to have world readable permissions as per https://bugs.launchpad.net/ubuntu/+source/apt/+bug/1904068

Revision history for this message
Alex Murray (alexmurray) :
review: Needs Fixing
Revision history for this message
David Fernandez Gonzalez (litios) wrote :

Hey Alex, thanks for the review!

I refactored it to be a new file and added an echo statement in the original file for people to know they should run it too to get the credentials.

Regarding the permission issue, having the group as sudo for the file should allow the main user and any other user in the sudo group to be able to read the file. It's the same as we are doing for the files in sources.list.d too. https://pastebin.canonical.com/p/3CXGFDPzqV/.

It makes it work for umt while not setting world-read permission. But let me know if you find an issue with this and I can revert it to world-read.

Revision history for this message
Spyros Seimenis (sespiros) wrote :

Great improvement idd although the more I edit build-sources-list the more I feel it needs a bigger revamp.

In sec-buildenv I create esm-ppa.conf with permissions "640 root sudo" and I haven't encountered issues.

I also think it's a bit awkward to run sudo in the (now) new script but that's exactly what build-sources-list.install does at the moment.

I suggest having 3 scripts in total so that we also avoid duplicating things:

1) Have the new esm-auth-conf script simply output the right auth configuration and rename it to setup-esm-auth-conf

2) Have build-sources-list.install run both build-sources-list and esm-auth-conf and then install both source list and auth file. Maybe also rename it to something like setup-sources-list.

3) Remove sudo from build-sources-list.install, let if produce an error if it is not run with sudo

We would also need to update sec-buildenv and the wiki.

+ minor comments

review: Needs Fixing
Revision history for this message
Steve Beattie (sbeattie) wrote :

On Wed, Oct 18, 2023 at 12:00:43PM -0000, Spyros Seimenis wrote:
> 3) Remove sudo from build-sources-list.install, let if produce an error if it is not run with sudo

One challenge here is that the auth credentials are queried
from launchpad through an authenticated connection via
launchpadlib. Depending on what the user has done before this and/or
how sudo is invoked, it could end up with either a cached credential
in the /root/ homedir *or* root owned files in the user's directory.

I did not like embedding sudo in the original script, but trying to
navigate things that need to happen as the user and things that need
to happen as root was tricky, which is the current compromise is the
way it is.

One comment I'd also make is that when I thought about doing this,
my intent was to create an auth.conf.d file per ppa.

As to blatting over the existing one, I'm not sure the ppa
subscription credentials ever change -- maybe if you unsubscribe and
then resubscribe to the ppa -- but if they did, I'd want to think
carefully about what situations the existing file should or should
not get overwritten.

+1 on adding the esm-*-updates ppas by default.

--
Steve Beattie
<email address hidden>

Revision history for this message
David Fernandez Gonzalez (litios) wrote (last edit ):

Hi everyone, thanks for the reviews.
I've made several changes based on the feedback:

----

> 1) Have the new esm-auth-conf script simply output the right auth configuration.

  - Updated!

> 2) Have build-sources-list.install run both build-sources-list and esm-auth-conf and then install both source list and auth file. Maybe also rename it to something like setup-sources-list.

  - Related to 1), now the installation is handled by setup-sources-list.install.

> 3) One comment I'd also make is that when I thought about doing this, my intent was to create an auth.conf.d file per ppa.

  - Applied too! This was an interesting combination with 2) so the bash to do so is not the easiest. It works though. One thing is that I'm using the auth.conf.d itself with a sudo tmp directory so everything stays "secure" and no race condition is present.

----

I did not rename the script to setup-esm-auth-conf as I think the word `setup` means we are actually installing something (as the other script called setup-sources-list)

In regards to:

> 3) Remove sudo from build-sources-list.install, let it produce an error if it is not run with sudo

I agree we can have a discussion for this but it seems like an issue for another PR. We need the -updates set up ASAP for OVAL and I feel like also dealing with the sudo issue in this PR may things take longer for this particular change.

In regards to:

> As to blatting over the existing one, I'm not sure the ppa
subscription credentials ever change -- maybe if you unsubscribe and
then resubscribe to the ppa -- but if they did, I'd want to think
carefully about what situations the existing file should or should
not get overwritten.

If it doesn't change, overriding it wouldn't make a difference. If it does change, that means the old credentials are no longer valid (I assume) and therefore there is no use in keeping it, overriding makes sense to me for that particular case.

Let me know of any improvements!

Revision history for this message
Spyros Seimenis (sespiros) :
review: Approve
Revision history for this message
Alex Murray (alexmurray) wrote :

LGTM - the one thing perhaps would be to have build-sources-list have a comment or even output a message saying that in general it shouldn't be run directly now and instead to run setup-sources-list.install - but that is fine to leave for later.

review: Approve
Revision history for this message
David Fernandez Gonzalez (litios) wrote :

Added to the help of both scripts, as the output is used to create the files. Thanks for all the reviews :)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/build-tools/build-sources-list b/build-tools/build-sources-list
2index 97c8e0a..73d4c28 100755
3--- a/build-tools/build-sources-list
4+++ b/build-tools/build-sources-list
5@@ -24,7 +24,8 @@ debmirror, you might want to specify '--add-fallbacks' to this script, which
6 will add the official Ubuntu archive after your mirrors. This will ensure you
7 have the most up to date sources.
8
9-Typical usage:
10+Typical usage should be through the use of setup-sources-list.install.
11+In case you want to run it yourself, the expected usage is:
12 $ build-sources-list | sudo sh -c 'cat > /etc/apt/sources.list.d/ubuntu-security.list && \
13 chgrp root.sudo /etc/apt/sources.list.d/ubuntu-security.list && \
14 chmod 640 /etc/apt/sources.list.d/ubuntu-security.list'
15@@ -63,34 +64,6 @@ deb-src http://ppa.launchpad.net/${team}/${ppa}/ubuntu ${release} main
16 EOM
17 }
18
19-emit_private_ppa_overlay() {
20- local team=$1
21- local ppa=$2
22- local release=$3
23- local keyring="$4"
24- local url
25- local signed_by
26-
27- url="$("$UST"/lp_tools/get_ppa_subscription_url.py "$team" "$ppa")"
28- if [ "$?" -ne 0 ] ; then
29- echo "failed to get subscription info for $team/$ppa" 1>&2
30- return
31- fi
32- if [ -n "$keyring" ] ; then
33- signed_by="[signed-by=$keyring]"
34- fi
35-
36- cat <<EOM
37-# Reference: https://launchpad.net/~${team}/+archive/ubuntu/$ppa
38-#
39-# this should be protected via mode 640 root:sudo
40-# ideally, we'd separate out the auth bits into files in /etc/apt/auth.conf.d/
41-deb-src $signed_by $url $release main
42-
43-EOM
44- echo "# IMPORTANT: if this is the first time you have configured ppa:$team/$ppa, you will need to import the ppa archive key into ${ESM_PPA_KEYRING}" 1>&2
45-}
46-
47 cat <<EOM
48 #
49 # Output of 'build-sources-list' for ubuntu-security-tools
50@@ -125,21 +98,28 @@ EOM
51 # ESM overlay ppas
52 elif echo "$r" | grep -q "/esm" ; then
53 ppa_overlay_release=$(echo "$r" | cut -f 1 -d '/')
54- ppa_overlay="esm"
55 if [ $ppa_overlay_release = "trusty" ]; then
56- ppa_overlay="esm-infra-security"
57+ cat <<EOM
58+deb-src [signed-by=${ESM_PPA_KEYRING}] https://private-ppa.launchpadcontent.net/ubuntu-esm/esm-infra-security/ubuntu $ppa_overlay_release main
59+deb-src [signed-by=${ESM_PPA_KEYRING}] https://private-ppa.launchpadcontent.net/ubuntu-esm/esm-infra-updates/ubuntu $ppa_overlay_release main
60+EOM
61+ else
62+ cat <<EOM
63+deb-src [signed-by=${ESM_PPA_KEYRING}] https://private-ppa.launchpadcontent.net/ubuntu-esm/esm/ubuntu $ppa_overlay_release main
64+EOM
65 fi
66-
67- emit_private_ppa_overlay "ubuntu-esm" "$ppa_overlay" "${ppa_overlay_release}" "${ESM_PPA_KEYRING}"
68 echo "# Requires gpg keyid DBB1FC89762BF6B96707C4059BC0A1A1622CF918 stored in ${ESM_PPA_KEYRING}"
69 # newer ESM releases are in the esm-infra-security and
70 # esm-apps-security ppas
71 elif echo "$r" | grep -q "esm-" ; then
72 ppa_overlay_release=$(echo "$r" | cut -f 2 -d '/')
73 product=$(echo "$r" | cut -f 1 -d '/')
74- ppa_overlay="$product-security"
75-
76- emit_private_ppa_overlay "ubuntu-esm" "$ppa_overlay" "${ppa_overlay_release}" "${ESM_PPA_KEYRING}"
77+ for pocket in "security" "updates" ; do
78+ ppa_overlay="$product-$pocket"
79+ cat <<EOM
80+deb-src [signed-by=${ESM_PPA_KEYRING}] https://private-ppa.launchpadcontent.net/ubuntu-esm/$ppa_overlay/ubuntu $ppa_overlay_release main
81+EOM
82+ done
83 echo "# Requires gpg keyid DBB1FC89762BF6B96707C4059BC0A1A1622CF918 stored in ${ESM_PPA_KEYRING}"
84 # normal ubuntu releases.
85 else
86diff --git a/build-tools/esm-auth-conf b/build-tools/esm-auth-conf
87new file mode 100755
88index 0000000..8e9c154
89--- /dev/null
90+++ b/build-tools/esm-auth-conf
91@@ -0,0 +1,41 @@
92+#!/bin/bash
93+#
94+# Copyright (C) 2023 Canonical, Ltd.
95+# Author: David Fernandez Gonzalez <david.fernandezgonzalez@canonical.com>
96+# License: GPLv3
97+#
98+
99+set -e
100+
101+help() {
102+ cat <<EOM
103+Usage: esm-auth-conf
104+
105+This script will query Launchpad to obtaing the credentials needed
106+by apt to query ESM PPAs and output this information to stdout.
107+
108+Typical usage should be through the use of setup-sources-list.install.
109+EOM
110+}
111+
112+add_esm_auth_conf() {
113+ ppas=(
114+ "esm-infra-updates"
115+ "esm-infra-security"
116+ "esm-apps-updates"
117+ "esm-apps-security"
118+ "esm"
119+ )
120+
121+ for ppa in "${ppas[@]}"; do
122+ echo "# Auth info for ${ppa}"
123+ $UST/lp_tools/get_ppa_subscription_url.py -a ubuntu-esm ${ppa}
124+ done
125+}
126+
127+if [ "$1" = "help" ] || [ "$1" = "--help" ] || [ "$1" = "-h" ]; then
128+ help
129+ exit 0
130+fi
131+
132+add_esm_auth_conf
133diff --git a/build-tools/build-sources-list.install b/build-tools/setup-sources-list.install
134similarity index 89%
135rename from build-tools/build-sources-list.install
136rename to build-tools/setup-sources-list.install
137index 8fd7e34..11280e7 100755
138--- a/build-tools/build-sources-list.install
139+++ b/build-tools/setup-sources-list.install
140@@ -10,9 +10,11 @@ set -e
141 # command
142
143 SOURCES_LIST="/etc/apt/sources.list.d/ubuntu-security.list"
144-PER_REPO_GPG_DIR="/etc/apt/keyrings/"
145+PER_REPO_GPG_DIR="/etc/apt/keyrings"
146 ESM_GPG_PUBLIC_KEY="${PER_REPO_GPG_DIR}/ubuntu-esm-ppas.gpg"
147 ESM_GPG_KEY_FINGERPRINT="DBB1FC89762BF6B96707C4059BC0A1A1622CF918"
148+AUTH_CONF_DIRECTORY="/etc/apt/auth.conf.d"
149+AUTH_CONF_TMP_DIRECTORY="esm-tmp-auth-conf"
150
151 help() {
152 cat <<EOM
153@@ -62,6 +64,15 @@ echo "Setting up the sources.list in ${SOURCES_LIST}"
154 chgrp sudo ${SOURCES_LIST} && \
155 chmod 640 ${SOURCES_LIST}"
156
157+echo "Setting up the auth.conf files for ESM"
158+sudo sh -c "mkdir -p ${AUTH_CONF_DIRECTORY}/${AUTH_CONF_TMP_DIRECTORY} && \
159+ chmod 640 ${AUTH_CONF_DIRECTORY}/${AUTH_CONF_TMP_DIRECTORY}"
160+sudo sh -c " echo \"$("${UST}"/build-tools/esm-auth-conf)\" | awk -v RS= '{print > (\"${AUTH_CONF_DIRECTORY}/${AUTH_CONF_TMP_DIRECTORY}/\"\$5\".conf\")}'"
161+sudo sh -c "chgrp sudo -R ${AUTH_CONF_DIRECTORY}/${AUTH_CONF_TMP_DIRECTORY} && \
162+ chmod 640 -R ${AUTH_CONF_DIRECTORY}/${AUTH_CONF_TMP_DIRECTORY} && \
163+ mv ${AUTH_CONF_DIRECTORY}/${AUTH_CONF_TMP_DIRECTORY}/* ${AUTH_CONF_DIRECTORY}/ && \
164+ rm -rf ${AUTH_CONF_DIRECTORY}/${AUTH_CONF_TMP_DIRECTORY}"
165+
166 if [ -f "${ESM_GPG_PUBLIC_KEY}" ] ; then
167 echo "ppa key file ${ESM_GPG_PUBLIC_KEY} already exists"
168 else

Subscribers

People subscribed via source and target branches