fix(p7-privilege): close re-review gaps (implicit dir ownership, daemon --super, write-devices gate)
This commit is contained in:
+20
-1
@@ -195,6 +195,17 @@ static const char* server_module_gate(const Config* config, void* context) {
|
||||
"client-chosen ownership); refusing");
|
||||
return "--copy-as is not permitted by this daemon";
|
||||
}
|
||||
/* --super (SUPER_MODE_ON) with no explicit identity policy implies raw
|
||||
numeric-id ownership, i.e. a client-chosen owner. A daemon has no
|
||||
per-module opt-in, so refuse the explicit ON request for the same reason it
|
||||
refuses --copy-as; the pre-existing --numeric-ids/--chown/--usermap surfaces
|
||||
are unchanged (documented daemon trust model). --no-super still works. */
|
||||
if (g_daemon_conf != NULL && config->super_mode == SUPER_MODE_ON) {
|
||||
log_message(LOG_LEVEL_ERROR,
|
||||
"--super is refused by the daemon (no per-module opt-in for client-chosen "
|
||||
"ownership); refusing");
|
||||
return "--super is not permitted by this daemon";
|
||||
}
|
||||
/* Operator veto: --no-super forces SUPER_MODE_OFF for this connection before
|
||||
the copy-as gate is evaluated, and the caller clamps the accepted config
|
||||
again after this returns so the ownership/device gates see it too. */
|
||||
@@ -202,8 +213,13 @@ static const char* server_module_gate(const Config* config, void* context) {
|
||||
if (server_no_super)
|
||||
effective->super_mode = SUPER_MODE_OFF;
|
||||
if (identity_copy_as_refused(effective)) {
|
||||
if (geteuid() != 0)
|
||||
log_message(LOG_LEVEL_ERROR, "--copy-as requires a privileged receiver (root); refusing");
|
||||
return "--copy-as requires a privileged receiver (root)";
|
||||
else
|
||||
log_message(LOG_LEVEL_ERROR,
|
||||
"--copy-as refused: super-user activities are disabled by the server "
|
||||
"(--no-super); refusing");
|
||||
return "cannot perform --copy-as on this receiver";
|
||||
}
|
||||
/* --iconv (protocol 2.16.0): the receiver's exact conversion direction (the
|
||||
client spec's wire charset into this server's local charset, including a
|
||||
@@ -606,6 +622,9 @@ static void print_server_usage(void) {
|
||||
printf(" -6, --ipv6 Bind an IPv6 socket\n");
|
||||
printf(" --allow-delete Permit manifest deletion\n");
|
||||
printf(" --trust-sender Trust the remote sender's file list\n");
|
||||
printf(" --no-super Operator veto: never attempt super-user activities\n");
|
||||
printf(" (ownership, device nodes) even as root, and refuse\n");
|
||||
printf(" any client --copy-as/--super request\n");
|
||||
printf(" --iconv=LOCAL[,REMOTE] Declare this server's LOCAL charset for file-name\n");
|
||||
printf(" conversion: received names are translated to this\n");
|
||||
printf(" charset (the wire charset still comes from the\n");
|
||||
|
||||
+13
-1
@@ -16,6 +16,7 @@
|
||||
#include "data.h"
|
||||
#include "delta.h"
|
||||
#include "file.h"
|
||||
#include "identity.h"
|
||||
#include "log.h"
|
||||
#include "metadata.h"
|
||||
#include "utils.h"
|
||||
@@ -572,9 +573,20 @@ int file_open_secure_parent(const char* path, char** leaf_out, bool create_dirs)
|
||||
if (strcmp(component, ".") != 0) {
|
||||
int next = openat(fd, component, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
|
||||
if (next < 0 && create_dirs && errno == ENOENT) {
|
||||
if (mkdirat(fd, component, 0755) == 0 || errno == EEXIST)
|
||||
bool created = mkdirat(fd, component, 0755) == 0;
|
||||
if (created || errno == EEXIST) {
|
||||
/* P7 Wave E: --copy-as owns EVERY entry, including the intermediate
|
||||
directories this walk creates implicitly. Its target ids are a
|
||||
global policy, so they are available here without per-entry source
|
||||
metadata. Only a directory this walk actually created is chowned
|
||||
(a pre-existing destination directory is left alone, matching
|
||||
rsync's transferred-entry scope); the helper is a no-op unless an
|
||||
identity policy is active. */
|
||||
if (created && identity_copy_as_active())
|
||||
identity_apply_ownership_link(fd, component, 0, 0);
|
||||
next = openat(fd, component, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
|
||||
}
|
||||
}
|
||||
/* --keep-dirlinks (-K): a path component that is an existing symlink to
|
||||
an in-root directory is used as THAT directory rather than failing the
|
||||
O_NOFOLLOW walk. Only honoured when the symlink resolves to a
|
||||
|
||||
@@ -357,13 +357,15 @@ static FileSaveResult file_save_special_to_disk(const char* root_directory, cons
|
||||
if (!config || !config->preserve_devices)
|
||||
return FILE_SAVE_SKIPPED;
|
||||
/* --super / --no-super (P7 Wave E): char/block device-node creation is a
|
||||
super-user activity. --no-super forbids it even for a root receiver; the
|
||||
default AUTO only attempts it when already root. Pure FIFO creation is
|
||||
unprivileged and deliberately NOT gated here. */
|
||||
if (!privilege_super_permitted()) {
|
||||
super-user activity. --no-super forbids it even for a root receiver;
|
||||
AUTO and --super attempt it (an unprivileged attempt is refused by the
|
||||
kernel and skipped). The helper is evaluated against THIS config's mode
|
||||
so the policy does not depend on a prior identity_set_active(). Pure
|
||||
FIFO creation is unprivileged and deliberately NOT gated here. */
|
||||
if (!privilege_super_mode_permitted(config->super_mode)) {
|
||||
log_message(LOG_LEVEL_WARNING,
|
||||
"skipping %s: super-user device-node creation is not permitted "
|
||||
"(--no-super, or the receiver is not privileged)",
|
||||
"(super-user activities disabled by --no-super)",
|
||||
file->path);
|
||||
return FILE_SAVE_SKIPPED;
|
||||
}
|
||||
@@ -586,9 +588,19 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi
|
||||
writing content (privilege-gated, confined, rdev-validated). */
|
||||
if (file->is_special)
|
||||
return file_save_special_to_disk(root_directory, file, config);
|
||||
/* --write-devices: write straight into an existing device node. */
|
||||
if (config && config->write_devices)
|
||||
/* --write-devices: write straight into an existing device node. Writing
|
||||
into a device is a super-user activity, so --no-super must suppress it just
|
||||
like device-node creation; the default AUTO/--super attempt it (the wide
|
||||
open below keeps its own confinement and best-effort skip semantics). */
|
||||
if (config && config->write_devices) {
|
||||
if (!privilege_super_mode_permitted(config->super_mode)) {
|
||||
log_message(LOG_LEVEL_WARNING,
|
||||
"write-devices: %s skipped: super-user activities disabled by --no-super",
|
||||
file->path ? file->path : "(null)");
|
||||
return FILE_SAVE_SKIPPED;
|
||||
}
|
||||
return file_save_write_device(root_directory, file);
|
||||
}
|
||||
|
||||
/* Explicit directory entries (--dirs) carry an empty payload; the entry is
|
||||
created as a directory under the receive root, applying the same secure
|
||||
|
||||
@@ -93,7 +93,6 @@ void identity_set_active(const Config* config) {
|
||||
g_identity.groupmap_count = config->groupmap_count;
|
||||
}
|
||||
}
|
||||
g_identity.super_mode = config->super_mode;
|
||||
g_identity.set = true;
|
||||
/* A root receiver would honor any client-supplied ownership request (a
|
||||
--usermap/--groupmap/--chown/--copy-as, or raw ids under --numeric-ids).
|
||||
@@ -467,7 +466,7 @@ int identity_parse_copy_as(Config* config, const char* value) {
|
||||
return -1;
|
||||
}
|
||||
char* user_token = spec;
|
||||
char* group_token = NULL;
|
||||
const char* group_token = NULL;
|
||||
char* colon = strchr(spec, ':');
|
||||
if (colon) {
|
||||
*colon = '\0';
|
||||
@@ -726,5 +725,5 @@ void identity_apply_ownership_link(int parent_fd, const char* leaf, int32_t sour
|
||||
if (!identity_resolve_targets(&st, source_uid, source_gid, &uid, &gid))
|
||||
return;
|
||||
if (fchownat(parent_fd, leaf, uid, gid, AT_SYMLINK_NOFOLLOW) != 0)
|
||||
identity_log_chown_failure("symlink", uid, gid);
|
||||
identity_log_chown_failure("no-follow entry", uid, gid);
|
||||
}
|
||||
|
||||
@@ -341,6 +341,28 @@ class TestDaemonRejection:
|
||||
f"daemon did not log the copy-as refusal: {tail[-400:]!r}"
|
||||
)
|
||||
|
||||
def test_super_refused_by_daemon(self, daemon):
|
||||
"""P7 Wave E: --super (SUPER_MODE_ON) implies raw numeric-id ownership
|
||||
with no explicit identity flag, so a daemon refuses it for the same
|
||||
reason it refuses --copy-as: there is no per-module opt-in for
|
||||
client-chosen ownership. The refusal happens at the config handshake,
|
||||
before any data lands."""
|
||||
log_path = os.path.join(TEST_DATA_DIR, "fastsyncd.log")
|
||||
before = os.path.getsize(log_path) if os.path.exists(log_path) else 0
|
||||
before_files = self._tree_files()
|
||||
result, _ = run_client(SOURCE_DIR, "127.0.0.1::files", port=daemon.port,
|
||||
flags=["--super", "--preserve"])
|
||||
assert result.returncode != 0, "the daemon must refuse --super"
|
||||
assert self._tree_files() == before_files, \
|
||||
"--super refusal wrote under the module root"
|
||||
time.sleep(0.3)
|
||||
with open(log_path, "rb") as f:
|
||||
f.seek(before)
|
||||
tail = f.read().decode("utf-8", "replace")
|
||||
assert "super is refused by the daemon" in tail, (
|
||||
f"daemon did not log the --super refusal: {tail[-400:]!r}"
|
||||
)
|
||||
|
||||
@pytest.mark.daemon_detach
|
||||
def test_real_detach_path(self):
|
||||
"""--daemon WITHOUT --no-detach double-forks a real background daemon;
|
||||
|
||||
@@ -5291,6 +5291,38 @@ class TestCopyAs:
|
||||
f"--copy-as did not own the directory: uid={st.st_uid} gid={st.st_gid}"
|
||||
)
|
||||
|
||||
@pytest.mark.ci
|
||||
@pytest.mark.skipif(os.geteuid() != 0, reason="requires a root receiver to chown")
|
||||
def test_root_copy_as_owns_implicit_parent_dirs(self, shared_server):
|
||||
"""--copy-as must also own the intermediate directories that the receiver
|
||||
creates implicitly while writing a nested file (the scanner does not emit
|
||||
STATUS_MKDIR entries for ordinary traversal directories), not just the
|
||||
file itself."""
|
||||
source = os.path.join(TEST_DATA_DIR, "copyas_nested_src")
|
||||
dest = os.path.join(TEST_DATA_DIR, "copyas_nested_dst")
|
||||
clean_dir(source)
|
||||
clean_dir(dest)
|
||||
nested = os.path.join(source, "top", "mid", "leaf")
|
||||
os.makedirs(nested, exist_ok=True)
|
||||
with open(os.path.join(nested, "deep.txt"), "wb") as fh:
|
||||
fh.write(b"nested copy-as ownership\n")
|
||||
|
||||
result, _ = run_client(source, dest,
|
||||
flags=["--copy-as=@65534:@65534"],
|
||||
port=shared_server.port)
|
||||
assert result.returncode == 0, (
|
||||
f"--copy-as nested transfer failed: {(result.stderr or result.stdout)[:400]}"
|
||||
)
|
||||
received = get_dest_received_dir(dest, source)
|
||||
for rel in ("top", os.path.join("top", "mid"), os.path.join("top", "mid", "leaf")):
|
||||
target = os.path.join(received, rel)
|
||||
assert os.path.isdir(target), f"implicit directory missing at {target}"
|
||||
st = os.stat(target)
|
||||
assert (st.st_uid, st.st_gid) == (65534, 65534), (
|
||||
f"--copy-as did not own implicit directory {rel}: "
|
||||
f"uid={st.st_uid} gid={st.st_gid}"
|
||||
)
|
||||
|
||||
@pytest.mark.ci
|
||||
@pytest.mark.skipif(os.geteuid() != 0, reason="requires a root receiver to chown")
|
||||
def test_root_copy_as_owns_fifo(self, shared_server):
|
||||
|
||||
Reference in New Issue
Block a user