Bug#579028: pbuilder: installs untrusted packages without asking
Junichi Uekawa
dancer at netfort.gr.jp
Thu Mar 8 22:36:25 UTC 2012
At Tue, 06 Mar 2012 02:29:25 +0100,
Simon Ruderich wrote:
>
> [1 <text/plain; us-ascii (7bit)>]
> Package: pbuilder
> Version: 0.206
> Tags: patch
> Followup-For: Bug #579028
>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> Dear Maintainer,
>
> The attached patch changes the defaults to always enforce signed
> repositories and aborts if an untrusted/manipulated package is
> installed. It adds the new option --keyring (APTKEYRINGS) to add
> additional keyrings, which are then used to verify the (local)
> signed repositories. This way no untrusted packages can be
> installed.
I don't share the opinion that this should be a grave bug to change
the default, and in order to change the default you need to deprecate
a command-line option and introduce two new command-line options when
you could have just changed the default shipping pbuilderrc.
> To still allow untrusted/unsigned repositories - they are a very
> bad idea and allow remote attackers performing a MITM to take
> over the system, including all built packages - the new option
> - --allow-untrusted (ALLOWUNTRUSTED) was added.
I don't care what you think is a bad idea.
>
> I tested it with the official Debian repository, signed and
> unsigned local repositories and it works fine for me. But I'm
> only a "normal" pbuilder user, so I might have missed something.
> Please test the patch.
>
> I haven't tested it with cdebootstrap, but it should work as
> well.
>
> The old PBUILDERSATISFYDEPENDSOPT --check-key option was
> deprecated and is no longer used (it emits a warning now) as
> validation is the default now.
>
> The patch also contains documentation updates for the new
> options/variables and updates for the NEWS file describing the
> necessary changes to continue using untrusted packages (but
> please don't do that - especially as a Debian developer).
>
> Please have a look and include the patch as soon as possible to
> fix this security issue.
Although I don't agree with most of the things you have stated in the
mail, your patch looks reasonably well written and I don't object to
applying it first and see who suffers; I suppose cowbuilder /
qemubuilder people will, because they don't yet support the new
option.
>
> Regards,
> Simon
>
> - -- System Information:
> Debian Release: wheezy/sid
> APT prefers unstable
> APT policy: (500, 'unstable')
> Architecture: amd64 (x86_64)
>
> Kernel: Linux 3.2.0-1-amd64 (SMP w/8 CPU cores)
> Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
> Shell: /bin/sh linked to /bin/dash
>
> Versions of packages pbuilder depends on:
> ii cdebootstrap 0.5.8+b1
> ii coreutils 8.13-3
> ii debconf [debconf-2.0] 1.5.41
> ii debianutils 4.2.1
> ii debootstrap 1.0.38
> ii dpkg-dev 1.16.1.2
> ii wget 1.13.4-2
>
> Versions of packages pbuilder recommends:
> pn devscripts 2.11.4
> pn fakeroot 1.18.2-1
> pn sudo <none>
>
> Versions of packages pbuilder suggests:
> pn cowdancer <none>
> pn gdebi-core <none>
> pn pbuilder-uml <none>
>
> - -- debconf information excluded
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.12 (GNU/Linux)
>
> iQIcBAEBCAAGBQJPVWhvAAoJEJL+/bfkTDL5ivAP/iayE8NRQnyk2HW8R+NiRXU3
> uavLilwwpmEZyuciu8GxMQIAhT9HYd/DlkhF9I+yBSd30TO3fl0xW7YV9SaIZ+bv
> IPwnZbHri4KfeV9Zob/gd2jrT9A2QCoFRW0ny4XNCK3NvtWH5KuH+TG2Mq5CQqdN
> j4VJ3+76oJcbQbU7AUYXfvKDAsEb7gX+VwTEFLS4GrPkni/FIQJ8HHJhlTscyuCD
> gQANCoRFZHVSMaas3xqi9KYFKgVS4BZ5Z/9FZuLeY5kWBfcbnIhQloVOWTQZIMRI
> PhnqP1g62XlPu71K3a/Y2RMAcy3Gs6sUbW4OianIr2iskCndejih/MCb+3LmBFCg
> Ekxi/CcJGrc7a0pV57Qs8Iwkm1siRZZUxcp4xdD3mo9iayoOt4sfFyrvBCYryilQ
> 7JKpQc3iNoV3EQql6KBu5G+GmFFWHmokpLvVY27n8LgkV2YSb2wrgxqXPfxcYHj7
> 0j/y2MFw+HOX/d5YSESMLxn9aiZBi7CkMtlMemzqizxlNlL/+OOZiDsi4vdH8L/j
> Y0c2i9efjNeooc0/B9wASu/Ck8SWV8wW1EcfTag0p9Rp0avy4hoQUmG+MtgQsV0l
> MQuWWysyxeJFX4Z8ooau82L6sIGC0L073JH6Y/C7uTOz9gKt+e5tV3fnU+pkWpqH
> oF3CcmlykKX4SYzhUI/e
> =6EPj
> -----END PGP SIGNATURE-----
> [2 0001-Enforce-valid-signed-repositories-by-default.patch <text/x-diff; us-ascii (7bit)>]
> >From cadc48fb599d436577a6efedc7f25e175652a3a1 Mon Sep 17 00:00:00 2001
> Message-Id: <cadc48fb599d436577a6efedc7f25e175652a3a1.1330997290.git.simon at ruderich.org>
> From: Simon Ruderich <simon at ruderich.org>
> Date: Tue, 6 Mar 2012 02:00:48 +0100
> Subject: [PATCH] Enforce valid signed repositories by default.
>
> ---
> debian/NEWS | 19 ++++++++++++++++++
> debian/pbuilder-test/00_prepinstall | 2 +-
> pbuilder-checkparams | 18 +++++++++++++++++
> pbuilder-createbuildenv | 5 ++++
> pbuilder-satisfydepends-aptitude | 2 +-
> pbuilder-satisfydepends-checkparams | 19 +++++++++--------
> pbuilder-updatebuildenv | 6 +++++
> pbuilder.8 | 20 ++++++++++++++++++-
> pbuilderrc | 23 +++++++++++++++++----
> pbuilderrc.5 | 36 ++++++++++++++++++++++++++--------
> 10 files changed, 124 insertions(+), 26 deletions(-)
>
> diff --git a/debian/NEWS b/debian/NEWS
> index 6d144b9..80d36e9 100644
> --- a/debian/NEWS
> +++ b/debian/NEWS
> @@ -1,3 +1,22 @@
> +pbuilder (0.207) unstable; urgency=low
> +
> + The default configuration will now only install trusted packages. This
> + prevents building packages with manipulated sources or a system compromise
> + due to a man-in-the-middle attack.
> +
> + However this also prevents installing packages from unsigned repositories by
> + default.
> +
> + If you really want to continue using unsigned repositories, you have to set
> + ALLOWUNTRUSTED=yes in your .pbuilderrc or use the --allow-untrusted option.
> + But if possible use a signed repository and set the used keys with the new
> + --keyring option (can be passed multiple times).
> +
> + Due to this change the PBUILDERSATISFYDEPENDSOPT option --check-key is no
> + longer necessary and thus deprecated.
> +
> + -- Simon Ruderich <simon at ruderich.org> Tue, 06 Mar 2012 02:02:38 +0100
> +
> pbuilder (0.197) unstable; urgency=low
>
> The default configuration will now enable ccache. To disable installation
> diff --git a/debian/pbuilder-test/00_prepinstall b/debian/pbuilder-test/00_prepinstall
> index 0c1d44c..1769cdd 100755
> --- a/debian/pbuilder-test/00_prepinstall
> +++ b/debian/pbuilder-test/00_prepinstall
> @@ -1,4 +1,4 @@
> #!/bin/bash
> # Prepare and install packages used in the tests.
>
> -apt-get install -y --force-yes sudo
> +apt-get install -y sudo
> diff --git a/pbuilder-checkparams b/pbuilder-checkparams
> index c031f18..3cdc48e 100755
> --- a/pbuilder-checkparams
> +++ b/pbuilder-checkparams
> @@ -207,6 +207,14 @@ while [ -n "$1" ]; do
> DEBOOTSTRAP="$2";
> shift; shift;
> ;;
> + --allow-untrusted)
> + ALLOWUNTRUSTED=yes;
> + shift;
> + ;;
> + --keyring)
> + APTKEYRINGS[${#APTKEYRINGS[@]}]="$2";
> + shift; shift;
> + ;;
> --save-after-login|--save-after-exec)
> SAVE_AFTER_LOGIN=yes;
> shift;
> @@ -312,3 +320,13 @@ fi
>
> # sort BINDMOUNTS to ensure that deeper directories are mounted last
> BINDMOUNTS="$(for i in $BINDMOUNTS; do echo $i; done | sort -u)"
> +
> +if [ "$ALLOWUNTRUSTED" = "yes" ]; then
> + PBUILDERSATISFYDEPENDSOPT[${#PBUILDERSATISFYDEPENDSOPT[@]}]='--allow-untrusted'
> + # Also duplicated in pbuilder-satisfydepends-checkparams!
> + # apt flag to accept untrusted packages
> + APTGETOPT[${#APTGETOPT[@]}]='--force-yes'
> + # aptitude flag to accept untrusted packages
> + APTITUDEOPT[${#APTITUDEOPT[@]}]='-o'
> + APTITUDEOPT[${#APTITUDEOPT[@]}]='Aptitude::CmdLine::Ignore-Trust-Violations=true'
> +fi
> diff --git a/pbuilder-createbuildenv b/pbuilder-createbuildenv
> index 6c69f98..b23c343 100755
> --- a/pbuilder-createbuildenv
> +++ b/pbuilder-createbuildenv
> @@ -78,6 +78,11 @@ mkdir -p "$BUILDPLACE/tmp/buildd"
>
> copy_local_configuration
> installaptlines
> +# To support package verification inside the repository we may have to import
> +# additional keys.
> +for KEY in "${APTKEYRINGS[@]}"; do
> + $CHROOTEXEC /usr/bin/apt-key add - < "${KEY}" > /dev/null
> +done
>
> executehooks "G"
>
> diff --git a/pbuilder-satisfydepends-aptitude b/pbuilder-satisfydepends-aptitude
> index 6ea6cb0..35a8374 100755
> --- a/pbuilder-satisfydepends-aptitude
> +++ b/pbuilder-satisfydepends-aptitude
> @@ -89,7 +89,7 @@ EOF
> $CHROOTEXEC sh -c "cat \"$BUILD_DEP_DEB_CONTROL\""
> $CHROOTEXEC sh -c "dpkg-deb -b \"$BUILD_DEP_DEB_DIR/pbuilder-satisfydepends-dummy\""
> $CHROOTEXEC dpkg --force-depends --force-conflicts -i "$BUILD_DEP_DEB_DIR/pbuilder-satisfydepends-dummy.deb" || true
> - $CHROOTEXEC aptitude -y --without-recommends -o APT::Install-Recommends=false "${PBUILDER_APTITUDE_CHECK_OPTS[@]}" -o Aptitude::ProblemResolver::StepScore=100 -o "Aptitude::ProblemResolver::Hints::KeepDummy=reject pbuilder-satisfydepends-dummy :UNINST" -o Aptitude::ProblemResolver::Keep-All-Level=55000 -o Aptitude::ProblemResolver::Remove-Essential-Level=maximum install pbuilder-satisfydepends-dummy
> + $CHROOTEXEC aptitude -y --without-recommends -o APT::Install-Recommends=false "${APTITUDEOPT[@]}" -o Aptitude::ProblemResolver::StepScore=100 -o "Aptitude::ProblemResolver::Hints::KeepDummy=reject pbuilder-satisfydepends-dummy :UNINST" -o Aptitude::ProblemResolver::Keep-All-Level=55000 -o Aptitude::ProblemResolver::Remove-Essential-Level=maximum install pbuilder-satisfydepends-dummy
> # check whether the aptitude's resolver kept the package
> if ! $CHROOTEXEC dpkg -l pbuilder-satisfydepends-dummy 2>/dev/null | grep -q ^ii; then
> echo "Aptitude couldn't satisfy the build dependencies"
> diff --git a/pbuilder-satisfydepends-checkparams b/pbuilder-satisfydepends-checkparams
> index 249b67e..b81c765 100755
> --- a/pbuilder-satisfydepends-checkparams
> +++ b/pbuilder-satisfydepends-checkparams
> @@ -27,13 +27,6 @@ FORCEVERSION=""
> CONTINUE_FAIL="no"
> CHROOTEXEC_AFTER_INTERNAL_CHROOTEXEC=no
>
> -# aptitude flag to ignore key verification
> -PBUILDER_APTITUDE_CHECK_OPTS=(
> - '-o'
> - 'Aptitude::CmdLine::Ignore-Trust-Violations=true' )
> -# apt flag to ignore key verification
> -PBUILDER_APT_GET_CHECK_OPTS="--force-yes"
> -
> while [ -n "$1" ]; do
> case "$1" in
> --control|-c)
> @@ -80,8 +73,16 @@ while [ -n "$1" ]; do
> shift;
> ;;
> --check-key)
> - unset PBUILDER_APTITUDE_CHECK_OPTS
> - unset PBUILDER_APT_GET_CHECK_OPTS
> + log 'W: --check-key is now enabled by default and thus deprecated.'
> + shift;
> + ;;
> + --allow-untrusted)
> + # Also duplicated in pbuilder-checkparams!
> + # apt flag to accept untrusted packages
> + APTGETOPT[${#APTGETOPT[@]}]='--force-yes'
> + # aptitude flag to accept untrusted packages
> + APTITUDEOPT[${#APTITUDEOPT[@]}]='-o'
> + APTITUDEOPT[${#APTITUDEOPT[@]}]='Aptitude::CmdLine::Ignore-Trust-Violations=true'
> shift;
> ;;
> --help|-h|*)
> diff --git a/pbuilder-updatebuildenv b/pbuilder-updatebuildenv
> index cdcc22c..737f53d 100755
> --- a/pbuilder-updatebuildenv
> +++ b/pbuilder-updatebuildenv
> @@ -72,6 +72,12 @@ $CHROOTEXEC /usr/bin/apt-get -q -y "${APTGETOPT[@]}" autoremove || true
> $CHROOTEXEC /usr/bin/apt-get -q -y "${APTGETOPT[@]}" install build-essential dpkg-dev $EXTRAPACKAGES
> save_aptcache
>
> +# To support package verification inside the repository we may have to import
> +# additional keys.
> +for KEY in "${APTKEYRINGS[@]}"; do
> + $CHROOTEXEC /usr/bin/apt-key add - < "${KEY}" > /dev/null
> +done
> +
> # optionally auto-clean apt-cache
> if [ "${AUTOCLEANAPTCACHE}" = "yes" -a -n "$APTCACHE" ]; then
> log "I: Cleaning the cached apt archive"
> diff --git a/pbuilder.8 b/pbuilder.8
> index 478be40..a2f37c3 100644
> --- a/pbuilder.8
> +++ b/pbuilder.8
> @@ -361,7 +361,7 @@ specified in a space-delimited manner, surrounded in double quotations, like:
> .B """/srv /somedir /someotherdir"""
>
> .TP
> -.BI "\-\-debootstrapopts " "\-\-variant=buildd"
> +.BI "\-\-debootstrapopts " "\-\-variant=buildd" " " "\-\-keyring" " " "/usr/share/keyrings/debian\-archive\-keyring.gpg"
> Add extra command-line options to debootstrap.
>
> Specify multiple options through multiple instance of this
> @@ -380,6 +380,24 @@ and default is to use
> .B debootstrap.
>
> .TP
> +.BI "\-\-allow\-untrusted "
> +Allow untrusted (no key installed) and unsigned repositories.
> +.BI Warning:
> +Enabling this option may allow remote attackers to compromise the system.
> +Better use signed repositories and
> +.B "\-\-keyring"
> +to add the key(s).
> +
> +.TP
> +.BI "\-\-keyring " "path/to/keyring"
> +Additional keyrings to use for package verification with apt, not used for
> +debootstrap (use
> +.B "\-\-debootstrapopts"
> +). Use this to add (local) signed repositories. By default the
> +debian-archive-keyring package inside the chroot is used. Can be specified
> +multiple times.
> +
> +.TP
> .BI "\-\-save\-after\-login "
> .TP
> .BI "\-\-save\-after\-exec "
> diff --git a/pbuilderrc b/pbuilderrc
> index 2ee51e6..b9f1b95 100644
> --- a/pbuilderrc
> +++ b/pbuilderrc
> @@ -53,11 +53,19 @@ PBUILDERROOTCMD="sudo -E"
> # not support unsigned APT repositories
> PBUILDERSATISFYDEPENDSCMD="/usr/lib/pbuilder/pbuilder-satisfydepends"
>
> -# You can optionally make pbuilder check key by setting the following flags
> -# PBUILDERSATISFYDEPENDSOPT=('--check-key')
> -# unset PBUILDERSATISFYDEPENDSOPT
> -# option to pass to apt-get always.
> -export APTGETOPT=('--force-yes')
> +# Arguments for $PBUILDERSATISFYDEPENDSCMD.
> +# PBUILDERSATISFYDEPENDSOPT=()
> +
> +# You can optionally make pbuilder accept untrusted repositories by setting
> +# this option to yes, but this may allow remote attackers to compromise the
> +# system. Better set a valid key for the signed (local) repository with
> +# $APTKEYRINGS (see below).
> +ALLOWUNTRUSTED=no
> +
> +# Option to pass to apt-get always.
> +export APTGETOPT=()
> +# Option to pass to aptitude always.
> +export APTITUDEOPT=()
>
> #Command-line option passed on to dpkg-buildpackage.
> #DEBBUILDOPTS="-IXXX -iXXX"
> @@ -82,6 +90,11 @@ DEBOOTSTRAPOPTS=(
> # or unset it to make it not a buildd type.
> # unset DEBOOTSTRAPOPTS
>
> +# Keyrings to use for package verification with apt, not used for debootstrap
> +# (use DEBOOTSTRAPOPTS). By default the debian-archive-keyring package inside
> +# the chroot is used.
> +APTKEYRINGS=()
> +
> # Set the PATH I am going to use inside pbuilder: default is "/usr/sbin:/usr/bin:/sbin:/bin:/usr/X11R6/bin"
> export PATH="/usr/sbin:/usr/bin:/sbin:/bin:/usr/X11R6/bin"
>
> diff --git a/pbuilderrc.5 b/pbuilderrc.5
> index 14fde73..40fc8bb 100644
> --- a/pbuilderrc.5
> +++ b/pbuilderrc.5
> @@ -178,17 +178,25 @@ may also be used to reset the list of options.
>
> The default value is to build source and binary package.
> .TP
> -.BI "DEBOOTSTRAPOPTS=" "( '\-\-variant=buildd' )"
> +.BI "DEBOOTSTRAPOPTS=" "( '\-\-variant=buildd' '\-\-keyring' '/usr/share/keyrings/debian\-archive\-keyring.gpg' )"
> When this option is set to
> .B "\-\-variant=buildd"
> .B "pbuilder"
> will invoke
> .B "$DEBOOTSTRAP"
> -with "\-\-variant=buildd"
> +with
> +.B "\-\-variant=buildd"
> option, which results in debootstrap creating a minimal chroot for
> buildd instead of trying to create a minimal installation chroot.
> -.B "DEBOOTSTRAP"
> -is another directive in this file.
> +.B "\-\-keyring"
> +is used to specify a keyring for debootstrap.
> +.TP
> +.BI "APTKEYRINGS=" "()"
> +Additional keyrings to use for package verification with apt, not used for
> +debootstrap (use
> +.B "$DEBOOTSTRAPOPTS"
> +). Use this to add (local) signed repositories. By default the
> +debian-archive-keyring package inside the chroot is used.
> .TP
> .BI "DEBOOTSTRAP=" "debootstrap"
> Use this option to switch the implementation of
> @@ -329,15 +337,25 @@ used until 0.172.
>
> The default is now "aptitude".
> .TP
> -.BI "PBUILDERSATISFYDEPENDSOPT=" "('\-\-check\-key')"
> +.BI "PBUILDERSATISFYDEPENDSOPT=" "()"
> Array of flags to give to pbuilder\-satisfydepends.
> -Specifying \-\-check\-key here will try to verify key signatures.
>
> .TP
> -.BI "APTGETOPT=" "('\-\-force\-yes')"
> +.BI "ALLOWUNTRUSTED=" "no"
> +Allow untrusted (no key installed) and unsigned repositories.
> +.BI Warning:
> +Enabling this option may allow remote attackers to compromise the system.
> +Better use signed repositories and
> +.B "$APTKEYRINGS"
> +to add the key(s).
> +
> +.TP
> +.BI "APTGETOPT=" "()"
> Extra flags to give to apt\-get.
> -Default is \-\-force\-yes, which will skip key verification of packages
> -to be installed. Unset if you want to enable key verification.
> +
> +.TP
> +.BI "APTITUDEGETOPT=" "()"
> +Extra flags to give to aptitude.
>
> .TP
> .BI "REMOVEPACKAGES=" "lilo"
> --
> 1.7.9.1
>
> [3 0001-Enforce-valid-signed-repositories-by-default.patch.asc <application/pgp-signature (base64)>]
>
> [4 <text/plain; us-ascii (7bit)>]
> _______________________________________________
> Pbuilder-maint mailing list
> Pbuilder-maint at lists.alioth.debian.org
> http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pbuilder-maint
More information about the Pbuilder-maint
mailing list