On Sun, 26 Feb 2017 at 14:31:31 +0000, Simon McVittie wrote: > On Wed, 15 Feb 2017 at 11:13:39 +0000, Simon McVittie wrote: > > If debootstrap inside a container is meant to work, then this would seem > > like a job for an autopkgtest. I'll try to write one if someone can tell me > > which containerization technologies (systemd-nspawn? lxc? both? others?) > > are the ones of interest here. > > I've had some trouble making a working autopkgtest that runs under qemu > and runs debootstrap under an additional layer of systemd-nspawn, so the > next best thing for now is manual testing.
Here is an updated version of my earlier patches. The actual RC bug fix is the same (I think). The autopkgtest is a rewrite in a not-/bin/sh language (Perl) for better robustness/error handling/etc., with better coverage, including use of systemd-nspawn to exercise the code path where mknod is restricted (which I believe was Marco's use case for the symlink) if the autopkgtest is run under autopkgtest-virt-qemu. Tested locally under autopkgtest-virt-lxc, autopkgtest-virt-qemu and autopkgtest-virt-schroot, with the following results: * My patch lets simplified fake versions of current pbuilder, and current schroot with the default profile, access /dev/ptmx in debootstrap-created chroots again, if debootstrap was not run with mknod restricted (systemd-nspawn). I think that's good enough to close the RC bug. * If running in lxc on a jessie kernel, schroot with profile=sbuild will not be allowed to access /dev/ptmx. This does not appear to be a regression, or debootstrap's fault. lxc on a stretch kernel is fine. * If mknod is restricted (systemd-nspawn), debootstrap prints a warning, and current schroot and pbuilder will fail to access /dev/ptmx. I think this can only be addressed by patching schroot and pbuilder. Any thoughts on this from the debootstrap maintainers? S
>From e070e9c9b837815b43eb39140b7005124ed3906f Mon Sep 17 00:00:00 2001 From: Simon McVittie <s...@debian.org> Date: Mon, 20 Feb 2017 09:22:07 +0000 Subject: [PATCH 1/2] Don't make /dev/ptmx a symlink to pts/ptmx if we don't have to In a plain chroot or on real hardware, it is preferable to use mknod to create /dev/ptmx. This works as intended with older chroot managers such as sbuild and pbuilder, which were designed for the semantics of "legacy" /dev/pts (a single non-virtualized pty subsystem per kernel) and so mount /dev/pts without the newinstance option. It also works in newer kernels where /dev/pts always behaves as though the newinstance option was given, because on those kernels, opening a (c,5,2) device node automatically looks for an adjacent pts directory and uses its ptmx device node instead. However, if we are running debootstrap inside a restricted container such as lxc or systemd-nspawn, mknod ptmx c 5 2 might not be allowed. If so, fall back to a symlink with a warning. This mode is fine if the debootstrap will be used with systemd-nspawn or lxc, or if a devtmpfs will be mounted over its /dev, but will not work for older chroot managers like sbuild or pbuilder, because those chroot managers leave the ptmxmode mount option at its default 000, causing permission to open the pts/ptmx device node to be denied. Bug: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=817236 Signed-off-by: Simon McVittie <s...@debian.org> --- functions | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/functions b/functions index 6cbbd3b..4a4f2f9 100644 --- a/functions +++ b/functions @@ -1173,7 +1173,12 @@ setup_devices_simple () { mknod -m 666 $TARGET/dev/urandom c 1 9 mknod -m 666 $TARGET/dev/tty c 5 0 mkdir $TARGET/dev/pts/ $TARGET/dev/shm/ - ln -s pts/ptmx $TARGET/dev/ptmx + # Inside a container, we might not be allowed to create /dev/ptmx. + # If not, do the next best thing. + if ! mknod -m 666 $TARGET/dev/ptmx c 5 2; then + warning MKNOD "Could not create /dev/ptmx, falling back to symlink. This chroot will require /dev/pts mounted with ptmxmode=666" + ln -s pts/ptmx $TARGET/dev/ptmx + fi ln -s /proc/self/fd $TARGET/dev/fd ln -s /proc/self/fd/0 $TARGET/dev/stdin ln -s /proc/self/fd/1 $TARGET/dev/stdout -- 2.11.0
>From 3926ec16fbbcea6b9fd1699f0be3e99a3bf346f8 Mon Sep 17 00:00:00 2001 From: Simon McVittie <s...@debian.org> Date: Fri, 24 Feb 2017 09:45:33 +0000 Subject: [PATCH 2/2] Add an autopkgtest covering #817236 and various other sanity checks Because debootstrap is relatively slow, I've named the test according to what is being bootstrapped (Debian testing) rather than the checks that are performed, with the intention that additional checks can be added to it. Signed-off-by: Simon McVittie <s...@debian.org> --- debian/tests/control | 10 ++ debian/tests/debian-testing | 270 +++++++++++++++++++++++++++++++++++ debian/tests/fake/pbuilder-0.228.4-1 | 31 ++++ debian/tests/fake/schroot-1.6.10-3 | 47 ++++++ 4 files changed, 358 insertions(+) create mode 100644 debian/tests/control create mode 100755 debian/tests/debian-testing create mode 100755 debian/tests/fake/pbuilder-0.228.4-1 create mode 100755 debian/tests/fake/schroot-1.6.10-3 diff --git a/debian/tests/control b/debian/tests/control new file mode 100644 index 0000000..12d78c7 --- /dev/null +++ b/debian/tests/control @@ -0,0 +1,10 @@ +Tests: debian-testing +Depends: + debootstrap, + libdistro-info-perl, + libdpkg-perl, + libipc-run-perl, + perl, + systemd [linux-any], + systemd-container [linux-any], +Restrictions: allow-stderr, needs-root diff --git a/debian/tests/debian-testing b/debian/tests/debian-testing new file mode 100755 index 0000000..fb7da66 --- /dev/null +++ b/debian/tests/debian-testing @@ -0,0 +1,270 @@ +#!/usr/bin/perl +# Verify that debootstrap'ing Debian testing produces a usable chroot, +# and in particular that using it with early 2017 versions of schroot and +# pbuilder results in working pseudo-terminals (#817236) +# +# Copyright © 2017 Simon McVittie +# SPDX-License-Identifier: MIT +# (see debian/copyright) + +use strict; +use warnings; + +use Cwd qw(getcwd); +use Debian::DistroInfo; +use Dpkg::Version; +use IPC::Run qw(run); +use Test::More; + +my $srcdir = getcwd; + +sub verbose_run { + my $argv = shift; + diag("Running: @{$argv}"); + return run($argv, @_); +} + +sub capture { + my $output; + my $argv = shift; + ok(verbose_run($argv, '>', \$output), "@{$argv}"); + chomp $output; + return $output; +} + +sub check_fake_schroot { + my %params = @_; + my $reference = $params{reference}; + my $extra_argv = $params{extra_argv} || []; + + my $response = capture([qw(unshare -m), + "$srcdir/debian/tests/fake/schroot-1.6.10-3", @{$extra_argv}, + $params{chroot}, + qw(runuser -u nobody --), + qw(script -q -c), 'cat /etc/debian_version', '/dev/null']); + $response =~ s/\r//g; + is($response, $reference, 'script(1) should work under (fake) schroot'); +} + +sub check_fake_pbuilder { + my %params = @_; + my $reference = $params{reference}; + + my $response = capture([qw(unshare -m), + "$srcdir/debian/tests/fake/pbuilder-0.228.4-1", $params{chroot}, + qw(runuser -u nobody --), + qw(script -q -c), 'cat /etc/debian_version', '/dev/null']); + $response =~ s/\r//g; + is($response, $reference, + 'script(1) should work under (fake) pbuilder'); +} + +sub check_chroot { + my %params = @_; + my $chroot = $params{chroot}; + my $response; + + ok(-f "$chroot/etc/debian_version", + 'chroot should have /etc/debian_version'); + ok(-x "$chroot/usr/bin/env", + 'chroot should have /usr/bin/env which is Essential'); + ok(-x "$chroot/usr/bin/hello", 'chroot should have /usr/bin/hello due to --include'); + ok(-d "$chroot/usr/share/doc", 'chroot should have /usr/share/doc'); + + ok(-c "$chroot/dev/full", '/dev/full should be a character device'); + is(capture(['/usr/bin/stat', '--printf=%t %T %a', "$chroot/dev/full"]), + '1 7 666', '/dev/full should be device 1,7 with 0666 permissions'); + ok(-c "$chroot/dev/null"); + is(capture(['/usr/bin/stat', '--printf=%t %T %a', "$chroot/dev/null"]), + '1 3 666', '/dev/null should be device 1,3 with 0666 permissions'); + + my $did_mknod_ptmx; + + if (-l "$chroot/dev/ptmx") { + # Necessary if debootstrap is run inside some containers, see + # https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=817236#77 + diag("/dev/ptmx is a symbolic link"); + like(readlink("$chroot/dev/ptmx"), qr{(?:/dev/)?pts/ptmx}, + 'if /dev/ptmx is a symlink it should be to /dev/pts/ptmx'); + $did_mknod_ptmx = 0; + } + else { + diag("/dev/ptmx is not a symbolic link"); + ok(-c "$chroot/dev/ptmx", + 'if /dev/pts is not a symlink it should be a character device'); + is(capture(['/usr/bin/stat', '--printf=%t %T %a', + "$chroot/dev/ptmx"]), '5 2 666', + 'if /dev/pts is a device node it should be 5,2 with 0666 permissions'); + $did_mknod_ptmx = 1; + } + + if ($params{can_mknod_ptmx}) { + ok($did_mknod_ptmx, 'able to mknod ptmx so should have done so'); + } + + my $reference = capture(['cat', "$chroot/etc/debian_version"]); + + is(capture([qw(chroot chroot.d runuser -u nobody -- + cat /etc/debian_version)]), + $reference); + + # Use unshare -m to make sure the /dev mount gets cleaned up on exit, even + # on failures + check_fake_schroot(%params, reference => $reference); + + # https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=817236 + if (Dpkg::Version->new($params{kernel}) < Dpkg::Version->new('4.7') && + defined $params{container} && $params{container} eq 'lxc') { + TODO: { + local $TODO = "schroot --sbuild doesn't work in lxc on older ". + "kernels"; + check_fake_schroot(%params, reference => $reference, + extra_argv => ['--sbuild']); + } + } + elsif (! $params{can_mknod_ptmx}) { + TODO: { + local $TODO = "schroot --sbuild doesn't work when /dev/ptmx is ". + "a symlink to /dev/pts/ptmx"; + check_fake_schroot(%params, reference => $reference, + extra_argv => ['--sbuild']); + } + } + else { + check_fake_schroot(%params, reference => $reference, + extra_argv => ['--sbuild']); + } + + # https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=817236 + if (! $params{can_mknod_ptmx}) { + TODO: { + local $TODO = "schroot --sbuild doesn't work when /dev/ptmx is ". + "a symlink to /dev/pts/ptmx"; + check_fake_pbuilder(%params, reference => $reference); + } + } + else { + check_fake_pbuilder(%params, reference => $reference); + } +} + +my $mirror = 'http://deb.debian.org/debian'; +my $tmp = $ENV{AUTOPKGTEST_TMP} || $ENV{ADTTMP}; +die "no autopkgtest temporary directory specified" unless $tmp; +chdir $tmp or die "chdir $tmp: $!"; + +$ENV{LC_ALL} = 'C.UTF-8'; + +# Try to inherit a Debian mirror from the host +foreach my $file ('/etc/apt/sources.list', + glob('/etc/apt/sources.list.d/*.list')) { + open(my $fh, '<', $file); + while (<$fh>) { + if (m{^deb\s+(http://[-a-zA-Z0-9.:]+/debian)\s}) { + $mirror = $1; + last; + } + } + close $fh; +} + +if (run(['ischroot'], '>&2')) { + diag("In a chroot according to ischroot(1)"); +} +else { + diag("Not in a chroot according to ischroot(1)"); +} + +my $virtualization; +if ($^O ne 'linux') { + diag("Cannot use systemd-detect-virt on non-Linux"); +} +elsif (run(['systemd-detect-virt', '--vm'], '>', \$virtualization)) { + chomp $virtualization; + diag("Virtualization: $virtualization"); +} +else { + $virtualization = undef; + diag("Virtualization: (not in a virtual machine)"); +} + +my $in_container = 0; +my $container; +if ($^O ne 'linux') { + diag("Cannot use systemd-detect-virt on non-Linux"); +} +elsif (run(['systemd-detect-virt', '--container'], '>', \$container)) { + $in_container = 1; + chomp $container; + diag("Container: $container"); +} +else { + $container = undef; + diag("Container: (not in a container)"); +} + +my $kernel = capture([qw(uname -r)]); +chomp $kernel; + +open(my $fh, '<', '/proc/self/mountinfo'); +while (<$fh>) { + chomp; + diag("mountinfo: $_"); +} +close $fh; + +my $can_mknod_ptmx; +if (run([qw(mknod -m000 ptmx c 5 2)], '&>', '/dev/null')) { + diag("mknod ptmx succeeded"); + $can_mknod_ptmx = 1; +} +else { + diag("mknod ptmx failed, are we in a container?"); + $can_mknod_ptmx = 0; +} + +my $distro_info = DebianDistroInfo->new; +my $testing = $distro_info->testing; + +if (!verbose_run(['debootstrap', + '--include=debootstrap,debian-archive-keyring,gnupg,hello', + '--variant=minbase', + $testing, 'chroot.d', $mirror], '>&2')) { + BAIL_OUT("debootstrap failed: $?"); +} + +check_chroot(chroot => 'chroot.d', can_mknod_ptmx => $can_mknod_ptmx, + kernel => $kernel, container => $container); + +if ($^O ne 'linux') { + diag("Cannot use systemd-nspawn on non-Linux"); +} +elsif ($in_container) { + diag('in a container according to systemd --container, not trying to '. + 'use systemd-nspawn'); +} +elsif (! -d '/run/systemd/system') { + diag('systemd not booted, not trying to use systemd-nspawn'); +} +else { + if (!verbose_run(['systemd-nspawn', '-D', 'chroot.d', + "--bind=$ENV{ADTTMP}:/mnt", + '--bind-ro=/usr/sbin/debootstrap', + '--bind-ro=/usr/share/debootstrap', + '--', + 'debootstrap', '--include=hello', '--variant=minbase', + $testing, '/mnt/from-nspawn.d', $mirror], '>&2')) { + BAIL_OUT("debootstrap wrapped in systemd-nspawn failed: $?"); + } + + check_chroot(chroot => 'from-nspawn.d', can_mknod_ptmx => 0, + kernel => $kernel, container => "nspawn"); +} + +if (!run([qw(rm -fr --one-file-system chroot.d)], '>&2')) { + BAIL_OUT('Unable to remove chroot.d'); +} + +done_testing; + +# vim:set sw=4 sts=4 et: diff --git a/debian/tests/fake/pbuilder-0.228.4-1 b/debian/tests/fake/pbuilder-0.228.4-1 new file mode 100755 index 0000000..308ed34 --- /dev/null +++ b/debian/tests/fake/pbuilder-0.228.4-1 @@ -0,0 +1,31 @@ +#!/bin/sh +# fake/pbuilder-0.228.4-1 -- emulate how pbuilder/0.228.4-1 would chroot. +# It mounts /dev/pts, without explicitly requesting a new instance or a +# usable /dev/pts/ptmx. +# (There is of course a lot more that it does, but these are the parts that +# affect pty users like script(1).) +# +# Copyright © 2017 Simon McVittie +# SPDX-License-Identifier: MIT +# (see debian/copyright) + +set -e + +chroot="$1" +shift +if test -z "$chroot" || test -z "$1"; then + echo "Usage: $0 CHROOT COMMAND...">&2 + exit 2 +fi + +mkdir -p "$chroot/dev/pts" +mount -t devpts none "$chroot/dev/pts" -onoexec,nosuid,gid=5,mode=620 + +ls -l "$chroot/dev/ptmx" | sed -e 's/^/# fake-pbuilder: /' >&2 +ls -l "$chroot/dev/pts/ptmx" | sed -e 's/^/# fake-pbuilder: /' >&2 + +e=0 +chroot "$chroot" "$@" || e=$? + +umount "$chroot/dev/pts" +exit "$e" diff --git a/debian/tests/fake/schroot-1.6.10-3 b/debian/tests/fake/schroot-1.6.10-3 new file mode 100755 index 0000000..d6e0bbe --- /dev/null +++ b/debian/tests/fake/schroot-1.6.10-3 @@ -0,0 +1,47 @@ +#!/bin/sh +# fake/schroot-1.6.10-3 -- emulate how schroot/1.6.10-3 would chroot. +# It bind-mounts /dev/pts and maybe /dev from the host system. +# (There is of course a lot more that it does, but these are the parts that +# affect pty users like script(1).) +# +# Copyright © 2017 Simon McVittie +# SPDX-License-Identifier: MIT +# (see debian/copyright) + +set -e + +# /etc/schroot/default/fstab +bind_dev=yes + +while true; do + case "$1" in + (--sbuild) + shift + # /etc/schroot/sbuild/fstab + bind_dev=no + ;; + (*) + break + esac +done + +chroot="$1" +shift +if test -z "$chroot" || test -z "$1"; then + echo "Usage: $0 CHROOT COMMAND...">&2 + exit 2 +fi + +[ "$bind_dev" = no ] || mount --bind /dev "$chroot/dev" +mount --bind /dev/pts "$chroot/dev/pts" + +ls -l "$chroot/dev/ptmx" | sed -e 's/^/# fake-schroot: /' >&2 +ls -l "$chroot/dev/pts/ptmx" | sed -e 's/^/# fake-schroot: /' >&2 + +e=0 +chroot "$chroot" "$@" || e=$? + +umount "$chroot/dev/pts" +[ "$bind_dev" = no ] || umount "$chroot/dev" + +exit "$e" -- 2.11.0