From 0d6c1f784fc9470eb79dc8ce1abad0516e7757a8 Mon Sep 17 00:00:00 2001 From: TapTap Date: Mon, 14 Sep 2026 16:09:01 +0200 Subject: [PATCH] fix(daemon-conf): reject empty hosts/auth allow-lists (C4) A present hosts allow/hosts deny/auth users key with an empty or separator-only value produced a zero-length list, silently meaning no ACL / no auth and contradicting the strict-parse contract. store_host_list and the auth users parser now track how many entries a present key actually added and fail the load with a clear error when it is zero, so a restrictive directive can never silently become open. Unit tests cover empty, whitespace-only and comma-only values. --- src/shared/daemon_conf.c | 22 ++++++++++++++++++ tests/test_daemon_conf.c | 50 ++++++++++++++++++++++++++++++++++------ 2 files changed, 65 insertions(+), 7 deletions(-) diff --git a/src/shared/daemon_conf.c b/src/shared/daemon_conf.c index 300e196..0e30cb3 100644 --- a/src/shared/daemon_conf.c +++ b/src/shared/daemon_conf.c @@ -133,6 +133,7 @@ static bool store_host_list(char*** list, int* count, const char* value, const c return false; } char* save = NULL; + int added = 0; for (char* token = strtok_r(copy, ", \t", &save); token; token = strtok_r(NULL, ", \t", &save)) { if (!host_pattern_valid(token)) { if (module_name) @@ -163,8 +164,20 @@ static bool store_host_list(char*** list, int* count, const char* value, const c return false; } (*list)[(*count)++] = dup; + added++; } free(copy); + /* A present key with an empty (or separator-only) value would otherwise + * install a zero-length list, i.e. no ACL at all: a strict-parse config must + * never silently turn a restrictive directive into "allow everyone". */ + if (added == 0) { + if (module_name) + set_error(err, err_size, "module '%s': '%s' must list at least one host pattern", module_name, + key); + else + set_error(err, err_size, "'%s' must list at least one host pattern", key); + return false; + } return true; } @@ -403,6 +416,7 @@ static bool apply_module_key(DaemonModule* module, char* key, char* value, char* return false; } char* save = NULL; + int added = 0; for (char* token = strtok_r(list, ",", &save); token; token = strtok_r(NULL, ",", &save)) { const char* user = trim_ws(token); if (*user == '\0') @@ -430,8 +444,16 @@ static bool apply_module_key(DaemonModule* module, char* key, char* value, char* return false; } module->auth_users[module->auth_user_count++] = dup; + added++; } free(list); + /* An empty/separator-only value must not silently disable authentication: + * the key's presence is an explicit request for an allow-list. */ + if (added == 0) { + set_error(err, err_size, "module '%s': 'auth users' must list at least one user", + module->name); + return false; + } return true; } if (key_equals(key, "max connections")) diff --git a/tests/test_daemon_conf.c b/tests/test_daemon_conf.c index 3cddf0f..9b8f412 100644 --- a/tests/test_daemon_conf.c +++ b/tests/test_daemon_conf.c @@ -391,6 +391,22 @@ static void test_daemon_conf_auth_users_validated() { EXPECT_EQ_STR(ok_conf->modules[0].auth_users[0], "alice"); EXPECT_EQ_STR(ok_conf->modules[0].auth_users[1], "bob"); daemon_conf_free(ok_conf); + + /* C4: an empty or separator-only `auth users` value is a parse error. It + * would otherwise leave the module with a zero-length allow-list, silently + * disabling the authentication the operator asked for. */ + const char* empty_auth[] = { + "[m]\npath = /x\nauth users = \n", + "[m]\npath = /x\nauth users = , ,\n", + "[m]\npath = /x\nauth users = \t\n", + }; + for (size_t i = 0; i < sizeof(empty_auth) / sizeof(empty_auth[0]); i++) { + EXPECT_EQ_INT(write_conf(empty_auth[i], &path), 0); + const DaemonConf* rejected = daemon_conf_load(path, err, sizeof(err)); + free(path); + EXPECT_NULL(rejected); + EXPECT_TRUE(strstr(err, "'auth users' must list at least one user") != NULL); + } } /* Wave 3 daemon hardening: configurable global/per-module connection caps, @@ -475,13 +491,33 @@ static void test_daemon_conf_limits_and_hosts_parse() { EXPECT_EQ_INT(conf->modules[0].max_connections, 0); daemon_conf_free(conf); - /* An empty hosts list is not an error (no patterns are added). */ - EXPECT_EQ_INT(write_conf("hosts allow = \n[m]\npath = /x\n", &path), 0); - conf = daemon_conf_load(path, err, sizeof(err)); - free(path); - EXPECT_NOT_NULL(conf); - EXPECT_EQ_INT(conf->global.hosts_allow_count, 0); - daemon_conf_free(conf); + /* C4: a present hosts key with an empty/separator-only value must not silently + * install a zero-length (allow-everyone) list. */ + const char* empty_hosts[] = { + "hosts allow = \n[m]\npath = /x\n", + "hosts deny = \n[m]\npath = /x\n", + "hosts allow = , ,\n[m]\npath = /x\n", + "hosts deny = \t\n[m]\npath = /x\n", + }; + for (size_t i = 0; i < sizeof(empty_hosts) / sizeof(empty_hosts[0]); i++) { + EXPECT_EQ_INT(write_conf(empty_hosts[i], &path), 0); + const DaemonConf* rejected = daemon_conf_load(path, err, sizeof(err)); + free(path); + EXPECT_NULL(rejected); + EXPECT_TRUE(strstr(err, "must list at least one host pattern") != NULL); + } + + const char* empty_module_hosts[] = { + "[m]\npath = /x\nhosts allow = \n", + "[m]\npath = /x\nhosts deny = ,\n", + }; + for (size_t i = 0; i < sizeof(empty_module_hosts) / sizeof(empty_module_hosts[0]); i++) { + EXPECT_EQ_INT(write_conf(empty_module_hosts[i], &path), 0); + const DaemonConf* rejected = daemon_conf_load(path, err, sizeof(err)); + free(path); + EXPECT_NULL(rejected); + EXPECT_TRUE(strstr(err, "must list at least one host pattern") != NULL); + } } static void test_daemon_hosts_allowed() {