diff options
author | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2022-05-10 06:59:20 +0000 |
---|---|---|
committer | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2022-05-10 06:59:20 +0000 |
commit | 29e89f78974cd8c543f602681924e1c3f81cb06f (patch) | |
tree | 4533b7472f0475d2ba0bb28d9e366d7cfb03b8ef | |
parent | 83a55d6e4c8d513ce8385bce06dd0c8ac8583f0f (diff) | |
parent | add50186ecbe274faa395ce13a790e94a524b408 (diff) | |
download | minijail-android13-mainline-adbd-release.tar.gz |
Snap for 8564071 from add50186ecbe274faa395ce13a790e94a524b408 to mainline-adbd-releaseaml_adb_331610000aml_adb_331314020aml_adb_331113120aml_adb_331011050aml_adb_331011040android13-mainline-adbd-release
Change-Id: If85fa95369ad51dee8bb9cb69bbd308c3c6b6878
56 files changed, 3423 insertions, 978 deletions
diff --git a/.github/mistaken-pull-closer.yml b/.github/mistaken-pull-closer.yml deleted file mode 100644 index eb4cb89..0000000 --- a/.github/mistaken-pull-closer.yml +++ /dev/null @@ -1,17 +0,0 @@ -# The JSONPath filter expression used to identify which PRs to close. -# The data filtered is the pull request data along with other metadata passed in -# by probot. -# See https://goessner.net/articles/JsonPath/ -# `true` will close all PRs. -filters: - - true - -# The message to post to the closed PR. -commentBody: | - Thanks for your contribution! Unfortunately, we don't use GitHub pull - requests to manage code contributions to this repository. Instead, please - see [README.md](../blob/master/README.md) which provides full instructions - on how to get involved. - -# Whether to add a label to the closed PR. -addLabel: false diff --git a/.github/workflows/build-test-ci.yml b/.github/workflows/build-test-ci.yml index bed8a43..74a95aa 100644 --- a/.github/workflows/build-test-ci.yml +++ b/.github/workflows/build-test-ci.yml @@ -8,6 +8,14 @@ on: branches: [master] tags: [linux-v*] + schedule: + # The GH mirroring from Google GoB does not trigger push actions. + # Fire it every other day to provide some coverage. This will run ~8 AM PT. + - cron: '39 3 */2 * *' + + # Allow for manual triggers from the web. + workflow_dispatch: + jobs: build-test: strategy: diff --git a/.github/workflows/close-pull-request.yml b/.github/workflows/close-pull-request.yml new file mode 100644 index 0000000..da11c4a --- /dev/null +++ b/.github/workflows/close-pull-request.yml @@ -0,0 +1,22 @@ +# GitHub actions workflow. +# https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions + +# https://github.com/superbrothers/close-pull-request +name: Close Pull Request + +on: + pull_request_target: + types: [opened] + +jobs: + run: + runs-on: ubuntu-latest + steps: + - uses: superbrothers/close-pull-request@v3 + with: + comment: > + Thanks for your contribution! + Unfortunately, we don't use GitHub pull requests to manage code + contributions to this repository. + Instead, please see [README.md](../blob/HEAD/README.md) which + provides full instructions on how to get involved. diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml new file mode 100644 index 0000000..42839cc --- /dev/null +++ b/.github/workflows/coverity.yml @@ -0,0 +1,36 @@ +# GitHub actions workflow. +# https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions + +# https://scan.coverity.com/projects/google-minijail +name: Coverity Scan + +on: + push: + branches: [master] + + schedule: + # The GH mirroring from Google GoB does not trigger push actions. + # Fire it once a week to provide some coverage. + - cron: '39 2 * * WED' + + # Allow for manual triggers from the web. + workflow_dispatch: + +jobs: + coverity: + strategy: + matrix: + os: [ubuntu-latest] + cc: [clang] + runs-on: ${{ matrix.os }} + env: + CC: ${{ matrix.cc }} + steps: + - uses: actions/checkout@v2 + - name: Install system packages + run: sudo apt-get install -y libcap-dev + - uses: vapier/coverity-scan-action@v0 + with: + project: google%2Fminijail + email: vapier@google.com + token: ${{ secrets.COVERITY_SCAN_TOKEN }} @@ -24,13 +24,9 @@ # Executables when compiling in-tree. /dump_constants -/libminijail_unittest /minijail0 -/minijail0_cli_unittest -/syscall_filter_unittest -/system_unittest -/util_unittest /parse_seccomp_policy +/*_unittest # common.mk TEST(). *.test @@ -62,6 +62,7 @@ libminijailSrcFiles = [ unittestSrcFiles = [ "testrunner.cc", + "test_util.cc", ] minijailCommonLibraries = ["libcap"] @@ -91,7 +92,6 @@ cc_object { vendor_available: true, product_available: true, recovery_available: true, - header_libs: ["libc_headers"], // TODO(b/153662223): Clean this up. srcs: ["gen_syscalls.c"], cflags: [ "-dD", @@ -102,6 +102,7 @@ cc_object { apex_available: [ "//apex_available:platform", "com.android.adbd", + "com.android.compos", "com.android.media.swcodec", "com.android.virt", ], @@ -120,6 +121,7 @@ cc_genrule { apex_available: [ "//apex_available:platform", "com.android.adbd", + "com.android.compos", "com.android.media.swcodec", "com.android.virt", ], @@ -130,7 +132,6 @@ cc_object { vendor_available: true, product_available: true, recovery_available: true, - header_libs: ["libc_headers"], // TODO(b/153662223): Clean this up. srcs: ["gen_constants.c"], cflags: [ "-dD", @@ -141,6 +142,7 @@ cc_object { apex_available: [ "//apex_available:platform", "com.android.adbd", + "com.android.compos", "com.android.media.swcodec", "com.android.virt", ], @@ -159,6 +161,7 @@ cc_genrule { apex_available: [ "//apex_available:platform", "com.android.adbd", + "com.android.compos", "com.android.media.swcodec", "com.android.virt", ], @@ -189,6 +192,7 @@ cc_library_static { apex_available: [ "//apex_available:platform", "com.android.adbd", + "com.android.compos", "com.android.media.swcodec", "com.android.virt", ], @@ -201,7 +205,6 @@ cc_object { product_available: true, recovery_available: true, host_supported: true, - header_libs: ["libc_headers"], // TODO(b/153662223): Clean this up. cflags: [ "-S", "-O0", @@ -224,7 +227,6 @@ cc_object { product_available: true, recovery_available: true, host_supported: true, - header_libs: ["libc_headers"], // TODO(b/153662223): Clean this up. cflags: [ "-S", "-O0", @@ -277,6 +279,7 @@ cc_library { apex_available: [ "//apex_available:platform", "com.android.adbd", + "com.android.compos", "com.android.media.swcodec", "com.android.virt", ], @@ -441,6 +444,7 @@ cc_test { "-DPRELOADPATH=\"/invalid\"", ], srcs: libminijailSrcFiles + [ + "config_parser.c", "elfparse.c", "minijail0_cli.c", "minijail0_cli_unittest.cc", @@ -454,6 +458,41 @@ cc_test { test_suites: ["device-tests"], }, }, + data: ["test/*"], +} + + +// Configuration file parser functionality unit tests using gtest. +// +// For a device, run with: +// adb shell /data/nativetest/config_parser_unittest_gtest/config_parser_unittest_gtest +// +// For host, run with: +// out/host/linux-x86/nativetest(64)/config_parser_unittest_gtest/config_parser_unittest_gtest +// ========================================================= +cc_test { + name: "config_parser_unittest_gtest", + defaults: ["libminijail_flags"], + host_supported: true, + + srcs: [ + "config_parser.c", + "util.c", + "config_parser_unittest.cc", + ] + unittestSrcFiles, + + static_libs: ["libminijail_generated"], + shared_libs: minijailCommonLibraries, + + target: { + android: { + test_suites: ["device-tests"], + }, + }, + test_options: { + unit_test: true, + }, + data: ["test/*"], } // libminijail_test executable for brillo_Minijail test. @@ -502,6 +541,7 @@ cc_binary { "-DPRELOADPATH=\"/invalidminijailpreload.so\"", ], srcs: [ + "config_parser.c", "elfparse.c", "minijail0.c", "minijail0_cli.c", @@ -538,6 +578,7 @@ rust_library { ], apex_available: [ "//apex_available:platform", + "com.android.compos", "com.android.virt", ], } @@ -557,6 +598,7 @@ rust_library { ], apex_available: [ "//apex_available:platform", + "com.android.compos", "com.android.virt", ], } diff --git a/DIR_METADATA b/DIR_METADATA new file mode 100644 index 0000000..4a465c7 --- /dev/null +++ b/DIR_METADATA @@ -0,0 +1,17 @@ +# Metadata information for this directory. +# +# For more information on DIR_METADATA files, see: +# https://source.chromium.org/chromium/infra/infra/+/HEAD:go/src/infra/tools/dirmd/README.md +# +# For the schema of this file, see Metadata message: +# https://source.chromium.org/chromium/infra/infra/+/HEAD:go/src/infra/tools/dirmd/proto/dir_metadata.proto + +buganizer { + component_id: 1099158 # ChromeOS > Security > Minijail +} + +monorail { + component: "OS>Systems>Minijail" +} + +team_email: "minijail@chromium.org" @@ -10,6 +10,9 @@ PRELOADNAME = libminijailpreload.so PRELOADPATH = "$(LIBDIR)/$(PRELOADNAME)" CPPFLAGS += -DPRELOADPATH='$(PRELOADPATH)' +# We don't build static libs by default. +BUILD_STATIC_LIBS ?= no + # Defines the pivot root path used by the minimalistic-mountns profile. DEFAULT_PIVOT_ROOT ?= /var/empty CPPFLAGS += -DDEFAULT_PIVOT_ROOT='"$(DEFAULT_PIVOT_ROOT)"' @@ -18,10 +21,23 @@ ifeq ($(USE_seccomp),no) CPPFLAGS += -DUSE_SECCOMP_SOFTFAIL endif +BLOCK_NOEXEC_CONF ?= no +ifeq ($(BLOCK_NOEXEC_CONF),yes) +CPPFLAGS += -DBLOCK_NOEXEC_CONF +endif + +ENFORCE_ROOTFS_CONF ?= no +ifeq ($(ENFORCE_ROOTFS_CONF),yes) +CPPFLAGS += -DENFORCE_ROOTFS_CONF +endif + # Allow people to use -L and related flags. ALLOW_DEBUG_LOGGING ?= yes ifeq ($(ALLOW_DEBUG_LOGGING),yes) CPPFLAGS += -DALLOW_DEBUG_LOGGING +ifeq ($(SECCOMP_DEFAULT_RET_LOG),yes) +CPPFLAGS += -DSECCOMP_DEFAULT_RET_LOG +endif endif ifeq ($(USE_ASAN),yes) @@ -46,20 +62,27 @@ MJ_COMMON_FLAGS = -Wunused-parameter -Wextra -Wno-missing-field-initializers CFLAGS += $(MJ_COMMON_FLAGS) CXXFLAGS += $(MJ_COMMON_FLAGS) +# Dependencies that all gtest based unittests should have. +UNITTEST_LIBS := -lcap +UNITTEST_DEPS := testrunner.o test_util.o + USE_SYSTEM_GTEST ?= no ifeq ($(USE_SYSTEM_GTEST),no) GTEST_CXXFLAGS := -std=gnu++14 GTEST_LIBS := gtest.a +UNITTEST_DEPS += $(GTEST_LIBS) else GTEST_CXXFLAGS := $(shell gtest-config --cxxflags 2>/dev/null || \ echo "-pthread") GTEST_LIBS := $(shell gtest-config --libs 2>/dev/null || \ echo "-lgtest -pthread -lpthread") endif +UNITTEST_LIBS += $(GTEST_LIBS) CORE_OBJECT_FILES := libminijail.o syscall_filter.o signal_handler.o \ bpf.o util.o system.o syscall_wrapper.o \ - libconstants.gen.o libsyscalls.gen.o + config_parser.o libconstants.gen.o libsyscalls.gen.o +UNITTEST_DEPS += $(CORE_OBJECT_FILES) all: CC_BINARY(minijail0) CC_LIBRARY(libminijail.so) \ CC_LIBRARY(libminijailpreload.so) @@ -72,7 +95,7 @@ tests: TEST(CXX_BINARY(libminijail_unittest)) \ TEST(CXX_BINARY(syscall_filter_unittest)) \ TEST(CXX_BINARY(system_unittest)) \ TEST(CXX_BINARY(util_unittest)) \ - + TEST(CXX_BINARY(config_parser_unittest)) CC_BINARY(minijail0): LDLIBS += -lcap -ldl CC_BINARY(minijail0): $(CORE_OBJECT_FILES) \ @@ -88,14 +111,14 @@ CC_STATIC_LIBRARY(libminijail.pic.a): $(CORE_OBJECT_FILES) CC_STATIC_LIBRARY(libminijail.pie.a): $(CORE_OBJECT_FILES) clean: CLEAN(libminijail.*.a) +ifeq ($(BUILD_STATIC_LIBS),yes) +all: CC_STATIC_LIBRARY(libminijail.pic.a) CC_STATIC_LIBRARY(libminijail.pie.a) +endif + CXX_BINARY(libminijail_unittest): CXXFLAGS += -Wno-write-strings \ $(GTEST_CXXFLAGS) -CXX_BINARY(libminijail_unittest): LDLIBS += -lcap $(GTEST_LIBS) -ifeq ($(USE_SYSTEM_GTEST),no) -CXX_BINARY(libminijail_unittest): $(GTEST_LIBS) -endif -CXX_BINARY(libminijail_unittest): libminijail_unittest.o $(CORE_OBJECT_FILES) \ - testrunner.o +CXX_BINARY(libminijail_unittest): LDLIBS += $(UNITTEST_LIBS) +CXX_BINARY(libminijail_unittest): $(UNITTEST_DEPS) libminijail_unittest.o clean: CLEAN(libminijail_unittest) TEST(CXX_BINARY(libminijail_unittest)): CC_LIBRARY(libminijailpreload.so) @@ -107,43 +130,33 @@ clean: CLEAN(libminijailpreload.so) CXX_BINARY(minijail0_cli_unittest): CXXFLAGS += $(GTEST_CXXFLAGS) -CXX_BINARY(minijail0_cli_unittest): LDLIBS += -lcap $(GTEST_LIBS) -ifeq ($(USE_SYSTEM_GTEST),no) -CXX_BINARY(minijail0_cli_unittest): $(GTEST_LIBS) -endif -CXX_BINARY(minijail0_cli_unittest): minijail0_cli_unittest.o \ - $(CORE_OBJECT_FILES) minijail0_cli.o elfparse.o testrunner.o +CXX_BINARY(minijail0_cli_unittest): LDLIBS += $(UNITTEST_LIBS) +CXX_BINARY(minijail0_cli_unittest): $(UNITTEST_DEPS) minijail0_cli_unittest.o \ + minijail0_cli.o elfparse.o clean: CLEAN(minijail0_cli_unittest) +CXX_BINARY(config_parser_unittest): CXXFLAGS += $(GTEST_CXXFLAGS) +CXX_BINARY(config_parser_unittest): LDLIBS += $(UNITTEST_LIBS) +CXX_BINARY(config_parser_unittest): $(UNITTEST_DEPS) config_parser_unittest.o +clean: CLEAN(config_parser_unittest) + CXX_BINARY(syscall_filter_unittest): CXXFLAGS += -Wno-write-strings \ $(GTEST_CXXFLAGS) -CXX_BINARY(syscall_filter_unittest): LDLIBS += -lcap $(GTEST_LIBS) -ifeq ($(USE_SYSTEM_GTEST),no) -CXX_BINARY(syscall_filter_unittest): $(GTEST_LIBS) -endif -CXX_BINARY(syscall_filter_unittest): syscall_filter_unittest.o \ - $(CORE_OBJECT_FILES) testrunner.o +CXX_BINARY(syscall_filter_unittest): LDLIBS += $(UNITTEST_LIBS) +CXX_BINARY(syscall_filter_unittest): $(UNITTEST_DEPS) syscall_filter_unittest.o clean: CLEAN(syscall_filter_unittest) CXX_BINARY(system_unittest): CXXFLAGS += $(GTEST_CXXFLAGS) -CXX_BINARY(system_unittest): LDLIBS += -lcap $(GTEST_LIBS) -ifeq ($(USE_SYSTEM_GTEST),no) -CXX_BINARY(system_unittest): $(GTEST_LIBS) -endif -CXX_BINARY(system_unittest): system_unittest.o \ - $(CORE_OBJECT_FILES) testrunner.o +CXX_BINARY(system_unittest): LDLIBS += $(UNITTEST_LIBS) +CXX_BINARY(system_unittest): $(UNITTEST_DEPS) system_unittest.o clean: CLEAN(system_unittest) CXX_BINARY(util_unittest): CXXFLAGS += $(GTEST_CXXFLAGS) -CXX_BINARY(util_unittest): LDLIBS += -lcap $(GTEST_LIBS) -ifeq ($(USE_SYSTEM_GTEST),no) -CXX_BINARY(util_unittest): $(GTEST_LIBS) -endif -CXX_BINARY(util_unittest): util_unittest.o \ - $(CORE_OBJECT_FILES) testrunner.o +CXX_BINARY(util_unittest): LDLIBS += $(UNITTEST_LIBS) +CXX_BINARY(util_unittest): $(UNITTEST_DEPS) util_unittest.o clean: CLEAN(util_unittest) @@ -170,10 +183,9 @@ libsyscalls.gen.o.depends: libsyscalls.gen.c # Only regenerate libsyscalls.gen.c if the Makefile or header changes. # NOTE! This will not detect if the file is not appropriate for the target. -libsyscalls.gen.c: $(SRC)/Makefile $(SRC)/libsyscalls.h - @printf "Generating target-arch specific $@...\n" +libsyscalls.gen.c: $(SRC)/libsyscalls.h $(SRC)/Makefile + @/bin/echo -e "GEN $(subst $(SRC)/,,$<) -> $@" $(QUIET)CC="$(CC)" $(SRC)/gen_syscalls.sh "$@" - @printf "$@ done.\n" clean: CLEAN(libsyscalls.gen.c) $(eval $(call add_object_rules,libsyscalls.gen.o,CC,c,CFLAGS)) @@ -184,10 +196,9 @@ libconstants.gen.o.depends: libconstants.gen.c # Only regenerate libconstants.gen.c if the Makefile or header changes. # NOTE! This will not detect if the file is not appropriate for the target. -libconstants.gen.c: $(SRC)/Makefile $(SRC)/libconstants.h - @printf "Generating target-arch specific $@...\n" +libconstants.gen.c: $(SRC)/libconstants.h $(SRC)/Makefile + @/bin/echo -e "GEN $(subst $(SRC)/,,$<) -> $@" $(QUIET)CC="$(CC)" $(SRC)/gen_constants.sh "$@" - @printf "$@ done.\n" clean: CLEAN(libconstants.gen.c) $(eval $(call add_object_rules,libconstants.gen.o,CC,c,CFLAGS)) @@ -199,7 +210,7 @@ $(eval $(call add_object_rules,libconstants.gen.o,CC,c,CFLAGS)) ifeq ($(USE_SYSTEM_GTEST),no) # Points to the root of Google Test, relative to where this file is. # Remember to tweak this if you move this file. -GTEST_DIR = googletest-release-1.10.0/googletest +GTEST_DIR = googletest-release-1.11.0/googletest # Flags passed to the preprocessor. # Set Google Test's header directory as a system directory, such that @@ -218,7 +229,7 @@ GTEST_HEADERS = $(GTEST_DIR)/include/gtest/*.h \ clean: clean_gtest clean_gtest: - rm -f gtest.a gtest_main.a *.o + $(QUIET)rm -f gtest.a gtest_main.a *.o # Builds gtest.a and gtest_main.a. @@ -1,5 +1,5 @@ set noparent -jorgelo@google.com +include OWNERS_GENERAL +# Emeritus. drewry@google.com keescook@google.com -vapier@google.com diff --git a/OWNERS_GENERAL b/OWNERS_GENERAL new file mode 100644 index 0000000..e5179ef --- /dev/null +++ b/OWNERS_GENERAL @@ -0,0 +1,3 @@ +allenwebb@google.com +jorgelo@google.com +vapier@google.com diff --git a/PRESUBMIT.cfg b/PRESUBMIT.cfg index d76d432..a7ce524 100644 --- a/PRESUBMIT.cfg +++ b/PRESUBMIT.cfg @@ -6,3 +6,6 @@ tab_check: false # This is an AOSP project, but we still use a BSD license. cros_license_check: true aosp_license_check: false + +[Hook Overrides Options] +cros_license_check: --exclude_regex=^test/ @@ -114,3 +114,51 @@ controlled surprise system call use. https://crrev.com/c/4585/ added the original implementation. Source: Conversations with original authors, ellyjones@ and wad@. + +## How to manually upgrade Minijail on Chrome OS + +Minijail is manually upgraded on Chrome OS so that there is a way to test +changes in the Chrome OS commit queue. Committed changes have already passed +Android's presubmit checks, but the ebuild upgrade CL goes through the Chrome +OS commit queue and must pass the tests before any additional changes are +available for use on Chrome OS. To upgrade minijail on Chrome OS, complete the +following steps. + +```bash +# Sync Minijail repo +cd ~/chromiumos/src/aosp/external/minijail +git checkout m/main +repo sync . + +# Set up local branch. +cd ~/trunk/src/third_party/chromiumos-overlay/ +repo start minijail . # replace minijail with the local branch name you want. + +# Run upgrade script. +~/trunk/chromite/scripts/cros_uprev --force --overlay-type public \ + --packages chromeos-base/minijail:dev-rust/minijail-sys:dev-rust/minijail +``` + +At this point the Minijail-related packages should be upgraded, so you may want +to add the changes to a commit and do some local testing before uploading a +change list. Here are the recommended local tests to try (make sure you are +**not** working on the minijail packages first i.e. `cros_workon list-all`): + +```bash +# Check build. +./build_packages --board=${BOARD} + +# Check unit tests. +FEATURES=test emerge-${BOARD} chromeos-base/minijail dev-rust/minijail-sys \ + dev-rust/minijail + +# Check integration tests. +cros deploy <DUT> chromeos-base/minijail +tast run <DUT> security.Minijail.* security.MinijailSeccomp +``` + +Finally, when uploading the CL make sure to include the list of changes +since the last uprev. The command to generate the list is as follows: +```bash +git log --oneline --no-merges <previous hash in ebuild file>..HEAD +``` diff --git a/TEST_MAPPING b/TEST_MAPPING index bdded19..539379e 100644 --- a/TEST_MAPPING +++ b/TEST_MAPPING @@ -12,5 +12,19 @@ { "name": "syscall_filter_unittest_gtest" } + ], + "hwasan-postsubmit": [ + { + "name": "libminijail_unittest_gtest" + }, + { + "name": "mj_system_unittest_gtest" + }, + { + "name": "mj_util_unittest_gtest" + }, + { + "name": "syscall_filter_unittest_gtest" + } ] } @@ -62,6 +62,17 @@ #elif defined(__powerpc__) # define MINIJAIL_ARCH_NR AUDIT_ARCH_PPC # define MINIJAIL_ARCH_NAME "ppc" +#elif defined(__riscv) +# if defined(__riscv_xlen) +# if (__riscv_xlen == 64) +# define MINIJAIL_ARCH_NR AUDIT_ARCH_RISCV64 +# define MINIJAIL_ARCH_NAME "riscv64" +# else +# error "Only 64bit riscv is supported" +# endif +# else +# error "AUDIT_ARCH value unavailable" +# endif #elif defined(__s390x__) # define MINIJAIL_ARCH_NR AUDIT_ARCH_S390X # define MINIJAIL_ARCH_NAME "s390x" diff --git a/config_parser.c b/config_parser.c new file mode 100644 index 0000000..4972cf5 --- /dev/null +++ b/config_parser.c @@ -0,0 +1,148 @@ +/* Copyright 2021 The Chromium OS Authors. All rights reserved. + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include <errno.h> +#include <stdbool.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> + +#include "config_parser.h" + +#include "util.h" + +#define LIST_DEFAULT_SIZE (100) + +struct config_entry_list *new_config_entry_list(void) +{ + /* + * There are <100 CLI options, configuration file will likely have + * a similar number of config entries. + */ + struct config_entry *entries = + calloc(LIST_DEFAULT_SIZE, sizeof(struct config_entry)); + if (!entries) + return NULL; + + struct config_entry_list *list = + calloc(1, sizeof(struct config_entry_list)); + if (!list) { + free(entries); + return NULL; + } + list->entries = entries; + list->num_allocated_ = LIST_DEFAULT_SIZE; + list->num_entries = 0; + return list; +} + +void clear_config_entry(struct config_entry *entry) +{ + free((char *)entry->key); + free((char *)entry->value); +} + +void free_config_entry_list(struct config_entry_list *list) +{ + if (!list) + return; + for (size_t i = 0; i < list->num_entries; i++) { + clear_config_entry(&list->entries[i]); + } + free(list->entries); + free(list); +} + +bool parse_config_line(const char *config_line, struct config_entry *entry) +{ + /* Parsing will modify |config_line| in place, so make a copy. */ + attribute_cleanup_str char *line = strdup(config_line); + if (!line) + return false; + char *value = line; + + /* After tokenize call, |value| will point to a substring after '='. + * If there is no '=' in the string, |key| will contain the entire + * string while |value| will be NULL. + */ + char *key = tokenize(&value, "="); + if (key) + key = strip(key); + if (value) + value = strip(value); + if (!key || key[0] == '\0' || (value && value[0] == '\0')) { + warn("unable to parse %s", config_line); + return false; + } + entry->key = strdup(key); + entry->value = value ? strdup(value) : NULL; + if (!entry->key || (value && !entry->value)) { + clear_config_entry(entry); + return false; + } + return true; +} + +static bool match_special_directive(const char *line) +{ + return (strcmp(line, "% minijail-config-file v0\n") == 0); +} + +bool parse_config_file(FILE *config_file, struct config_entry_list *list) +{ + attribute_cleanup_str char *line = NULL; + size_t len = 0; + + /* The first line must match the special directive */ + if (getline(&line, &len, config_file) == -1 || + !match_special_directive(line)) + return false; + while (getmultiline(&line, &len, config_file) != -1) { + char *stripped_line = strip(line); + /* + * Skip blank lines and all comments. Comment lines start with + * '#'. + */ + if (stripped_line[0] == '\0' || stripped_line[0] == '#') + continue; + + /* + * Check if the list is full, and reallocate with doubled + * capacity if so. + */ + if (list->num_entries >= list->num_allocated_) { + list->num_allocated_ = list->num_allocated_ * 2; + list->entries = realloc( + list->entries, + list->num_allocated_ * sizeof(struct config_entry)); + if (list->entries == NULL) { + return false; + } + } + + struct config_entry *entry = &list->entries[list->num_entries]; + if (!parse_config_line(stripped_line, entry)) { + return false; + } + ++list->num_entries; + } + /* + * getmultiline() behaves similarly with getline(3). It returns -1 + * when read into EOF or the following errors. + */ + if (errno == EINVAL || errno == ENOMEM) { + return false; + } + + /* Shrink the list to save memory. */ + if (list->num_entries < list->num_allocated_) { + list->entries = + realloc(list->entries, + list->num_entries * sizeof(struct config_entry)); + list->num_allocated_ = list->num_entries; + } + + return true; +} diff --git a/config_parser.h b/config_parser.h new file mode 100644 index 0000000..36c96db --- /dev/null +++ b/config_parser.h @@ -0,0 +1,55 @@ +/* Copyright 2021 The Chromium OS Authors. All rights reserved. + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#ifndef CONFIG_PARSER_H +#define CONFIG_PARSER_H + +#include <stdbool.h> +#include <stddef.h> +#include <stdio.h> + +#ifdef __cplusplus +extern "C" { +#endif + +struct config_entry { + const char *key; + const char *value; +}; + +struct config_entry_list { + struct config_entry *entries; + size_t num_entries; + size_t num_allocated_; +}; + +/* Allocate a new |config_entry_list| struct. */ +struct config_entry_list *new_config_entry_list(void); + +/* Free allocated pointers in |config_entry|. */ +void clear_config_entry(struct config_entry *entry); + +/* Free a |config_entry_list| struct. */ +void free_config_entry_list(struct config_entry_list *list); + +/* + * Parse one config line into a entry. + * + * Returns true for success, otherwise false for parsing failures. + */ +bool parse_config_line(const char *config_line, struct config_entry *entry); + +/* + * Parse a minijail config file into a |config_entry_list|. + * + * Returns true for success, otherwise false for parsing failures. + */ +bool parse_config_file(FILE *config_file, struct config_entry_list *list); + +#ifdef __cplusplus +}; /* extern "C" */ +#endif + +#endif /* CONFIG_PARSER_H */ diff --git a/config_parser_unittest.cc b/config_parser_unittest.cc new file mode 100644 index 0000000..a9c6571 --- /dev/null +++ b/config_parser_unittest.cc @@ -0,0 +1,132 @@ +/* Copyright 2021 The Chromium OS Authors. All rights reserved. + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + * + * Test config_parser.c using gtest. + */ + +#include <gtest/gtest.h> +#include <string> + +#include "config_parser.h" +#include "test_util.h" +#include "util.h" + +namespace { + +class ConfigFileTest : public ::testing::Test { +protected: + virtual void SetUp() { + list_ = new_config_entry_list(); + ASSERT_NE(list_, nullptr); + } + virtual void TearDown() { free_config_entry_list(list_); } + struct config_entry_list *list_; +}; + +} // namespace + +TEST(ParsingConfigTest, valid_config_line) { + ScopedConfigEntry entry( + (config_entry *)calloc(1, sizeof(struct config_entry))); + const std::vector<std::string> valid_conf_lines = { + "mount=none", + "valueless_key" + "binding = none", + " xyz = abc ", + }; + + for (const auto& conf_line : valid_conf_lines) { + ASSERT_TRUE(parse_config_line(conf_line.c_str(), entry.get())); + clear_config_entry(entry.get()); + } +} + +TEST(ParsingConfigTest, invalid_config_line) { + ScopedConfigEntry entry( + (config_entry *)calloc(1, sizeof(struct config_entry))); + const std::vector<std::string> invalid_conf_lines = { + "= none", + "", + "empty_arg=", + "empty_arg= ", + }; + + for (const auto& conf_line : invalid_conf_lines) { + ASSERT_FALSE(parse_config_line(conf_line.c_str(), entry.get())); + } +} + +TEST_F(ConfigFileTest, malformed_config_line) { + std::string config = "% minijail-config-file v0\n" + "=malformed"; + ScopedFILE config_file(write_to_pipe(config)); + ASSERT_NE(config_file.get(), nullptr); + + bool res = parse_config_file(config_file.get(), list_); + + // Policy is malformed, but process should not crash. + ASSERT_FALSE(res); + ASSERT_EQ(list_->num_entries, 0); +} + +TEST_F(ConfigFileTest, bad_directive) { + std::string config = "% bad-directive\n" + "# comments"; + ScopedFILE config_file(write_to_pipe(config)); + ASSERT_NE(config_file.get(), nullptr); + + bool res = parse_config_file(config_file.get(), list_); + + // Policy is malformed, but process should not crash. + ASSERT_FALSE(res); + ASSERT_EQ(list_->num_entries, 0); +} + +TEST_F(ConfigFileTest, wellformed_single_line) { + std::string config = "% minijail-config-file v0\n" + "# Comments \n" + "\n" + "uts\n" + "mount= xyz\n" + "binding = none,/tmp"; + ScopedFILE config_file(write_to_pipe(config)); + ASSERT_NE(config_file.get(), nullptr); + + bool res = parse_config_file(config_file.get(), list_); + + ASSERT_TRUE(res); + ASSERT_EQ(list_->num_entries, 3); + struct config_entry *first_entry = list_->entries; + struct config_entry *second_entry = list_->entries + 1; + struct config_entry *third_entry = list_->entries + 2; + ASSERT_EQ(std::string(first_entry->key), "uts"); + ASSERT_EQ(first_entry->value, nullptr); + ASSERT_EQ(std::string(second_entry->key), "mount"); + ASSERT_EQ(std::string(second_entry->value), "xyz"); + ASSERT_EQ(std::string(third_entry->key), "binding"); + ASSERT_EQ(std::string(third_entry->value), "none,/tmp"); +} + +TEST_F(ConfigFileTest, wellformed_multi_line) { + std::string config = "% minijail-config-file v0\n" + "# Comments \n" + "\n" + "mount = \\\n" + "none\n" + "binding = none,\\\n" + "/tmp"; + ScopedFILE config_file(write_to_pipe(config)); + ASSERT_NE(config_file.get(), nullptr); + + int res = parse_config_file(config_file.get(), list_); + + ASSERT_TRUE(res); + ASSERT_EQ(list_->num_entries, 2); + struct config_entry *first_entry = list_->entries; + struct config_entry *second_entry = list_->entries + 1; + ASSERT_EQ(std::string(first_entry->key), "mount"); + ASSERT_EQ(std::string(first_entry->value), "none"); + ASSERT_EQ(std::string(second_entry->key), "binding"); + ASSERT_EQ(std::string(second_entry->value), "none, /tmp"); +} @@ -11,6 +11,7 @@ #include <unistd.h> #include "elfparse.h" +#include "util.h" int is_elf_magic (const uint8_t *buf) { @@ -67,7 +68,7 @@ parseElftemplate(32) ElfType get_elf_linkage(const char *path) { ElfType ret = ELFERROR; - FILE *elf_file = NULL; + attribute_cleanup_fp FILE *elf_file = NULL; uint8_t pHeader[HEADERSIZE] = ""; elf_file = fopen(path, "re"); @@ -112,7 +113,6 @@ ElfType get_elf_linkage(const char *path) */ ret = ELFDYNAMIC; } - fclose(elf_file); } return ret; } diff --git a/gen_syscalls-inl.h b/gen_syscalls-inl.h index 2004f7a..d7c43aa 100644 --- a/gen_syscalls-inl.h +++ b/gen_syscalls-inl.h @@ -73,3 +73,7 @@ #ifndef __NR_io_uring_setup #define __NR_io_uring_setup 425 #endif + +#ifndef __NR_faccessat2 +#define __NR_faccessat2 439 +#endif diff --git a/get_googletest.sh b/get_googletest.sh index b379441..07910b4 100755 --- a/get_googletest.sh +++ b/get_googletest.sh @@ -1,6 +1,6 @@ #/bin/bash -PV="1.10.0" +PV="1.11.0" wget -q -nc --secure-protocol=TLSv1 "https://github.com/google/googletest/archive/release-${PV}.tar.gz" -O "googletest-release-${PV}.tar.gz" tar zxvf "googletest-release-${PV}.tar.gz" diff --git a/libminijail-private.h b/libminijail-private.h index 503f09b..8feec55 100644 --- a/libminijail-private.h +++ b/libminijail-private.h @@ -19,8 +19,9 @@ extern "C" { */ #define API __attribute__((__visibility__("default"))) -static const char *kFdEnvVar = "__MINIJAIL_FD"; -static const char *kLdPreloadEnvVar = "LD_PRELOAD"; +static const char kFdEnvVar[] = "__MINIJAIL_FD"; +static const char kLdPreloadEnvVar[] = "LD_PRELOAD"; +static const char kSeccompPolicyPathEnvVar[] = "SECCOMP_POLICY_PATH"; struct minijail; @@ -46,9 +47,7 @@ extern size_t minijail_size(const struct minijail *j); * The marshalled data is not robust to differences between the child * and parent process (personality, etc). */ -extern int minijail_marshal(const struct minijail *j, - char *buf, - size_t size); +extern int minijail_marshal(const struct minijail *j, char *buf, size_t size); /* minijail_unmarshal: initializes @j from @serialized * @j minijail to initialize @@ -57,9 +56,8 @@ extern int minijail_marshal(const struct minijail *j, * * Returns 0 on success. */ -extern int minijail_unmarshal(struct minijail *j, - char *serialized, - size_t length); +extern int minijail_unmarshal(struct minijail *j, char *serialized, + size_t length); /* minijail_from_fd: builds @j from @fd * @j minijail to initialize diff --git a/libminijail.c b/libminijail.c index 0820dbb..aab1294 100644 --- a/libminijail.c +++ b/libminijail.c @@ -36,8 +36,8 @@ #include <syscall.h> #include <unistd.h> -#include "libminijail.h" #include "libminijail-private.h" +#include "libminijail.h" #include "signal_handler.h" #include "syscall_filter.h" @@ -47,19 +47,19 @@ /* Until these are reliably available in linux/prctl.h. */ #ifndef PR_ALT_SYSCALL -# define PR_ALT_SYSCALL 0x43724f53 +#define PR_ALT_SYSCALL 0x43724f53 #endif /* New cgroup namespace might not be in linux-headers yet. */ #ifndef CLONE_NEWCGROUP -# define CLONE_NEWCGROUP 0x02000000 +#define CLONE_NEWCGROUP 0x02000000 #endif #define MAX_CGROUPS 10 /* 10 different controllers supported by Linux. */ #define MAX_RLIMITS 32 /* Currently there are 15 supported by Linux. */ -#define MAX_PRESERVED_FDS 32U +#define MAX_PRESERVED_FDS 128U /* Keyctl commands. */ #define KEYCTL_JOIN_SESSION_KEYRING 1 @@ -189,11 +189,17 @@ struct minijail { struct hook *hooks_tail; struct preserved_fd preserved_fds[MAX_PRESERVED_FDS]; size_t preserved_fd_count; + char *seccomp_policy_path; }; static void run_hooks_or_die(const struct minijail *j, minijail_hook_event_t event); +static bool seccomp_is_logging_allowed(const struct minijail *j) +{ + return seccomp_default_ret_log() || j->flags.seccomp_filter_logging; +} + static void free_mounts_list(struct minijail *j) { while (j->mounts_head) { @@ -368,7 +374,8 @@ void API minijail_set_supplementary_gids(struct minijail *j, size_t size, j->flags.set_suppl_gids = 1; } -void API minijail_keep_supplementary_gids(struct minijail *j) { +void API minijail_keep_supplementary_gids(struct minijail *j) +{ j->flags.keep_suppl_gids = 1; } @@ -419,7 +426,7 @@ void API minijail_set_seccomp_filter_tsync(struct minijail *j) "before minijail_parse_seccomp_filters()"); } - if (j->flags.seccomp_filter_logging && !seccomp_ret_log_available()) { + if (seccomp_is_logging_allowed(j) && !seccomp_ret_log_available()) { /* * If SECCOMP_RET_LOG is not available, we don't want to use * SECCOMP_RET_TRAP to both kill the entire process and report @@ -455,8 +462,8 @@ void API minijail_log_seccomp_filter_failures(struct minijail *j) * SECCOMP_RET_TRAP to both kill the entire process and report * failing syscalls, since it will be brittle. Just bail. */ - die("SECCOMP_RET_LOG not available, cannot use thread sync with " - "logging at the same time"); + die("SECCOMP_RET_LOG not available, cannot use thread sync " + "with logging at the same time"); } if (debug_logging_allowed()) { @@ -723,7 +730,7 @@ char API *minijail_get_original_path(struct minijail *j, */ if (!strncmp(b->dest, path_inside_chroot, strlen(b->dest))) { const char *relative_path = - path_inside_chroot + strlen(b->dest); + path_inside_chroot + strlen(b->dest); return path_join(b->src, relative_path); } b = b->next; @@ -799,7 +806,8 @@ int API minijail_forward_signals(struct minijail *j) return 0; } -int API minijail_create_session(struct minijail *j) { +int API minijail_create_session(struct minijail *j) +{ j->flags.setsid = 1; return 0; } @@ -977,6 +985,10 @@ static void clear_seccomp_options(struct minijail *j) j->filter_len = 0; j->filter_prog = NULL; j->flags.no_new_privs = 0; + if (j->seccomp_policy_path) { + free(j->seccomp_policy_path); + } + j->seccomp_policy_path = NULL; } static int seccomp_should_use_filters(struct minijail *j) @@ -1086,7 +1098,7 @@ static int parse_seccomp_filters(struct minijail *j, const char *filename, * Allow logging? */ filteropts.allow_logging = - debug_logging_allowed() && j->flags.seccomp_filter_logging; + debug_logging_allowed() && seccomp_is_logging_allowed(j); /* What to do on a blocked system call? */ if (filteropts.allow_logging) { @@ -1129,7 +1141,7 @@ void API minijail_parse_seccomp_filters(struct minijail *j, const char *path) if (!seccomp_should_use_filters(j)) return; - FILE *file = fopen(path, "re"); + attribute_cleanup_fp FILE *file = fopen(path, "re"); if (!file) { pdie("failed to open seccomp filter file '%s'", path); } @@ -1138,13 +1150,16 @@ void API minijail_parse_seccomp_filters(struct minijail *j, const char *path) die("failed to compile seccomp filter BPF program in '%s'", path); } - fclose(file); + if (j->seccomp_policy_path) { + free(j->seccomp_policy_path); + } + j->seccomp_policy_path = strdup(path); } void API minijail_parse_seccomp_filters_from_fd(struct minijail *j, int fd) { char *fd_path, *path; - FILE *file; + attribute_cleanup_fp FILE *file = NULL; if (!seccomp_should_use_filters(j)) return; @@ -1165,8 +1180,10 @@ void API minijail_parse_seccomp_filters_from_fd(struct minijail *j, int fd) die("failed to compile seccomp filter BPF program from fd %d", fd); } - free(path); - fclose(file); + if (j->seccomp_policy_path) { + free(j->seccomp_policy_path); + } + j->seccomp_policy_path = path; } void API minijail_set_seccomp_filters(struct minijail *j, @@ -1175,7 +1192,7 @@ void API minijail_set_seccomp_filters(struct minijail *j, if (!seccomp_should_use_filters(j)) return; - if (j->flags.seccomp_filter_logging) { + if (seccomp_is_logging_allowed(j)) { die("minijail_log_seccomp_filter_failures() is incompatible " "with minijail_set_seccomp_filters()"); } @@ -1276,6 +1293,8 @@ static void minijail_marshal_helper(struct marshal_state *state, } for (i = 0; i < j->cgroup_count; ++i) marshal_append_string(state, j->cgroups[i]); + if (j->seccomp_policy_path) + marshal_append_string(state, j->seccomp_policy_path); } size_t API minijail_size(const struct minijail *j) @@ -1319,7 +1338,7 @@ int minijail_unmarshal(struct minijail *j, char *serialized, size_t length) j->hooks_head = NULL; j->hooks_tail = NULL; - if (j->user) { /* stale pointer */ + if (j->user) { /* stale pointer */ char *user = consumestr(&serialized, &length); if (!user) goto clear_pointers; @@ -1328,7 +1347,7 @@ int minijail_unmarshal(struct minijail *j, char *serialized, size_t length) goto clear_pointers; } - if (j->suppl_gid_list) { /* stale pointer */ + if (j->suppl_gid_list) { /* stale pointer */ if (j->suppl_gid_count > NGROUPS_MAX) { goto bad_gid_list; } @@ -1345,7 +1364,7 @@ int minijail_unmarshal(struct minijail *j, char *serialized, size_t length) memcpy(j->suppl_gid_list, gid_list_bytes, gid_list_size); } - if (j->chrootdir) { /* stale pointer */ + if (j->chrootdir) { /* stale pointer */ char *chrootdir = consumestr(&serialized, &length); if (!chrootdir) goto bad_chrootdir; @@ -1354,7 +1373,7 @@ int minijail_unmarshal(struct minijail *j, char *serialized, size_t length) goto bad_chrootdir; } - if (j->hostname) { /* stale pointer */ + if (j->hostname) { /* stale pointer */ char *hostname = consumestr(&serialized, &length); if (!hostname) goto bad_hostname; @@ -1363,7 +1382,7 @@ int minijail_unmarshal(struct minijail *j, char *serialized, size_t length) goto bad_hostname; } - if (j->alt_syscall_table) { /* stale pointer */ + if (j->alt_syscall_table) { /* stale pointer */ char *alt_syscall_table = consumestr(&serialized, &length); if (!alt_syscall_table) goto bad_syscall_table; @@ -1412,8 +1431,8 @@ int minijail_unmarshal(struct minijail *j, char *serialized, size_t length) type = consumestr(&serialized, &length); if (!type) goto bad_mounts; - has_data = consumebytes(sizeof(*has_data), &serialized, - &length); + has_data = + consumebytes(sizeof(*has_data), &serialized, &length); if (!has_data) goto bad_mounts; if (*has_data) { @@ -1440,8 +1459,23 @@ int minijail_unmarshal(struct minijail *j, char *serialized, size_t length) ++j->cgroup_count; } + if (j->seccomp_policy_path) { /* stale pointer */ + char *seccomp_policy_path = consumestr(&serialized, &length); + if (!seccomp_policy_path) + goto bad_cgroups; + j->seccomp_policy_path = strdup(seccomp_policy_path); + if (!j->seccomp_policy_path) + goto bad_cgroups; + } + return 0; + /* + * If more is added after j->seccomp_policy_path, then this is needed: + * if (j->seccomp_policy_path) + * free(j->seccomp_policy_path); + */ + bad_cgroups: free_mounts_list(j); free_remounts_list(j); @@ -1457,12 +1491,12 @@ bad_filters: if (j->alt_syscall_table) free(j->alt_syscall_table); bad_syscall_table: - if (j->chrootdir) - free(j->chrootdir); -bad_chrootdir: if (j->hostname) free(j->hostname); bad_hostname: + if (j->chrootdir) + free(j->chrootdir); +bad_chrootdir: if (j->suppl_gid_list) free(j->suppl_gid_list); bad_gid_list: @@ -1475,6 +1509,7 @@ clear_pointers: j->hostname = NULL; j->alt_syscall_table = NULL; j->cgroup_count = 0; + j->seccomp_policy_path = NULL; out: return ret; } @@ -1485,39 +1520,56 @@ struct dev_spec { dev_t major, minor; }; +// clang-format off static const struct dev_spec device_nodes[] = { - { - "null", - S_IFCHR | 0666, 1, 3, - }, - { - "zero", - S_IFCHR | 0666, 1, 5, - }, - { - "full", - S_IFCHR | 0666, 1, 7, - }, - { - "urandom", - S_IFCHR | 0444, 1, 9, - }, - { - "tty", - S_IFCHR | 0666, 5, 0, - }, + { +"null", + S_IFCHR | 0666, 1, 3, + }, + { + "zero", + S_IFCHR | 0666, 1, 5, + }, + { + "full", + S_IFCHR | 0666, 1, 7, + }, + { + "urandom", + S_IFCHR | 0444, 1, 9, + }, + { + "tty", + S_IFCHR | 0666, 5, 0, + }, }; +// clang-format on struct dev_sym_spec { const char *source, *dest; }; static const struct dev_sym_spec device_symlinks[] = { - { "ptmx", "pts/ptmx", }, - { "fd", "/proc/self/fd", }, - { "stdin", "fd/0", }, - { "stdout", "fd/1", }, - { "stderr", "fd/2", }, + { + "ptmx", + "pts/ptmx", + }, + { + "fd", + "/proc/self/fd", + }, + { + "stdin", + "fd/0", + }, + { + "stdout", + "fd/1", + }, + { + "stderr", + "fd/2", + }, }; /* @@ -1538,7 +1590,7 @@ static void mount_dev_cleanup(char *dev_path) static int mount_dev(char **dev_path_ret) { int ret; - int dev_fd; + attribute_cleanup_fd int dev_fd = -1; size_t i; mode_t mask; char *dev_path; @@ -1552,8 +1604,8 @@ static int mount_dev(char **dev_path_ret) pdie("could not create temp path for /dev"); /* Set up the empty /dev mount point first. */ - ret = mount("minijail-devfs", dev_path, "tmpfs", - MS_NOEXEC | MS_NOSUID, "size=5M,mode=755"); + ret = mount("minijail-devfs", dev_path, "tmpfs", MS_NOEXEC | MS_NOSUID, + "size=5M,mode=755"); if (ret) { rmdir(dev_path); return ret; @@ -1563,7 +1615,7 @@ static int mount_dev(char **dev_path_ret) mask = umask(0); /* Get a handle to the temp dev path for *at funcs below. */ - dev_fd = open(dev_path, O_DIRECTORY|O_PATH|O_CLOEXEC); + dev_fd = open(dev_path, O_DIRECTORY | O_PATH | O_CLOEXEC); if (dev_fd < 0) { ret = 1; goto done; @@ -1573,7 +1625,7 @@ static int mount_dev(char **dev_path_ret) for (i = 0; i < ARRAY_SIZE(device_nodes); ++i) { const struct dev_spec *ds = &device_nodes[i]; ret = mknodat(dev_fd, ds->name, ds->mode, - makedev(ds->major, ds->minor)); + makedev(ds->major, ds->minor)); if (ret) goto done; } @@ -1592,8 +1644,7 @@ static int mount_dev(char **dev_path_ret) goto done; /* Restore old mask. */ - done: - close(dev_fd); +done: umask(mask); if (ret) @@ -1616,14 +1667,14 @@ static int mount_dev_finalize(const struct minijail *j, char *dev_path) if (umount2("/dev", MNT_DETACH)) goto done; - if (asprintf(&dest, "%s/dev", j->chrootdir ? : "") < 0) + if (asprintf(&dest, "%s/dev", j->chrootdir ?: "") < 0) goto done; if (mount(dev_path, dest, NULL, MS_MOVE, NULL)) goto done; ret = 0; - done: +done: free(dest); mount_dev_cleanup(dev_path); @@ -1752,7 +1803,8 @@ static int enter_chroot(const struct minijail *j) static int enter_pivot_root(const struct minijail *j) { - int oldroot, newroot; + attribute_cleanup_fd int oldroot = -1; + attribute_cleanup_fd int newroot = -1; run_hooks_or_die(j, MINIJAIL_HOOK_EVENT_PRE_CHROOT); @@ -1802,10 +1854,6 @@ static int enter_pivot_root(const struct minijail *j) /* Change back to the new root. */ if (fchdir(newroot)) return -errno; - if (close(oldroot)) - return -errno; - if (close(newroot)) - return -errno; if (chroot("/")) return -errno; /* Set correct CWD for getcwd(3). */ @@ -1924,7 +1972,10 @@ static void write_ugid_maps_or_die(const struct minijail *j) int ret = write_proc_file(j->initpid, "deny", "setgroups"); if (ret != 0) { if (ret == -ENOENT) { - /* See http://man7.org/linux/man-pages/man7/user_namespaces.7.html. */ + /* + * See + * http://man7.org/linux/man-pages/man7/user_namespaces.7.html. + */ warn("could not disable setgroups(2)"); } else kill_child_and_die( @@ -1974,7 +2025,8 @@ static void wait_for_parent_setup(int *pipe_fds) static void drop_ugid(const struct minijail *j) { if (j->flags.inherit_suppl_gids + j->flags.keep_suppl_gids + - j->flags.set_suppl_gids > 1) { + j->flags.set_suppl_gids > + 1) { die("can only do one of inherit, keep, or set supplementary " "groups"); } @@ -2139,7 +2191,7 @@ static void set_seccomp_filter(const struct minijail *j) } if (j->flags.seccomp_filter) { - if (j->flags.seccomp_filter_logging) { + if (seccomp_is_logging_allowed(j)) { warn("logging seccomp filter failures"); if (!seccomp_ret_log_available()) { /* @@ -2189,8 +2241,7 @@ static void set_seccomp_filter(const struct minijail *j) static pid_t forward_pid = -1; -static void forward_signal(int sig, - siginfo_t *siginfo attribute_unused, +static void forward_signal(int sig, siginfo_t *siginfo attribute_unused, void *void_context attribute_unused) { if (forward_pid != -1) { @@ -2316,12 +2367,12 @@ void API minijail_enter(const struct minijail *j) if (mount(NULL, temp->mount_name, NULL, MS_REC | temp->remount_mode, NULL)) pdie("mount(NULL, %s, NULL, " - "MS_REC | temp->remount_mode, NULL) " - "failed", temp->mount_name); + "MS_REC | temp->remount_mode, " + "NULL) failed", + temp->mount_name); temp = temp->next; } } - } if (j->flags.ipc && unshare(CLONE_NEWIPC)) { @@ -2332,7 +2383,8 @@ void API minijail_enter(const struct minijail *j) if (unshare(CLONE_NEWUTS)) pdie("unshare(CLONE_NEWUTS) failed"); - if (j->hostname && sethostname(j->hostname, strlen(j->hostname))) + if (j->hostname && + sethostname(j->hostname, strlen(j->hostname))) pdie("sethostname(%s) failed", j->hostname); } @@ -2473,22 +2525,19 @@ int API minijail_from_fd(int fd, struct minijail *j) { size_t sz = 0; size_t bytes = read(fd, &sz, sizeof(sz)); - char *buf; + attribute_cleanup_str char *buf = NULL; int r; if (sizeof(sz) != bytes) return -EINVAL; - if (sz > USHRT_MAX) /* arbitrary sanity check */ + if (sz > USHRT_MAX) /* arbitrary check */ return -E2BIG; buf = malloc(sz); if (!buf) return -ENOMEM; bytes = read(fd, buf, sz); - if (bytes != sz) { - free(buf); + if (bytes != sz) return -EINVAL; - } r = minijail_unmarshal(j, buf, sz); - free(buf); return r; } @@ -2498,24 +2547,20 @@ int API minijail_to_fd(struct minijail *j, int fd) if (!sz) return -EINVAL; - char *buf = malloc(sz); + attribute_cleanup_str char *buf = malloc(sz); if (!buf) return -ENOMEM; int err = minijail_marshal(j, buf, sz); if (err) - goto error; + return err; /* Sends [size][minijail]. */ err = write_exactly(fd, &sz, sizeof(sz)); if (err) - goto error; - - err = write_exactly(fd, buf, sz); + return err; -error: - free(buf); - return err; + return write_exactly(fd, buf, sz); } int API minijail_copy_jail(const struct minijail *from, struct minijail *out) @@ -2524,18 +2569,15 @@ int API minijail_copy_jail(const struct minijail *from, struct minijail *out) if (!sz) return -EINVAL; - char *buf = malloc(sz); + attribute_cleanup_str char *buf = malloc(sz); if (!buf) return -ENOMEM; int err = minijail_marshal(from, buf, sz); if (err) - goto error; + return err; - err = minijail_unmarshal(out, buf, sz); -error: - free(buf); - return err; + return minijail_unmarshal(out, buf, sz); } static int setup_preload(const struct minijail *j attribute_unused, @@ -2548,7 +2590,7 @@ static int setup_preload(const struct minijail *j attribute_unused, const char *preload_path = j->preload_path ?: PRELOADPATH; char *newenv = NULL; int ret = 0; - const char *oldenv = getenv(kLdPreloadEnvVar); + const char *oldenv = minijail_getenv(*child_env, kLdPreloadEnvVar); if (!oldenv) oldenv = ""; @@ -2565,6 +2607,19 @@ static int setup_preload(const struct minijail *j attribute_unused, #endif } +/* + * This is for logging purposes and does not change the enforced seccomp + * filter. + */ +static int setup_seccomp_policy_path(const struct minijail *j, + char ***child_env) +{ + return minijail_setenv(child_env, kSeccompPolicyPathEnvVar, + j->seccomp_policy_path ? j->seccomp_policy_path + : "NO-LABEL", + 1 /* overwrite */); +} + static int setup_pipe(char ***child_env, int fds[2]) { int r = pipe(fds); @@ -2623,11 +2678,11 @@ static int fd_is_open(int fd) static_assert(FD_SETSIZE >= MAX_PRESERVED_FDS * 2 - 1, "If true, ensure_no_fd_conflict will always find an unused fd."); -/* If p->parent_fd will be used by a child_fd, move it to an unused fd. */ -static int ensure_no_fd_conflict(const fd_set* child_fds, - struct preserved_fd* p) +/* If parent_fd will be used by a child fd, move it to an unused fd. */ +static int ensure_no_fd_conflict(const fd_set *child_fds, int child_fd, + int *parent_fd) { - if (!FD_ISSET(p->parent_fd, child_fds)){ + if (!FD_ISSET(*parent_fd, child_fds)) { return 0; } @@ -2635,8 +2690,8 @@ static int ensure_no_fd_conflict(const fd_set* child_fds, * If no other parent_fd matches the child_fd then use it instead of a * temporary. */ - int fd = p->child_fd; - if (fd_is_open(fd)) { + int fd = child_fd; + if (fd == -1 || fd_is_open(fd)) { fd = FD_SETSIZE - 1; while (FD_ISSET(fd, child_fds) || fd_is_open(fd)) { --fd; @@ -2646,45 +2701,115 @@ static int ensure_no_fd_conflict(const fd_set* child_fds, } } - int ret = dup2(p->parent_fd, fd); + int ret = dup2(*parent_fd, fd); /* * warn() opens a file descriptor so it needs to happen after dup2 to * avoid unintended side effects. This can be avoided by reordering the * mapping requests so that the source fds with overlap are mapped * first (unless there are cycles). */ - warn("mapped fd overlap: moving %d to %d", p->parent_fd, fd); + warn("mapped fd overlap: moving %d to %d", *parent_fd, fd); if (ret == -1) { return -1; } - p->parent_fd = fd; + *parent_fd = fd; return 0; } -static int redirect_fds(struct minijail *j) +/* + * Populate child_fds_out with the set of file descriptors that will be replaced + * by redirect_fds(). + * + * NOTE: This creates temporaries for parent file descriptors that would + * otherwise be overwritten during redirect_fds(). + */ +static int get_child_fds(struct minijail *j, fd_set *child_fds_out) { - fd_set child_fds; - FD_ZERO(&child_fds); - /* Relocate parent_fds that would be replaced by a child_fd. */ for (size_t i = 0; i < j->preserved_fd_count; i++) { int child_fd = j->preserved_fds[i].child_fd; - if (FD_ISSET(child_fd, &child_fds)) { + if (FD_ISSET(child_fd, child_fds_out)) { die("fd %d is mapped more than once", child_fd); } - if (ensure_no_fd_conflict(&child_fds, - &j->preserved_fds[i]) == -1) { + int *parent_fd = &j->preserved_fds[i].parent_fd; + if (ensure_no_fd_conflict(child_fds_out, child_fd, parent_fd) == + -1) { return -1; } - FD_SET(child_fd, &child_fds); + FD_SET(child_fd, child_fds_out); + } + return 0; +} + +/* + * Structure holding resources and state created when running a minijail. + */ +struct minijail_run_state { + pid_t child_pid; + int pipe_fds[2]; + int stdin_fds[2]; + int stdout_fds[2]; + int stderr_fds[2]; + int child_sync_pipe_fds[2]; + char **child_env; +}; + +/* + * Move pipe_fds if they conflict with a child_fd. + */ +static int avoid_pipe_conflicts(struct minijail_run_state *state, + fd_set *child_fds_out) +{ + int *pipe_fds[] = { + state->pipe_fds, state->child_sync_pipe_fds, state->stdin_fds, + state->stdout_fds, state->stderr_fds, + }; + for (size_t i = 0; i < ARRAY_SIZE(pipe_fds); ++i) { + if (pipe_fds[i][0] != -1 && + ensure_no_fd_conflict(child_fds_out, -1, &pipe_fds[i][0]) == + -1) { + return -1; + } + if (pipe_fds[i][1] != -1 && + ensure_no_fd_conflict(child_fds_out, -1, &pipe_fds[i][1]) == + -1) { + return -1; + } } + return 0; +} +/* + * Redirect j->preserved_fds from the parent_fd to the child_fd. + * + * NOTE: This will clear FD_CLOEXEC since otherwise the child_fd would not be + * inherited after the exec call. + */ +static int redirect_fds(struct minijail *j, fd_set *child_fds) +{ for (size_t i = 0; i < j->preserved_fd_count; i++) { if (j->preserved_fds[i].parent_fd == j->preserved_fds[i].child_fd) { + // Clear CLOEXEC if it is set so the FD will be + // inherited by the child. + int flags = + fcntl(j->preserved_fds[i].child_fd, F_GETFD); + if (flags == -1 || (flags & FD_CLOEXEC) == 0) { + continue; + } + + // Currently FD_CLOEXEC is cleared without being + // restored. It may make sense to track when this + // happens and restore FD_CLOEXEC in the child process. + flags &= ~FD_CLOEXEC; + if (fcntl(j->preserved_fds[i].child_fd, F_SETFD, + flags) == -1) { + pwarn("failed to clear CLOEXEC for %d", + j->preserved_fds[i].parent_fd); + } continue; } if (dup2(j->preserved_fds[i].parent_fd, @@ -2692,32 +2817,20 @@ static int redirect_fds(struct minijail *j) return -1; } } + /* * After all fds have been duped, we are now free to close all parent * fds that are *not* child fds. */ for (size_t i = 0; i < j->preserved_fd_count; i++) { int parent_fd = j->preserved_fds[i].parent_fd; - if (!FD_ISSET(parent_fd, &child_fds)) { + if (!FD_ISSET(parent_fd, child_fds)) { close(parent_fd); } } return 0; } -/* - * Structure holding resources and state created when running a minijail. - */ -struct minijail_run_state { - pid_t child_pid; - int pipe_fds[2]; - int stdin_fds[2]; - int stdout_fds[2]; - int stderr_fds[2]; - int child_sync_pipe_fds[2]; - char **child_env; -}; - static void minijail_free_run_state(struct minijail_run_state *state) { state->child_pid = -1; @@ -2776,13 +2889,21 @@ static void setup_child_std_fds(struct minijail *j, if (setsid() < 0) { pdie("setsid() failed"); } + + if (isatty(STDIN_FILENO)) { + if (ioctl(STDIN_FILENO, TIOCSCTTY, 0) != 0) { + pwarn("failed to set controlling terminal"); + } + } } } /* * Structure that specifies how to start a minijail. * - * filename - The program to exec in the child. Required if |exec_in_child| = 1. + * filename - The program to exec in the child. Should be NULL if elf_fd is set. + * elf_fd - A fd to be used with fexecve. Should be -1 if filename is set. + * NOTE: either filename or elf_fd is required if |exec_in_child| = 1. * argv - Arguments for the child program. Required if |exec_in_child| = 1. * envp - Environment for the child program. Available if |exec_in_child| = 1. * use_preload - If true use LD_PRELOAD. @@ -2795,6 +2916,7 @@ static void setup_child_std_fds(struct minijail *j, */ struct minijail_run_config { const char *filename; + int elf_fd; char *const *argv; char *const *envp; int use_preload; @@ -2814,6 +2936,7 @@ int API minijail_run(struct minijail *j, const char *filename, { struct minijail_run_config config = { .filename = filename, + .elf_fd = -1, .argv = argv, .envp = NULL, .use_preload = true, @@ -2822,11 +2945,26 @@ int API minijail_run(struct minijail *j, const char *filename, return minijail_run_config_internal(j, &config); } +int API minijail_run_env(struct minijail *j, const char *filename, + char *const argv[], char *const envp[]) +{ + struct minijail_run_config config = { + .filename = filename, + .elf_fd = -1, + .argv = argv, + .envp = envp, + .use_preload = true, + .exec_in_child = true, + }; + return minijail_run_config_internal(j, &config); +} + int API minijail_run_pid(struct minijail *j, const char *filename, char *const argv[], pid_t *pchild_pid) { struct minijail_run_config config = { .filename = filename, + .elf_fd = -1, .argv = argv, .envp = NULL, .use_preload = true, @@ -2841,6 +2979,7 @@ int API minijail_run_pipe(struct minijail *j, const char *filename, { struct minijail_run_config config = { .filename = filename, + .elf_fd = -1, .argv = argv, .envp = NULL, .use_preload = true, @@ -2856,6 +2995,7 @@ int API minijail_run_pid_pipes(struct minijail *j, const char *filename, { struct minijail_run_config config = { .filename = filename, + .elf_fd = -1, .argv = argv, .envp = NULL, .use_preload = true, @@ -2875,6 +3015,27 @@ int API minijail_run_env_pid_pipes(struct minijail *j, const char *filename, { struct minijail_run_config config = { .filename = filename, + .elf_fd = -1, + .argv = argv, + .envp = envp, + .use_preload = true, + .exec_in_child = true, + .pstdin_fd = pstdin_fd, + .pstdout_fd = pstdout_fd, + .pstderr_fd = pstderr_fd, + .pchild_pid = pchild_pid, + }; + return minijail_run_config_internal(j, &config); +} + +int API minijail_run_fd_env_pid_pipes(struct minijail *j, int elf_fd, + char *const argv[], char *const envp[], + pid_t *pchild_pid, int *pstdin_fd, + int *pstdout_fd, int *pstderr_fd) +{ + struct minijail_run_config config = { + .filename = NULL, + .elf_fd = elf_fd, .argv = argv, .envp = envp, .use_preload = true, @@ -2892,6 +3053,7 @@ int API minijail_run_no_preload(struct minijail *j, const char *filename, { struct minijail_run_config config = { .filename = filename, + .elf_fd = -1, .argv = argv, .envp = NULL, .use_preload = false, @@ -2902,14 +3064,13 @@ int API minijail_run_no_preload(struct minijail *j, const char *filename, int API minijail_run_pid_pipes_no_preload(struct minijail *j, const char *filename, - char *const argv[], - pid_t *pchild_pid, - int *pstdin_fd, - int *pstdout_fd, + char *const argv[], pid_t *pchild_pid, + int *pstdin_fd, int *pstdout_fd, int *pstderr_fd) { struct minijail_run_config config = { .filename = filename, + .elf_fd = -1, .argv = argv, .envp = NULL, .use_preload = false, @@ -2931,6 +3092,7 @@ int API minijail_run_env_pid_pipes_no_preload(struct minijail *j, { struct minijail_run_config config = { .filename = filename, + .elf_fd = -1, .argv = argv, .envp = envp, .use_preload = false, @@ -2945,7 +3107,9 @@ int API minijail_run_env_pid_pipes_no_preload(struct minijail *j, pid_t API minijail_fork(struct minijail *j) { - struct minijail_run_config config = {}; + struct minijail_run_config config = { + .elf_fd = -1, + }; return minijail_run_config_internal(j, &config); } @@ -2964,6 +3128,25 @@ static int minijail_run_internal(struct minijail *j, int do_init = j->flags.do_init && !j->flags.run_as_init; int use_preload = config->use_preload; + if (config->filename != NULL && config->elf_fd != -1) { + die("filename and elf_fd cannot be set at the same time"); + } + + /* + * Only copy the environment if we need to modify it. If this is done + * unconditionally, it triggers odd behavior in the ARC container. + */ + if (use_preload || j->seccomp_policy_path) { + state_out->child_env = + minijail_copy_env(config->envp ? config->envp : environ); + if (!state_out->child_env) + return ENOMEM; + } + + if (j->seccomp_policy_path && + setup_seccomp_policy_path(j, &state_out->child_env)) + return -EFAULT; + if (use_preload) { if (j->hooks_head != NULL) die("Minijail hooks are not supported with LD_PRELOAD"); @@ -2974,10 +3157,6 @@ static int minijail_run_internal(struct minijail *j, * Before we fork(2) and execve(2) the child process, we need * to open a pipe(2) to send the minijail configuration over. */ - state_out->child_env = - minijail_copy_env(config->envp ? config->envp : environ); - if (!state_out->child_env) - return ENOMEM; if (setup_preload(j, &state_out->child_env) || setup_pipe(&state_out->child_env, state_out->pipe_fds)) return -EFAULT; @@ -3070,8 +3249,9 @@ static int minijail_run_internal(struct minijail *j, if (child_pid < 0) { if (errno == EPERM) - pdie("clone(CLONE_NEWPID | ...) failed with EPERM; " - "is this process missing CAP_SYS_ADMIN?"); + pdie("clone(CLONE_NEWPID | ...) failed with " + "EPERM; is this process missing " + "CAP_SYS_ADMIN?"); pdie("clone(CLONE_NEWPID | ...) failed"); } } else { @@ -3137,13 +3317,15 @@ static int minijail_run_internal(struct minijail *j, /* Accept any pending SIGPIPE. */ while (true) { const struct timespec zero_time = {0, 0}; - const int sig = sigtimedwait(&to_block, NULL, &zero_time); + const int sig = + sigtimedwait(&to_block, NULL, &zero_time); if (sig < 0) { if (errno != EINTR) break; } else { if (sig != SIGPIPE) - die("unexpected signal %d", sig); + die("unexpected signal %d", + sig); } } @@ -3186,7 +3368,7 @@ static int minijail_run_internal(struct minijail *j, } if (j->flags.close_open_fds) { - const size_t kMaxInheritableFdsSize = 10 + MAX_PRESERVED_FDS; + const size_t kMaxInheritableFdsSize = 11 + MAX_PRESERVED_FDS; int inheritable_fds[kMaxInheritableFdsSize]; size_t size = 0; @@ -3223,11 +3405,29 @@ static int minijail_run_internal(struct minijail *j, inheritable_fds[size++] = j->preserved_fds[i].parent_fd; } + if (config->elf_fd > -1) { + inheritable_fds[size++] = config->elf_fd; + } + if (close_open_fds(inheritable_fds, size) < 0) die("failed to close open file descriptors"); } - if (redirect_fds(j)) + /* The set of fds will be replaced. */ + fd_set child_fds; + FD_ZERO(&child_fds); + if (get_child_fds(j, &child_fds)) + die("failed to set up fd redirections"); + + if (avoid_pipe_conflicts(state_out, &child_fds)) + die("failed to redirect conflicting pipes"); + + /* The elf_fd needs to be mutable so use a stack copy from now on. */ + int elf_fd = config->elf_fd; + if (elf_fd != -1 && ensure_no_fd_conflict(&child_fds, -1, &elf_fd)) + die("failed to redirect elf_fd"); + + if (redirect_fds(j, &child_fds)) die("failed to set up fd redirections"); if (sync_child) @@ -3282,7 +3482,7 @@ static int minijail_run_internal(struct minijail *j, * Best effort. Don't bother checking the return value. */ prctl(PR_SET_NAME, "minijail-init"); - init(child_pid); /* Never returns. */ + init(child_pid); /* Never returns. */ } state_out->child_pid = child_pid; } @@ -3317,10 +3517,16 @@ static int minijail_run_internal(struct minijail *j, */ if (!child_env) child_env = config->envp ? config->envp : environ; - execve(config->filename, config->argv, child_env); + if (elf_fd > -1) { + fexecve(elf_fd, config->argv, child_env); + pwarn("fexecve(%d) failed", config->elf_fd); + } else { + execve(config->filename, config->argv, child_env); + pwarn("execve(%s) failed", config->filename); + } - ret = (errno == ENOENT ? MINIJAIL_ERR_NO_COMMAND : MINIJAIL_ERR_NO_ACCESS); - pwarn("execve(%s) failed", config->filename); + ret = (errno == ENOENT ? MINIJAIL_ERR_NO_COMMAND + : MINIJAIL_ERR_NO_ACCESS); _exit(ret); } @@ -3385,32 +3591,38 @@ static int minijail_wait_internal(struct minijail *j, int expected_signal) if (!WIFEXITED(st)) { int error_status = st; - if (WIFSIGNALED(st)) { - int signum = WTERMSIG(st); + if (!WIFSIGNALED(st)) { + return error_status; + } + + int signum = WTERMSIG(st); + /* + * We return MINIJAIL_ERR_JAIL if the process received + * SIGSYS, which happens when a syscall is blocked by + * seccomp filters. + * If not, we do what bash(1) does: + * $? = 128 + signum + */ + if (signum == SIGSYS) { + warn("child process %d had a policy violation (%s)", + j->initpid, + j->seccomp_policy_path ? j->seccomp_policy_path + : "NO-LABEL"); + error_status = MINIJAIL_ERR_JAIL; + } else { if (signum != expected_signal) { warn("child process %d received signal %d", j->initpid, signum); } - /* - * We return MINIJAIL_ERR_JAIL if the process received - * SIGSYS, which happens when a syscall is blocked by - * seccomp filters. - * If not, we do what bash(1) does: - * $? = 128 + signum - */ - if (signum == SIGSYS) { - error_status = MINIJAIL_ERR_JAIL; - } else { - error_status = MINIJAIL_ERR_SIG_BASE + signum; - } + error_status = MINIJAIL_ERR_SIG_BASE + signum; } return error_status; } int exit_status = WEXITSTATUS(st); if (exit_status != 0) - info("child process %d exited with status %d", - j->initpid, exit_status); + info("child process %d exited with status %d", j->initpid, + exit_status); return exit_status; } @@ -3467,6 +3679,8 @@ void API minijail_destroy(struct minijail *j) free(j->alt_syscall_table); for (i = 0; i < j->cgroup_count; ++i) free(j->cgroups[i]); + if (j->seccomp_policy_path) + free(j->seccomp_policy_path); free(j); } diff --git a/libminijail.h b/libminijail.h index cfd42d2..d2dce7a 100644 --- a/libminijail.h +++ b/libminijail.h @@ -344,6 +344,13 @@ void minijail_enter(const struct minijail *j); /* * Run the specified command in the given minijail, execve(2)-style. + * Pass |envp| as the full environment for the child. + */ +int minijail_run_env(struct minijail *j, const char *filename, + char *const argv[], char *const envp[]); + +/* + * Run the specified command in the given minijail, execve(2)-style. * If minijail_namespace_pids() or minijail_namespace_user() are used, * this or minijail_fork() is required instead of minijail_enter(). */ @@ -404,6 +411,23 @@ int minijail_run_env_pid_pipes(struct minijail *j, const char *filename, int *pstdout_fd, int *pstderr_fd); /* + * Execute the specified file descriptor in the given minijail, + * fexecve(3)-style. + * Pass |envp| as the full environment for the child or NULL to inherit. + * Update |*pchild_pid| with the pid of the child. + * Update |*pstdin_fd| with a fd that allows writing to the child's + * standard input. + * Update |*pstdout_fd| with a fd that allows reading from the child's + * standard output. + * Update |*pstderr_fd| with a fd that allows reading from the child's + * standard error. + */ +int minijail_run_fd_env_pid_pipes(struct minijail *j, int elf_fd, + char *const argv[], char *const envp[], + pid_t *pchild_pid, int *pstdin_fd, + int *pstdout_fd, int *pstderr_fd); + +/* * Run the specified command in the given minijail, execve(2)-style. * Update |*pchild_pid| with the pid of the child. * Update |*pstdin_fd| with a fd that allows writing to the child's diff --git a/libminijail_unittest.cc b/libminijail_unittest.cc index 78e3cfb..868b7d7 100644 --- a/libminijail_unittest.cc +++ b/libminijail_unittest.cc @@ -9,6 +9,7 @@ #include <dirent.h> #include <fcntl.h> +#include <sys/mman.h> #include <sys/mount.h> #include <sys/stat.h> #include <sys/types.h> @@ -473,6 +474,94 @@ TEST(Test, close_original_pipes_after_dup2) { EXPECT_EQ(minijail_wait(j.get()), 42); } +TEST(Test, minijail_no_clobber_pipe_fd) { + const ScopedMinijail j(minijail_new()); + char* const script = R"( + echo Hi >&1; + exec 1>&-; + exec 4>&-; + exec 7>&-; + read line1; + read line2; + echo "$line1$line2 and Goodbye" >&2; + exit 42; + )"; + char* const argv[] = {"sh", "-c", script, nullptr}; + + const int npipes = 3; + int fds[npipes][2]; + + // Create pipes. + for (int i = 0; i < npipes; ++i) { + ASSERT_EQ(pipe(fds[i]), 0); + } + + // All pipes are output pipes except for the first one which is used as + // input pipe. + std::swap(fds[0][0], fds[0][1]); + + // Generate a lot of mappings to try to clobber any file descriptors generated + // by libminijail. + for (int offset = 0; offset < npipes * 3; offset += npipes) { + for (int i = 0 ; i < npipes; ++i) { + const int fd = fds[i][1]; + minijail_preserve_fd(j.get(), fd, i + offset); + } + } + + minijail_close_open_fds(j.get()); + + EXPECT_EQ(minijail_run_no_preload(j.get(), kShellPath, argv), 0); + + // Close unused end of pipes. + for (int i = 0; i < npipes; ++i) { + const int fd = fds[i][1]; + ASSERT_EQ(close(fd), 0); + } + + const int in = fds[0][0]; + const int out = fds[1][0]; + const int err = fds[2][0]; + + char buf[PIPE_BUF]; + ssize_t nbytes; + + // Check that stdout pipe works. + nbytes = read(out, buf, PIPE_BUF); + ASSERT_GT(nbytes, 0); + EXPECT_EQ(std::string(buf, nbytes), "Hi\n"); + + // Check that the write end of stdout pipe got closed by the child process. If + // the child process kept other file descriptors connected to stdout, then the + // parent process wouldn't be able to detect that all write ends of this pipe + // are closed and it would block here. + EXPECT_EQ(read(out, buf, PIPE_BUF), 0); + ASSERT_EQ(close(out), 0); + + // Check that stdin pipe works. + const std::string s = "Greetings\n"; + EXPECT_EQ(write(in, s.data(), s.size()), s.size()); + + // Close write end of pipe connected to child's stdin. If there was another + // file descriptor connected to this write end, then the child process + // wouldn't be able to detect that this write end is closed and it would + // block. + ASSERT_EQ(close(in), 0); + + // Check that child process continued and ended. + nbytes = read(err, buf, PIPE_BUF); + ASSERT_GT(nbytes, 0); + EXPECT_EQ(std::string(buf, nbytes), "Greetings and Goodbye\n"); + + // Check that the write end of the stderr pipe is closed when the child + // process finishes. + EXPECT_EQ(read(err, buf, PIPE_BUF), 0); + ASSERT_EQ(close(err), 0); + + // Check the child process termination status. + EXPECT_EQ(minijail_wait(j.get()), 42); +} + TEST(Test, minijail_run_env_pid_pipes) { // TODO(crbug.com/895875): The preload library interferes with ASan since they // both need to use LD_PRELOAD. @@ -536,6 +625,56 @@ TEST(Test, minijail_run_env_pid_pipes) { EXPECT_EQ(WEXITSTATUS(status), 0); } +TEST(Test, minijail_run_fd_env_pid_pipes) { + // TODO(crbug.com/895875): The preload library interferes with ASan since they + // both need to use LD_PRELOAD. + if (running_with_asan()) + GTEST_SKIP(); + + ScopedMinijail j(minijail_new()); + minijail_set_preload_path(j.get(), kPreloadPath); + + char *argv[4]; + argv[0] = const_cast<char*>(kShellPath); + argv[1] = "-c"; + argv[2] = "echo \"${TEST_PARENT+set}|${TEST_VAR}\" >&2\n"; + argv[3] = nullptr; + + char *envp[2]; + envp[0] = "TEST_VAR=test"; + envp[1] = nullptr; + + // Set a canary env var in the parent that should not be present in the child. + ASSERT_EQ(setenv("TEST_PARENT", "test", 1 /*overwrite*/), 0); + + int elf_fd = open(const_cast<char*>(kShellPath), O_RDONLY | O_CLOEXEC); + ASSERT_NE(elf_fd, -1); + + int dev_null = open("/dev/null", O_RDONLY); + ASSERT_NE(dev_null, -1); + // Create a mapping to dev_null that would clobber elf_fd if it is not + // relocated. + minijail_preserve_fd(j.get(), dev_null, elf_fd); + + pid_t pid; + int child_stdin, child_stdout, child_stderr; + int mj_run_ret = + minijail_run_fd_env_pid_pipes(j.get(), elf_fd, argv, envp, &pid, + &child_stdin, &child_stdout, &child_stderr); + EXPECT_EQ(mj_run_ret, 0); + close(dev_null); + + char buf[kBufferSize] = {}; + ssize_t read_ret = read(child_stderr, buf, sizeof(buf) - 1); + EXPECT_GE(read_ret, 0); + EXPECT_STREQ(buf, "|test\n"); + + int status; + EXPECT_EQ(waitpid(pid, &status, 0), pid); + ASSERT_TRUE(WIFEXITED(status)); + EXPECT_EQ(WEXITSTATUS(status), 0); +} + TEST(Test, minijail_run_env_pid_pipes_with_local_preload) { // TODO(crbug.com/895875): The preload library interferes with ASan since they // both need to use LD_PRELOAD. @@ -601,6 +740,56 @@ TEST(Test, minijail_run_env_pid_pipes_with_local_preload) { EXPECT_EQ(WEXITSTATUS(status), 0); } +TEST(Test, test_minijail_no_clobber_fds) { + int dev_null = open("/dev/null", O_RDONLY); + ASSERT_NE(dev_null, -1); + + ScopedMinijail j(minijail_new()); + + // Keep stderr. + minijail_preserve_fd(j.get(), 2, 2); + // Create a lot of mappings to dev_null to possibly clobber libminijail.c fds. + for (int i = 3; i < 15; ++i) { + minijail_preserve_fd(j.get(), dev_null, i); + } + + char *argv[4]; + argv[0] = const_cast<char*>(kShellPath); + argv[1] = "-c"; + argv[2] = "echo Hello; read line1; echo \"${line1}\" >&2"; + argv[3] = nullptr; + + pid_t pid; + int child_stdin; + int child_stdout; + int child_stderr; + int mj_run_ret = minijail_run_pid_pipes_no_preload( + j.get(), argv[0], argv, &pid, &child_stdin, &child_stdout, &child_stderr); + EXPECT_EQ(mj_run_ret, 0); + + char buf[kBufferSize]; + ssize_t read_ret = read(child_stdout, buf, sizeof(buf)); + EXPECT_GE(read_ret, 0); + buf[read_ret] = '\0'; + EXPECT_STREQ(buf, "Hello\n"); + + constexpr char to_write[] = "test in and err\n"; + ssize_t write_ret = write(child_stdin, to_write, sizeof(to_write)); + EXPECT_EQ(write_ret, sizeof(to_write)); + + read_ret = read(child_stderr, buf, sizeof(buf)); + EXPECT_GE(read_ret, 0); + buf[read_ret] = '\0'; + EXPECT_STREQ(buf, to_write); + + int status; + waitpid(pid, &status, 0); + ASSERT_TRUE(WIFEXITED(status)); + EXPECT_EQ(WEXITSTATUS(status), 0); + + close(dev_null); +} + TEST(Test, test_minijail_no_fd_leaks) { pid_t pid; int child_stdout; @@ -1092,7 +1281,7 @@ TEST_F(NamespaceTest, test_remount_all_private) { argv[0] = const_cast<char*>(kShellPath); argv[1] = "-c"; argv[2] = "grep -E 'shared:|master:|propagate_from:|unbindable:' " - "/proc/self/mountinfo"; + "/proc/self/mountinfo"; argv[3] = NULL; mj_run_ret = minijail_run_pid_pipes_no_preload( j, argv[0], argv, &pid, NULL, &child_stdout, NULL); diff --git a/libminijailpreload.c b/libminijailpreload.c index a98b736..b5a3c75 100644 --- a/libminijailpreload.c +++ b/libminijailpreload.c @@ -11,6 +11,7 @@ #include "libminijail.h" #include "libminijail-private.h" +#include "util.h" #include <dlfcn.h> #include <stdio.h> @@ -23,18 +24,24 @@ static int (*real_main) (int, char **, char **); static void *libc_handle; -static void die(const char *failed) +static void truncate_preload_env(char **envp, const char *name) { - syslog(LOG_ERR, "libminijail: %s", failed); - abort(); -} - -static void unset_in_env(char **envp, const char *name) -{ - int i; - for (i = 0; envp[i]; i++) - if (!strncmp(envp[i], name, strlen(name))) - envp[i][0] = '\0'; + char *env_value = minijail_getenv(envp, name); + if (env_value) { + /* + * if we have more than just libminijailpreload.so in + * LD_PRELOAD, cut out libminijailpreload.so from it, + * as it is guaranteed to always be last in the + * LD_PRELOAD list. + */ + char *last_space = strrchr(env_value, ' '); + if (last_space) { + *last_space = '\0'; + } else { + /* Only our lib was in LD_PRELOAD, just unset it. */ + minijail_unsetenv(envp, name); + } + } } /** @brief Fake main(), spliced in before the real call to main() by @@ -71,12 +78,10 @@ static int fake_main(int argc, char **argv, char **envp) die("preload: failed to parse minijail from parent"); close(fd); - unset_in_env(envp, kFdEnvVar); - /* TODO(ellyjones): this trashes existing preloads, so one can't do: - * LD_PRELOAD="/tmp/test.so libminijailpreload.so" prog; the - * descendants of prog will have no LD_PRELOAD set at all. - */ - unset_in_env(envp, kLdPreloadEnvVar); + minijail_unsetenv(envp, kFdEnvVar); + + truncate_preload_env(envp, kLdPreloadEnvVar); + /* Strip out flags meant for the parent. */ minijail_preenter(j); minijail_enter(j); diff --git a/linux-x86/libsyscalls.gen.c b/linux-x86/libsyscalls.gen.c index e0b2e53..1403380 100644 --- a/linux-x86/libsyscalls.gen.c +++ b/linux-x86/libsyscalls.gen.c @@ -1038,6 +1038,27 @@ const struct syscall_entry syscall_table[] = { #ifdef __NR_fspick { "fspick", __NR_fspick }, #endif +#ifdef __NR_pidfd_open +{ "pidfd_open", __NR_pidfd_open }, +#endif +#ifdef __NR_clone3 +{ "clone3", __NR_clone3 }, +#endif +#ifdef __NR_close_range +{ "close_range", __NR_close_range }, +#endif +#ifdef __NR_openat2 +{ "openat2", __NR_openat2 }, +#endif +#ifdef __NR_pidfd_getfd +{ "pidfd_getfd", __NR_pidfd_getfd }, +#endif +#ifdef __NR_faccessat2 +{ "faccessat2", __NR_faccessat2 }, +#endif +#ifdef __NR_process_madvise +{ "process_madvise", __NR_process_madvise }, +#endif { NULL, -1 }, }; diff --git a/minijail0.1 b/minijail0.1 index 7dc6f74..a53ec6f 100644 --- a/minijail0.1 +++ b/minijail0.1 @@ -12,7 +12,7 @@ Runs PROGRAM inside a sandbox. Run using the alternate syscall table named \fItable\fR. Only available on kernels and architectures that support the \fBPR_ALT_SYSCALL\fR option of \fBprctl\fR(2). .TP -\fB-b <src>[,[dest][,<writeable>]] +\fB-b <src>[,[dest][,<writeable>]]\fR, \fB--bind-mount=<src>[,[dest][,<writeable>]]\fR Bind-mount \fIsrc\fR into the chroot directory at \fIdest\fR, optionally writeable. The \fIsrc\fR path must be an absolute path. @@ -116,7 +116,7 @@ make sure the \fIprogram\fR continues to work as expected. the first minijail process outside of the pid namespace while the \fB-I\fR option controls the minijail process inside of the pid namespace. .TP -\fB-k <src>,<dest>,<type>[,<flags>[,<data>]]\fR +\fB-k <src>,<dest>,<type>[,<flags>[,<data>]]\fR, \fB--mount=<src>,<dest>,<type>[,<flags>[,<data>]]\fR Mount \fIsrc\fR, a \fItype\fR filesystem, at \fIdest\fR. If a chroot or pivot root is active, \fIdest\fR will automatically be placed below that path. @@ -257,7 +257,7 @@ Change users to the specified \fIuser\fR name, or numeric user ID \fIuid\fR. \fB-U\fR Enter a new user namespace (implies \fB-p\fR). .TP -\fB-v\fR +\fB-v\fR, \fB--ns-mount\fR Run inside a new VFS namespace. This option prevents mounts performed by the program from affecting the rest of the system (but see \fB-K\fR). .TP @@ -289,6 +289,16 @@ capability-dumb binaries. See \fBcapabilities\fR(7). Create a new UTS/hostname namespace, and optionally set the hostname in the new namespace to \fIhostname\fR. .TP +\fB--env-reset\fR +Clear the current environment instead of having the program inherit the active +environment. This is often used to start the program with a minimal +sanitized environment. +.TP +\fB--env-add <NAME=value>\fR +Adds or replace the specified environment variable \fINAME\fR in the program's +environment before starting it, and set it to the specified \fIvalue\fR. +This option can be used several times to set any number of environment variables. +.TP \fB--logging=<system>\fR Use \fIsystem\fR as the logging system. \fIsystem\fR must be one of \fBauto\fR (the default), \fBsyslog\fR, or \fBstderr\fR. @@ -327,6 +337,11 @@ this option unless you know what you're doing. See the kernel documentation \fIDocumentation/userspace-api/spec_ctrl.rst\fR and \fIDocumentation/admin-guide/hw-vuln/spectre.rst\fR for more information. +.TP +\fB--config <file path>\fR +Use a Minijail configuration file to set options, through +commandline-option-equivalent key-value pairs. +See \fBminijail0\fR(5) for more details on the format of the configuration file. .SH SANDBOXING PROFILES The following sandboxing profiles are supported: .TP diff --git a/minijail0.5 b/minijail0.5 index 65d1626..3e4f114 100644 --- a/minijail0.5 +++ b/minijail0.5 @@ -157,6 +157,31 @@ functions and determining if any of them issue system calls. There is no active implementation for this, but something like code.google.com/p/seccompsandbox is one possible runtime variant. +.SH CONFIGURATION FILE +A configuration file can be used to specify command line options and other +settings. + +It supports the following syntax: + \fB% minijail-config-file v0\fR + \fB<option>\fR=\fB<argument>\fR + \fB<no-argument-option>\fR + \fB<empty line>\fR + \fB# any single line comment\fR + +Long lines may be broken up using \\ at the end. + +The special directive "% minijail-config-file v0" must occupy the first line. +"v0" also declares the version of the config file format. + +Keys contain only alphabetic characters and '-'. Values can be any non-empty +string. Leading and trailing whitespaces around keys and +values are permitted but will be stripped before processing. + +Currently all long options are supported such as +\fBmount\fR, \fBbind-mount\fR. For a option that has no argument, the option +will occupy a single line, without '=' and value. Otherwise, any string that +is given after the '=' is interpreted as the argument. + .SH AUTHOR The Chromium OS Authors <chromiumos-dev@chromium.org> .SH COPYRIGHT diff --git a/minijail0.c b/minijail0.c index f216d6e..9b1fcf3 100644 --- a/minijail0.c +++ b/minijail0.c @@ -4,6 +4,7 @@ */ #include <dlfcn.h> +#include <err.h> #include <errno.h> #include <stdio.h> #include <stdlib.h> @@ -15,15 +16,17 @@ #include "minijail0_cli.h" #include "util.h" -int main(int argc, char *argv[]) +int main(int argc, char *argv[], char *environ[]) { struct minijail *j = minijail_new(); const char *dl_mesg = NULL; const char *preload_path = PRELOADPATH; int exit_immediately = 0; ElfType elftype = ELFERROR; - int consumed = parse_args(j, argc, argv, &exit_immediately, &elftype, - &preload_path); + char **envp = NULL; + int consumed = parse_args(j, argc, argv, environ, + &exit_immediately, &elftype, + &preload_path, &envp); argc -= consumed; argv += consumed; @@ -37,10 +40,8 @@ int main(int argc, char *argv[]) * the process is already a process group leader. */ if (setpgid(0 /* use calling PID */, 0 /* make PGID = PID */)) { - if (errno != EPERM) { - fprintf(stderr, "setpgid(0, 0) failed\n"); - exit(1); - } + if (errno != EPERM) + err(1, "setpgid(0, 0) failed"); } if (elftype == ELFSTATIC) { @@ -58,16 +59,17 @@ int main(int argc, char *argv[]) /* Check that we can dlopen() libminijailpreload.so. */ if (!dlopen(preload_path, RTLD_LAZY | RTLD_LOCAL)) { dl_mesg = dlerror(); - fprintf(stderr, "dlopen(): %s\n", dl_mesg); + errx(1, "dlopen(): %s", dl_mesg); return 1; } minijail_set_preload_path(j, preload_path); - minijail_run(j, argv[0], argv); + if (envp) { + minijail_run_env(j, argv[0], argv, envp); + } else { + minijail_run(j, argv[0], argv); + } } else { - fprintf(stderr, - "Target program '%s' is not a valid ELF file.\n", - argv[0]); - return 1; + errx(1, "Target program '%s' is not a valid ELF file", argv[0]); } if (exit_immediately) diff --git a/minijail0_cli.c b/minijail0_cli.c index 3461579..e366846 100644 --- a/minijail0_cli.c +++ b/minijail0_cli.c @@ -4,7 +4,9 @@ */ #include <dlfcn.h> +#include <err.h> #include <errno.h> +#include <fcntl.h> #include <getopt.h> #include <inttypes.h> #include <stdbool.h> @@ -13,7 +15,9 @@ #include <string.h> #include <sys/capability.h> #include <sys/mount.h> +#include <sys/stat.h> #include <sys/types.h> +#include <sys/vfs.h> #include <unistd.h> #include <linux/filter.h> @@ -21,6 +25,7 @@ #include "libminijail.h" #include "libsyscalls.h" +#include "config_parser.h" #include "elfparse.h" #include "minijail0_cli.h" #include "system.h" @@ -36,20 +41,16 @@ static void *xmalloc(size_t size) { void *ret = malloc(size); - if (!ret) { - perror("malloc() failed"); - exit(1); - } + if (!ret) + err(1, "malloc() failed"); return ret; } static char *xstrdup(const char *s) { char *ret = strdup(s); - if (!ret) { - perror("strdup() failed"); - exit(1); - } + if (!ret) + err(1, "strdup() failed"); return ret; } @@ -57,7 +58,7 @@ static void set_user(struct minijail *j, const char *arg, uid_t *out_uid, gid_t *out_gid) { char *end = NULL; - int uid = strtod(arg, &end); + uid_t uid = strtoul(arg, &end, 10); if (!*end && *arg) { *out_uid = uid; minijail_change_uid(j, uid); @@ -66,22 +67,21 @@ static void set_user(struct minijail *j, const char *arg, uid_t *out_uid, int ret = lookup_user(arg, out_uid, out_gid); if (ret) { - fprintf(stderr, "Bad user '%s': %s\n", arg, strerror(-ret)); - exit(1); + errno = -ret; + err(1, "Bad user '%s'", arg); } ret = minijail_change_user(j, arg); if (ret) { - fprintf(stderr, "minijail_change_user('%s') failed: %s\n", arg, - strerror(-ret)); - exit(1); + errno = -ret; + err(1, "minijail_change_user('%s') failed", arg); } } static void set_group(struct minijail *j, const char *arg, gid_t *out_gid) { char *end = NULL; - int gid = strtod(arg, &end); + gid_t gid = strtoul(arg, &end, 10); if (!*end && *arg) { *out_gid = gid; minijail_change_gid(j, gid); @@ -90,8 +90,8 @@ static void set_group(struct minijail *j, const char *arg, gid_t *out_gid) int ret = lookup_group(arg, out_gid); if (ret) { - fprintf(stderr, "Bad group '%s': %s\n", arg, strerror(-ret)); - exit(1); + errno = -ret; + err(1, "Bad group '%s'", arg); } minijail_change_gid(j, *out_gid); @@ -102,33 +102,30 @@ static void set_group(struct minijail *j, const char *arg, gid_t *out_gid) * to build the supplementary gids array. */ static void suppl_group_add(size_t *suppl_gids_count, gid_t **suppl_gids, - char *arg) { + char *arg) +{ char *end = NULL; - int groupid = strtod(arg, &end); - gid_t gid; + gid_t gid = strtoul(arg, &end, 10); int ret; if (!*end && *arg) { /* A gid number has been specified, proceed. */ - gid = groupid; } else if ((ret = lookup_group(arg, &gid))) { /* * A group name has been specified, * but doesn't exist: we bail out. */ - fprintf(stderr, "Bad group '%s': %s\n", arg, strerror(-ret)); - exit(1); + errno = -ret; + err(1, "Bad group '%s'", arg); } /* * From here, gid is guaranteed to be set and valid, * we add it to our supplementary gids array. */ - *suppl_gids = realloc(*suppl_gids, - sizeof(gid_t) * ++(*suppl_gids_count)); - if (!suppl_gids) { - fprintf(stderr, "failed to allocate memory.\n"); - exit(1); - } + *suppl_gids = + realloc(*suppl_gids, sizeof(gid_t) * ++(*suppl_gids_count)); + if (!suppl_gids) + err(1, "failed to allocate memory"); (*suppl_gids)[*suppl_gids_count - 1] = gid; } @@ -138,10 +135,8 @@ static void skip_securebits(struct minijail *j, const char *arg) uint64_t securebits_skip_mask; char *end = NULL; securebits_skip_mask = strtoull(arg, &end, 16); - if (*end) { - fprintf(stderr, "Invalid securebit mask: '%s'\n", arg); - exit(1); - } + if (*end) + errx(1, "Invalid securebit mask: '%s'", arg); minijail_skip_setting_securebits(j, securebits_skip_mask); } @@ -167,11 +162,10 @@ static void use_caps(struct minijail *j, const char *arg) */ continue; } - fprintf(stderr, - "Could not get the value of " - "the %d-th capability: %m\n", - i); - exit(1); + err(1, + "Could not get the value of the %d-th " + "capability", + i); } if (cap_value == CAP_SET) caps |= (one << i); @@ -180,10 +174,8 @@ static void use_caps(struct minijail *j, const char *arg) } else { char *end = NULL; caps = strtoull(arg, &end, 16); - if (*end) { - fprintf(stderr, "Invalid cap set: '%s'\n", arg); - exit(1); - } + if (*end) + errx(1, "Invalid cap set: '%s'", arg); } minijail_use_caps(j, caps); @@ -194,10 +186,8 @@ static void add_binding(struct minijail *j, char *arg) char *src = tokenize(&arg, ","); char *dest = tokenize(&arg, ","); char *flags = tokenize(&arg, ","); - if (!src || src[0] == '\0' || arg != NULL) { - fprintf(stderr, "Bad binding: %s %s\n", src, dest); - exit(1); - } + if (!src || src[0] == '\0' || arg != NULL) + errx(1, "Bad binding: %s %s", src, dest); if (dest == NULL || dest[0] == '\0') dest = src; int writable; @@ -205,14 +195,10 @@ static void add_binding(struct minijail *j, char *arg) writable = 0; else if (!strcmp(flags, "1")) writable = 1; - else { - fprintf(stderr, "Bad value for <writable>: %s\n", flags); - exit(1); - } - if (minijail_bind(j, src, dest, writable)) { - fprintf(stderr, "minijail_bind failed.\n"); - exit(1); - } + else + errx(1, "Bad value for <writable>: %s", flags); + if (minijail_bind(j, src, dest, writable)) + errx(1, "minijail_bind failed"); } static void add_rlimit(struct minijail *j, char *arg) @@ -221,10 +207,9 @@ static void add_rlimit(struct minijail *j, char *arg) char *cur = tokenize(&arg, ","); char *max = tokenize(&arg, ","); char *end; - if (!type || type[0] == '\0' || !cur || cur[0] == '\0' || - !max || max[0] == '\0' || arg != NULL) { - fprintf(stderr, "Bad rlimit '%s'.\n", arg); - exit(1); + if (!type || type[0] == '\0' || !cur || cur[0] == '\0' || !max || + max[0] == '\0' || arg != NULL) { + errx(1, "Bad rlimit '%s'", arg); } rlim_t cur_rlim; rlim_t max_rlim; @@ -233,34 +218,25 @@ static void add_rlimit(struct minijail *j, char *arg) } else { end = NULL; cur_rlim = strtoul(cur, &end, 0); - if (*end) { - fprintf(stderr, "Bad soft limit: '%s'.\n", cur); - exit(1); - } + if (*end) + errx(1, "Bad soft limit: '%s'", cur); } if (!strcmp(max, "unlimited")) { max_rlim = RLIM_INFINITY; } else { end = NULL; max_rlim = strtoul(max, &end, 0); - if (*end) { - fprintf(stderr, "Bad hard limit: '%s'.\n", max); - exit(1); - } + if (*end) + errx(1, "Bad hard limit: '%s'", max); } end = NULL; int resource = parse_single_constant(type, &end); - if (type == end) { - fprintf(stderr, "Bad rlimit: '%s'.\n", type); - exit(1); - } + if (type == end) + errx(1, "Bad rlimit: '%s'", type); - if (minijail_rlimit(j, resource, cur_rlim, max_rlim)) { - fprintf(stderr, "minijail_rlimit '%s,%s,%s' failed.\n", type, - cur, max); - exit(1); - } + if (minijail_rlimit(j, resource, cur_rlim, max_rlim)) + errx(1, "minijail_rlimit '%s,%s,%s' failed", type, cur, max); } static void add_mount(struct minijail *j, char *arg) @@ -271,10 +247,9 @@ static void add_mount(struct minijail *j, char *arg) char *flags = tokenize(&arg, ","); char *data = tokenize(&arg, ","); char *end; - if (!src || src[0] == '\0' || !dest || dest[0] == '\0' || - !type || type[0] == '\0') { - fprintf(stderr, "Bad mount: %s %s %s\n", src, dest, type); - exit(1); + if (!src || src[0] == '\0' || !dest || dest[0] == '\0' || !type || + type[0] == '\0') { + errx(1, "Bad mount: %s %s %s", src, dest, type); } /* @@ -297,17 +272,12 @@ static void add_mount(struct minijail *j, char *arg) } else { end = NULL; mountflags = parse_constant(flags, &end); - if (flags == end) { - fprintf(stderr, "Bad mount flags: %s\n", flags); - exit(1); - } + if (flags == end) + errx(1, "Bad mount flags: %s", flags); } - if (minijail_mount_with_data(j, src, dest, type, - mountflags, data)) { - fprintf(stderr, "minijail_mount failed.\n"); - exit(1); - } + if (minijail_mount_with_data(j, src, dest, type, mountflags, data)) + errx(1, "minijail_mount failed"); } static char *build_idmap(id_t id, id_t lowerid) @@ -317,8 +287,7 @@ static char *build_idmap(id_t id, id_t lowerid) ret = snprintf(idmap, IDMAP_LEN, "%d %d 1", id, lowerid); if (ret < 0 || (size_t)ret >= IDMAP_LEN) { free(idmap); - fprintf(stderr, "Could not build id map.\n"); - exit(1); + errx(1, "Could not build id map"); } return idmap; } @@ -332,20 +301,14 @@ static int has_cap_setgid(void) return 0; caps = cap_get_proc(); - if (!caps) { - fprintf(stderr, "Could not get process' capabilities: %m\n"); - exit(1); - } + if (!caps) + err(1, "Could not get process' capabilities"); - if (cap_get_flag(caps, CAP_SETGID, CAP_EFFECTIVE, &cap_value)) { - fprintf(stderr, "Could not get the value of CAP_SETGID: %m\n"); - exit(1); - } + if (cap_get_flag(caps, CAP_SETGID, CAP_EFFECTIVE, &cap_value)) + err(1, "Could not get the value of CAP_SETGID"); - if (cap_free(caps)) { - fprintf(stderr, "Could not free capabilities: %m\n"); - exit(1); - } + if (cap_free(caps)) + err(1, "Could not free capabilities"); return cap_value == CAP_SET; } @@ -366,10 +329,8 @@ static void set_ugid_mapping(struct minijail *j, int set_uidmap, uid_t uid, */ uidmap = build_idmap(uid, getuid()); } - if (0 != minijail_uidmap(j, uidmap)) { - fprintf(stderr, "Could not set uid map.\n"); - exit(1); - } + if (0 != minijail_uidmap(j, uidmap)) + errx(1, "Could not set uid map"); free(uidmap); } if (set_gidmap) { @@ -393,10 +354,8 @@ static void set_ugid_mapping(struct minijail *j, int set_uidmap, uid_t uid, */ minijail_namespace_user_disable_setgroups(j); } - if (0 != minijail_gidmap(j, gidmap)) { - fprintf(stderr, "Could not set gid map.\n"); - exit(1); - } + if (0 != minijail_gidmap(j, gidmap)) + errx(1, "Could not set gid map"); free(gidmap); } } @@ -404,30 +363,20 @@ static void set_ugid_mapping(struct minijail *j, int set_uidmap, uid_t uid, static void use_chroot(struct minijail *j, const char *path, int *chroot, int pivot_root) { - if (pivot_root) { - fprintf(stderr, "Could not set chroot because " - "'-P' was specified.\n"); - exit(1); - } - if (minijail_enter_chroot(j, path)) { - fprintf(stderr, "Could not set chroot.\n"); - exit(1); - } + if (pivot_root) + errx(1, "Could not set chroot because -P was specified"); + if (minijail_enter_chroot(j, path)) + errx(1, "Could not set chroot"); *chroot = 1; } static void use_pivot_root(struct minijail *j, const char *path, int *pivot_root, int chroot) { - if (chroot) { - fprintf(stderr, "Could not set pivot_root because " - "'-C' was specified.\n"); - exit(1); - } - if (minijail_enter_pivot_root(j, path)) { - fprintf(stderr, "Could not set pivot_root.\n"); - exit(1); - } + if (chroot) + errx(1, "Could not set pivot_root because -C was specified"); + if (minijail_enter_pivot_root(j, path)) + errx(1, "Could not set pivot_root"); minijail_namespace_vfs(j); *pivot_root = 1; } @@ -440,19 +389,13 @@ static void use_profile(struct minijail *j, const char *profile, if (!strcmp(profile, "minimalistic-mountns") || !strcmp(profile, "minimalistic-mountns-nodev")) { minijail_namespace_vfs(j); - if (minijail_bind(j, "/", "/", 0)) { - fprintf(stderr, "minijail_bind(/) failed.\n"); - exit(1); - } - if (minijail_bind(j, "/proc", "/proc", 0)) { - fprintf(stderr, "minijail_bind(/proc) failed.\n"); - exit(1); - } + if (minijail_bind(j, "/", "/", 0)) + errx(1, "minijail_bind(/) failed"); + if (minijail_bind(j, "/proc", "/proc", 0)) + errx(1, "minijail_bind(/proc) failed"); if (!strcmp(profile, "minimalistic-mountns")) { - if (minijail_bind(j, "/dev/log", "/dev/log", 0)) { - fprintf(stderr, "minijail_bind(/dev/log) failed.\n"); - exit(1); - } + if (minijail_bind(j, "/dev/log", "/dev/log", 0)) + errx(1, "minijail_bind(/dev/log) failed"); minijail_mount_dev(j); } if (!*tmp_size) { @@ -461,10 +404,8 @@ static void use_profile(struct minijail *j, const char *profile, } minijail_remount_proc_readonly(j); use_pivot_root(j, DEFAULT_PIVOT_ROOT, pivot_root, chroot); - } else { - fprintf(stderr, "Unrecognized profile name '%s'\n", profile); - exit(1); - } + } else + errx(1, "Unrecognized profile name '%s'", profile); } static void set_remount_mode(struct minijail *j, const char *mode) @@ -478,35 +419,25 @@ static void set_remount_mode(struct minijail *j, const char *mode) msmode = MS_SLAVE; else if (!strcmp(mode, "unbindable")) msmode = MS_UNBINDABLE; - else { - fprintf(stderr, "Unknown remount mode: '%s'\n", mode); - exit(1); - } + else + errx(1, "Unknown remount mode: '%s'", mode); minijail_remount_mode(j, msmode); } static void read_seccomp_filter(const char *filter_path, struct sock_fprog *filter) { - FILE *f = fopen(filter_path, "re"); - if (!f) { - fprintf(stderr, "failed to open %s: %m", filter_path); - exit(1); - } + attribute_cleanup_fp FILE *f = fopen(filter_path, "re"); + if (!f) + err(1, "failed to open %s", filter_path); off_t filter_size = 0; - if (fseeko(f, 0, SEEK_END) == -1 || (filter_size = ftello(f)) == -1) { - fclose(f); - fprintf(stderr, "failed to get file size of %s: %m", - filter_path); - exit(1); - } + if (fseeko(f, 0, SEEK_END) == -1 || (filter_size = ftello(f)) == -1) + err(1, "failed to get file size of %s", filter_path); if (filter_size % sizeof(struct sock_filter) != 0) { - fclose(f); - fprintf(stderr, - "filter size (%" PRId64 - ") of %s is not a multiple of %zu: %m", - filter_size, filter_path, sizeof(struct sock_filter)); - exit(1); + errx(1, + "filter size (%" PRId64 ") of %s is not a multiple of" + " %zu", + filter_size, filter_path, sizeof(struct sock_filter)); } rewind(f); @@ -514,116 +445,195 @@ static void read_seccomp_filter(const char *filter_path, filter->filter = xmalloc(filter_size); if (fread(filter->filter, sizeof(struct sock_filter), filter->len, f) != filter->len) { - fclose(f); - fprintf(stderr, "failed read %s: %m", filter_path); - exit(1); + err(1, "failed read %s", filter_path); } - fclose(f); } +/* + * Long options use values starting at 0x100 so that they're out of range of + * bytes which is how command line options are processed. Practically speaking, + * we could get by with the (7-bit) ASCII range, but UTF-8 codepoints would be a + * bit confusing, and honestly there's no reason to "optimize" here. + * + * The long enum values are internal to this file and can freely change at any + * time without breaking anything. Please keep alphabetically ordered. + */ +enum { + /* Everything after this point only have long options. */ + LONG_OPTION_BASE = 0x100, + OPT_ADD_SUPPL_GROUP, + OPT_ALLOW_SPECULATIVE_EXECUTION, + OPT_AMBIENT, + OPT_CONFIG, + OPT_ENV_ADD, + OPT_ENV_RESET, + OPT_LOGGING, + OPT_PRELOAD_LIBRARY, + OPT_PROFILE, + OPT_SECCOMP_BPF_BINARY, + OPT_UTS, +}; + +/* + * NB: When adding new options, prefer long-option only. Add a short option + * only if its meaning is intuitive/obvious at a glance. + * + * Keep this sorted. + */ +static const char optstring[] = + "+a:b:c:de::f:g:hik:lm::nprst::u:vwyzB:C:GHIK::LM::NP:R:S:T:UV:Y"; + +static const struct option long_options[] = { + {"help", no_argument, 0, 'h'}, + {"mount-dev", no_argument, 0, 'd'}, + {"ambient", no_argument, 0, OPT_AMBIENT}, + {"uts", optional_argument, 0, OPT_UTS}, + {"logging", required_argument, 0, OPT_LOGGING}, + {"profile", required_argument, 0, OPT_PROFILE}, + {"preload-library", required_argument, 0, OPT_PRELOAD_LIBRARY}, + {"seccomp-bpf-binary", required_argument, 0, OPT_SECCOMP_BPF_BINARY}, + {"add-suppl-group", required_argument, 0, OPT_ADD_SUPPL_GROUP}, + {"allow-speculative-execution", no_argument, 0, + OPT_ALLOW_SPECULATIVE_EXECUTION}, + {"config", required_argument, 0, OPT_CONFIG}, + {"env-add", required_argument, 0, OPT_ENV_ADD}, + {"env-reset", no_argument, 0, OPT_ENV_RESET}, + {"mount", required_argument, 0, 'k'}, + {"bind-mount", required_argument, 0, 'b'}, + {"ns-mount", no_argument, 0, 'v'}, + {0, 0, 0, 0}, +}; + +/* + * Pull the usage string out into the top-level to help with long-lines. We + * want the output to be wrapped at 80 cols when it's shown to the user in the + * terminal, but we don't want the source wrapped to 80 cols because that will + * effectively make terminal output wrap to much lower levels (like <70). + */ +/* clang-format off */ +static const char help_text[] = +"Account (user/group) options:\n" +" -u <user> Change uid to <user>.\n" +" -g <group> Change gid to <group>.\n" +" -G Inherit supplementary groups from new uid.\n" +" Incompatible with -y or --add-suppl-group.\n" +" -y Keep original uid's supplementary groups.\n" +" Incompatible with -G or --add-suppl-group.\n" +" --add-suppl-group <group>\n" +" Add <group> to the proccess' supplementary groups.\n" +" Can be specified multiple times to add several groups.\n" +" Incompatible with -y or -G.\n" +"\n" +"Mount/path options:\n" +" -b <src[,dst[,writable]]>, --bind-mount <...>\n" +" Bind <src> to <dst>.\n" +" -k <src,dst,fstype[,flags[,data]]>, --mount <...>\n" +" Mount <src> at <dst>. <flags> and <data> can be specified as\n" +" in mount(2). Multiple instances allowed.\n" +" -K Do not change share mode of any existing mounts.\n" +" -K<mode> Mark all existing mounts as <mode> instead of MS_PRIVATE.\n" +" -r Remount /proc read-only (implies -v).\n" +" -d, --mount-dev\n" +" Create a new /dev with a minimal set of device nodes\n" +" (implies -v). See minijail0(1) for exact list.\n" +" -t[size] Mount tmpfs at /tmp (implies -v).\n" +" Optional argument specifies size (default \"64M\").\n" +" -C <dir> chroot(2) to <dir>. Incompatible with -P.\n" +" -P <dir> pivot_root(2) to <dir> (implies -v). Incompatible with -C.\n" +"\n" +"Namespace options:\n" +" -N Enter a new cgroup namespace.\n" +" -l Enter new IPC namespace.\n" +" -v, --ns-mount\n" +" Enter new mount namespace.\n" +" -V <file> Enter specified mount namespace.\n" +" -e[file] Enter new network namespace, or existing |file| if provided.\n" +" -p Enter new pid namespace (implies -vr).\n" +" -I Run as init (pid 1) inside a new pid namespace (implies -p).\n" +" -U Enter new user namespace (implies -p).\n" +" -m[<uid> <loweruid> <count>]\n" +" Set the uid map of a user namespace (implies -pU).\n" +" Same arguments as newuidmap(1); mappings are comma separated.\n" +" With no mapping, map the current uid to root.\n" +" Incompatible with -b without the 'writable' option.\n" +" -M[<gid> <lowergid> <count>]\n" +" Set the gid map of a user namespace (implies -pU).\n" +" Same arguments as newgidmap(1); mappings are comma separated.\n" +" With no mapping, map the current gid to root.\n" +" Incompatible with -b without the 'writable' option.\n" +" --uts[=name] Enter a new UTS namespace (and set hostname).\n" +"\n" +"Seccomp options:\n" +" -S <file> Set seccomp filter using <file>.\n" +" E.g., '-S /usr/share/filters/<prog>.$(uname -m)'.\n" +" Requires -n when not running as root.\n" +" --seccomp-bpf-binary=<f>\n" +" Set a pre-compiled seccomp filter using <f>.\n" +" E.g., '-S /usr/share/filters/<prog>.$(uname -m).bpf'.\n" +" Requires -n when not running as root.\n" +" The user is responsible for ensuring that the binary\n" +" was compiled for the correct architecture / kernel version.\n" +" -L Report blocked syscalls when using seccomp filter.\n" +" If the kernel does not support SECCOMP_RET_LOG, some syscalls\n" +" will automatically be allowed (see below).\n" +" -Y Synchronize seccomp filters across thread group.\n" +" -a <table> Use alternate syscall table <table>.\n" +" -s Use seccomp mode 1 (not the same as -S).\n" +"\n" +"Other options:\n" +" --config <file>\n" +" Load the Minijail configuration file <file>.\n" +" If used, must be specified ahead of other options.\n" +" --profile <p>\n" +" Configure minijail0 to run with the <p> sandboxing profile,\n" +" which is a convenient way to express multiple flags\n" +" that are typically used together.\n" +" See the minijail0(1) man page for the full list.\n" +" -n Set no_new_privs. See prctl(2) for details.\n" +" -c <caps> Restrict caps to <caps>.\n" +" --ambient Raise ambient capabilities. Requires -c.\n" +" -B <mask> Skip setting <mask> securebits when restricting caps (-c).\n" +" By default, SECURE_NOROOT, SECURE_NO_SETUID_FIXUP, and \n" +" SECURE_KEEP_CAPS (with their respective locks) are set.\n" +" -f <file> Write the pid of the jailed process to <file>.\n" +" -i Exit immediately after fork(2); i.e. background the program.\n" +" -z Don't forward signals to jailed process.\n" +" -R <type,cur,max>\n" +" Call setrlimit(3); can be specified multiple times.\n" +" -T <type> Assume <program> is a <type> ELF binary;\n" +" <type> may be 'static' or 'dynamic'.\n" +" This will avoid accessing <program> binary before execve(2).\n" +" Type 'static' will avoid preload hooking.\n" +" -w Create and join a new anonymous session keyring.\n" +" --env-reset Clear the current environment instead of having <program>\n" +" inherit the active environment. Often used to start <program>\n" +" with a minimal sanitized environment.\n" +" --env-add <NAME=value>\n" +" Sets the specified environment variable <NAME>\n" +" in the <program>'s environment before starting it.\n" +"\n" +"Uncommon options:\n" +" --allow-speculative-execution\n" +" Allow speculative execution by disabling mitigations.\n" +" --preload-library=<file>\n" +" Overrides the path to \"" PRELOADPATH "\".\n" +" This is only really useful for local testing.\n" +" --logging=<output>\n" +" Set the logging system output: 'auto' (default),\n" +" 'syslog', or 'stderr'.\n" +" -h Help (this message).\n" +" -H Seccomp filter help message.\n"; +/* clang-format on */ + static void usage(const char *progn) { - size_t i; - /* clang-format off */ - printf("Usage: %s [-dGhHiIKlLnNprRstUvyYz]\n" - " [-a <table>]\n" - " [-b <src>[,[dest][,<writeable>]]] [-k <src>,<dest>,<type>[,<flags>[,<data>]]]\n" - " [-c <caps>] [-C <dir>] [-P <dir>] [-e[file]] [-f <file>] [-g <group>]\n" - " [-m[<uid> <loweruid> <count>]*] [-M[<gid> <lowergid> <count>]*] [--profile <name>]\n" - " [-R <type,cur,max>] [-S <file>] [-t[size]] [-T <type>] [-u <user>] [-V <file>]\n" - " <program> [args...]\n" - " -a <table>: Use alternate syscall table <table>.\n" - " -b <...>: Bind <src> to <dest> in chroot.\n" - " Multiple instances allowed.\n" - " -B <mask>: Skip setting securebits in <mask> when restricting capabilities (-c).\n" - " By default, SECURE_NOROOT, SECURE_NO_SETUID_FIXUP, and \n" - " SECURE_KEEP_CAPS (together with their respective locks) are set.\n" - " There are eight securebits in total.\n" - " -k <...>: Mount <src> at <dest> in chroot.\n" - " <flags> and <data> can be specified as in mount(2).\n" - " Multiple instances allowed.\n" - " -c <caps>: Restrict caps to <caps>.\n" - " -C <dir>: chroot(2) to <dir>.\n" - " Not compatible with -P.\n" - " -P <dir>: pivot_root(2) to <dir> (implies -v).\n" - " Not compatible with -C.\n" - " --mount-dev, Create a new /dev with a minimal set of device nodes (implies -v).\n" - " -d: See the minijail0(1) man page for the exact set.\n" - " -e[file]: Enter new network namespace, or existing one if |file| is provided.\n" - " -f <file>: Write the pid of the jailed process to <file>.\n" - " -g <group>: Change gid to <group>.\n" - " -G: Inherit supplementary groups from new uid.\n" - " Not compatible with -y or --add-suppl-group.\n" - " -y: Keep original uid's supplementary groups.\n" - " Not compatible with -G or --add-suppl-group.\n" - " --add-suppl-group <g>:Add <g> to the proccess' supplementary groups,\n" - " can be specified multiple times to add several groups.\n" - " Not compatible with -y or -G.\n" - " -h: Help (this message).\n" - " -H: Seccomp filter help message.\n" - " -i: Exit immediately after fork(2). The jailed process will run\n" - " in the background.\n" - " -I: Run <program> as init (pid 1) inside a new pid namespace (implies -p).\n" - " -K: Do not change share mode of any existing mounts.\n" - " -K<mode>: Mark all existing mounts as <mode> instead of MS_PRIVATE.\n" - " -l: Enter new IPC namespace.\n" - " -L: Report blocked syscalls when using seccomp filter.\n" - " If the kernel does not support SECCOMP_RET_LOG,\n" - " forces the following syscalls to be allowed:\n" - " ", progn); - /* clang-format on */ - for (i = 0; i < log_syscalls_len; i++) - printf("%s ", log_syscalls[i]); - - /* clang-format off */ - printf("\n" - " -m[map]: Set the uid map of a user namespace (implies -pU).\n" - " Same arguments as newuidmap(1), multiple mappings should be separated by ',' (comma).\n" - " With no mapping, map the current uid to root inside the user namespace.\n" - " Not compatible with -b without the 'writable' option.\n" - " -M[map]: Set the gid map of a user namespace (implies -pU).\n" - " Same arguments as newgidmap(1), multiple mappings should be separated by ',' (comma).\n" - " With no mapping, map the current gid to root inside the user namespace.\n" - " Not compatible with -b without the 'writable' option.\n" - " -n: Set no_new_privs.\n" - " -N: Enter a new cgroup namespace.\n" - " -p: Enter new pid namespace (implies -vr).\n" - " -r: Remount /proc read-only (implies -v).\n" - " -R: Set rlimits, can be specified multiple times.\n" - " -s: Use seccomp mode 1 (not the same as -S).\n" - " -S <file>: Set seccomp filter using <file>.\n" - " E.g., '-S /usr/share/filters/<prog>.$(uname -m)'.\n" - " Requires -n when not running as root.\n" - " -t[size]: Mount tmpfs at /tmp (implies -v).\n" - " Optional argument specifies size (default \"64M\").\n" - " -T <type>: Assume <program> is a <type> ELF binary; <type> can be 'static' or 'dynamic'.\n" - " This will avoid accessing <program> binary before execve(2).\n" - " Type 'static' will avoid preload hooking.\n" - " -u <user>: Change uid to <user>.\n" - " -U: Enter new user namespace (implies -p).\n" - " -v: Enter new mount namespace.\n" - " -V <file>: Enter specified mount namespace.\n" - " -w: Create and join a new anonymous session keyring.\n" - " -Y: Synchronize seccomp filters across thread group.\n" - " -z: Don't forward signals to jailed process.\n" - " --ambient: Raise ambient capabilities. Requires -c.\n" - " --uts[=name]: Enter a new UTS namespace (and set hostname).\n" - " --logging=<s>:Use <s> as the logging system.\n" - " <s> must be 'auto' (default), 'syslog', or 'stderr'.\n" - " --profile <p>:Configure minijail0 to run with the <p> sandboxing profile,\n" - " which is a convenient way to express multiple flags\n" - " that are typically used together.\n" - " See the minijail0(1) man page for the full list.\n" - " --preload-library=<f>:Overrides the path to \"" PRELOADPATH "\".\n" - " This is only really useful for local testing.\n" - " --seccomp-bpf-binary=<f>:Set a pre-compiled seccomp filter using <f>.\n" - " E.g., '-S /usr/share/filters/<prog>.$(uname -m).bpf'.\n" - " Requires -n when not running as root.\n" - " The user is responsible for ensuring that the binary\n" - " was compiled for the correct architecture / kernel version.\n" - " --allow-speculative-execution:Allow speculative execution and disable\n" - " mitigations for speculative execution attacks.\n"); - /* clang-format on */ + printf("Usage: %s [options] [--] <program> [args...]\n\n%s", progn, + help_text); + + printf("\nsyscalls allowed when logging (-L):\n "); + for (size_t i = 0; i < log_syscalls_len; ++i) + printf(" %s", log_syscalls[i]); + printf("\n"); } static void seccomp_filter_usage(const char *progn) @@ -637,12 +647,98 @@ static void seccomp_filter_usage(const char *progn) printf("\nSee minijail0(5) for example policies.\n"); } +/* + * Return the next unconsumed option char/value parsed from + * |*conf_entry_list|. |optarg| is updated to point to an argument from + * the entry value. If all options have been consumed, |*conf_entry_list| + * will be freed and -1 will be returned. + */ +static int getopt_from_conf(const struct option *longopts, + struct config_entry_list **conf_entry_list, + size_t *conf_index) +{ + int opt = -1; + /* If we've consumed all the options in the this config, reset it. */ + if (*conf_index >= (*conf_entry_list)->num_entries) { + free_config_entry_list(*conf_entry_list); + *conf_entry_list = NULL; + *conf_index = 0; + return opt; + } + + struct config_entry *entry = &(*conf_entry_list)->entries[*conf_index]; + /* Look up a matching long option. */ + size_t i = 0; + const struct option *curr_opt; + for (curr_opt = &longopts[0]; curr_opt->name != NULL; + curr_opt = &longopts[++i]) + if (strcmp(entry->key, curr_opt->name) == 0) + break; + if (curr_opt->name == NULL) { + errx(1, + "Unable to recognize '%s' as Minijail conf entry key, " + "please refer to minijail0(5) for syntax and examples.", + entry->key); + } + opt = curr_opt->val; + optarg = (char *)entry->value; + (*conf_index)++; + return opt; +} + +/* + * Similar to getopt(3), return the next option char/value as it + * parses through the CLI argument list. Config entries in + * |*conf_entry_list| will be parsed with precendences over cli options. + * Same as getopt(3), |optarg| is pointing to the option argument. + */ +static int getopt_conf_or_cli(int argc, char *const argv[], + struct config_entry_list **conf_entry_list, + size_t *conf_index) +{ + int opt = -1; + if (*conf_entry_list != NULL) + opt = + getopt_from_conf(long_options, conf_entry_list, conf_index); + if (opt == -1) + opt = getopt_long(argc, argv, optstring, long_options, NULL); + return opt; +} + +static void set_child_env(char ***envp, char *arg, char *const environ[]) +{ + /* We expect VAR=value format for arg. */ + char *delim = strchr(arg, '='); + if (!delim) { + errx(1, "Expected an argument of the " + "form VAR=value (got '%s')", arg); + } + *delim = '\0'; + const char *env_value = delim + 1; + if (!*envp) { + /* + * We got our first --env-add. Initialize *envp by + * copying our current env to the future child env. + */ + *envp = minijail_copy_env(environ); + if (!*envp) + err(1, "Failed to allocate memory."); + } + if (minijail_setenv(envp, arg, env_value, 1)) + err(1, "minijail_setenv() failed."); +} + int parse_args(struct minijail *j, int argc, char *const argv[], - int *exit_immediately, ElfType *elftype, - const char **preload_path) + char *const environ[], int *exit_immediately, + ElfType *elftype, const char **preload_path, + char ***envp) { + enum seccomp_type { None, Strict, Filter, BpfBinaryFilter }; + enum seccomp_type seccomp = None; int opt; - int use_seccomp_filter = 0, use_seccomp_filter_binary = 0; + int use_seccomp_filter = 0; + int use_seccomp_filter_binary = 0; + int use_seccomp_log = 0; int forward = 1; int binding = 0; int chroot = 0, pivot_root = 0; @@ -650,7 +746,6 @@ int parse_args(struct minijail *j, int argc, char *const argv[], const char *remount_mode = NULL; int inherit_suppl_gids = 0, keep_suppl_gids = 0; int caps = 0, ambient_caps = 0; - int seccomp = -1; bool use_uid = false, use_gid = false; uid_t uid = 0; gid_t gid = 0; @@ -661,43 +756,21 @@ int parse_args(struct minijail *j, int argc, char *const argv[], size_t tmp_size = 0; const char *filter_path = NULL; int log_to_stderr = -1; + struct config_entry_list *conf_entry_list = NULL; + size_t conf_index = 0; - const char *optstring = - "+u:g:sS:c:C:P:b:B:V:f:m::M::k:a:e::R:T:vrGhHinNplLt::IUK::wyYzd"; - /* clang-format off */ - const struct option long_options[] = { - {"help", no_argument, 0, 'h'}, - {"mount-dev", no_argument, 0, 'd'}, - {"ambient", no_argument, 0, 128}, - {"uts", optional_argument, 0, 129}, - {"logging", required_argument, 0, 130}, - {"profile", required_argument, 0, 131}, - {"preload-library", required_argument, 0, 132}, - {"seccomp-bpf-binary", required_argument, 0, 133}, - {"add-suppl-group", required_argument, 0, 134}, - {"allow-speculative-execution", no_argument, 0, 135}, - {0, 0, 0, 0}, - }; - /* clang-format on */ - - while ((opt = getopt_long(argc, argv, optstring, long_options, NULL)) != - -1) { + while ((opt = getopt_conf_or_cli(argc, argv, &conf_entry_list, + &conf_index)) != -1) { switch (opt) { case 'u': - if (use_uid) { - fprintf(stderr, - "-u provided multiple times.\n"); - exit(1); - } + if (use_uid) + errx(1, "-u provided multiple times."); use_uid = true; set_user(j, optarg, &uid, &gid); break; case 'g': - if (use_gid) { - fprintf(stderr, - "-g provided multiple times.\n"); - exit(1); - } + if (use_gid) + errx(1, "-g provided multiple times."); use_gid = true; set_group(j, optarg, &gid); break; @@ -705,23 +778,19 @@ int parse_args(struct minijail *j, int argc, char *const argv[], minijail_no_new_privs(j); break; case 's': - if (seccomp != -1 && seccomp != 1) { - fprintf(stderr, - "Do not use -s, -S, or " - "--seccomp-bpf-binary together.\n"); - exit(1); + if (seccomp != None && seccomp != Strict) { + errx(1, "Do not use -s, -S, or " + "--seccomp-bpf-binary together"); } - seccomp = 1; + seccomp = Strict; minijail_use_seccomp(j); break; case 'S': - if (seccomp != -1 && seccomp != 2) { - fprintf(stderr, - "Do not use -s, -S, or " - "--seccomp-bpf-binary together.\n"); - exit(1); + if (seccomp != None && seccomp != Filter) { + errx(1, "Do not use -s, -S, or " + "--seccomp-bpf-binary together"); } - seccomp = 2; + seccomp = Filter; minijail_use_seccomp_filter(j); filter_path = optarg; use_seccomp_filter = 1; @@ -730,6 +799,11 @@ int parse_args(struct minijail *j, int argc, char *const argv[], minijail_namespace_ipc(j); break; case 'L': + if (seccomp == BpfBinaryFilter) { + errx(1, "-L does not work with " + "--seccomp-bpf-binary"); + } + use_seccomp_log = 1; minijail_log_seccomp_filter_failures(j); break; case 'b': @@ -757,11 +831,8 @@ int parse_args(struct minijail *j, int argc, char *const argv[], use_pivot_root(j, optarg, &pivot_root, chroot); break; case 'f': - if (0 != minijail_write_pid_file(j, optarg)) { - fprintf(stderr, - "Could not prepare pid file path.\n"); - exit(1); - } + if (0 != minijail_write_pid_file(j, optarg)) + errx(1, "Could not prepare pid file path"); break; case 't': minijail_namespace_vfs(j); @@ -774,8 +845,7 @@ int parse_args(struct minijail *j, int argc, char *const argv[], } if (optarg != NULL && 0 != parse_size(&tmp_size, optarg)) { - fprintf(stderr, "Invalid /tmp tmpfs size.\n"); - exit(1); + errx(1, "Invalid /tmp tmpfs size"); } break; case 'v': @@ -820,20 +890,14 @@ int parse_args(struct minijail *j, int argc, char *const argv[], minijail_remount_proc_readonly(j); break; case 'G': - if (keep_suppl_gids) { - fprintf(stderr, - "-y and -G are not compatible.\n"); - exit(1); - } + if (keep_suppl_gids) + errx(1, "-y and -G are not compatible"); minijail_inherit_usergroups(j); inherit_suppl_gids = 1; break; case 'y': - if (inherit_suppl_gids) { - fprintf(stderr, - "-y and -G are not compatible.\n"); - exit(1); - } + if (inherit_suppl_gids) + errx(1, "-y and -G are not compatible"); minijail_keep_supplementary_gids(j); keep_suppl_gids = 1; break; @@ -882,11 +946,8 @@ int parse_args(struct minijail *j, int argc, char *const argv[], gidmap = xstrdup(optarg); break; case 'a': - if (0 != minijail_use_alt_syscall(j, optarg)) { - fprintf(stderr, - "Could not set alt-syscall table.\n"); - exit(1); - } + if (0 != minijail_use_alt_syscall(j, optarg)) + errx(1, "Could not set alt-syscall table"); break; case 'R': add_rlimit(j, optarg); @@ -897,9 +958,8 @@ int parse_args(struct minijail *j, int argc, char *const argv[], else if (!strcmp(optarg, "dynamic")) *elftype = ELFDYNAMIC; else { - fprintf(stderr, "ELF type must be 'static' or " - "'dynamic'.\n"); - exit(1); + errx(1, "ELF type must be 'static' or " + "'dynamic'"); } break; case 'w': @@ -916,53 +976,125 @@ int parse_args(struct minijail *j, int argc, char *const argv[], minijail_mount_dev(j); break; /* Long options. */ - case 128: /* Ambient caps. */ + case OPT_AMBIENT: ambient_caps = 1; minijail_set_ambient_caps(j); break; - case 129: /* UTS/hostname namespace. */ + case OPT_UTS: minijail_namespace_uts(j); if (optarg) minijail_namespace_set_hostname(j, optarg); break; - case 130: /* Logging. */ - if (!strcmp(optarg, "auto")) { + case OPT_LOGGING: + if (!strcmp(optarg, "auto")) log_to_stderr = -1; - } else if (!strcmp(optarg, "syslog")) { + else if (!strcmp(optarg, "syslog")) log_to_stderr = 0; - } else if (!strcmp(optarg, "stderr")) { + else if (!strcmp(optarg, "stderr")) log_to_stderr = 1; - } else { - fprintf(stderr, "--logger must be 'syslog' or " - "'stderr'.\n"); - exit(1); - } + else + errx(1, + "--logger must be 'syslog' or 'stderr'"); break; - case 131: /* Profile */ + case OPT_PROFILE: use_profile(j, optarg, &pivot_root, chroot, &tmp_size); break; - case 132: /* PRELOADPATH */ + case OPT_PRELOAD_LIBRARY: *preload_path = optarg; break; - case 133: /* seccomp-bpf binary. */ - if (seccomp != -1 && seccomp != 3) { - fprintf(stderr, - "Do not use -s, -S, or " - "--seccomp-bpf-binary together.\n"); - exit(1); + case OPT_SECCOMP_BPF_BINARY: + if (seccomp != None && seccomp != BpfBinaryFilter) { + errx(1, "Do not use -s, -S, or " + "--seccomp-bpf-binary together"); } - seccomp = 3; + if (use_seccomp_log == 1) + errx(1, "-L does not work with " + "--seccomp-bpf-binary"); + seccomp = BpfBinaryFilter; minijail_use_seccomp_filter(j); filter_path = optarg; use_seccomp_filter_binary = 1; break; - case 134: - suppl_group_add(&suppl_gids_count, &suppl_gids, - optarg); + case OPT_ADD_SUPPL_GROUP: + suppl_group_add(&suppl_gids_count, &suppl_gids, optarg); break; - case 135: + case OPT_ALLOW_SPECULATIVE_EXECUTION: minijail_set_seccomp_filter_allow_speculation(j); break; + case OPT_CONFIG: { + if (conf_entry_list != NULL) { + errx(1, "Nested config file specification is " + "not allowed."); + } + conf_entry_list = new_config_entry_list(); + conf_index = 0; +#if defined(BLOCK_NOEXEC_CONF) + /* + * Check the conf file is in a exec mount. + * With a W^X invariant, it excludes writable + * mounts. + */ + struct statfs conf_statfs; + if (statfs(optarg, &conf_statfs) != 0) + err(1, "statfs(%s) failed.", optarg); + if ((conf_statfs.f_flags & MS_NOEXEC) != 0) + errx(1, + "Conf file must be in a exec " + "mount: %s", + optarg); +#endif +#if defined(ENFORCE_ROOTFS_CONF) + /* Make sure the conf file is in the same device as the + * rootfs. */ + struct stat root_stat; + struct stat conf_stat; + if (stat("/", &root_stat) != 0) + err(1, "stat(/) failed."); + if (stat(optarg, &conf_stat) != 0) + err(1, "stat(%s) failed.", optarg); + if (root_stat.st_dev != conf_stat.st_dev) + errx(1, "Conf file must be in the rootfs."); +#endif + attribute_cleanup_fp FILE *config_file = + fopen(optarg, "re"); + if (!config_file) + err(1, "Failed to open %s", optarg); + if (!parse_config_file(config_file, conf_entry_list)) { + errx( + 1, + "Unable to parse %s as Minijail conf file, " + "please refer to minijail0(5) for syntax " + "and examples.", + optarg); + } + break; + } + case OPT_ENV_ADD: + /* + * We either copy our current env to the child env + * then add the requested envvar to it, or just + * add the requested envvar to the already existing + * envp. + */ + set_child_env(envp, optarg, environ); + break; + case OPT_ENV_RESET: + if (*envp && *envp != environ) { + /* + * We already started to initialize the future + * child env, because we got some --env-add + * earlier on the command-line, so first, + * free the memory we allocated. + * If |*envp| happens to point to |environ|, + * don't attempt to free it. + */ + minijail_free_env(*envp); + } + /* Allocate an empty environment for the child. */ + *envp = calloc(1, sizeof(char *)); + if (!*envp) + err(1, "Failed to allocate memory."); + break; default: usage(argv[0]); exit(opt == 'h' ? 0 : 1); @@ -980,8 +1112,7 @@ int parse_args(struct minijail *j, int argc, char *const argv[], */ if (0 != minijail_preserve_fd(j, STDERR_FILENO, STDERR_FILENO)) { - fprintf(stderr, "Could not preserve stderr.\n"); - exit(1); + errx(1, "Could not preserve stderr"); } } @@ -993,9 +1124,8 @@ int parse_args(struct minijail *j, int argc, char *const argv[], /* Can only set ambient caps when using regular caps. */ if (ambient_caps && !caps) { - fprintf(stderr, "Can't set ambient capabilities (--ambient) " - "without actually using capabilities (-c).\n"); - exit(1); + errx(1, "Can't set ambient capabilities (--ambient) " + "without actually using capabilities (-c)"); } /* Set up signal handlers in minijail unless asked not to. */ @@ -1007,9 +1137,8 @@ int parse_args(struct minijail *j, int argc, char *const argv[], * a new mount namespace. */ if (binding && !(chroot || pivot_root || mount_ns)) { - fprintf(stderr, "Bind mounts require a chroot, pivot_root, or " - " new mount namespace.\n"); - exit(1); + errx(1, "Bind mounts require a chroot, pivot_root, or " + " new mount namespace"); } /* @@ -1017,11 +1146,10 @@ int parse_args(struct minijail *j, int argc, char *const argv[], * that's set there is no need for the -K/-K<mode> flags. */ if (change_remount && !mount_ns) { - fprintf(stderr, "No need to use -K (skip remounting '/') or " - "-K<mode> (remount '/' as <mode>)\n" - "without -v (new mount namespace).\n" - "Do you need to add '-v' explicitly?\n"); - exit(1); + errx(1, "No need to use -K (skip remounting '/') or " + "-K<mode> (remount '/' as <mode>) " + "without -v (new mount namespace).\n" + "Do you need to add '-v' explicitly?"); } /* Configure the remount flag here to avoid having -v override it. */ @@ -1039,7 +1167,7 @@ int parse_args(struct minijail *j, int argc, char *const argv[], */ if (suppl_gids_count) { minijail_set_supplementary_gids(j, suppl_gids_count, - suppl_gids); + suppl_gids); free(suppl_gids); } @@ -1061,6 +1189,16 @@ int parse_args(struct minijail *j, int argc, char *const argv[], minijail_mount_tmp_size(j, tmp_size); /* + * Copy our current env to the child if its |*envp| has not + * already been initialized from --env-(reset|add) usage. + */ + if (!*envp) { + *envp = minijail_copy_env(environ); + if (!*envp) + err(1, "Failed to allocate memory."); + } + + /* * There should be at least one additional unparsed argument: the * executable name. */ @@ -1079,10 +1217,8 @@ int parse_args(struct minijail *j, int argc, char *const argv[], /* Check that we can access the target program. */ if (access(program_path, X_OK)) { - fprintf(stderr, - "Target program '%s' is not accessible.\n", - argv[optind]); - exit(1); + errx(1, "Target program '%s' is not accessible", + argv[optind]); } /* Check if target is statically or dynamically linked. */ @@ -1096,11 +1232,9 @@ int parse_args(struct minijail *j, int argc, char *const argv[], * execve(2). */ if (caps && *elftype == ELFSTATIC && !ambient_caps) { - fprintf(stderr, "Can't run statically-linked binaries with " - "capabilities (-c) without also setting " - "ambient capabilities. Try passing " - "--ambient.\n"); - exit(1); + errx(1, "Can't run statically-linked binaries with capabilities" + " (-c) without also setting ambient capabilities. " + "Try passing --ambient."); } return optind; diff --git a/minijail0_cli.h b/minijail0_cli.h index 583c763..cd504b3 100644 --- a/minijail0_cli.h +++ b/minijail0_cli.h @@ -17,8 +17,9 @@ extern "C" { struct minijail; int parse_args(struct minijail *j, int argc, char *const argv[], - int *exit_immediately, ElfType *elftype, - const char **preload_path); + char *const environ[], int *exit_immediately, + ElfType *elftype, const char **preload_path, + char ***envp); #ifdef __cplusplus }; /* extern "C" */ diff --git a/minijail0_cli_unittest.cc b/minijail0_cli_unittest.cc index 76ff37f..7b20ecd 100644 --- a/minijail0_cli_unittest.cc +++ b/minijail0_cli_unittest.cc @@ -14,8 +14,10 @@ #include <gtest/gtest.h> +#include "config_parser.h" #include "libminijail.h" #include "minijail0_cli.h" +#include "test_util.h" namespace { @@ -58,9 +60,10 @@ class CliTest : public ::testing::Test { testing::internal::CaptureStdout(); const char* preload_path = PRELOADPATH; + char **envp = NULL; int ret = parse_args(j, pargv.size(), const_cast<char* const*>(pargv.data()), - exit_immediately, elftype, &preload_path); + NULL, exit_immediately, elftype, &preload_path, &envp); testing::internal::GetCapturedStdout(); minijail_destroy(j); @@ -526,3 +529,80 @@ TEST_F(CliTest, invalid_remount_mode) { argv[1] = "-Kfoo"; ASSERT_EXIT(parse_args_(argv), testing::ExitedWithCode(1), ""); } + +TEST_F(CliTest, invalid_L_combo) { + std::vector<std::string> argv = {"", "", "", "/bin/sh"}; + + // Cannot call minijail0 with -L and a pre-compiled seccomp policy. + argv[0] = "-L"; + argv[1] = "--seccomp-bpf-binary"; + argv[2] = "source"; + ASSERT_EXIT(parse_args_(argv), testing::ExitedWithCode(1), ""); + + argv[0] = "--seccomp-bpf-binary"; + argv[1] = "source"; + argv[2] = "-L"; + ASSERT_EXIT(parse_args_(argv), testing::ExitedWithCode(1), ""); +} + +// Valid calls to the clear env option. +TEST_F(CliTest, valid_clear_env) { + std::vector<std::string> argv = {"--env-reset", "/bin/sh"}; + + ASSERT_TRUE(parse_args_(argv)); +} + +// Valid calls to the set env option. +TEST_F(CliTest, valid_set_env) { + std::vector<std::string> argv1 = {"--env-add", "NAME=value", "/bin/sh"}; + ASSERT_TRUE(parse_args_(argv1)); + + // multiple occurences are allowed. + std::vector<std::string> argv2 = {"--env-add", "A=b", + "--env-add", "b=C=D", "/bin/sh"}; + ASSERT_TRUE(parse_args_(argv2)); + + // --env-reset before any --env-add to not pass our own env. + std::vector<std::string> argv3 = {"--env-reset", "--env-add", "A=b", "/bin/sh"}; + ASSERT_TRUE(parse_args_(argv3)); + + // --env-add before an --env-reset doesn't have any effect, but is allowed. + std::vector<std::string> argv4 = {"--env-add", "A=b", "--env-reset", "/bin/sh"}; + ASSERT_TRUE(parse_args_(argv4)); +} + +// Invalid calls to the set env options. +TEST_F(CliTest, invalid_set_env) { + + // invalid env=value arguments. + std::vector<std::string> argv2 = {"--env-add", "", "/bin/sh"}; + + argv2[1] = "INVALID"; + ASSERT_EXIT(parse_args_(argv2), testing::ExitedWithCode(1), ""); + + argv2[1] = "="; + ASSERT_EXIT(parse_args_(argv2), testing::ExitedWithCode(1), ""); + + argv2[1] = "=foo"; + ASSERT_EXIT(parse_args_(argv2), testing::ExitedWithCode(1), ""); +} + +// Android unit tests do not support data file yet. +#if !defined(__ANDROID__) + +TEST_F(CliTest, conf_parsing_invalid_key) { + std::vector<std::string> argv = {"--config", source_path("test/invalid.conf"), + "/bin/sh"}; + + ASSERT_EXIT(parse_args_(argv), testing::ExitedWithCode(1), ""); +} + +TEST_F(CliTest, conf_parsing) { + std::vector<std::string> argv = {"--config", + source_path("test/valid.conf"), + "/bin/sh"}; + + ASSERT_TRUE(parse_args_(argv)); +} + +#endif // !__ANDROID__ diff --git a/parse_seccomp_policy.cc b/parse_seccomp_policy.cc index 000b80d..a6daac5 100644 --- a/parse_seccomp_policy.cc +++ b/parse_seccomp_policy.cc @@ -47,8 +47,8 @@ void Usage(const char* progn, int status) { int main(int argc, char** argv) { init_logging(LOG_TO_FD, STDERR_FILENO, LOG_INFO); - const char* optstring = "d:h"; - const struct option long_options[] = { + static const char optstring[] = "d:h"; + static const struct option long_options[] = { {"help", no_argument, 0, 'h'}, {"dump", optional_argument, 0, 'd'}, {0, 0, 0, 0}, diff --git a/rust/OWNERS b/rust/OWNERS index 1db5ac3..f8111bb 100644 --- a/rust/OWNERS +++ b/rust/OWNERS @@ -2,5 +2,5 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. -smbarber@google.com -zachr@google.com +allenwebb@google.com +victorhsieh@google.com diff --git a/rust/minijail-sys/Cargo.toml b/rust/minijail-sys/Cargo.toml index 934c605..4c49c95 100644 --- a/rust/minijail-sys/Cargo.toml +++ b/rust/minijail-sys/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "minijail-sys" -version = "0.0.11" +version = "0.0.13" description = "Provides raw (unsafe) bindings to the libminijail C library." authors = ["The Chromium OS Authors"] edition = "2018" @@ -14,3 +14,4 @@ libc = "0.2.44" [build-dependencies] pkg-config = "0.3" +which = "4.0.0" diff --git a/rust/minijail-sys/build.rs b/rust/minijail-sys/build.rs index 8c7ea6d..f73191c 100644 --- a/rust/minijail-sys/build.rs +++ b/rust/minijail-sys/build.rs @@ -7,10 +7,35 @@ /// This script prefers linking against a pkg-config provided libminijail, but will fall back to /// building libminijail statically. use std::env; +use std::fs::remove_file; use std::io; +use std::path::Path; use std::process::Command; -fn main() -> io::Result<()> { +/// Returns the target triplet prefix for gcc commands. No prefix is required +/// for native builds. +fn get_cross_compile_prefix() -> String { + if let Ok(cross_compile) = env::var("CROSS_COMPILE") { + return cross_compile; + } + + let target = env::var("TARGET").unwrap(); + + if env::var("HOST").unwrap() == target { + return String::from(""); + } + + let arch = env::var("CARGO_CFG_TARGET_ARCH").unwrap(); + let os = env::var("CARGO_CFG_TARGET_OS").unwrap(); + let env = if target.ends_with("-gnueabihf") { + String::from("gnueabihf") + } else { + env::var("CARGO_CFG_TARGET_ENV").unwrap() + }; + return format!("{}-{}-{}-", arch, os, env); +} + +fn set_up_libminijail() -> io::Result<()> { // Minijail requires libcap at runtime. pkg_config::Config::new().probe("libcap").unwrap(); @@ -27,9 +52,10 @@ fn main() -> io::Result<()> { .current_dir(&out_dir) .env("OUT", &out_dir) .env("MODE", if profile == "release" { "opt" } else { "debug" }) + .env("CROSS_COMPILE", get_cross_compile_prefix()) + .env("BUILD_STATIC_LIBS", "yes") .arg("-C") .arg(¤t_dir) - .arg("CC_STATIC_LIBRARY(libminijail.pic.a)") .status()?; if !status.success() { std::process::exit(status.code().unwrap_or(1)); @@ -38,3 +64,53 @@ fn main() -> io::Result<()> { println!("cargo:rustc-link-lib=static=minijail.pic"); Ok(()) } + +fn bindings_generation() -> io::Result<()> { + let bindgen = match which::which("bindgen") { + Ok(v) => v, + // Use already generated copy if bindgen is not present. + _ => return Ok(()), + }; + + // If CROS_RUST is set, skip generation. + let gen_file = Path::new("./libminijail.rs"); + if gen_file.exists() { + if env::var("CROS_RUST") == Ok(String::from("1")) { + return Ok(()); + } + remove_file(gen_file).expect("Failed to remove generated file."); + } + let header_dir = Path::new("../../"); + let header_path = header_dir.join("libminijail.h"); + println!("cargo:rerun-if-changed={}", header_path.display()); + let status = Command::new(&bindgen) + .args(&["--default-enum-style", "rust"]) + .args(&["--blacklist-type", "__rlim64_t"]) + .args(&["--raw-line", "pub type __rlim64_t = u64;"]) + .args(&["--blacklist-type", "__u\\d{1,2}"]) + .args(&["--raw-line", "pub type __u8 = u8;"]) + .args(&["--raw-line", "pub type __u16 = u16;"]) + .args(&["--raw-line", "pub type __u32 = u32;"]) + .args(&["--blacklist-type", "__uint64_t"]) + .args(&["--whitelist-function", "^minijail_.*"]) + .args(&["--whitelist-var", "^MINIJAIL_.*"]) + .arg("--no-layout-tests") + .arg("--disable-header-comment") + .args(&["--output", gen_file.to_str().unwrap()]) + .arg(header_path.to_str().unwrap()) + .args(&[ + "--", + "-DUSE_BINDGEN", + "-D_FILE_OFFSET_BITS=64", + "-D_LARGEFILE_SOURCE", + "-D_LARGEFILE64_SOURCE", + ]) + .status()?; + assert!(status.success()); + Ok(()) +} + +fn main() -> io::Result<()> { + set_up_libminijail()?; + bindings_generation() +} diff --git a/rust/minijail-sys/lib.rs b/rust/minijail-sys/lib.rs index 59db51e..c418150 100644 --- a/rust/minijail-sys/lib.rs +++ b/rust/minijail-sys/lib.rs @@ -37,6 +37,11 @@ // // Enum variants in rust are customarily camel case, but bindgen will leave the original names // intact. -#[allow(non_camel_case_types)] +#[allow( + clippy::all, + non_camel_case_types, + non_snake_case, + non_upper_case_globals +)] mod libminijail; pub use crate::libminijail::*; diff --git a/rust/minijail-sys/libminijail.rs b/rust/minijail-sys/libminijail.rs index 594a479..42b1a8d 100644 --- a/rust/minijail-sys/libminijail.rs +++ b/rust/minijail-sys/libminijail.rs @@ -1,6 +1,3 @@ -/* automatically generated by rust-bindgen */ -#![allow(clippy::all)] - pub type __rlim64_t = u64; pub type __u8 = u8; pub type __u16 = u16; @@ -13,6 +10,7 @@ pub type rlim_t = __rlim64_t; pub type gid_t = __gid_t; pub type uid_t = __uid_t; pub type pid_t = __pid_t; +pub type size_t = ::std::os::raw::c_ulong; #[repr(C)] pub struct sock_filter { pub code: __u16, @@ -29,15 +27,17 @@ pub struct sock_fprog { pub const MINIJAIL_ERR_NO_ACCESS: _bindgen_ty_1 = _bindgen_ty_1::MINIJAIL_ERR_NO_ACCESS; pub const MINIJAIL_ERR_NO_COMMAND: _bindgen_ty_1 = _bindgen_ty_1::MINIJAIL_ERR_NO_COMMAND; pub const MINIJAIL_ERR_SIG_BASE: _bindgen_ty_1 = _bindgen_ty_1::MINIJAIL_ERR_SIG_BASE; +pub const MINIJAIL_ERR_MOUNT: _bindgen_ty_1 = _bindgen_ty_1::MINIJAIL_ERR_MOUNT; pub const MINIJAIL_ERR_PRELOAD: _bindgen_ty_1 = _bindgen_ty_1::MINIJAIL_ERR_PRELOAD; pub const MINIJAIL_ERR_JAIL: _bindgen_ty_1 = _bindgen_ty_1::MINIJAIL_ERR_JAIL; pub const MINIJAIL_ERR_INIT: _bindgen_ty_1 = _bindgen_ty_1::MINIJAIL_ERR_INIT; #[repr(u32)] -#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] pub enum _bindgen_ty_1 { MINIJAIL_ERR_NO_ACCESS = 126, MINIJAIL_ERR_NO_COMMAND = 127, MINIJAIL_ERR_SIG_BASE = 128, + MINIJAIL_ERR_MOUNT = 251, MINIJAIL_ERR_PRELOAD = 252, MINIJAIL_ERR_JAIL = 253, MINIJAIL_ERR_INIT = 254, @@ -51,7 +51,7 @@ pub type minijail_hook_t = ::std::option::Option< unsafe extern "C" fn(context: *mut ::std::os::raw::c_void) -> ::std::os::raw::c_int, >; #[repr(u32)] -#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] pub enum minijail_hook_event_t { MINIJAIL_HOOK_EVENT_PRE_DROP_CAPS = 0, MINIJAIL_HOOK_EVENT_PRE_EXECVE = 1, @@ -68,7 +68,7 @@ extern "C" { pub fn minijail_change_gid(j: *mut minijail, gid: gid_t); } extern "C" { - pub fn minijail_set_supplementary_gids(j: *mut minijail, size: usize, list: *const gid_t); + pub fn minijail_set_supplementary_gids(j: *mut minijail, size: size_t, list: *const gid_t); } extern "C" { pub fn minijail_keep_supplementary_gids(j: *mut minijail); @@ -98,6 +98,9 @@ extern "C" { pub fn minijail_set_seccomp_filter_tsync(j: *mut minijail); } extern "C" { + pub fn minijail_set_seccomp_filter_allow_speculation(j: *mut minijail); +} +extern "C" { pub fn minijail_set_seccomp_filters(j: *mut minijail, filter: *const sock_fprog); } extern "C" { @@ -253,7 +256,7 @@ extern "C" { pub fn minijail_mount_tmp(j: *mut minijail); } extern "C" { - pub fn minijail_mount_tmp_size(j: *mut minijail, size: usize); + pub fn minijail_mount_tmp_size(j: *mut minijail, size: size_t); } extern "C" { pub fn minijail_mount_dev(j: *mut minijail); @@ -286,7 +289,11 @@ extern "C" { ) -> ::std::os::raw::c_int; } extern "C" { - pub fn minijail_copy_jail(from: *const minijail, out: *mut minijail) -> ::std::os::raw::c_int; + pub fn minijail_add_remount( + j: *mut minijail, + mount_name: *const ::std::os::raw::c_char, + remount_mode: ::std::os::raw::c_ulong, + ) -> ::std::os::raw::c_int; } extern "C" { pub fn minijail_add_hook( @@ -354,6 +361,30 @@ extern "C" { ) -> ::std::os::raw::c_int; } extern "C" { + pub fn minijail_run_env_pid_pipes( + j: *mut minijail, + filename: *const ::std::os::raw::c_char, + argv: *const *mut ::std::os::raw::c_char, + envp: *const *mut ::std::os::raw::c_char, + pchild_pid: *mut pid_t, + pstdin_fd: *mut ::std::os::raw::c_int, + pstdout_fd: *mut ::std::os::raw::c_int, + pstderr_fd: *mut ::std::os::raw::c_int, + ) -> ::std::os::raw::c_int; +} +extern "C" { + pub fn minijail_run_fd_env_pid_pipes( + j: *mut minijail, + elf_fd: ::std::os::raw::c_int, + argv: *const *mut ::std::os::raw::c_char, + envp: *const *mut ::std::os::raw::c_char, + pchild_pid: *mut pid_t, + pstdin_fd: *mut ::std::os::raw::c_int, + pstdout_fd: *mut ::std::os::raw::c_int, + pstderr_fd: *mut ::std::os::raw::c_int, + ) -> ::std::os::raw::c_int; +} +extern "C" { pub fn minijail_run_pid_pipes_no_preload( j: *mut minijail, filename: *const ::std::os::raw::c_char, @@ -389,5 +420,8 @@ extern "C" { pub fn minijail_destroy(j: *mut minijail); } extern "C" { + pub fn minijail_copy_jail(from: *const minijail, out: *mut minijail) -> ::std::os::raw::c_int; +} +extern "C" { pub fn minijail_log_to_fd(fd: ::std::os::raw::c_int, min_priority: ::std::os::raw::c_int); } diff --git a/rust/minijail/Cargo.toml b/rust/minijail/Cargo.toml index db0652f..e6c08b8 100644 --- a/rust/minijail/Cargo.toml +++ b/rust/minijail/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "minijail" -version = "0.2.1" +version = "0.2.3" description = "Provides a safe Rust friendly interface to libminijail." authors = ["The Chromium OS Authors"] edition = "2018" diff --git a/rust/minijail/src/lib.rs b/rust/minijail/src/lib.rs index ba59075..5028041 100644 --- a/rust/minijail/src/lib.rs +++ b/rust/minijail/src/lib.rs @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -use libc::pid_t; -use minijail_sys::*; use std::ffi::CString; use std::fmt::{self, Display}; use std::fs; @@ -12,6 +10,154 @@ use std::os::raw::{c_char, c_ulong, c_ushort}; use std::os::unix::io::{AsRawFd, IntoRawFd, RawFd}; use std::path::{Path, PathBuf}; use std::ptr::{null, null_mut}; +use std::result::Result as StdResult; + +use libc::pid_t; +use minijail_sys::*; + +enum Program { + Filename(PathBuf), + FileDescriptor(RawFd), +} + +/// Configuration of a command to be run in a jail. +pub struct Command { + program: Program, + preserve_fds: Vec<(RawFd, RawFd)>, + + // Ownership of the backing data of args_cptr is provided by args_cstr. + args_cstr: Vec<CString>, + args_cptr: Vec<*const c_char>, + + // Ownership of the backing data of env_cptr is provided by env_cstr. + env_cstr: Option<Vec<CString>>, + env_cptr: Option<Vec<*const c_char>>, +} + +impl Command { + /// This exposes a subset of what Command can do, before we are ready to commit to a stable + /// API. + pub fn new_for_path<P: AsRef<Path>, S: AsRef<str>, A: AsRef<str>>( + path: P, + keep_fds: &[RawFd], + args: &[S], + env_vars: Option<&[A]>, + ) -> Result<Command> { + let mut cmd = Command::new(Program::Filename(path.as_ref().to_path_buf())) + .keep_fds(keep_fds) + .args(args)?; + if let Some(env_vars) = env_vars { + cmd = cmd.envs(env_vars)?; + } + + Ok(cmd) + } + + fn new(program: Program) -> Command { + Command { + program, + preserve_fds: Vec::new(), + args_cstr: Vec::new(), + args_cptr: Vec::new(), + env_cstr: None, + env_cptr: None, + } + } + + fn keep_fds(mut self, keep_fds: &[RawFd]) -> Command { + self.preserve_fds = keep_fds + .iter() + .map(|&a| (a, a)) + .collect::<Vec<(RawFd, RawFd)>>(); + self + } + + fn remap_fds(mut self, remap_fds: &[(RawFd, RawFd)]) -> Command { + self.preserve_fds = remap_fds.to_vec(); + self + } + + fn args<S: AsRef<str>>(mut self, args: &[S]) -> Result<Command> { + let (args_cstr, args_cptr) = to_execve_cstring_array(args)?; + self.args_cstr = args_cstr; + self.args_cptr = args_cptr; + Ok(self) + } + + fn envs<S: AsRef<str>>(mut self, vars: &[S]) -> Result<Command> { + let (env_cstr, env_cptr) = to_execve_cstring_array(vars)?; + self.env_cstr = Some(env_cstr); + self.env_cptr = Some(env_cptr); + Ok(self) + } + + fn argv(&self) -> *const *mut c_char { + self.args_cptr.as_ptr() as *const *mut c_char + } + + fn envp(&self) -> *const *mut c_char { + (match self.env_cptr { + Some(ref env_cptr) => env_cptr.as_ptr(), + None => null_mut(), + }) as *const *mut c_char + } +} + +/// Abstracts paths and executable file descriptors in a way that the run implementation can cover +/// both. +trait Runnable { + fn run_command(&self, jail: &Minijail, cmd: &Command) -> Result<pid_t>; +} + +impl Runnable for &Path { + fn run_command(&self, jail: &Minijail, cmd: &Command) -> Result<pid_t> { + let path_str = self + .to_str() + .ok_or_else(|| Error::PathToCString(self.to_path_buf()))?; + let path_cstr = + CString::new(path_str).map_err(|_| Error::StrToCString(path_str.to_owned()))?; + + let mut pid: pid_t = 0; + let ret = unsafe { + minijail_run_env_pid_pipes( + jail.jail, + path_cstr.as_ptr(), + cmd.argv(), + cmd.envp(), + &mut pid, + null_mut(), + null_mut(), + null_mut(), + ) + }; + if ret < 0 { + return Err(Error::ForkingMinijail(ret)); + } + Ok(pid) + } +} + +impl Runnable for RawFd { + fn run_command(&self, jail: &Minijail, cmd: &Command) -> Result<pid_t> { + let mut pid: pid_t = 0; + let ret = unsafe { + minijail_run_fd_env_pid_pipes( + jail.jail, + *self, + cmd.argv(), + cmd.envp(), + &mut pid, + null_mut(), + null_mut(), + null_mut(), + ) + }; + if ret < 0 { + return Err(Error::ForkingMinijail(ret)); + } + Ok(pid) + } +} #[derive(Debug)] pub enum Error { @@ -83,6 +229,8 @@ pub enum Error { Killed(u8), /// Process finished returning a non-zero code. ReturnCode(u8), + /// Failed to wait the process. + Wait(i32), } impl Display for Error { @@ -171,13 +319,14 @@ impl Display for Error { SeccompViolation(s) => write!(f, "seccomp violation syscall #{}", s), Killed(s) => write!(f, "killed with signal number {}", s), ReturnCode(e) => write!(f, "exited with code {}", e), + Wait(errno) => write!(f, "failed to wait: {}", io::Error::from_raw_os_error(*errno)), } } } impl std::error::Error for Error {} -pub type Result<T> = std::result::Result<T, Error>; +pub type Result<T> = StdResult<T, Error>; /// Configuration to jail a process based on wrapping libminijail. /// @@ -233,6 +382,9 @@ fn translate_wait_error(ret: libc::c_int) -> Result<()> { if ret == 0 { return Ok(()); } + if ret < 0 { + return Err(Error::Wait(ret)); + } if ret == MINIJAIL_ERR_NO_COMMAND as libc::c_int { return Err(Error::NoCommand); } @@ -249,7 +401,7 @@ fn translate_wait_error(ret: libc::c_int) -> Result<()> { if ret > 0 && ret <= 0xff { return Err(Error::ReturnCode(ret as u8)); } - unreachable!(); + unreachable!("Unexpected returned value from wait: {}", ret); } impl Minijail { @@ -313,7 +465,7 @@ impl Minijail { } pub fn set_supplementary_gids(&mut self, ids: &[libc::gid_t]) { unsafe { - minijail_set_supplementary_gids(self.jail, ids.len(), ids.as_ptr()); + minijail_set_supplementary_gids(self.jail, ids.len() as size_t, ids.as_ptr()); } } pub fn keep_supplementary_gids(&mut self) { @@ -358,6 +510,9 @@ impl Minijail { } let buffer = fs::read(path).map_err(Error::ReadProgram)?; + self.parse_seccomp_bytes(&buffer) + } + pub fn parse_seccomp_bytes(&mut self, buffer: &[u8]) -> Result<()> { if buffer.len() % std::mem::size_of::<sock_filter>() != 0 { return Err(Error::WrongProgramSize); } @@ -612,7 +767,7 @@ impl Minijail { } pub fn mount_tmp_size(&mut self, size: usize) { unsafe { - minijail_mount_tmp_size(self.jail, size); + minijail_mount_tmp_size(self.jail, size as size_t); } } pub fn mount_bind<P1: AsRef<Path>, P2: AsRef<Path>>( @@ -662,13 +817,10 @@ impl Minijail { inheritable_fds: &[RawFd], args: &[S], ) -> Result<pid_t> { - self.run_remap( - cmd, - &inheritable_fds - .iter() - .map(|&a| (a, a)) - .collect::<Vec<(RawFd, RawFd)>>(), - args, + self.run_internal( + Command::new(Program::Filename(cmd.as_ref().to_path_buf())) + .keep_fds(inheritable_fds) + .args(args)?, ) } @@ -680,25 +832,49 @@ impl Minijail { inheritable_fds: &[(RawFd, RawFd)], args: &[S], ) -> Result<pid_t> { - let cmd_os = cmd - .as_ref() - .to_str() - .ok_or_else(|| Error::PathToCString(cmd.as_ref().to_owned()))?; - let cmd_cstr = CString::new(cmd_os).map_err(|_| Error::StrToCString(cmd_os.to_owned()))?; + self.run_internal( + Command::new(Program::Filename(cmd.as_ref().to_path_buf())) + .remap_fds(inheritable_fds) + .args(args)?, + ) + } - // Converts each incoming `args` string to a `CString`, and then puts each `CString` pointer - // into a null terminated array, suitable for use as an argv parameter to `execve`. - let mut args_cstr = Vec::with_capacity(args.len()); - let mut args_array = Vec::with_capacity(args.len()); - for arg in args { - let arg_cstr = CString::new(arg.as_ref()) - .map_err(|_| Error::StrToCString(arg.as_ref().to_owned()))?; - args_array.push(arg_cstr.as_ptr()); - args_cstr.push(arg_cstr); - } - args_array.push(null()); + /// Behaves the same as `run()` except cmd is a file descriptor to the executable. + pub fn run_fd<F: AsRawFd, S: AsRef<str>>( + &self, + cmd: &F, + inheritable_fds: &[RawFd], + args: &[S], + ) -> Result<pid_t> { + self.run_internal( + Command::new(Program::FileDescriptor(cmd.as_raw_fd())) + .keep_fds(inheritable_fds) + .args(args)?, + ) + } - for (src_fd, dst_fd) in inheritable_fds { + /// Behaves the same as `run()` except cmd is a file descriptor to the executable, and + /// `inheritable_fds` is a list of fd mappings rather than just a list of fds to preserve. + pub fn run_fd_remap<F: AsRawFd, S: AsRef<str>>( + &self, + cmd: &F, + inheritable_fds: &[(RawFd, RawFd)], + args: &[S], + ) -> Result<pid_t> { + self.run_internal( + Command::new(Program::FileDescriptor(cmd.as_raw_fd())) + .remap_fds(inheritable_fds) + .args(args)?, + ) + } + + /// A generic version of `run()` that is a super set of all variants. + pub fn run_command(&self, cmd: Command) -> Result<pid_t> { + self.run_internal(cmd) + } + + fn run_internal(&self, cmd: Command) -> Result<pid_t> { + for (src_fd, dst_fd) in cmd.preserve_fds.iter() { let ret = unsafe { minijail_preserve_fd(self.jail, *src_fd, *dst_fd) }; if ret < 0 { return Err(Error::PreservingFd(ret)); @@ -713,7 +889,7 @@ impl Minijail { // Set stdin, stdout, and stderr to /dev/null unless they are in the inherit list. // These will only be closed when this process exits. for io_fd in &[libc::STDIN_FILENO, libc::STDOUT_FILENO, libc::STDERR_FILENO] { - if !inheritable_fds.iter().any(|(_, fd)| *fd == *io_fd) { + if !cmd.preserve_fds.iter().any(|(_, fd)| *fd == *io_fd) { let ret = unsafe { minijail_preserve_fd(self.jail, dev_null.as_raw_fd(), *io_fd) }; if ret < 0 { return Err(Error::PreservingFd(ret)); @@ -725,22 +901,10 @@ impl Minijail { minijail_close_open_fds(self.jail); } - let mut pid = 0; - let ret = unsafe { - minijail_run_pid_pipes( - self.jail, - cmd_cstr.as_ptr(), - args_array.as_ptr() as *const *mut c_char, - &mut pid, - null_mut(), - null_mut(), - null_mut(), - ) - }; - if ret < 0 { - return Err(Error::ForkingMinijail(ret)); + match cmd.program { + Program::Filename(ref path) => path.as_path().run_command(&self, &cmd), + Program::FileDescriptor(fd) => fd.run_command(&self, &cmd), } - Ok(pid) } /// Forks a child and puts it in the previously configured minijail. @@ -860,15 +1024,54 @@ fn is_single_threaded() -> io::Result<bool> { } } +fn to_execve_cstring_array<S: AsRef<str>>( + slice: &[S], +) -> Result<(Vec<CString>, Vec<*const c_char>)> { + // Converts each incoming `str` to a `CString`, and then puts each `CString` pointer into a + // null terminated array, suitable for use as an argv or envp parameter to `execve`. + let mut vec_cstr = Vec::with_capacity(slice.len()); + let mut vec_cptr = Vec::with_capacity(slice.len() + 1); + for s in slice { + let cstr = + CString::new(s.as_ref()).map_err(|_| Error::StrToCString(s.as_ref().to_owned()))?; + + vec_cstr.push(cstr); + vec_cptr.push(vec_cstr.last().unwrap().as_ptr()); + } + + vec_cptr.push(null()); + + Ok((vec_cstr, vec_cptr)) +} + #[cfg(test)] mod tests { - use std::process::exit; - use super::*; + use std::fs::File; + + use libc::{FD_CLOEXEC, F_GETFD, F_SETFD}; + const SHELL: &str = "/bin/sh"; const EMPTY_STRING_SLICE: &[&str] = &[]; + fn clear_cloexec<A: AsRawFd>(fd_owner: &A) -> StdResult<(), io::Error> { + let fd = fd_owner.as_raw_fd(); + // Safe because fd is read only. + let flags = unsafe { libc::fcntl(fd, F_GETFD) }; + if flags == -1 { + return Err(io::Error::last_os_error()); + } + + let masked_flags = flags & !FD_CLOEXEC; + // Safe because this has no side effect(s) on the current process. + if masked_flags != flags && unsafe { libc::fcntl(fd, F_SETFD, masked_flags) } == -1 { + Err(io::Error::last_os_error()) + } else { + Ok(()) + } + } + #[test] fn create_and_free() { unsafe { @@ -889,9 +1092,7 @@ mod tests { j.no_new_privs(); j.parse_seccomp_filters("src/test_filter.policy").unwrap(); j.use_seccomp_filter(); - if unsafe { j.fork(None).unwrap() } == 0 { - exit(0); - } + j.run("/bin/true", &[], &EMPTY_STRING_SLICE).unwrap(); } #[test] @@ -903,14 +1104,28 @@ mod tests { let j = Minijail::new().unwrap(); let first = libc::open(FILE_PATH.as_ptr() as *const c_char, libc::O_RDONLY); assert!(first >= 0); + // This appears unused but its function is to be a file descriptor that is closed + // inside run_remap after the fork. If it is not closed, the script will fail. let second = libc::open(FILE_PATH.as_ptr() as *const c_char, libc::O_RDONLY); assert!(second >= 0); - let fds: Vec<RawFd> = vec![0, 1, 2, first]; - if j.fork(Some(&fds)).unwrap() == 0 { - assert!(libc::close(second) < 0); // Should fail as second should be closed already. - assert_eq!(libc::close(first), 0); // Should succeed as first should be untouched. - exit(0); - } + + let fds: Vec<(RawFd, RawFd)> = vec![(first, 0), (1, 1), (2, 2)]; + j.run_remap( + SHELL, + &fds, + &[ + SHELL, + "-c", + r#" +if [ `ls -l /proc/self/fd/ | grep '/dev/null' | wc -l` != '1' ]; then + ls -l /proc/self/fd/ # Included to make debugging easier. + exit 1 +fi +"#, + ], + ) + .unwrap(); + j.wait().unwrap(); } } @@ -964,14 +1179,29 @@ mod tests { let j = Minijail::new().unwrap(); j.run("/bin/does not exist", &[1, 2], &EMPTY_STRING_SLICE) .unwrap(); - expect_result!(j.wait(), Err(Error::NoCommand)); + // TODO(b/194221986) Fix libminijail so that Error::NoAccess is not sometimes returned. + assert!(matches!( + j.wait(), + Err(Error::NoCommand) | Err(Error::NoAccess) + )); + } + + #[test] + fn runnable_fd_success() { + let bin_file = File::open("/bin/true").unwrap(); + // On Chrome OS targets /bin/true is actually a script, so drop CLOEXEC to prevent ENOENT. + clear_cloexec(&bin_file).unwrap(); + + let j = Minijail::new().unwrap(); + j.run_fd(&bin_file, &[1, 2], &EMPTY_STRING_SLICE).unwrap(); + expect_result!(j.wait(), Ok(())); } #[test] fn kill_success() { let j = Minijail::new().unwrap(); j.run( - Path::new("usr/bin/sleep"), + Path::new("/usr/bin/sleep"), &[1, 2], &["/usr/bin/sleep", "5"], ) @@ -985,9 +1215,7 @@ mod tests { fn chroot() { let mut j = Minijail::new().unwrap(); j.enter_chroot(".").unwrap(); - if unsafe { j.fork(None).unwrap() } == 0 { - exit(0); - } + j.run("/bin/true", &[], &EMPTY_STRING_SLICE).unwrap(); } #[test] @@ -995,9 +1223,7 @@ mod tests { fn namespace_vfs() { let mut j = Minijail::new().unwrap(); j.namespace_vfs(); - if unsafe { j.fork(None).unwrap() } == 0 { - exit(0); - } + j.run("/bin/true", &[], &EMPTY_STRING_SLICE).unwrap(); } #[test] diff --git a/syscall_filter.c b/syscall_filter.c index fcdbaa8..de5441c 100644 --- a/syscall_filter.c +++ b/syscall_filter.c @@ -568,49 +568,6 @@ int parse_include_statement(struct parser_state *state, char *policy_line, return 0; } -/* - * This is like getline() but supports line wrapping with \. - */ -static ssize_t getmultiline(char **lineptr, size_t *n, FILE *stream) -{ - ssize_t ret = getline(lineptr, n, stream); - if (ret < 0) - return ret; - - char *line = *lineptr; - /* Eat the newline to make processing below easier. */ - if (ret > 0 && line[ret - 1] == '\n') - line[--ret] = '\0'; - - /* If the line doesn't end in a backslash, we're done. */ - if (ret <= 0 || line[ret - 1] != '\\') - return ret; - - /* This line ends in a backslash. Get the nextline. */ - line[--ret] = '\0'; - size_t next_n = 0; - char *next_line = NULL; - ssize_t next_ret = getmultiline(&next_line, &next_n, stream); - if (next_ret == -1) { - free(next_line); - /* We couldn't fully read the line, so return an error. */ - return -1; - } - - /* Merge the lines. */ - *n = ret + next_ret + 2; - line = realloc(line, *n); - if (!line) { - free(next_line); - return -1; - } - line[ret] = ' '; - memcpy(&line[ret + 1], next_line, next_ret + 1); - free(next_line); - *lineptr = line; - return *n - 1; -} - int compile_file(const char *filename, FILE *policy_file, struct filter_block *head, struct filter_block **arg_blocks, struct bpf_labels *labels, @@ -632,7 +589,7 @@ int compile_file(const char *filename, FILE *policy_file, * Chain the filter sections together and dump them into * the final buffer at the end. */ - char *line = NULL; + attribute_cleanup_str char *line = NULL; size_t len = 0; int ret = 0; @@ -667,15 +624,16 @@ int compile_file(const char *filename, FILE *policy_file, &state, "failed to parse include statement"); ret = -1; - goto free_line; + goto out; } - FILE *included_file = fopen(filename, "re"); + attribute_cleanup_fp FILE *included_file = + fopen(filename, "re"); if (included_file == NULL) { compiler_pwarn(&state, "fopen('%s') failed", filename); ret = -1; - goto free_line; + goto out; } if (compile_file(filename, included_file, head, arg_blocks, labels, filteropts, @@ -683,11 +641,9 @@ int compile_file(const char *filename, FILE *policy_file, include_level + 1) == -1) { compiler_warn(&state, "'@include %s' failed", filename); - fclose(included_file); ret = -1; - goto free_line; + goto out; } - fclose(included_file); continue; } @@ -700,14 +656,14 @@ int compile_file(const char *filename, FILE *policy_file, warn("compile_file: malformed policy line, missing " "':'"); ret = -1; - goto free_line; + goto out; } policy_line = strip(policy_line); if (*policy_line == '\0') { compiler_warn(&state, "empty policy line"); ret = -1; - goto free_line; + goto out; } syscall_name = strip(syscall_name); @@ -732,7 +688,7 @@ int compile_file(const char *filename, FILE *policy_file, continue; } ret = -1; - goto free_line; + goto out; } if (!insert_and_check_duplicate_syscall(previous_syscalls, @@ -775,7 +731,7 @@ int compile_file(const char *filename, FILE *policy_file, } warn("could not allocate filter block"); ret = -1; - goto free_line; + goto out; } if (*arg_blocks) { @@ -796,8 +752,7 @@ int compile_file(const char *filename, FILE *policy_file, ret = -1; } -free_line: - free(line); +out: return ret; } diff --git a/syscall_filter_unittest.cc b/syscall_filter_unittest.cc index 74f79da..79755f9 100644 --- a/syscall_filter_unittest.cc +++ b/syscall_filter_unittest.cc @@ -15,21 +15,11 @@ #include "bpf.h" #include "syscall_filter.h" #include "syscall_filter_unittest_macros.h" +#include "test_util.h" #include "util.h" namespace { -// TODO(jorgelo): Android unit tests don't currently support data files. -// Re-enable by creating a temporary policy file at runtime. -#if !defined(__ANDROID__) - -std::string source_path(std::string file) { - std::string srcdir = getenv("SRC") ? : "."; - return srcdir + "/" + file; -} - -#endif - // Simple C++ -> C wrappers to simplify test code. enum ret_trap { @@ -44,7 +34,7 @@ enum use_logging { }; int test_compile_filter( - std::string filename, + const std::string& filename, FILE* policy_file, struct sock_fprog* prog, enum block_action action = ACTION_RET_KILL, @@ -89,7 +79,7 @@ int test_compile_file( struct filter_block* test_compile_policy_line( struct parser_state* state, int nr, - std::string policy_line, + const std::string& policy_line, unsigned int label_id, struct bpf_labels* labels, enum block_action action = ACTION_RET_KILL) { @@ -510,7 +500,7 @@ TEST_F(ArgFilterTest, arg0_equals_log) { } TEST_F(ArgFilterTest, arg0_short_gt_ge_comparisons) { - for (std::string fragment : + for (const std::string fragment : {"arg1 < 0xff", "arg1 <= 0xff", "arg1 > 0xff", "arg1 >= 0xff"}) { struct filter_block* block = test_compile_policy_line(&state_, nr_, fragment, id_, &labels_); @@ -552,7 +542,7 @@ TEST_F(ArgFilterTest, arg0_short_gt_ge_comparisons) { #if defined(BITS64) TEST_F(ArgFilterTest, arg0_long_gt_ge_comparisons) { - for (std::string fragment : + for (const std::string fragment : {"arg1 < 0xbadc0ffee0ddf00d", "arg1 <= 0xbadc0ffee0ddf00d", "arg1 > 0xbadc0ffee0ddf00d", "arg1 >= 0xbadc0ffee0ddf00d"}) { struct filter_block* block = @@ -1040,44 +1030,6 @@ TEST_F(ArgFilterTest, no_log_bad_ret_error) { namespace { -FILE* write_policy_to_pipe(std::string policy) { - int pipefd[2]; - if (pipe(pipefd) == -1) { - pwarn("pipe(pipefd) failed"); - return nullptr; - } - - size_t len = policy.length(); - size_t i = 0; - unsigned int attempts = 0; - ssize_t ret; - while (i < len) { - ret = write(pipefd[1], policy.c_str() + i, len - i); - if (ret == -1) { - close(pipefd[0]); - close(pipefd[1]); - return nullptr; - } - - /* If we write 0 bytes three times in a row, fail. */ - if (ret == 0) { - if (++attempts >= 3) { - close(pipefd[0]); - close(pipefd[1]); - warn("write() returned 0 three times in a row"); - return nullptr; - } - continue; - } - - attempts = 0; - i += (size_t)ret; - } - - close(pipefd[1]); - return fdopen(pipefd[0], "r"); -} - class FileTest : public ::testing::Test { protected: virtual void SetUp() { @@ -1101,7 +1053,7 @@ TEST_F(FileTest, malformed_policy) { std::string policy = "malformed"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_file("policy", policy_file, head_, &arg_blocks_, &labels_); @@ -1118,7 +1070,7 @@ TEST_F(FileTest, double_free_on_compile_error) { "read:arg0 == 0\n" "write:0"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_file("policy", policy_file, head_, &arg_blocks_, &labels_); @@ -1134,7 +1086,7 @@ TEST_F(FileTest, invalid_return) { std::string policy = "read:arg0 == 0; ;"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_file("policy", policy_file, head_, &arg_blocks_, &labels_); @@ -1153,7 +1105,7 @@ TEST_F(FileTest, seccomp_mode1) { "rt_sigreturn: 1\n" "exit: 1\n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_file("policy", policy_file, head_, &arg_blocks_, &labels_); @@ -1188,7 +1140,7 @@ TEST_F(FileTest, seccomp_read) { const int LABEL_ID = 0; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_file("policy", policy_file, head_, &arg_blocks_, &labels_); @@ -1255,7 +1207,7 @@ TEST_F(FileTest, multiline) { const int LABEL_ID = 0; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_file("policy", policy_file, head_, &arg_blocks_, &labels_); @@ -1291,7 +1243,7 @@ TEST(FilterTest, seccomp_mode1) { "rt_sigreturn: 1\n" "exit: 1\n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); @@ -1328,7 +1280,7 @@ TEST(FilterTest, seccomp_mode1_with_check) { "rt_sigreturn: 1\n" "exit: 1\n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual, @@ -1373,7 +1325,7 @@ TEST(FilterTest, duplicate_read_with_args) { "rt_sigreturn: 1\n" "exit: 1\n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); fclose(policy_file); @@ -1393,7 +1345,7 @@ TEST(FilterTest, duplicate_read_with_one_arg) { "rt_sigreturn: 1\n" "exit: 1\n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); fclose(policy_file); @@ -1412,7 +1364,7 @@ TEST(FilterTest, seccomp_mode1_trap) { "rt_sigreturn: 1\n" "exit: 1\n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = @@ -1451,7 +1403,7 @@ TEST(FilterTest, seccomp_mode1_log) { "rt_sigreturn: 1\n" "exit: 1\n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual, ACTION_RET_LOG, @@ -1490,7 +1442,7 @@ TEST(FilterTest, seccomp_mode1_log_fails) { "rt_sigreturn: 1\n" "exit: 1\n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual, ACTION_RET_LOG, @@ -1511,7 +1463,7 @@ TEST(FilterTest, seccomp_mode1_ret_kill_process) { "rt_sigreturn: 1\n" "exit: 1\n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual, ACTION_RET_KILL_PROCESS, @@ -1550,7 +1502,7 @@ TEST(FilterTest, seccomp_read_write) { "rt_sigreturn: 1\n" "exit: 1\n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); @@ -1591,7 +1543,7 @@ TEST(FilterTest, misplaced_whitespace) { struct sock_fprog actual; std::string policy = "read :1\n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); @@ -1609,7 +1561,7 @@ TEST(FilterTest, missing_atom) { struct sock_fprog actual; std::string policy = "open:\n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); @@ -1621,7 +1573,7 @@ TEST(FilterTest, whitespace_atom) { struct sock_fprog actual; std::string policy = "open:\t \n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); @@ -1633,7 +1585,7 @@ TEST(FilterTest, invalid_name) { struct sock_fprog actual; std::string policy = "notasyscall: 1\n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); @@ -1645,7 +1597,7 @@ TEST(FilterTest, invalid_arg) { struct sock_fprog actual; std::string policy = "open: argnn ==\n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); @@ -1657,7 +1609,7 @@ TEST(FilterTest, invalid_tokens) { struct sock_fprog actual; std::string policy = "read: arg0 == 1 |||| arg0 == 2\n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); @@ -1679,7 +1631,7 @@ TEST(FilterTest, log) { "rt_sigreturn: 1\n" "exit: 1\n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual, ACTION_RET_TRAP, @@ -1726,7 +1678,7 @@ TEST(FilterTest, allow_log_but_kill) { "rt_sigreturn: 1\n" "exit: 1\n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual, ACTION_RET_KILL, @@ -1769,7 +1721,7 @@ TEST(FilterTest, frequency) { struct sock_fprog actual; std::string frequency = "@frequency ./path/is/ignored.frequency\n"; - FILE* policy_file = write_policy_to_pipe(frequency); + FILE* policy_file = write_to_pipe(frequency); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); fclose(policy_file); @@ -1782,7 +1734,7 @@ TEST(FilterTest, include_invalid_token) { struct sock_fprog actual; std::string invalid_token = "@unclude ./test/seccomp.policy\n"; - FILE* policy_file = write_policy_to_pipe(invalid_token); + FILE* policy_file = write_to_pipe(invalid_token); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); fclose(policy_file); @@ -1793,7 +1745,7 @@ TEST(FilterTest, include_no_space) { struct sock_fprog actual; std::string no_space = "@includetest/seccomp.policy\n"; - FILE* policy_file = write_policy_to_pipe(no_space); + FILE* policy_file = write_to_pipe(no_space); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); fclose(policy_file); @@ -1804,7 +1756,7 @@ TEST(FilterTest, include_double_token) { struct sock_fprog actual; std::string double_token = "@includeinclude ./test/seccomp.policy\n"; - FILE* policy_file = write_policy_to_pipe(double_token); + FILE* policy_file = write_to_pipe(double_token); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); fclose(policy_file); @@ -1815,7 +1767,7 @@ TEST(FilterTest, include_no_file) { struct sock_fprog actual; std::string no_file = "@include\n"; - FILE* policy_file = write_policy_to_pipe(no_file); + FILE* policy_file = write_to_pipe(no_file); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); fclose(policy_file); @@ -1826,7 +1778,7 @@ TEST(FilterTest, include_space_no_file) { struct sock_fprog actual; std::string space_no_file = "@include \n"; - FILE* policy_file = write_policy_to_pipe(space_no_file); + FILE* policy_file = write_to_pipe(space_no_file); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); fclose(policy_file); @@ -1837,7 +1789,7 @@ TEST(FilterTest, include_implicit_relative_path) { struct sock_fprog actual; std::string implicit_relative_path = "@include test/seccomp.policy\n"; - FILE* policy_file = write_policy_to_pipe(implicit_relative_path); + FILE* policy_file = write_to_pipe(implicit_relative_path); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); fclose(policy_file); @@ -1848,7 +1800,7 @@ TEST(FilterTest, include_extra_text) { struct sock_fprog actual; std::string extra_text = "@include /some/file: sneaky comment\n"; - FILE* policy_file = write_policy_to_pipe(extra_text); + FILE* policy_file = write_to_pipe(extra_text); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); fclose(policy_file); @@ -1859,7 +1811,7 @@ TEST(FilterTest, include_split_filename) { struct sock_fprog actual; std::string split_filename = "@include /some/file:colon.policy\n"; - FILE* policy_file = write_policy_to_pipe(split_filename); + FILE* policy_file = write_to_pipe(split_filename); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); fclose(policy_file); @@ -1870,7 +1822,7 @@ TEST(FilterTest, include_nonexistent_file) { struct sock_fprog actual; std::string include_policy = "@include ./nonexistent.policy\n"; - FILE* policy_file = write_policy_to_pipe(include_policy); + FILE* policy_file = write_to_pipe(include_policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); @@ -1893,7 +1845,7 @@ TEST(FilterTest, include) { "rt_sigreturn: 1\n" "exit: 1\n"; - FILE* file_plain = write_policy_to_pipe(policy_plain); + FILE* file_plain = write_to_pipe(policy_plain); ASSERT_NE(file_plain, nullptr); int res_plain = test_compile_filter("policy", file_plain, &compiled_plain, ACTION_RET_KILL); @@ -1902,7 +1854,7 @@ TEST(FilterTest, include) { std::string policy_with_include = "@include " + source_path("test/seccomp.policy") + "\n"; - FILE* file_with_include = write_policy_to_pipe(policy_with_include); + FILE* file_with_include = write_to_pipe(policy_with_include); ASSERT_NE(file_with_include, nullptr); int res_with_include = test_compile_filter( "policy", file_with_include, &compiled_with_include, ACTION_RET_KILL); @@ -1949,7 +1901,7 @@ TEST(FilterTest, include_same_syscalls) { "exit: 1\n" "@include " + source_path("test/seccomp.policy") + "\n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); @@ -1971,7 +1923,7 @@ TEST(FilterTest, include_same_syscalls_with_check) { "exit: 1\n" "@include " + source_path("test/seccomp.policy") + "\n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual, @@ -1988,7 +1940,7 @@ TEST(FilterTest, include_two) { "@include " + source_path("test/seccomp.policy") + "\n" + "@include " + source_path("test/seccomp.policy") + "\n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); @@ -2010,7 +1962,7 @@ TEST(FilterTest, include_invalid_policy) { "exit: 1\n" "@include ./test/invalid_syscall_name.policy\n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); /* Ensure the included (invalid) policy file exists. */ @@ -2029,7 +1981,7 @@ TEST(FilterTest, include_nested) { struct sock_fprog actual; std::string policy = "@include ./test/nested.policy\n"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); /* Ensure the policy file exists. */ @@ -2051,7 +2003,7 @@ TEST(FilterTest, error_cleanup_leak) { "read:&&\n" "read:&&"; - FILE* policy_file = write_policy_to_pipe(policy); + FILE* policy_file = write_to_pipe(policy); ASSERT_NE(policy_file, nullptr); int res = test_compile_filter("policy", policy_file, &actual); fclose(policy_file); @@ -111,7 +111,8 @@ int lock_securebits(uint64_t skip_mask, bool require_keep_caps) int write_proc_file(pid_t pid, const char *content, const char *basename) { - int fd, ret; + attribute_cleanup_fd int fd = -1; + int ret; size_t sz, len; ssize_t written; char filename[32]; @@ -140,7 +141,6 @@ int write_proc_file(pid_t pid, const char *content, const char *basename) warn("failed to write %zu bytes to '%s'", len, filename); return -1; } - close(fd); return 0; } @@ -167,8 +167,10 @@ unsigned int get_last_valid_cap(void) last_valid_cap--; } } else { - const char cap_file[] = "/proc/sys/kernel/cap_last_cap"; + static const char cap_file[] = "/proc/sys/kernel/cap_last_cap"; FILE *fp = fopen(cap_file, "re"); + if (!fp) + pdie("fopen(%s)", cap_file); if (fscanf(fp, "%u", &last_valid_cap) != 1) pdie("fscanf(%s)", cap_file); fclose(fp); @@ -185,7 +187,7 @@ int cap_ambient_supported(void) int config_net_loopback(void) { const char ifname[] = "lo"; - int sock; + attribute_cleanup_fd int sock = -1; struct ifreq ifr; /* Make sure people don't try to add really long names. */ @@ -214,7 +216,6 @@ int config_net_loopback(void) return -1; } - close(sock); return 0; } @@ -229,6 +230,7 @@ int write_pid_to_path(pid_t pid, const char *path) if (fprintf(fp, "%d\n", (int)pid) < 0) { /* fprintf(3) does not set errno on failure. */ warn("fprintf(%s) failed", path); + fclose(fp); return -1; } if (fclose(fp)) { @@ -365,13 +367,13 @@ int setup_mount_destination(const char *source, const char *dest, uid_t uid, if (rc) return rc; if (!domkdir) { - int fd = open(dest, O_RDWR | O_CREAT | O_CLOEXEC, 0700); + attribute_cleanup_fd int fd = open( + dest, O_RDWR | O_CREAT | O_CLOEXEC, 0700); if (fd < 0) { rc = errno; pwarn("open(%s) failed", dest); return -rc; } - close(fd); } if (chown(dest, uid, gid)) { rc = errno; @@ -498,11 +500,10 @@ static bool seccomp_action_is_available(const char *wanted) return false; } - char *actions_avail = NULL; + attribute_cleanup_str char *actions_avail = NULL; size_t buf_size = 0; if (getline(&actions_avail, &buf_size, f) < 0) { pwarn("getline() failed"); - free(actions_avail); return false; } @@ -512,9 +513,7 @@ static bool seccomp_action_is_available(const char *wanted) * seccomp actions which include other actions though, so we're good for * now. Eventually we might want to split the string by spaces. */ - bool available = strstr(actions_avail, wanted) != NULL; - free(actions_avail); - return available; + return strstr(actions_avail, wanted) != NULL; } int seccomp_ret_log_available(void) diff --git a/test/invalid.conf b/test/invalid.conf new file mode 100644 index 0000000..d15d289 --- /dev/null +++ b/test/invalid.conf @@ -0,0 +1,3 @@ +% minijail-config-file v0 +# Comments +bad-key = whatever diff --git a/test/valid.conf b/test/valid.conf new file mode 100644 index 0000000..a574e6c --- /dev/null +++ b/test/valid.conf @@ -0,0 +1,8 @@ +% minijail-config-file v0 +# Comments +# enable mount namespace +ns-mount +# mounts and bind-mounts +mount = none,/,none +bind-mount = /,/ +mount-dev diff --git a/test_util.cc b/test_util.cc new file mode 100644 index 0000000..cb751ff --- /dev/null +++ b/test_util.cc @@ -0,0 +1,60 @@ +/* Copyright 2021 The Chromium OS Authors. All rights reserved. + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "test_util.h" + +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> + +#include "util.h" + +#define MAX_PIPE_CAPACITY (4096) + +FILE *write_to_pipe(const std::string& content) +{ + int pipefd[2]; + if (pipe(pipefd) == -1) { + die("pipe(pipefd) failed"); + } + + size_t len = content.length(); + if (len > MAX_PIPE_CAPACITY) + die("write_to_pipe cannot handle >4KB content."); + size_t i = 0; + unsigned int attempts = 0; + ssize_t ret; + while (i < len) { + ret = write(pipefd[1], content.c_str() + i, len - i); + if (ret == -1) { + close(pipefd[0]); + close(pipefd[1]); + return NULL; + } + + /* If we write 0 bytes three times in a row, fail. */ + if (ret == 0) { + if (++attempts >= 3) { + close(pipefd[0]); + close(pipefd[1]); + warn("write() returned 0 three times in a row"); + return NULL; + } + continue; + } + + attempts = 0; + i += (size_t)ret; + } + + close(pipefd[1]); + return fdopen(pipefd[0], "r"); +} + +std::string source_path(const std::string& file) { + std::string srcdir = getenv("SRC") ? : "."; + return srcdir + "/" + file; +} diff --git a/test_util.h b/test_util.h new file mode 100644 index 0000000..e915086 --- /dev/null +++ b/test_util.h @@ -0,0 +1,69 @@ +/* test_util.h + * Copyright 2021 The Chromium OS Authors. All rights reserved. + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + * + * Utility functions in testing. + */ + +#ifndef _TEST_UTIL_H_ +#define _TEST_UTIL_H_ + +#include <stdio.h> + +#include <memory> +#include <string> + +#include "config_parser.h" + +namespace mj { + +namespace internal { + +// Functor for |ScopedFILE| (below). +struct ScopedFILECloser { + inline void operator()(FILE *x) const { + if (x) { + fclose(x); + } + } +}; + +// Functor for |ScopedConfigEntry| (below). +struct ScopedConfigEntryDeleter { + inline void operator()(config_entry *entry) const { + if (entry) { + free(entry); + } + } +}; + +} // namespace internal + +} // namespace mj + +using ScopedFILE = std::unique_ptr<FILE, mj::internal::ScopedFILECloser>; +using ScopedConfigEntry = + std::unique_ptr<config_entry, mj::internal::ScopedConfigEntryDeleter>; + +/* + * write_to_pipe: write a string as the file content into a pipe based + * file handle. This is particularly useful when testing with temporary data + * files, without dealing with complexities such as relative file path, file + * permission and etc. However, a pipe has limited capacity so write_to_pipe + * will hang when a big enough string is written. This is for use in testing + * only. + * + * Returns a FILE* that contains @content. + */ + +FILE *write_to_pipe(const std::string& content); + +/* + * source_path: return the path to a test fixture located in the current + * source tree. This uses the `SRC` environment variable as the root of the + * tree, falling back to the current directory. + */ +std::string source_path(const std::string& file); + +#endif /* _TEST_UTIL_H_ */ diff --git a/tools/compile_seccomp_policy.py b/tools/compile_seccomp_policy.py index f2b714b..2219ae5 100755 --- a/tools/compile_seccomp_policy.py +++ b/tools/compile_seccomp_policy.py @@ -40,6 +40,25 @@ except ImportError: CONSTANTS_ERR_MSG = """Could not find 'constants.json' file. See 'generate_constants_json.py -h'.""" +HEADER_TEMPLATE = """/* DO NOT EDIT GENERATED FILE */ +#ifndef MJ_SECCOMP_%(upper_name)s_H +#define MJ_SECCOMP_%(upper_name)s_H +#include <stdint.h> + +static const unsigned char %(name)s_binary_seccomp_policy[] __attribute__((__aligned__(4))) = { + %(program)s +}; + +static const struct { + uint16_t cnt; + const void *bpf; +} %(name)s_seccomp_bpf_program = { + .cnt = sizeof(%(name)s_binary_seccomp_policy) / 8, + .bpf = %(name)s_binary_seccomp_policy, +}; + +#endif +""" def parse_args(argv): """Return the parsed CLI arguments for this tool.""" @@ -51,6 +70,10 @@ def parse_args(argv): arg_parser.add_argument('--include-depth-limit', default=10) arg_parser.add_argument('--arch-json', default='constants.json') arg_parser.add_argument( + '--denylist', + action='store_true', + help='Compile as a denylist policy rather than the default allowlist.') + arg_parser.add_argument( '--default-action', type=str, help=('Use the specified default action, overriding any @default ' @@ -63,12 +86,22 @@ def parse_args(argv): action='store_true', help=('Use SECCOMP_RET_KILL_PROCESS instead of ' 'SECCOMP_RET_KILL_THREAD (requires Linux v4.14+).')) + arg_parser.add_argument( + '--use-ret-log', + action='store_true', + help=('Change all seccomp failures to return SECCOMP_RET_LOG instead ' + 'of killing (requires SECCOMP_RET_LOG kernel support).')) + arg_parser.add_argument( + '--output-header-file', + action='store_true', + help=('Output the compiled bpf to a constant variable in a C header ' + 'file instead of a binary file (output should not have a .h ' + 'extension, one will be added).')) arg_parser.add_argument('policy', help='The seccomp policy.', type=argparse.FileType('r')) arg_parser.add_argument('output', - help='The BPF program.', - type=argparse.FileType('wb')) + help='The BPF program.') return arg_parser.parse_args(argv), arg_parser @@ -84,7 +117,21 @@ def main(argv=None): parsed_arch = arch.Arch.load_from_json(opts.arch_json) policy_compiler = compiler.PolicyCompiler(parsed_arch) - if opts.use_kill_process: + # Set ret_log to true if the MINIJAIL_DEFAULT_RET_LOG environment variable + # is present. + if 'MINIJAIL_DEFAULT_RET_LOG' in os.environ: + print(""" + \n********************** +Warning: MINJAIL_DEFAULT_RET_LOG is on, policy will not have any effect +**********************\n +""") + opts.use_ret_log = True + if opts.use_ret_log: + kill_action = bpf.Log() + elif opts.denylist: + # Default action for a denylist policy is return EPERM + kill_action = bpf.ReturnErrno(parsed_arch.constants['EPERM']) + elif opts.use_kill_process: kill_action = bpf.KillProcess() else: kill_action = bpf.KillThread() @@ -94,14 +141,29 @@ def main(argv=None): override_default_action = parser.PolicyParser( parsed_arch, kill_action=bpf.KillProcess()).parse_action( next(parser_state.tokenize([opts.default_action]))) - with opts.output as outf: - outf.write( - policy_compiler.compile_file( - opts.policy.name, - optimization_strategy=opts.optimization_strategy, - kill_action=kill_action, - include_depth_limit=opts.include_depth_limit, - override_default_action=override_default_action).opcodes) + + compiled_policy = policy_compiler.compile_file( + opts.policy.name, + optimization_strategy=opts.optimization_strategy, + kill_action=kill_action, + include_depth_limit=opts.include_depth_limit, + override_default_action=override_default_action, + denylist=opts.denylist, + ret_log=opts.use_ret_log) + # Outputs the bpf binary to a c header file instead of a binary file. + if opts.output_header_file: + output_file_base = opts.output + with open(output_file_base + '.h', 'w') as output_file: + program = ', '.join('%i' % x for x in compiled_policy.opcodes) + output_file.write(HEADER_TEMPLATE % { + 'upper_name': output_file_base.upper(), + 'name': output_file_base, + 'program': program, + }) + + else: + with open(opts.output, 'wb') as outf: + outf.write(compiled_policy.opcodes) return 0 diff --git a/tools/compiler.py b/tools/compiler.py index 161eadf..f239740 100644 --- a/tools/compiler.py +++ b/tools/compiler.py @@ -270,24 +270,32 @@ class PolicyCompiler: optimization_strategy, kill_action, include_depth_limit=10, - override_default_action=None): + override_default_action=None, + denylist=False, + ret_log=False): """Return a compiled BPF program from the provided policy file.""" policy_parser = parser.PolicyParser( self._arch, kill_action=kill_action, include_depth_limit=include_depth_limit, - override_default_action=override_default_action) + override_default_action=override_default_action, + denylist=denylist, + ret_log=ret_log) parsed_policy = policy_parser.parse_file(policy_filename) entries = [ self.compile_filter_statement( - filter_statement, kill_action=kill_action) + filter_statement, kill_action=kill_action, denylist=denylist) for filter_statement in parsed_policy.filter_statements ] visitor = bpf.FlatteningVisitor( arch=self._arch, kill_action=kill_action) - accept_action = bpf.Allow() - reject_action = parsed_policy.default_action + if denylist: + accept_action = kill_action + reject_action = bpf.Allow() + else: + accept_action = bpf.Allow() + reject_action = parsed_policy.default_action if entries: if optimization_strategy == OptimizationStrategy.BST: next_action = _compile_entries_bst(entries, accept_action, @@ -304,7 +312,11 @@ class PolicyCompiler: bpf.ValidateArch(reject_action).accept(visitor) return visitor.result - def compile_filter_statement(self, filter_statement, *, kill_action): + def compile_filter_statement(self, + filter_statement, + *, + kill_action, + denylist=False): """Compile one parser.FilterStatement into BPF.""" policy_entry = SyscallPolicyEntry(filter_statement.syscall.name, filter_statement.syscall.number, @@ -314,7 +326,7 @@ class PolicyCompiler: # false action taken here is the one that applies if the whole # expression fails to match. false_action = filter_statement.filters[-1].action - if false_action == bpf.Allow(): + if not denylist and false_action == bpf.Allow(): return policy_entry # We then traverse the list of filters backwards since we want # the root of the DAG to be the very first boolean operation in diff --git a/tools/parser.py b/tools/parser.py index 0db0f62..87e1493 100644 --- a/tools/parser.py +++ b/tools/parser.py @@ -31,6 +31,12 @@ except ImportError: from minijail import bpf +# Representations of numbers with different radix (base) in C. +HEX_REGEX = r'-?0[xX][0-9a-fA-F]+' +OCTAL_REGEX = r'-?0[0-7]+' +DECIMAL_REGEX = r'-?[0-9]+' + + Token = collections.namedtuple( 'Token', ['type', 'value', 'filename', 'line', 'line_number', 'column']) @@ -42,8 +48,9 @@ _TOKEN_SPECIFICATION = ( ('DEFAULT', r'@default\b'), ('INCLUDE', r'@include\b'), ('FREQUENCY', r'@frequency\b'), + ('DENYLIST', r'@denylist$'), ('PATH', r'(?:\.)?/\S+'), - ('NUMERIC_CONSTANT', r'-?0[xX][0-9a-fA-F]+|-?0[Oo][0-7]+|-?[0-9]+'), + ('NUMERIC_CONSTANT', f'{HEX_REGEX}|{OCTAL_REGEX}|{DECIMAL_REGEX}'), ('COLON', r':'), ('SEMICOLON', r';'), ('COMMA', r','), @@ -219,14 +226,21 @@ class PolicyParser: *, kill_action, include_depth_limit=10, - override_default_action=None): + override_default_action=None, + denylist=False, + ret_log=False): self._parser_states = [ParserState("<memory>")] self._kill_action = kill_action self._include_depth_limit = include_depth_limit - self._default_action = self._kill_action + if denylist: + self._default_action = bpf.Allow() + else: + self._default_action = self._kill_action self._override_default_action = override_default_action self._frequency_mapping = collections.defaultdict(int) self._arch = arch + self._denylist = denylist + self._ret_log = ret_log @property def _parser_state(self): @@ -241,8 +255,21 @@ class PolicyParser: self._parser_state.error('invalid constant', token=token) single_constant = self._arch.constants[token.value] elif token.type == 'NUMERIC_CONSTANT': + # As `int(_, 0)` in Python != `strtol(_, _, 0)` in C, to make sure + # the number parsing behaves exactly in C, instead of using `int()` + # directly, we list out all the possible formats for octal, decimal + # and hex numbers, and determine the corresponding base by regex. try: - single_constant = int(token.value, base=0) + if re.match(HEX_REGEX, token.value): + base = 16 + elif re.match(OCTAL_REGEX, token.value): + base = 8 + elif re.match(DECIMAL_REGEX, token.value): + base = 10 + else: + # This should never happen. + raise ValueError + single_constant = int(token.value, base=base) except ValueError: self._parser_state.error('invalid constant', token=token) else: @@ -290,7 +317,7 @@ class PolicyParser: Constants can be: - - A number that can be parsed with int(..., base=0) + - A number that can be parsed with strtol() in C. - A named constant expression. - A parenthesized, valid constant expression. - A valid constant expression prefixed with the unary bitwise @@ -406,6 +433,11 @@ class PolicyParser: if not tokens: self._parser_state.error('missing action') action_token = tokens.pop(0) + # denylist policies must specify a return for every line. + if self._denylist: + if action_token.type != 'RETURN': + self._parser_state.error('invalid denylist policy') + if action_token.type == 'ACTION': if action_token.value == 'allow': return bpf.Allow() @@ -430,17 +462,22 @@ class PolicyParser: elif action_token.type == 'RETURN': if not tokens: self._parser_state.error('missing return value') - return bpf.ReturnErrno(self._parse_single_constant(tokens.pop(0))) + if self._ret_log: + tokens.pop(0) + return bpf.Log() + else: + return bpf.ReturnErrno(self._parse_single_constant(tokens.pop(0))) return self._parser_state.error('invalid action', token=action_token) # single-filter = action # | argument-expression , [ ';' , action ] + # | '!','(', argument-expression, [ ';', action ], ')' # ; def _parse_single_filter(self, tokens): if not tokens: self._parser_state.error('missing filter') if tokens[0].type == 'ARGUMENT': - # Only argument expressions can start with an ARGUMENT token. + # Only argument expressions can start with an ARGUMENT token. argument_expression = self.parse_argument_expression(tokens) if tokens and tokens[0].type == 'SEMICOLON': tokens.pop(0) @@ -697,6 +734,7 @@ class PolicyParser: self._parser_states.append(ParserState(filename)) try: statements = [] + denylist_header = False with open(filename) as policy_file: for tokens in self._parser_state.tokenize(policy_file): if tokens[0].type == 'INCLUDE': @@ -710,6 +748,14 @@ class PolicyParser: elif tokens[0].type == 'DEFAULT': self._default_action = self._parse_default_statement( tokens) + elif tokens[0].type == 'DENYLIST': + tokens.pop() + if not self._denylist: + self._parser_state.error('policy is denylist, but ' + 'flag --denylist not ' + 'passed in.') + else: + denylist_header = True else: statement = self.parse_filter_statement(tokens) if statement is None: @@ -721,6 +767,9 @@ class PolicyParser: if tokens: self._parser_state.error( 'extra tokens', token=tokens[0]) + if self._denylist and not denylist_header: + self._parser_state.error('policy must contain @denylist flag to' + ' be compiled with --denylist flag.') return statements finally: self._parser_states.pop() diff --git a/tools/parser_unittest.py b/tools/parser_unittest.py index 1570e51..9e7d6d8 100755 --- a/tools/parser_unittest.py +++ b/tools/parser_unittest.py @@ -60,7 +60,7 @@ class TokenizerTests(unittest.TestCase): ]) self.assertEqual( [(token.type, token.value) for token in TokenizerTests._tokenize( - 'read: arg0 in ~0xffff || arg0 & (1|2) && arg0 == 0o755; ' + 'read: arg0 in ~0xffff || arg0 & (1|2) && arg0 == 0755; ' 'return ENOSYS # ignored')], [ ('IDENTIFIER', 'read'), ('COLON', ':'), @@ -79,7 +79,7 @@ class TokenizerTests(unittest.TestCase): ('AND', '&&'), ('ARGUMENT', 'arg0'), ('OP', '=='), - ('NUMERIC_CONSTANT', '0o755'), + ('NUMERIC_CONSTANT', '0755'), ('SEMICOLON', ';'), ('RETURN', 'return'), ('IDENTIFIER', 'ENOSYS'), @@ -276,6 +276,27 @@ class ParseFilterExpressionTests(unittest.TestCase): parser.Atom(1, '==', 2)], ]) + def test_parse_number_argument_expression(self): + """Accept valid argument expressions with any octal/decimal/hex number.""" + # 4607 == 010777 == 0x11ff + self.assertEqual( + self.parser.parse_argument_expression( + self._tokenize('arg0 in 4607')), [ + [parser.Atom(0, 'in', 4607)], + ]) + + self.assertEqual( + self.parser.parse_argument_expression( + self._tokenize('arg0 in 010777')), [ + [parser.Atom(0, 'in', 4607)], + ]) + + self.assertEqual( + self.parser.parse_argument_expression( + self._tokenize('arg0 in 0x11ff')), [ + [parser.Atom(0, 'in', 4607)], + ]) + def test_parse_empty_argument_expression(self): """Reject empty argument expressions.""" with self.assertRaisesRegex(parser.ParseException, @@ -395,6 +416,29 @@ class ParseFilterTests(unittest.TestCase): self.parser.parse_filter(self._tokenize('{ allow')) +class ParseFilterDenylistTests(unittest.TestCase): + """Tests for PolicyParser.parse_filter with a denylist policy.""" + + def setUp(self): + self.arch = ARCH_64 + self.kill_action = bpf.KillProcess() + self.parser = parser.PolicyParser( + self.arch, kill_action=self.kill_action, denylist=True) + + def _tokenize(self, line): + # pylint: disable=protected-access + return list(self.parser._parser_state.tokenize([line]))[0] + + def test_parse_filter(self): + """Accept only filters that return an errno.""" + self.assertEqual( + self.parser.parse_filter(self._tokenize('arg0 == 0; return ENOSYS')), + [ + parser.Filter([[parser.Atom(0, '==', 0)]], + bpf.ReturnErrno(self.arch.constants['ENOSYS'])), + ]) + + class ParseFilterStatementTests(unittest.TestCase): """Tests for PolicyParser.parse_filter_statement.""" @@ -868,6 +912,112 @@ class ParseFileTests(unittest.TestCase): r'applied')): self.parser.parse_file(path) + def test_parse_allowlist_denylist_header(self): + """Reject trying to compile denylist policy file as allowlist.""" + with self.assertRaisesRegex(parser.ParseException, + r'policy is denylist, but flag --denylist ' + 'not passed in'): + path = self._write_file( + 'test.policy', """ + @denylist + """) + self.parser.parse_file(path) + + +class ParseFileDenylistTests(unittest.TestCase): + """Tests for PolicyParser.parse_file.""" + + def setUp(self): + self.arch = ARCH_64 + self.kill_action = bpf.KillProcess() + self.parser = parser.PolicyParser( + self.arch, kill_action=self.kill_action, denylist=True) + self.tempdir = tempfile.mkdtemp() + + def tearDown(self): + shutil.rmtree(self.tempdir) + + def _write_file(self, filename, contents): + """Helper to write out a file for testing.""" + path = os.path.join(self.tempdir, filename) + with open(path, 'w') as outf: + outf.write(contents) + return path + + def test_parse_simple(self): + """Allow simple denylist policy files.""" + path = self._write_file( + 'test.policy', """ + # Comment. + @denylist + read: return ENOSYS + write: return ENOSYS + """) + + self.assertEqual( + self.parser.parse_file(path), + parser.ParsedPolicy( + default_action=bpf.Allow(), + filter_statements=[ + parser.FilterStatement( + syscall=parser.Syscall('read', 0), + frequency=1, + filters=[ + parser.Filter(None, bpf.ReturnErrno( + self.arch.constants['ENOSYS'])), + ]), + parser.FilterStatement( + syscall=parser.Syscall('write', 1), + frequency=1, + filters=[ + parser.Filter(None, bpf.ReturnErrno( + self.arch.constants['ENOSYS'])), + ]), + ])) + + def test_parse_simple_with_arg(self): + """Allow simple denylist policy files.""" + path = self._write_file( + 'test.policy', """ + # Comment. + @denylist + read: return ENOSYS + write: arg0 == 0 ; return ENOSYS + """) + + self.assertEqual( + self.parser.parse_file(path), + parser.ParsedPolicy( + default_action=bpf.Allow(), + filter_statements=[ + parser.FilterStatement( + syscall=parser.Syscall('read', 0), + frequency=1, + filters=[ + parser.Filter(None, bpf.ReturnErrno( + self.arch.constants['ENOSYS'])), + ]), + parser.FilterStatement( + syscall=parser.Syscall('write', 1), + frequency=1, + filters=[ + parser.Filter([[parser.Atom(0, '==', 0)]], + bpf.ReturnErrno(self.arch.constants['ENOSYS'])), + parser.Filter(None, bpf.Allow()), + ]), + ])) + + + def test_parse_denylist_no_header(self): + """Reject trying to compile denylist policy file as allowlist.""" + with self.assertRaisesRegex(parser.ParseException, + r'policy must contain @denylist flag to be ' + 'compiled with --denylist flag'): + path = self._write_file( + 'test.policy', """ + read: return ENOSYS + """) + self.parser.parse_file(path) if __name__ == '__main__': unittest.main() diff --git a/tools/seccomp_policy_lint.py b/tools/seccomp_policy_lint.py new file mode 100755 index 0000000..f7621b0 --- /dev/null +++ b/tools/seccomp_policy_lint.py @@ -0,0 +1,151 @@ +#!/usr/bin/env python3 +# +# Copyright (C) 2021 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""A linter for the Minijail seccomp policy file.""" + +import argparse +import re +import sys + +from typing import List, NamedTuple + +# The syscalls we have determined are more dangerous and need justification +# for inclusion in a policy. +DANGEROUS_SYSCALLS = ( + 'clone', + 'mount', + 'setns', + 'kill', + 'execve', + 'execveat', + 'bpf', + 'socket', + 'ptrace', + 'swapon', + 'swapoff', + # TODO(b/193169195): Add argument granularity for the below syscalls. + 'prctl', + 'ioctl', +# 'mmap', +# 'mprotect', +# 'mmap2', +) + +class CheckPolicyReturn(NamedTuple): + """Represents a return value from check_seccomp_policy + + Contains a message to print to the user and a list of errors that were + found in the file. + """ + message: str + errors: List[str] + +def parse_args(argv): + """Return the parsed CLI arguments for this tool.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + '--denylist', + action='store_true', + help='Check as a denylist policy rather than the default allowlist.') + parser.add_argument( + '--dangerous-syscalls', + action='store', + default=','.join(DANGEROUS_SYSCALLS), + help='Comma-separated list of dangerous sycalls (overrides default).' + ) + parser.add_argument('policy', + help='The seccomp policy.', + type=argparse.FileType('r', encoding='utf-8')) + return parser.parse_args(argv), parser + +def check_seccomp_policy(check_file, dangerous_syscalls): + """Fail if the seccomp policy file has dangerous, undocumented syscalls. + + Takes in a file object and a set of dangerous syscalls as arguments. + """ + + found_syscalls = set() + errors = [] + msg = '' + contains_dangerous_syscall = False + prev_line_comment = False + + for line_num, line in enumerate(check_file): + if re.match(r'^\s*#', line): + prev_line_comment = True + elif re.match(r'^\s*$', line): + # Empty lines shouldn't reset prev_line_comment. + continue + else: + match = re.match(fr'^\s*(\w*)\s*:', line) + if match: + syscall = match.group(1) + if syscall in found_syscalls: + errors.append(f'{check_file.name}, line {line_num}: repeat ' + f'syscall: {syscall}') + else: + found_syscalls.add(syscall) + for dangerous in dangerous_syscalls: + if dangerous == syscall: + # Dangerous syscalls must be preceded with a + # comment. + contains_dangerous_syscall = True + if not prev_line_comment: + errors.append(f'{check_file.name}, line ' + f'{line_num}: {syscall} syscall ' + 'is a dangerous syscall so ' + 'requires a comment on the ' + 'preceding line') + prev_line_comment = False + else: + # This line is probably a continuation from the previous line. + # TODO(b/203216289): Support line breaks. + pass + + if contains_dangerous_syscall: + msg = (f'seccomp: {check_file.name} contains dangerous syscalls, so' + ' requires review from chromeos-security@') + else: + msg = (f'seccomp: {check_file.name} does not contain any dangerous' + ' syscalls, so does not require review from' + ' chromeos-security@') + + if errors: + return CheckPolicyReturn(msg, errors) + + return CheckPolicyReturn(msg, errors) + +def main(argv=None): + """Main entrypoint.""" + + if argv is None: + argv = sys.argv[1:] + + opts, _arg_parser = parse_args(argv) + + check = check_seccomp_policy(opts.policy, + set(opts.dangerous_syscalls.split(','))) + + formatted_items = '' + if check.errors: + item_prefix = '\n * ' + formatted_items = item_prefix + item_prefix.join(check.errors) + + print('* ' + check.message + formatted_items) + + return 1 if check.errors else 0 + +if __name__ == '__main__': + sys.exit(main(sys.argv[1:])) diff --git a/tools/seccomp_policy_lint_unittest.py b/tools/seccomp_policy_lint_unittest.py new file mode 100644 index 0000000..192739f --- /dev/null +++ b/tools/seccomp_policy_lint_unittest.py @@ -0,0 +1,116 @@ +#!/usr/bin/env python3 +# +# Copyright (C) 2021 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Unittests for the seccomp policy linter module.""" + +from pathlib import Path +import tempfile +import unittest + +import seccomp_policy_lint + +class CheckSeccompPolicyTests(unittest.TestCase): + """Tests for check_seccomp_policy.""" + + def setUp(self): + self.tempdir = Path(tempfile.mkdtemp()) + + def _write_file(self, filename, contents): + """Helper to write out a file for testing.""" + path = self.tempdir / filename + path.write_text(contents) + return path + + def test_check_simple(self): + """Allow simple policy files.""" + path = self._write_file( + 'test.policy', """ + # Comment.\n + read: 1\n + write: 1\n + """) + + exp_out = seccomp_policy_lint.CheckPolicyReturn( + f'seccomp: {path.resolve()} does not contain any dangerous' + ' syscalls, so does not require review from' + ' chromeos-security@', + []) + + with path.open('r', encoding='utf-8') as check_file: + self.assertEqual(seccomp_policy_lint.check_seccomp_policy( + check_file, seccomp_policy_lint.DANGEROUS_SYSCALLS), + exp_out) + + def test_check_dangerous_comment(self): + """Dangerous syscalls must have a comment and need to be reviewed.""" + path = self._write_file( + 'test.policy', """ + # Comment.\n\n\n + clone: 1\n + write: 1\n + """) + + exp_out = seccomp_policy_lint.CheckPolicyReturn( + f'seccomp: {path.resolve()} contains dangerous syscalls,' + ' so requires review from chromeos-security@', + []) + + with path.open('r', encoding='utf-8') as check_file: + self.assertEqual(seccomp_policy_lint.check_seccomp_policy( + check_file, seccomp_policy_lint.DANGEROUS_SYSCALLS), + exp_out) + + def test_check_dangerous_no_comment(self): + """Dangerous syscalls without a comment should cause an error.""" + path = self._write_file( + 'test.policy', """ + # Comment.\n + mount: 1\n + clone: 1\n + """) + + exp_out = seccomp_policy_lint.CheckPolicyReturn( + f'seccomp: {path.resolve()} contains dangerous syscalls,' + ' so requires review from chromeos-security@', + [(f'{path.resolve()}, line 5: clone syscall is a dangerous ' + 'syscall so requires a comment on the preceding line')]) + + with path.open('r', encoding='utf-8') as check_file: + self.assertEqual(seccomp_policy_lint.check_seccomp_policy( + check_file, seccomp_policy_lint.DANGEROUS_SYSCALLS), + exp_out) + + def test_check_duplicate_syscall(self): + """Policy files cannot have duplicate syscalls..""" + path = self._write_file( + 'test.policy', """ + # Comment.\n + clone: 1\n + clone: arg0 == 3 + """) + + exp_out = seccomp_policy_lint.CheckPolicyReturn( + f'seccomp: {path.resolve()} contains dangerous syscalls,' + ' so requires review from chromeos-security@', + [(f'{path.resolve()}, line 5: repeat syscall: clone')]) + + with path.open('r', encoding='utf-8') as check_file: + self.assertEqual(seccomp_policy_lint.check_seccomp_policy( + check_file, seccomp_policy_lint.DANGEROUS_SYSCALLS), + exp_out) + + +if __name__ == '__main__': + unittest.main() @@ -35,34 +35,37 @@ */ #if defined(__x86_64__) #if defined(__ANDROID__) -const char *log_syscalls[] = {"socket", "connect", "fcntl", "writev"}; +const char *const log_syscalls[] = {"socket", "connect", "fcntl", "writev"}; #else -const char *log_syscalls[] = {"socket", "connect", "sendto", "writev"}; +const char *const log_syscalls[] = {"socket", "connect", "sendto", "writev"}; #endif #elif defined(__i386__) #if defined(__ANDROID__) -const char *log_syscalls[] = {"socketcall", "writev", "fcntl64", - "clock_gettime"}; +const char *const log_syscalls[] = {"socketcall", "writev", "fcntl64", + "clock_gettime"}; #else -const char *log_syscalls[] = {"socketcall", "time", "writev"}; +const char *const log_syscalls[] = {"socketcall", "time", "writev"}; #endif #elif defined(__arm__) #if defined(__ANDROID__) -const char *log_syscalls[] = {"clock_gettime", "connect", "fcntl64", "socket", - "writev"}; +const char *const log_syscalls[] = {"clock_gettime", "connect", "fcntl64", + "socket", "writev"}; #else -const char *log_syscalls[] = {"socket", "connect", "gettimeofday", "send", - "writev"}; +const char *const log_syscalls[] = {"socket", "connect", "gettimeofday", "send", + "writev"}; #endif #elif defined(__aarch64__) #if defined(__ANDROID__) -const char *log_syscalls[] = {"connect", "fcntl", "sendto", "socket", "writev"}; +const char *const log_syscalls[] = {"connect", "fcntl", "sendto", "socket", + "writev"}; #else -const char *log_syscalls[] = {"socket", "connect", "send", "writev"}; +const char *const log_syscalls[] = {"socket", "connect", "send", "writev"}; #endif #elif defined(__powerpc__) || defined(__ia64__) || defined(__hppa__) || \ - defined(__sparc__) || defined(__mips__) -const char *log_syscalls[] = {"socket", "connect", "send"}; + defined(__sparc__) || defined(__mips__) +const char *const log_syscalls[] = {"socket", "connect", "send"}; +#elif defined(__riscv) +const char *const log_syscalls[] = {"socket", "connect", "sendto"}; #else #error "Unsupported platform" #endif @@ -440,16 +443,10 @@ char *tokenize(char **stringp, const char *delim) char *path_join(const char *external_path, const char *internal_path) { - char *path; - size_t pathlen; - - /* One extra char for '/' and one for '\0', hence + 2. */ - pathlen = strlen(external_path) + strlen(internal_path) + 2; - path = malloc(pathlen); - if (path) - snprintf(path, pathlen, "%s/%s", external_path, internal_path); - - return path; + char *path = NULL; + return asprintf(&path, "%s/%s", external_path, internal_path) < 0 + ? NULL + : path; } void *consumebytes(size_t length, char **buf, size_t *buflength) @@ -514,24 +511,46 @@ char **minijail_copy_env(char *const *env) return copy; } +/* + * Utility function used by minijail_setenv, minijail_unsetenv and + * minijail_getenv, returns true if |name| is found, false if not. + * If found, |*i| is |name|'s index. If not, |*i| is the length of |envp|. + */ +static bool getenv_index(char **envp, const char *name, int *i) { + if (!envp || !name || !i) + return false; + + size_t name_len = strlen(name); + for (*i = 0; envp[*i]; ++(*i)) { + /* + * If we find a match the size of |name|, we must check + * that the next character is a '=', indicating that + * the full varname of envp[i] is exactly |name| and + * not just happening to start with |name|. + */ + if (!strncmp(envp[*i], name, name_len) && + (envp[*i][name_len] == '=')) { + return true; + } + } + /* No match found, |*i| contains the number of elements in |envp|. */ + return false; +} + int minijail_setenv(char ***env, const char *name, const char *value, int overwrite) { if (!env || !*env || !name || !*name || !value) return EINVAL; - size_t name_len = strlen(name); - char **dest = NULL; - size_t env_len = 0; - for (char **entry = *env; *entry; ++entry, ++env_len) { - if (!dest && strncmp(name, *entry, name_len) == 0 && - (*entry)[name_len] == '=') { - if (!overwrite) - return 0; + int i; - dest = entry; - } + /* Look in env to check if this var name already exists. */ + if (getenv_index(*env, name, &i)) { + if (!overwrite) + return 0; + dest = &(*env)[i]; } char *new_entry = NULL; @@ -544,15 +563,87 @@ int minijail_setenv(char ***env, const char *name, const char *value, return 0; } - env_len++; - char **new_env = realloc(*env, (env_len + 1) * sizeof(char *)); + /* getenv_index has set |i| to the length of |env|. */ + ++i; + char **new_env = realloc(*env, (i + 1) * sizeof(char *)); if (!new_env) { free(new_entry); return ENOMEM; } - new_env[env_len - 1] = new_entry; - new_env[env_len] = NULL; + new_env[i - 1] = new_entry; + new_env[i] = NULL; *env = new_env; return 0; } + +/* + * This is like getline() but supports line wrapping with \. + */ +ssize_t getmultiline(char **lineptr, size_t *n, FILE *stream) +{ + ssize_t ret = getline(lineptr, n, stream); + if (ret < 0) + return ret; + + char *line = *lineptr; + /* Eat the newline to make processing below easier. */ + if (ret > 0 && line[ret - 1] == '\n') + line[--ret] = '\0'; + + /* If the line doesn't end in a backslash, we're done. */ + if (ret <= 0 || line[ret - 1] != '\\') + return ret; + + /* This line ends in a backslash. Get the nextline. */ + line[--ret] = '\0'; + size_t next_n = 0; + attribute_cleanup_str char *next_line = NULL; + ssize_t next_ret = getmultiline(&next_line, &next_n, stream); + if (next_ret == -1) { + /* We couldn't fully read the line, so return an error. */ + return -1; + } + + /* Merge the lines. */ + *n = ret + next_ret + 2; + line = realloc(line, *n); + if (!line) + return -1; + line[ret] = ' '; + memcpy(&line[ret + 1], next_line, next_ret + 1); + *lineptr = line; + return *n - 1; +} + +char *minijail_getenv(char **envp, const char *name) { + if (!envp || !name) + return NULL; + + int i; + if (!getenv_index(envp, name, &i)) + return NULL; + + /* Return a ptr to the value after the '='. */ + return envp[i] + strlen(name) + 1; +} + +bool minijail_unsetenv(char **envp, const char *name) +{ + if (!envp || !name) + return false; + + int i; + if (!getenv_index(envp, name, &i)) + return false; + + /* We found a match, replace it by the last entry of the array. */ + int last; + for (last = i; envp[last]; ++last) + continue; + --last; + envp[i] = envp[last]; + envp[last] = NULL; + + return true; +} @@ -55,6 +55,72 @@ extern "C" { #define attribute_printf(format_idx, check_idx) \ __attribute__((__format__(__printf__, format_idx, check_idx))) +#ifndef __cplusplus +/* If writing C++, use std::unique_ptr with a destructor instead. */ + +/* + * Mark a local variable for automatic cleanup when exiting its scope. + * See attribute_cleanup_fp as an example below. + * Make sure any variable using this is always initialized to something. + * @func The function to call on (a pointer to) the variable. + */ +#define attribute_cleanup(func) \ + __attribute__((__cleanup__(func))) + +/* + * Automatically close a FILE* when exiting its scope. + * Make sure the pointer is always initialized. + * Some examples: + * attribute_cleanup_fp FILE *fp = fopen(...); + * attribute_cleanup_fp FILE *fp = NULL; + * ... + * fp = fopen(...); + * + * NB: This will automatically close the underlying fd, so do not use this + * with fdopen calls if the fd should be left open. + */ +#define attribute_cleanup_fp attribute_cleanup(_cleanup_fp) +static inline void _cleanup_fp(FILE **fp) +{ + if (*fp) + fclose(*fp); +} + +/* + * Automatically close a fd when exiting its scope. + * Make sure the fd is always initialized. + * Some examples: + * attribute_cleanup_fd int fd = open(...); + * attribute_cleanup_fd int fd = -1; + * ... + * fd = open(...); + * + * NB: Be careful when using this with attribute_cleanup_fp and fdopen. + */ +#define attribute_cleanup_fd attribute_cleanup(_cleanup_fd) +static inline void _cleanup_fd(int *fd) +{ + if (*fd >= 0) + close(*fd); +} + +/* + * Automatically free a heap allocation when exiting its scope. + * Make sure the pointer is always initialized. + * Some examples: + * attribute_cleanup_str char *s = strdup(...); + * attribute_cleanup_str char *s = NULL; + * ... + * s = strdup(...); + */ +#define attribute_cleanup_str attribute_cleanup(_cleanup_str) +static inline void _cleanup_str(char **ptr) +{ + free(*ptr); +} + +#endif /* __cplusplus */ + /* clang-format off */ #define die(_msg, ...) \ do_fatal_log(LOG_ERR, "libminijail[%d]: " _msg, getpid(), ## __VA_ARGS__) @@ -74,7 +140,7 @@ extern "C" { #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) /* clang-format on */ -extern const char *log_syscalls[]; +extern const char *const log_syscalls[]; extern const size_t log_syscalls_len; enum logging_system_t { @@ -136,7 +202,8 @@ static inline bool running_with_asan(void) return compiled_with_asan() || &__asan_init != 0 || &__hwasan_init != 0; } -static inline bool debug_logging_allowed(void) { +static inline bool debug_logging_allowed(void) +{ #if defined(ALLOW_DEBUG_LOGGING) return true; #else @@ -144,6 +211,15 @@ static inline bool debug_logging_allowed(void) { #endif } +static inline bool seccomp_default_ret_log(void) +{ +#if defined(SECCOMP_DEFAULT_RET_LOG) + return true; +#else + return false; +#endif +} + static inline size_t get_num_syscalls(void) { return syscall_table_size; @@ -198,7 +274,7 @@ char *consumestr(char **buf, size_t *buflength); * @fd The file descriptor to log into. Ignored unless * @logger = LOG_TO_FD. * @min_priority The minimum priority to display. Corresponds to syslog's - priority parameter. Ignored unless @logger = LOG_TO_FD. + * priority parameter. Ignored unless @logger = LOG_TO_FD. */ void init_logging(enum logging_system_t logger, int fd, int min_priority); @@ -242,6 +318,45 @@ char **minijail_copy_env(char *const *env); int minijail_setenv(char ***env, const char *name, const char *value, int overwrite); +/* + * getmultiline: This is like getline() but supports line wrapping with \. + * + * @lineptr Address of a buffer that a mutli-line is stored. + * @n Number of bytes stored in *lineptr. + * @stream Input stream to read from. + * + * Returns number of bytes read or -1 on failure to read (including EOF). + */ +ssize_t getmultiline(char **lineptr, size_t *n, FILE *stream); + +/* + * minjail_getenv: Get an environment variable from @envp. Semantics match the + * standard getenv() function, but this operates on @envp, not the global + * environment (usually referred to as `extern char **environ`). + * + * @env Address of the environment to read from. + * @name Name of the key to get. + * + * Returns a pointer to the corresponding environment value. The caller must + * take care not to modify the pointed value, as this points directly to memory + * pointed to by @envp. + * If the environment variable name is not found, returns NULL. + */ +char *minijail_getenv(char **env, const char *name); + +/* + * minjail_unsetenv: Clear the environment variable @name from the @envp array + * of pointers to strings that have the KEY=VALUE format. If the operation is + * successful, the array will contain one item less than before the call. + * Only the first occurence is removed. + * + * @envp Address of the environment to clear the variable from. + * @name Name of the variable to clear. + * + * Returns false and modifies *@envp on success, returns true otherwise. + */ +bool minijail_unsetenv(char **envp, const char *name); + #ifdef __cplusplus }; /* extern "C" */ #endif diff --git a/util_unittest.cc b/util_unittest.cc index 35a99e5..b9e6dfc 100644 --- a/util_unittest.cc +++ b/util_unittest.cc @@ -14,6 +14,7 @@ #include <gtest/gtest.h> #include "bpf.h" +#include "test_util.h" #include "util.h" namespace { @@ -157,6 +158,42 @@ TEST(environment, copy_and_modify) { EXPECT_EQ("val1=3\nval2=4\ndup=5\ndup=2\nempty=\nnew1=7\nnew2=8\n", dump_env(env)); + EXPECT_EQ(nullptr, minijail_getenv(nullptr, "dup")); + EXPECT_EQ(nullptr, minijail_getenv(nullptr, nullptr)); + EXPECT_EQ(nullptr, minijail_getenv(env, nullptr)); + EXPECT_EQ(nullptr, minijail_getenv(env, "dup=")); + EXPECT_EQ(nullptr, minijail_getenv(env, "du")); + EXPECT_EQ(std::string("8"), minijail_getenv(env, "new2")); + EXPECT_EQ(std::string("3"), minijail_getenv(env, "val1")); + EXPECT_EQ(std::string("5"), minijail_getenv(env, "dup")); + + EXPECT_EQ(false, minijail_unsetenv(env, "nonexisting")); + EXPECT_EQ("val1=3\nval2=4\ndup=5\ndup=2\nempty=\nnew1=7\nnew2=8\n", + dump_env(env)); + EXPECT_EQ(false, minijail_unsetenv(env, "")); + EXPECT_EQ("val1=3\nval2=4\ndup=5\ndup=2\nempty=\nnew1=7\nnew2=8\n", + dump_env(env)); + EXPECT_EQ(false, minijail_unsetenv(env, nullptr)); + EXPECT_EQ("val1=3\nval2=4\ndup=5\ndup=2\nempty=\nnew1=7\nnew2=8\n", + dump_env(env)); + EXPECT_EQ(false, minijail_unsetenv(nullptr, nullptr)); + EXPECT_EQ("val1=3\nval2=4\ndup=5\ndup=2\nempty=\nnew1=7\nnew2=8\n", + dump_env(env)); + EXPECT_EQ(false, minijail_unsetenv(nullptr, "nonexisting")); + EXPECT_EQ("val1=3\nval2=4\ndup=5\ndup=2\nempty=\nnew1=7\nnew2=8\n", + dump_env(env)); + EXPECT_EQ(false, minijail_unsetenv(env, "val1=")); + EXPECT_EQ("val1=3\nval2=4\ndup=5\ndup=2\nempty=\nnew1=7\nnew2=8\n", + dump_env(env)); + EXPECT_EQ(true, minijail_unsetenv(env, "val1")); + EXPECT_EQ("new2=8\nval2=4\ndup=5\ndup=2\nempty=\nnew1=7\n", dump_env(env)); + EXPECT_EQ(true, minijail_unsetenv(env, "empty")); + EXPECT_EQ("new2=8\nval2=4\ndup=5\ndup=2\nnew1=7\n", dump_env(env)); + EXPECT_EQ(true, minijail_unsetenv(env, "new2")); + EXPECT_EQ("new1=7\nval2=4\ndup=5\ndup=2\n", dump_env(env)); + EXPECT_EQ(true, minijail_unsetenv(env, "new1")); + EXPECT_EQ("dup=2\nval2=4\ndup=5\n", dump_env(env)); + minijail_free_env(env); } @@ -358,3 +395,36 @@ TEST(parse_size, complete) { ASSERT_EQ(-EINVAL, parse_size(&size, "-1G")); ASSERT_EQ(-EINVAL, parse_size(&size, "; /bin/rm -- ")); } + +TEST(path_join, basic) { + char *path = path_join("a", "b"); + ASSERT_EQ(std::string("a/b"), path); + free(path); +} + +TEST(getmultiline, basic) { + std::string config = + "\n" + "mount = none\n" + "mount =\\\n" + "none\n" + "binding = none,/tmp\n" + "binding = none,\\\n" + "/tmp"; + FILE *config_file = write_to_pipe(config); + ASSERT_NE(config_file, nullptr); + + char *line = NULL; + size_t len = 0; + ASSERT_EQ(0, getmultiline(&line, &len, config_file)); + EXPECT_EQ(std::string(line), ""); + ASSERT_EQ(12, getmultiline(&line, &len, config_file)); + EXPECT_EQ(std::string(line), "mount = none"); + ASSERT_EQ(12, getmultiline(&line, &len, config_file)); + EXPECT_EQ(std::string(line), "mount = none"); + ASSERT_EQ(19, getmultiline(&line, &len, config_file)); + EXPECT_EQ(std::string(line), "binding = none,/tmp"); + ASSERT_EQ(20, getmultiline(&line, &len, config_file)); + EXPECT_EQ(std::string(line), "binding = none, /tmp"); + free(line); +} |