diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 534a152..cdfa0dd 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -22,7 +22,8 @@ jobs: sudo apt-get update --quiet sudo apt-get install --quiet --assume-yes --no-install-recommends gnu-efi build-essential sudo apt-get install --quiet --assume-yes --no-install-recommends \ - dosfstools mtools ovmf python3-minimal qemu-system-x86 qemu-utils shim-signed swtpm + dosfstools mtools ovmf python3-minimal python3-prettytable \ + qemu-system-x86 qemu-utils shim-signed swtpm - name: Collect Inputs run: | mkdir ./test-inputs/ @@ -51,7 +52,8 @@ jobs: KVM=false ./test/harness run "--boot-mode=$m" \ --inputs=./test-inputs "--results=./test-results/$m" \ --sbat=sbat.csv --stubby=stubby.efi \ - --num-threads=4 --timeout=120 \ + --num-threads=$(grep -c processor /proc/cpuinfo) \ + --timeout=120 \ test/tests.yaml || fails="$fails $m" done diff --git a/kcmdline.c b/kcmdline.c index 49c8ab4..591f70c 100644 --- a/kcmdline.c +++ b/kcmdline.c @@ -130,14 +130,14 @@ EFI_STATUS check_cmdline(CONST CHAR8 *cmdline, UINTN cmdline_len, CHAR16 *errmsg // (as returned by strlen). // return values: // EFI_SUCCESS: -// - builtin command line is valid AND -// ( insecureboot || (secureboot and allowed runtime) ) +// - builtin command line is valid AND +// ( insecureboot || (secureboot and allowed runtime) ) // EFI_OUT_OF_RESOURCES: AllocatePool failed. // EFI_INVALID_PARAMETER: // - builtin cmdline is invalid -// - runtime parameters given to non-empty builtin without marker. +// - runtime parameters given to non-empty builtin without marker. // EFI_SECURITY_VIOLATION: -// - secureboot and runtime is not allowed +// - secureboot and runtime is not allowed // EFI_STATUS get_cmdline( BOOLEAN secure, @@ -189,13 +189,16 @@ EFI_STATUS get_cmdline( * |------|---------|---------|--------------------|----------|--------| * | a | True | False | False | insecure | ok | * | b | True | False | True | insecure | ok | - * | c | True | True | False | insecure | ok | + * | c | True | True | False | insecure | fail | * | d | True | True | True | insecure | ok | * |--------------- ---------------------------------------------------| * | e | True | False | False | secure | ok | * | f | True | False | True | secure | ok | * | g | True | True | False | secure | fail | * | h | True | True | True | secure | ok | + * |--------------- ---------------------------------------------------| + * | j | False | True | False | insecure | fail | + * | k | False | True | False | secure | fail | * `-------------------------------------------------------------------' */ @@ -205,25 +208,15 @@ EFI_STATUS get_cmdline( if (p == NULL) { // cases: a, c, e, g if (runtime_len != 0) { - // cases: c, g - if (secure) { - // cases: g - status = EFI_INVALID_PARAMETER; - UnicodeSPrint(errbuf, errbuf_buflen, - L"runtime arguments cannot be given to non-empty builtin without marker"); - goto out; - } - // cases: c handled in common path below + // cases: c, g -- cannot use runtime without a built-in marker + status = EFI_INVALID_PARAMETER; + UnicodeSPrint(errbuf, errbuf_buflen, + L"runtime arguments cannot be given to non-empty builtin without marker"); + goto out; } - // cases: a, c, e + // cases: a, e CopyMem(part1, builtin, builtin_len); part1_len = builtin_len; - // make space for joining builtin and runtime if we have both - if (runtime_len != 0) { - part1_len = builtin_len + 1; - *(part1 + part1_len - 1) = ' '; - *(part1 + part1_len) = '\0'; - } } else { // cases: b, d, f, h // builtin has a marker, check that there is only one. @@ -248,6 +241,14 @@ EFI_STATUS get_cmdline( CopyMem(part2, p + marker_len, part2_len); *(part2 + part2_len) = '\0'; } + } else { + // cases: j, k + if (runtime_len != 0) { + status = EFI_INVALID_PARAMETER; + UnicodeSPrint(errbuf, errbuf_buflen, + L"runtime arguments cannot be given to non-empty builtin without marker"); + goto out; + } } // namespace appeared in the builtin (other than marker) @@ -384,6 +385,9 @@ EFI_STATUS get_cmdline_with_print( err = EFI_SUCCESS; } } + if (err == EFI_INVALID_PARAMETER) { + Print(L"Custom kernel command line rejected\n"); + } out: if (errmsg != NULL) { diff --git a/test-get-cmdline.c b/test-get-cmdline.c index bc5bfce..b7c0eff 100644 --- a/test-get-cmdline.c +++ b/test-get-cmdline.c @@ -28,18 +28,18 @@ TestData tests[] = { "", "root=atomix console=ttyS0 verbose"}, // no builtin, use runtime. - {EFI_SUCCESS, true, + {EFI_INVALID_PARAMETER, true, "", "root=atomix verbose", - "root=atomix verbose"}, + ResultNotChecked}, // no builtin no runtime means empty, do not boot {EFI_SECURITY_VIOLATION, true, "", "", - ""}, - // insecure, no builtin, bad runtime token allows runtime + ResultNotChecked}, + // insecure, built-in marker, bad runtime arg, inscure boots {EFI_SECURITY_VIOLATION, false, - "", + "STUBBY_RT_CLI1", "root=atomix verbose rootkit=yes", "root=atomix verbose rootkit=yes"}, // all good secure marker at beginning @@ -77,11 +77,11 @@ TestData tests[] = { "root=atomix", "console=ttyS0", ResultNotChecked}, - // no marker in insecure - just append. - {EFI_SUCCESS, false, + // no marker in insecure - runtime args requires marker even insecure + {EFI_INVALID_PARAMETER, false, "root=atomix", "console=ttyS0", - "root=atomix console=ttyS0"}, + ""}, // namespace for marker found twice in builtin secure {EFI_INVALID_PARAMETER, true, "root=atomix STUBBY_RT debug STUBBY_RT_CLI1 ", diff --git a/test/harness b/test/harness index 47d6e23..6b9ca63 100755 --- a/test/harness +++ b/test/harness @@ -5,6 +5,7 @@ import functools import logging import multiprocessing import os.path +import pdb import queue import re import shlex @@ -16,8 +17,11 @@ import threading import tempfile import textwrap import time +import traceback import yaml +import prettytable + MODE_NVRAM = 'nvram' MODE_UEFI_SHELL = 'efi-shell' @@ -915,10 +919,17 @@ class TestError(Exception): class Validator: - def __init__(self, results_d): - self.results_d = results_d - self.testinfo = load_test_data(path_join(results_d, "tests.yaml")) + def __init__(self, args): + self.args = args + self.results_d = args.results + self.testinfo = load_test_data(path_join(self.results_d, "tests.yaml")) self._tests = None + self._table = prettytable.PrettyTable() + columns = ["Name", "Passed", "Errors"] + if self.args.show_config_column: + columns.append("Config") + self._table.field_names = columns + self._table.align = "l" def tests(self): tests = {} @@ -945,12 +956,7 @@ class Validator: except TestError as e: errors[ast] = e - if len(errors) == 0: - print(f"{name}: PASS") - else: - print("%s: FAIL (%s)" % (name, ",".join(errors.keys()))) - - return errors + return len(errors) == 0, errors def validate_all(self): errors = {} @@ -959,16 +965,26 @@ class Validator: if not os.path.isdir(trdir): print(f"no results for test {name}") continue - found = self.validate_test(tdata) - if found: - errors[name] = found + result, errs = self.validate_test(tdata) + if errs: + errors[name] = errs + + row = [name, result, ",".join(errs.keys())] + if self.args.show_config_column: + row.append(tdata) + self._table.add_row(row) return errors + def print_results(self): + print("== Results ==") + print(self._table) + -def validate_results(results_d): - vdator = Validator(results_d) +def validate_results(cliargs): + vdator = Validator(cliargs) errors = vdator.validate_all() + vdator.print_results() if not errors: return 0 @@ -991,14 +1007,14 @@ def main_run(cliargs): finally: runner.cleanup() - errors = validate_results(cliargs.results) + errors = validate_results(cliargs) if errors: return 1 return 0 def main_validate(args): - validate_results(args.results) + validate_results(args) def main(): @@ -1014,6 +1030,9 @@ def main(): s.add_argument( "-r", "--results", action="store", default="./results", help="Write results output to dir") + s.add_argument( + "-C", "--show-config-column", action="store_true", default=False, + help="Display the 'Config' column in validate results") s.add_argument("testdata", help="The yaml formated test definition file") s.set_defaults(handler=main_run) @@ -1035,4 +1054,11 @@ def main(): if __name__ == "__main__": - sys.exit(main()) + ret = 0 + try: + ret = main() + except Exception: + traceback.print_exc() + pdb.post_mortem() + ret = 1 + sys.exit(ret) diff --git a/test/tests.yaml b/test/tests.yaml index 963337c..fed30ce 100644 --- a/test/tests.yaml +++ b/test/tests.yaml @@ -10,13 +10,26 @@ # - booted-expected-cli: the kernel booted and had the provided 'expected'. # - denied-boot: stubby refused to boot kernel. # - warned-cmdline: stubby warned that secureboot would not allow. +# +# notes: +# sb-shim -> secureboot=true shim=yes +# ib-shim -> secureboot=false shim=yes +# no-shim -> secureboot=false shim=no + tests: - - name: "sb-shim-allowed-rt-only" + - name: "sb-shim-denied-rt-only" builtin: "" runtime: "root=atomix console=ttyS0" - expected: "root=atomix console=ttyS0" + expected: "" + assert: + - denied-boot + + - name: "ib-shim-denied-rt-only" + builtin: "" + runtime: "root=atomix console=ttyS0" + expected: "" assert: - - booted-runtime-cli + - denied-boot - name: "sb-shim-allowed-built-in-no-rt-no-marker" builtin: "root=atomix console=ttyS0" @@ -25,28 +38,94 @@ tests: assert: - booted-expected-cli - - name: "sb-noshim-allowed" + - name: "ib-shim-allowed-built-in-no-rt-no-marker" + builtin: "root=atomix console=ttyS0" + runtime: "" + expected: "root=atomix console=ttyS0" + assert: + - booted-expected-cli + + - name: "sb-shim-denied-built-in-rt-no-marker" + builtin: "root=atomix console=ttyS0" + runtime: "crashkernel=256M" + expected: "" + assert: + - denied-boot + + - name: "ib-shim-denied-built-in-rt-no-marker" + builtin: "root=atomix console=ttyS0" + runtime: "crashkernel=256M" + expected: "" + assert: + - denied-boot + + - name: "sb-shim-denied-empty-built-in-rt" + builtin: "" runtime: "root=atomix console=ttyS0" + expected: "" assert: - - booted-runtime-cli + - denied-boot - - name: "ib-shim-allowed" + - name: "ib-shim-denied-empty-built-in-rt" + builtin: "" runtime: "root=atomix console=ttyS0" + expected: "" + assert: + - denied-boot + + - name: "sb-shim-allowed-built-in-rt-has-marker" + builtin: "STUBBY_RT_CLI1" + runtime: "root=atomix verbose console=ttyS0" + expected: "" assert: - booted-runtime-cli - - name: "ib-noshim-allowed" - runtime: "root=atomix console=ttyS0" + - name: "ib-shim-allowed-built-in-rt-has-marker" + builtin: "STUBBY_RT_CLI1" + runtime: "root=atomix verbose console=ttyS0" + expected: "" assert: - booted-runtime-cli - - name: "sb-shim-denied" + - name: "sb-noshim-denied-no-marker" + builtin: "" + runtime: "root=atomix console=ttyS0" + expected: "" + assert: + - denied-boot + + - name: "ib-noshim-denied-no-marker" + builtin: "" + runtime: "root=atomix console=ttyS0" + expected: "" + assert: + - denied-boot + + - name: "sb-noshim-allowed-with-marker" + builtin: "STUBBY_RT_CLI1" + runtime: "root=atomix console=ttyS0" + expected: "root=atomix console=ttyS0" + assert: + - booted-expected-cli + + - name: "ib-noshim-denied-with-marker" + builtin: "" + runtime: "root=atomix console=ttyS0" + expected: "" + assert: + - denied-boot + + - name: "sb-shim-denied-builtin-marker-bad-args" + builtin: "STUBBY_RT_CLI1" runtime: "root=atomix console=ttyS0 rootkit=yes" + expected: "" assert: - denied-boot - - name: "ib-shim-denied" + - name: "ib-shim-warned-builtin-marker-bad-args" + builtin: "STUBBY_RT_CLI1" runtime: "root=atomix console=ttyS0 rootkit=yes" + expected: "" assert: - warned-cmdline - booted-runtime-cli