From 69ee132a5805149032b86b7a42fe4adf8fbc3248 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= Date: Mon, 2 Dec 2024 22:21:03 +0100 Subject: [PATCH 1/5] tests: move pylint config to pyproject.toml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It makes it easier to configure pylint Signed-off-by: Pablo Barbáchano --- tests/integration_tests/style/test_python.py | 12 +---- tests/pyproject.toml | 47 ++++++++++++++++++++ 2 files changed, 48 insertions(+), 11 deletions(-) diff --git a/tests/integration_tests/style/test_python.py b/tests/integration_tests/style/test_python.py index b04b93dace2..447a1969daa 100644 --- a/tests/integration_tests/style/test_python.py +++ b/tests/integration_tests/style/test_python.py @@ -28,17 +28,7 @@ def test_python_pylint(): Test that python code passes linter checks. """ # List of linter commands that should be executed for each file - linter_cmd = ( - # Pylint - "pylint --jobs=0 --persistent=no --score=no " - '--output-format=colorized --attr-rgx="[a-z_][a-z0-9_]{1,30}$" ' - '--argument-rgx="[a-z_][a-z0-9_]{1,35}$" ' - '--variable-rgx="[a-z_][a-z0-9_]{1,30}$" --disable=' - "fixme,too-many-instance-attributes,import-error," - "too-many-locals,too-many-arguments,consider-using-f-string," - "consider-using-with,implicit-str-concat,line-too-long,redefined-outer-name," - "broad-exception-raised,duplicate-code,too-many-positional-arguments tests tools .buildkite/*.py" - ) + linter_cmd = "pylint --rcfile tests/pyproject.toml --output-format=colorized tests/ tools/ .buildkite/*.py" run( linter_cmd, # we let pytest capture stdout/stderr for us diff --git a/tests/pyproject.toml b/tests/pyproject.toml index a81f188f996..19a77b5e190 100644 --- a/tests/pyproject.toml +++ b/tests/pyproject.toml @@ -8,3 +8,50 @@ exclude = "/(\\.direnv|\\.eggs|\\.git|\\.hg|\\.mypy_cache|\\.nox|\\.tox|\\.venv| # https://pycqa.github.io/isort/docs/configuration/multi_line_output_modes.html multi_line_output = 3 profile = "black" + +[tool.pylint.main] + +# Use multiple processes to speed up Pylint. Specifying 0 will auto-detect the +# number of processors available to use, and will cap the count on Windows to +# avoid hangs. +jobs = 0 + +score = false + +# Pickle collected data for later comparisons. +persistent = false + +# Disable the message, report, category or checker with the given id(s). You can +# either give multiple identifiers separated by comma (,) or put this option +# multiple times (only on the command line, not in the configuration file where +# it should appear only once). You can also use "--disable=all" to disable +# everything first and then re-enable specific checks. For example, if you want +# to run only the similarities checker, you can use "--disable=all +# --enable=similarities". If you want to run only the classes checker, but have +# no Warning level messages displayed, use "--disable=all --enable=classes +# --disable=W". +disable = [ + "raw-checker-failed", + "bad-inline-option", + "locally-disabled", + "file-ignored", + "suppressed-message", + "useless-suppression", + "deprecated-pragma", + "use-implicit-booleaness-not-comparison-to-string", + "use-implicit-booleaness-not-comparison-to-zero", + "use-symbolic-message-instead", + "fixme", + "too-many-instance-attributes", + "import-error", + "too-many-locals", + "too-many-arguments", + "consider-using-f-string", + "consider-using-with", + "implicit-str-concat", + "line-too-long", + "redefined-outer-name", + "broad-exception-raised", + "duplicate-code", + "too-many-positional-arguments", +] From deeb4873c7be434b714d8478b40d0b4e9db4cb4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= Date: Mon, 2 Dec 2024 22:37:15 +0100 Subject: [PATCH 2/5] style: disable pylint too-few-public-methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I think this lint is not useful, and makes writing small classes more difficult than it needs to be. Signed-off-by: Pablo Barbáchano --- tests/framework/http_api.py | 2 -- tests/framework/properties.py | 1 - tests/framework/state_machine.py | 2 -- tests/framework/utils.py | 1 - tests/integration_tests/functional/test_serial_io.py | 6 +++--- tests/pyproject.toml | 1 + 6 files changed, 4 insertions(+), 9 deletions(-) diff --git a/tests/framework/http_api.py b/tests/framework/http_api.py index a1ee37174b0..ea8efd3df4f 100644 --- a/tests/framework/http_api.py +++ b/tests/framework/http_api.py @@ -3,8 +3,6 @@ """A simple HTTP client for the Firecracker API""" -# pylint:disable=too-few-public-methods - import urllib from http import HTTPStatus diff --git a/tests/framework/properties.py b/tests/framework/properties.py index 7dd1cb46588..7072a2ff3ca 100644 --- a/tests/framework/properties.py +++ b/tests/framework/properties.py @@ -2,7 +2,6 @@ # SPDX-License-Identifier: Apache-2.0 # pylint:disable=broad-except -# pylint:disable=too-few-public-methods """ Metadata we want to attach to tests for further analysis and troubleshooting diff --git a/tests/framework/state_machine.py b/tests/framework/state_machine.py index 97975c75526..1d8dd664e6b 100644 --- a/tests/framework/state_machine.py +++ b/tests/framework/state_machine.py @@ -3,8 +3,6 @@ """Defines a stream based string matcher and a generic state object.""" -# Too few public methods (1/2) (too-few-public-methods) -# pylint: disable=R0903 class MatchStaticString: """Match a static string versus input.""" diff --git a/tests/framework/utils.py b/tests/framework/utils.py index a8715f00e94..e71302545c9 100644 --- a/tests/framework/utils.py +++ b/tests/framework/utils.py @@ -196,7 +196,6 @@ def __del__(self): self.proc.kill() -# pylint: disable=too-few-public-methods class CpuMap: """Cpu map from real cpu cores to containers visible cores. diff --git a/tests/integration_tests/functional/test_serial_io.py b/tests/integration_tests/functional/test_serial_io.py index db1521d4a44..aee9047f531 100644 --- a/tests/integration_tests/functional/test_serial_io.py +++ b/tests/integration_tests/functional/test_serial_io.py @@ -16,7 +16,7 @@ PLATFORM = platform.machine() -class WaitTerminal(TestState): # pylint: disable=too-few-public-methods +class WaitTerminal(TestState): """Initial state when we wait for the login prompt.""" def handle_input(self, serial, input_char) -> TestState: @@ -27,7 +27,7 @@ def handle_input(self, serial, input_char) -> TestState: return self -class WaitIDResult(TestState): # pylint: disable=too-few-public-methods +class WaitIDResult(TestState): """Wait for the console to show the result of the 'id' shell command.""" def handle_input(self, unused_serial, input_char) -> TestState: @@ -37,7 +37,7 @@ def handle_input(self, unused_serial, input_char) -> TestState: return self -class TestFinished(TestState): # pylint: disable=too-few-public-methods +class TestFinished(TestState): """Test complete and successful.""" def handle_input(self, unused_serial, _) -> TestState: diff --git a/tests/pyproject.toml b/tests/pyproject.toml index 19a77b5e190..70df5a75331 100644 --- a/tests/pyproject.toml +++ b/tests/pyproject.toml @@ -54,4 +54,5 @@ disable = [ "broad-exception-raised", "duplicate-code", "too-many-positional-arguments", + "too-few-public-methods", ] From 9a492fad3b8cd1a974cb523cb8fca0ee50a8c7dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= Date: Tue, 5 Nov 2024 13:25:40 +0100 Subject: [PATCH 3/5] tests: cleanup test_seccomp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - convert inline JSON to dicts - use pytest temporary files instead of tempfile - create a seccompiler fixture to make running it easy Signed-off-by: Pablo Barbáchano --- tests/integration_tests/security/conftest.py | 29 +++ .../security/test_custom_seccomp.py | 198 +++++------------- .../security/test_seccomp.py | 196 ++++++----------- 3 files changed, 136 insertions(+), 287 deletions(-) create mode 100644 tests/integration_tests/security/conftest.py diff --git a/tests/integration_tests/security/conftest.py b/tests/integration_tests/security/conftest.py new file mode 100644 index 00000000000..1cce3067b2f --- /dev/null +++ b/tests/integration_tests/security/conftest.py @@ -0,0 +1,29 @@ +# Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Fixtures for security tests""" + +import json +from pathlib import Path + +import pytest + +from host_tools.cargo_build import run_seccompiler_bin + + +@pytest.fixture() +def seccompiler(tmp_path): + "A seccompiler helper fixture" + + class Seccompiler: + "A seccompiler helper class" + + def compile(self, data: dict, basic=False) -> Path: + "Use seccompiler-bin to compile a filter from a dict" + inp = tmp_path / "input.json" + inp.write_text(json.dumps(data)) + bpf = tmp_path / "output.bpfmap" + run_seccompiler_bin(bpf_path=bpf, json_path=inp, basic=basic) + return bpf + + return Seccompiler() diff --git a/tests/integration_tests/security/test_custom_seccomp.py b/tests/integration_tests/security/test_custom_seccomp.py index 5fffb83f6cc..05f9b9aa96e 100644 --- a/tests/integration_tests/security/test_custom_seccomp.py +++ b/tests/integration_tests/security/test_custom_seccomp.py @@ -2,179 +2,76 @@ # SPDX-License-Identifier: Apache-2.0 """Tests that the --seccomp-filter parameter works as expected.""" -import os import platform -import tempfile import time +from pathlib import Path -import pytest import requests from framework import utils -from host_tools.cargo_build import run_seccompiler_bin -def _custom_filter_setup(test_microvm, json_filter): - json_temp = tempfile.NamedTemporaryFile(delete=False) - json_temp.write(json_filter) - json_temp.flush() +def install_filter(microvm, bpf_path): + """Install seccomp filter in microvm.""" + microvm.create_jailed_resource(bpf_path) + microvm.jailer.extra_args.update({"seccomp-filter": bpf_path.name}) - bpf_path = os.path.join(test_microvm.path, "bpf.out") - run_seccompiler_bin(bpf_path=bpf_path, json_path=json_temp.name) +def test_allow_all(uvm_plain, seccompiler): + """Test --seccomp-filter, allowing all syscalls.""" + seccomp_filter = { + thread: {"default_action": "allow", "filter_action": "trap", "filter": []} + for thread in ["vmm", "api", "vcpu"] + } - os.unlink(json_temp.name) - test_microvm.create_jailed_resource(bpf_path) - test_microvm.jailer.extra_args.update({"seccomp-filter": "bpf.out"}) - - -def _config_file_setup(test_microvm, vm_config_file): - test_microvm.create_jailed_resource(test_microvm.kernel_file) - test_microvm.create_jailed_resource(test_microvm.rootfs_file) - - vm_config_path = os.path.join(test_microvm.path, os.path.basename(vm_config_file)) - with open(vm_config_file, encoding="utf-8") as f1: - with open(vm_config_path, "w", encoding="utf-8") as f2: - for line in f1: - f2.write(line) - test_microvm.create_jailed_resource(vm_config_path) - test_microvm.jailer.extra_args = {"config-file": os.path.basename(vm_config_file)} - - test_microvm.jailer.extra_args.update({"no-api": None}) - - -def test_allow_all(uvm_plain): - """ - Test --seccomp-filter, allowing all syscalls. - """ + bpf_path = seccompiler.compile(seccomp_filter) test_microvm = uvm_plain - - _custom_filter_setup( - test_microvm, - """{ - "Vmm": { - "default_action": "allow", - "filter_action": "trap", - "filter": [] - }, - "Api": { - "default_action": "allow", - "filter_action": "trap", - "filter": [] - }, - "Vcpu": { - "default_action": "allow", - "filter_action": "trap", - "filter": [] - } - }""".encode( - "utf-8" - ), - ) - + install_filter(test_microvm, bpf_path) test_microvm.spawn() - test_microvm.basic_config() - test_microvm.start() - utils.assert_seccomp_level(test_microvm.firecracker_pid, "2") -def test_working_filter(uvm_plain): - """ - Test --seccomp-filter, rejecting some dangerous syscalls. - """ - test_microvm = uvm_plain +def test_working_filter(uvm_plain, seccompiler): + """Test --seccomp-filter, rejecting some dangerous syscalls.""" - _custom_filter_setup( - test_microvm, - """{ - "Vmm": { - "default_action": "allow", - "filter_action": "kill_process", - "filter": [ - { - "syscall": "clone" - }, - { - "syscall": "execve" - } - ] - }, - "Api": { - "default_action": "allow", - "filter_action": "kill_process", - "filter": [ - { - "syscall": "clone" - }, - { - "syscall": "execve" - } - ] - }, - "Vcpu": { + seccomp_filter = { + thread: { "default_action": "allow", "filter_action": "kill_process", - "filter": [ - { - "syscall": "clone" - }, - { - "syscall": "execve", - "comment": "sample comment" - } - ] + "filter": [{"syscall": "clone"}, {"syscall": "execve"}], } - }""".encode( - "utf-8" - ), - ) + for thread in ["vmm", "api", "vcpu"] + } + bpf_path = seccompiler.compile(seccomp_filter) + test_microvm = uvm_plain + install_filter(test_microvm, bpf_path) test_microvm.spawn() - test_microvm.basic_config() - test_microvm.start() # level should be 2, with no additional errors utils.assert_seccomp_level(test_microvm.firecracker_pid, "2") -def test_failing_filter(uvm_plain): - """ - Test --seccomp-filter, denying some needed syscalls. - """ - test_microvm = uvm_plain +def test_failing_filter(uvm_plain, seccompiler): + """Test --seccomp-filter, denying some needed syscalls.""" - _custom_filter_setup( - test_microvm, - """{ - "Vmm": { + seccomp_filter = { + "vmm": {"default_action": "allow", "filter_action": "trap", "filter": []}, + "api": {"default_action": "allow", "filter_action": "trap", "filter": []}, + "vcpu": { "default_action": "allow", "filter_action": "trap", - "filter": [] + "filter": [{"syscall": "ioctl"}], }, - "Api": { - "default_action": "allow", - "filter_action": "trap", - "filter": [] - }, - "Vcpu": { - "default_action": "allow", - "filter_action": "trap", - "filter": [ - { - "syscall": "ioctl" - } - ] - } - }""".encode( - "utf-8" - ), - ) + } + bpf_path = seccompiler.compile(seccomp_filter) + test_microvm = uvm_plain + install_filter(test_microvm, bpf_path) test_microvm.spawn() test_microvm.basic_config(vcpu_count=1) @@ -190,8 +87,7 @@ def test_failing_filter(uvm_plain): # Check the logger output ioctl_num = 16 if platform.machine() == "x86_64" else 29 test_microvm.check_log_message( - "Shutting down VM after intercepting a bad" - " syscall ({})".format(str(ioctl_num)) + f"Shutting down VM after intercepting a bad syscall ({ioctl_num})" ) # Check the metrics @@ -208,28 +104,28 @@ def test_failing_filter(uvm_plain): test_microvm.mark_killed() -@pytest.mark.parametrize("vm_config_file", ["framework/vm_config.json"]) -def test_invalid_bpf(uvm_plain, vm_config_file): - """ - Test that FC does not start, given an invalid binary filter. - """ +def test_invalid_bpf(uvm_plain): + """Test that FC does not start, given an invalid binary filter.""" test_microvm = uvm_plain # Configure VM from JSON. Otherwise, the test will error because # the process will be killed before configuring the API socket. - _config_file_setup(uvm_plain, vm_config_file) + test_microvm.create_jailed_resource(test_microvm.kernel_file) + test_microvm.create_jailed_resource(test_microvm.rootfs_file) - bpf_path = os.path.join(test_microvm.path, "bpf.out") - file = open(bpf_path, "w", encoding="utf-8") - file.write("Invalid BPF!") - file.close() + vm_config_file = Path("framework/vm_config.json") + test_microvm.create_jailed_resource(vm_config_file) + test_microvm.jailer.extra_args = {"config-file": vm_config_file.name} + test_microvm.jailer.extra_args.update({"no-api": None}) + bpf_path = Path(test_microvm.path) / "bpf.out" + bpf_path.write_bytes(b"Invalid BPF!") test_microvm.create_jailed_resource(bpf_path) - test_microvm.jailer.extra_args.update({"seccomp-filter": "bpf.out"}) + test_microvm.jailer.extra_args.update({"seccomp-filter": bpf_path.name}) test_microvm.spawn() - # give time for the process to get killed time.sleep(1) + assert "Seccomp error: Filter deserialization failed" in test_microvm.log_data test_microvm.mark_killed() diff --git a/tests/integration_tests/security/test_seccomp.py b/tests/integration_tests/security/test_seccomp.py index 0fa9aefafcb..8ae87271daf 100644 --- a/tests/integration_tests/security/test_seccomp.py +++ b/tests/integration_tests/security/test_seccomp.py @@ -2,91 +2,47 @@ # SPDX-License-Identifier: Apache-2.0 """Tests that the seccomp filters don't let denied syscalls through.""" -import json as json_lib +import json import os import platform -import tempfile +from pathlib import Path from framework import utils -from host_tools.cargo_build import run_seccompiler_bin + +ARCH = platform.machine() def _get_basic_syscall_list(): """Return the JSON list of syscalls that the demo jailer needs.""" - if platform.machine() == "x86_64": - sys_list = [ - "rt_sigprocmask", - "rt_sigaction", - "execve", - "mmap", - "mprotect", + sys_list = [ + "rt_sigprocmask", + "rt_sigaction", + "execve", + "mmap", + "mprotect", + "set_tid_address", + "read", + "close", + "brk", + "sched_getaffinity", + "sigaltstack", + "munmap", + "exit_group", + ] + if ARCH == "x86_64": + sys_list += [ "arch_prctl", - "set_tid_address", "readlink", "open", - "read", - "close", - "brk", - "sched_getaffinity", - "sigaltstack", - "munmap", - "exit_group", "poll", ] - else: - # platform.machine() == "aarch64" - sys_list = [ - "rt_sigprocmask", - "rt_sigaction", - "execve", - "mmap", - "mprotect", - "set_tid_address", - "read", - "close", - "brk", - "sched_getaffinity", - "sigaltstack", - "munmap", - "exit_group", - "ppoll", - ] - - json = "" - for syscall in sys_list[0:-1]: - json += """ - {{ - "syscall": \"{}\" - }}, - """.format( - syscall - ) - - json += """ - {{ - "syscall": \"{}\" - }} - """.format( - sys_list[-1] - ) - - return json - - -def _run_seccompiler_bin(json_data, basic=False): - json_temp = tempfile.NamedTemporaryFile(delete=False) - json_temp.write(json_data.encode("utf-8")) - json_temp.flush() - - bpf_temp = tempfile.NamedTemporaryFile(delete=False) + elif ARCH == "aarch64": + sys_list += ["ppoll"] - run_seccompiler_bin(bpf_path=bpf_temp.name, json_path=json_temp.name, basic=basic) + return sys_list - os.unlink(json_temp.name) - return bpf_temp.name - -def test_seccomp_ls(bin_seccomp_paths): +def test_seccomp_ls(bin_seccomp_paths, seccompiler): """ Assert that the seccomp filter denies an unallowed syscall. """ @@ -99,32 +55,26 @@ def test_seccomp_ls(bin_seccomp_paths): demo_jailer = bin_seccomp_paths["demo_jailer"] assert os.path.exists(demo_jailer) - json_filter = """{{ - "main": {{ + json_filter = { + "main": { "default_action": "trap", "filter_action": "allow", - "filter": [ - {} - ] - }} - }}""".format( - _get_basic_syscall_list() - ) + "filter": [{"syscall": x} for x in _get_basic_syscall_list()], + } + } # Run seccompiler-bin. - bpf_path = _run_seccompiler_bin(json_filter) + bpf_path = seccompiler.compile(json_filter) # Run the mini jailer. outcome = utils.run_cmd([demo_jailer, ls_command_path, bpf_path], shell=False) - os.unlink(bpf_path) - # The seccomp filters should send SIGSYS (31) to the binary. `ls` doesn't # handle it, so it will exit with error. assert outcome.returncode != 0 -def test_advanced_seccomp(bin_seccomp_paths): +def test_advanced_seccomp(bin_seccomp_paths, seccompiler): """ Test seccompiler-bin with `demo_jailer`. @@ -143,39 +93,37 @@ def test_advanced_seccomp(bin_seccomp_paths): assert os.path.exists(demo_harmless) assert os.path.exists(demo_malicious) - json_filter = """{{ - "main": {{ + json_filter = { + "main": { "default_action": "trap", "filter_action": "allow", "filter": [ - {}, - {{ + *[{"syscall": x} for x in _get_basic_syscall_list()], + { "syscall": "write", "args": [ - {{ + { "index": 0, "type": "dword", "op": "eq", "val": 1, - "comment": "stdout fd" - }}, - {{ + "comment": "stdout fd", + }, + { "index": 2, "type": "qword", "op": "eq", "val": 14, - "comment": "nr of bytes" - }} - ] - }} - ] - }} - }}""".format( - _get_basic_syscall_list() - ) + "comment": "nr of bytes", + }, + ], + }, + ], + } + } # Run seccompiler-bin. - bpf_path = _run_seccompiler_bin(json_filter) + bpf_path = seccompiler.compile(json_filter) # Run the mini jailer for harmless binary. outcome = utils.run_cmd([demo_jailer, demo_harmless, bpf_path], shell=False) @@ -189,10 +137,8 @@ def test_advanced_seccomp(bin_seccomp_paths): # The demo malicious binary should have received `SIGSYS`. assert outcome.returncode == -31 - os.unlink(bpf_path) - # Run seccompiler-bin with `--basic` flag. - bpf_path = _run_seccompiler_bin(json_filter, basic=True) + bpf_path = seccompiler.compile(json_filter, basic=True) # Run the mini jailer for malicious binary. outcome = utils.run_cmd([demo_jailer, demo_malicious, bpf_path], shell=False) @@ -201,28 +147,20 @@ def test_advanced_seccomp(bin_seccomp_paths): # disables all argument checks. assert outcome.returncode == 0 - os.unlink(bpf_path) - # Run the mini jailer with an empty allowlist. It should trap on any # syscall. - json_filter = """{ - "main": { - "default_action": "trap", - "filter_action": "allow", - "filter": [] - } - }""" + json_filter = { + "main": {"default_action": "trap", "filter_action": "allow", "filter": []} + } # Run seccompiler-bin. - bpf_path = _run_seccompiler_bin(json_filter) + bpf_path = seccompiler.compile(json_filter) outcome = utils.run_cmd([demo_jailer, demo_harmless, bpf_path], shell=False) # The demo binary should have received `SIGSYS`. assert outcome.returncode == -31 - os.unlink(bpf_path) - def test_no_seccomp(uvm_plain): """ @@ -231,11 +169,8 @@ def test_no_seccomp(uvm_plain): test_microvm = uvm_plain test_microvm.jailer.extra_args.update({"no-seccomp": None}) test_microvm.spawn() - test_microvm.basic_config() - test_microvm.start() - utils.assert_seccomp_level(test_microvm.firecracker_pid, "0") @@ -245,15 +180,12 @@ def test_default_seccomp_level(uvm_plain): """ test_microvm = uvm_plain test_microvm.spawn() - test_microvm.basic_config() - test_microvm.start() - utils.assert_seccomp_level(test_microvm.firecracker_pid, "2") -def test_seccomp_rust_panic(bin_seccomp_paths): +def test_seccomp_rust_panic(bin_seccomp_paths, seccompiler): """ Test seccompiler-bin with `demo_panic`. @@ -266,19 +198,15 @@ def test_seccomp_rust_panic(bin_seccomp_paths): demo_panic = bin_seccomp_paths["demo_panic"] assert os.path.exists(demo_panic) - fc_filters_path = "../resources/seccomp/{}-unknown-linux-musl.json".format( - platform.machine() - ) - with open(fc_filters_path, "r", encoding="utf-8") as fc_filters: - filter_threads = list(json_lib.loads(fc_filters.read())) + fc_filters = Path(f"../resources/seccomp/{ARCH}-unknown-linux-musl.json") + fc_filters_data = json.loads(fc_filters.read_text(encoding="ascii")) + filter_threads = list(fc_filters_data) - bpf_temp = tempfile.NamedTemporaryFile(delete=False) - run_seccompiler_bin(bpf_path=bpf_temp.name, json_path=fc_filters_path) - bpf_path = bpf_temp.name + bpf_path = seccompiler.compile(fc_filters_data) # Run the panic binary with all filters. for thread in filter_threads: - code, _, _ = utils.run_cmd([demo_panic, bpf_path, thread], shell=False) + code, _, _ = utils.run_cmd([demo_panic, str(bpf_path), thread], shell=False) # The demo panic binary should have terminated with SIGABRT # and not with a seccomp violation. # On a seccomp violation, the program exits with code -31 for @@ -286,8 +214,4 @@ def test_seccomp_rust_panic(bin_seccomp_paths): # is for SIGABRT. assert ( code == -6 - ), "Panic binary failed with exit code {} on {} " "filters.".format( - code, thread - ) - - os.unlink(bpf_path) + ), f"Panic binary failed with exit code {code} on {thread} filters." From 5104188e59802b085b0783bd38589bd2e61fc056 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= Date: Thu, 5 Dec 2024 16:18:39 +0100 Subject: [PATCH 4/5] docs: reflect that seccompiler --basic filters are deprecated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit seccompiler --basic filters are deprecated Signed-off-by: Pablo Barbáchano --- DEPRECATED.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/DEPRECATED.md b/DEPRECATED.md index ca9068d773d..e20e4af628a 100644 --- a/DEPRECATED.md +++ b/DEPRECATED.md @@ -19,3 +19,5 @@ a future major Firecracker release, in accordance with our - \[[#4428](https://github.com/firecracker-microvm/firecracker/pull/4428)\] Booting microVMs using MPTable and command line parameters for VirtIO devices. The functionality is substituted with ACPI. +- \[[#2628](https://github.com/firecracker-microvm/firecracker/pull/2628)\] The + `--basic` parameter of `seccompiler-bin`. From 350e04cbfce0f4ca635b18d3851732699125a20f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= Date: Mon, 9 Dec 2024 23:50:50 +0100 Subject: [PATCH 5/5] tests: add test to validate seccomp filters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a test to validate that a seccomp filter works as defined in the JSON description. To do this we use a simple C program that just loads a given seccomp filter and calls a syscall also given in the arguments. Signed-off-by: Pablo Barbáchano --- tests/host_tools/test_syscalls.c | 77 ++++++++++ .../security/test_seccomp_validate.py | 138 ++++++++++++++++++ 2 files changed, 215 insertions(+) create mode 100644 tests/host_tools/test_syscalls.c create mode 100644 tests/integration_tests/security/test_seccomp_validate.py diff --git a/tests/host_tools/test_syscalls.c b/tests/host_tools/test_syscalls.c new file mode 100644 index 00000000000..6a58edf0983 --- /dev/null +++ b/tests/host_tools/test_syscalls.c @@ -0,0 +1,77 @@ +// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +// This is used by `test_seccomp_validate.py` + +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include + + +void install_bpf_filter(char *bpf_file) { + int fd = open(bpf_file, O_RDONLY); + if (fd == -1) { + perror("open"); + exit(EXIT_FAILURE); + } + struct stat sb; + if (fstat(fd, &sb) == -1) { + perror("stat"); + exit(EXIT_FAILURE); + } + size_t size = sb.st_size; + size_t insn_len = size / sizeof(struct sock_filter); + struct sock_filter *filterbuf = (struct sock_filter*)malloc(size); + if (read(fd, filterbuf, size) == -1) { + perror("read"); + exit(EXIT_FAILURE); + } + + /* Install seccomp filter */ + struct sock_fprog prog = { + .len = (unsigned short)(insn_len), + .filter = filterbuf, + }; + if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)) { + perror("prctl(NO_NEW_PRIVS)"); + exit(EXIT_FAILURE); + } + if (prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog)) { + perror("prctl(SECCOMP)"); + exit(EXIT_FAILURE); + } +} + + +int main(int argc, char **argv) { + /* parse arguments */ + if (argc < 3) { + fprintf(stderr, "Usage: %s BPF_FILE ARG0..\n", argv[0]); + exit(EXIT_FAILURE); + } + char *bpf_file = argv[1]; + long syscall_id = atoi(argv[2]); + long arg0, arg1, arg2, arg3; + arg0 = arg1 = arg2 = arg3 = 0; + if (argc > 3) arg0 = atoi(argv[3]); + if (argc > 4) arg1 = atoi(argv[4]); + if (argc > 5) arg2 = atoi(argv[5]); + if (argc > 6) arg3 = atoi(argv[6]); + + /* read seccomp filter from file */ + if (strcmp(bpf_file, "/dev/null") != 0) { + install_bpf_filter(bpf_file); + } + + long res = syscall(syscall_id, arg0, arg1, arg2, arg3); + printf("%ld\n", res); + return EXIT_SUCCESS; +} diff --git a/tests/integration_tests/security/test_seccomp_validate.py b/tests/integration_tests/security/test_seccomp_validate.py new file mode 100644 index 00000000000..5a0be067899 --- /dev/null +++ b/tests/integration_tests/security/test_seccomp_validate.py @@ -0,0 +1,138 @@ +# Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Test that validates that seccompiler filters work as expected""" + +import json +import platform +import resource +import struct +from pathlib import Path + +import pytest +import seccomp + +from framework import utils +from host_tools import cargo_build + +ARCH = platform.machine() + + +@pytest.fixture(scope="session") +def bin_test_syscall(test_fc_session_root_path): + """Build the test_syscall binary.""" + test_syscall_bin = Path(test_fc_session_root_path) / "test_syscall" + cargo_build.gcc_compile("host_tools/test_syscalls.c", test_syscall_bin) + assert test_syscall_bin.exists() + yield test_syscall_bin + + +class BpfMapReader: + """ + Simple reader for the files that seccompiler-bin produces + + The files are serialized with bincode[1] in format that is easy to parse. + + sock_filter = + BpfProgram = Vec + BpfMap = BTreeMap(str, BpfProgram) + str = Vec + + [1] https://github.com/bincode-org/bincode/blob/trunk/docs/spec.md + """ + + INSN_FMT = "