fix(server): reject --allow-super with --stdio, fix module host-list append
Re-review findings on the C3/C4 hardening branch: - --stdio is the SSH transport whose remote argv is composed by the client (including via --remote-option), so accepting --allow-super there let a client defeat the C3 secure default for a root receiver. Reject it at CLI parse time (standalone TCP only) and force the process-global flag off for --stdio as defense in depth. Correct the help text and README/RSYNC_COMPAT: the --stdio argv is client-composed, super stays off, and a forced command is needed if the default must hold. - daemon_conf: the per-module 'hosts allow'/'hosts deny' call sites passed module_name and replace in the wrong order, so multiple lines replaced instead of appended and the empty-value error omitted the module name. Pass (module->name, false) like the global keys; add a unit test for two per-module allow/deny lines appending. - tls: read the client CN via ASN1_STRING_to_UTF8 so an exactly-required-length name is accepted and only actual over-length CNs are rejected.
This commit is contained in:
@@ -517,7 +517,30 @@ static void test_daemon_conf_limits_and_hosts_parse() {
|
||||
free(path);
|
||||
EXPECT_NULL(rejected);
|
||||
EXPECT_TRUE(strstr(err, "must list at least one host pattern") != NULL);
|
||||
/* The diagnostic must name the offending module. */
|
||||
EXPECT_TRUE(strstr(err, "module 'm'") != NULL);
|
||||
}
|
||||
|
||||
/* Per-module host lists APPEND across lines like the global ones. (A swapped
|
||||
* store_host_list call passed the module name as `replace`, so each line
|
||||
* silently replaced the previous one and only the last survived.) */
|
||||
EXPECT_EQ_INT(write_conf("[m]\npath = /x\n"
|
||||
"hosts allow = 127.0.0.1\n"
|
||||
"hosts allow = 10.0.0.0/8\n"
|
||||
"hosts deny = 192.168.0.1\n"
|
||||
"hosts deny = 2001:db8::/32\n",
|
||||
&path),
|
||||
0);
|
||||
conf = daemon_conf_load(path, err, sizeof(err));
|
||||
free(path);
|
||||
EXPECT_NOT_NULL(conf);
|
||||
EXPECT_EQ_INT(conf->modules[0].hosts_allow_count, 2);
|
||||
EXPECT_EQ_STR(conf->modules[0].hosts_allow[0], "127.0.0.1");
|
||||
EXPECT_EQ_STR(conf->modules[0].hosts_allow[1], "10.0.0.0/8");
|
||||
EXPECT_EQ_INT(conf->modules[0].hosts_deny_count, 2);
|
||||
EXPECT_EQ_STR(conf->modules[0].hosts_deny[0], "192.168.0.1");
|
||||
EXPECT_EQ_STR(conf->modules[0].hosts_deny[1], "2001:db8::/32");
|
||||
daemon_conf_free(conf);
|
||||
}
|
||||
|
||||
static void test_daemon_hosts_allowed() {
|
||||
|
||||
+10
-3
@@ -36,9 +36,10 @@ static void test_server_cli_defaults() {
|
||||
server_cli_options_free(&opts);
|
||||
}
|
||||
|
||||
/* C3: --allow-super is the standalone/--stdio opt-in for a privileged receiver;
|
||||
* it never combines with --no-super, and daemon modules use their own per-module
|
||||
* `client owner = yes` opt-in instead. */
|
||||
/* C3: --allow-super is the locally-launched standalone TCP opt-in for a
|
||||
* privileged receiver; it never combines with --no-super, is refused with
|
||||
* --stdio (whose client-composed remote argv must not defeat the default), and
|
||||
* daemon modules use their own per-module `client owner = yes` opt-in instead. */
|
||||
static void test_server_cli_allow_super() {
|
||||
const char* args[] = {"fastsync-server", "--allow-super", "--destination-root", "/srv"};
|
||||
ServerCliOptions opts;
|
||||
@@ -54,6 +55,12 @@ static void test_server_cli_allow_super() {
|
||||
const char* a2[] = {"s", "--daemon", "--config=/tmp/x.conf", "--allow-super"};
|
||||
EXPECT_EQ_INT(server_cli_parse(4, (char**)a2, &opts, err, sizeof(err)), -1);
|
||||
EXPECT_TRUE(strstr(err, "client owner") != NULL);
|
||||
|
||||
/* The SSH/--stdio receiver argv is composed by the client, so --allow-super
|
||||
* must be rejected there and the C3 secure default stays in force. */
|
||||
const char* a3[] = {"s", "--stdio", "--allow-super", "--destination-root", "/srv"};
|
||||
EXPECT_EQ_INT(server_cli_parse(5, (char**)a3, &opts, err, sizeof(err)), -1);
|
||||
EXPECT_TRUE(strstr(err, "--stdio") != NULL);
|
||||
server_cli_options_free(&opts);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user