From 2888deeb8f1048d3240ef6a563c1da2c5e85c6e3 Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Mon, 25 Mar 2019 08:50:38 -0600 Subject: [PATCH] pr2730 - write unprivileged check as standalone test Also: - made it actually work - the initial revisions had not been tested, and had multiple typos and errors. - make more readable, by running the tests as a helper scriptlet instead of a hard-to-read oneliner. - make it a better test, by actually running containers that create world-writable files. Signed-off-by: Ed Santiago --- test/system/030-run.bats | 10 --- test/system/060-mount.bats | 2 - test/system/400-unprivileged-access.bats | 94 ++++++++++++++++++++++++ test/system/helpers.bash | 11 --- 4 files changed, 94 insertions(+), 23 deletions(-) create mode 100644 test/system/400-unprivileged-access.bats diff --git a/test/system/030-run.bats b/test/system/030-run.bats index ad5f59702a..8ae68f33dd 100644 --- a/test/system/030-run.bats +++ b/test/system/030-run.bats @@ -29,16 +29,6 @@ echo $rand | 0 | $rand run_podman $expected_rc run $IMAGE "$@" is "$output" "$expected_output" "podman run $cmd - output" done < <(parse_table "$tests") - - check_no_unprivileged_access -} - -@test "podman run - basic tests --uidmapping" { - run_podman --uidmapping 0:10000:10000 run $IMAGE true - - run_podman --uidmapping 0:10000:10000 -v foo:/foo run $IMAGE true - - check_no_unprivileged_access } # vim: filetype=sh diff --git a/test/system/060-mount.bats b/test/system/060-mount.bats index 3e93029a90..e249b28839 100644 --- a/test/system/060-mount.bats +++ b/test/system/060-mount.bats @@ -27,8 +27,6 @@ load helpers reported_mountpoint=$(echo "$output" | awk '{print $2}') is $reported_mountpoint $mount_path "mountpoint reported by 'podman mount'" - check_no_unprivileged_access - # umount, and make sure files are gone run_podman umount $c_name if [ -e "$mount_path/$f_path" ]; then diff --git a/test/system/400-unprivileged-access.bats b/test/system/400-unprivileged-access.bats new file mode 100644 index 0000000000..b92f81f254 --- /dev/null +++ b/test/system/400-unprivileged-access.bats @@ -0,0 +1,94 @@ +#!/usr/bin/env bats -*- bats -*- +# +# Tests #2730 - regular users are not able to read/write container storage +# + +load helpers + +@test "podman container storage is not accessible by unprivileged users" { + skip_if_rootless "test meaningless without suid" + + run_podman run --name c_uidmap --uidmap 0:10000:10000 $IMAGE true + run_podman run --name c_uidmap_v --uidmap 0:10000:10000 -v foo:/foo $IMAGE true + + run_podman run --name c_mount $IMAGE \ + sh -c "echo hi > /myfile;mkdir -p /mydir/mysubdir; chmod 777 /myfile /mydir /mydir/mysubdir" + + run_podman mount c_mount + mount_path=$output + + # Do all the work from within a test script. Since we'll be invoking it + # as a user, the parent directory must be world-readable. + test_script=$PODMAN_TMPDIR/fail-if-writable + cat >$test_script <<"EOF" +#!/bin/sh + +path="$1" + +die() { + echo "#/vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv" >&2 + echo "#| FAIL: $*" >&2 + echo "#\\^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^" >&2 + + exit 1 +} + +parent=$(dirname "$path") +if chmod +w $parent; then + die "Able to chmod $parent" +fi +if chmod +w "$path"; then + die "Able to chmod $path" +fi + +if [ -d "$path" ]; then + if ls "$path" >/dev/null; then + die "Able to run 'ls $path' without error" + fi + if echo hi >"$path"/test; then + die "Able to write to file under $path" + fi +else + # Plain file + if cat "$path" >/dev/null; then + die "Able to read $path" + fi + if echo hi >"$path"; then + die "Able to write to $path" + fi +fi + +exit 0 +EOF + chmod 755 $PODMAN_TMPDIR $test_script + + # get podman image and container storage directories + run_podman info --format '{{.store.GraphRoot}}' + GRAPH_ROOT="$output" + run_podman info --format '{{.store.RunRoot}}' + RUN_ROOT="$output" + + # The main test: find all world-writable files or directories underneath + # container storage, run the test script as a nonroot user, and try to + # access each path. + find $GRAPH_ROOT $RUN_ROOT \! -type l -perm -o+w -print | while read i; do + dprint " o+w: $i" + + # use chroot because su fails if uid/gid don't exist or have no shell + # For development: test all this by removing the "--userspec x:x" + chroot --userspec 1000:1000 / $test_script "$i" + done + + # Done. Clean up. + rm -f $test_script + + run_podman umount c_mount + run_podman rm c_mount + + # FIXME: without pr2730, this consistently fails with: + # Error: error removing userns root "/run/libpod/containers-root/b654794be6262c8d7f8f4e32b1afbf5734818e45abd6f331311b7e4accf01643": remove /run/libpod/containers-root/b654794be6262c8d7f8f4e32b1afbf5734818e45abd6f331311b7e4accf01643/shm: device or resource busy + # FIXME: with this pr, it works. Is that a deliberate fix in 2730? + run_podman rm c_uidmap c_uidmap_v +} + +# vim: filetype=sh diff --git a/test/system/helpers.bash b/test/system/helpers.bash index aeb0daad9a..431228498d 100644 --- a/test/system/helpers.bash +++ b/test/system/helpers.bash @@ -55,17 +55,6 @@ function basic_setup() { PODMAN_TMPDIR=$(mktemp -d --tmpdir=${BATS_TMPDIR:-/tmp} podman_bats.XXXXXX) } -# Check that an unprivileged user cannot read or write to the storage -function check_no_unprivileged_access() { - GRAPH_ROOT=$(run_podman info --format '{{.store.GraphRoot}}') - RUN_ROOT=$(run_podman info --format '{{.store.RunRoot}}') - find $GRAPH_ROOT $RUN_ROOT -perm -o+w | while read i; do - if chroot --userspec 1000:1000 / bash -c 'chmod +w $(dirname "$i"); chmod +w "$i"; cat "$i" || echo hello > "$i" || echo hello > "$i"/test || rm "$i"' 2> /dev/null; then - die unprivileged user can access "$i" - fi -done -} - # Basic teardown: remove all pods and containers function basic_teardown() { echo "# [teardown]" >&2