From d0be0d7ba6e9fa1d805b0f6a5d9314d4be871d45 Mon Sep 17 00:00:00 2001 From: imlk Date: Sat, 17 Jul 2021 19:01:33 +0800 Subject: [PATCH 01/17] fix: set exit code if proot-rs encounters an error --- src/kernel/groups.rs | 6 ++++++ src/main.rs | 1 + 2 files changed, 7 insertions(+) diff --git a/src/kernel/groups.rs b/src/kernel/groups.rs index 3d0a710..ec3664b 100644 --- a/src/kernel/groups.rs +++ b/src/kernel/groups.rs @@ -38,6 +38,12 @@ pub enum SyscallGroup { UnlinkMkdirAt, } +// TODO: We also need to consider the unshare() system call. For example, +// the `CLONE_FS` flag may cause errors in our simulation of tracee's `cwd` +// field. + +// TODO: modify the result of getdents64() so that we can handle binded entries. + #[cfg(all(target_os = "linux", target_arch = "x86_64"))] pub fn syscall_group_from_sysnum(sysnum: usize) -> SyscallGroup { match sysnum { diff --git a/src/main.rs b/src/main.rs index 21082c1..47d3077 100644 --- a/src/main.rs +++ b/src/main.rs @@ -55,5 +55,6 @@ fn main() { env_logger::init(); if let Err(err) = run() { error!("Exited with error: {}", err); + std::process::exit(1); } } From e7336dd9bdb94ae215c423f60c52ebcdcff242a6 Mon Sep 17 00:00:00 2001 From: imlk Date: Sat, 17 Jul 2021 19:00:39 +0800 Subject: [PATCH 02/17] test: add integration testing --- tests/applets.bats | 332 +++++++++++++++++++++++ tests/bind.bats | 57 ++++ tests/cli.bats | 32 +++ tests/cwd.bats | 13 + tests/event.bats | 9 + tests/execve/test.bats | 23 ++ tests/execve/test_execve.c | 3 + tests/helper.bash | 43 +++ tests/multi-tracee/clone_with_clone_fs.c | 77 ++++++ tests/multi-tracee/test.bats | 30 ++ 10 files changed, 619 insertions(+) create mode 100644 tests/applets.bats create mode 100644 tests/bind.bats create mode 100644 tests/cli.bats create mode 100644 tests/cwd.bats create mode 100644 tests/event.bats create mode 100644 tests/execve/test.bats create mode 100644 tests/execve/test_execve.c create mode 100644 tests/helper.bash create mode 100644 tests/multi-tracee/clone_with_clone_fs.c create mode 100644 tests/multi-tracee/test.bats diff --git a/tests/applets.bats b/tests/applets.bats new file mode 100644 index 0000000..212ddf0 --- /dev/null +++ b/tests/applets.bats @@ -0,0 +1,332 @@ +#!/usr/bin/env bats + +load helper + + +function script_test_run_sh { + echo $(/bin/ls -la | /bin/wc -l) +} + + +@test "test proot-rs run /bin/sh" { + proot-rs --rootfs "$ROOTFS" -- /bin/sh -e -c "$(declare -f script_test_run_sh); script_test_run_sh" +} + + +function script_test_run_applets_file_ops { + PATH=/bin + + # pwd + [ "$(/bin/pwd -L)" = "/" ] + cd /tmp/test_applets_file_ops/ + [ "$(/bin/pwd -L)" = "/tmp/test_applets_file_ops" ] + + # ls + [ "$(ls)" = $'dir1\nfile1' ] + + # touch + /bin/touch file2 + [ "$(ls)" = $'dir1\nfile1\nfile2' ] + + # cp + /bin/echo "test" > file2 + cp file2 file3 + [ "$(ls)" = $'dir1\nfile1\nfile2\nfile3' ] + + # mv + mv file3 dir1/file4 + [ "$(ls)" = $'dir1\nfile1\nfile2' ] + + # find + [ "$(find .)" = $'.\n./dir1\n./dir1/file4\n./file1\n./file2' ] + + # cat + [ "$(cat file2)" = "test" ] + [ "$(cat dir1/file4)" = "test" ] + + # rm + rm dir1/file4 + [ ! -f dir1/file4 ] + + # ln + ln -s non_exist_file link1 + [ "$(readlink link1)" = "non_exist_file" ] + + # mkdir + [[ "$(mkdir dir2/dir3/dir4 2>&1)" == *"No such file or directory"* ]] + mkdir -p dir2/dir3/dir4 + [[ "$(find .)" == *"./dir2/dir3/dir4"* ]] + + # rmdir + [[ "$(rmdir dir2 2>&1)" == *"Directory not empty"* ]] + rmdir dir2/dir3/dir4 + rmdir dir2/dir3 + rmdir dir2 + + # mktemp + local tmp_file="$(mktemp)" + [[ "$(stat ${tmp_file})" == *"regular empty file"* ]] + rm "${tmp_file}" + + # stat + [[ "$(stat dir1)" == *"directory"* ]] + [[ "$(stat file1)" == *"regular file"* ]] + + # chattr + local output="$(chattr +i file2 2>&1)" + local status="$?" + [[ "$output" == *"Inappropriate ioctl for device"* ]] || [[ "$output" == *"Operation not permitted"* ]] || ( [ "$output" = "" ] && [ "$status" -eq 0 ] && chattr -i file2 ) + + # mknod + mknod test_mknod p + [[ "$(stat test_mknod)" == *"fifo"* ]] + + # chmod + touch file2 + chmod 700 file2 + [[ "$(stat file2)" == *"-rwx------"* ]] + + # chown + # Only users with the `CAP_CHOWN` capability can change the owner of a file. + local output="$(chown 65535 file2 2>&1)" + [ "$?" -eq 0 ] || [[ "$output" = *"Operation not permitted"* ]] + +} + + +@test "test proot-rs run applets file operations" { + local test_dir="$ROOTFS/tmp/test_applets_file_ops" + mkdir -p "$test_dir" + mkdir -p "$test_dir/dir1" + echo "this is file1" > "$test_dir/file1" + + runp proot-rs --rootfs "$ROOTFS" -- /bin/sh -e -x -c "$(declare -f script_test_run_applets_file_ops); script_test_run_applets_file_ops" + rm -rf "$test_dir" + [ "$status" -eq 0 ] +} + + +function script_test_run_applets_common_tools { + PATH=/bin + + # clear + clear + + # reset + reset + + # false + ! /bin/false + + # true + /bin/true + + # yes + # head + [ "$(yes 2>&- | head -n 3)" = $'y\ny\ny' ] + + # tail + [ "$(/bin/echo -e '1\n2\n3' | tail -n 1)" = "3" ] + + # echo + # wc + [ "$(/bin/echo -e '1\n2\n3' | wc -l)" == 3 ] + + # tee + [ "$(echo 'test tee' | tee file1)" = "test tee" ] + [ "$(cat file1)" = "test tee" ] + + # du + [ "$(du -h file1)" = "4.0K file1" ] + + # base64 + [ "$(echo 'base64' | base64)" = "YmFzZTY0Cg==" ] + + # md5sum + [ "$(echo 'md5sum' | md5sum)" = "e65b0dce58cbecf21e7c9ff7318f3b57 -" ] + + # sha256sum + [ "$(echo 'sha256sum' | sha256sum)" = "5e913e218e1f3fcac8487d7fbb954bd9669f72a7ef6e9d9f519d94b6a8cc88b9 -" ] + + # sha512sum + [ "$(echo 'sha512sum' | sha512sum)" = "0749dddfaecad1445f66f118738cdb8917dd6adc7f7589c9d30e1d243c541f3b22cf5a77c5bbf6a70a4d078170427439b6e236249815e1281da238eefb5ec1d7 -" ] + + # sed + [ "$(echo 'hello world' | sed 's/world/sed/g')" = "hello sed" ] + + # sort + # uniq + [ "$(echo -e '1\n6\n7\n8\n3\n5\n6\n3\n2\n1\n3\n3\n3\n2\n1' | sort | uniq)" = "$(echo -e '1\n2\n3\n5\n6\n7\n8')" ] + + # grep + [ "$(echo -e 'hello\nworld' | grep 'world')" = "world" ] + + # awk + [ "$(echo 'hello' | awk '{print $1 " world" }')" = "hello world" ] + + # bc + [ "$(echo "10^2" | bc)" = "100" ] + + # strings + /bin/strings /bin/sh | grep "/bin/sh" + + # split + echo "0123456789" | split -b 4 - test_split. + [ "$(ls test_split.*)" = $'test_split.aa\ntest_split.ab\ntest_split.ac' ] + + # date + [ "$(date -d @0)" = "Thu Jan 1 00:00:00 UTC 1970" ] + + # less + [ "$(less file1)" = "test tee" ] + + # tr + [ "$(echo "hello WORLD" | tr "[:upper:]" "[:lower:]")" = "hello world" ] + + # args + [ "$(echo "echo 'hello world'" | xargs)" = "hello world" ] + + # which + [ "$(which sh)" = "/bin/sh" ] + + # which + [ "$(printf '%s world' hello)" = "hello world" ] + [ "$(printf '%x\n' 3735928559)" = "deadbeef" ] + + # sleep + sleep 0.001 + + # ar + ar -r archive.a file1 + [ "$(ar -t archive.a)" = "file1" ] + rm archive.a + + # bzip2 + # bunzip2 + bzip2 file1 + bunzip2 file1.bz2 + [ -f file1 ] + + # gzip + # gunzip + gzip file1 + gunzip file1.gz + [ -f file1 ] + + # tar + tar cf archive.tar file1 + [ "$(tar tf archive.tar)" = "file1" ] + tar xf archive.tar + [ -f file1 ] + + # unzip + echo "UEsDBC0AAAAAAB1G8VItOwiv//////////8BABQALQEAEAAMAAAAAAAAAAwAAAAAAAAAaGVsbG8gd29ybGQKUEsBAh4DLQAAAAAAHUbxUi07CK8MAAAADAAAAAEAAAAAAAAAAQAAAIARAAAAAC1QSwYGLAAAAAAAAAAeAy0AAAAAAAAAAAABAAAAAAAAAAEAAAAAAAAALwAAAAAAAAA/AAAAAAAAAFBLBgcAAAAAbgAAAAAAAAABAAAAUEsFBgAAAAABAAEALwAAAD8AAAAAAA==" | base64 -d > archive.zip + [ "$(unzip -p archive.zip)" = "hello world" ] + + # dd + dd if=file1 of=file2 + [ -f file1 ] + [ -f file2 ] + diff file1 file2 + + # sh + [ "$(sh -c "echo hello world")" = "hello world" ] + + # ash + [ "$(ash -c "echo hello world")" = "hello world" ] + +} + + +@test "test proot-rs run applets common tools" { + local test_dir="$ROOTFS/tmp/test_applets_common_tools" + mkdir "$test_dir" + runp proot-rs --rootfs "$ROOTFS" --cwd "/tmp/test_applets_common_tools" -- /bin/sh -e -x -c "$(declare -f script_test_run_applets_common_tools); script_test_run_applets_common_tools" + rm -rf "$test_dir" + [ "$status" -eq 0 ] +} + + +@test "test proot-rs run uname" { + local output_on_host="$(uname)" + runp proot-rs --rootfs "$ROOTFS" -- /bin/uname + [ "$status" -eq 0 ] + local output_on_guest="$output" + + [ "$(output_on_host)" = "$(output_on_guest)" ] +} + + +@test "test proot-rs run whoami" { + local output_on_host="$(whoami)" + runp proot-rs -- /bin/whoami + [ "$status" -eq 0 ] + local output_on_guest="$output" + + [ "$(output_on_host)" = "$(output_on_guest)" ] +} + + +@test "test proot-rs run man " { + local output_on_host="$(man man)" + runp proot-rs -- /bin/man man + [ "$status" -eq 0 ] + local output_on_guest="$output" + + [ "$(output_on_host)" = "$(output_on_guest)" ] +} + + +@test "test proot-rs run ps" { + if [ ! -d "/proc" ]; then + skip "/proc not found in host" + fi + runp proot-rs -- /bin/ps -o pid,ppid,user + [ "$status" -eq 0 ] + echo "${lines[0]}" | grep 'PID\s*PPID\s*USER' +} + +@test "test proot-rs run kill" { + + runp proot-rs --rootfs "$ROOTFS" -- /bin/sh -e -x -c ' + PATH=/bin + sleep 10 & + pid="$!" + # kill the background process + kill "$pid" + # check if the process is really dead + ! kill -0 "$pid" + ' + [ "$status" -eq 0 ] +} + + +@test "test proot-rs run pkill" { + if [ ! -d "/proc" ]; then + skip "/proc not found in host" + fi + + runp proot-rs -- /bin/sh -e -x -c ' + PATH=/bin + sleep 10 & + pid="$!" + # kill the background process + pkill sleep + # check if the process is really dead + ! kill -0 "$pid" + ' + [ "$status" -eq 0 ] +} + + + +@test "test proot-rs run ping" { + # ping localhost once + proot-rs -- /bin/ping -c 1 127.0.0.1 +} + + +@test "test proot-rs run wget" { + proot-rs --rootfs "$ROOTFS" -- /bin/wget http://1.1.1.1 -O - +} + diff --git a/tests/bind.bats b/tests/bind.bats new file mode 100644 index 0000000..45f2a9d --- /dev/null +++ b/tests/bind.bats @@ -0,0 +1,57 @@ +#!/usr/bin/env bats + +load helper + + +@test "test bind dir to dir" { + # bind /etc to /home + proot-rs --bind "/etc:/home" -- /bin/sh -c "/bin/diff /etc /home" +} + + +@test "test bind file to file" { + # bind /etc/group to /etc/passwd + proot-rs --bind "/etc/group:/etc/passwd" -- /bin/sh -c "/bin/diff /etc/group /etc/passwd" +} + + +@test "test bind dir to file" { + # bind /bin to /etc/passwd + # this may seem odd, but it is allowed + proot-rs --bind "/bin:/etc/passwd" -- /bin/sh -c "/bin/diff /bin /etc/passwd" +} + + +@test "test bind file to dir" { + # bind /etc/passwd to /bin + # this may seem odd, but it is allowed + proot-rs --bind "/etc/passwd:/bin" -- /bin/sh -c "/bin/diff /etc/passwd /bin" +} + + +# Will be removed after the implementation of bind glue +@test "test bind target must exist" { + runp proot-rs --bind "/etc/passwd:/etc/non_exist_path" -- /bin/sh -c "/bin/true" + [ $status -ne 0 ] +} + + +@test "test --bind with getdents64() results" { + mkdir "$ROOTFS/tmp/test_bind_with_getdents64" + echo "first" > "$ROOTFS/tmp/test_bind_with_getdents64/file1" + echo "second" > "$ROOTFS/tmp/test_bind_with_getdents64/file2" + chmod 644 "$ROOTFS/tmp/test_bind_with_getdents64/file1" + chmod 777 "$ROOTFS/tmp/test_bind_with_getdents64/file2" + # bind "$ROOTFS/tmp/test_bind_with_getdents64/file1" to "/tmp/test_bind_with_getdents64/file2" + runp proot-rs --rootfs "$ROOTFS" --bind "$ROOTFS/tmp/test_bind_with_getdents64/file1:/tmp/test_bind_with_getdents64/file2" -- /bin/sh -e -c ' \ + PATH=/bin + # Get the output of ls -l and filter out the lines related to file1 and file2 + output=$(ls -l /tmp/test_bind_with_getdents64 | grep "file" | sed "s/file.*//g") \ + # The $output should contain two lines + [ "$(echo $output | wc -l)" -eq 2 ] + # And their attributes should be the same. + [ "$(echo $output | sort | uniq | wc -l)" -eq 1 ] + ' + rm -rf "$ROOTFS/tmp/test_bind_with_getdents64" + [ "$status" -eq 0 ] +} \ No newline at end of file diff --git a/tests/cli.bats b/tests/cli.bats new file mode 100644 index 0000000..62f8e2c --- /dev/null +++ b/tests/cli.bats @@ -0,0 +1,32 @@ +#!/usr/bin/env bats + +load helper + + +@test "test proot-rs --version" { + proot-rs --version +} + +@test "test proot-rs with default options" { + proot-rs /bin/true +} + +@test "test proot-rs options --cwd" { + proot-rs --cwd /bin -- ./true + proot-rs -w /bin -- ./true +} + +@test "test proot-rs options --rootfs" { + proot-rs --rootfs "$ROOTFS" -- /bin/true + proot-rs -r "$ROOTFS" -- /bin/true +} + +@test "test proot-rs options --cwd and --rootfs" { + proot-rs --rootfs "$ROOTFS" --cwd /bin -- ./true +} + +@test "test proot-rs options --bind" { + proot-rs --bind "/etc:/home" -- /bin/stat /home/passwd + proot-rs -b "/etc:/home" -- /bin/stat /home/passwd +} + diff --git a/tests/cwd.bats b/tests/cwd.bats new file mode 100644 index 0000000..705ea7d --- /dev/null +++ b/tests/cwd.bats @@ -0,0 +1,13 @@ +#!/usr/bin/env bats + +load helper + + +@test "test get and set cwd" { + run proot-rs --rootfs "$ROOTFS" --cwd /bin -- /bin/sh -c "pwd -P; cd /; pwd -P; cd /etc/; pwd -P" + [ "$status" -eq 0 ] + [ "${lines[0]}" = "/bin" ] + [ "${lines[1]}" = "/" ] + [ "${lines[2]}" = "/etc" ] + [ "${#lines[@]}" -eq 3 ] +} diff --git a/tests/event.bats b/tests/event.bats new file mode 100644 index 0000000..26dccca --- /dev/null +++ b/tests/event.bats @@ -0,0 +1,9 @@ +#!/usr/bin/env bats + +load helper + + +@test "test terminate tracees after proot-rs exits" { + runp proot-rs --rootfs "$ROOTFS" -- /bin/sh -c '/bin/kill -11 $PPID; /bin/echo "The tracee is still alive, which is not allowed";' + [[ "$output" != *"still alive"* ]] +} diff --git a/tests/execve/test.bats b/tests/execve/test.bats new file mode 100644 index 0000000..e4a83bf --- /dev/null +++ b/tests/execve/test.bats @@ -0,0 +1,23 @@ +#!/usr/bin/env bats + +load ../helper + + +@test "test execute dynamically linked binary" { + # compile test case + compile_c_dynamic "$ROOTFS/bin/test_execve_dynamic" "$BATS_TEST_DIRNAME/test_execve.c" + runp proot-rs --rootfs "$ROOTFS" --cwd / -- /bin/test_execve_dynamic + # remember to delete the binary file + rm "$ROOTFS/bin/test_execve_dynamic" + [ "$status" -eq 0 ] +} + + +@test "test execute statically linked binary" { + # compile test case + compile_c_static "$ROOTFS/bin/test_execve_static" "$BATS_TEST_DIRNAME/test_execve.c" + runp proot-rs --rootfs "$ROOTFS" --cwd / -- /bin/test_execve_static + # remember to delete the binary file + rm "$ROOTFS/bin/test_execve_static" + [ "$status" -eq 0 ] +} \ No newline at end of file diff --git a/tests/execve/test_execve.c b/tests/execve/test_execve.c new file mode 100644 index 0000000..7043a46 --- /dev/null +++ b/tests/execve/test_execve.c @@ -0,0 +1,3 @@ +int main(int argc, char const *argv[]) { + return 0; +} \ No newline at end of file diff --git a/tests/helper.bash b/tests/helper.bash new file mode 100644 index 0000000..163cbee --- /dev/null +++ b/tests/helper.bash @@ -0,0 +1,43 @@ +#!/bin/bash + +# The root directory where the integration test files are placed +TEST_ROOT=$(dirname "$(readlink -f "$BASH_SOURCE")") + +# The root directory of this project +PROJECT_ROOT="$TEST_ROOT/../" + +# Path to the proot-rs binary +PROOT_RS="$PROJECT_ROOT/target/debug/proot-rs" + +# Set the default path to the new rootfs, which is created in advance by `scripts/mkrootfs.sh` +# Note that if `PROOT_TEST_ROOTFS` is set, then the value of `ROOTFS` will the same as it; otherwise, the default value of `ROOTFS` is `$PROJECT_ROOT/rootfs` +[[ -z "${PROOT_TEST_ROOTFS}" ]] && ROOTFS="$PROJECT_ROOT/rootfs" || ROOTFS="${PROOT_TEST_ROOTFS}" + +# A wrapper for bats' built-in `run` command. +# This function will first execute the original `run` command, and then print the `$status` and `$output` to the `stderr`. +# One advantage over `run` is that the results of the command will be displayed when the test fails, making it easier for developers to debug. +function runp() { + run "$@" + echo "command: $@" >&2 + echo "status: $status" >&2 + echo "output: $output" >&2 +} + +# A wrapper function for proot-rs binary. +function proot-rs() { + "$PROOT_RS" "$@" +} + +# Compile a single C source code file ($2) to statically linked binary ($1) +function compile_c_static() { + local target_path=$1 + local source_path=$2 + gcc -static -o "$target_path" "$source_path" +} + +# Same as `compile_c_static()`, but the final binary is dynamically linked +function compile_c_dynamic() { + local target_path=$1 + local source_path=$2 + gcc -o "$target_path" "$source_path" +} diff --git a/tests/multi-tracee/clone_with_clone_fs.c b/tests/multi-tracee/clone_with_clone_fs.c new file mode 100644 index 0000000..c13ce46 --- /dev/null +++ b/tests/multi-tracee/clone_with_clone_fs.c @@ -0,0 +1,77 @@ +/** + * This code is used to test whether the `CLONE_FS` flag in the clone() system + * call is handled correctly by proot-rs. + * + * The program first spawns a child process with `clone(CLONE_FS)`, then calls + * `chdir()` in the child process, and then calls `getcwd()` in both the child + * and the parent process. + * + * The expected result is that the `cwd` of the child process and the `cwd` of + * the parent process are always the same. + */ + +#define _GNU_SOURCE + +#include +#include +#include +#include +#include +#include + +// Stack size for cloned child +#define STACK_SIZE (1024 * 1024) + +static void exit_with_error(char *msg) { + fprintf(stderr, "%s\n", msg); + exit(1); +} + +// Start function for cloned child +static int child_func() { + char buf[1024]; + // Get the cwd value before modification, and print it. + getcwd(buf, sizeof(buf)); + puts(buf); + + // Change cwd + chdir("/etc"); + + // Query the cwd value after modification, and print it. + getcwd(buf, sizeof(buf)); + puts(buf); + + // We need to flush stdout manually otherwise in some cases (e.g. output to + // pipe) the content will be lost. + fflush(stdout); + return 0; +} + +// Start function for parent process +static int parent_func() { + char buf[1024]; + // Query the value of cwd from the parent process and print it out + getcwd(buf, sizeof(buf)); + puts(buf); + return 0; +} + +int main(int argc, char const *argv[]) { + // Allocate stack area for child + char *stack = mmap(NULL, STACK_SIZE, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0); + if (stack == MAP_FAILED) + exit_with_error("Error while mmap()"); + char *stack_top = stack + STACK_SIZE; + + // Create a child to execute some code. + pid_t pid = clone(child_func, stack_top, CLONE_FS | SIGCHLD, NULL); + if (pid == -1) + exit_with_error("Error while clone()"); + + // Wait for child to return from it's function + if (waitpid(pid, NULL, 0) == -1) + exit_with_error("Error while waitpid()"); + + return parent_func(); +} diff --git a/tests/multi-tracee/test.bats b/tests/multi-tracee/test.bats new file mode 100644 index 0000000..4667f89 --- /dev/null +++ b/tests/multi-tracee/test.bats @@ -0,0 +1,30 @@ +#!/usr/bin/env bats + +load ../helper + + +@test "test clone with CLONE_FS set" { + # compile test case + compile_c_dynamic "$ROOTFS/bin/clone_with_clone_fs" "$BATS_TEST_DIRNAME/clone_with_clone_fs.c" + runp proot-rs --rootfs "$ROOTFS" --cwd / -- /bin/clone_with_clone_fs + # remember to delete the binary file + rm "$ROOTFS/bin/clone_with_clone_fs" + [ "$status" -eq 0 ] + [ "${lines[0]}" = "/" ] + # both the cwd of the child process and the cwd of the parent process are changed to /etc + [ "${lines[1]}" = "/etc" ] + [ "${lines[2]}" = "/etc" ] + [ "${#lines[@]}" -eq 3 ] +} + +@test "test clone without CLONE_FS set" { + # by default, child process spawned in shell does not contains `CLONE_FS` + runp proot-rs --rootfs "$ROOTFS" --cwd / -- /bin/sh -c "/bin/pwd -P; \ + /bin/sh -c 'cd /etc; /bin/pwd -P;'; \ + /bin/pwd -P;" + [ "$status" -eq 0 ] + [ "${lines[0]}" = "/" ] + [ "${lines[1]}" = "/etc" ] + [ "${lines[2]}" = "/" ] + [ "${#lines[@]}" -eq 3 ] +} From 7c0b6944d5dfb10d5906c7d656e7e976452a8d81 Mon Sep 17 00:00:00 2001 From: imlk Date: Sun, 18 Jul 2021 00:36:01 +0800 Subject: [PATCH 03/17] ci: add integration testing to workflow ci: fix workflow file syntax problem --- .github/workflows/{rust.yml => tests.yml} | 27 +++++++++++++++++------ 1 file changed, 20 insertions(+), 7 deletions(-) rename .github/workflows/{rust.yml => tests.yml} (82%) diff --git a/.github/workflows/rust.yml b/.github/workflows/tests.yml similarity index 82% rename from .github/workflows/rust.yml rename to .github/workflows/tests.yml index 8ad21d9..6ba32a1 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/tests.yml @@ -1,4 +1,4 @@ -name: Rust +name: Tests on: [push, pull_request] @@ -6,11 +6,9 @@ env: CARGO_TERM_COLOR: always jobs: - build: - + lints: runs-on: ubuntu-latest timeout-minutes: 10 - steps: - uses: actions/checkout@v2 with: @@ -57,12 +55,27 @@ jobs: # Run clippy check cargo clippy + + tests: + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v2 - name: Build run: cargo build --verbose - - name: Run tests + - name: Setup rootfs shell: bash run: | - # Setup rootfs bash scripts/mkrootfs.sh - # Run cargo test + - name: Run unit tests + shell: bash + run: | PROOT_TEST_ROOTFS=./rootfs cargo test --verbose -- --nocapture + - name: Setup bats-core + uses: mig4/setup-bats@v1 + with: + bats-version: 1.3.0 + - name: Run integration tests + shell: bash + run: | + PROOT_TEST_ROOTFS=./rootfs bats -r tests From a6e6a820eed6c4de4a4f70a7ab2f8501a6befaf1 Mon Sep 17 00:00:00 2001 From: imlk Date: Sun, 18 Jul 2021 00:36:45 +0800 Subject: [PATCH 04/17] docs: add documentation of integration testing docs: fix badge link in README.md --- README.md | 12 +++++++++- tests/README.md | 63 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 tests/README.md diff --git a/README.md b/README.md index 0bdc5b9..8276dec 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # proot-rs -[![](https://github.com/proot-me/proot-rs/workflows/Rust/badge.svg)](https://github.com/proot-me/proot-rs/actions) +[![Tests](https://github.com/proot-me/proot-rs/actions/workflows/tests.yml/badge.svg)](https://github.com/proot-me/proot-rs/actions/workflows/tests.yml) _Rust implementation of PRoot, a ptrace-based sandbox._ @@ -65,6 +65,8 @@ cargo build --release ## Tests +### Setup new rootfs for testing + Typically, we need to specify a new rootfs path for testing proot-rs. This script provided below can be used to create one: @@ -86,12 +88,20 @@ If you want to use the same rootfs as the host, just set it to `/`: export PROOT_TEST_ROOTFS=/ ``` +### Unit testing + +> Note: When running unit tests, it is required that `PROOT_TEST_ROOTFS` must be set + Start running tests: ```shell cargo test ``` +### Integration testing + +For the section on running integration tests, please read the [Integration Testing documentation](./tests/README.md) + ## Contributing We use git hooks to check files staged for commit to ensure the consistency of Rust code style. diff --git a/tests/README.md b/tests/README.md new file mode 100644 index 0000000..abc3d3b --- /dev/null +++ b/tests/README.md @@ -0,0 +1,63 @@ +# Integration Testing + +This document describes the integration testing part of proot-rs. It is intended for developers to understand the current status of integration testing in proot-rs. + + +## Why and how to + +proot-rs currently uses unit testing and integration testing to find potential software faults. + +For unit testing, we use `cargo test` to ensure the correctness of the functions, in a similar way to the normal rust program. However, we still need a way to be able to test the whole program, and that is the purpose of integration testing. + + +We use [Bats](https://github.com/bats-core/bats-core) to run integration tests. Bats is a Bash-based testing framework that allows developers to write simple Bash scripts to test their command-line programs. + +We also considered ShellSpec and shUnit2, but they seem to be more suitable for testing shell scripts rather than command line programs. + + +## Run tests + +### Run manually + +To launch integration testing, you need to [install bats](https://github.com/bats-core/bats-core/blob/master/docs/source/installation.rst) first, and make sure `bats` is in your `PATH`. + +> Note: Before running integration tests, you need to set `PROOT_TEST_ROOTFS` to the path of new rootfs, or the `rootfs` directory in the root of the project will be used as the new rootfs. + + +Before you start, make sure you are in the root directory of this project. + +1. Compile proot-rs + + ```shell + cargo build + ``` + This will generate a executable file at `target/debug/proot-rs`, which will be used in during integration tests. + +2. Run all test scripts under the `tests/` directory + ```shell + bats -r tests + ``` + +### Run in Docker + +TODO + +### Run in CI + +Integration tests are now added to github [workflow](.github/workflows/tests.yml) + + +## Write tests + +All integration test scripts are placed in the `tests/` directory. They all use `.bats` as suffix and are named as the category of the tests. A script file may contains more than one test case. + +`helper.bash` is a Bash script which is included by all test scripts, which provided some helper functions and global variables. + +We usually use proot-rs to run a shell script to test it's correctness, i.e. (`proot-rs -- /bin/sh -x -e -c " #some tests"`). But for some tests that cannot be written in shell script, we can also write them in C. + + +For more information, please read [Bats: Writing tests](https://bats-core.readthedocs.io/en/stable/writing-tests.html) + + + + From d9c2a8d609e4db8755b5f30c0214fb35b114ef71 Mon Sep 17 00:00:00 2001 From: imlk Date: Sun, 18 Jul 2021 11:01:32 +0800 Subject: [PATCH 05/17] refactor: use wget instead of curl in mkrootfs.sh --- scripts/mkrootfs.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/mkrootfs.sh b/scripts/mkrootfs.sh index aac4cd5..e4fcbb9 100755 --- a/scripts/mkrootfs.sh +++ b/scripts/mkrootfs.sh @@ -41,7 +41,7 @@ trap 'rm -f "${rootfs_archive}"' EXIT rootfs_archive="$(mktemp)" || { echo "Failed to create temp file"; exit 1; } -curl -o "${rootfs_archive}" -L -C - "https://github.com/docker-library/busybox/raw/dist-${arch}/stable/glibc/busybox.tar.xz" || { echo "Failed to download busybox archive"; exit 1; } +wget -O "${rootfs_archive}" "https://github.com/docker-library/busybox/raw/dist-${arch}/stable/glibc/busybox.tar.xz" || { echo "Failed to download busybox archive"; exit 1; } tar -C "${PROOT_TEST_ROOTFS}" -xf "${rootfs_archive}" || { echo "Failed to unpack busybox tarball. Maybe the file is broken"; exit 1; } From fc0b5a7a57b10f1a00bb4febb3d5396eaf201f80 Mon Sep 17 00:00:00 2001 From: imlk Date: Sun, 18 Jul 2021 13:33:24 +0800 Subject: [PATCH 06/17] test: add Dockerfile for test --- tests/Dockerfile | 21 +++++++++++++++++++++ tests/README.md | 14 +++++++++++++- 2 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 tests/Dockerfile diff --git a/tests/Dockerfile b/tests/Dockerfile new file mode 100644 index 0000000..eb28ab8 --- /dev/null +++ b/tests/Dockerfile @@ -0,0 +1,21 @@ +FROM rust:buster + +# Install bats +ARG BATS_VERSION=1.3.0 +WORKDIR /opt +RUN wget -O bats-core.tar.gz https://github.com/bats-core/bats-core/archive/refs/tags/v${BATS_VERSION}.tar.gz && \ + tar xf bats-core.tar.gz && \ + rm bats-core.tar.gz && \ + ln -s /opt/bats-core-${BATS_VERSION}/bin/bats /usr/local/bin/bats + + +# Copy source code into image +WORKDIR /code +COPY . /code + +# Build new rootfs into "/rootfs" +RUN bash /code/scripts/mkrootfs.sh -d /rootfs +ENV PROOT_TEST_ROOTFS="/rootfs" + + +CMD ["sh", "-c", "cargo build && bats -r tests;"] \ No newline at end of file diff --git a/tests/README.md b/tests/README.md index abc3d3b..b1e71bd 100644 --- a/tests/README.md +++ b/tests/README.md @@ -40,7 +40,19 @@ Before you start, make sure you are in the root directory of this project. ### Run in Docker -TODO +We provide [Dockerfile](./Dockerfile) for running integration tests in docker. + +First go to the root of the project, and build docker image from Dockerfile + +```shell +docker build -f tests/Dockerfile -t proot-rs-test . +``` + +Then, start a container to run the test + +```shell +docker run --rm -it proot-rs-test +``` ### Run in CI From 46576a4a211b69a68a3e486d631e942c6d5ee03a Mon Sep 17 00:00:00 2001 From: imlk Date: Tue, 20 Jul 2021 00:16:28 +0800 Subject: [PATCH 07/17] fix: fix unable to run statically linked elf file docs: add a optimization todo --- src/build_loader.rs | 36 ++++++++++++++++++++++++++++++++-- src/kernel/execve/enter.rs | 7 +------ src/kernel/execve/load_info.rs | 19 +++++++++++++++++- 3 files changed, 53 insertions(+), 9 deletions(-) diff --git a/src/build_loader.rs b/src/build_loader.rs index 3e64d08..4ec358e 100644 --- a/src/build_loader.rs +++ b/src/build_loader.rs @@ -1,10 +1,42 @@ extern crate gcc; +#[cfg(target_arch = "x86")] +mod config { + pub const LOADER_ADDRESS: u64 = 0xa0000000; + pub const LOADER_ARCH_CFLAGS: &[&'static str] = &["-mregparm=3"]; +} + +#[cfg(target_arch = "x86_64")] +mod arch { + /// The virtual address(p_vaddr) of `.text` section of our custom loader. + pub const LOADER_ADDRESS: u64 = 0x600000000000; + /// Additional flags to be passed when compiling the loader. + pub const LOADER_ARCH_CFLAGS: &[&'static str] = &[]; +} + +#[cfg(target_arch = "arm")] +mod arch { + pub const LOADER_ADDRESS: u64 = 0x10000000; + pub const LOADER_ARCH_CFLAGS: &[&'static str] = &[]; +} + +#[cfg(target_arch = "aarch64")] +mod arch { + pub const LOADER_ADDRESS: u64 = 0x2000000000; + pub const LOADER_ARCH_CFLAGS: &[&'static str] = &[]; +} + fn main() { - gcc::Config::new() + let mut config = gcc::Config::new(); + config .flag("-static") .flag("-nostdlib") - .flag("-ffreestanding") + .flag("-ffreestanding"); + arch::LOADER_ARCH_CFLAGS.iter().for_each(|flag| { + config.flag(flag); + }); + config.flag(&format!("-Wl,-Ttext=0x{:x}", arch::LOADER_ADDRESS)); + config .file("src/kernel/execve/loader/loader.c") .out_dir("src/kernel/execve/loader") .compile_binary("binary_loader_exe"); diff --git a/src/kernel/execve/enter.rs b/src/kernel/execve/enter.rs index f0a5625..bf72581 100644 --- a/src/kernel/execve/enter.rs +++ b/src/kernel/execve/enter.rs @@ -65,12 +65,7 @@ pub fn translate(tracee: &mut Tracee, loader: &dyn LoaderFile) -> Result<()> { load_info.user_path = Some(raw_path); load_info.host_path = Some(host_path); - if load_info.interp.is_none() { - return Err(Error::errno_with_msg( - EINVAL, - "When translating enter execve, interp is none", - )); - } + // An interpreter should not depend on another interpreter if let Some(ref interp) = load_info.interp { if interp.interp.is_some() { return Err(Error::errno_with_msg( diff --git a/src/kernel/execve/load_info.rs b/src/kernel/execve/load_info.rs index 77a798c..0f4b41b 100644 --- a/src/kernel/execve/load_info.rs +++ b/src/kernel/execve/load_info.rs @@ -131,16 +131,27 @@ impl LoadInfo { fd: None, // unknown yet offset: offset & *PAGE_MASK, addr: start_address, + // TODO: This can be optimized. + // The calculation of the `length` may be wrong. The field `length` should not be + // calculated using the paged-aligned `end_address`. It will cause more content in the + // target file to be mapped into the memory area. Due to `clear_length`, the + // over-mapped area is later cleared by `clear_length`. However, according to the man + // page of mmap(2), the remaining bytes of a file mapping will be zeroed automatically. + // So the best way is to correct the calculation of `length` and remove `clear_length` + // field. length: end_address - start_address, flags: MapFlags::MAP_PRIVATE | MapFlags::MAP_FIXED, prot: prot, clear_length: 0, }; + // According to the description in man page elf(5), `p_filesz` may not be larger + // than the `p_memsz`. + // "If the segment's memory size p_memsz is larger than the // file size p_filesz, the "extra" bytes are defined to hold // the value 0 and to follow the segment's initialized area." - // -- man 7 elf. + // -- man 5 elf. if memsz > filesz { // How many extra bytes in the current page? mapping.clear_length = end_address - vaddr - filesz; @@ -287,10 +298,16 @@ pub struct LoadStatementOpen { #[repr(C)] #[derive(Debug)] pub struct LoadStatementMmap { + /// The starting address for the new mapping. pub addr: libc::c_ulong, + /// The length of the mapping. pub length: libc::c_ulong, + /// The desired memory protection of the mapping. pub prot: libc::c_ulong, + /// The offset in the file which the mapping will start at. pub offset: libc::c_ulong, + /// The byte size of the memory area to be zeroed forward at the end of the + /// page in this memory mapping. pub clear_length: libc::c_ulong, } From 10907579916b3137f9911b45cbd448e00c35e1b3 Mon Sep 17 00:00:00 2001 From: imlk Date: Tue, 20 Jul 2021 17:48:42 +0800 Subject: [PATCH 08/17] fix: fix ENOTDIR when binding a file to dir/file --- src/filesystem/binding.rs | 11 ++++++++++- src/filesystem/fs.rs | 2 +- tests/bind.bats | 8 ++++---- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/filesystem/binding.rs b/src/filesystem/binding.rs index 7fb726e..9665aa7 100644 --- a/src/filesystem/binding.rs +++ b/src/filesystem/binding.rs @@ -78,7 +78,16 @@ impl Binding { })?; // and then add what remains of the path when removing the old prefix - new_path.push(stripped_path); + if !stripped_path.is_empty() { + // If the `stripped_path` is empty, we will not call `.push("")`, to avoid + // adding the extra "/" at the end of the path. + // + // Note: As mentioned in the document of `std::path::PathBuf::components()`, "A + // trailing slash is normalized away" in a path. And it means `foo/bar` is the + // same as `foo/bar/` . However, many Linux system call are sensitive to + // trailing slash, and they assume a path with a trailing slash as a directory. + new_path.push(stripped_path); + } if new_path.len() >= PATH_MAX as usize { return Err(Error::errno_with_msg( diff --git a/src/filesystem/fs.rs b/src/filesystem/fs.rs index 2c677a3..cad43c6 100644 --- a/src/filesystem/fs.rs +++ b/src/filesystem/fs.rs @@ -66,7 +66,7 @@ impl FileSystem { // Skip the check for "/" because "/" always exists. if canonical_guest_path != Path::new("/") { self.substitute(&canonical_guest_path, Side::Guest)? - .metadata()?; + .metadata()?; // call .metadata() to check if the path exist } // Add a binding at the beginning of the list, so that we get the most recent diff --git a/tests/bind.bats b/tests/bind.bats index 45f2a9d..3c8a0bf 100644 --- a/tests/bind.bats +++ b/tests/bind.bats @@ -16,16 +16,16 @@ load helper @test "test bind dir to file" { - # bind /bin to /etc/passwd + # bind /home to /etc/passwd # this may seem odd, but it is allowed - proot-rs --bind "/bin:/etc/passwd" -- /bin/sh -c "/bin/diff /bin /etc/passwd" + proot-rs --bind "/home:/etc/passwd" -- /bin/sh -c "/bin/diff /home /etc/passwd" } @test "test bind file to dir" { - # bind /etc/passwd to /bin + # bind /etc/passwd to /home # this may seem odd, but it is allowed - proot-rs --bind "/etc/passwd:/bin" -- /bin/sh -c "/bin/diff /etc/passwd /bin" + proot-rs --bind "/etc/passwd:/home" -- /bin/sh -c "/bin/diff /etc/passwd /home" } From d3f5b77580477a7f751961b0f0301a62d6e4f994 Mon Sep 17 00:00:00 2001 From: Lucas Ramage Date: Wed, 21 Jul 2021 11:12:02 -0400 Subject: [PATCH 09/17] Use original Dockerfile as base for tests Dockerfile Add test image to docker-compose file Update testing documentation to reflect the changes --- Dockerfile | 1 + docker-compose.yml | 15 +++++++++++---- tests/Dockerfile | 20 +++----------------- tests/README.md | 47 ++++++++++++++++++++-------------------------- 4 files changed, 35 insertions(+), 48 deletions(-) diff --git a/Dockerfile b/Dockerfile index 1419d30..19ef584 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,6 +2,7 @@ FROM rust:alpine as build RUN apk update && \ apk add bash \ + bats \ curl \ shellcheck diff --git a/docker-compose.yml b/docker-compose.yml index 54a9c82..6b85fcd 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,10 +1,17 @@ version: "3" services: - proot-rs: - container_name: proot-rs - hostname: proot-rs + proot-rs-sdk: + container_name: proot-rs-sdk + hostname: proot-rs-sdk build: . - image: proot/proot-rs:latest + image: proot/proot-rs-sdk:latest + volumes: + - ~/src/proot-rs/:/usr/src/proot-rs + proot-rs-test: + container_name: proot-rs-test + hostname: proot-rs-tests + build: ./tests + image: proot/proot-rs-test:latest volumes: - ~/src/proot-rs/:/usr/src/proot-rs diff --git a/tests/Dockerfile b/tests/Dockerfile index eb28ab8..219a740 100644 --- a/tests/Dockerfile +++ b/tests/Dockerfile @@ -1,21 +1,7 @@ -FROM rust:buster +FROM proot/proot-rs-sdk:latest -# Install bats -ARG BATS_VERSION=1.3.0 -WORKDIR /opt -RUN wget -O bats-core.tar.gz https://github.com/bats-core/bats-core/archive/refs/tags/v${BATS_VERSION}.tar.gz && \ - tar xf bats-core.tar.gz && \ - rm bats-core.tar.gz && \ - ln -s /opt/bats-core-${BATS_VERSION}/bin/bats /usr/local/bin/bats - - -# Copy source code into image -WORKDIR /code -COPY . /code - -# Build new rootfs into "/rootfs" -RUN bash /code/scripts/mkrootfs.sh -d /rootfs +RUN /bin/bash /usr/src/proot-rs/scripts/mkrootfs.sh -d /rootfs ENV PROOT_TEST_ROOTFS="/rootfs" +CMD ["sh", "-c", "cargo build && bats -r tests;"] -CMD ["sh", "-c", "cargo build && bats -r tests;"] \ No newline at end of file diff --git a/tests/README.md b/tests/README.md index b1e71bd..1e793ae 100644 --- a/tests/README.md +++ b/tests/README.md @@ -2,74 +2,67 @@ This document describes the integration testing part of proot-rs. It is intended for developers to understand the current status of integration testing in proot-rs. - ## Why and how to proot-rs currently uses unit testing and integration testing to find potential software faults. -For unit testing, we use `cargo test` to ensure the correctness of the functions, in a similar way to the normal rust program. However, we still need a way to be able to test the whole program, and that is the purpose of integration testing. - +For unit testing, we use `cargo test` to ensure the correctness of the functions in a similar way to the normal rust program. However, we still need a way to be able to test the whole program, and that is the purpose of integration testing. We use [Bats](https://github.com/bats-core/bats-core) to run integration tests. Bats is a Bash-based testing framework that allows developers to write simple Bash scripts to test their command-line programs. We also considered ShellSpec and shUnit2, but they seem to be more suitable for testing shell scripts rather than command line programs. - ## Run tests ### Run manually -To launch integration testing, you need to [install bats](https://github.com/bats-core/bats-core/blob/master/docs/source/installation.rst) first, and make sure `bats` is in your `PATH`. +To launch integration testing, you need to [install bats](https://github.com/bats-core/bats-core/blob/master/docs/source/installation.rst) first, and make sure that `bats` is in your `PATH`. > Note: Before running integration tests, you need to set `PROOT_TEST_ROOTFS` to the path of new rootfs, or the `rootfs` directory in the root of the project will be used as the new rootfs. - Before you start, make sure you are in the root directory of this project. -1. Compile proot-rs +1. Compile proot-rs: - ```shell - cargo build - ``` - This will generate a executable file at `target/debug/proot-rs`, which will be used in during integration tests. +```shell +cargo build +``` -2. Run all test scripts under the `tests/` directory - ```shell - bats -r tests - ``` +This will generate a executable file at `target/debug/proot-rs`, which will be used in during integration tests. + +2. Run all test scripts under the `tests/` directory: + +```shell +bats -r tests +``` ### Run in Docker -We provide [Dockerfile](./Dockerfile) for running integration tests in docker. +We provide a [Dockerfile](./Dockerfile) for running integration tests in docker. First go to the root of the project, and build docker image from Dockerfile ```shell -docker build -f tests/Dockerfile -t proot-rs-test . +docker build -f tests/Dockerfile -t proot/proot-rs-test:latest . ``` -Then, start a container to run the test +Then, start a container to run the test: ```shell -docker run --rm -it proot-rs-test +docker run --rm -it proot/proot-rs-test:latest ``` ### Run in CI -Integration tests are now added to github [workflow](.github/workflows/tests.yml) - +Integration tests are now added to the GitHub [workflow](.github/workflows/tests.yml) ## Write tests All integration test scripts are placed in the `tests/` directory. They all use `.bats` as suffix and are named as the category of the tests. A script file may contains more than one test case. -`helper.bash` is a Bash script which is included by all test scripts, which provided some helper functions and global variables. - -We usually use proot-rs to run a shell script to test it's correctness, i.e. (`proot-rs -- /bin/sh -x -e -c " #some tests"`). But for some tests that cannot be written in shell script, we can also write them in C. +The file `helper.bash` is a Bash script which is included by all test scripts, and provides some helper functions and global variables. +We usually use `proot-rs` to run a shell script to test for correctness, i.e. (`proot-rs -- /bin/sh -x -e -c " #some tests"`). But for some tests that cannot be written in shell script, we can also write them in C. For more information, please read [Bats: Writing tests](https://bats-core.readthedocs.io/en/stable/writing-tests.html) - - - From a52d81dc017217c8d8c42893c7d66a7c38ff7ea2 Mon Sep 17 00:00:00 2001 From: Lucas Ramage Date: Wed, 21 Jul 2021 11:15:13 -0400 Subject: [PATCH 10/17] Fix hostname for test in docker-compose file --- docker-compose.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker-compose.yml b/docker-compose.yml index 6b85fcd..474e6ad 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -9,7 +9,7 @@ services: - ~/src/proot-rs/:/usr/src/proot-rs proot-rs-test: container_name: proot-rs-test - hostname: proot-rs-tests + hostname: proot-rs-test build: ./tests image: proot/proot-rs-test:latest volumes: From 70af66131f145bdca0b0cdfdb1d3eff658ff2e75 Mon Sep 17 00:00:00 2001 From: imlk Date: Thu, 22 Jul 2021 09:40:47 +0800 Subject: [PATCH 11/17] fix: fix the handling of trailing slash test: add unit test for canonicalization.rs --- src/filesystem/canonicalization.rs | 56 +++-- src/filesystem/ext.rs | 269 +++++++++++++++++++++++++ src/filesystem/mod.rs | 1 + src/filesystem/translation.rs | 10 +- src/kernel/standard/dir_link_attr.rs | 20 +- src/kernel/standard/link_at.rs | 10 +- src/kernel/standard/link_rename.rs | 4 +- src/kernel/standard/open.rs | 34 +++- src/kernel/standard/rename_at.rs | 5 +- src/kernel/standard/stat_at.rs | 7 +- src/kernel/standard/sym_link.rs | 1 + src/kernel/standard/sym_link_at.rs | 1 + src/kernel/standard/unlink_mkdir_at.rs | 9 +- tests/applets.bats | 20 ++ 14 files changed, 417 insertions(+), 30 deletions(-) create mode 100644 src/filesystem/ext.rs diff --git a/src/filesystem/canonicalization.rs b/src/filesystem/canonicalization.rs index 04db263..90843b7 100644 --- a/src/filesystem/canonicalization.rs +++ b/src/filesystem/canonicalization.rs @@ -1,9 +1,9 @@ +use std::path::{Component, Path, PathBuf}; + use crate::errors::*; +use crate::filesystem::binding::Side; use crate::filesystem::substitution::Substitutor; use crate::filesystem::FileSystem; -use std::path::{Component, Path, PathBuf}; - -use super::binding::Side; pub trait Canonicalizer { fn canonicalize>(&self, path: P, deref_final: bool) -> Result; @@ -92,16 +92,20 @@ impl Canonicalizer for FileSystem { let host_path = self.substitute(&guest_path_new, Side::Guest)?; let metadata = host_path.symlink_metadata(); - // `metadata` is error if we cannot access this file or file is not exist. - // However, we can accept this path because some syscall (e.g. mkdir, mknod) - // allow final component not exist. - if is_last_component && metadata.is_err() { - continue; - } - // We can continue if we are now on the last component and are explicitly asked - // not to dereference 'user_path'. - if is_last_component && !deref_final { - continue; + + if is_last_component { + // `metadata` is error if we cannot access this file or file is not exist. + // However, we can accept this path because some syscall (e.g. mkdir, mknod) + // allow final component not exist. + if metadata.is_err() { + continue; + } + + // We can continue if we are now on the last component and are explicitly + // asked not to dereference 'user_path'. + if !deref_final { + continue; + } } let file_type = metadata?.file_type(); @@ -129,7 +133,7 @@ impl Canonicalizer for FileSystem { if let Some(comp) = next_comp { new_user_path.push(comp); } - it.for_each(|comp| new_user_path.push(comp)); + new_user_path.push(it); // use new_user_path to call this function again and return // TODO: Can be optimized by replacing `it` return self.canonicalize(&new_user_path, deref_final); @@ -151,11 +155,14 @@ impl Canonicalizer for FileSystem { #[cfg(test)] mod tests { + use std::path::PathBuf; + + use nix::sys::stat::Mode; + use super::*; + use crate::filesystem::ext::PathExt; use crate::filesystem::FileSystem; use crate::utils::tests::get_test_rootfs_path; - use nix::sys::stat::Mode; - use std::path::PathBuf; #[test] fn test_canonicalize_invalid_path() { @@ -264,4 +271,21 @@ mod tests { PathBuf::from("/lib") ); } + + #[test] + fn test_canonicalize_trailing_slash() { + let fs = FileSystem::with_root(get_test_rootfs_path()).unwrap(); + + let path = fs.canonicalize(&PathBuf::from("/lib64"), true).unwrap(); + assert!(!path.with_trailing_slash()); + + let path = fs.canonicalize(&PathBuf::from("/lib64/"), true).unwrap(); + assert!(!path.with_trailing_slash()); + + let path = fs.canonicalize(&PathBuf::from("/lib64"), false).unwrap(); + assert!(!path.with_trailing_slash()); + + let path = fs.canonicalize(&PathBuf::from("/lib64/"), false).unwrap(); + assert!(!path.with_trailing_slash()); + } } diff --git a/src/filesystem/ext.rs b/src/filesystem/ext.rs new file mode 100644 index 0000000..4168c7e --- /dev/null +++ b/src/filesystem/ext.rs @@ -0,0 +1,269 @@ +//! This module is used to help to solve the trailing slash problem. +//! +//! For most syscalls (except mkdir() and rmdir() and something else), a +//! trailing slash cannot be ignored, because it causes two side effects: +//! 1. The kernel will assuming that the last component of the path is a +//! directory or a symbol link to a directory. +//! 2. If the path is a symbol link file, the kernel will follow the link +//! file recursively. +//! We only need to deal with the second case, the first case is left to the +//! kernel to deal with. That is, adding a possibility to make the value of +//! `deref_final` be `true`: when there is a trailing slash at the end of the +//! path. +//! +//! As mentioned in docs of [`PathBuf::components()`], the rust std library will +//! preform a small amount of normalization on path in some case. This +//! unperceived behavior brings some imperceptible effects to the proot-rs' +//! behavior. However rust lacks functions for handling trailing slash. So we +//! extended `PathBuf` and `AsRef` to meet our requirements. +//! +//! Note: Due to the rule of normalization in rust PathBuf, both the trailing +//! "/" and trailing "/." are ignored. Since the two mean almost the same thing, +//! they are both considered to be "trailing slash" in proot-rs. +//! +//! [`PathBuf::components()`]: https://doc.rust-lang.org/std/path/struct.PathBuf.html#method.components + +use std::{ + os::unix::prelude::OsStrExt, + path::{Path, PathBuf}, +}; + +use nix::NixPath; + +pub trait PathExt { + /// Check if this path contains trailing slash (i.e. ends with "/" or "/."). + fn with_trailing_slash(&self) -> bool; +} + +impl PathExt for T +where + T: AsRef, +{ + fn with_trailing_slash(&self) -> bool { + let bytes = self.as_ref().as_os_str().as_bytes(); + let len = bytes.len(); + (len >= 1 && bytes.get(len - 1) == Some(&b'/')) + || (len >= 2 && bytes.get(len - 2) == Some(&b'/') && bytes.get(len - 1) == Some(&b'.')) + } +} + +pub trait PathBufExt { + fn try_add_trailing_slash(&mut self); +} + +impl PathBufExt for PathBuf { + /// Add a trailing slash ("/") to PathBuf. + /// You can't assume that `self` will eventually ends with "/", since it + /// could also be ends with "/.". Furthermore, if self is empty path (""), + /// then no changes will be made to `self`. + fn try_add_trailing_slash(&mut self) { + if !self.with_trailing_slash() && !self.is_empty() { + unsafe { + let old_self = std::ptr::read(self); + let mut os_string = old_self.into_os_string(); + os_string.push("/"); + let new_self = PathBuf::from(os_string); + std::ptr::write(self, new_self); + } + } + } +} + +#[cfg(test)] +mod tests { + use std::fs::File; + + use crate::utils::tests::test_with_proot; + + use super::*; + + #[test] + fn test_with_trailing_slash() { + assert_eq!("".with_trailing_slash(), false); + assert_eq!("/".with_trailing_slash(), true); + assert_eq!("foo".with_trailing_slash(), false); + assert_eq!("foo/".with_trailing_slash(), true); + assert_eq!("foo/.".with_trailing_slash(), true); + assert_eq!("foo/./".with_trailing_slash(), true); + assert_eq!("foo/..".with_trailing_slash(), false); + } + + #[test] + fn test_try_add_trailing_slash() { + let mut pathbuf = PathBuf::from(""); + pathbuf.try_add_trailing_slash(); + assert_eq!(pathbuf.as_os_str(), ""); + + let mut pathbuf = PathBuf::from("/"); + pathbuf.try_add_trailing_slash(); + assert_eq!(pathbuf.as_os_str(), "/"); + + let mut pathbuf = PathBuf::from("foo"); + pathbuf.try_add_trailing_slash(); + assert_eq!(pathbuf.as_os_str(), "foo/"); + + let mut pathbuf = PathBuf::from("foo/"); + pathbuf.try_add_trailing_slash(); + assert_eq!(pathbuf.as_os_str(), "foo/"); + + let mut pathbuf = PathBuf::from("foo/."); + pathbuf.try_add_trailing_slash(); + assert_eq!(pathbuf.as_os_str(), "foo/."); + + let mut pathbuf = PathBuf::from("foo/./"); + pathbuf.try_add_trailing_slash(); + assert_eq!(pathbuf.as_os_str(), "foo/./"); + + let mut pathbuf = PathBuf::from("foo/.."); + pathbuf.try_add_trailing_slash(); + assert_eq!(pathbuf.as_os_str(), "foo/../"); + } + + #[test] + fn test_trailing_slash_problem_with_lstat() { + test_with_proot( + |_tracee, _is_sysenter, _before_translation| {}, + || { + let base_dir = "/tmp/test_lstat_path_with_trailing_slash"; + + let file_name = String::from(base_dir) + "/" + "file"; + let dir_name = String::from(base_dir) + "/" + "dir"; + let link1_name = String::from(base_dir) + "/" + "link1"; + let link2_name = String::from(base_dir) + "/" + "link2"; + + let result = std::panic::catch_unwind(|| { + // create a temporary dir for test + nc::mkdir(base_dir, 0o755).unwrap(); + + // init file and dir + File::create(&file_name).unwrap(); + nc::mkdir(&dir_name, 0o755).unwrap(); + + nc::symlink(&file_name, &link1_name).unwrap(); + nc::symlink(&dir_name, &link2_name).unwrap(); + + let mut stat = nc::stat_t::default(); + + // lstat("link1") + nc::lstat(&link1_name, &mut stat).unwrap(); + assert_eq!((stat.st_mode & nc::S_IFMT), nc::S_IFLNK); + + // lstat("link1/") + assert_eq!( + nc::lstat(format!("{}/", link1_name).as_str(), &mut stat), + Err(nc::ENOTDIR) + ); + + // lstat("link2") + nc::lstat(&link2_name, &mut stat).unwrap(); + assert_eq!((stat.st_mode & nc::S_IFMT), nc::S_IFLNK); + + // lstat("link2/") + nc::lstat(format!("{}/", link2_name).as_str(), &mut stat).unwrap(); + assert_eq!((stat.st_mode & nc::S_IFMT), nc::S_IFDIR); + }); + + let _ = std::fs::remove_dir_all(base_dir); + if let Err(err) = result { + std::panic::resume_unwind(err); + } + }, + ) + } + + #[test] + fn test_trailing_slash_problem_with_mkdir() { + test_with_proot( + |_tracee, _is_sysenter, _before_translation| {}, + || { + let base_dir = "/tmp/test_trailing_slash_problem_with_mkdir"; + + let dir1_name = String::from(base_dir) + "/" + "dir1"; + let dir2_name = String::from(base_dir) + "/" + "dir2"; + let dir3_name = String::from(base_dir) + "/" + "dir3"; + let link1_name = String::from(base_dir) + "/" + "link1"; + + let result = std::panic::catch_unwind(|| { + // create a temporary dir for test + nc::mkdir(base_dir, 0o755).unwrap(); + + let mut stat = nc::stat_t::default(); + + // mkdir("dir1"), test mkdir without trailing slash + nc::mkdir(&dir1_name, 0o755).unwrap(); + nc::lstat(&dir1_name, &mut stat).unwrap(); + assert_eq!((stat.st_mode & nc::S_IFMT), nc::S_IFDIR); + + // mkdir("dir2/"), test mkdir with trailing slash + nc::mkdir(format!("{}/", dir2_name).as_str(), 0o755).unwrap(); + nc::lstat(format!("{}/", dir2_name).as_str(), &mut stat).unwrap(); + assert_eq!((stat.st_mode & nc::S_IFMT), nc::S_IFDIR); + + nc::symlink(&dir3_name, &link1_name).unwrap(); + + // mkdir("link1"), test mkdir with a symlink path without trailing slash + assert_eq!(nc::mkdir(&link1_name, 0o755), Err(nc::EEXIST)); + assert_eq!(nc::lstat(&dir3_name, &mut stat), Err(nc::ENOENT)); + + // mkdir("link1/"), test mkdir with a symlink path with trailing slash + // Some sys-call(e.g. mkdir() and rmdir()) should never dereference the final + // component, even if the path contains a trailing slash. + assert_eq!( + nc::mkdir(format!("{}/", link1_name).as_str(), 0o755), + Err(nc::EEXIST) + ); + assert_eq!(nc::lstat(&dir3_name, &mut stat), Err(nc::ENOENT)); + }); + + let _ = std::fs::remove_dir_all(base_dir); + if let Err(err) = result { + std::panic::resume_unwind(err); + } + }, + ) + } + + #[test] + fn test_trailing_slash_problem_follow_symlink_recursively() { + test_with_proot( + |_tracee, _is_sysenter, _before_translation| {}, + || { + let base_dir = "/tmp/test_trailing_slash_problem_follow_symlink_recursively"; + + let dir_name = String::from(base_dir) + "/" + "dir"; + let link1_name = String::from(base_dir) + "/" + "link1"; + let link2_name = String::from(base_dir) + "/" + "link2"; + + let result = std::panic::catch_unwind(|| { + // create a temporary dir for test + nc::mkdir(base_dir, 0o755).unwrap(); + + // init file and dir + nc::mkdir(&dir_name, 0o755).unwrap(); + + nc::symlink(&link2_name, &link1_name).unwrap(); + nc::symlink(&dir_name, &link2_name).unwrap(); + + let mut stat = nc::stat_t::default(); + + // lstat("link1") + nc::lstat(&link1_name, &mut stat).unwrap(); + assert_eq!((stat.st_mode & nc::S_IFMT), nc::S_IFLNK); + + // lstat("link2") + nc::lstat(&link2_name, &mut stat).unwrap(); + assert_eq!((stat.st_mode & nc::S_IFMT), nc::S_IFLNK); + + // lstat("link1/") -> lstat("link2/") -> lstat("dir/") + nc::lstat(format!("{}/", link1_name).as_str(), &mut stat).unwrap(); + assert_eq!((stat.st_mode & nc::S_IFMT), nc::S_IFDIR); + }); + + let _ = std::fs::remove_dir_all(base_dir); + if let Err(err) = result { + std::panic::resume_unwind(err); + } + }, + ) + } +} diff --git a/src/filesystem/mod.rs b/src/filesystem/mod.rs index b60d2ff..72377ae 100644 --- a/src/filesystem/mod.rs +++ b/src/filesystem/mod.rs @@ -1,5 +1,6 @@ pub mod binding; pub mod canonicalization; +pub mod ext; mod fs; pub mod readers; pub mod substitution; diff --git a/src/filesystem/translation.rs b/src/filesystem/translation.rs index ebe26a7..29f58c0 100644 --- a/src/filesystem/translation.rs +++ b/src/filesystem/translation.rs @@ -6,6 +6,8 @@ use crate::filesystem::substitution::Substitutor; use crate::filesystem::FileSystem; use std::path::{Path, PathBuf}; +use super::ext::{PathBufExt, PathExt}; + pub trait Translator { fn translate_path>(&self, guest_path: P, deref_final: bool) -> Result; fn translate_absolute_path>( @@ -41,8 +43,14 @@ impl Translator for FileSystem { guest_path: P, deref_final: bool, ) -> Result { + let trailing_slash = guest_path.with_trailing_slash(); let canonical_guest_path = self.canonicalize(&guest_path, deref_final)?; - let host_path = self.substitute(&canonical_guest_path, Guest)?; + let mut host_path = self.substitute(&canonical_guest_path, Guest)?; + + if trailing_slash { + // recover the trailing slash + host_path.try_add_trailing_slash(); + } Ok(host_path) } diff --git a/src/kernel/standard/dir_link_attr.rs b/src/kernel/standard/dir_link_attr.rs index c108bb8..28f2f19 100644 --- a/src/kernel/standard/dir_link_attr.rs +++ b/src/kernel/standard/dir_link_attr.rs @@ -1,14 +1,30 @@ use crate::errors::*; +use crate::filesystem::ext::PathExt; use crate::filesystem::Translator; use crate::process::tracee::Tracee; use crate::register::PtraceWriter; -use crate::register::{PtraceReader, SysArg1}; +use crate::register::{Current, PtraceReader, SysArg1}; pub fn enter(tracee: &mut Tracee) -> Result<()> { + let sys_num = tracee.regs.get_sys_num(Current); let raw_path = tracee.regs.get_sysarg_path(SysArg1)?; - let host_path = tracee.fs.borrow().translate_path(raw_path, false)?; + let deref_final = match sys_num { + // First, create/delete/rename related system calls cannot follow final component. + sc::nr::UNLINK | sc::nr::RMDIR | sc::nr::MKDIR => false, + _ => { + // Second, since there is no flags here, skip check for LOOKUP_FOLLOW. + // Third, follow if the pathname has trailing slashes. + if raw_path.with_trailing_slash() { + true + } else { + // Default value for those syscalls + false + } + } + }; + let host_path = tracee.fs.borrow().translate_path(raw_path, deref_final)?; tracee.regs.set_sysarg_path( SysArg1, diff --git a/src/kernel/standard/link_at.rs b/src/kernel/standard/link_at.rs index f913d50..2efbe96 100644 --- a/src/kernel/standard/link_at.rs +++ b/src/kernel/standard/link_at.rs @@ -3,6 +3,7 @@ use std::os::unix::prelude::RawFd; use nix::fcntl::AtFlags; use crate::errors::*; +use crate::filesystem::ext::PathExt; use crate::process::tracee::Tracee; use crate::register::PtraceWriter; use crate::register::{Current, PtraceReader, SysArg, SysArg1, SysArg2, SysArg3, SysArg4, SysArg5}; @@ -14,7 +15,7 @@ pub fn enter(tracee: &mut Tracee) -> Result<()> { let new_path = tracee.regs.get_sysarg_path(SysArg4)?; let flags = AtFlags::from_bits_truncate(tracee.regs.get(Current, SysArg(SysArg5)) as _); - let deref_final = flags.contains(AtFlags::AT_SYMLINK_FOLLOW); + let deref_final = flags.contains(AtFlags::AT_SYMLINK_FOLLOW) || old_path.with_trailing_slash(); let old_host_path = tracee.translate_path_at(olddirfd, old_path, deref_final)?; let new_host_path = tracee.translate_path_at(newdirfd, new_path, false)?; @@ -85,6 +86,13 @@ mod tests { let mut old_filestat = nc::stat_t::default(); nc::lstat(oldfilepath, &mut old_filestat).unwrap(); assert_eq!(new_filestat.st_ino, old_filestat.st_ino); + + // If the oldfilename end with a trailing slash, symlink + // follow will also happen. + assert_eq!( + nc::linkat(fd, format!("{}/", oldlinkname).as_str(), fd, newfilename, 0), + Err(nc::ENOTDIR) + ); }); std::fs::remove_file(oldfilepath).unwrap(); std::fs::remove_file(oldlinkpath).unwrap(); diff --git a/src/kernel/standard/link_rename.rs b/src/kernel/standard/link_rename.rs index 853bdbd..d847d84 100644 --- a/src/kernel/standard/link_rename.rs +++ b/src/kernel/standard/link_rename.rs @@ -1,4 +1,5 @@ use crate::errors::*; +use crate::filesystem::ext::PathExt; use crate::filesystem::Translator; use crate::process::tracee::Tracee; use crate::register::PtraceWriter; @@ -8,8 +9,9 @@ use crate::register::{PtraceReader, SysArg1, SysArg2}; pub fn enter(tracee: &mut Tracee) -> Result<()> { let old_path = tracee.regs.get_sysarg_path(SysArg1)?; let new_path = tracee.regs.get_sysarg_path(SysArg2)?; + let deref_final = old_path.with_trailing_slash(); - let old_host_path = tracee.fs.borrow().translate_path(old_path, false)?; + let old_host_path = tracee.fs.borrow().translate_path(old_path, deref_final)?; let new_host_path = tracee.fs.borrow().translate_path(new_path, false)?; tracee.regs.set_sysarg_path( diff --git a/src/kernel/standard/open.rs b/src/kernel/standard/open.rs index 6814b2b..8d29459 100644 --- a/src/kernel/standard/open.rs +++ b/src/kernel/standard/open.rs @@ -43,10 +43,34 @@ mod tests { // init symlink std::os::unix::fs::symlink(filepath, linkpath).unwrap(); - // test open() + // Test open(linkpath + "/") with `O_CREAT` and `O_EXCL`, and this will get a + // EISDIR, and symlink follow didn't not happen. + assert_eq!( + nc::open( + format!("{}/", linkpath).as_str(), + nc::O_RDONLY | nc::O_CREAT | nc::O_EXCL, + 0o755 + ), + Err(nc::EISDIR) + ); - // test open() with `O_CREAT` and `O_EXCL` - // this will create a regular file at `filepath` + // Test open(linkpath) with `O_CREAT` and `O_EXCL`, and this will get a EEXIST, + // because symlink follow didn't not happen. + assert_eq!( + nc::open(linkpath, nc::O_RDONLY | nc::O_CREAT | nc::O_EXCL, 0o755), + Err(nc::EEXIST) + ); + + // Test open(linkpath) with `O_CREAT`, and this will create a regular file at + // `filepath`, because symlink follow happened. + let file_fd = nc::open(linkpath, nc::O_RDONLY | nc::O_CREAT, 0o755).unwrap(); + let mut stat = nc::stat_t::default(); + nc::fstat(file_fd, &mut stat).unwrap(); + assert_eq!((stat.st_mode & nc::S_IFMT), nc::S_IFREG); + nc::unlink(filepath).unwrap(); + + // Test open(filepath) with `O_CREAT` and `O_EXCL`, and this will create a + // regular file at `filepath`. let file_fd = nc::open(filepath, nc::O_RDONLY | nc::O_CREAT | nc::O_EXCL, 0o755).unwrap(); let mut stat = nc::stat_t::default(); @@ -68,8 +92,8 @@ mod tests { assert_eq!((stat.st_mode & nc::S_IFMT), nc::S_IFREG); nc::close(file_fd).unwrap(); }); - std::fs::remove_file(linkpath).unwrap(); - std::fs::remove_file(filepath).unwrap(); + let _ = std::fs::remove_file(linkpath); + let _ = std::fs::remove_file(filepath); if let Err(err) = result { std::panic::resume_unwind(err); } diff --git a/src/kernel/standard/rename_at.rs b/src/kernel/standard/rename_at.rs index cb317d0..1ba9f7a 100644 --- a/src/kernel/standard/rename_at.rs +++ b/src/kernel/standard/rename_at.rs @@ -1,6 +1,7 @@ use std::os::unix::prelude::RawFd; use crate::errors::*; +use crate::filesystem::ext::PathExt; use crate::kernel::standard::link_rename; use crate::process::tracee::Tracee; use crate::register::PtraceWriter; @@ -12,7 +13,9 @@ pub fn enter(tracee: &mut Tracee) -> Result<()> { let old_path = tracee.regs.get_sysarg_path(SysArg2)?; let new_path = tracee.regs.get_sysarg_path(SysArg4)?; - let old_host_path = tracee.translate_path_at(olddirfd, old_path, false)?; + let deref_final = old_path.with_trailing_slash(); + + let old_host_path = tracee.translate_path_at(olddirfd, old_path, deref_final)?; let new_host_path = tracee.translate_path_at(newdirfd, new_path, false)?; tracee.regs.set_sysarg_path( diff --git a/src/kernel/standard/stat_at.rs b/src/kernel/standard/stat_at.rs index 2c61b1c..e320ec6 100644 --- a/src/kernel/standard/stat_at.rs +++ b/src/kernel/standard/stat_at.rs @@ -3,6 +3,7 @@ use std::os::unix::prelude::RawFd; use nix::fcntl::AtFlags; use crate::errors::*; +use crate::filesystem::ext::PathExt; use crate::kernel::syscall; use crate::register::PtraceWriter; use crate::register::{Current, PtraceReader, SysArg, SysArg1, SysArg2, SysArg3, SysArg4, SysArg5}; @@ -37,9 +38,11 @@ pub fn enter(tracee: &mut Tracee) -> Result<()> { // Some system calls will dereference the path by default, while others do not, // which can also be controlled by `flags`. let deref_final = match sys_num { - sc::nr::NAME_TO_HANDLE_AT => flags.contains(AtFlags::AT_SYMLINK_FOLLOW), + sc::nr::NAME_TO_HANDLE_AT => { + flags.contains(AtFlags::AT_SYMLINK_FOLLOW) || raw_path.with_trailing_slash() + } sc::nr::STATX | sc::nr::NEWFSTATAT | sc::nr::UTIMENSAT | sc::nr::FCHOWNAT => { - !flags.contains(AtFlags::AT_SYMLINK_NOFOLLOW) + !flags.contains(AtFlags::AT_SYMLINK_NOFOLLOW) || raw_path.with_trailing_slash() } _ => true, }; diff --git a/src/kernel/standard/sym_link.rs b/src/kernel/standard/sym_link.rs index 42e27fa..274dca1 100644 --- a/src/kernel/standard/sym_link.rs +++ b/src/kernel/standard/sym_link.rs @@ -7,6 +7,7 @@ use crate::register::{PtraceReader, SysArg2}; pub fn enter(tracee: &mut Tracee) -> Result<()> { let raw_path = tracee.regs.get_sysarg_path(SysArg2)?; + // create/delete/rename related system calls cannot follow final component. let host_path = tracee.fs.borrow().translate_path(raw_path, false)?; tracee.regs.set_sysarg_path( diff --git a/src/kernel/standard/sym_link_at.rs b/src/kernel/standard/sym_link_at.rs index abf9cfd..46c65bc 100644 --- a/src/kernel/standard/sym_link_at.rs +++ b/src/kernel/standard/sym_link_at.rs @@ -9,6 +9,7 @@ pub fn enter(tracee: &mut Tracee) -> Result<()> { let dirfd = tracee.regs.get(Current, SysArg(SysArg2)) as RawFd; let raw_path = tracee.regs.get_sysarg_path(SysArg3)?; + // create/delete/rename related system calls cannot follow final component. let host_path = tracee.translate_path_at(dirfd, raw_path, false)?; tracee.regs.set_sysarg_path( diff --git a/src/kernel/standard/unlink_mkdir_at.rs b/src/kernel/standard/unlink_mkdir_at.rs index 78e8dec..8f5620b 100644 --- a/src/kernel/standard/unlink_mkdir_at.rs +++ b/src/kernel/standard/unlink_mkdir_at.rs @@ -1,15 +1,22 @@ use std::os::unix::prelude::RawFd; use crate::errors::*; +use crate::filesystem::ext::PathExt; use crate::process::tracee::Tracee; use crate::register::PtraceWriter; use crate::register::{Current, PtraceReader, SysArg, SysArg1, SysArg2}; pub fn enter(tracee: &mut Tracee) -> Result<()> { + let sys_num = tracee.regs.get_sys_num(Current); let dirfd = tracee.regs.get(Current, SysArg(SysArg1)) as RawFd; let raw_path = tracee.regs.get_sysarg_path(SysArg2)?; - let host_path = tracee.translate_path_at(dirfd, raw_path, false)?; + let deref_final = match sys_num { + sc::nr::UNLINKAT | sc::nr::MKDIRAT => false, + _ => raw_path.with_trailing_slash(), + }; + + let host_path = tracee.translate_path_at(dirfd, raw_path, deref_final)?; tracee.regs.set_sysarg_path( SysArg2, diff --git a/tests/applets.bats b/tests/applets.bats index 212ddf0..a1a46ce 100644 --- a/tests/applets.bats +++ b/tests/applets.bats @@ -330,3 +330,23 @@ function script_test_run_applets_common_tools { proot-rs --rootfs "$ROOTFS" -- /bin/wget http://1.1.1.1 -O - } + +function script_test_proot_rs_path_with_railing_slash { + PATH=/bin + [[ "$(stat -c "%F" /lib64)" == *"symbolic link"* ]] + [[ "$(stat -c "%F" /lib64/)" == *"directory"* ]] + [[ "$(stat -c "%F" /lib64/.)" == *"directory"* ]] + + [[ "$(stat -c "%i %F" /lib64/)" == "$(stat -L -c "%i %F" /lib64)" ]] + + [[ "$(stat -c "%F" /etc/passwd)" == *"regular file"* ]] + [[ "$(stat -L -c "%F" /etc/passwd)" == *"regular file"* ]] + + [[ "$(stat -c "%F" /etc/passwd/ 2>&1)" == *"Not a directory"* ]] + [[ "$(stat -c "%F" /etc/passwd/. 2>&1)" == *"Not a directory"* ]] + +} + +@test "test proot-rs path with railing slash" { + proot-rs --rootfs "$ROOTFS" -- /bin/sh -e -x -c "$(declare -f script_test_proot_rs_path_with_railing_slash); script_test_proot_rs_path_with_railing_slash" +} From b1db7f237501a432ab49bf7c767ec5280ce2b284 Mon Sep 17 00:00:00 2001 From: imlk Date: Thu, 22 Jul 2021 11:34:42 +0800 Subject: [PATCH 12/17] test: add test for not follow trailing slash --- tests/applets.bats | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/applets.bats b/tests/applets.bats index a1a46ce..cdb9947 100644 --- a/tests/applets.bats +++ b/tests/applets.bats @@ -350,3 +350,31 @@ function script_test_proot_rs_path_with_railing_slash { @test "test proot-rs path with railing slash" { proot-rs --rootfs "$ROOTFS" -- /bin/sh -e -x -c "$(declare -f script_test_proot_rs_path_with_railing_slash); script_test_proot_rs_path_with_railing_slash" } + + +function script_test_should_not_follow { + PATH=/bin + + cd /tmp/test_should_not_follow + + ln -s should_not_be_created link + + [[ "$(mkdir link 2>&1)" == *"File exists"* ]] + [ ! -e should_not_be_created ] + + [[ "$(mkdir link/ 2>&1)" == *"File exists"* ]] + [ ! -e should_not_be_created ] + + [[ "$(mkdir link/. 2>&1)" == *"File exists"* ]] + [ ! -e should_not_be_created ] + +} + +@test "test should not follow" { + local test_dir="$ROOTFS/tmp/test_should_not_follow" + mkdir "$test_dir" + runp proot-rs --rootfs "$ROOTFS" -- /bin/sh -e -x -c "$(declare -f script_test_should_not_follow); script_test_should_not_follow" + rm -rf "$test_dir" + [ "$status" -eq 0 ] + +} From 49f13020a4775a3d39a2528adc0eec29325eb6f1 Mon Sep 17 00:00:00 2001 From: imlk Date: Thu, 22 Jul 2021 22:40:36 +0800 Subject: [PATCH 13/17] feat: kill all tracee if the proot-rs exits --- src/process/tracee.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/process/tracee.rs b/src/process/tracee.rs index 37e0976..300e9e1 100644 --- a/src/process/tracee.rs +++ b/src/process/tracee.rs @@ -172,6 +172,7 @@ impl Tracee { | Options::PTRACE_O_TRACEVFORKDONE | Options::PTRACE_O_TRACEEXEC | Options::PTRACE_O_TRACECLONE + | Options::PTRACE_O_EXITKILL | Options::PTRACE_O_TRACEEXIT; //TODO: seccomp From f0eb1f6f4abd06394b655ec27fb338ff7fe0a360 Mon Sep 17 00:00:00 2001 From: imlk Date: Mon, 26 Jul 2021 01:16:02 +0800 Subject: [PATCH 14/17] fix: fix thread 'main' panicked at 'get stopped tracee' --- src/filesystem/binding.rs | 1 - src/process/event.rs | 2 +- src/process/proot.rs | 49 ++++++- src/process/tracee.rs | 18 ++- tests/multi-tracee/nested_fork_vfork_clone.c | 135 +++++++++++++++++++ tests/multi-tracee/test.bats | 13 ++ 6 files changed, 209 insertions(+), 9 deletions(-) create mode 100644 tests/multi-tracee/nested_fork_vfork_clone.c diff --git a/src/filesystem/binding.rs b/src/filesystem/binding.rs index 9665aa7..5e79b3c 100644 --- a/src/filesystem/binding.rs +++ b/src/filesystem/binding.rs @@ -18,7 +18,6 @@ impl Side { } } -// TODO: Maybe we should canonicalize guest path during initialization #[derive(Debug, Clone)] pub struct Binding { /// Host side path of this binding in canonical form. diff --git a/src/process/event.rs b/src/process/event.rs index 5920bec..a2afb37 100644 --- a/src/process/event.rs +++ b/src/process/event.rs @@ -196,7 +196,7 @@ impl EventHandler for Tracee { // (void) restart_tracee(child, 0); // } - child_tracee.sigstop_status = SigStopStatus::RaisedByTraceClone; + child_tracee.sigstop_status = SigStopStatus::WaitForSigStopClone; Ok(child_tracee) } diff --git a/src/process/proot.rs b/src/process/proot.rs index 0be8b89..0035de6 100644 --- a/src/process/proot.rs +++ b/src/process/proot.rs @@ -197,7 +197,38 @@ impl PRoot { let mut signal_to_delivery = Some(stop_signal); - let tracee = self.tracees.get_mut(&pid).expect("get stopped tracee"); + let maybe_tracee = self.tracees.get_mut(&pid); + + let tracee = if maybe_tracee.is_none() { + if stop_signal == Signal::SIGSTOP { + debug!("-- {}, SIGSTOP arrives before ptrace event but tracee is not initialized, so create a placeholder to record this.", pid); + // Get tracee instance of init process, note that at this point + // `init_pid` must not be none, so we can unwrap() it safely. + let init_tracee = self.tracees.get(&self.init_pid.unwrap()).unwrap(); + // Create a new tracee instance as placeholder, only for record the pid + // and sigstop status of this newly created process. + // Since the `fs` field cannot be none value, we'll temporarily use the + // value of the init process's fs field in its place, even though it + // should be actually derived from the parent process. But please + // remember that the `fs` field should not be used until the tracee is + // fully initialized in the ptrace event handler function. + let mut tracee = Tracee::new(pid, init_tracee.fs.clone()); + // We are waiting for a ptrace event to initialize this tracee. + tracee.sigstop_status = SigStopStatus::WaitForEventClone; + self.insert_new_tracee(tracee); + signal_to_delivery = None; + self.tracees.get_mut(&pid).unwrap() + } else { + error!("-- {}, Received a signal from an unknown tracee.", pid); + // Deliver this SIGSTOP signal to this unknown tracee + ptrace::syscall(pid, Some(stop_signal)) + .expect("deliver stop signal to unknown tracee"); + // continue the event loop + continue; + } + } else { + maybe_tracee.unwrap() + }; tracee.reset_restart_how(); match stop_signal { Signal::SIGSTOP => { @@ -208,7 +239,7 @@ impl PRoot { tracee.check_and_set_ptrace_options(&mut self.info_bag)?; signal_to_delivery = None; tracee.sigstop_status = SigStopStatus::AllowDelivery; - } else if tracee.sigstop_status == SigStopStatus::RaisedByTraceClone { + } else if tracee.sigstop_status == SigStopStatus::WaitForSigStopClone { signal_to_delivery = None; tracee.sigstop_status = SigStopStatus::AllowDelivery; } @@ -280,8 +311,20 @@ impl PRoot { | Some(PtraceEvent::PTRACE_EVENT_VFORK) | Some(PtraceEvent::PTRACE_EVENT_CLONE) => { match tracee.handle_new_child_event() { - Ok(child_tracee) => { + Ok(mut child_tracee) => { info!("-- {}, new process with pid {}", pid, child_tracee.pid); + // If a placeholder exists, replace it with fully initialized + // tracee. + if let Some(tracee_placeholder) = + self.tracees.get(&child_tracee.pid) + { + if tracee_placeholder.sigstop_status + == SigStopStatus::WaitForEventClone + { + child_tracee.sigstop_status = + SigStopStatus::AllowDelivery; + } + } self.insert_new_tracee(child_tracee) } Err(error) => { diff --git a/src/process/tracee.rs b/src/process/tracee.rs index 300e9e1..fa72c40 100644 --- a/src/process/tracee.rs +++ b/src/process/tracee.rs @@ -60,13 +60,23 @@ pub enum TraceeRestartMethod { pub enum SigStopStatus { /// Allow SIGSTOP to be passed to tracee, which is the most common case. AllowDelivery, - /// The current process is a new process and the next SIGSTOP signal is - /// caused by automatically start tracing the new child process. - /// See the description of PTRACE_O_TRACE(FORK|VFORK|CLONE) in ptrace(2). - RaisedByTraceClone, /// The next SIGSTOP signal is used to synchronize with Proot process, and /// is only used during creating the first tracee. EventloopSync, + /// The current process is a new process created by + /// `fork()`/`vfork()`/`clone()`, and one of the ptrace events + /// `PTRACE_EVENT_(FORK|VFORK|CLONE)` has been received. So we are waiting + /// to eliminate the next associated SIGSTOP signal. + /// See the description of PTRACE_O_TRACE(FORK|VFORK|CLONE) in ptrace(2). + WaitForSigStopClone, + /// The current process is a new process created by + /// `fork()`/`vfork()`/`clone()`, and the SIGSTOP signal arrives before the + /// ptrace events. In this case, initialization of this tracee object is not + /// completed, because we have no way to known the parent id of this tracee. + /// So that we are waiting for one of the ptrace events + /// `PTRACE_EVENT_(FORK|VFORK|CLONE)` to arrive. + /// See the description of PTRACE_O_TRACE(FORK|VFORK|CLONE) in ptrace(2). + WaitForEventClone, } #[derive(Debug)] diff --git a/tests/multi-tracee/nested_fork_vfork_clone.c b/tests/multi-tracee/nested_fork_vfork_clone.c new file mode 100644 index 0000000..6b6ca44 --- /dev/null +++ b/tests/multi-tracee/nested_fork_vfork_clone.c @@ -0,0 +1,135 @@ +/** + * This program is designed to test the handling of fork() and vfork() and + * clone() functions. Not only test the call of those functions, this program + * will combine them, and test the correctness of these functions when called + * in nesting. + * We define the nested calls of these three functions as a series of + * actions: ACTION_FORK(1), ACTION_VFORK(2), ACTION_CLONE(3). + * The test is performed by generating a sequence of a certain length, and the + * length is the depth of the iteration. which means that a total of + * 3^DEPTH_OF_FORK tests will be executed. + * In each test, we call one of these three functions in the sequence defined + * previously in nesting. For example, for a sequence [1, 2, 3], this program + * call fork() to create a child, and then the child will call vfork() to + * generate another new child, which will then call clone(). To observe the + * results, each child process will print out the id of the current action, and + * we can test whether it passes by checking the program output. + */ + +#define _GNU_SOURCE + +#include +#include +#include +#include +#include +#include +#include + +#define ACTION_EMPTY 0 +#define ACTION_FORK 1 +#define ACTION_VFORK 2 +#define ACTION_CLONE 3 + +#define ACTION_BITS_LEN 2 +#define ACTION_BITS_MASK ((1 << ACTION_BITS_LEN) - 1) + +#define STACK_SIZE (1024 * 1024) + +#define DEPTH_OF_FORK 3 + +static void exit_with_error(char *msg) { + fprintf(stderr, "%s\n", msg); + exit(1); +} + +void do_things(size_t action) { printf("%d", action); } + +void wait_for_child_exit(pid_t pid) { + while (1) { + int wstatus = 0; + waitpid(pid, &wstatus, 0); + if (WIFEXITED(wstatus)) { + if (WEXITSTATUS(wstatus) != 0) { + char buf[1024]; + sprintf(buf, "child process %d terminated with status %d", pid, + WEXITSTATUS(wstatus)); + exit_with_error(buf); + } + return; + } else if (WIFSIGNALED(wstatus)) { + char buf[1024]; + sprintf(buf, "child process %d terminated by signal %d", pid, + WTERMSIG(wstatus)); + exit_with_error(buf); + return; + } + } +} + +int clone_child_func(size_t actions) { + do_things(ACTION_CLONE); + perform(actions >> ACTION_BITS_LEN); + return 0; +} + +void perform(size_t actions) { + size_t action = actions & ACTION_BITS_MASK; + if (action == ACTION_EMPTY) { + } else if (action == ACTION_FORK) { + pid_t pid = fork(); + if (pid == 0) { // child + do_things(ACTION_FORK); + perform(actions >> ACTION_BITS_LEN); + _exit(0); + } else if (pid == -1) { + exit_with_error("Error while fork()"); + } else { // parent + wait_for_child_exit(pid); + } + } else if (action == ACTION_VFORK) { + pid_t pid = vfork(); + if (pid == 0) { // child + do_things(ACTION_VFORK); + perform(actions >> ACTION_BITS_LEN); + _exit(0); + } else if (pid == -1) { + exit_with_error("Error while vfork()"); + } else { // parent + wait_for_child_exit(pid); + } + } else if (action == ACTION_CLONE) { + char *stack = mmap(NULL, STACK_SIZE, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0); + if (stack == MAP_FAILED) + exit_with_error("Error while mmap()"); + char *stack_top = stack + STACK_SIZE; + + pid_t pid = + clone(clone_child_func, stack_top, CLONE_FS | SIGCHLD, actions); + if (pid == -1) + exit_with_error("Error while clone()"); + + wait_for_child_exit(pid); + } +} + +void build_and_perform(int depth, size_t actions) { + if (depth == 0) { + perform(actions); + printf(" "); + } else { + build_and_perform(depth - 1, + (actions << ACTION_BITS_LEN) | ACTION_FORK); + build_and_perform(depth - 1, + (actions << ACTION_BITS_LEN) | ACTION_VFORK); + build_and_perform(depth - 1, + (actions << ACTION_BITS_LEN) | ACTION_CLONE); + } +} + +int main(int argc, char const *argv[]) { + setvbuf(stdout, NULL, _IONBF, 0); + build_and_perform(DEPTH_OF_FORK, 0); + return 0; +} diff --git a/tests/multi-tracee/test.bats b/tests/multi-tracee/test.bats index 4667f89..e7c6e89 100644 --- a/tests/multi-tracee/test.bats +++ b/tests/multi-tracee/test.bats @@ -28,3 +28,16 @@ load ../helper [ "${lines[2]}" = "/" ] [ "${#lines[@]}" -eq 3 ] } + + +@test "test nested calls between fork() / vfork() / clone()" { + # compile test case + compile_c_dynamic "$ROOTFS/bin/nested_fork_vfork_clone" "$BATS_TEST_DIRNAME/nested_fork_vfork_clone.c" + runp proot-rs --rootfs "$ROOTFS" --cwd / -- /bin/nested_fork_vfork_clone + # remember to delete the binary file + rm "$ROOTFS/bin/nested_fork_vfork_clone" + [ "$status" -eq 0 ] + [ "${lines[0]}" = "111 211 311 121 221 321 131 231 331 112 212 312 122 222 322 132 232 332 113 213 313 123 223 323 133 233 333 " ] + [ "${#lines[@]}" -eq 1 ] +} + From b08507e0d980a450dffc15fb5620eb6cee74cb35 Mon Sep 17 00:00:00 2001 From: imlk Date: Mon, 26 Jul 2021 07:58:20 +0800 Subject: [PATCH 15/17] refactor: fix an error in a comment --- src/kernel/standard/open_at.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kernel/standard/open_at.rs b/src/kernel/standard/open_at.rs index df34ae0..e1a627c 100644 --- a/src/kernel/standard/open_at.rs +++ b/src/kernel/standard/open_at.rs @@ -34,7 +34,7 @@ mod tests { use crate::utils::tests::test_with_proot; /// Unit test for the following syscalls: - /// - linkat + /// - openat #[test] fn test_open_at() { test_with_proot( From 7485b2ee02f0f645f3c376fa35c8eb73c1831245 Mon Sep 17 00:00:00 2001 From: imlk Date: Mon, 26 Jul 2021 08:30:50 +0800 Subject: [PATCH 16/17] fix: add check for .push("") in translate_path_at() --- src/process/tracee.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/process/tracee.rs b/src/process/tracee.rs index fa72c40..145d09f 100644 --- a/src/process/tracee.rs +++ b/src/process/tracee.rs @@ -6,6 +6,7 @@ use std::rc::Rc; use nix::sys::ptrace::{self, Options}; use nix::sys::signal::Signal; use nix::unistd::Pid; +use nix::NixPath; use crate::errors::*; use crate::filesystem::Substitutor; @@ -259,7 +260,10 @@ impl Tracee { ) -> Result { if guest_path.as_ref().is_relative() { let mut dir_path = self.get_path_from_fd(dirfd, Side::Guest)?; - dir_path.push(guest_path); + // Check if guest_path is empty to avoid side effects of .push() + if !guest_path.as_ref().is_empty() { + dir_path.push(guest_path); + } self.fs .borrow() .translate_absolute_path(dir_path, deref_final) From 021074f514412829f5c32ddba198f72b67f28ac4 Mon Sep 17 00:00:00 2001 From: imlk Date: Mon, 26 Jul 2021 08:53:59 +0800 Subject: [PATCH 17/17] test: fix some test files --- tests/applets.bats | 38 +++++++++++++++++++++++++++----------- tests/bind.bats | 1 + 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/tests/applets.bats b/tests/applets.bats index cdb9947..ff3db04 100644 --- a/tests/applets.bats +++ b/tests/applets.bats @@ -22,23 +22,23 @@ function script_test_run_applets_file_ops { [ "$(/bin/pwd -L)" = "/tmp/test_applets_file_ops" ] # ls - [ "$(ls)" = $'dir1\nfile1' ] + [ "$(ls | sort)" = $'dir1\nfile1' ] # touch /bin/touch file2 - [ "$(ls)" = $'dir1\nfile1\nfile2' ] + [ "$(ls | sort)" = $'dir1\nfile1\nfile2' ] # cp /bin/echo "test" > file2 cp file2 file3 - [ "$(ls)" = $'dir1\nfile1\nfile2\nfile3' ] + [ "$(ls | sort)" = $'dir1\nfile1\nfile2\nfile3' ] # mv mv file3 dir1/file4 - [ "$(ls)" = $'dir1\nfile1\nfile2' ] + [ "$(ls | sort)" = $'dir1\nfile1\nfile2' ] # find - [ "$(find .)" = $'.\n./dir1\n./dir1/file4\n./file1\n./file2' ] + [ "$(find . | sort)" = $'.\n./dir1\n./dir1/file4\n./file1\n./file2' ] # cat [ "$(cat file2)" = "test" ] @@ -137,7 +137,7 @@ function script_test_run_applets_common_tools { [ "$(cat file1)" = "test tee" ] # du - [ "$(du -h file1)" = "4.0K file1" ] + du -h file1 | grep '4\.0K\s*file1' # base64 [ "$(echo 'base64' | base64)" = "YmFzZTY0Cg==" ] @@ -172,7 +172,7 @@ function script_test_run_applets_common_tools { # split echo "0123456789" | split -b 4 - test_split. - [ "$(ls test_split.*)" = $'test_split.aa\ntest_split.ab\ntest_split.ac' ] + [ "$(ls test_split.* | sort)" = $'test_split.aa\ntest_split.ab\ntest_split.ac' ] # date [ "$(date -d @0)" = "Thu Jan 1 00:00:00 UTC 1970" ] @@ -183,8 +183,9 @@ function script_test_run_applets_common_tools { # tr [ "$(echo "hello WORLD" | tr "[:upper:]" "[:lower:]")" = "hello world" ] - # args - [ "$(echo "echo 'hello world'" | xargs)" = "hello world" ] + # xargs + [ "$(echo "'echo hello world'" | xargs sh -c)" = "hello world" ] + [ "$(echo "hello world" | xargs echo)" = "hello world" ] # which [ "$(which sh)" = "/bin/sh" ] @@ -220,7 +221,7 @@ function script_test_run_applets_common_tools { [ -f file1 ] # unzip - echo "UEsDBC0AAAAAAB1G8VItOwiv//////////8BABQALQEAEAAMAAAAAAAAAAwAAAAAAAAAaGVsbG8gd29ybGQKUEsBAh4DLQAAAAAAHUbxUi07CK8MAAAADAAAAAEAAAAAAAAAAQAAAIARAAAAAC1QSwYGLAAAAAAAAAAeAy0AAAAAAAAAAAABAAAAAAAAAAEAAAAAAAAALwAAAAAAAAA/AAAAAAAAAFBLBgcAAAAAbgAAAAAAAAABAAAAUEsFBgAAAAABAAEALwAAAD8AAAAAAA==" | base64 -d > archive.zip + echo "UEsDBAoAAAAAABtG+lItOwivDAAAAAwAAAAEABwAZmlsZVVUCQADdQb+YIcG/mB1eAsAAQToAwAABOgDAABoZWxsbyB3b3JsZApQSwECHgMKAAAAAAAbRvpSLTsIrwwAAAAMAAAABAAYAAAAAAABAAAApIEAAAAAZmlsZVVUBQADdQb+YHV4CwABBOgDAAAE6AMAAFBLBQYAAAAAAQABAEoAAABKAAAAAAA=" | base64 -d > archive.zip [ "$(unzip -p archive.zip)" = "hello world" ] # dd @@ -327,7 +328,22 @@ function script_test_run_applets_common_tools { @test "test proot-rs run wget" { - proot-rs --rootfs "$ROOTFS" -- /bin/wget http://1.1.1.1 -O - + resolv_conf_exists=true + if [ ! -f "$ROOTFS/etc/resolv.conf" ]; then + touch "$ROOTFS/etc/resolv.conf" + resolv_conf_exists=false + fi + + # bind /etc/resolv.conf so that wget can read dns server address from it + runp proot-rs --rootfs "$ROOTFS" --bind "/etc/resolv.conf:/etc/resolv.conf" -- /bin/sh -e -x -c ' + [[ "$(/bin/wget http://example.com/ -O -)" == *"Example Domain"* ]] + ' + + if [ "$resolv_conf_exists" == false ]; then + rm "$ROOTFS/etc/resolv.conf" + fi + + [ "$status" -eq 0 ] } diff --git a/tests/bind.bats b/tests/bind.bats index 3c8a0bf..3161b6b 100644 --- a/tests/bind.bats +++ b/tests/bind.bats @@ -37,6 +37,7 @@ load helper @test "test --bind with getdents64() results" { + skip "this is an enhancement, see https://github.com/proot-me/proot-rs/issues/43" mkdir "$ROOTFS/tmp/test_bind_with_getdents64" echo "first" > "$ROOTFS/tmp/test_bind_with_getdents64/file1" echo "second" > "$ROOTFS/tmp/test_bind_with_getdents64/file2"