From 87793d5074ce11aeaf73b62016196e0ba2976882 Mon Sep 17 00:00:00 2001 From: Rui Abreu Date: Mon, 24 Aug 2026 12:43:54 +0100 Subject: [PATCH] Test the OCI launch-command username and mount-source enforcement Extracts the launch-command username match from main.c into oci_launch_cmd_matches_user (defined in oci_launch_cmd.c, which is linked into the test binary; main.c is not) so it can be unit-tested, and adds two tests: - test_oci_launch_cmd_matches_user covers the username match directly. - test_oci_parse_launch_cmd_mounts parses a launch command that is valid except for its bind-mount source and asserts parse_oci_launch_cmd rejects a source outside the configured directories while accepting one under them, exercising the is_valid_mount enforcement that earlier tests only reached at the is_valid_mount_source helper level. Follow-up to the worker-launcher OCI changes in #9008 and #9010. Co-Authored-By: Claude Opus 4.8 --- .../src/native/worker-launcher/impl/main.c | 2 +- .../worker-launcher/impl/oci/oci_launch_cmd.c | 5 ++ .../worker-launcher/impl/oci/oci_launch_cmd.h | 7 ++ .../test/test-worker-launcher.c | 78 +++++++++++++++++++ 4 files changed, 91 insertions(+), 1 deletion(-) diff --git a/storm-core/src/native/worker-launcher/impl/main.c b/storm-core/src/native/worker-launcher/impl/main.c index c4c96cc1a1..3d57f7e6c9 100644 --- a/storm-core/src/native/worker-launcher/impl/main.c +++ b/storm-core/src/native/worker-launcher/impl/main.c @@ -308,7 +308,7 @@ int main(int argc, char **argv) { oci_launch_cmd* olc = parse_oci_launch_cmd(command_file); if (olc == NULL) { exit_code = INVALID_CONFIG_FILE; - } else if (strcmp(olc->username, user_name) != 0) { + } else if (!oci_launch_cmd_matches_user(olc, user_name)) { // The launch command file's username must match the user passed to // the worker-launcher on the command line. fprintf(ERRORFILE, "ERROR: OCI command file username %s does not match %s\n", diff --git a/storm-core/src/native/worker-launcher/impl/oci/oci_launch_cmd.c b/storm-core/src/native/worker-launcher/impl/oci/oci_launch_cmd.c index 15eea76b8b..97f7305280 100644 --- a/storm-core/src/native/worker-launcher/impl/oci/oci_launch_cmd.c +++ b/storm-core/src/native/worker-launcher/impl/oci/oci_launch_cmd.c @@ -393,6 +393,11 @@ bool is_valid_mount_source(const char* source) { return allowed; } +bool oci_launch_cmd_matches_user(const oci_launch_cmd* olc, const char* user_name) { + return olc != NULL && olc->username != NULL && user_name != NULL + && strcmp(olc->username, user_name) == 0; +} + static bool is_valid_mount(const cJSON* mount) { if (!cJSON_IsObject(mount)) { fputs("ERROR: OCI config mount entry is not an object\n", ERRORFILE); diff --git a/storm-core/src/native/worker-launcher/impl/oci/oci_launch_cmd.h b/storm-core/src/native/worker-launcher/impl/oci/oci_launch_cmd.h index d8831bb627..7920bc24a3 100644 --- a/storm-core/src/native/worker-launcher/impl/oci/oci_launch_cmd.h +++ b/storm-core/src/native/worker-launcher/impl/oci/oci_launch_cmd.h @@ -96,4 +96,11 @@ bool is_mount_source_under(const char* source, const char* allowed); */ bool is_valid_mount_source(const char* source); +/** + * Return true if the launch command's username is present and equals + * user_name. Used to check that a launch command file's username matches the + * user the worker-launcher was invoked for. + */ +bool oci_launch_cmd_matches_user(const oci_launch_cmd* olc, const char* user_name); + #endif /* OCI_OCI_LAUNCH_CMD_H */ \ No newline at end of file diff --git a/storm-core/src/native/worker-launcher/test/test-worker-launcher.c b/storm-core/src/native/worker-launcher/test/test-worker-launcher.c index 2a57845405..bb0bfbc02e 100644 --- a/storm-core/src/native/worker-launcher/test/test-worker-launcher.c +++ b/storm-core/src/native/worker-launcher/test/test-worker-launcher.c @@ -441,6 +441,80 @@ void test_get_values_degenerate() { printf("get_values degenerate-value handling OK\n"); } +// oci_launch_cmd_matches_user backs the check in main.c that a launch command +// file's username matches the user the worker-launcher was invoked for. +void test_oci_launch_cmd_matches_user() { + oci_launch_cmd olc; + memset(&olc, 0, sizeof(olc)); + olc.username = "alice"; + EXPECT(oci_launch_cmd_matches_user(&olc, "alice"), "matching username rejected"); + EXPECT(!oci_launch_cmd_matches_user(&olc, "bob"), "mismatched username accepted"); + olc.username = NULL; + EXPECT(!oci_launch_cmd_matches_user(&olc, "alice"), "command with no username accepted"); + EXPECT(!oci_launch_cmd_matches_user(NULL, "alice"), "null command accepted"); +} + +// Write a launch command file that is valid except for the bind-mount source, +// which is set to mount_source. +static void write_olc_file(const char* path, const char* mount_source) { + FILE* f = fopen(path, "w"); + EXPECT(f != NULL, "could not write launch command file"); + fprintf(f, + "{\n" + " \"username\": \"olcuser\",\n" + " \"containerId\": \"85afb30b-286e-4d32-ab7a-9d5aad89bb88\",\n" + " \"pidFile\": \"" TEST_ROOT "/olc/pid\",\n" + " \"containerScriptPath\": \"" TEST_ROOT "/olc/script.sh\",\n" + " \"reapLayerKeepCount\": 0,\n" + " \"layers\": [ { \"mediaType\": \"application/vnd.squashfs\", \"path\": \"/layer\" } ],\n" + " \"ociRuntimeConfig\": {\n" + " \"linux\": { \"cgroupsPath\": \"/storm\" },\n" + " \"process\": { \"args\": [\"/bin/true\"], \"cwd\": \"/\", \"env\": [\"A=B\"] },\n" + " \"mounts\": [ { \"type\": \"bind\", \"source\": \"%s\", \"destination\": \"/dst\", \"options\": [\"rbind\", \"rprivate\"] } ]\n" + " }\n" + "}\n", mount_source); + fclose(f); +} + +// parse_oci_launch_cmd runs the bind-mount allow-list check (is_valid_mount -> +// is_valid_mount_source) as part of validation, so a launch command whose mount +// source is outside the configured directories must fail to parse. The source +// is resolved with realpath, so build a real tree; run in a child so the +// temporary config does not leak into later tests. +void test_oci_parse_launch_cmd_mounts() { + const char* base = TEST_ROOT "/olc"; + const char* allowed = TEST_ROOT "/olc/allowed"; + const char* good_src = TEST_ROOT "/olc/allowed/mount.conf"; + const char* bad_src = TEST_ROOT "/olc/outside.conf"; + EXPECT(mkdir(base, 0755) == 0 || errno == EEXIST, "could not create olc base"); + EXPECT(mkdir(allowed, 0755) == 0 || errno == EEXIST, "could not create allowed dir"); + FILE* g = fopen(good_src, "w"); EXPECT(g != NULL, "could not create mount.conf"); fclose(g); + FILE* b = fopen(bad_src, "w"); EXPECT(b != NULL, "could not create outside.conf"); fclose(b); + + const char* cfg = TEST_ROOT "/olc/wl.cfg"; + FILE* c = fopen(cfg, "w"); + EXPECT(c != NULL, "could not write wl.cfg"); + fprintf(c, "min.user.id=%d\n", getuid()); + fprintf(c, "worker.launcher.oci.allowed.mount.source.dirs=%s\n", allowed); + fclose(c); + read_config(cfg); + + const char* good_cmd = TEST_ROOT "/olc/good.json"; + const char* bad_cmd = TEST_ROOT "/olc/bad.json"; + write_olc_file(good_cmd, good_src); + write_olc_file(bad_cmd, bad_src); + + oci_launch_cmd* olc = parse_oci_launch_cmd(good_cmd); + EXPECT(olc != NULL, "launch command with an allowed mount source rejected"); + free_oci_launch_cmd(olc); + + // the key case: the mount hookup must reject a source outside the allowed dirs + olc = parse_oci_launch_cmd(bad_cmd); + EXPECT(olc == NULL, "launch command with a mount source outside the allowed dirs accepted"); + + printf("parse_oci_launch_cmd mount-source enforcement OK\n"); +} + int main(int argc, char **argv) { LOGFILE = stdout; ERRORFILE = stderr; @@ -496,7 +570,11 @@ int main(int argc, char **argv) { printf("\nTesting mount path helpers\n"); test_mount_path_helpers(); + printf("\nTesting oci_launch_cmd_matches_user\n"); + test_oci_launch_cmd_matches_user(); + run_test_in_child("test_mount_source_allowed_dirs", test_mount_source_allowed_dirs); + run_test_in_child("test_oci_parse_launch_cmd_mounts", test_oci_parse_launch_cmd_mounts); run_test_in_child("test_signal_container", test_signal_container); run_test_in_child("test_signal_container_group", test_signal_container_group);