fix(server): gate --force by --allow-delete and secure root super default
C2: --force is deletion authority (an incoming regular file may remove a non-empty destination directory tree, and --delete-missing-args may remove a non-empty directory mirror), but it was not masked by the operator --allow-delete policy. The handler now clears config->force_delete unless --allow-delete was given, exactly like --delete and --delete-missing-args. C3: a standalone TCP / --stdio server running as root defaulted to SUPER_MODE_AUTO, so an untrusted client --devices/--write-devices/ --super could make it create device nodes, write raw devices, or apply client-chosen ownership. A privileged standalone receiver now forces SUPER_MODE_OFF unless the operator opts in with the new server-only --allow-super flag. Non-root receivers are unchanged, and the daemon path keeps its per-module `client owner = yes` gate. --allow-super is rejected with --no-super or --daemon. C6: tls_client_identity_allowed now rejects a CN whose reported length reached the buffer bound, so a truncated over-long CN cannot be matched by a required --client-cn prefix. Tests: an integration regression proving --force cannot replace a destination directory without --allow-delete; standalone-default tests for --copy-as refusal and (root-only) skipped device creation; a CLI unit test for the new flag. The integration shared_server fixture opts in with --allow-super so the existing root-only ownership/device/copy-as tests continue to exercise the opted-in configuration. README and RSYNC_COMPAT document the flag and the force/delete gating.
This commit is contained in:
+7
-1
@@ -16,7 +16,13 @@ def shared_server():
|
||||
Under pytest-xdist this session fixture is instantiated once per worker
|
||||
process, so each worker gets its own server on an ephemeral port."""
|
||||
server = ServerManager()
|
||||
server.start()
|
||||
# --allow-super keeps the historical permissive super mode for a root
|
||||
# receiver: the integration suite's root-only ownership/device/copy-as tests
|
||||
# exercise that opted-in configuration. The secure default (a root
|
||||
# standalone server without --allow-super forces SUPER_MODE_OFF) is covered
|
||||
# explicitly by TestStandaloneSuperDefault in test_features.py. Non-root
|
||||
# runs are unaffected by the flag.
|
||||
server.start(extra_args=["--allow-super"])
|
||||
yield server
|
||||
server.stop()
|
||||
|
||||
|
||||
@@ -1561,8 +1561,33 @@ class TestDelete:
|
||||
assert not missing, f"Missing: {missing}"
|
||||
assert not mismatches, f"Mismatch: {mismatches}"
|
||||
|
||||
|
||||
class TestProgress:
|
||||
@pytest.mark.ci
|
||||
def test_force_cannot_replace_directory_without_allow_delete(self):
|
||||
"""C2: --force is deletion authority (an incoming file may recursively
|
||||
remove a non-empty destination directory tree). A server started without
|
||||
--allow-delete must clear it, so the operator's delete policy cannot be
|
||||
bypassed with --force."""
|
||||
source = os.path.join(TEST_DATA_DIR, "force_src")
|
||||
dest = os.path.join(TEST_DATA_DIR, "force_dst")
|
||||
clean_dir(source)
|
||||
clean_dir(dest)
|
||||
with open(os.path.join(source, "blocker"), "wb") as f:
|
||||
f.write(b"incoming file\n")
|
||||
received = get_dest_received_dir(dest, source)
|
||||
blocker = os.path.join(received, "blocker")
|
||||
os.makedirs(blocker)
|
||||
nested = os.path.join(blocker, "nested.txt")
|
||||
with open(nested, "w") as f:
|
||||
f.write("survivor")
|
||||
# Deliberately NO --allow-delete.
|
||||
server = ServerManager()
|
||||
server.start()
|
||||
try:
|
||||
run_client(source, dest, flags=["--force"], port=server.port)
|
||||
finally:
|
||||
server.stop()
|
||||
assert os.path.isdir(blocker), "unauthorized --force removed a destination directory"
|
||||
assert os.path.exists(nested), "unauthorized --force removed a nested file"
|
||||
def test_progress_output(self, shared_server):
|
||||
clean_dir(DEST_DIR)
|
||||
result, dur = run_client(
|
||||
@@ -4578,6 +4603,61 @@ class TestSuperPrivilege:
|
||||
f"--no-super must suppress fake-super's owner replay: uid={st.st_uid} gid={st.st_gid}"
|
||||
|
||||
|
||||
class TestStandaloneSuperDefault:
|
||||
"""C3: a privileged (root) STANDALONE server without --allow-super forces
|
||||
SUPER_MODE_OFF, so a client cannot make it create device nodes, write raw
|
||||
devices, apply ownership, or use --copy-as. The shared_server fixture opts in
|
||||
with --allow-super to keep the historical behavior available to the existing
|
||||
root-only tests; these tests start their own un-opted server."""
|
||||
|
||||
@pytest.mark.ci
|
||||
def test_copy_as_refused_without_allow_super(self):
|
||||
"""--copy-as is a client-chosen-ownership request and must be refused by
|
||||
a standalone server that did not opt in with --allow-super (on a non-root
|
||||
receiver it is refused for lack of privilege either way)."""
|
||||
source = os.path.join(TEST_DATA_DIR, "super_default_src")
|
||||
dest = os.path.join(TEST_DATA_DIR, "super_default_dst")
|
||||
clean_dir(source)
|
||||
clean_dir(dest)
|
||||
with open(os.path.join(source, "f.txt"), "wb") as f:
|
||||
f.write(b"no copy-as\n")
|
||||
server = ServerManager()
|
||||
server.start() # deliberately no --allow-super
|
||||
try:
|
||||
result, _ = run_client(source, dest,
|
||||
flags=["--preserve", "--copy-as=@65534:@65534"],
|
||||
port=server.port)
|
||||
finally:
|
||||
server.stop()
|
||||
assert result.returncode != 0, (
|
||||
"standalone server accepted --copy-as without --allow-super"
|
||||
)
|
||||
|
||||
@pytest.mark.skipif(os.geteuid() != 0, reason="root can create the source device node")
|
||||
def test_devices_skipped_without_allow_super(self):
|
||||
"""Root standalone server without --allow-super must skip device-node
|
||||
creation even for a client --devices request (the run still succeeds and
|
||||
the regular file transfers)."""
|
||||
source = os.path.join(TEST_DATA_DIR, "super_default_dev_src")
|
||||
dest = os.path.join(TEST_DATA_DIR, "super_default_dev_dst")
|
||||
clean_dir(source)
|
||||
clean_dir(dest)
|
||||
with open(os.path.join(source, "plain.txt"), "wb") as f:
|
||||
f.write(b"regular\n")
|
||||
os.mknod(os.path.join(source, "null"), stat.S_IFCHR | 0o666, os.makedev(1, 3))
|
||||
server = ServerManager()
|
||||
server.start() # deliberately no --allow-super
|
||||
try:
|
||||
result, _ = run_client(source, dest, flags=["--devices"], port=server.port)
|
||||
finally:
|
||||
server.stop()
|
||||
assert result.returncode == 0, f"exit {result.returncode}: {(result.stderr or '')[:200]}"
|
||||
received = get_dest_received_dir(dest, source)
|
||||
assert not os.path.lexists(os.path.join(received, "null")), (
|
||||
"root standalone server created a device node without --allow-super"
|
||||
)
|
||||
|
||||
|
||||
class TestHardLinks:
|
||||
"""-H/--hard-links: source files sharing an inode are re-created as hard
|
||||
links to one another on the destination (dedup preserved, first copy
|
||||
|
||||
@@ -32,6 +32,28 @@ static void test_server_cli_defaults() {
|
||||
EXPECT_FALSE(opts.allow_delete);
|
||||
EXPECT_FALSE(opts.allow_unauthenticated);
|
||||
EXPECT_FALSE(opts.no_super);
|
||||
EXPECT_FALSE(opts.allow_super);
|
||||
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. */
|
||||
static void test_server_cli_allow_super() {
|
||||
const char* args[] = {"fastsync-server", "--allow-super", "--destination-root", "/srv"};
|
||||
ServerCliOptions opts;
|
||||
EXPECT_EQ_INT(parse_ok(args, 4, &opts), 0);
|
||||
EXPECT_TRUE(opts.allow_super);
|
||||
server_cli_options_free(&opts);
|
||||
|
||||
char err[256];
|
||||
const char* a1[] = {"s", "--allow-super", "--no-super"};
|
||||
EXPECT_EQ_INT(server_cli_parse(3, (char**)a1, &opts, err, sizeof(err)), -1);
|
||||
EXPECT_TRUE(strstr(err, "mutually exclusive") != NULL);
|
||||
|
||||
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);
|
||||
server_cli_options_free(&opts);
|
||||
}
|
||||
|
||||
@@ -224,5 +246,6 @@ void test_server_cli() {
|
||||
test_server_cli_password_and_early_input();
|
||||
test_server_cli_password_requires_daemon();
|
||||
test_server_cli_no_super();
|
||||
test_server_cli_allow_super();
|
||||
test_server_cli_help();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user