From e3840c832691f19ccf7f5aacca87b67f6ec48014 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sat, 12 Sep 2026 14:00:55 +0200 Subject: [PATCH 1/3] fix(p8-security): enforce daemon ownership policy, gate fake-super replay, drop implicit numeric-ids, make copy-as failures per-entry A1: daemon refuses every client-chosen ownership/super-user request (--numeric-ids/--chown/--usermap/--groupmap/--fake-super/--copy-as/--super) unless the selected module opts in with 'client owner = yes'. A2: fake-super owner replay requires an explicit ownership identity policy. A3: --super no longer implies --numeric-ids (ownership stays opt-in). A5: a failed --copy-as chown marks the entry failed instead of reporting success with the wrong owner. --- RSYNC_COMPAT.md | 15 +++--- src/server/server.c | 68 ++++++++++++----------- src/shared/config.h | 15 +++--- src/shared/daemon_conf.c | 11 ++++ src/shared/daemon_conf.h | 21 +++++--- src/shared/file_receive.c | 14 +++-- src/shared/identity.c | 72 ++++++++++++++----------- src/shared/identity.h | 31 ++++++++--- src/shared/metadata.c | 12 +++-- src/shared/xattr.c | 9 +++- tests/integration/test_daemon.py | 87 ++++++++++++++++++------------ tests/integration/test_features.py | 29 +++++++--- tests/test_config.c | 64 ++++++++++++++++++++++ tests/test_daemon_conf.c | 12 +++++ tests/test_xattr.c | 19 +++++-- 15 files changed, 337 insertions(+), 142 deletions(-) diff --git a/RSYNC_COMPAT.md b/RSYNC_COMPAT.md index 84ebe67..fd53e9d 100644 --- a/RSYNC_COMPAT.md +++ b/RSYNC_COMPAT.md @@ -257,14 +257,14 @@ why plain `--append` works on the normal atomic path, not only with `--inplace`. | `-N`, `--crtimes` | Preserve create times | ⛔ Impossible/Divergence | Birth-times cannot be set by any portable filesystem call (`utimensat`/`futimens` only set atime/mtime), so this row is an explicit **Impossible/Divergence** (Phase 7 Wave B). Capture + transmit stays: `statx(STATX_BTIME)` on Linux records the source birth time as a wire field; the receiver logs a debug note that it cannot be applied and continues — never failing the transfer and never pretending it worked. On platforms without `statx` it parses as a documented no-op (flag accepted; nothing is captured). Implies metadata transmission. Wire: new `crtime` fields + a `preserve_crtimes` config boolean; `PROTOCOL_VERSION` bumped **2.11.0 → 2.12.0** (see the Phase-4 metadata-time notes) | | `-O`, `--omit-dir-times` | Omit dirs from --times | ✅ Implemented | Real modifier now that FastSync preserves directory times. With metadata on, the scanner captures every traversed source directory's mtime (and atime under `-U`) and the sender transmits them in trailing `STATUS_DIR_TIMES` frame(s) **after all file data and the optional delete manifest** (chunked at the receiver's `MAX_MANIFEST_ENTRIES` per-frame cap); a dir-time entry only RECORDS metadata and never creates the directory, so empty source directories stay untransferred. The receiver defers applying them until its delete / `--delay-updates` publication phases have committed, so writing or removing a child never clobbers a parent directory's mtime (rsync applies directory times at the end for exactly this reason). When `-O` is set (the boolean crosses the wire) the receiver does not apply any of them; without `-O` an `-a`/`--preserve` transfer now restores directory times (reversing the old "never preserves dir times" divergence). Wire change: the terminal `STATUS_DIR_TIMES` frame; `PROTOCOL_VERSION` bumped **2.16.0 → 2.17.0** | | `-J`, `--omit-link-times` | Omit symlinks from --times | ✅ Implemented | Real modifier now that FastSync preserves symlink times. Symlink entries already carried their metadata on `STATUS_SYMLINK`; the receiver now applies it with **no-follow primitives only** (`utimensat(..., AT_SYMLINK_NOFOLLOW)`, plus best-effort `fchmodat(..., AT_SYMLINK_NOFOLLOW)` and policy-gated `fchownat(..., AT_SYMLINK_NOFOLLOW)`), so the link itself is stamped without ever dereferencing it, confined fd-relative below the authorized receive root. A symlink has no children, so the times are applied immediately at creation. When `-J` is set (the boolean crosses the wire) the receiver skips the timestamps (mode/ownership are unaffected); without `-J` an `-a`/`-l` transfer restores symlink mtimes. Wire change alongside `-O`: the shared `STATUS_DIR_TIMES` frame; `PROTOCOL_VERSION` bumped **2.16.0 → 2.17.0** | -| `--super` | Receiver attempts super-user activities | ✅ Implemented | Phase 7 Wave E: receiver-side **safe-subset + clear-refusal** privilege model, tri-state `super_mode` (auto/on/off). `--super` **permits** the receiver to attempt super-user activities — ownership application and char/block device-node creation — that are already confined fd-relative below the authorized receive root; `--no-super` **forbids** them even when the receiver is root; the default (`auto`) preserves the pre-existing **best-effort** behavior of *attempting* them (not only when already root: an unprivileged attempt is refused by the kernel and skipped per entry, matching FastSync's history). The server additionally accepts an operator-level `--no-super` veto that forces `OFF` for every connection it accepts (so it also refuses any client `--copy-as`/`--super`); the `--fake-super` owner replay and the `--write-devices` write path are gated by the same policy. **FastSync never elevates**: no `setuid`/`seteuid`/`setgid` is ever called, and `--super` never bypasses the confinement floor (`file_open_secure_parent`, `O_NOFOLLOW`, root checks) — it only permits an attempt that is already confined. With `--super` and **no** explicit identity policy (`--usermap`/`--groupmap`/`--chown`/`--numeric-ids`), ownership is treated as raw numeric-id preservation (as if `--numeric-ids`); an explicit identity policy still wins. A non-root receiver given `--super` logs exactly one warning at activation and each confined attempt is then refused by the kernel and skipped per entry (never aborts); `--no-super` suppresses ownership, char/block `mknod`, `--write-devices` and the fake-super owner replay, while unprivileged FIFO creation is unaffected. Wire: one trailing `super_mode` int on the config frame (validated 0..2), sent **before** the `--copy-as` block (fixed order: super int, then copy-as presence int + ids); `PROTOCOL_VERSION` bumped **2.17.0 → 2.18.0**. **Documented divergence from rsync:** rsync's `--super` runs the receiver with elevated privilege; FastSync only permits a confined attempt and never elevates | -| `--fake-super` | Store/recover privileged attrs via xattrs | ✅ Implemented | Phase 7 Wave B: full record **and replay**. The receiver writes the source `uid:gid:mode:mtime_sec:mtime_nsec` into a reserved `user.fastsync.stat` xattr on each written file (best-effort, fd-relative, format unchanged), then immediately re-applies it via `fake_super_restore_fd`: `fchown` (only where privileged — a non-root EPERM/EACCES is skipped silently, matching FastSync's identity philosophy), `fchmod`, and `futimens`. The OWNER leg is additionally skipped when `--no-super` forbids super-user activities (even for root) or when an active `--copy-as` is authoritative, so the recorded source owner can never override a forced `--copy-as` owner; the xattr record is still stored/replayed for a later privileged restore and mode/mtime still apply, so unprivileged `--fake-super` keeps working. The restored mode goes through the same sanitization as the normal metadata path (group/other write bits are never granted, so a recorded 0666 restores as 0644), so fake-super replay can never grant group/other-write that plain `--preserve` would refuse. Absence or a malformed record is a silent no-op, never fatal. The recording format diverges from rsync's `user.rsync.%stat%`; no cross-tool conversion is attempted. Implies metadata transmission so the source uid/gid/mode/mtime are available. Both it and `-X`/`-A` are incompatible with `-s` (chunk serialization), rejected up front | +| `--super` | Receiver attempts super-user activities | ✅ Implemented | Phase 7 Wave E: receiver-side **safe-subset + clear-refusal** privilege model, tri-state `super_mode` (auto/on/off). `--super` **permits** the receiver to attempt super-user activities — ownership application and char/block device-node creation — that are already confined fd-relative below the authorized receive root; `--no-super` **forbids** them even when the receiver is root; the default (`auto`) preserves the pre-existing **best-effort** behavior of *attempting* them (not only when already root: an unprivileged attempt is refused by the kernel and skipped per entry, matching FastSync's history). The server additionally accepts an operator-level `--no-super` veto that forces `OFF` for every connection it accepts (so it also refuses any client `--copy-as`/`--super`); the `--fake-super` owner replay and the `--write-devices` write path are gated by the same policy. **FastSync never elevates**: no `setuid`/`seteuid`/`setgid` is ever called, and `--super` never bypasses the confinement floor (`file_open_secure_parent`, `O_NOFOLLOW`, root checks) — it only permits an attempt that is already confined. `--super` does **not** imply `--numeric-ids` and never enables client-chosen ownership on its own: ownership is applied only when an explicit identity policy (`--usermap`/`--groupmap`/`--chown`/`--numeric-ids`/`--copy-as`) is also given. A non-root receiver given `--super` logs exactly one warning at activation and each confined attempt is then refused by the kernel and skipped per entry (never aborts); `--no-super` suppresses ownership, char/block `mknod`, `--write-devices` and the fake-super owner replay, while unprivileged FIFO creation is unaffected. Wire: one trailing `super_mode` int on the config frame (validated 0..2), sent **before** the `--copy-as` block (fixed order: super int, then copy-as presence int + ids); `PROTOCOL_VERSION` bumped **2.17.0 → 2.18.0**. **Documented divergence from rsync:** rsync's `--super` runs the receiver with elevated privilege; FastSync only permits a confined attempt and never elevates | +| `--fake-super` | Store/recover privileged attrs via xattrs | ✅ Implemented | Phase 7 Wave B: full record **and replay**. The receiver writes the source `uid:gid:mode:mtime_sec:mtime_nsec` into a reserved `user.fastsync.stat` xattr on each written file (best-effort, fd-relative, format unchanged), then immediately re-applies it via `fake_super_restore_fd`: `fchown` (only where privileged — a non-root EPERM/EACCES is skipped silently, matching FastSync's identity philosophy), `fchmod`, and `futimens`. The OWNER leg is additionally skipped unless an explicit ownership identity policy (`--numeric-ids`/`--usermap`/`--groupmap`/`--chown`/`--copy-as`) is active — `--fake-super` on its own only *records* the source owner and must not act as an un-gated chown primitive — when `--no-super` forbids super-user activities (even for root), or when an active `--copy-as` is authoritative, so the recorded source owner can never override a forced `--copy-as` owner; the xattr record is still stored/replayed for a later privileged restore and mode/mtime still apply, so unprivileged `--fake-super` keeps working. The restored mode goes through the same sanitization as the normal metadata path (group/other write bits are never granted, so a recorded 0666 restores as 0644), so fake-super replay can never grant group/other-write that plain `--preserve` would refuse. Absence or a malformed record is a silent no-op, never fatal. The recording format diverges from rsync's `user.rsync.%stat%`; no cross-tool conversion is attempted. Implies metadata transmission so the source uid/gid/mode/mtime are available. Both it and `-X`/`-A` are incompatible with `-s` (chunk serialization), rejected up front | | `--open-noatime` | Avoid changing access time when opening files | ✅ Implemented | Sender-side policy: the sender opens source files with `O_NOATIME` (Linux) when reading them for transfer, so the open/read does NOT bump the source's on-disk access time. Degrades safely when `O_NOATIME` is unavailable (not defined) or refused (`EPERM`, since it needs `CAP_FOWNER` or file ownership): the code falls back to a normal open, so the data always transfers — only the atime-bump is skipped. It does not itself capture/preserve atime; it only avoids modifying it. **Client-only, never crosses the wire.** Exposed as `file_open_for_read()` and applied to both the buffered data path and the sendfile path | | `--numeric-ids` | Do not map uid/gid by name | ✅ Implemented | Ownership is applied through FastSync's opt-in identity path (see the Phase-4 identity notes below). `--numeric-ids` is a mapping-policy modifier: when applying ownership it uses the transmitted numeric uid/gid directly, skipping the name lookup. Without an ownership-affecting option it is inert (FastSync only applies ownership when the user opts in). It does not need `-M` to be parsed, but ownership is only applied when metadata (hence the source uid/gid) is actually transmitted (see the notes) | | `--usermap=STRING` | Map usernames | ✅ Implemented | Opt-in ownership application. rsync subset implemented: comma-separated `FROM:TO` rules evaluated in order, first match wins; `FROM`/`TO` are group/user names (resolved on the SOURCE machine at parse time), `*` (FROM matches any id / TO = the receiving process's current euid), and an `@N` or bare `N` numeric id. Rules are carried over the wire as resolved numeric id pairs; the receiver applies a matching rule (else falls back to `--chown`, `--numeric-ids`, then a best-effort name lookup) via an fd-relative `fchown`. Malformed/unresolvable specs are rejected with a clear error, never a silent no-op. Implies metadata preservation so the source uid/gid travel. Only effective when the receiver can actually change ownership (root or membership); otherwise it warns and continues | | `--groupmap=STRING` | Map group names | ✅ Implemented | Same rsync subset and semantics as `--usermap` but for the group (gid) side and the group databases. See the Phase-4 identity notes | | `--chown=USER:GROUP` | Map owner and group | ✅ Implemented | Opt-in ownership override applied receiver-side. Forms: `USER:GROUP`, `USER` (owner only), `:GROUP` (group only); a `*` for USER/GROUP means the current/root user or group as appropriate; an `@N`/bare `N` numeric id is accepted. A `:` inside a name may be escaped as `\:`. Equivalent to a trailing `*:*` usermap+groupmap rule (so an explicit `--usermap`/`--groupmap` match wins). Malformed or unresolvable specs are clear parse errors. Implies metadata preservation. Only effective when the receiver has permission to chown; otherwise it warns and continues (rsync parity) | -| `--copy-as=USER[:GROUP]` | Perform the copy as another user/group | ✅ Implemented | Safe-subset implementation, an explicit divergence from rsync's **real identity switching**. rsync makes the receiving process actually assume USER/GROUP (setuid/setgid); FastSync's receiver is multithreaded, so a real credential drop would be unsafe and is never attempted — FastSync never calls `setuid`/`seteuid`/`setgid`. Instead the receiver FORCES the ownership of every entry it writes to `copy_as_uid`/`copy_as_gid` through the existing confined, fd-relative identity path (the same `fchown`/`fchownat` mechanism as `--chown`/`--usermap`/`--groupmap`; symlinks use `fchownat(..., AT_SYMLINK_NOFOLLOW)`, and directories — including intermediate parents created implicitly while writing a nested file — and char/block/FIFO nodes are owned no-follow too, so a directory never keeps the receiver's owner while its children get the target owner), with `--copy-as` at the **highest priority** — it beats usermap/groupmap/`--chown`/`--numeric-ids` and the best-effort name lookup. This REQUIRES a privileged (root) receiver: an unprivileged receiver REFUSES the whole transfer up front at the config handshake (`server_module_gate`, running inside `config_receive_with_validate` before the `STATUS_OK` ack) with a clear error and no file data exchanged — never a silent wrong-ownership result. A server running with an operator `--no-super` veto also refuses it, and a **daemon** refuses `--copy-as` outright even when root: FastSync has no per-module opt-in for client-chosen ownership, so a daemon must not honor an arbitrary client-selected owner (the standalone listener and SSH `--stdio` server keep honoring it for their single operator-authorized root). `--fake-super` interaction: `--copy-as` is authoritative, so the recorded source owner is never replayed over the forced target owner. If the ownership apply still fails with EPERM/EACCES (capability-restricted root, root-squash, read-only mount) the failure is logged at ERROR but remains non-fatal (the multithreaded receiver is never aborted). USER is resolved on the client against the user database (a name, an `@N`/bare `N` numeric id, or `*` meaning the client's current euid); when `:GROUP` is present it is resolved against the group database (`*` meaning the client's egid). **Group-default rule:** when the group is omitted FastSync uses the user's primary gid (`getpwuid(uid)->pw_gid`); a numeric id with no local passwd entry has no primary gid to look up, so `gid` falls back to `uid` (documented divergence). Malformed/empty/unresolvable specs are clear parse errors, never a silent no-op. Never elevates privileges and never bypasses the confined receive root. Implies metadata preservation (the source uid/gid must be transmitted). Wire: a new trailing config-frame block **sent after** the `--super` int (presence int, then the two int32 ids, both validated `>= 0` on receive; the ids are also rejected if they do not fit int32 at CLI parse time); `PROTOCOL_VERSION` bumped **2.17.0 → 2.18.0** | +| `--copy-as=USER[:GROUP]` | Perform the copy as another user/group | ✅ Implemented | Safe-subset implementation, an explicit divergence from rsync's **real identity switching**. rsync makes the receiving process actually assume USER/GROUP (setuid/setgid); FastSync's receiver is multithreaded, so a real credential drop would be unsafe and is never attempted — FastSync never calls `setuid`/`seteuid`/`setgid`. Instead the receiver FORCES the ownership of every entry it writes to `copy_as_uid`/`copy_as_gid` through the existing confined, fd-relative identity path (the same `fchown`/`fchownat` mechanism as `--chown`/`--usermap`/`--groupmap`; symlinks use `fchownat(..., AT_SYMLINK_NOFOLLOW)`, and directories — including intermediate parents created implicitly while writing a nested file — and char/block/FIFO nodes are owned no-follow too, so a directory never keeps the receiver's owner while its children get the target owner), with `--copy-as` at the **highest priority** — it beats usermap/groupmap/`--chown`/`--numeric-ids` and the best-effort name lookup. This REQUIRES a privileged (root) receiver: an unprivileged receiver REFUSES the whole transfer up front at the config handshake (`server_module_gate`, running inside `config_receive_with_validate` before the `STATUS_OK` ack) with a clear error and no file data exchanged — never a silent wrong-ownership result. A server running with an operator `--no-super` veto also refuses it, and a **daemon** refuses `--copy-as`, like every other client-chosen-ownership request (`--numeric-ids`/`--chown`/`--usermap`/`--groupmap`/`--fake-super`/explicit `--super`), unless the selected module opts in with `client owner = yes`; without that per-module opt-in a daemon must not honor an arbitrary client-selected owner (the standalone listener and SSH `--stdio` server keep honoring these for their single operator-authorized root). `--fake-super` interaction: `--copy-as` is authoritative, so the recorded source owner is never replayed over the forced target owner. If the ownership apply still fails with EPERM/EACCES (capability-restricted root, root-squash, read-only mount) the failure is logged at ERROR and the **entry is reported as failed** rather than written with the wrong owner (the receiver never claims a `--copy-as` success it did not achieve), while a single entry failure does not abort the multithreaded run. USER is resolved on the client against the user database (a name, an `@N`/bare `N` numeric id, or `*` meaning the client's current euid); when `:GROUP` is present it is resolved against the group database (`*` meaning the client's egid). **Group-default rule:** when the group is omitted FastSync uses the user's primary gid (`getpwuid(uid)->pw_gid`); a numeric id with no local passwd entry has no primary gid to look up, so `gid` falls back to `uid` (documented divergence). Malformed/empty/unresolvable specs are clear parse errors, never a silent no-op. Never elevates privileges and never bypasses the confined receive root. Implies metadata preservation (the source uid/gid must be transmitted). Wire: a new trailing config-frame block **sent after** the `--super` int (presence int, then the two int32 ids, both validated `>= 0` on receive; the ids are also rejected if they do not fit int32 at CLI parse time); `PROTOCOL_VERSION` bumped **2.17.0 → 2.18.0** | **Phase-4 metadata-time notes:** `-U/--atimes`, `-N/--crtimes`, `-O/--omit-dir-times`, `-J/--omit-link-times`, and `--open-noatime` are new. @@ -625,7 +625,7 @@ now transmits targets (the prior behavior was broken/partial); its status moved | Flag | Rsync Description | FastSync Status | Notes | |------|-------------------|-----------------|-------| -| `--daemon` | Run as rsync daemon | ✅ Implemented | Wave A: a real persistent listener. `fastsync-server --daemon --config FILE` (plus `--no-detach` to stay foreground; without it the listener detaches to the background after binding) reads a FastSync-native module config file and serves each connection confined to the requested module's `path` root (never a client-chosen root; a client `--copy-as` is refused outright and the operator `--no-super` veto is honored). TCP/TLS via the existing `--tls` stack; plaintext still requires `--allow-unauthenticated` (same secure default as the standalone server). Client destinations use rsync's `host::module/path` form. Wire/protocol: the config frame gained a trailing daemon-module string and `PROTOCOL_VERSION` was bumped **2.14.0 → 2.15.0** (see the Daemon Mode notes below). Daemon mode is built in FastSync's own protocol/config grammar, not rsync's SMB/daemon option encoding | +| `--daemon` | Run as rsync daemon | ✅ Implemented | Wave A: a real persistent listener. `fastsync-server --daemon --config FILE` (plus `--no-detach` to stay foreground; without it the listener detaches to the background after binding) reads a FastSync-native module config file and serves each connection confined to the requested module's `path` root (never a client-chosen root; every client-chosen-ownership/super-user request (`--numeric-ids`/`--chown`/`--usermap`/`--groupmap`/`--fake-super`/`--copy-as`/explicit `--super`) is refused unless the module opts in with `client owner = yes`, and the operator `--no-super` veto is honored). TCP/TLS via the existing `--tls` stack; plaintext still requires `--allow-unauthenticated` (same secure default as the standalone server). Client destinations use rsync's `host::module/path` form. Wire/protocol: the config frame gained a trailing daemon-module string and `PROTOCOL_VERSION` was bumped **2.14.0 → 2.15.0** (see the Daemon Mode notes below). Daemon mode is built in FastSync's own protocol/config grammar, not rsync's SMB/daemon option encoding | | `--config=FILE` | Alternate rsyncd.conf file | ✅ Implemented | Wave A: selects the daemon config file. Default when omitted (in `--daemon` mode): `~/.config/fastsync/fastsyncd.conf` if it exists, else `/etc/fastsyncd.conf`. The grammar is FastSync-native (documented in the Daemon Mode notes below) and strictly rejects unknown keys so a typo can never silently change what a module serves; requires `--daemon` | | `--dparam=OVERRIDE` | Override global daemon config | ✅ Implemented | Wave A: overrides one global scalar from the command line (`--dparam port=8734` and `--dparam=KEY=VALUE` both work). Limited to the global scalar keys the grammar defines (`port`, `motd file`, `address`); keys are case-insensitive and unknown keys/invalid values are rejected. Requires `--daemon` | | `--no-detach` | Don't detach from parent | ✅ Implemented | Wave A: with `--daemon`, keeps the listener in the foreground (what integration tests use). Without it the daemonizes (fork/setsid, stdio redirected to /dev/null) after the listening socket is bound. Requires `--daemon` | @@ -634,8 +634,9 @@ now transmits targets (the prior behavior was broken/partial); its status moved **Daemon Mode notes (Wave A, protocol 2.15.0; Wave B auth, Wave C MOTD, no bump):** FastSync daemon mode is supported in FastSync's own protocol/config grammar, not rsync's SMB/daemon option encoding. -- **Config grammar** (`fastsyncd.conf`): line-based; an implicit global section first, then `[module]` sections. Keys are case-insensitive, values are trimmed and may be wrapped in one layer of double quotes (`path = "/srv/my dir"`). `#` and `;` at the start of a line (after leading whitespace) are full-line comments; inline comments and `\` continuations are not supported. Lines are bounded (4096 chars). Global keys: `port` (default 873), `motd file` (the daemon sends its bounded, escaped content to a client after the module gate/auth accepts, unless the client passes `--no-motd`), `address` (optional bind address). Module keys: `path` (required; the daemon-side authorized root for that module), `read only` (yes/no/true/false/1/0, default no), `auth users` (comma list). **Unknown keys and malformed lines are parse-and-reject errors** (never silently ignored), so a typo cannot change what a module serves. -- **Module selection & confinement:** the client requests a module with an rsync-style `host::module[/path]` destination. The module name crosses the wire as a trailing string on the config frame (bumping `PROTOCOL_VERSION` 2.14.0 → 2.15.0; the bump is required because the config-frame layout changed and the strict same-version handshake is what prevents a peer from desynchronizing on the new trailing field). The daemon looks the module up in ITS OWN config and uses the module's `path` as the authorized root through the exact same `configure_authorization` confinement the standalone server applies to `--destination-root` (`file_open_secure_parent`, `has_path_traversal`, `path_is_within`); the client never supplies the root, a client `--copy-as` is refused outright (no per-module opt-in for client-chosen ownership), and the operator `--no-super` veto forces super-user activities off for every daemon connection. The client's `/path` part is relative inside the module and is rejected if absolute or if it contains `..`. Unknown modules are refused before any data moves (the run fails cleanly at the config handshake). An absolute destination and a module request against a non-daemon server are also refused. +- **Config grammar** (`fastsyncd.conf`): line-based; an implicit global section first, then `[module]` sections. Keys are case-insensitive, values are trimmed and may be wrapped in one layer of double quotes (`path = "/srv/my dir"`). `#` and `;` at the start of a line (after leading whitespace) are full-line comments; inline comments and `\` continuations are not supported. Lines are bounded (4096 chars). Global keys: `port` (default 873), `motd file` (the daemon sends its bounded, escaped content to a client after the module gate/auth accepts, unless the client passes `--no-motd`), `address` (optional bind address). Module keys: `path` (required; the daemon-side authorized root for that module), `read only` (yes/no/true/false/1/0, default no), `client owner` (yes/no/true/false/1/0, default no; opts the module into client-chosen ownership — see below), `auth users` (comma list). **Unknown keys and malformed lines are parse-and-reject errors** (never silently ignored), so a typo cannot change what a module serves. +- **Module selection & confinement:** the client requests a module with an rsync-style `host::module[/path]` destination. The module name crosses the wire as a trailing string on the config frame (bumping `PROTOCOL_VERSION` 2.14.0 → 2.15.0; the bump is required because the config-frame layout changed and the strict same-version handshake is what prevents a peer from desynchronizing on the new trailing field). The daemon looks the module up in ITS OWN config and uses the module's `path` as the authorized root through the exact same `configure_authorization` confinement the standalone server applies to `--destination-root` (`file_open_secure_parent`, `has_path_traversal`, `path_is_within`); the client never supplies the root, every client-chosen-ownership/super-user request is refused unless the module declares `client owner = yes` (the daemon's per-module opt-in, see below), and the operator `--no-super` veto forces super-user activities off for every daemon connection. The client's `/path` part is relative inside the module and is rejected if absolute or if it contains `..`. Unknown modules are refused before any data moves (the run fails cleanly at the config handshake). An absolute destination and a module request against a non-daemon server are also refused. +- **`client owner` (client-chosen-ownership opt-in):** by default a daemon module refuses every request that would let the client pick an owner or ask for super-user activities — `--numeric-ids`, `--chown`, `--usermap`/`--groupmap`, `--fake-super`, `--copy-as`, and an explicit `--super` — at the config handshake (before `STATUS_OK`), because a daemon has no per-module opt-in for client-chosen ownership and any anonymous client could otherwise force arbitrary owner ids inside the module root. `client owner = yes` opts a single module in, allowing those requests within that module's root (the standalone listener and the SSH `--stdio` server always honor them for their single operator-authorized root). The opt-in does **not** lift the privilege requirement: `--copy-as` still needs a root receiver, and the operator `--no-super` veto still forces super-user activities off for every connection. The daemon logs a prominent startup warning for each `client owner = yes` module so the operator's deliberate choice is visible. - **`read only` safe default:** every network transfer FastSync currently supports is a push that writes under the module root, so a `read only` module refuses the connection (clear server log "module is read only"; the client exits non-zero, nothing is transferred). A future pull/list operation can be opened up when it exists; the knob is already stored. - **`auth users` (Wave B password authentication):** a module that declares `auth users` requires the client to present credentials. The client sends a username + the lowercase hex SHA-256 of the password (never the literal password) in the config frame; the daemon accepts a connection only when the presented username is **on the module's `auth users` list** AND the presented digest matches that user's credential-store entry. Verification is constant-time (username present/absent both take the same comparison work, so there is no timing oracle distinguishing "unknown user" from "wrong password"), and the daemon logs the username but **never the digest or the password**. A module WITHOUT `auth users` stays open (legitimate rsync configuration); credentials sent to such a module are ignored. Read-only is orthogonal: even a correctly authenticated push to a `read only` module is still refused (all FastSync network transfers write). Fail-closed policy: a daemon whose config declares `auth users` on any module refuses to start unless a credential store was given (`--password-file` and/or `--early-input`); a missing or empty store is never silently treated as "open". - **Credential store format:** server `--password-file`/`--early-input` files are line-based `user:SHA256HEX`, one per line, where `SHA256HEX` is the lowercase hex SHA-256 of the user's password (exactly what the client transmits). Blank lines and lines starting with `#`/`;` are comments; the parser is strict (a malformed line fails the whole load, so a typo can never let a different set of users in). The client `--password-file` holds `user:password` on its first meaningful line (the literal password, hashed client-side then wiped from memory); keep both files readable only by their owner (mode 0600) since the client file holds the password and the server file holds the equivalent credential. Per-username wire length is bounded (256 chars) and digests are validated to be exactly 64 lowercase hex on receive. @@ -830,7 +831,7 @@ These are the last compatibility items and the closing phase toward rsync flag p `--super`/`--no-super` set a receiver-side tri-state `Config->super_mode` (`SUPER_MODE_AUTO`/`ON`/`OFF`). `privilege_super_permitted()` / `privilege_super_mode_permitted()` (src/shared/identity.c) return true for `ON` and `AUTO` (AUTO preserves FastSync's historical best-effort attempt, where the kernel refuses an unprivileged call and the caller skips it) and false only for `OFF`. The gate covers every super-user activity FastSync performs: ownership application (`identity_apply_ownership`/`_link`), char/block device-node creation (`file_save_special_to_disk`), writes into an existing device (`--write-devices`), and the `--fake-super` owner replay. Unprivileged FIFO creation is deliberately unaffected. With `ON` and no explicit identity policy, ownership falls back to raw numeric-id preservation (as `--numeric-ids`); explicit `--usermap`/`--groupmap`/`--chown`/`--numeric-ids` still win. `--no-super` suppresses those activities even for a root receiver. A non-root receiver given `--super` logs one warning at activation (`identity_set_active`); each confined attempt is then refused by the kernel and skipped, never aborting. The confinement floor is unchanged (`file_open_secure_parent`, `O_NOFOLLOW`, root/path checks). Operator control: the server CLI accepts `--no-super`, a veto that forces `OFF` for every connection and refuses client `--copy-as`/`--super`. -`--copy-as=USER[:GROUP]` is the safe subset. FastSync's receiver is multithreaded, so a real credential switch is unsafe; instead the receiver forces the ownership of **every entry it writes** — regular files, symlinks, directories (including implicitly-created parents), and special nodes — to the resolved target ids through the confined fd-relative identity path. USER is resolved on the client (name, `@N`/bare N, or `*` = client euid); when `:GROUP` is omitted the user's primary gid is used (falling back to `gid == uid` for a numeric id with no local passwd entry). It requires a privileged (root) receiver: an unprivileged receiver refuses the whole transfer at the config handshake, before `STATUS_OK`, so no data is ever written with the wrong ownership. A `--copy-as` chown failure on a capability-restricted root is logged at ERROR (never silently downgraded). `--copy-as` implies metadata (`--no-preserve` is rejected) and `--fake-super` cannot override it. Daemon divergence: a `--daemon` receiver refuses `--copy-as` **and** `--super`=ON outright, because there is no per-module operator opt-in for client-chosen ownership (the standalone listener and the SSH-launched `--stdio` server, which each serve one operator-authorized root, honor them). +`--copy-as=USER[:GROUP]` is the safe subset. FastSync's receiver is multithreaded, so a real credential switch is unsafe; instead the receiver forces the ownership of **every entry it writes** — regular files, symlinks, directories (including implicitly-created parents), and special nodes — to the resolved target ids through the confined fd-relative identity path. USER is resolved on the client (name, `@N`/bare N, or `*` = client euid); when `:GROUP` is omitted the user's primary gid is used (falling back to `gid == uid` for a numeric id with no local passwd entry). It requires a privileged (root) receiver: an unprivileged receiver refuses the whole transfer at the config handshake, before `STATUS_OK`, so no data is ever written with the wrong ownership. A `--copy-as` chown failure on a capability-restricted root is logged at ERROR (never silently downgraded). `--copy-as` implies metadata (`--no-preserve` is rejected) and `--fake-super` cannot override it. Daemon policy: a `--daemon` receiver refuses **every** client-chosen-ownership / super-user request — `--numeric-ids`, `--chown`, `--usermap`/`--groupmap`, `--fake-super`, `--copy-as`, and explicit `--super` — unless the selected module opts in with `client owner = yes`; without that per-module opt-in any client could force arbitrary ownership inside the module root (the standalone listener and the SSH-launched `--stdio` server, which each serve one operator-authorized root, honor these requests). A `--copy-as` chown failure on a capability-restricted root marks the entry as failed rather than reporting success with the wrong owner. **Wire:** two trailing config-frame blocks after the `--iconv` spec, in fixed order — `send_privilege_options`/`receive_privilege_options` (one `super_mode` int, validated `0..2`), then `send_copy_as_options`/`receive_copy_as_options` (presence int + two int32 ids, validated `>= 0`, with `copy_as_set ⇒ use_metadata`). `PROTOCOL_VERSION` bumped **2.17.0 → 2.18.0**. **Divergences from rsync:** rsync's `--super` elevates the receiver and `--copy-as` actually switches its credentials; FastSync never elevates and only permits/forwards confined attempts, and `--copy-as` forces ownership rather than switching identity. diff --git a/src/server/server.c b/src/server/server.c index e8edb13..a29dfc4 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -172,46 +172,29 @@ static bool configure_authorization(const char* root) { * --destination-root, but per-module and NEVER client-chosen. The module is * refused (with a clear log) when it is unknown, when it is `read only` (every * FastSync network transfer writes; there is no read-only wire operation yet), - * or when the presented daemon credentials fail for a module that declares - * `auth users`. Wave A refused every auth-required module (auth was not yet - * implemented); Wave B authenticates the client instead (see below). */ + * when it requests client-chosen ownership without the module's + * `client owner = yes` opt-in (P7 Wave E hardening), or when the presented + * daemon credentials fail for a module that declares `auth users`. Wave A + * refused every auth-required module (auth was not yet implemented); Wave B + * authenticates the client instead (see below). */ static const char* server_module_gate(const Config* config, void* context) { ModuleGateContext* gate_ctx = (ModuleGateContext*)context; if (!config) return "missing config frame"; - /* --copy-as (P7 Wave E, protocol 2.18.0): FastSync's safe subset forces the - ownership of every written entry to the requested ids, which needs a - privileged (root) receiver. An unprivileged receiver REFUSES the whole - transfer here, at the config handshake and BEFORE the STATUS_OK ack, so no - file data is exchanged and there is never a silent wrong-ownership result. - Placed first so it applies to the standalone server and daemon alike. */ - /* Daemon divergence (P7 Wave E): a daemon has no per-module opt-in for - client-chosen ownership, so it refuses --copy-as outright even when running - as root -- otherwise any anonymous client could pick an arbitrary owner. - The standalone listener and the SSH-launched --stdio server keep honoring - it (they serve exactly one operator-authorized root). */ - if (g_daemon_conf != NULL && config->copy_as_set) { - log_message(LOG_LEVEL_ERROR, "--copy-as is refused by the daemon (no per-module opt-in for " - "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. */ Config* effective = (Config*)config; if (server_no_super) effective->super_mode = SUPER_MODE_OFF; + /* --copy-as (P7 Wave E, protocol 2.18.0): FastSync's safe subset forces the + ownership of every written entry to the requested ids, which needs a + privileged (root) receiver. An unprivileged receiver REFUSES the whole + transfer here, at the config handshake and BEFORE the STATUS_OK ack, so no + file data is exchanged and there is never a silent wrong-ownership result. + The daemon's per-module client-chosen-ownership refusal is enforced after + the module lookup below (it needs the module's opt-in) and covers --copy-as + like every other ownership flag. */ if (identity_copy_as_refused(effective)) { if (geteuid() != 0) log_message(LOG_LEVEL_ERROR, "--copy-as requires a privileged receiver (root); refusing"); @@ -256,6 +239,20 @@ static const char* server_module_gate(const Config* config, void* context) { config->module); return "requested daemon module is read only"; } + /* Client-chosen ownership / super-user policy (P7 Wave E hardening): a daemon + module refuses EVERY ownership-affecting request (--numeric-ids, --chown, + --usermap/--groupmap, --fake-super, --copy-as, explicit --super) unless the + operator opted THIS module in with `client owner = yes`. Otherwise any + client could force arbitrary ownership inside the module root. The + standalone/SSH server has a single operator-authorized root and keeps + honoring these. */ + if (!module->client_owner && identity_ownership_requested(effective)) { + log_message(LOG_LEVEL_ERROR, + "daemon module '%s' refuses client-chosen ownership/super-user activities " + "(no `client owner = yes` opt-in); refusing", + config->module); + return "client-chosen ownership is not permitted by this daemon module"; + } if (module->auth_user_count > 0) { /* Auth-required module (Wave B): verify the presented credentials against * the store BEFORE the module root is installed and before any data moves. @@ -766,6 +763,17 @@ int main(int argc, char* argv[]) { if (g_daemon_conf->module_count == 0) log_message(LOG_LEVEL_WARNING, "daemon config has no modules; every connection will be refused"); + /* Surface the operator's client-chosen-ownership opt-in prominently: an + opted-in module lets its clients request arbitrary owner ids inside that + module root. */ + for (int i = 0; i < g_daemon_conf->module_count; i++) { + if (g_daemon_conf->modules[i].client_owner) + log_message(LOG_LEVEL_WARNING, + "daemon module '%s' allows client-chosen ownership " + "(`client owner = yes`); clients may request arbitrary owner ids within " + "that module root", + g_daemon_conf->modules[i].name); + } /* Daemon credential store (Wave B). --password-file and --early-input * feed the same store, loaded BEFORE the listener forks so every * connection child shares one read-only store. Fail closed at startup: a diff --git a/src/shared/config.h b/src/shared/config.h index a7525ab..9ffe0ea 100644 --- a/src/shared/config.h +++ b/src/shared/config.h @@ -404,18 +404,19 @@ typedef struct Config { /* --super / --no-super (P7 Wave E, protocol 2.18.0): receiver-side privilege * policy for super-user activities confined below the authorized receive - * root. SUPER_MODE_AUTO (default) preserves the pre-existing behavior: a - * privileged operation is only attempted when the receiver is ALREADY root - * (geteuid() == 0). SUPER_MODE_ON (--super) PERMITS the receiver to attempt - * those activities (ownership application, char/block device-node creation) - * even when it is not root -- the attempt is then confined exactly as before - * and simply fails/skips if the kernel refuses it. SUPER_MODE_OFF + * root. SUPER_MODE_AUTO (default) preserves the pre-existing best-effort + * behavior: the confined super-user operation is ALWAYS attempted and an + * unprivileged attempt is refused by the kernel and skipped per entry. + * SUPER_MODE_ON (--super) explicitly REQUESTS those activities (char/block + * device-node creation, --write-devices); it does NOT imply --numeric-ids and + * never enables ownership application on its own. SUPER_MODE_OFF * (--no-super) FORBIDS them even when running as root. FastSync NEVER * elevates privileges (no setuid/seteuid/setgid) and never bypasses the * fd-relative confinement (file_open_secure_parent, O_NOFOLLOW, root checks); * --super only permits an attempt that is already confined. Crosses the wire * as a trailing int so the receiver can enforce the policy. See - * privilege_super_permitted() in identity.h. */ + * privilege_super_permitted() and identity_ownership_requested() in + * identity.h. */ int super_mode; // Receiver-side runtime staging registry for --delay-updates. Never sent diff --git a/src/shared/daemon_conf.c b/src/shared/daemon_conf.c index 60e3d27..e806549 100644 --- a/src/shared/daemon_conf.c +++ b/src/shared/daemon_conf.c @@ -170,6 +170,17 @@ static bool apply_module_key(DaemonModule* module, char* key, char* value, char* module->read_only = parsed; return true; } + if (key_equals(key, "client owner")) { + bool parsed; + if (!parse_bool_value(value, &parsed)) { + set_error(err, err_size, + "module '%s': 'client owner' must be yes/no (or true/false/1/0), got '%s'", + module->name, value); + return false; + } + module->client_owner = parsed; + return true; + } if (key_equals(key, "auth users")) { char* list = str_dup(value); if (!list) { diff --git a/src/shared/daemon_conf.h b/src/shared/daemon_conf.h index c25a719..0ecfe24 100644 --- a/src/shared/daemon_conf.h +++ b/src/shared/daemon_conf.h @@ -24,12 +24,16 @@ * selects this module to this path (file_open_secure_parent / * has_path_traversal / path_is_within all keep the existing confinement, just * per-module). There is never any client-chosen root: a module path always - * stays confined. A daemon also REFUSES a client --copy-as outright, because - * there is no per-module opt-in for client-chosen ownership (unlike the - * standalone/SSH server, which honors it for its single operator-authorized - * root); the operator-level --no-super veto additionally forces super-user - * activities off for every daemon connection. See server_module_gate in - * server.c and RSYNC_COMPAT.md. + * stays confined. A daemon REFUSES every client-chosen ownership / super-user + * request by default -- --numeric-ids, --chown, --usermap/--groupmap, + * --fake-super, --copy-as and an explicit --super -- because there is no + * per-module opt-in unless the operator adds one. An operator opts a single + * module in with `client owner = yes` (DaemonModule.client_owner), which allows + * that client to choose ownership within that module's root (the standalone/SSH + * server honors such requests for its single operator-authorized root). The + * operator-level --no-super veto additionally forces super-user activities off + * for every daemon connection, even an opted-in module. See server_module_gate + * in server.c and RSYNC_COMPAT.md. * * `auth_users` is honored by Wave B daemon authentication: a module that * declares auth users accepts a connection only when the presented username is @@ -41,6 +45,11 @@ typedef struct DaemonModule { char* name; /* module name, as the client requests it */ char* path; /* module root (daemon-side authorized root) */ bool read_only; /* `read only = yes/no`; default no */ + bool client_owner; /* `client owner = yes/no`; default no. Per-module opt-in + that lets this module's clients choose ownership + (--numeric-ids/--chown/--usermap/--groupmap/--fake-super/ + --copy-as) and request explicit --super super-user + activities. Without it the daemon refuses all of them. */ char** auth_users; /* `auth users = a,b`; Wave B credential list */ int auth_user_count; } DaemonModule; diff --git a/src/shared/file_receive.c b/src/shared/file_receive.c index 10037c1..5d23361 100644 --- a/src/shared/file_receive.c +++ b/src/shared/file_receive.c @@ -468,13 +468,16 @@ static FileSaveResult file_save_special_to_disk(const char* root_directory, cons every entry (a char/block node path is already privilege-gated above). The no-follow helper changes the node's own ownership without dereferencing it; it is a no-op unless an identity policy is active. */ + bool owner_ok = true; if (identity_active_enabled()) - identity_apply_ownership_link(parent_fd, leaf, (int32_t)file->metadata->uid, - (int32_t)file->metadata->gid); + owner_ok = identity_apply_ownership_link(parent_fd, leaf, (int32_t)file->metadata->uid, + (int32_t)file->metadata->gid); close(parent_fd); free(leaf); free(destination); - return FILE_SAVE_WRITTEN; + /* A failed required --copy-as ownership marks the node as failed; every other + * identity policy stays best-effort. */ + return owner_ok ? FILE_SAVE_WRITTEN : FILE_SAVE_ERROR; } /* --write-devices (receiver): write the received data directly into an EXISTING @@ -626,8 +629,9 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi char* leaf = NULL; int parent_fd = file_open_secure_parent(dir_path, &leaf, false); if (parent_fd >= 0) { - identity_apply_ownership_link(parent_fd, leaf, (int32_t)file->metadata->uid, - (int32_t)file->metadata->gid); + if (!identity_apply_ownership_link(parent_fd, leaf, (int32_t)file->metadata->uid, + (int32_t)file->metadata->gid)) + ok = false; close(parent_fd); } free(leaf); diff --git a/src/shared/identity.c b/src/shared/identity.c index 023d530..6f00efd 100644 --- a/src/shared/identity.c +++ b/src/shared/identity.c @@ -128,29 +128,29 @@ bool privilege_super_mode_permitted(int mode) { return mode != SUPER_MODE_OFF; } -/* --super with NO explicit identity policy implies raw numeric-id preservation, - * exactly as if --numeric-ids had been given. An explicit usermap/groupmap/ - * --chown/--numeric-ids always wins: identity_resolve_targets() checks those - * before the numeric fallback, and this predicate is false whenever any of them - * is present. In AUTO (the default) no implication is made, preserving the - * opt-in-only behavior. */ -static bool identity_super_implies_numeric(void) { - return g_identity.super_mode == SUPER_MODE_ON && !g_identity.numeric_ids && - !g_identity.chown_uid_set && !g_identity.chown_gid_set && g_identity.usermap_count == 0 && - g_identity.groupmap_count == 0; -} - bool identity_active_enabled(void) { /* numeric_ids is included: this set only gates identity_apply_ownership, which runs only when metadata is present (a -M/--preserve transfer). A standalone --numeric-ids (no ownership-affecting flag) carries no metadata, never reaches identity_apply_ownership, and therefore correctly - stays inert; combined with -M it activates raw-id application. --super - with no explicit identity policy acts like --numeric-ids here. */ + stays inert; combined with -M it activates raw-id application. --super / + --no-super does NOT enable ownership: it only permits or forbids the + already-requested super-user activities, so a --super with no explicit + identity flag must never silently apply client-chosen ownership. */ return g_identity.set && (g_identity.numeric_ids || g_identity.chown_uid_set || g_identity.chown_gid_set || - g_identity.usermap_count > 0 || g_identity.groupmap_count > 0 || g_identity.copy_as_set || - identity_super_implies_numeric()); + g_identity.usermap_count > 0 || g_identity.groupmap_count > 0 || g_identity.copy_as_set); +} + +bool identity_ownership_requested(const Config* config) { + if (!config) + return false; + /* Every value that makes the receiver act on a client-chosen owner, plus an + * explicit --super (super-user device-node activities). Pure config, so the + * daemon gate can evaluate it before identity_set_active(). */ + return config->numeric_ids || config->chown_uid_set || config->chown_gid_set || + config->usermap_count > 0 || config->groupmap_count > 0 || config->copy_as_set || + config->fake_super || config->super_mode == SUPER_MODE_ON; } bool identity_copy_as_active(void) { @@ -613,7 +613,7 @@ static bool identity_resolve_targets(const struct stat* st, int32_t source_uid, } else if (g_identity.chown_uid_set) { uid = g_identity.chown_uid == IDENTITY_CURRENT ? geteuid() : (uid_t)g_identity.chown_uid; set_uid = true; - } else if (g_identity.numeric_ids || identity_super_implies_numeric()) { + } else if (g_identity.numeric_ids) { uid = (uid_t)source_uid; set_uid = true; } else { @@ -637,7 +637,7 @@ static bool identity_resolve_targets(const struct stat* st, int32_t source_uid, } else if (g_identity.chown_gid_set) { gid = g_identity.chown_gid == IDENTITY_CURRENT ? getegid() : (gid_t)g_identity.chown_gid; set_gid = true; - } else if (g_identity.numeric_ids || identity_super_implies_numeric()) { + } else if (g_identity.numeric_ids) { gid = (gid_t)source_gid; set_gid = true; } else { @@ -676,9 +676,11 @@ static void identity_log_chown_failure(const char* what, uid_t uid, gid_t gid) { * --copy-as is different: the whole point of the flag is that the target * ownership is REQUIRED (the pre-flight gate already refused an unprivileged * receiver). If the chown still fails with EPERM/EACCES (a capability- - * restricted root, root-squash, or a read-only mount) the run is silently - * producing the WRONG ownership, so surface it at ERROR. It stays - * non-fatal: never abort the multithreaded receiver mid-transfer. */ + * restricted root, root-squash, or a read-only mount) the run would be + * silently producing the WRONG ownership, so surface it at ERROR. The + * caller (identity_apply_ownership*) then reports the ENTRY as failed rather + * than as written; the receiver never claims a --copy-as success it did not + * achieve, but a single entry failure does not abort the whole run. */ if (errno == EPERM || errno == EACCES) { if (identity_copy_as_active()) log_message(LOG_LEVEL_ERROR, @@ -695,35 +697,43 @@ static void identity_log_chown_failure(const char* what, uid_t uid, gid_t gid) { } } -void identity_apply_ownership(int fd, int32_t source_uid, int32_t source_gid) { +bool identity_apply_ownership(int fd, int32_t source_uid, int32_t source_gid) { /* Ownership application is OFF unless the client requested an identity flag. * This is the controlled gate: a default (or plain -M) transfer never changes * ownership, byte-for-byte preserving FastSync's existing behavior. --no-super * additionally forbids it even when the receiver is root. */ if (!identity_active_enabled() || !privilege_super_permitted() || fd < 0) - return; + return true; struct stat st; if (fstat(fd, &st) != 0) - return; + return !identity_copy_as_active(); uid_t uid; gid_t gid; if (!identity_resolve_targets(&st, source_uid, source_gid, &uid, &gid)) - return; - if (fchown(fd, uid, gid) != 0) + return true; + if (fchown(fd, uid, gid) != 0) { identity_log_chown_failure("file", uid, gid); + /* A required --copy-as ownership that did not land is a per-entry failure; + * every other policy stays best-effort (rsync parity). */ + return !identity_copy_as_active(); + } + return true; } -void identity_apply_ownership_link(int parent_fd, const char* leaf, int32_t source_uid, +bool identity_apply_ownership_link(int parent_fd, const char* leaf, int32_t source_uid, int32_t source_gid) { if (!identity_active_enabled() || !privilege_super_permitted() || parent_fd < 0 || !leaf) - return; + return true; struct stat st; if (fstatat(parent_fd, leaf, &st, AT_SYMLINK_NOFOLLOW) != 0) - return; + return !identity_copy_as_active(); uid_t uid; gid_t gid; if (!identity_resolve_targets(&st, source_uid, source_gid, &uid, &gid)) - return; - if (fchownat(parent_fd, leaf, uid, gid, AT_SYMLINK_NOFOLLOW) != 0) + return true; + if (fchownat(parent_fd, leaf, uid, gid, AT_SYMLINK_NOFOLLOW) != 0) { identity_log_chown_failure("no-follow entry", uid, gid); + return !identity_copy_as_active(); + } + return true; } diff --git a/src/shared/identity.h b/src/shared/identity.h index 8973ae8..610988b 100644 --- a/src/shared/identity.h +++ b/src/shared/identity.h @@ -73,23 +73,40 @@ void identity_clear_active(void); /* True when any ownership-affecting identity option is present in the active * snapshot. Ownership stays OFF ("do not apply") for every transfer that - * requests none of them, preserving FastSync's existing behavior. */ + * requests none of them, preserving FastSync's existing behavior. --super / + * --no-super alone does NOT enable ownership; an explicit identity flag + * (--numeric-ids / --chown / --usermap / --groupmap / --copy-as) is required. */ bool identity_active_enabled(void); +/* Pure, config-only predicate: true when the client requested ANY + * client-chosen ownership or super-user activity (--numeric-ids, --chown, + * --usermap/--groupmap, --copy-as, --fake-super, or an explicit --super). Used + * by the daemon module gate to decide whether a module's per-module opt-in is + * required; it never reads the per-connection snapshot. */ +bool identity_ownership_requested(const Config* config); + /* Apply the negotiated ownership to an already-written file descriptor. * source_uid/source_gid are the transmitted numeric ids. Resolution order: * a matching usermap/groupmap rule, then --chown, then --numeric-ids (raw), * then a best-effort name lookup on the receiver's own databases (skipped when * the transmitted id has no name on this system). Only calls fchown() when the - * result differs from the current value; EPERM/EACCES are logged and ignored, - * never fatal (rsync parity: the transfer must not abort). */ -void identity_apply_ownership(int fd, int32_t source_uid, int32_t source_gid); + * result differs from the current value. + * + * Returns false ONLY when an active --copy-as ownership application failed: its + * forced ownership is REQUIRED, so the caller must treat the entry as failed + * rather than reporting success with the wrong owner. For every other identity + * policy an fchown EPERM/EACCES is logged and ignored and true is returned + * (rsync parity: the transfer must not abort). A no-op when no identity policy + * is active returns true. */ +bool identity_apply_ownership(int fd, int32_t source_uid, int32_t source_gid); /* P7 Wave D: the no-follow (symlink) counterpart. Resolves the same - * usermap/groupmap/chown/numeric-ids policy but applies it with + * usermap/groupmap/chown/numeric-ids/copy-as policy but applies it with * fchownat(..., AT_SYMLINK_NOFOLLOW) so a symlink's own ownership is changed - * without ever dereferencing it. A no-op unless an identity flag is active. */ -void identity_apply_ownership_link(int parent_fd, const char* leaf, int32_t source_uid, + * without ever dereferencing it. A no-op unless an identity flag is active. + * The return value follows identity_apply_ownership(): false only when an + * active --copy-as application failed. */ +bool identity_apply_ownership_link(int parent_fd, const char* leaf, int32_t source_uid, int32_t source_gid); /* Receiver-side wire validation of the resolved identity fields. */ diff --git a/src/shared/metadata.c b/src/shared/metadata.c index b5cdc1f..937e324 100644 --- a/src/shared/metadata.c +++ b/src/shared/metadata.c @@ -409,10 +409,14 @@ bool file_restore_metadata_fd(int fd, const FileMetadata* metadata, bool preserv --groupmap / --chown). identity_apply_ownership is the controlled, privilege-gated path: it consults the negotiated policy, resolves the target ids, and applies them via an fd-relative fchown() that is confined - to the just-written file (EPERM/EACCES are logged, never fatal). With no - identity flag set it is a no-op, so a default or plain -M transfer keeps - FastSync's existing behavior of never applying client ownership. */ - identity_apply_ownership(fd, (int32_t)metadata->uid, (int32_t)metadata->gid); + to the just-written file (EPERM/EACCES are logged, never fatal) -- EXCEPT + for an active --copy-as, whose forced ownership is REQUIRED: a failure + marks this entry as failed instead of reporting a wrong-owner write as + success. With no identity flag set it is a no-op, so a default or plain -M + transfer keeps FastSync's existing behavior of never applying client + ownership. */ + if (!identity_apply_ownership(fd, (int32_t)metadata->uid, (int32_t)metadata->gid)) + ok = false; struct timespec times[2] = {{.tv_sec = 0, .tv_nsec = UTIME_OMIT}, {.tv_sec = metadata->mtime_sec, .tv_nsec = metadata->mtime_nsec}}; if (metadata->atime_valid) { diff --git a/src/shared/xattr.c b/src/shared/xattr.c index 591d3e7..d01a38b 100644 --- a/src/shared/xattr.c +++ b/src/shared/xattr.c @@ -340,7 +340,12 @@ void fake_super_store_fd(int fd, uint32_t uid, uint32_t gid, uint32_t mode, int6 * mirroring the normal metadata identity path; other errors are logged) and * still applies mode/mtime where permitted. * - * The OWNER leg additionally honors two policies: + * The OWNER leg additionally honors three policies: + * - an explicit ownership identity policy must be active (numeric-ids / + * chown / usermap / groupmap / copy-as). --fake-super on its own only + * RECORDS the source owner; replaying that owner as a live chown without an + * explicit ownership opt-in would be an un-gated client-chosen-ownership + * primitive. * - --no-super (privilege_super_permitted() false) suppresses it even for a * root receiver, exactly like the normal metadata identity path. * - an active --copy-as is AUTHORITATIVE: the identity path already forced the @@ -370,7 +375,7 @@ bool fake_super_restore_fd(int fd) { not hidden. --no-super suppresses the owner leg even for root, and an active --copy-as is authoritative so its forced owner must not be overwritten by the recorded source owner. */ - if (privilege_super_permitted() && !identity_copy_as_active() && + if (identity_active_enabled() && privilege_super_permitted() && !identity_copy_as_active() && fchown(fd, (uid_t)ul_uid, (gid_t)ul_gid) != 0 && errno != EPERM && errno != EACCES) log_message(LOG_LEVEL_WARNING, "--fake-super: could not restore owner on destination file: %s", strerror(errno)); diff --git a/tests/integration/test_daemon.py b/tests/integration/test_daemon.py index 03c5057..0091c4a 100644 --- a/tests/integration/test_daemon.py +++ b/tests/integration/test_daemon.py @@ -41,6 +41,7 @@ FILES_MODULE = os.path.join(MODULE_ROOT, "files") READONLY_MODULE = os.path.join(MODULE_ROOT, "readonly") AUTH_MODULE = os.path.join(MODULE_ROOT, "auth") TEAM_MODULE = os.path.join(MODULE_ROOT, "team") +OWNER_MODULE = os.path.join(MODULE_ROOT, "owner") CONF_FILE = os.path.join(TEST_DATA_DIR, "fastsyncd.conf") CRED_FILE = os.path.join(TEST_DATA_DIR, "fastsyncd.passwd") STARTFAIL_CONF = os.path.join(TEST_DATA_DIR, "fastsyncd_startfail.conf") @@ -149,7 +150,8 @@ def _config_port(config_path): @pytest.fixture(scope="module", autouse=True) def daemon_env(): - for d in (MODULE_ROOT, FILES_MODULE, READONLY_MODULE, AUTH_MODULE, TEAM_MODULE, DETACH_MODULE): + for d in (MODULE_ROOT, FILES_MODULE, READONLY_MODULE, AUTH_MODULE, TEAM_MODULE, OWNER_MODULE, + DETACH_MODULE): shutil.rmtree(d, ignore_errors=True) os.makedirs(d, exist_ok=True) generate_test_files(SOURCE_DIR, full=False) @@ -184,7 +186,11 @@ def daemon_env(): "[team]\n" "path = %s\n" "auth users = alice,bob\n" - % (config_port, FILES_MODULE, READONLY_MODULE, AUTH_MODULE, TEAM_MODULE)) + "\n" + "[owner]\n" + "path = %s\n" + "client owner = yes\n" + % (config_port, FILES_MODULE, READONLY_MODULE, AUTH_MODULE, TEAM_MODULE, OWNER_MODULE)) # A dedicated config for the fail-closed startup check: an auth-required # module with no credential store must refuse to start. Its own free port @@ -320,49 +326,62 @@ class TestDaemonRejection: assert result.returncode != 0 assert _tree_file_count(AUTH_MODULE) == 0 - def test_copy_as_refused_by_daemon(self, daemon): - """P7 Wave E: a daemon refuses client-chosen ownership (--copy-as) - outright. There is no per-module opt-in, so even a root daemon must not - honor an arbitrary client-selected owner. 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=["--copy-as=@65534:@65534"]) - assert result.returncode != 0, "the daemon must refuse --copy-as" - assert self._tree_files() == before_files, \ - "--copy-as 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 "copy-as is refused by the daemon" in tail, ( - 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, + def _assert_ownership_refused(self, daemon, module, flags): + """A daemon module without `client owner = yes` refuses every + client-chosen ownership / super-user request 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" + result, _ = run_client(SOURCE_DIR, f"127.0.0.1::{module}", port=daemon.port, flags=flags) + assert result.returncode != 0, f"the daemon must refuse {flags}" assert self._tree_files() == before_files, \ - "--super refusal wrote under the module root" + f"{flags} 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}" + assert "client-chosen ownership" in tail, ( + f"daemon did not log the ownership refusal: {tail[-400:]!r}" ) + def test_copy_as_refused_by_daemon(self, daemon): + """P7 Wave E hardening: a daemon refuses client-chosen ownership + (--copy-as) outright unless the module opts in with `client owner = yes`, + so even a root daemon must not honor an arbitrary client-selected owner + by default. The refusal happens at the config handshake, before any data + lands.""" + self._assert_ownership_refused(daemon, "files", ["--copy-as=@65534:@65534"]) + + def test_super_refused_by_daemon(self, daemon): + """An explicit --super is a super-user activity request, so a daemon + module refuses it unless it opts in with `client owner = yes`. The + refusal happens at the config handshake, before any data lands.""" + self._assert_ownership_refused(daemon, "files", ["--super", "--preserve"]) + + def test_numeric_ids_refused_by_daemon(self, daemon): + """P7 Wave E hardening (A1): the daemon ownership gate must cover the + pre-existing identity flags too, not only --copy-as/--super. A module + without `client owner = yes` refuses --numeric-ids at the handshake.""" + self._assert_ownership_refused(daemon, "files", ["--numeric-ids", "--preserve"]) + + def test_chown_refused_by_daemon(self, daemon): + """--chown is client-chosen ownership too and must be refused by a + non-opted-in module.""" + self._assert_ownership_refused(daemon, "files", ["--chown=@65534:@65534", "--preserve"]) + + def test_owner_opt_in_allows_numeric_ids(self, daemon): + """A module that opts in with `client owner = yes` accepts the + client-chosen ownership flags (here --numeric-ids); the transfer + succeeds and lands inside that module root.""" + result, _ = run_client(SOURCE_DIR, "127.0.0.1::owner", port=daemon.port, + flags=["--numeric-ids", "--preserve"]) + assert result.returncode == 0, result.stderr or result.stdout + received = get_dest_received_dir(OWNER_MODULE, SOURCE_DIR) + mismatches, missing = verify_transfer(SOURCE_DIR, received) + assert not missing, f"missing: {missing[:5]}" + assert not mismatches, f"mismatch: {mismatches[:5]}" + @pytest.mark.daemon_detach def test_real_detach_path(self): """--daemon WITHOUT --no-detach double-forks a real background daemon; diff --git a/tests/integration/test_features.py b/tests/integration/test_features.py index 26c1855..bdbec46 100644 --- a/tests/integration/test_features.py +++ b/tests/integration/test_features.py @@ -4165,11 +4165,11 @@ class TestSuperPrivilege: f"--no-super must not apply ownership (uid={st.st_uid} gid={st.st_gid})" @pytest.mark.skipif(os.geteuid() != 0, reason="only root can change ownership") - def test_super_applies_ownership_as_root(self, shared_server): - """Control/proof the flag is not inert for root: --super with no explicit - identity policy treats ownership as raw numeric ids (as --numeric-ids), - applying the very ownership --no-super suppressed.""" - source, dest = self._seed("super") + def test_super_alone_does_not_apply_ownership_as_root(self, shared_server): + """A3: --super no longer implies --numeric-ids, so --super alone must NOT + apply client-chosen ownership even for root; the destination keeps the + receiver's owner (the exact ownership --no-super would also suppress).""" + source, dest = self._seed("superonly") os.chown(os.path.join(source, "f.txt"), 12345, 12346) result, _ = run_client(source, dest, flags=["--preserve", "--super"], @@ -4178,8 +4178,25 @@ class TestSuperPrivilege: f"exit {result.returncode}: {(result.stderr or '')[:300]}" received = get_dest_received_dir(dest, source) st = os.stat(os.path.join(received, "f.txt")) + assert (st.st_uid, st.st_gid) != (12345, 12346), \ + f"--super alone must not apply ownership (uid={st.st_uid} gid={st.st_gid})" + + @pytest.mark.skipif(os.geteuid() != 0, reason="only root can change ownership") + def test_super_with_numeric_ids_applies_ownership_as_root(self, shared_server): + """Control: an explicit identity policy is what enables ownership, so + --numeric-ids --super still applies the raw ids as root (the very + ownership --no-super suppresses).""" + source, dest = self._seed("supernumeric") + os.chown(os.path.join(source, "f.txt"), 12345, 12346) + result, _ = run_client(source, dest, + flags=["--preserve", "--numeric-ids", "--super"], + port=shared_server.port) + assert result.returncode == 0, \ + f"exit {result.returncode}: {(result.stderr or '')[:300]}" + received = get_dest_received_dir(dest, source) + st = os.stat(os.path.join(received, "f.txt")) assert (st.st_uid, st.st_gid) == (12345, 12346), \ - f"--super should apply raw ids: uid={st.st_uid} gid={st.st_gid}" + f"--numeric-ids --super should apply raw ids: uid={st.st_uid} gid={st.st_gid}" @pytest.mark.ci @pytest.mark.skipif(os.geteuid() != 0, reason="only root can change ownership") diff --git a/tests/test_config.c b/tests/test_config.c index efb4c40..77a7ba0 100644 --- a/tests/test_config.c +++ b/tests/test_config.c @@ -1899,6 +1899,68 @@ static void test_privilege_super_permitted_modes() { EXPECT_TRUE(privilege_super_permitted()); } +/* P7 Wave E hardening (A1): identity_ownership_requested() is the pure, + config-only predicate the daemon module gate uses. It must fire for every + client-chosen ownership / super-user request and stay false for a plain + transfer and for SUPER_MODE_AUTO (the default) alone. */ +static void test_identity_ownership_requested() { + EXPECT_FALSE(identity_ownership_requested(NULL)); + + Config* c = config_create(); + EXPECT_NOT_NULL(c); + EXPECT_FALSE(identity_ownership_requested(c)); + c->super_mode = SUPER_MODE_AUTO; + EXPECT_FALSE(identity_ownership_requested(c)); /* AUTO alone is not ownership */ + c->super_mode = SUPER_MODE_ON; + EXPECT_TRUE(identity_ownership_requested(c)); /* explicit --super is */ + c->super_mode = SUPER_MODE_AUTO; + + c->numeric_ids = true; + EXPECT_TRUE(identity_ownership_requested(c)); + c->numeric_ids = false; + c->chown_uid_set = true; + EXPECT_TRUE(identity_ownership_requested(c)); + c->chown_uid_set = false; + c->chown_gid_set = true; + EXPECT_TRUE(identity_ownership_requested(c)); + c->chown_gid_set = false; + c->copy_as_set = true; + EXPECT_TRUE(identity_ownership_requested(c)); + c->copy_as_set = false; + c->fake_super = true; + EXPECT_TRUE(identity_ownership_requested(c)); + config_delete(c); + + Config* um = config_create(); + EXPECT_NOT_NULL(um); + EXPECT_EQ_INT(identity_parse_map(um, "@1:@2", false), 0); + EXPECT_TRUE(identity_ownership_requested(um)); + config_delete(um); + + Config* gm = config_create(); + EXPECT_NOT_NULL(gm); + EXPECT_EQ_INT(identity_parse_map(gm, "@1:@2", true), 0); + EXPECT_TRUE(identity_ownership_requested(gm)); + config_delete(gm); +} + +/* P7 Wave E hardening (A3): --super no longer implies raw numeric-id + preservation, so it must never enable ownership application on its own; an + explicit identity flag is required. */ +static void test_super_does_not_imply_numeric() { + Config* c = config_create(); + EXPECT_NOT_NULL(c); + c->super_mode = SUPER_MODE_ON; + c->use_metadata = true; + identity_set_active(c); + EXPECT_FALSE(identity_active_enabled()); + c->numeric_ids = true; + identity_set_active(c); + EXPECT_TRUE(identity_active_enabled()); + identity_clear_active(); + config_delete(c); +} + void test_config() { test_config_lifecycle(); test_config_ssh_dest(); @@ -1953,6 +2015,8 @@ void test_config() { test_config_receive_with_validate_rejects(); } test_identity_copy_as_refused(); + test_identity_ownership_requested(); + test_super_does_not_imply_numeric(); test_privilege_super_permitted_modes(); test_config_delete_timing_early_helper(); test_config_is_remote_dest(); diff --git a/tests/test_daemon_conf.c b/tests/test_daemon_conf.c index c6a7211..9580f8b 100644 --- a/tests/test_daemon_conf.c +++ b/tests/test_daemon_conf.c @@ -43,6 +43,7 @@ static void test_daemon_conf_full_parse() { "[backup]\n" "path = /srv/backup\n" "read only = yes\n" + "client owner = yes\n" "auth users = alice, bob\n", &path), 0); @@ -57,6 +58,7 @@ static void test_daemon_conf_full_parse() { EXPECT_EQ_STR(conf->modules[0].name, "backup"); EXPECT_EQ_STR(conf->modules[0].path, "/srv/backup"); EXPECT_TRUE(conf->modules[0].read_only); + EXPECT_TRUE(conf->modules[0].client_owner); EXPECT_EQ_INT(conf->modules[0].auth_user_count, 2); EXPECT_EQ_STR(conf->modules[0].auth_users[0], "alice"); EXPECT_EQ_STR(conf->modules[0].auth_users[1], "bob"); @@ -84,6 +86,10 @@ static void test_daemon_conf_comments_and_blank_lines() { EXPECT_EQ_INT(conf->module_count, 2); EXPECT_EQ_STR(conf->modules[0].name, "alpha"); EXPECT_EQ_STR(conf->modules[1].name, "beta"); + /* `client owner` defaults to off: a module must opt in to client-chosen + ownership. */ + EXPECT_FALSE(conf->modules[0].client_owner); + EXPECT_FALSE(conf->modules[1].client_owner); daemon_conf_free(conf); } @@ -207,6 +213,12 @@ static void test_daemon_conf_malformed_rejected() { EXPECT_NULL(conf); EXPECT_TRUE(strstr(err, "read only") != NULL); + EXPECT_EQ_INT(write_conf("[m]\npath = /x\nclient owner = maybe\n", &path), 0); + conf = daemon_conf_load(path, err, sizeof(err)); + free(path); + EXPECT_NULL(conf); + EXPECT_TRUE(strstr(err, "client owner") != NULL); + EXPECT_EQ_INT(write_conf("= value\n", &path), 0); conf = daemon_conf_load(path, err, sizeof(err)); free(path); diff --git a/tests/test_xattr.c b/tests/test_xattr.c index 24b9e40..c5e4316 100644 --- a/tests/test_xattr.c +++ b/tests/test_xattr.c @@ -305,6 +305,10 @@ static void test_fake_super_owner_gate() { Config* c = config_create(); EXPECT_NOT_NULL(c); + /* An explicit ownership policy is required before fake-super replay may + chown; --fake-super alone only records the source owner (A2). */ + c->numeric_ids = true; + /* --no-super: the owner leg is skipped even as root. */ c->super_mode = SUPER_MODE_OFF; identity_set_active(c); @@ -314,7 +318,7 @@ static void test_fake_super_owner_gate() { EXPECT_EQ_INT((int)st.st_uid, 0); EXPECT_EQ_INT((int)st.st_gid, 0); - /* AUTO: the recorded source owner is applied. */ + /* AUTO with an identity policy: the recorded source owner is applied. */ c->super_mode = SUPER_MODE_AUTO; identity_set_active(c); EXPECT_TRUE(fake_super_restore_fd(fd)); @@ -322,10 +326,19 @@ static void test_fake_super_owner_gate() { EXPECT_EQ_INT((int)st.st_uid, 12345); EXPECT_EQ_INT((int)st.st_gid, 12346); + /* --super / --fake-super with NO explicit identity flag must NOT apply a + client-chosen owner: super_mode alone never enables ownership. */ + EXPECT_EQ_INT(fchown(fd, 0, 0), 0); + c->numeric_ids = false; + c->super_mode = SUPER_MODE_ON; + identity_set_active(c); + EXPECT_TRUE(fake_super_restore_fd(fd)); + EXPECT_EQ_INT(fstat(fd, &st), 0); + EXPECT_EQ_INT((int)st.st_uid, 0); + EXPECT_EQ_INT((int)st.st_gid, 0); + /* Active --copy-as is authoritative: the recorded source owner must not override it, even with AUTO/ON. */ - EXPECT_EQ_INT(fchown(fd, 0, 0), 0); - c->super_mode = SUPER_MODE_ON; c->copy_as_set = true; c->copy_as_uid = 777; c->copy_as_gid = 778; From b216ed31fb6d3976acd1512dc711004974a14832 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sat, 12 Sep 2026 14:18:03 +0200 Subject: [PATCH 2/3] fix(p8-security): close review gaps in the ownership gate and copy-as failure propagation - H3: a daemon module without 'client owner = yes' now also has super-user device activity forced off (char/block mknod, --write-devices), so a root daemon can no longer be made to create/write raw devices under AUTO. The entries are skipped, preserving ordinary -a pushes. - H1/H2: propagate a failed required --copy-as chown from symlink metadata restore and implicitly-created parent directories, so the entry (and run) reports failure instead of a wrong-owner success. - Docs/help/headers updated for A2/A3 and the device clamp; startup warning spells out the client-owner risk. - Tests: daemon device clamp (skipped without opt-in, created with opt-in), updated --super/--fake-super expectations. --- RSYNC_COMPAT.md | 6 ++-- src/client/usage.c | 10 +++--- src/server/server.c | 32 +++++++++++++------ src/shared/file.c | 12 +++++-- src/shared/file_receive.c | 2 +- src/shared/identity.c | 4 +-- src/shared/identity.h | 9 +++--- src/shared/metadata.c | 14 ++++++--- src/shared/metadata.h | 6 ++-- src/shared/xattr.h | 12 ++++--- tests/integration/test_daemon.py | 54 ++++++++++++++++++++++++++++++++ 11 files changed, 123 insertions(+), 38 deletions(-) diff --git a/RSYNC_COMPAT.md b/RSYNC_COMPAT.md index fd53e9d..e8c19ef 100644 --- a/RSYNC_COMPAT.md +++ b/RSYNC_COMPAT.md @@ -264,7 +264,7 @@ why plain `--append` works on the normal atomic path, not only with `--inplace`. | `--usermap=STRING` | Map usernames | ✅ Implemented | Opt-in ownership application. rsync subset implemented: comma-separated `FROM:TO` rules evaluated in order, first match wins; `FROM`/`TO` are group/user names (resolved on the SOURCE machine at parse time), `*` (FROM matches any id / TO = the receiving process's current euid), and an `@N` or bare `N` numeric id. Rules are carried over the wire as resolved numeric id pairs; the receiver applies a matching rule (else falls back to `--chown`, `--numeric-ids`, then a best-effort name lookup) via an fd-relative `fchown`. Malformed/unresolvable specs are rejected with a clear error, never a silent no-op. Implies metadata preservation so the source uid/gid travel. Only effective when the receiver can actually change ownership (root or membership); otherwise it warns and continues | | `--groupmap=STRING` | Map group names | ✅ Implemented | Same rsync subset and semantics as `--usermap` but for the group (gid) side and the group databases. See the Phase-4 identity notes | | `--chown=USER:GROUP` | Map owner and group | ✅ Implemented | Opt-in ownership override applied receiver-side. Forms: `USER:GROUP`, `USER` (owner only), `:GROUP` (group only); a `*` for USER/GROUP means the current/root user or group as appropriate; an `@N`/bare `N` numeric id is accepted. A `:` inside a name may be escaped as `\:`. Equivalent to a trailing `*:*` usermap+groupmap rule (so an explicit `--usermap`/`--groupmap` match wins). Malformed or unresolvable specs are clear parse errors. Implies metadata preservation. Only effective when the receiver has permission to chown; otherwise it warns and continues (rsync parity) | -| `--copy-as=USER[:GROUP]` | Perform the copy as another user/group | ✅ Implemented | Safe-subset implementation, an explicit divergence from rsync's **real identity switching**. rsync makes the receiving process actually assume USER/GROUP (setuid/setgid); FastSync's receiver is multithreaded, so a real credential drop would be unsafe and is never attempted — FastSync never calls `setuid`/`seteuid`/`setgid`. Instead the receiver FORCES the ownership of every entry it writes to `copy_as_uid`/`copy_as_gid` through the existing confined, fd-relative identity path (the same `fchown`/`fchownat` mechanism as `--chown`/`--usermap`/`--groupmap`; symlinks use `fchownat(..., AT_SYMLINK_NOFOLLOW)`, and directories — including intermediate parents created implicitly while writing a nested file — and char/block/FIFO nodes are owned no-follow too, so a directory never keeps the receiver's owner while its children get the target owner), with `--copy-as` at the **highest priority** — it beats usermap/groupmap/`--chown`/`--numeric-ids` and the best-effort name lookup. This REQUIRES a privileged (root) receiver: an unprivileged receiver REFUSES the whole transfer up front at the config handshake (`server_module_gate`, running inside `config_receive_with_validate` before the `STATUS_OK` ack) with a clear error and no file data exchanged — never a silent wrong-ownership result. A server running with an operator `--no-super` veto also refuses it, and a **daemon** refuses `--copy-as`, like every other client-chosen-ownership request (`--numeric-ids`/`--chown`/`--usermap`/`--groupmap`/`--fake-super`/explicit `--super`), unless the selected module opts in with `client owner = yes`; without that per-module opt-in a daemon must not honor an arbitrary client-selected owner (the standalone listener and SSH `--stdio` server keep honoring these for their single operator-authorized root). `--fake-super` interaction: `--copy-as` is authoritative, so the recorded source owner is never replayed over the forced target owner. If the ownership apply still fails with EPERM/EACCES (capability-restricted root, root-squash, read-only mount) the failure is logged at ERROR and the **entry is reported as failed** rather than written with the wrong owner (the receiver never claims a `--copy-as` success it did not achieve), while a single entry failure does not abort the multithreaded run. USER is resolved on the client against the user database (a name, an `@N`/bare `N` numeric id, or `*` meaning the client's current euid); when `:GROUP` is present it is resolved against the group database (`*` meaning the client's egid). **Group-default rule:** when the group is omitted FastSync uses the user's primary gid (`getpwuid(uid)->pw_gid`); a numeric id with no local passwd entry has no primary gid to look up, so `gid` falls back to `uid` (documented divergence). Malformed/empty/unresolvable specs are clear parse errors, never a silent no-op. Never elevates privileges and never bypasses the confined receive root. Implies metadata preservation (the source uid/gid must be transmitted). Wire: a new trailing config-frame block **sent after** the `--super` int (presence int, then the two int32 ids, both validated `>= 0` on receive; the ids are also rejected if they do not fit int32 at CLI parse time); `PROTOCOL_VERSION` bumped **2.17.0 → 2.18.0** | +| `--copy-as=USER[:GROUP]` | Perform the copy as another user/group | ✅ Implemented | Safe-subset implementation, an explicit divergence from rsync's **real identity switching**. rsync makes the receiving process actually assume USER/GROUP (setuid/setgid); FastSync's receiver is multithreaded, so a real credential drop would be unsafe and is never attempted — FastSync never calls `setuid`/`seteuid`/`setgid`. Instead the receiver FORCES the ownership of every entry it writes to `copy_as_uid`/`copy_as_gid` through the existing confined, fd-relative identity path (the same `fchown`/`fchownat` mechanism as `--chown`/`--usermap`/`--groupmap`; symlinks use `fchownat(..., AT_SYMLINK_NOFOLLOW)`, and directories — including intermediate parents created implicitly while writing a nested file — and char/block/FIFO nodes are owned no-follow too, so a directory never keeps the receiver's owner while its children get the target owner), with `--copy-as` at the **highest priority** — it beats usermap/groupmap/`--chown`/`--numeric-ids` and the best-effort name lookup. This REQUIRES a privileged (root) receiver: an unprivileged receiver REFUSES the whole transfer up front at the config handshake (`server_module_gate`, running inside `config_receive_with_validate` before the `STATUS_OK` ack) with a clear error and no file data exchanged — never a silent wrong-ownership result. A server running with an operator `--no-super` veto also refuses it, and a **daemon** refuses `--copy-as`, like every other client-chosen-ownership request (`--numeric-ids`/`--chown`/`--usermap`/`--groupmap`/`--fake-super`/explicit `--super`), unless the selected module opts in with `client owner = yes`; without that per-module opt-in a daemon must not honor an arbitrary client-selected owner (the standalone listener and SSH `--stdio` server keep honoring these for their single operator-authorized root). `--fake-super` interaction: `--copy-as` is authoritative, so the recorded source owner is never replayed over the forced target owner. If the ownership apply still fails with EPERM/EACCES (capability-restricted root, root-squash, read-only mount) the failure is logged at ERROR and the **entry is reported as failed** rather than written with the wrong owner, which fails the transfer (fail-fast) so overall success is never reported with the wrong owner. USER is resolved on the client against the user database (a name, an `@N`/bare `N` numeric id, or `*` meaning the client's current euid); when `:GROUP` is present it is resolved against the group database (`*` meaning the client's egid). **Group-default rule:** when the group is omitted FastSync uses the user's primary gid (`getpwuid(uid)->pw_gid`); a numeric id with no local passwd entry has no primary gid to look up, so `gid` falls back to `uid` (documented divergence). Malformed/empty/unresolvable specs are clear parse errors, never a silent no-op. Never elevates privileges and never bypasses the confined receive root. Implies metadata preservation (the source uid/gid must be transmitted). Wire: a new trailing config-frame block **sent after** the `--super` int (presence int, then the two int32 ids, both validated `>= 0` on receive; the ids are also rejected if they do not fit int32 at CLI parse time); `PROTOCOL_VERSION` bumped **2.17.0 → 2.18.0** | **Phase-4 metadata-time notes:** `-U/--atimes`, `-N/--crtimes`, `-O/--omit-dir-times`, `-J/--omit-link-times`, and `--open-noatime` are new. @@ -636,7 +636,7 @@ now transmits targets (the prior behavior was broken/partial); its status moved - **Config grammar** (`fastsyncd.conf`): line-based; an implicit global section first, then `[module]` sections. Keys are case-insensitive, values are trimmed and may be wrapped in one layer of double quotes (`path = "/srv/my dir"`). `#` and `;` at the start of a line (after leading whitespace) are full-line comments; inline comments and `\` continuations are not supported. Lines are bounded (4096 chars). Global keys: `port` (default 873), `motd file` (the daemon sends its bounded, escaped content to a client after the module gate/auth accepts, unless the client passes `--no-motd`), `address` (optional bind address). Module keys: `path` (required; the daemon-side authorized root for that module), `read only` (yes/no/true/false/1/0, default no), `client owner` (yes/no/true/false/1/0, default no; opts the module into client-chosen ownership — see below), `auth users` (comma list). **Unknown keys and malformed lines are parse-and-reject errors** (never silently ignored), so a typo cannot change what a module serves. - **Module selection & confinement:** the client requests a module with an rsync-style `host::module[/path]` destination. The module name crosses the wire as a trailing string on the config frame (bumping `PROTOCOL_VERSION` 2.14.0 → 2.15.0; the bump is required because the config-frame layout changed and the strict same-version handshake is what prevents a peer from desynchronizing on the new trailing field). The daemon looks the module up in ITS OWN config and uses the module's `path` as the authorized root through the exact same `configure_authorization` confinement the standalone server applies to `--destination-root` (`file_open_secure_parent`, `has_path_traversal`, `path_is_within`); the client never supplies the root, every client-chosen-ownership/super-user request is refused unless the module declares `client owner = yes` (the daemon's per-module opt-in, see below), and the operator `--no-super` veto forces super-user activities off for every daemon connection. The client's `/path` part is relative inside the module and is rejected if absolute or if it contains `..`. Unknown modules are refused before any data moves (the run fails cleanly at the config handshake). An absolute destination and a module request against a non-daemon server are also refused. -- **`client owner` (client-chosen-ownership opt-in):** by default a daemon module refuses every request that would let the client pick an owner or ask for super-user activities — `--numeric-ids`, `--chown`, `--usermap`/`--groupmap`, `--fake-super`, `--copy-as`, and an explicit `--super` — at the config handshake (before `STATUS_OK`), because a daemon has no per-module opt-in for client-chosen ownership and any anonymous client could otherwise force arbitrary owner ids inside the module root. `client owner = yes` opts a single module in, allowing those requests within that module's root (the standalone listener and the SSH `--stdio` server always honor them for their single operator-authorized root). The opt-in does **not** lift the privilege requirement: `--copy-as` still needs a root receiver, and the operator `--no-super` veto still forces super-user activities off for every connection. The daemon logs a prominent startup warning for each `client owner = yes` module so the operator's deliberate choice is visible. +- **`client owner` (client-chosen-ownership opt-in):** by default a daemon module refuses every request that would let the client pick an owner or ask for super-user activities — `--numeric-ids`, `--chown`, `--usermap`/`--groupmap`, `--fake-super`, `--copy-as`, and an explicit `--super` — at the config handshake (before `STATUS_OK`), because a daemon has no per-module opt-in for client-chosen ownership and any anonymous client could otherwise force arbitrary owner ids inside the module root. `client owner = yes` opts a single module in, allowing those requests within that module's root (the standalone listener and the SSH `--stdio` server always honor them for their single operator-authorized root). Without the opt-in the daemon also forces super-user **device** activity off for that connection — char/block device-node creation (`--devices`) and `--write-devices` — even under the default `AUTO` mode, so a non-opted module can never be made to `mknod` or write a raw device; those entries are skipped (not refused) so an ordinary `-a` push still succeeds without device nodes. The opt-in does **not** lift the privilege requirement: `--copy-as` still needs a root receiver, and the operator `--no-super` veto still forces super-user activities off for every connection. The daemon logs a prominent startup warning for each `client owner = yes` module so the operator's deliberate choice is visible. - **`read only` safe default:** every network transfer FastSync currently supports is a push that writes under the module root, so a `read only` module refuses the connection (clear server log "module is read only"; the client exits non-zero, nothing is transferred). A future pull/list operation can be opened up when it exists; the knob is already stored. - **`auth users` (Wave B password authentication):** a module that declares `auth users` requires the client to present credentials. The client sends a username + the lowercase hex SHA-256 of the password (never the literal password) in the config frame; the daemon accepts a connection only when the presented username is **on the module's `auth users` list** AND the presented digest matches that user's credential-store entry. Verification is constant-time (username present/absent both take the same comparison work, so there is no timing oracle distinguishing "unknown user" from "wrong password"), and the daemon logs the username but **never the digest or the password**. A module WITHOUT `auth users` stays open (legitimate rsync configuration); credentials sent to such a module are ignored. Read-only is orthogonal: even a correctly authenticated push to a `read only` module is still refused (all FastSync network transfers write). Fail-closed policy: a daemon whose config declares `auth users` on any module refuses to start unless a credential store was given (`--password-file` and/or `--early-input`); a missing or empty store is never silently treated as "open". - **Credential store format:** server `--password-file`/`--early-input` files are line-based `user:SHA256HEX`, one per line, where `SHA256HEX` is the lowercase hex SHA-256 of the user's password (exactly what the client transmits). Blank lines and lines starting with `#`/`;` are comments; the parser is strict (a malformed line fails the whole load, so a typo can never let a different set of users in). The client `--password-file` holds `user:password` on its first meaningful line (the literal password, hashed client-side then wiped from memory); keep both files readable only by their owner (mode 0600) since the client file holds the password and the server file holds the equivalent credential. Per-username wire length is bounded (256 chars) and digests are validated to be exactly 64 lowercase hex on receive. @@ -829,7 +829,7 @@ These are the last compatibility items and the closing phase toward rsync flag p **Wave E (LAST) — Privilege: `--super`/`--no-super` and `--copy-as=USER[:GROUP]` (✅ implemented).** FastSync adopts a **safe-subset + clear-refusal** privilege model: it never blind-elevates and never calls `setuid`/`seteuid`/`setgid`. All privileged operations remain fd-relative and confined below the authorized receive root. -`--super`/`--no-super` set a receiver-side tri-state `Config->super_mode` (`SUPER_MODE_AUTO`/`ON`/`OFF`). `privilege_super_permitted()` / `privilege_super_mode_permitted()` (src/shared/identity.c) return true for `ON` and `AUTO` (AUTO preserves FastSync's historical best-effort attempt, where the kernel refuses an unprivileged call and the caller skips it) and false only for `OFF`. The gate covers every super-user activity FastSync performs: ownership application (`identity_apply_ownership`/`_link`), char/block device-node creation (`file_save_special_to_disk`), writes into an existing device (`--write-devices`), and the `--fake-super` owner replay. Unprivileged FIFO creation is deliberately unaffected. With `ON` and no explicit identity policy, ownership falls back to raw numeric-id preservation (as `--numeric-ids`); explicit `--usermap`/`--groupmap`/`--chown`/`--numeric-ids` still win. `--no-super` suppresses those activities even for a root receiver. A non-root receiver given `--super` logs one warning at activation (`identity_set_active`); each confined attempt is then refused by the kernel and skipped, never aborting. The confinement floor is unchanged (`file_open_secure_parent`, `O_NOFOLLOW`, root/path checks). Operator control: the server CLI accepts `--no-super`, a veto that forces `OFF` for every connection and refuses client `--copy-as`/`--super`. +`--super`/`--no-super` set a receiver-side tri-state `Config->super_mode` (`SUPER_MODE_AUTO`/`ON`/`OFF`). `privilege_super_permitted()` / `privilege_super_mode_permitted()` (src/shared/identity.c) return true for `ON` and `AUTO` (AUTO preserves FastSync's historical best-effort attempt, where the kernel refuses an unprivileged call and the caller skips it) and false only for `OFF`. The gate covers every super-user activity FastSync performs: ownership application (`identity_apply_ownership`/`_link`), char/block device-node creation (`file_save_special_to_disk`), writes into an existing device (`--write-devices`), and the `--fake-super` owner replay. Unprivileged FIFO creation is deliberately unaffected. `--super` does **not** imply `--numeric-ids`: ownership is applied only when an explicit identity policy (`--usermap`/`--groupmap`/`--chown`/`--numeric-ids`/`--copy-as`) is also given. `--no-super` suppresses those activities even for a root receiver. A non-root receiver given `--super` logs one warning at activation (`identity_set_active`); each confined attempt is then refused by the kernel and skipped, never aborting. The confinement floor is unchanged (`file_open_secure_parent`, `O_NOFOLLOW`, root/path checks). Operator control: the server CLI accepts `--no-super`, a veto that forces `OFF` for every connection, refuses any client `--copy-as`, and neutralizes an explicit `--super` (the connection is accepted but no super-user activity is attempted). On a daemon, a module that has not opted in with `client owner = yes` additionally has super-user device activity forced off (see the Daemon Mode notes). `--copy-as=USER[:GROUP]` is the safe subset. FastSync's receiver is multithreaded, so a real credential switch is unsafe; instead the receiver forces the ownership of **every entry it writes** — regular files, symlinks, directories (including implicitly-created parents), and special nodes — to the resolved target ids through the confined fd-relative identity path. USER is resolved on the client (name, `@N`/bare N, or `*` = client euid); when `:GROUP` is omitted the user's primary gid is used (falling back to `gid == uid` for a numeric id with no local passwd entry). It requires a privileged (root) receiver: an unprivileged receiver refuses the whole transfer at the config handshake, before `STATUS_OK`, so no data is ever written with the wrong ownership. A `--copy-as` chown failure on a capability-restricted root is logged at ERROR (never silently downgraded). `--copy-as` implies metadata (`--no-preserve` is rejected) and `--fake-super` cannot override it. Daemon policy: a `--daemon` receiver refuses **every** client-chosen-ownership / super-user request — `--numeric-ids`, `--chown`, `--usermap`/`--groupmap`, `--fake-super`, `--copy-as`, and explicit `--super` — unless the selected module opts in with `client owner = yes`; without that per-module opt-in any client could force arbitrary ownership inside the module root (the standalone listener and the SSH-launched `--stdio` server, which each serve one operator-authorized root, honor these requests). A `--copy-as` chown failure on a capability-restricted root marks the entry as failed rather than reporting success with the wrong owner. diff --git a/src/client/usage.c b/src/client/usage.c index 12c644c..9d793d3 100644 --- a/src/client/usage.c +++ b/src/client/usage.c @@ -169,11 +169,11 @@ void print_usage(void) { printf(" re-apply it (fd-relative) on a privileged run; the\n"); printf(" recording format diverges from rsync's user.rsync.%%stat%%\n"); printf(" --super Permit the receiver to attempt super-user activities\n"); - printf(" (ownership application, char/block device-node\n"); - printf(" creation) within the confined receive root. Never\n"); - printf(" elevates privileges and never bypasses confinement;\n"); - printf(" with no explicit identity policy, ownership follows\n"); - printf(" raw numeric ids (as if --numeric-ids)\n"); + printf(" (char/block device-node creation, --write-devices)\n"); + printf(" within the confined receive root. Never elevates\n"); + printf(" privileges and never bypasses confinement; ownership\n"); + printf(" is still applied only with an explicit identity flag\n"); + printf(" (--numeric-ids/--chown/--usermap/--groupmap)\n"); printf(" --no-super Forbid those super-user activities even when the\n"); printf(" receiver is running as root\n"); printf(" --chmod Modify transferred permissions (rsync syntax)\n"); diff --git a/src/server/server.c b/src/server/server.c index a29dfc4..f68615e 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -246,12 +246,25 @@ static const char* server_module_gate(const Config* config, void* context) { client could force arbitrary ownership inside the module root. The standalone/SSH server has a single operator-authorized root and keeps honoring these. */ - if (!module->client_owner && identity_ownership_requested(effective)) { - log_message(LOG_LEVEL_ERROR, - "daemon module '%s' refuses client-chosen ownership/super-user activities " - "(no `client owner = yes` opt-in); refusing", - config->module); - return "client-chosen ownership is not permitted by this daemon module"; + if (!module->client_owner) { + /* Ownership: refuse the whole transfer up front (a clear failure). Uses the + original config so an explicit --super is caught even though super_mode is + clamped to OFF below. */ + if (identity_ownership_requested(config)) { + log_message(LOG_LEVEL_ERROR, + "daemon module '%s' refuses client-chosen ownership/super-user activities " + "(no `client owner = yes` opt-in); refusing", + config->module); + return "client-chosen ownership is not permitted by this daemon module"; + } + /* Super-user DEVICE activities (char/block mknod and --write-devices) are + permitted under the default AUTO mode, so without this clamp a root daemon + would still let a non-opted module create arbitrary device nodes and write + raw devices. Force them off for this connection: those entries are + skipped (never mknod'ed) while an ordinary `-a` push still succeeds + without device nodes, matching the operator's least-privilege choice. + The operator-level --no-super veto is already folded into this. */ + effective->super_mode = SUPER_MODE_OFF; } if (module->auth_user_count > 0) { /* Auth-required module (Wave B): verify the presented credentials against @@ -769,9 +782,10 @@ int main(int argc, char* argv[]) { for (int i = 0; i < g_daemon_conf->module_count; i++) { if (g_daemon_conf->modules[i].client_owner) log_message(LOG_LEVEL_WARNING, - "daemon module '%s' allows client-chosen ownership " - "(`client owner = yes`); clients may request arbitrary owner ids within " - "that module root", + "daemon module '%s' allows client-chosen ownership and super-user device " + "activities (`client owner = yes`); clients may request arbitrary owner ids " + "and device nodes within that module root -- pair it with `auth users` " + "unless the module is intentionally open to the network", g_daemon_conf->modules[i].name); } /* Daemon credential store (Wave B). --password-file and --early-input diff --git a/src/shared/file.c b/src/shared/file.c index f402763..c6bc4ef 100644 --- a/src/shared/file.c +++ b/src/shared/file.c @@ -582,8 +582,16 @@ int file_open_secure_parent(const char* path, char** leaf_out, bool create_dirs) (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); + if (created && identity_copy_as_active() && + !identity_apply_ownership_link(fd, component, 0, 0)) { + /* A REQUIRED --copy-as ownership that cannot be applied to a + directory this walk just created must fail the entry rather than + leave that implicit parent owned by the receiver. */ + close(fd); + free(copy); + free(leaf); + return -1; + } next = openat(fd, component, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); } } diff --git a/src/shared/file_receive.c b/src/shared/file_receive.c index 5d23361..9ce4d90 100644 --- a/src/shared/file_receive.c +++ b/src/shared/file_receive.c @@ -689,7 +689,7 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi suppresses the timestamps; ownership stays gated by the identity policy. A symlink has no children, so this can be applied immediately. */ if (ok && config && config->use_metadata) - file_restore_symlink_metadata(link_path, file->metadata, config->omit_link_times); + ok = file_restore_symlink_metadata(link_path, file->metadata, config->omit_link_times); free(link_path); return ok ? FILE_SAVE_WRITTEN : FILE_SAVE_ERROR; } diff --git a/src/shared/identity.c b/src/shared/identity.c index 6f00efd..b30f8d7 100644 --- a/src/shared/identity.c +++ b/src/shared/identity.c @@ -679,8 +679,8 @@ static void identity_log_chown_failure(const char* what, uid_t uid, gid_t gid) { * restricted root, root-squash, or a read-only mount) the run would be * silently producing the WRONG ownership, so surface it at ERROR. The * caller (identity_apply_ownership*) then reports the ENTRY as failed rather - * than as written; the receiver never claims a --copy-as success it did not - * achieve, but a single entry failure does not abort the whole run. */ + * than as written, which becomes a FILE_SAVE_ERROR and fails the transfer + * (fail-fast) instead of reporting overall success with the wrong owner. */ if (errno == EPERM || errno == EACCES) { if (identity_copy_as_active()) log_message(LOG_LEVEL_ERROR, diff --git a/src/shared/identity.h b/src/shared/identity.h index 610988b..8b07e0c 100644 --- a/src/shared/identity.h +++ b/src/shared/identity.h @@ -7,7 +7,7 @@ #include /* - * Identity mapping: --numeric-ids / --usermap / --groupmap / --chown. + * Identity mapping: --numeric-ids / --usermap / --groupmap / --chown / --copy-as. * * FastSync transmits uid/gid numerically (int32 on the wire) and, by design, * NEVER applies client-supplied ownership unless a user explicitly opts in with @@ -87,9 +87,10 @@ bool identity_ownership_requested(const Config* config); /* Apply the negotiated ownership to an already-written file descriptor. * source_uid/source_gid are the transmitted numeric ids. Resolution order: - * a matching usermap/groupmap rule, then --chown, then --numeric-ids (raw), - * then a best-effort name lookup on the receiver's own databases (skipped when - * the transmitted id has no name on this system). Only calls fchown() when the + * --copy-as (highest priority, forces both ids), then a matching + * usermap/groupmap rule, then --chown, then --numeric-ids (raw), then a + * best-effort name lookup on the receiver's own databases (skipped when the + * transmitted id has no name on this system). Only calls fchown() when the * result differs from the current value. * * Returns false ONLY when an active --copy-as ownership application failed: its diff --git a/src/shared/metadata.c b/src/shared/metadata.c index 937e324..949d365 100644 --- a/src/shared/metadata.c +++ b/src/shared/metadata.c @@ -357,17 +357,20 @@ void file_restore_metadata(const char* path, const FileMetadata* metadata, } } -void file_restore_symlink_metadata(const char* path, const FileMetadata* metadata, +bool file_restore_symlink_metadata(const char* path, const FileMetadata* metadata, bool omit_link_times) { if (path == NULL || metadata == NULL) - return; + return true; char* leaf = NULL; int parent_fd = file_open_secure_parent(path, &leaf, false); if (parent_fd < 0) - return; + return !identity_copy_as_active(); /* Ownership (only when the identity policy is active) via lchown semantics: - fchownat with AT_SYMLINK_NOFOLLOW never dereferences the link. */ - identity_apply_ownership_link(parent_fd, leaf, (int32_t)metadata->uid, (int32_t)metadata->gid); + fchownat with AT_SYMLINK_NOFOLLOW never dereferences the link. A failed + REQUIRED --copy-as ownership marks the entry failed; every other policy is + best-effort. */ + bool owned = identity_apply_ownership_link(parent_fd, leaf, (int32_t)metadata->uid, + (int32_t)metadata->gid); /* Symlink mode: not settable on Linux (fchmodat AT_SYMLINK_NOFOLLOW returns EOPNOTSUPP/ENOTSUP); attempt it for platforms that support it and quietly ignore the unsupported case so the transfer never fails over it. */ @@ -392,6 +395,7 @@ void file_restore_symlink_metadata(const char* path, const FileMetadata* metadat } close(parent_fd); free(leaf); + return owned; } bool file_restore_metadata_fd(int fd, const FileMetadata* metadata, bool preserve_executability) { diff --git a/src/shared/metadata.h b/src/shared/metadata.h index 28fe5f9..faf5194 100644 --- a/src/shared/metadata.h +++ b/src/shared/metadata.h @@ -44,8 +44,10 @@ bool file_restore_metadata_fd(int fd, const FileMetadata* metadata, bool preserv * under the authorized root. `omit_link_times` (-J/--omit-link-times) * suppresses the timestamps; the link's mode/ownership are still attempted * (ownership stays gated by the identity policy and by default is not applied). - * A null metadata or an unfollowable parent is a harmless no-op. */ -void file_restore_symlink_metadata(const char* path, const FileMetadata* metadata, + * A null metadata or an unfollowable parent is a harmless no-op. Returns false + * only when a REQUIRED --copy-as ownership application failed, so the caller can + * report the entry as failed instead of claiming a wrong-owner success. */ +bool file_restore_symlink_metadata(const char* path, const FileMetadata* metadata, bool omit_link_times); /* Compare timestamps using rsync's whole-second modification window. */ diff --git a/src/shared/xattr.h b/src/shared/xattr.h index f615fbe..55f22dd 100644 --- a/src/shared/xattr.h +++ b/src/shared/xattr.h @@ -89,11 +89,13 @@ void fake_super_store_fd(int fd, uint32_t uid, uint32_t gid, uint32_t mode, int6 /* --fake-super replay: parse the FAKESUPER_XATTR record previously written on * `fd` by fake_super_store_fd and re-apply uid/gid/mode/mtime fd-relative. * Best-effort: absence of the xattr or a malformed record is a silent no-op - * that never fails the transfer; fchown is applied only when permitted (a - * non-root EPERM/EACCES is skipped silently, matching FastSync's identity - * philosophy), and the mode is sanitized exactly like the normal metadata path - * (group/other write bits never granted). Returns true when the xattr was - * present and parsed. */ + * that never fails the transfer. The OWNER leg is applied only when an explicit + * ownership identity policy is active (numeric-ids/chown/usermap/groupmap/ + * copy-as), when super-user activities are permitted, and when --copy-as is not + * authoritative; a non-root EPERM/EACCES is skipped silently, matching + * FastSync's identity philosophy. The mode is sanitized exactly like the normal + * metadata path (group/other write bits never granted). Returns true when the + * xattr was present and parsed. */ bool fake_super_restore_fd(int fd); #endif \ No newline at end of file diff --git a/tests/integration/test_daemon.py b/tests/integration/test_daemon.py index 0091c4a..d0ea7ca 100644 --- a/tests/integration/test_daemon.py +++ b/tests/integration/test_daemon.py @@ -16,8 +16,10 @@ import hashlib import os import shutil import signal +import stat import subprocess import sys +import tempfile import time import pytest @@ -244,6 +246,21 @@ def _tree_file_count(root): return sum(len(files) for _, _, files in os.walk(root)) if os.path.exists(root) else 0 +def _can_mknod(): + """True when this process may create a char device (needs root/CAP_MKNOD).""" + probe = os.path.join(tempfile.gettempdir(), "._fastsync_mknod_probe_%d" % os.getpid()) + try: + os.mknod(probe, stat.S_IFCHR | 0o600, os.makedev(1, 3)) + os.unlink(probe) + return True + except (OSError, AttributeError): + try: + os.unlink(probe) + except OSError: + pass + return False + + class TestDaemonModuleSelection: @pytest.mark.ci def test_module_transfer(self, daemon): @@ -382,6 +399,43 @@ class TestDaemonRejection: assert not missing, f"missing: {missing[:5]}" assert not mismatches, f"mismatch: {mismatches[:5]}" + def _device_source(self, name): + src = os.path.join(TEST_DATA_DIR, name) + shutil.rmtree(src, ignore_errors=True) + os.makedirs(src) + with open(os.path.join(src, "f.txt"), "wb") as fh: + fh.write(b"device gate\n") + os.mknod(os.path.join(src, "null"), stat.S_IFCHR | 0o666, os.makedev(1, 3)) + return src + + @pytest.mark.skipif(not _can_mknod(), reason="device nodes need root/CAP_MKNOD") + def test_devices_skipped_without_owner_opt_in(self, daemon): + """H3: a non-opted daemon module must not create device nodes even under + the default AUTO super mode (a root daemon would otherwise let any client + mknod arbitrary devices). An ordinary -a push still succeeds; the device + entry is skipped.""" + src = self._device_source("devsrc_noowner") + os.makedirs(os.path.join(FILES_MODULE, "devskip"), exist_ok=True) + result, _ = run_client(src, "127.0.0.1::files/devskip", port=daemon.port, flags=["-a"]) + assert result.returncode == 0, result.stderr or result.stdout + received = get_dest_received_dir(os.path.join(FILES_MODULE, "devskip"), src) + node = os.path.join(received, "null") + assert not os.path.exists(node) or not stat.S_ISCHR(os.stat(node).st_mode), \ + "non-opted daemon module created a device node" + + @pytest.mark.skipif(not _can_mknod(), reason="device nodes need root/CAP_MKNOD") + def test_devices_created_with_owner_opt_in(self, daemon): + """Control: an opted-in module (`client owner = yes`) may create device + nodes under -a, proving the clamp is specific to non-opted modules.""" + src = self._device_source("devsrc_owner") + os.makedirs(os.path.join(OWNER_MODULE, "devok"), exist_ok=True) + result, _ = run_client(src, "127.0.0.1::owner/devok", port=daemon.port, flags=["-a"]) + assert result.returncode == 0, result.stderr or result.stdout + received = get_dest_received_dir(os.path.join(OWNER_MODULE, "devok"), src) + node = os.path.join(received, "null") + assert os.path.exists(node) and stat.S_ISCHR(os.stat(node).st_mode), \ + "opted-in daemon module did not create the device node" + @pytest.mark.daemon_detach def test_real_detach_path(self): """--daemon WITHOUT --no-detach double-forks a real background daemon; From ea0a0e2eafff45366567ff5896fe65faaff6c51c Mon Sep 17 00:00:00 2001 From: TapTap Date: Sat, 12 Sep 2026 14:33:42 +0200 Subject: [PATCH 3/3] fix(p8-security): make --copy-as directory ownership airtight; harden tests/logs - file_ensure_directory_secure() now chowns a final directory it creates under --copy-as and fails on error; the symlink parent-creation call site propagates it. The is_dir branch fails when the confined parent cannot be opened under --copy-as. Closes the residual wrong-owner gap for synthesized/symlink parent directories. - file_restore_symlink_metadata() early NULL return is copy-as-aware. - Preserve errno across the implicit-parent failure cleanup. - Neutral skip messages (the clamp, not --no-super, may be responsible). - Daemon copy-as test tolerates the non-root privilege refusal; usage text lists --copy-as. --- src/client/usage.c | 2 +- src/shared/file.c | 20 ++++++++++++++++++-- src/shared/file_receive.c | 17 ++++++++++++----- src/shared/metadata.c | 2 +- tests/integration/test_daemon.py | 13 +++++++++---- 5 files changed, 41 insertions(+), 13 deletions(-) diff --git a/src/client/usage.c b/src/client/usage.c index 9d793d3..0da87ba 100644 --- a/src/client/usage.c +++ b/src/client/usage.c @@ -173,7 +173,7 @@ void print_usage(void) { printf(" within the confined receive root. Never elevates\n"); printf(" privileges and never bypasses confinement; ownership\n"); printf(" is still applied only with an explicit identity flag\n"); - printf(" (--numeric-ids/--chown/--usermap/--groupmap)\n"); + printf(" (--numeric-ids/--chown/--usermap/--groupmap/--copy-as)\n"); printf(" --no-super Forbid those super-user activities even when the\n"); printf(" receiver is running as root\n"); printf(" --chmod Modify transferred permissions (rsync syntax)\n"); diff --git a/src/shared/file.c b/src/shared/file.c index c6bc4ef..58e2753 100644 --- a/src/shared/file.c +++ b/src/shared/file.c @@ -586,10 +586,14 @@ int file_open_secure_parent(const char* path, char** leaf_out, bool create_dirs) !identity_apply_ownership_link(fd, component, 0, 0)) { /* A REQUIRED --copy-as ownership that cannot be applied to a directory this walk just created must fail the entry rather than - leave that implicit parent owned by the receiver. */ + leave that implicit parent owned by the receiver. Preserve the + failing errno across the cleanup so the caller logs the real + reason. */ + int saved_errno = errno; close(fd); free(copy); free(leaf); + errno = saved_errno; return -1; } next = openat(fd, component, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); @@ -699,11 +703,23 @@ bool file_ensure_directory_secure(const char* path) { return false; int dir_fd = openat(parent_fd, leaf, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + bool created = false; if (dir_fd < 0 && errno == ENOENT) { - if (mkdirat(parent_fd, leaf, 0755) == 0 || errno == EEXIST) + if (mkdirat(parent_fd, leaf, 0755) == 0) { + created = true; dir_fd = openat(parent_fd, leaf, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + } else if (errno == EEXIST) { + dir_fd = openat(parent_fd, leaf, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + } } bool ok = dir_fd >= 0; + /* --copy-as owns a directory this call just created (the final component; + intermediate components were handled by file_open_secure_parent above). A + failed REQUIRED ownership fails the call rather than leaving the directory + owned by the receiver. */ + if (ok && created && identity_copy_as_active() && + !identity_apply_ownership_link(parent_fd, leaf, 0, 0)) + ok = false; if (dir_fd >= 0) close(dir_fd); close(parent_fd); diff --git a/src/shared/file_receive.c b/src/shared/file_receive.c index 9ce4d90..7059596 100644 --- a/src/shared/file_receive.c +++ b/src/shared/file_receive.c @@ -364,8 +364,7 @@ static FileSaveResult file_save_special_to_disk(const char* root_directory, cons 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 " - "(super-user activities disabled by --no-super)", + "skipping %s: super-user device-node creation is not permitted on this receiver", file->path); return FILE_SAVE_SKIPPED; } @@ -598,7 +597,8 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi 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", + "write-devices: %s skipped: super-user activities are not permitted on this " + "receiver", file->path ? file->path : "(null)"); return FILE_SAVE_SKIPPED; } @@ -633,6 +633,10 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi (int32_t)file->metadata->gid)) ok = false; close(parent_fd); + } else if (identity_copy_as_active()) { + /* The directory exists (ok) but its required --copy-as ownership could + not be applied because the confined parent could not be opened. */ + ok = false; } free(leaf); } @@ -679,10 +683,13 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi } char* parent = str_dup(link_path); if (parent) { - file_ensure_directory_secure(dirname(parent)); + /* Propagate a failed --copy-as ownership of the parent directory this + creates; every other failure mode stays best-effort as before. */ + ok = file_ensure_directory_secure(dirname(parent)); free(parent); } - ok = file_symlink_at_secure(link_path, target); + if (ok) + ok = file_symlink_at_secure(link_path, target); free(target); /* P7 Wave D: apply the symlink's own metadata with no-follow primitives (utimensat/lchown/fchmodat AT_SYMLINK_NOFOLLOW). -J/--omit-link-times diff --git a/src/shared/metadata.c b/src/shared/metadata.c index 949d365..b1cff1e 100644 --- a/src/shared/metadata.c +++ b/src/shared/metadata.c @@ -360,7 +360,7 @@ void file_restore_metadata(const char* path, const FileMetadata* metadata, bool file_restore_symlink_metadata(const char* path, const FileMetadata* metadata, bool omit_link_times) { if (path == NULL || metadata == NULL) - return true; + return !identity_copy_as_active(); char* leaf = NULL; int parent_fd = file_open_secure_parent(path, &leaf, false); if (parent_fd < 0) diff --git a/tests/integration/test_daemon.py b/tests/integration/test_daemon.py index d0ea7ca..d0781e0 100644 --- a/tests/integration/test_daemon.py +++ b/tests/integration/test_daemon.py @@ -343,10 +343,13 @@ class TestDaemonRejection: assert result.returncode != 0 assert _tree_file_count(AUTH_MODULE) == 0 - def _assert_ownership_refused(self, daemon, module, flags): + def _assert_ownership_refused(self, daemon, module, flags, + accept=("client-chosen ownership",)): """A daemon module without `client owner = yes` refuses every client-chosen ownership / super-user request at the config handshake, - before any data lands.""" + before any data lands. `accept` lists the log phrases that count as the + refusal (a non-root daemon refuses --copy-as earlier, at the privilege + check, so the caller accepts that phrase too).""" 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() @@ -358,7 +361,7 @@ class TestDaemonRejection: with open(log_path, "rb") as f: f.seek(before) tail = f.read().decode("utf-8", "replace") - assert "client-chosen ownership" in tail, ( + assert any(phrase in tail for phrase in accept), ( f"daemon did not log the ownership refusal: {tail[-400:]!r}" ) @@ -368,7 +371,9 @@ class TestDaemonRejection: so even a root daemon must not honor an arbitrary client-selected owner by default. The refusal happens at the config handshake, before any data lands.""" - self._assert_ownership_refused(daemon, "files", ["--copy-as=@65534:@65534"]) + self._assert_ownership_refused( + daemon, "files", ["--copy-as=@65534:@65534"], + accept=("client-chosen ownership", "requires a privileged receiver")) def test_super_refused_by_daemon(self, daemon): """An explicit --super is a super-user activity request, so a daemon