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.
This commit is contained in:
@@ -133,6 +133,7 @@ static bool store_host_list(char*** list, int* count, const char* value, const c
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
char* save = NULL;
|
char* save = NULL;
|
||||||
|
int added = 0;
|
||||||
for (char* token = strtok_r(copy, ", \t", &save); token; token = strtok_r(NULL, ", \t", &save)) {
|
for (char* token = strtok_r(copy, ", \t", &save); token; token = strtok_r(NULL, ", \t", &save)) {
|
||||||
if (!host_pattern_valid(token)) {
|
if (!host_pattern_valid(token)) {
|
||||||
if (module_name)
|
if (module_name)
|
||||||
@@ -163,8 +164,20 @@ static bool store_host_list(char*** list, int* count, const char* value, const c
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
(*list)[(*count)++] = dup;
|
(*list)[(*count)++] = dup;
|
||||||
|
added++;
|
||||||
}
|
}
|
||||||
free(copy);
|
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;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -403,6 +416,7 @@ static bool apply_module_key(DaemonModule* module, char* key, char* value, char*
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
char* save = NULL;
|
char* save = NULL;
|
||||||
|
int added = 0;
|
||||||
for (char* token = strtok_r(list, ",", &save); token; token = strtok_r(NULL, ",", &save)) {
|
for (char* token = strtok_r(list, ",", &save); token; token = strtok_r(NULL, ",", &save)) {
|
||||||
const char* user = trim_ws(token);
|
const char* user = trim_ws(token);
|
||||||
if (*user == '\0')
|
if (*user == '\0')
|
||||||
@@ -430,8 +444,16 @@ static bool apply_module_key(DaemonModule* module, char* key, char* value, char*
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
module->auth_users[module->auth_user_count++] = dup;
|
module->auth_users[module->auth_user_count++] = dup;
|
||||||
|
added++;
|
||||||
}
|
}
|
||||||
free(list);
|
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;
|
return true;
|
||||||
}
|
}
|
||||||
if (key_equals(key, "max connections"))
|
if (key_equals(key, "max connections"))
|
||||||
|
|||||||
@@ -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[0], "alice");
|
||||||
EXPECT_EQ_STR(ok_conf->modules[0].auth_users[1], "bob");
|
EXPECT_EQ_STR(ok_conf->modules[0].auth_users[1], "bob");
|
||||||
daemon_conf_free(ok_conf);
|
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,
|
/* 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);
|
EXPECT_EQ_INT(conf->modules[0].max_connections, 0);
|
||||||
daemon_conf_free(conf);
|
daemon_conf_free(conf);
|
||||||
|
|
||||||
/* An empty hosts list is not an error (no patterns are added). */
|
/* C4: a present hosts key with an empty/separator-only value must not silently
|
||||||
EXPECT_EQ_INT(write_conf("hosts allow = \n[m]\npath = /x\n", &path), 0);
|
* install a zero-length (allow-everyone) list. */
|
||||||
conf = daemon_conf_load(path, err, sizeof(err));
|
const char* empty_hosts[] = {
|
||||||
free(path);
|
"hosts allow = \n[m]\npath = /x\n",
|
||||||
EXPECT_NOT_NULL(conf);
|
"hosts deny = \n[m]\npath = /x\n",
|
||||||
EXPECT_EQ_INT(conf->global.hosts_allow_count, 0);
|
"hosts allow = , ,\n[m]\npath = /x\n",
|
||||||
daemon_conf_free(conf);
|
"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() {
|
static void test_daemon_hosts_allowed() {
|
||||||
|
|||||||
Reference in New Issue
Block a user