symlink-trust: -k/--copy-dirlinks, -K/--keep-dirlinks, --munge-links (+real -l/--links)
CI / lint (pull_request) Successful in 1m1s
CI / sanitizers (address) (pull_request) Skipped
CI / sanitizers (undefined) (pull_request) Skipped
CI / fuzz-build (pull_request) Skipped
CI / coverage (pull_request) Skipped
CI / valgrind (pull_request) Skipped
CI / build-and-test (pull_request) Successful in 1m19s
CI / lint (pull_request) Successful in 1m1s
CI / sanitizers (address) (pull_request) Skipped
CI / sanitizers (undefined) (pull_request) Skipped
CI / fuzz-build (pull_request) Skipped
CI / coverage (pull_request) Skipped
CI / valgrind (pull_request) Skipped
CI / build-and-test (pull_request) Successful in 1m19s
Adds symlink-target transmission (File is_symlink+symlink_target, STATUS_SYMLINK frame, chunk type 2), munge-links sender containment + receiver-side symmetric target containment, keep-dirlinks confined dir-symlink following (O_NOFOLLOW realpath-rechecked), and fixes -l to copy symlinks as symlinks. munge_links + keep_dirlinks cross the wire; copy_dirlinks client-only. PROTOCOL_VERSION 2.12.0->2.13.0. Review fixes: receiver rejects absolute/.. targets, gated unmunge, -K O_NOFOLLOW+re-fstat, keep_dirlinks set once at config-accept, rel_buf overflow fails the walk.
This commit is contained in:
@@ -4128,3 +4128,153 @@ class TestOmitTimes:
|
||||
received = get_dest_received_dir(dest, source)
|
||||
mismatches, missing = verify_transfer(source, received)
|
||||
assert not missing and not mismatches, f"missing={missing} mismatches={mismatches}"
|
||||
|
||||
|
||||
class TestSymlinkTrust:
|
||||
"""Phase-4 symlink trust boundaries: -k/--copy-dirlinks, -K/--keep-dirlinks
|
||||
and --munge-links. Destination paths mirror the absolute source path below
|
||||
the destination root (run_client uses absolute --source-dir/--dest-dir)."""
|
||||
|
||||
def test_copy_dirlinks_dereferences_dir_symlink(self, shared_server):
|
||||
source = os.path.join(TEST_DATA_DIR, "symlink_trust_copy_dirlinks")
|
||||
dest = os.path.join(TEST_DATA_DIR, "symlink_trust_copy_dirlinks_dst")
|
||||
clean_dir(source)
|
||||
clean_dir(dest)
|
||||
os.makedirs(os.path.join(source, "realdir"))
|
||||
with open(os.path.join(source, "realfile.txt"), "wb") as f:
|
||||
f.write(b"real file\n")
|
||||
with open(os.path.join(source, "realdir", "inside.txt"), "wb") as f:
|
||||
f.write(b"inside dir\n")
|
||||
os.symlink("realfile.txt", os.path.join(source, "link_file"))
|
||||
os.symlink("realdir", os.path.join(source, "link_dir"))
|
||||
|
||||
result, _ = run_client(source, dest, flags=["-k"], port=shared_server.port)
|
||||
assert result.returncode == 0, f"-k failed: {(result.stderr or result.stdout)[:300]}"
|
||||
|
||||
received = get_dest_received_dir(dest, source)
|
||||
# link -> realdir dereferences into a real directory tree...
|
||||
link_dir = os.path.join(received, "link_dir")
|
||||
assert os.path.isdir(link_dir)
|
||||
assert not os.path.islink(link_dir)
|
||||
assert os.path.isfile(os.path.join(link_dir, "inside.txt"))
|
||||
# ... while a symlink to a regular file stays a symlink.
|
||||
link_file = os.path.join(received, "link_file")
|
||||
assert os.path.islink(link_file)
|
||||
assert os.readlink(link_file) == "realfile.txt"
|
||||
|
||||
def test_keep_dirlinks_keeps_dest_symlink_to_dir(self, shared_server):
|
||||
source = os.path.join(TEST_DATA_DIR, "symlink_trust_keep_dirlinks")
|
||||
dest = os.path.join(TEST_DATA_DIR, "symlink_trust_keep_dirlinks_dst")
|
||||
clean_dir(source)
|
||||
clean_dir(dest)
|
||||
os.makedirs(os.path.join(source, "sub"))
|
||||
with open(os.path.join(source, "sub", "file.txt"), "wb") as f:
|
||||
f.write(b"under the kept symlinked dir\n")
|
||||
|
||||
# Plant the destination's symlink-to-directory at the exact mirror path:
|
||||
# sub -> realdir (relative, both siblings under the mirror parent).
|
||||
parent = os.path.join(dest, os.path.abspath(source).lstrip(os.sep))
|
||||
os.makedirs(parent)
|
||||
os.makedirs(os.path.join(parent, "realdir"))
|
||||
os.symlink("realdir", os.path.join(parent, "sub"))
|
||||
|
||||
result, _ = run_client(source, dest, flags=["-K"], port=shared_server.port)
|
||||
assert result.returncode == 0, f"-K failed: {(result.stderr or result.stdout)[:300]}"
|
||||
|
||||
received = get_dest_received_dir(dest, source)
|
||||
sub = os.path.join(received, "sub")
|
||||
# sub stays a symlink to the directory rather than being replaced...
|
||||
assert os.path.islink(sub)
|
||||
assert os.readlink(sub) == "realdir"
|
||||
# ... and the file is written beneath it, through to the referent dir.
|
||||
assert os.path.isfile(os.path.join(parent, "realdir", "file.txt"))
|
||||
|
||||
def test_munge_links_unmunged_target_and_containment(self, shared_server):
|
||||
source = os.path.join(TEST_DATA_DIR, "symlink_trust_munge")
|
||||
dest = os.path.join(TEST_DATA_DIR, "symlink_trust_munge_dst")
|
||||
clean_dir(source)
|
||||
clean_dir(dest)
|
||||
with open(os.path.join(source, "a.txt"), "wb") as f:
|
||||
f.write(b"a\n")
|
||||
os.symlink("a.txt", os.path.join(source, "good"))
|
||||
os.symlink("/etc/passwd", os.path.join(source, "abs_escape"))
|
||||
os.symlink("../../escape", os.path.join(source, "dotdot_escape"))
|
||||
|
||||
result, _ = run_client(source, dest, flags=["-l", "--munge-links"],
|
||||
port=shared_server.port)
|
||||
assert result.returncode == 0, f"--munge-links failed: {(result.stderr or result.stdout)[:300]}"
|
||||
|
||||
received = get_dest_received_dir(dest, source)
|
||||
# The safe symlink is created with its correct (unmunged) target.
|
||||
good = os.path.join(received, "good")
|
||||
assert os.path.islink(good)
|
||||
assert os.readlink(good) == "a.txt"
|
||||
# A target that would escape the receive root is contained (skip: never
|
||||
# transmitted, so nothing is created at the destination).
|
||||
assert not os.path.lexists(os.path.join(received, "abs_escape"))
|
||||
assert not os.path.lexists(os.path.join(received, "dotdot_escape"))
|
||||
assert os.path.isfile(os.path.join(received, "a.txt"))
|
||||
|
||||
def test_links_copies_symlinks_as_symlinks(self, shared_server):
|
||||
source = os.path.join(TEST_DATA_DIR, "symlink_trust_links")
|
||||
dest = os.path.join(TEST_DATA_DIR, "symlink_trust_links_dst")
|
||||
clean_dir(source)
|
||||
clean_dir(dest)
|
||||
os.makedirs(os.path.join(source, "realdir"))
|
||||
with open(os.path.join(source, "realfile.txt"), "wb") as f:
|
||||
f.write(b"real\n")
|
||||
with open(os.path.join(source, "realdir", "x.txt"), "wb") as f:
|
||||
f.write(b"x\n")
|
||||
os.symlink("realfile.txt", os.path.join(source, "lf"))
|
||||
os.symlink("realdir", os.path.join(source, "ld"))
|
||||
|
||||
result, _ = run_client(source, dest, flags=["-l"], port=shared_server.port)
|
||||
assert result.returncode == 0, f"-l failed: {(result.stderr or result.stdout)[:300]}"
|
||||
received = get_dest_received_dir(dest, source)
|
||||
assert os.path.islink(os.path.join(received, "lf"))
|
||||
assert os.readlink(os.path.join(received, "lf")) == "realfile.txt"
|
||||
assert os.path.islink(os.path.join(received, "ld"))
|
||||
assert os.readlink(os.path.join(received, "ld")) == "realdir"
|
||||
|
||||
def test_receiver_contains_absolute_target_even_without_munge(self, shared_server):
|
||||
# The trust boundary is symmetric and enforced receiver-side: a plain -l
|
||||
# (no --munge-links) run must refuse to materialize an out-of-root
|
||||
# absolute symlink target, while still copying a legitimate in-root one.
|
||||
source = os.path.join(TEST_DATA_DIR, "symlink_trust_abs")
|
||||
dest = os.path.join(TEST_DATA_DIR, "symlink_trust_abs_dst")
|
||||
clean_dir(source)
|
||||
clean_dir(dest)
|
||||
with open(os.path.join(source, "a.txt"), "wb") as f:
|
||||
f.write(b"a\n")
|
||||
os.symlink("a.txt", os.path.join(source, "good"))
|
||||
os.symlink("/etc/passwd", os.path.join(source, "unsafe_abs"))
|
||||
|
||||
result, _ = run_client(source, dest, flags=["-l"], port=shared_server.port)
|
||||
assert result.returncode == 0, f"-l failed: {(result.stderr or result.stdout)[:300]}"
|
||||
|
||||
received = get_dest_received_dir(dest, source)
|
||||
good = os.path.join(received, "good")
|
||||
assert os.path.islink(good)
|
||||
assert os.readlink(good) == "a.txt"
|
||||
# The absolute (non-contained) target was not materialized at the dest.
|
||||
assert not os.path.lexists(os.path.join(received, "unsafe_abs"))
|
||||
|
||||
def test_links_does_not_strip_munge_prefix_without_munge(self, shared_server):
|
||||
# A source symlink whose target genuinely begins with the #SYMLINK/ marker
|
||||
# must round-trip verbatim under plain -l: the receiver only unmunges when
|
||||
# the negotiated --munge-links policy is on, never unconditionally.
|
||||
source = os.path.join(TEST_DATA_DIR, "symlink_trust_prefix")
|
||||
dest = os.path.join(TEST_DATA_DIR, "symlink_trust_prefix_dst")
|
||||
clean_dir(source)
|
||||
clean_dir(dest)
|
||||
with open(os.path.join(source, "realfile.txt"), "wb") as f:
|
||||
f.write(b"real\n")
|
||||
os.symlink("#SYMLINK/realfile.txt", os.path.join(source, "prefixed"))
|
||||
|
||||
result, _ = run_client(source, dest, flags=["-l"], port=shared_server.port)
|
||||
assert result.returncode == 0, f"-l failed: {(result.stderr or result.stdout)[:300]}"
|
||||
|
||||
received = get_dest_received_dir(dest, source)
|
||||
prefixed = os.path.join(received, "prefixed")
|
||||
assert os.path.islink(prefixed)
|
||||
assert os.readlink(prefixed) == "#SYMLINK/realfile.txt"
|
||||
|
||||
@@ -159,8 +159,66 @@ static void test_chunk_dir_entry_roundtrip() {
|
||||
rmdir(dir_path);
|
||||
}
|
||||
|
||||
static void test_chunk_symlink_roundtrip() {
|
||||
const char* file_path = "temp_chunk_symlink_file.txt";
|
||||
const char* link_path = "temp_chunk_symlink";
|
||||
const char* content = "regular payload";
|
||||
const char* target = "temp_chunk_symlink_file.txt";
|
||||
|
||||
rmdir(link_path);
|
||||
unlink(file_path);
|
||||
|
||||
file_write_to_disk(file_path, content, strlen(content), false, false);
|
||||
|
||||
for (int use_metadata = 0; use_metadata <= 1; use_metadata++) {
|
||||
struct stat st;
|
||||
EXPECT_EQ_INT(stat(file_path, &st), 0);
|
||||
|
||||
File* reg = file_create(file_path);
|
||||
EXPECT_NOT_NULL(reg);
|
||||
reg->data->size = (unsigned long long)st.st_size;
|
||||
EXPECT_TRUE(file_load_data(reg));
|
||||
|
||||
File* link = file_create(link_path);
|
||||
EXPECT_NOT_NULL(link);
|
||||
link->is_symlink = true;
|
||||
link->symlink_target = str_dup(target);
|
||||
EXPECT_NOT_NULL(link->symlink_target);
|
||||
|
||||
if (use_metadata) {
|
||||
reg->metadata = file_metadata_create(file_path, &st, false, false);
|
||||
EXPECT_NOT_NULL(reg->metadata);
|
||||
link->metadata = file_metadata_create(file_path, &st, false, false);
|
||||
EXPECT_NOT_NULL(link->metadata);
|
||||
}
|
||||
|
||||
File* files[2] = {reg, link};
|
||||
Chunk* chunk = chunk_create(files, 2);
|
||||
EXPECT_NOT_NULL(chunk);
|
||||
|
||||
Data* serialized = chunk_serialize(chunk, use_metadata != 0);
|
||||
EXPECT_NOT_NULL(serialized);
|
||||
Chunk* deserialized = chunk_deserialize(serialized, use_metadata != 0);
|
||||
EXPECT_NOT_NULL(deserialized);
|
||||
EXPECT_EQ_INT(deserialized->element_count, 2);
|
||||
EXPECT_FALSE(deserialized->items[0]->is_symlink);
|
||||
EXPECT_TRUE(deserialized->items[1]->is_symlink);
|
||||
EXPECT_NULL(deserialized->items[0]->symlink_target);
|
||||
EXPECT_EQ_STR(deserialized->items[1]->symlink_target, target);
|
||||
EXPECT_EQ_INT((int)deserialized->items[1]->data->size, 0);
|
||||
|
||||
data_destroy(serialized);
|
||||
chunk_destroy(deserialized);
|
||||
chunk_destroy(chunk); /* frees reg and link */
|
||||
}
|
||||
|
||||
unlink(file_path);
|
||||
rmdir(link_path);
|
||||
}
|
||||
|
||||
void test_chunk() {
|
||||
test_file_operations();
|
||||
test_chunk_operations();
|
||||
test_chunk_dir_entry_roundtrip();
|
||||
test_chunk_symlink_roundtrip();
|
||||
}
|
||||
|
||||
@@ -1219,6 +1219,40 @@ static void test_parse_args_short_s_remains_chunk_serialization() {
|
||||
config_delete(cfg);
|
||||
}
|
||||
|
||||
/* Phase 4 symlink-trust flags: -k/--copy-dirlinks, -K/--keep-dirlinks and
|
||||
--munge-links must parse into their Config fields. */
|
||||
static void test_parse_args_symlink_trust() {
|
||||
Config* cfg = config_create();
|
||||
char* argv[] = {"fastsync", "-k", "-K", "--munge-links", "/src", "/dst"};
|
||||
int positional_args[2];
|
||||
int positional_count = 0;
|
||||
|
||||
EXPECT_EQ_INT(parse_args(cfg, 6, argv, positional_args, &positional_count), 0);
|
||||
EXPECT_TRUE(cfg->copy_dirlinks);
|
||||
EXPECT_TRUE(cfg->keep_dirlinks);
|
||||
EXPECT_TRUE(cfg->munge_links);
|
||||
config_delete(cfg);
|
||||
|
||||
cfg = config_create();
|
||||
char* long_argv[] = {"fastsync", "--copy-dirlinks", "--keep-dirlinks", "/src", "/dst"};
|
||||
positional_count = 0;
|
||||
EXPECT_EQ_INT(parse_args(cfg, 5, long_argv, positional_args, &positional_count), 0);
|
||||
EXPECT_TRUE(cfg->copy_dirlinks);
|
||||
EXPECT_TRUE(cfg->keep_dirlinks);
|
||||
EXPECT_FALSE(cfg->munge_links);
|
||||
config_delete(cfg);
|
||||
|
||||
/* Without any of the flags they stay off (additive, opt-in). */
|
||||
cfg = config_create();
|
||||
char* plain_argv[] = {"fastsync", "/src", "/dst"};
|
||||
positional_count = 0;
|
||||
EXPECT_EQ_INT(parse_args(cfg, 3, plain_argv, positional_args, &positional_count), 0);
|
||||
EXPECT_FALSE(cfg->copy_dirlinks);
|
||||
EXPECT_FALSE(cfg->keep_dirlinks);
|
||||
EXPECT_FALSE(cfg->munge_links);
|
||||
config_delete(cfg);
|
||||
}
|
||||
|
||||
static void test_parse_args_8_bit_output() {
|
||||
Config* cfg = config_create();
|
||||
char* long_argv[] = {"fastsync", "--8-bit-output", "/src", "/dst"};
|
||||
@@ -2429,6 +2463,7 @@ void test_client_cli() {
|
||||
test_parse_args_rejects_unsupported_stderr_modes();
|
||||
test_parse_args_secluded_args();
|
||||
test_parse_args_short_s_remains_chunk_serialization();
|
||||
test_parse_args_symlink_trust();
|
||||
test_parse_args_whole_file();
|
||||
test_parse_args_fuzzy_implies_delta();
|
||||
test_parse_args_fuzzy_negation();
|
||||
|
||||
+55
-3
@@ -622,9 +622,60 @@ static void test_config_delete_policy_wire_roundtrip() {
|
||||
}
|
||||
}
|
||||
|
||||
/* --delete-missing-args crosses the wire (the receiver executes the exact-path
|
||||
deletions) while --ignore-missing-args is client-only: the receiver must
|
||||
observe delete_missing_args unchanged and ignore_missing_args always false. */
|
||||
/* Phase 4 symlink-trust wire split: --munge-links and -K/--keep-dirlinks CROSS
|
||||
the wire (the receiver unmunges targets and follows an in-root dir-link),
|
||||
while -k/--copy-dirlinks is client/sender-only and must NOT reach the
|
||||
receiver (it would observe it false). */
|
||||
static void test_config_symlink_trust_wire_roundtrip() {
|
||||
if (is_running_under_valgrind())
|
||||
return;
|
||||
|
||||
struct {
|
||||
bool munge_links, keep_dirlinks, copy_dirlinks;
|
||||
} cases[] = {
|
||||
{false, false, false},
|
||||
{true, false, false},
|
||||
{false, true, false},
|
||||
{true, true, true},
|
||||
};
|
||||
for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) {
|
||||
int p[2];
|
||||
EXPECT_EQ_INT(socketpair(AF_UNIX, SOCK_STREAM, 0, p), 0);
|
||||
pid_t pid = fork();
|
||||
if (pid == 0) {
|
||||
close(p[1]);
|
||||
io_set_fds(p[0], p[0]);
|
||||
Config* recv = config_receive(p[0]);
|
||||
bool ok = recv != NULL;
|
||||
if (ok) {
|
||||
ok = recv->munge_links == cases[i].munge_links &&
|
||||
recv->keep_dirlinks == cases[i].keep_dirlinks &&
|
||||
/* copy_dirlinks never crosses the wire. */
|
||||
recv->copy_dirlinks == false;
|
||||
}
|
||||
config_delete(recv);
|
||||
close(p[0]);
|
||||
_exit(ok ? 0 : 1);
|
||||
} else {
|
||||
close(p[0]);
|
||||
io_set_fds(p[1], p[1]);
|
||||
Config* send_cfg = config_create();
|
||||
EXPECT_NOT_NULL(send_cfg);
|
||||
send_cfg->send_directory = str_dup("/src");
|
||||
send_cfg->receive_root_directory = str_dup("/dst");
|
||||
send_cfg->munge_links = cases[i].munge_links;
|
||||
send_cfg->keep_dirlinks = cases[i].keep_dirlinks;
|
||||
send_cfg->copy_dirlinks = cases[i].copy_dirlinks;
|
||||
bool sent = config_send(p[1], send_cfg);
|
||||
int status;
|
||||
waitpid(pid, &status, 0);
|
||||
close(p[1]);
|
||||
config_delete(send_cfg);
|
||||
EXPECT_TRUE(sent);
|
||||
EXPECT_TRUE(WIFEXITED(status) && WEXITSTATUS(status) == 0);
|
||||
}
|
||||
}
|
||||
}
|
||||
static void test_config_delete_missing_args_wire_roundtrip() {
|
||||
if (is_running_under_valgrind())
|
||||
return;
|
||||
@@ -1107,6 +1158,7 @@ void test_config() {
|
||||
test_config_delete_timing_wire_roundtrip();
|
||||
test_config_delete_timing_conflict_rejected();
|
||||
test_config_delete_policy_wire_roundtrip();
|
||||
test_config_symlink_trust_wire_roundtrip();
|
||||
test_config_delete_missing_args_wire_roundtrip();
|
||||
test_config_append_wire_roundtrip();
|
||||
test_config_basis_roundtrip();
|
||||
|
||||
@@ -468,6 +468,50 @@ static void test_file_write_to_disk_does_not_follow_symlink() {
|
||||
unlink(link);
|
||||
}
|
||||
|
||||
static void test_file_symlink_helpers() {
|
||||
/* Munge/unmunge round-trip restores the original target. */
|
||||
char* munged = file_symlink_munge("target.txt");
|
||||
EXPECT_NOT_NULL(munged);
|
||||
EXPECT_EQ_INT(memcmp(munged, SYMLINK_MUNGE_PREFIX, strlen(SYMLINK_MUNGE_PREFIX)), 0);
|
||||
EXPECT_TRUE(file_symlink_unmunge(munged));
|
||||
EXPECT_EQ_STR(munged, "target.txt");
|
||||
free(munged);
|
||||
|
||||
char noop[] = "plain-target";
|
||||
EXPECT_FALSE(file_symlink_unmunge(noop));
|
||||
EXPECT_EQ_STR(noop, "plain-target");
|
||||
|
||||
/* Containment: relative targets without ".." are safe; absolute or
|
||||
".."-escaping targets are not. */
|
||||
EXPECT_TRUE(file_symlink_target_contained("a.txt"));
|
||||
EXPECT_TRUE(file_symlink_target_contained("sub/dir/file"));
|
||||
EXPECT_FALSE(file_symlink_target_contained("/etc/passwd"));
|
||||
EXPECT_FALSE(file_symlink_target_contained("../escape"));
|
||||
EXPECT_FALSE(file_symlink_target_contained("a/../b"));
|
||||
EXPECT_FALSE(file_symlink_target_contained(""));
|
||||
}
|
||||
|
||||
static void test_file_symlink_at_secure() {
|
||||
const char* link = "test_symlink_at_secure_link";
|
||||
const char* outside = "test_symlink_at_secure_outside.txt";
|
||||
unlink(link);
|
||||
unlink(outside);
|
||||
EXPECT_TRUE(file_write_to_disk(outside, "out", 3, false, false));
|
||||
|
||||
EXPECT_TRUE(file_symlink_at_secure(link, "outside.text"));
|
||||
struct stat st;
|
||||
EXPECT_EQ_INT(lstat(link, &st), 0);
|
||||
EXPECT_TRUE(S_ISLNK(st.st_mode));
|
||||
|
||||
/* Replacing an existing non-directory entry is fine. */
|
||||
EXPECT_TRUE(file_symlink_at_secure(link, "other.txt"));
|
||||
EXPECT_EQ_INT(lstat(link, &st), 0);
|
||||
EXPECT_TRUE(S_ISLNK(st.st_mode));
|
||||
|
||||
unlink(link);
|
||||
unlink(outside);
|
||||
}
|
||||
|
||||
static void test_file_content_to_buffer() {
|
||||
const char* content = "Buffer content test";
|
||||
EXPECT_TRUE(file_write_to_disk("test_buffer_file.txt", content, strlen(content), false, false));
|
||||
@@ -978,6 +1022,8 @@ void test_file() {
|
||||
test_file_write_to_disk_creates_dirs();
|
||||
test_file_write_to_disk_does_not_follow_symlink();
|
||||
test_file_content_to_buffer();
|
||||
test_file_symlink_helpers();
|
||||
test_file_symlink_at_secure();
|
||||
test_file_save_to_disk_path_traversal();
|
||||
test_file_save_to_disk_deep_traversal();
|
||||
test_dir_entry_save_to_disk();
|
||||
|
||||
Reference in New Issue
Block a user