[arch-projects] [devtools] devtools maintenance
Hi, I'm going to stand in as maintainer of devtools as long as my time and self- interest permits. I've already scoured arch-projects for the past few months and pushed anything which was trivial and/or didn't have any strong objections. I also pushed another patch or two from my own branch which solved some issues which were raised in patch form. I realize that I've probably missed a few patches. If you care about a patch you sent recently and don't see it on master, please rebase and resend. I'm attaching the remaining 2 patches from my repo that I haven't pushed. Feedback welcome. If all goes well, I'll be tagging a new release of devtools by the end of the week. Cheers! Dave Reisner (2): Include end of options delimeter in makepkg_args makechrootpkg: build as same UID as invoker archbuild.in | 2 +- makechrootpkg.in | 20 +++++++++++--------- 2 files changed, 12 insertions(+), 10 deletions(-) -- 2.1.0
This prevents the need for: extra-x86_64-build -- -- -R --- archbuild.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/archbuild.in b/archbuild.in index dc45c7f..795bebf 100644 --- a/archbuild.in +++ b/archbuild.in @@ -38,7 +38,7 @@ while getopts 'hcr:' arg; do done # Pass all arguments after -- right to makepkg -makechrootpkg_args+=("${@:$OPTIND}") +makechrootpkg_args+=(-- "${@:$OPTIND}") check_root "$0" "$@" -- 2.1.0
Changing UID to that of 'nobody' is arbitrary at best, and an information leak at worst. Let's just drop back to the same UID of the invoker. --- makechrootpkg.in | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/makechrootpkg.in b/makechrootpkg.in index 8bc18a4..9bb0bfa 100644 --- a/makechrootpkg.in +++ b/makechrootpkg.in @@ -234,7 +234,9 @@ prepare_chroot() { echo 'SRCDEST="/srcdest"' >> "$copydir/etc/makepkg.conf" fi - chown -R nobody "$copydir"/{build,pkgdest,srcpkgdest,logdest,srcdest,startdir} + builduser_uid=${SUDO_UID:-$UID} + useradd -R "$copydir" -g users -u "$builduser_uid" -s /bin/nologin builduser + chown -R "$builduser_uid" "$copydir"/{build,pkgdest,srcpkgdest,logdest,srcdest,startdir} if [[ -n $MAKEFLAGS ]]; then sed -i '/^MAKEFLAGS=/d' "$copydir/etc/makepkg.conf" @@ -246,12 +248,12 @@ prepare_chroot() { echo "PACKAGER='${PACKAGER}'" >> "$copydir/etc/makepkg.conf" fi - if [[ ! -f $copydir/etc/sudoers.d/nobody-pacman ]]; then - cat > "$copydir/etc/sudoers.d/nobody-pacman" <<EOF + if [[ ! -f $copydir/etc/sudoers.d/builduser-pacman ]]; then + cat > "$copydir/etc/sudoers.d/builduser-pacman" <<EOF Defaults env_keep += "HOME" -nobody ALL = NOPASSWD: /usr/bin/pacman +builduser ALL = NOPASSWD: /usr/bin/pacman EOF - chmod 440 "$copydir/etc/sudoers.d/nobody-pacman" + chmod 440 "$copydir/etc/sudoers.d/builduser-pacman" fi # This is a little gross, but this way the script is recreated every time in the @@ -302,7 +304,7 @@ _chrootbuild() { for vcsdir in */.$vcs; do rm "${vcsdir%/.$vcs}" cp -a "${dir}_host/${vcsdir%/.$vcs}" . - chown -R nobody "${vcsdir%/.$vcs}" + chown -R builduser "${vcsdir%/.$vcs}" done done done @@ -312,7 +314,7 @@ _chrootbuild() { # XXX: Keep PKGBUILD writable for pkgver() rm PKGBUILD* cp /startdir_host/PKGBUILD* . - chown nobody PKGBUILD* + chown builduser PKGBUILD* # Safety check if [[ ! -w PKGBUILD ]]; then @@ -320,13 +322,13 @@ _chrootbuild() { exit 1 fi - sudo -u nobody makepkg $makepkg_args || exit 1 + sudo -u builduser makepkg $makepkg_args || exit 1 if $run_namcap; then pacman -S --needed --noconfirm namcap for pkgfile in /startdir/PKGBUILD /pkgdest/*; do echo "Checking ${pkgfile##*/}" - sudo -u nobody namcap "$pkgfile" 2>&1 | tee "/logdest/${pkgfile##*/}-namcap.log" + sudo -u builduser namcap "$pkgfile" 2>&1 | tee "/logdest/${pkgfile##*/}-namcap.log" done fi -- 2.1.0
On 22/09/2014 14:35, Dave Reisner wrote:
Changing UID to that of 'nobody' is arbitrary at best, and an information leak at worst. Let's just drop back to the same UID of the invoker.
Which information is leaking? This should also fix the permission issue on file introduced by bind mounting $startdir instread of copying and have files owned by nobody. Nice patch! -- Sébastien "Seblu" Luttringer https://seblu.net | Twitter: @seblu42 GPG: 0x2072D77A
On Tue, Sep 30, 2014 at 11:23:50PM +0200, Sébastien Luttringer wrote:
On 22/09/2014 14:35, Dave Reisner wrote:
Changing UID to that of 'nobody' is arbitrary at best, and an information leak at worst. Let's just drop back to the same UID of the invoker.
Which information is leaking?
"nobody" in the build chroot is exactly the same "nobody" as outside the chroot. So, someone running as "nobody" has full control over the build as it occurs in the chroot. ptrace it, do whatever you want to it (including creating a malicious binary). There's no reason not to drop privileges back to the user who invoked the build.
This should also fix the permission issue on file introduced by bind mounting $startdir instread of copying and have files owned by nobody.
Neat! I've found one breakage in the patch (user creation is a pain in the ass across architectures because of dlopen), but that's fixed locally. d
participants (3)
-
Dave Reisner
-
Dave Reisner
-
Sébastien Luttringer