diff --git a/RSYNC_COMPAT.md b/RSYNC_COMPAT.md index e8c19ef..e2ab969 100644 --- a/RSYNC_COMPAT.md +++ b/RSYNC_COMPAT.md @@ -611,7 +611,7 @@ now transmits targets (the prior behavior was broken/partial); its status moved | Flag | Rsync Description | FastSync Status | Notes | |------|-------------------|-----------------|-------| | `-e`, `--rsh=COMMAND` | Remote shell to use | ✅ Implemented | `-e`/`--rsh` (and `--rsh=COMMAND`) select the remote-shell program used to build the SSH child argv, overriding the default `ssh`. The command is whitespace-split into the leading argv words so rsync's `-e "ssh -p 2222"` works; the standard `-o` family, an optional `-p` port, `user@host` and the quoted remote command (`fastsync-server --stdio`) follow. Stored in the `rsh_command` config field. **Client-only, never crosses the wire** (it is a launch concern, not a handshake property) | -| `--rsync-path=PROGRAM` | rsync binary on remote | ✅ Implemented | Alias for `--fastsync-server-path`: both write the `fastsync_server_path` config field used as the remote-side server program (quoted as one remote-shell word unless `--old-args`), which CROSSES the wire as before. Kept separate from `--rsh`, which names the local connecting program | +| `--rsync-path=PROGRAM` | rsync binary on remote | ✅ Implemented | Alias for `--fastsync-server-path`: both write the `fastsync_server_path` config field used as the remote-side server program (always quoted as one remote-shell word), which CROSSES the wire as before. Kept separate from `--rsh`, which names the local connecting program | | `--port=PORT` | Alternate daemon port | ✅ Implemented | rsync's daemon-port flag maps to the client-side `server_port` config field: a client connects to a TCP/TLS server (incl. `host::module/path` daemon destinations) with `--server-port`, and the `fastsync-server --daemon` listener's port is taken from its config's `port` key (default 873) or overridden by `--dparam port=` / `-p` | | `--sockopts=OPTIONS` | Custom TCP options | ✅ Implemented | Comma-separated allowlist of `OPT=VAL` applied via `setsockopt` after `socket()` before `connect()`/`bind()`. Only `TCP_NODELAY`, `SO_KEEPALIVE`, `SO_REUSEADDR` (0/1) and `SO_RCVBUF`/`SO_SNDBUF` (byte count) are accepted; an unknown option name or a bad value is rejected up front, never silently ignored. A value is required for every option (`OPT=VAL`; a bare name is an error). Applied to the outgoing TCP and TLS client socket; absent by default. `SockOptEntry`/`sockopts` config fields. Local socket concern: never crosses the wire | | `--blocking-io` | Use blocking I/O for remote shell | ✅ Implemented | With `--blocking-io` the SSH-transport socketpair socket is left without `SO_RCVTIMEO`/`SO_SNDTIMEO`, so the transfer blocks naturally; by default it gets the same read/write timeout as the TCP transport (see `--timeout`). `blocking_io` config bool. **Client-only, never crosses the wire** | @@ -629,7 +629,7 @@ now transmits targets (the prior behavior was broken/partial); its status moved | `--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` | -| `--password-file=FILE` | Read daemon password from file | ✅ Implemented | Wave B daemon auth. Client: `--password-file` supplies `user:password` for a `host::module/path` destination (the username is taken from this file, so `user@host::module` stays rejected). Server (`fastsync-server --daemon --password-file FILE`): the credential store that modules with `auth users` are verified against. Only a SHA-256 digest of the password ever crosses the wire or is stored server-side; the literal password never appears in logs. See the Daemon Mode notes below for the file formats and the plaintext/TLS caveat | +| `--password-file=FILE` | Read daemon password from file | ✅ Implemented | Wave B daemon auth. Client: `--password-file` supplies `user:password` for a `host::module/path` destination (the username is taken from this file, so `user@host::module` stays rejected). Server (`fastsync-server --daemon --password-file FILE`): the credential store that modules with `auth users` are verified against. Only a SHA-256 digest of the password ever crosses the wire or is stored server-side; the literal password never appears in logs. The file must be private to its owner: both the client and server verify the exact inode they read (open-then-`fstat`, so the check cannot be raced) and refuse a `--password-file`/`--early-input` that is not owned by the current user or grants any group/other permission bit (mode 0600), mirroring the TLS private-key check. A process-substitution pipe (`--early-input <(vault ...)`) is still accepted when it satisfies those checks. See the Daemon Mode notes below for the file formats and the plaintext/TLS caveat | | `--early-input=FILE` | Use FILE for daemon early exec | ✅ Implemented | Server-only (requires `--daemon`): a second credential-store file, same `user:SHA256HEX` grammar as `--password-file`, read before the listener accepts connections (a secrets-manager / process-substitution source). Its entries layer over `--password-file`: identical entries dedupe, a conflicting secret for the same user is a startup error. A daemon whose modules declare `auth users` must be given at least one of the two, or it refuses to start (fail closed) | **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. @@ -657,7 +657,7 @@ now transmits targets (the prior behavior was broken/partial); its status moved | Per-connection memory limit | 1GB per connection | ✅ Implemented | `MAX_CONNECTION_MEMORY` | | `--max-alloc=SIZE` | Limit a single memory allocation | ✅ Implemented | Caps the largest single allocation; binary units, default 1G | | `--trust-sender` | Trust remote sender's file list | ✅ Implemented | Long-form-only, receiver-local policy that never crosses the wire. The receiver skips its redundant up-front re-validation of the incoming file list (empty/`..` path rejection and the escaping-symlink-target containment), trusting the sender instead of double-checking (fewer checks, faster, potentially unsafe, matching rsync). Off by default. The low-level fd-relative confinement primitives (`file_open_secure_parent`, the O_NOFOLLOW parent walk, leaf/destination confinement) are deliberately KEPT even under `--trust-sender`, so a hostile sender still cannot write or link outside the authorized root (see Phase-5 notes below) | -| `--old-args` | Disable modern arg protection | ✅ Implemented | SSH-only legacy mode; restores raw remote command construction and permits shell interpretation of the configured server path | +| `--old-args` | Disable modern arg protection | ✅ Implemented | SSH-only; accepted for CLI compatibility but is now a **documented no-op**: FastSync always single-quote-escapes the remote server path and each `--remote-option` value (`ssh_build_remote_command`), so a metacharacter-bearing `--rsync-path` can never be interpreted by the remote shell. The flag no longer disables that quoting (the old raw-construction behavior was an injection foot-gun and is removed); the safety-relevant behavior is identical either way | | `--ignore-missing-args` | Ignore missing source args | ✅ Implemented | FastSync has a single source-root argument (which always exists), so the "explicitly requested source arguments" are the `--files-from` entries and the flags only ever apply there (inert without `--files-from`, like `-R`). Without the flag a listed-but-missing entry stays a hard pre-transfer error (nothing is transferred). With it each missing entry is skipped: nothing is sent for it, it never enters the keep-set, and the run succeeds for the rest — an all-missing non-empty list succeeds transferring nothing, matching rsync. `--dirs` + `--files-from` missing entries are skipped the same way. Every skipped entry is logged and a per-run warning names the count, so the handling is never a silent no-op. Divergences: an EMPTY `--files-from` file stays a hard error in every mode (no argument was requested at all; rsync likewise reports "no source files specified"); missing-arg skipping only applies to the pre-transfer list validation, so an entry that is present at preflight and vanishes mid-transfer still fails (matching rsync, whose flag "does not affect subsequent vanished-file errors"); `--no-ignore-missing-args` is not a supported negation | | `--delete-missing-args` | Delete missing source args | ✅ Implemented | Implies `--ignore-missing-args` (order-independent) and additionally removes each missing entry's destination mirror receiver-side. The mirror is computed exactly like a present sibling's wire path: the bare relative entry under `-R`, otherwise the full source-mirror path below the destination root. rsync parity, verified against the man page: it does **not** imply `--delete` generally and is "independent of any other type of delete processing" — unrelated destination extras are untouched unless `--delete` is also present. Composition with `--delete` + timing: the exact-path deletions commit with the manifest, early for `--delete-before`/`--delete-during`, else only after a fully-successful transfer (delete-after/commit). A non-empty directory mirror is removed only when `--force` or `--delete` is in effect (otherwise it is left with a warning and the run continues, like rsync); an absent mirror is a no-op. An explicitly listed missing arg is a user request, not an excluded file: its deletion is never blocked by the filter-exclusion protection of excluded destination mirrors (a mirror sitting inside a filter-excluded directory is still removed). Safety/policy: gated by the server `--allow-delete` policy like `--delete`; the request paths cross the wire only in the delete-manifest frame and are confined by the same receiver validation as the keep-set (non-empty, relative, traversal-free, bounded by the per-section/per-frame manifest caps); the `--delay-updates` staging directory and basis snapshots are protected exactly as in the extras walker. Divergence: the missing-args deletions are not counted toward `--max-delete` (they are explicit per-path requests, not discovered extras). See the Phase-3 wire note below for the `PROTOCOL_VERSION` bump | diff --git a/src/client/client_cli.c b/src/client/client_cli.c index b2af76f..44856de 100644 --- a/src/client/client_cli.c +++ b/src/client/client_cli.c @@ -389,6 +389,21 @@ static int parse_ull_arg(const char* val, unsigned long long* out, const char* o return 0; } +/* Apply a --delta-block/--block-size value (both spellings and both the inline + * and separate argument forms share this one range check). An out-of-range + * value warns once and leaves the configured default untouched. Returns 0 on + * success, -1 on a non-numeric value. */ +static int set_delta_block_size(Config* config, const char* value) { + unsigned long long val; + if (parse_ull_arg(value, &val, "--block-size/--delta-block") != 0) + return -1; + if (val >= DELTA_BLOCK_SIZE_MIN && val <= DELTA_BLOCK_SIZE_MAX) + config->delta_block_size = (uint32_t)val; + else + log_message(LOG_LEVEL_WARNING, "block size value %llu out of range, using default", val); + return 0; +} + /* Parse a byte count with an optional single-letter binary suffix (K/M/G/T/P/E). * When allow_zero is false, a bare 0 is rejected (size limits use true, since 0 * means "no limit"). Returns 0 on success, -1 on error. */ @@ -1094,33 +1109,18 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args, "--include") != 0) return -1; } else if (strncmp(argv[i], "--delta-block=", 14) == 0) { - unsigned long long val; - if (parse_ull_arg(argv[i] + 14, &val, "--block-size/--delta-block") != 0) + if (set_delta_block_size(config, argv[i] + 14) != 0) return -1; - if (val >= DELTA_BLOCK_SIZE_MIN && val <= DELTA_BLOCK_SIZE_MAX) - config->delta_block_size = (uint32_t)val; - else - log_message(LOG_LEVEL_WARNING, "block size value %llu out of range, using default", val); } else if (strncmp(argv[i], "--block-size=", 13) == 0) { - unsigned long long val; - if (parse_ull_arg(argv[i] + 13, &val, "--block-size/--delta-block") != 0) + if (set_delta_block_size(config, argv[i] + 13) != 0) return -1; - if (val >= DELTA_BLOCK_SIZE_MIN && val <= DELTA_BLOCK_SIZE_MAX) - config->delta_block_size = (uint32_t)val; - else - log_message(LOG_LEVEL_WARNING, "block size value %llu out of range, using default", val); } else if (opt_is(argv[i], "--delta-block", "--block-size")) { if (i + 1 >= argc) { log_message(LOG_LEVEL_ERROR, "missing argument for %s", argv[i]); return -1; } - unsigned long long val; - if (parse_ull_arg(argv[++i], &val, "--block-size/--delta-block") != 0) + if (set_delta_block_size(config, argv[++i]) != 0) return -1; - if (val >= DELTA_BLOCK_SIZE_MIN && val <= DELTA_BLOCK_SIZE_MAX) - config->delta_block_size = (uint32_t)val; - else - log_message(LOG_LEVEL_WARNING, "block size value %llu out of range, using default", val); } else if (opt_is(argv[i], "--delta-max", NULL)) { if (i + 1 >= argc) { log_message(LOG_LEVEL_ERROR, "missing argument for %s", argv[i]); @@ -1431,7 +1431,6 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args, } else if (strncmp(argv[i], "--copy-as=", 10) == 0) { if (identity_parse_copy_as(config, argv[i] + 10) != 0) return -1; - config->use_metadata = true; } else if (opt_is(argv[i], "--copy-as", NULL)) { if (i + 1 >= argc) { log_message(LOG_LEVEL_ERROR, "missing argument for %s", argv[i]); @@ -1439,7 +1438,6 @@ int parse_args(Config* config, int argc, char* argv[], int* positional_args, } if (identity_parse_copy_as(config, argv[++i]) != 0) return -1; - config->use_metadata = true; } else if (strncmp(argv[i], "--outbuf=", 9) == 0) { if (set_outbuf_option(config, argv[i] + 9) != 0) return -1; diff --git a/src/client/usage.c b/src/client/usage.c index 0da87ba..f63a5c3 100644 --- a/src/client/usage.c +++ b/src/client/usage.c @@ -246,8 +246,8 @@ void print_usage(void) { printf(" -T, --temp-dir Scratch dir for temp files before atomic install\n"); printf(" --fastsync-server-path \n"); printf(" Path to fastsync-server on remote (default: fastsync-server)\n"); - printf( - " --old-args Disable safe SSH command argument quoting (legacy compatibility)\n"); + printf(" --old-args Accepted for rsync CLI compatibility; no effect (the\n"); + printf(" remote server path is always safely quoted now)\n"); printf(" -M, --remote-option=OPT Append OPT to the REMOTE server invocation over SSH\n"); printf(" (repeatable; each value is single-quote-escaped on the remote\n"); printf(" command line; empty values and values with control characters\n"); diff --git a/src/server/server.c b/src/server/server.c index f68615e..80d367b 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -55,9 +55,15 @@ static CredentialStore* g_credentials = NULL; /* Opaque context threaded through to the config-frame gate: the connection's * SSL object (NULL over plaintext) so the gate can warn when a credential - * exchange is not encrypted. */ + * exchange is not encrypted, plus the super-mode override the gate decides on. + * The gate never mutates the received (const) Config; it records a forced + * SUPER_MODE_OFF here and the handler applies it exactly once after acceptance. */ typedef struct ModuleGateContext { SSL* ssl; + /* SUPER_MODE_OFF when this connection must not attempt any super-user + activity (operator --no-super, or a daemon module without the + `client owner = yes` opt-in); -1 when the config's own mode stands. */ + int super_mode_override; } ModuleGateContext; /* Aggregate payload bytes the multithreaded receiver may buffer ahead of the @@ -182,11 +188,15 @@ static const char* server_module_gate(const Config* config, void* context) { if (!config) return "missing config frame"; /* 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; + the copy-as gate is evaluated. The received config is const, so the gates + below evaluate a shallow effective copy (only super_mode differs); the + handler applies the recorded override to the accepted config exactly once. */ + Config effective = *config; + if (server_no_super) { + effective.super_mode = SUPER_MODE_OFF; + if (gate_ctx) + gate_ctx->super_mode_override = 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 @@ -195,7 +205,7 @@ static const char* server_module_gate(const Config* config, void* context) { 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 (identity_copy_as_refused(&effective)) { if (geteuid() != 0) log_message(LOG_LEVEL_ERROR, "--copy-as requires a privileged receiver (root); refusing"); else @@ -247,9 +257,10 @@ static const char* server_module_gate(const Config* config, void* context) { standalone/SSH server has a single operator-authorized root and keeps honoring these. */ 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. */ + /* Ownership: refuse the whole transfer up front (a clear failure). + Evaluated against the ORIGINAL config so an explicit --super is refused + even when an operator --no-super veto already forced the effective copy + to OFF (the veto must not silently convert a refusal into an accept). */ if (identity_ownership_requested(config)) { log_message(LOG_LEVEL_ERROR, "daemon module '%s' refuses client-chosen ownership/super-user activities " @@ -258,13 +269,14 @@ static const char* server_module_gate(const Config* config, void* context) { 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 + permitted under the default AUTO mode, so without this override 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 (gate_ctx) + gate_ctx->super_mode_override = SUPER_MODE_OFF; } if (module->auth_user_count > 0) { /* Auth-required module (Wave B): verify the presented credentials against @@ -322,6 +334,7 @@ void handler(int file_descriptor) { protocol_session_bind(&session); ModuleGateContext gate_ctx; gate_ctx.ssl = ssl; + gate_ctx.super_mode_override = -1; Config* config = config_receive_with_validate(file_descriptor, server_module_gate, &gate_ctx); if (config == NULL) { log_message(LOG_LEVEL_ERROR, "Failed to receive config"); @@ -329,12 +342,13 @@ void handler(int file_descriptor) { protocol_session_unbind(); return; } - /* Operator --no-super veto: clamp the accepted config so every downstream - * gate (identity_apply_ownership via privilege_super_permitted, device-node - * creation) sees SUPER_MODE_OFF even if the gate callback did not already - * mutate a copy of it. */ - if (server_no_super) - config->super_mode = SUPER_MODE_OFF; + /* Apply the super-mode veto the gate decided on (operator --no-super, or a + * daemon module without the `client owner = yes` opt-in) exactly once, so + * every downstream gate (identity_apply_ownership via privilege_super_permitted, + * device-node creation) sees SUPER_MODE_OFF. The gate never mutated the + * received config. */ + if (gate_ctx.super_mode_override != -1) + config->super_mode = gate_ctx.super_mode_override; protocol_set_8_bit_output(config->eight_bit_output); if (!authorized_root) { log_message(LOG_LEVEL_ERROR, "No server-side destination root configured"); @@ -417,8 +431,10 @@ void handler(int file_descriptor) { before anything else; without it the root must pre-exist. A failure here aborts the connection cleanly before any file data is exchanged. */ if (!ensure_receive_root(config)) { + char* escaped_root = output_escape(config->receive_root_directory, log_get_8_bit_output()); log_message(LOG_LEVEL_ERROR, "destination root is not available: %s", - config->receive_root_directory); + escaped_root ? escaped_root : ""); + free(escaped_root); config_delete(config); close(file_descriptor); protocol_session_unbind(); @@ -440,8 +456,16 @@ void handler(int file_descriptor) { } /* Preserve the negotiated identity policy for the fd-relative ownership apply path. Each connection is its own forked process, so this - per-process snapshot never races another connection. */ - identity_set_active(config); + per-process snapshot never races another connection. A failed deep copy + (allocation failure) leaves the snapshot cleared, so refuse the connection + rather than silently applying the wrong ownership policy. */ + if (!identity_set_active(config)) { + log_message(LOG_LEVEL_ERROR, "Failed to activate identity policy"); + config_delete(config); + close(file_descriptor); + protocol_session_unbind(); + return; + } /* Persist the negotiated --keep-dirlinks policy once, here at config-accept, before any multithreaded receiver/writer threads are spawned, so the fd-walk reads a stable value during the whole transfer (and never bleeds @@ -485,6 +509,7 @@ void handler(int file_descriptor) { config_delete(config); close(file_descriptor); protocol_session_unbind(); + identity_clear_active(); return; } PipelineContextReceiver* context = diff --git a/src/server/server_cli.c b/src/server/server_cli.c index 120e7bd..6ea42d8 100644 --- a/src/server/server_cli.c +++ b/src/server/server_cli.c @@ -64,6 +64,7 @@ int server_cli_parse(int argc, char* argv[], ServerCliOptions* opts, char* err, server_cli_options_default(opts); for (int i = 1; i < argc; i++) { + const char* inline_value = NULL; if (arg_is(argv[i], "--help")) { opts->show_help = true; return 1; @@ -108,18 +109,24 @@ int server_cli_parse(int argc, char* argv[], ServerCliOptions* opts, char* err, } opts->destination_root = argv[++i]; opts->destination_root_set = true; - } else if (arg_is(argv[i], "--password-file")) { - if (i + 1 >= argc) { - set_error(err, err_size, "missing argument for --password-file"); - return -1; + } else if (arg_has_value(argv[i], "--password-file", &inline_value)) { + if (!inline_value) { + if (i + 1 >= argc) { + set_error(err, err_size, "missing argument for --password-file"); + return -1; + } + inline_value = argv[++i]; } - opts->password_file = argv[++i]; - } else if (arg_is(argv[i], "--early-input")) { - if (i + 1 >= argc) { - set_error(err, err_size, "missing argument for --early-input"); - return -1; + opts->password_file = inline_value; + } else if (arg_has_value(argv[i], "--early-input", &inline_value)) { + if (!inline_value) { + if (i + 1 >= argc) { + set_error(err, err_size, "missing argument for --early-input"); + return -1; + } + inline_value = argv[++i]; } - opts->early_input_file = argv[++i]; + opts->early_input_file = inline_value; } else if (arg_is(argv[i], "--address")) { if (i + 1 >= argc) { set_error(err, err_size, "missing argument for --address"); @@ -146,12 +153,15 @@ int server_cli_parse(int argc, char* argv[], ServerCliOptions* opts, char* err, opts->no_super = true; } else if (arg_is(argv[i], "--allow-unauthenticated")) { opts->allow_unauthenticated = true; - } else if (arg_is(argv[i], "--iconv")) { - if (i + 1 >= argc) { - set_error(err, err_size, "missing argument for --iconv"); - return -1; + } else if (arg_has_value(argv[i], "--iconv", &inline_value)) { + if (!inline_value) { + if (i + 1 >= argc) { + set_error(err, err_size, "missing argument for --iconv"); + return -1; + } + inline_value = argv[++i]; } - opts->iconv_spec = argv[++i]; + opts->iconv_spec = inline_value; } else if (arg_is(argv[i], "-p")) { if (i + 1 >= argc) { set_error(err, err_size, "missing argument for -p"); @@ -161,7 +171,6 @@ int server_cli_parse(int argc, char* argv[], ServerCliOptions* opts, char* err, if (parse_port_arg(argv[++i], &opts->port, err, err_size) != 0) return -1; } else { - const char* inline_value = NULL; if (arg_has_value(argv[i], "--config", &inline_value)) { if (!inline_value) { if (i + 1 >= argc) { @@ -171,33 +180,6 @@ int server_cli_parse(int argc, char* argv[], ServerCliOptions* opts, char* err, inline_value = argv[++i]; } opts->config_path = inline_value; - } else if (arg_has_value(argv[i], "--password-file", &inline_value)) { - if (!inline_value) { - if (i + 1 >= argc) { - set_error(err, err_size, "missing argument for --password-file"); - return -1; - } - inline_value = argv[++i]; - } - opts->password_file = inline_value; - } else if (arg_has_value(argv[i], "--early-input", &inline_value)) { - if (!inline_value) { - if (i + 1 >= argc) { - set_error(err, err_size, "missing argument for --early-input"); - return -1; - } - inline_value = argv[++i]; - } - opts->early_input_file = inline_value; - } else if (arg_has_value(argv[i], "--iconv", &inline_value)) { - if (!inline_value) { - if (i + 1 >= argc) { - set_error(err, err_size, "missing argument for --iconv"); - return -1; - } - inline_value = argv[++i]; - } - opts->iconv_spec = inline_value; } else if (arg_has_value(argv[i], "--dparam", &inline_value)) { if (!inline_value) { if (i + 1 >= argc) { @@ -260,4 +242,4 @@ void server_cli_options_free(ServerCliOptions* opts) { free((char**)opts->dparams); opts->dparams = NULL; opts->dparam_count = 0; -} \ No newline at end of file +} diff --git a/src/shared/credentials.c b/src/shared/credentials.c index c27152e..fa1088f 100644 --- a/src/shared/credentials.c +++ b/src/shared/credentials.c @@ -2,12 +2,15 @@ #include "utils.h" #include #include +#include #include #include #include #include #include #include +#include +#include /* One store entry: a username and its password's SHA-256 hex digest. The * plaintext password never appears here (and never on the daemon host). */ @@ -35,6 +38,46 @@ static bool is_comment_char(char c) { return c == '#' || c == ';'; } +/* Open a --password-file / --early-input after verifying the EXACT inode we + * will read: it must be owned by the effective user and grant no group/other + * permission bit (mode 0600), mirroring the TLS private-key check. We open by + * path and then fstat the resulting fd (rather than stat()ing the path first + * and reopening it), so the permission decision is made on the same inode that + * is read and cannot be raced by swapping the path between check and open. + * The path may be a process-substitution pipe (`<(...)` -> /dev/fd/N), so + * regular files and FIFOs are accepted when the ownership/mode checks pass. + * + * Returns a FILE* the caller must fclose, or NULL with `err` filled. */ +static FILE* secret_file_open(const char* path, char* err, size_t err_size) { + int fd = open(path, O_RDONLY | O_CLOEXEC); + if (fd < 0) { + set_error(err, err_size, "cannot open secret file '%s': %s", path, strerror(errno)); + return NULL; + } + struct stat st; + if (fstat(fd, &st) != 0) { + set_error(err, err_size, "cannot stat secret file '%s': %s", path, strerror(errno)); + close(fd); + return NULL; + } + bool is_readable_kind = S_ISREG(st.st_mode) || S_ISFIFO(st.st_mode); + if (!is_readable_kind || st.st_uid != geteuid() || (st.st_mode & (S_IRWXG | S_IRWXO)) != 0) { + set_error(err, err_size, + "refusing to read secret file '%s': it must be owned by the current user and " + "owner-only (0600), not accessible to group/other", + path); + close(fd); + return NULL; + } + FILE* fp = fdopen(fd, "r"); + if (!fp) { + set_error(err, err_size, "cannot read secret file '%s': %s", path, strerror(errno)); + close(fd); + return NULL; + } + return fp; +} + /* Trim leading/trailing ASCII space and tab in place; returns the new start. */ static char* trim_space(char* s) { while (*s == ' ' || *s == '\t') @@ -124,9 +167,8 @@ static CredentialStore* load_store_file(const char* path, char* err, size_t err_ if (!path) return store; - FILE* fp = fopen(path, "r"); + FILE* fp = secret_file_open(path, err, err_size); if (!fp) { - set_error(err, err_size, "cannot open credential file '%s': %s", path, strerror(errno)); credentials_free(store); return NULL; } @@ -305,11 +347,9 @@ int credentials_read_secret_file(const char* path, char** user_out, char** passw set_error(err, err_size, "no --password-file path"); return -1; } - FILE* fp = fopen(path, "r"); - if (!fp) { - set_error(err, err_size, "cannot open password file '%s': %s", path, strerror(errno)); + FILE* fp = secret_file_open(path, err, err_size); + if (!fp) return -1; - } int line_no = 0; char line[CREDENTIAL_MAX_LINE + 2]; diff --git a/src/shared/file.c b/src/shared/file.c index 58e2753..6220967 100644 --- a/src/shared/file.c +++ b/src/shared/file.c @@ -16,6 +16,7 @@ #include "data.h" #include "delta.h" #include "file.h" +#include "file_store.h" #include "identity.h" #include "log.h" #include "metadata.h" @@ -37,44 +38,6 @@ static bool write_all(int fd, const void* data, unsigned long long size) { return true; } -/* A run of NUL bytes at least this long is emitted as a hole (lseek) rather - * than written, so the resulting file is genuinely sparse on the filesystem. */ -#define SPARSE_HOLE_MIN 4096U - -/* Sparse-aware writer (--sparse/-S). Walks `data`; any all-zero run of at - * least SPARSE_HOLE_MIN bytes is skipped with lseek(SEEK_CUR) so the block is - * never allocated (a real hole on the destination); every other byte is written - * normally. The file is pre-sized with ftruncate by the callers before this - * runs, so holes are guaranteed and the offset bookkeeping stays correct - * (each lseek advances the fd offset exactly as a write of that many bytes - * would). After the final run, ftruncate(size) guarantees the logical size is - * exactly `size` even when the tail was a hole. The full file image is in - * memory, so no wire change is needed. Returns false on I/O error. */ -static bool write_all_sparse(int fd, const unsigned char* data, unsigned long long size) { - unsigned long long i = 0; - while (i < size) { - if (data[i] == 0) { - unsigned long long run_start = i; - while (i < size && data[i] == 0) - i++; - unsigned long long run_len = i - run_start; - if (run_len >= SPARSE_HOLE_MIN) { - if (lseek(fd, (off_t)run_len, SEEK_CUR) < 0) - return false; - } else if (!write_all(fd, data + run_start, run_len)) { - return false; - } - } else { - unsigned long long run_start = i; - while (i < size && data[i] != 0) - i++; - if (!write_all(fd, data + run_start, i - run_start)) - return false; - } - } - return ftruncate(fd, (off_t)size) == 0; -} - /* Preallocate `size` bytes on `fd` before any data is written (--preallocate). * posix_fallocate reserves real disk blocks, so an out-of-space condition * (ENOSPC/EDQUOT) surfaces up front instead of partway through a transfer; @@ -515,6 +478,50 @@ out: return ok; } +/* Open the directory named by canonical absolute `resolved`, which the caller + * has already verified lies beneath `root` (the canonical authorized root). + * Each component is opened relative to the authorized-root fd with O_NOFOLLOW, + * so a directory swapped for a symlink after the realpath() check cannot + * redirect the open outside the root -- the walk simply fails. This replaces + * re-opening the absolute resolved path (TOCTOU). Returns an O_DIRECTORY fd, + * or -1 (the root itself and any error are refused). */ +static int open_dir_beneath_root(const char* resolved, const char* root) { + size_t root_len = strlen(root); + const char* rel = resolved + root_len; + while (*rel == '/') + rel++; + if (*rel == '\0') + return -1; + int fd = dup(authorized_root_fd); + if (fd < 0) + return -1; + char* copy = str_dup(rel); + if (!copy) { + close(fd); + return -1; + } + char* save = NULL; + for (char* component = strtok_r(copy, "/", &save); component; + component = strtok_r(NULL, "/", &save)) { + if (strcmp(component, ".") == 0) + continue; + /* A canonical realpath() output never contains "." or ".."; refuse ".." + defensively rather than let it climb toward the root. */ + int next = strcmp(component, "..") == 0 + ? -1 + : openat(fd, component, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + if (next < 0) { + close(fd); + free(copy); + return -1; + } + close(fd); + fd = next; + } + free(copy); + return fd; +} + int file_open_secure_parent(const char* path, char** leaf_out, bool create_dirs) { char* copy = str_dup(path); if (!copy) @@ -619,16 +626,13 @@ int file_open_secure_parent(const char* path, char** leaf_out, bool create_dirs) (resolved[strlen(root)] == '/' || resolved[strlen(root)] == '\0')) { struct stat rst; if (stat(resolved, &rst) == 0 && S_ISDIR(rst.st_mode)) { - /* Re-open the resolved directory WITHOUT following a symlink and - re-verify it is still a directory inode, so a symlink swapped - in between realpath() and open() (TOCTOU) cannot redirect this - fd outside the root. */ - next = open(resolved, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); - struct stat ofst; - if (next >= 0 && (fstat(next, &ofst) != 0 || !S_ISDIR(ofst.st_mode))) { - close(next); - next = -1; - } + /* Open the resolved directory through a relative no-follow walk + from the authorized-root fd instead of re-opening the + absolute `resolved` path: swapping an intermediate directory + for a symlink between realpath() and open() (TOCTOU) then + merely fails the walk rather than redirecting the fd outside + the root. */ + next = open_dir_beneath_root(resolved, root); } } } @@ -926,9 +930,12 @@ static bool file_to_disk_secure_impl(const char* path, const void* data, int prealloc_rc = 0; if (preallocate && !sparse && data_size > 0) { prealloc_rc = preallocate_fd(fd, data_size); - if (prealloc_rc != 0) - log_message(LOG_LEVEL_ERROR, "preallocate failed for '%s' (%s); transfer aborted", path, - strerror(prealloc_rc)); + if (prealloc_rc != 0) { + char* escaped_path = output_escape(path, log_get_8_bit_output()); + log_message(LOG_LEVEL_ERROR, "preallocate failed for '%s' (%s); transfer aborted", + escaped_path ? escaped_path : "", strerror(prealloc_rc)); + free(escaped_path); + } } if (prealloc_rc == 0) { /* posix_fallocate does not guarantee the fd's file offset is left @@ -938,7 +945,7 @@ static bool file_to_disk_secure_impl(const char* path, const void* data, ok = ftruncate(fd, (off_t)data_size) == 0; if (ok || !sparse || data_size == 0) ok = sparse && data_size > 0 - ? write_all_sparse(fd, (const unsigned char*)data, data_size) + ? file_store_write_sparse(fd, (const unsigned char*)data, data_size) : write_all(fd, data, data_size); if (ok) ok = ftruncate(fd, (off_t)data_size) == 0; @@ -1033,9 +1040,12 @@ static bool file_to_disk_secure_impl(const char* path, const void* data, int prealloc_rc = 0; if (preallocate && !sparse && data_size > 0) { prealloc_rc = preallocate_fd(fd, data_size); - if (prealloc_rc != 0) - log_message(LOG_LEVEL_ERROR, "preallocate failed for '%s' (%s); transfer aborted", path, - strerror(prealloc_rc)); + if (prealloc_rc != 0) { + char* escaped_path = output_escape(path, log_get_8_bit_output()); + log_message(LOG_LEVEL_ERROR, "preallocate failed for '%s' (%s); transfer aborted", + escaped_path ? escaped_path : "", strerror(prealloc_rc)); + free(escaped_path); + } } if (prealloc_rc == 0) { lseek(fd, 0, SEEK_SET); @@ -1046,8 +1056,9 @@ static bool file_to_disk_secure_impl(const char* path, const void* data, failure may leave partial data that --partial retention can rename. */ if (ok || (!sparse || data_size == 0)) { write_attempted = true; - ok = sparse && data_size > 0 ? write_all_sparse(fd, (const unsigned char*)data, data_size) - : write_all(fd, data, data_size); + ok = sparse && data_size > 0 + ? file_store_write_sparse(fd, (const unsigned char*)data, data_size) + : write_all(fd, data, data_size); } if (ok && metadata) ok = file_restore_metadata_fd(fd, metadata, preserve_executability); diff --git a/src/shared/file_receive.c b/src/shared/file_receive.c index 7059596..d29bf77 100644 --- a/src/shared/file_receive.c +++ b/src/shared/file_receive.c @@ -350,7 +350,10 @@ static FileSaveResult file_save_special_to_disk(const char* root_directory, cons } if (is_sock) { /* No standard filesystem call recreates a socket; best-effort unsupported. */ - log_message(LOG_LEVEL_WARNING, "socket not recreated: %s (unsupported; skipped)", file->path); + char* escaped_path = output_escape(file->path, log_get_8_bit_output()); + log_message(LOG_LEVEL_WARNING, "socket not recreated: %s (unsupported; skipped)", + escaped_path ? escaped_path : ""); + free(escaped_path); return FILE_SAVE_SKIPPED; } if (is_char || is_blk) { @@ -363,9 +366,11 @@ static FileSaveResult file_save_special_to_disk(const char* root_directory, cons so the policy does not depend on a prior identity_set_active(). Pure FIFO creation is unprivileged and deliberately NOT gated here. */ if (!privilege_super_mode_permitted(config->super_mode)) { + char* escaped_path = output_escape(file->path, log_get_8_bit_output()); log_message(LOG_LEVEL_WARNING, "skipping %s: super-user device-node creation is not permitted on this receiver", - file->path); + escaped_path ? escaped_path : ""); + free(escaped_path); return FILE_SAVE_SKIPPED; } } else if (is_fifo) { @@ -438,18 +443,26 @@ static FileSaveResult file_save_special_to_disk(const char* root_directory, cons free(destination); return FILE_SAVE_SKIPPED; } + char* escaped_path = output_escape(file->path, log_get_8_bit_output()); log_message(LOG_LEVEL_WARNING, "refusing to replace existing entry with %s: %s (skipped)", - is_fifo ? "FIFO" : "device", file->path); + is_fifo ? "FIFO" : "device", escaped_path ? escaped_path : ""); + free(escaped_path); } else if (errno == EPERM || errno == EACCES) { /* Missing CAP_MKNOD / parent write permission: the environment cannot create the node, so skip instead of failing the whole run. */ + char* escaped_path = output_escape(file->path, log_get_8_bit_output()); log_message(LOG_LEVEL_WARNING, "skipping %s: cannot create %s node (%s)\n" " --devices/--specials node creation needs privilege (CAP_MKNOD)", - file->path, is_fifo ? "FIFO" : "device", strerror(errno)); + escaped_path ? escaped_path : "", is_fifo ? "FIFO" : "device", + strerror(errno)); + free(escaped_path); } else { + char* escaped_path = output_escape(file->path, log_get_8_bit_output()); log_message(LOG_LEVEL_WARNING, "failed to create %s %s: %s (skipped)", - is_fifo ? "FIFO" : "device", file->path, strerror(errno)); + is_fifo ? "FIFO" : "device", escaped_path ? escaped_path : "", + strerror(errno)); + free(escaped_path); } close(parent_fd); free(leaf); @@ -512,22 +525,28 @@ static FileSaveResult file_save_write_device(const char* root_directory, const F close(parent_fd); if (fd < 0) { free(destination); + char* escaped_path = output_escape(file->path, log_get_8_bit_output()); + const char* shown_path = escaped_path ? escaped_path : ""; if (saved_errno == ENXIO || saved_errno == EAGAIN) { /* A FIFO with no reader / an unreadable special: skip like every other unusable write-devices target instead of blocking or failing. */ - log_message(LOG_LEVEL_WARNING, "write-devices: %s not writable (%s); skipped", file->path, + log_message(LOG_LEVEL_WARNING, "write-devices: %s not writable (%s); skipped", shown_path, strerror(saved_errno)); } else { - log_message(LOG_LEVEL_WARNING, "write-devices: cannot open %s (%s); skipped", file->path, + log_message(LOG_LEVEL_WARNING, "write-devices: cannot open %s (%s); skipped", shown_path, strerror(saved_errno)); } + free(escaped_path); return FILE_SAVE_SKIPPED; } struct stat st; if (fstat(fd, &st) != 0 || !(S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode))) { close(fd); free(destination); - log_message(LOG_LEVEL_WARNING, "write-devices: %s is not a device node; skipped", file->path); + char* escaped_path = output_escape(file->path, log_get_8_bit_output()); + log_message(LOG_LEVEL_WARNING, "write-devices: %s is not a device node; skipped", + escaped_path ? escaped_path : ""); + free(escaped_path); return FILE_SAVE_SKIPPED; } bool ok = true; @@ -596,10 +615,12 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi open below keeps its own confinement and best-effort skip semantics). */ if (config && config->write_devices) { if (!privilege_super_mode_permitted(config->super_mode)) { + char* escaped_path = output_escape(file->path, log_get_8_bit_output()); log_message(LOG_LEVEL_WARNING, "write-devices: %s skipped: super-user activities are not permitted on this " "receiver", - file->path ? file->path : "(null)"); + escaped_path ? escaped_path : "(null)"); + free(escaped_path); return FILE_SAVE_SKIPPED; } return file_save_write_device(root_directory, file); diff --git a/src/shared/file_store.c b/src/shared/file_store.c index 842e15e..b14da25 100644 --- a/src/shared/file_store.c +++ b/src/shared/file_store.c @@ -152,7 +152,7 @@ static bool write_all(int fd, const void* data, unsigned long long size) { * would). After the final run, ftruncate(size) guarantees the logical size is * exactly `size` even when the tail was a hole. The full file image is in * memory, so no wire change is needed. Returns false on I/O error. */ -static bool write_all_sparse(int fd, const unsigned char* data, unsigned long long size) { +bool file_store_write_sparse(int fd, const unsigned char* data, unsigned long long size) { unsigned long long i = 0; while (i < size) { if (data[i] == 0) { @@ -191,7 +191,7 @@ bool file_store_write_secure(const char* path, const void* data, unsigned long l if (fd >= 0) { if (sparse && data_size > 0) { if (ftruncate(fd, (off_t)data_size) == 0) - ok = write_all_sparse(fd, data, data_size); + ok = file_store_write_sparse(fd, data, data_size); } else { ok = write_all(fd, data, data_size); } @@ -219,8 +219,9 @@ bool file_store_write_secure(const char* path, const void* data, unsigned long l if (sparse && data_size > 0) ok = ftruncate(fd, (off_t)data_size) == 0; if (ok || (!sparse || data_size == 0)) - ok = (sparse && data_size > 0) ? write_all_sparse(fd, (const unsigned char*)data, data_size) - : write_all(fd, data, data_size); + ok = (sparse && data_size > 0) + ? file_store_write_sparse(fd, (const unsigned char*)data, data_size) + : write_all(fd, data, data_size); if (ok && metadata) ok = file_restore_metadata_fd(fd, metadata, preserve_executability); if (close(fd) != 0) diff --git a/src/shared/file_store.h b/src/shared/file_store.h index 743cc96..4bfb39a 100644 --- a/src/shared/file_store.h +++ b/src/shared/file_store.h @@ -10,5 +10,12 @@ bool file_store_rename_secure(const char* old_path, const char* new_path); bool file_store_write_secure(const char* path, const void* data, unsigned long long data_size, bool inplace, bool sparse, const FileMetadata* metadata, bool preserve_executability); +/* Sparse-aware write (--sparse/-S): every all-zero run of at least + * SPARSE_HOLE_MIN bytes is skipped with lseek(SEEK_CUR) so it becomes a real + * hole; every other byte is written. The caller pre-sizes the file with + * ftruncate; this function also ftruncate()s to `size` at the end so a trailing + * hole keeps the exact logical length. Shared by the file_store and file write + * paths. Returns false on write/lseek/ftruncate error. */ +bool file_store_write_sparse(int fd, const unsigned char* data, unsigned long long size); #endif diff --git a/src/shared/identity.c b/src/shared/identity.c index b30f8d7..5a39f0e 100644 --- a/src/shared/identity.c +++ b/src/shared/identity.c @@ -64,10 +64,10 @@ void identity_clear_active(void) { identity_active_reset(); } -void identity_set_active(const Config* config) { +bool identity_set_active(const Config* config) { identity_active_reset(); if (!config) - return; + return true; g_identity.numeric_ids = config->numeric_ids; g_identity.chown_uid_set = config->chown_uid_set; g_identity.chown_uid = config->chown_uid; @@ -79,19 +79,19 @@ void identity_set_active(const Config* config) { g_identity.copy_as_gid = config->copy_as_gid; if (config->usermap_count > 0) { g_identity.usermap = calloc((size_t)config->usermap_count, sizeof(IdentityMap)); - if (g_identity.usermap) { - memcpy(g_identity.usermap, config->usermap, - (size_t)config->usermap_count * sizeof(IdentityMap)); - g_identity.usermap_count = config->usermap_count; - } + if (!g_identity.usermap) + goto alloc_failed; + memcpy(g_identity.usermap, config->usermap, + (size_t)config->usermap_count * sizeof(IdentityMap)); + g_identity.usermap_count = config->usermap_count; } if (config->groupmap_count > 0) { g_identity.groupmap = calloc((size_t)config->groupmap_count, sizeof(IdentityMap)); - if (g_identity.groupmap) { - memcpy(g_identity.groupmap, config->groupmap, - (size_t)config->groupmap_count * sizeof(IdentityMap)); - g_identity.groupmap_count = config->groupmap_count; - } + if (!g_identity.groupmap) + goto alloc_failed; + memcpy(g_identity.groupmap, config->groupmap, + (size_t)config->groupmap_count * sizeof(IdentityMap)); + g_identity.groupmap_count = config->groupmap_count; } g_identity.set = true; /* A root receiver would honor any client-supplied ownership request (a @@ -113,6 +113,15 @@ void identity_set_active(const Config* config) { "--super requested but the receiver is not privileged; super-user " "activities (ownership, device nodes) will be attempted but refused " "by the kernel and skipped per entry"); + return true; + +alloc_failed: + /* Never proceed with a partial (count-left-zero) map: that would silently + apply the WRONG ownership policy. Fail closed and let the caller refuse + the connection. */ + log_message(LOG_LEVEL_ERROR, "memory allocation failed while activating identity policy"); + identity_active_reset(); + return false; } bool privilege_super_permitted(void) { @@ -440,6 +449,25 @@ static bool identity_id_fits_int32(unsigned long id) { return id <= (unsigned long)INT32_MAX; } +/* Resolve one --copy-as id token. A '*' token means the caller's current + * effective uid (user) or gid (group). Returns 0 on success. On failure sets + * *overflow when a '*' id was wider than int32 so the caller can log the + * specific message; otherwise the token was simply unresolvable. */ +static int identity_resolve_copy_as_id(const char* token, bool is_group, int32_t* out, + bool* overflow) { + *overflow = false; + if (strcmp(token, "*") == 0) { + unsigned long current = is_group ? (unsigned long)getegid() : (unsigned long)geteuid(); + if (!identity_id_fits_int32(current)) { + *overflow = true; + return -1; + } + *out = (int32_t)current; + return 0; + } + return identity_resolve_token(token, is_group, out); +} + int identity_parse_copy_as(Config* config, const char* value) { if (!config || !value || *value == '\0') { log_message(LOG_LEVEL_ERROR, "--copy-as requires USER[:GROUP]"); @@ -465,7 +493,7 @@ int identity_parse_copy_as(Config* config, const char* value) { log_message(LOG_LEVEL_ERROR, "memory allocation failed for --copy-as"); return -1; } - char* user_token = spec; + const char* user_token = spec; const char* group_token = NULL; char* colon = strchr(spec, ':'); if (colon) { @@ -477,57 +505,39 @@ int identity_parse_copy_as(Config* config, const char* value) { * (8-bit-safe) so a control byte cannot forge a log line. */ char* escaped_spec = output_escape(value, false); const char* shown = escaped_spec ? escaped_spec : ""; + int ret = -1; - int32_t uid; if (*user_token == '\0') { log_message(LOG_LEVEL_ERROR, "--copy-as is missing the user (got '%s')", shown); - free(escaped_spec); - free(spec); - return -1; + goto done; } - if (strcmp(user_token, "*") == 0) { - /* '*' means the current/root user: the client's euid. */ - if (!identity_id_fits_int32((unsigned long)geteuid())) { + bool overflow = false; + int32_t uid; + if (identity_resolve_copy_as_id(user_token, false, &uid, &overflow) != 0) { + if (overflow) log_message(LOG_LEVEL_ERROR, "--copy-as: current user id %lu exceeds INT32_MAX", (unsigned long)geteuid()); - free(escaped_spec); - free(spec); - return -1; - } - uid = (int32_t)geteuid(); - } else if (identity_resolve_token(user_token, false, &uid) != 0) { - log_message(LOG_LEVEL_ERROR, - "--copy-as could not resolve user (use a name that exists on the " - "source, '*', or @N): %s", - shown); - free(escaped_spec); - free(spec); - return -1; + else + log_message(LOG_LEVEL_ERROR, + "--copy-as could not resolve user (use a name that exists on the " + "source, '*', or @N): %s", + shown); + goto done; } int32_t gid; if (group_token) { if (*group_token == '\0') { log_message(LOG_LEVEL_ERROR, "--copy-as group is empty (got '%s')", shown); - free(escaped_spec); - free(spec); - return -1; + goto done; } - if (strcmp(group_token, "*") == 0) { - if (!identity_id_fits_int32((unsigned long)getegid())) { + if (identity_resolve_copy_as_id(group_token, true, &gid, &overflow) != 0) { + if (overflow) log_message(LOG_LEVEL_ERROR, "--copy-as: current group id %lu exceeds INT32_MAX", (unsigned long)getegid()); - free(escaped_spec); - free(spec); - return -1; - } - gid = (int32_t)getegid(); - } else if (identity_resolve_token(group_token, true, &gid) != 0) { - log_message(LOG_LEVEL_ERROR, "--copy-as could not resolve group (got '%s'): %s", shown, - shown); - free(escaped_spec); - free(spec); - return -1; + else + log_message(LOG_LEVEL_ERROR, "--copy-as could not resolve group (got '%s')", shown); + goto done; } } else { /* Group omitted: use the user's primary gid. A numeric id with no local @@ -539,9 +549,7 @@ int identity_parse_copy_as(Config* config, const char* value) { log_message(LOG_LEVEL_ERROR, "--copy-as: primary group id %lu for the requested user exceeds INT32_MAX", (unsigned long)pw->pw_gid); - free(escaped_spec); - free(spec); - return -1; + goto done; } gid = (int32_t)pw->pw_gid; } else { @@ -553,12 +561,8 @@ int identity_parse_copy_as(Config* config, const char* value) { * identity_resolve_token. */ if (uid < 0 || gid < 0) { log_message(LOG_LEVEL_ERROR, "--copy-as resolved id does not fit in int32 (got '%s')", shown); - free(escaped_spec); - free(spec); - return -1; + goto done; } - free(escaped_spec); - free(spec); config->copy_as_set = true; config->copy_as_uid = uid; @@ -566,7 +570,12 @@ int identity_parse_copy_as(Config* config, const char* value) { /* Ownership application needs the metadata path (the source uid/gid must be * transmitted); imply it exactly like --chown/--usermap/--groupmap. */ config->use_metadata = true; - return 0; + ret = 0; + +done: + free(escaped_spec); + free(spec); + return ret; } /* ---- Receiver-side ownership application ---- */ diff --git a/src/shared/identity.h b/src/shared/identity.h index 8b07e0c..592c5e7 100644 --- a/src/shared/identity.h +++ b/src/shared/identity.h @@ -67,8 +67,13 @@ bool identity_copy_as_active(void); /* Receiver-side snapshot of the negotiated identity config. The server calls * identity_set_active() once per connection (before any file write) using the * config received over the wire; the snapshot is a deep copy so the caller may - * free its Config immediately. identity_clear_active() releases it. */ -void identity_set_active(const Config* config); + * free its Config immediately. identity_clear_active() releases it. + * + * Returns true on success. On an allocation failure while deep-copying a + * requested usermap/groupmap it logs a LOG_LEVEL_ERROR, leaves the snapshot + * cleared (never a partial/wrong policy) and returns false; the caller must + * refuse the connection. */ +bool identity_set_active(const Config* config); void identity_clear_active(void); /* True when any ownership-affecting identity option is present in the active diff --git a/src/shared/protocol.c b/src/shared/protocol.c index 8f9b7f1..ee70d87 100644 --- a/src/shared/protocol.c +++ b/src/shared/protocol.c @@ -430,10 +430,14 @@ static bool protocol_send_str_impl(ProtocolSession* session, const char* data, b return false; if (!protocol_send_n_data(session, data, size)) return false; - if (redact) + if (redact) { log_debug_message(LOG_DEBUG_PROTO, "Send String: "); - else - log_debug_message(LOG_DEBUG_PROTO, "Send String: %s", data); + } else { + char* escaped_data = output_escape(data, log_get_8_bit_output()); + log_debug_message(LOG_DEBUG_PROTO, "Send String: %s", + escaped_data ? escaped_data : ""); + free(escaped_data); + } return true; } @@ -459,10 +463,14 @@ static char* protocol_receive_str_impl(ProtocolSession* session, bool redact) { return NULL; } data[size] = '\0'; - if (redact) + if (redact) { log_debug_message(LOG_DEBUG_PROTO, "Received String: "); - else - log_debug_message(LOG_DEBUG_PROTO, "Received String: %s", data); + } else { + char* escaped_data = output_escape(data, log_get_8_bit_output()); + log_debug_message(LOG_DEBUG_PROTO, "Received String: %s", + escaped_data ? escaped_data : ""); + free(escaped_data); + } return data; } diff --git a/src/shared/transport_ssh.c b/src/shared/transport_ssh.c index bba73a7..b5010ac 100644 --- a/src/shared/transport_ssh.c +++ b/src/shared/transport_ssh.c @@ -84,32 +84,29 @@ char* ssh_build_remote_command(const char* server_path, bool old_args, char* con const char* suffix = " --stdio"; /* Each --remote-option=OPT is appended after " --stdio" as one shell word, - escaped with the SAME single-quote boundary used for the server path. This - stays safe even in --old-args mode (which leaves the server path unquoted): - remote options are always single-quoted individually, so a value containing - shell metacharacters (; & | ` $ ()) can never break out of the quoting to - inject an unrelated remote command. Values are already validated at CLI - parse time (non-empty, no control characters); this layer only adds the - escaping boundary. */ + escaped with the SAME single-quote boundary used for the server path, so a + value containing shell metacharacters (; & | ` $ ()) can never break out of + the quoting to inject an unrelated remote command. Values are already + validated at CLI parse time (non-empty, no control characters); this layer + only adds the escaping boundary. */ size_t path_len = strlen(path); size_t suffix_len = strlen(suffix); - /* The base command (server path, quoted unless --old-args, then " --stdio"). */ - size_t command_len; - if (old_args) { - if (path_len > SIZE_MAX - suffix_len - 1) - return NULL; - command_len = path_len + suffix_len + 1; - } else { - size_t quote_count = 0; - for (const char* p = path; *p; p++) - if (*p == '\'') - quote_count++; - if (path_len > SIZE_MAX - suffix_len - 4 || - quote_count > (SIZE_MAX - path_len - suffix_len - 4) / 4) - return NULL; - command_len = path_len + quote_count * 4 + suffix_len + 4; - } + /* The base command: the server path is ALWAYS quoted as one single-quoted + shell word (remote options below reuse the same escaping), then + " --stdio". Quoting the path is the only injection-safe construction: an + unquoted path would carry shell metacharacters straight into the remote + shell command. --old-args is kept for CLI/ABI compatibility but no longer + disables that protection. */ + (void)old_args; + size_t quote_count = 0; + for (const char* p = path; *p; p++) + if (*p == '\'') + quote_count++; + if (path_len > SIZE_MAX - suffix_len - 4 || + quote_count > (SIZE_MAX - path_len - suffix_len - 4) / 4) + return NULL; + size_t command_len = path_len + quote_count * 4 + suffix_len + 4; /* Add each remote option, escaped as one single-quoted word: " ''", i.e. 1 leading space + 1 open quote + body (len + 3 per @@ -141,25 +138,18 @@ char* ssh_build_remote_command(const char* server_path, bool old_args, char* con if (!command) return NULL; char* out = command; - if (old_args) { - memcpy(out, path, path_len); - out += path_len; - memcpy(out, suffix, suffix_len + 1); - out += suffix_len; - } else { - *out++ = '\''; - for (const char* p = path; *p; p++) { - if (*p == '\'') { - memcpy(out, "'\\''", 4); - out += 4; - } else { - *out++ = *p; - } + *out++ = '\''; + for (const char* p = path; *p; p++) { + if (*p == '\'') { + memcpy(out, "'\\''", 4); + out += 4; + } else { + *out++ = *p; } - *out++ = '\''; - memcpy(out, suffix, suffix_len + 1); - out += suffix_len; } + *out++ = '\''; + memcpy(out, suffix, suffix_len + 1); + out += suffix_len; for (int i = 0; i < remote_option_count; i++) { const char* opt = remote_options[i]; *out++ = ' '; diff --git a/src/shared/transport_ssh.h b/src/shared/transport_ssh.h index 7458961..08908ce 100644 --- a/src/shared/transport_ssh.h +++ b/src/shared/transport_ssh.h @@ -6,10 +6,13 @@ Client* client_connect_ssh(const char* destination, int port, const char* server_path, bool old_args, const char* rsh_command, bool blocking_io, char* const* remote_options, int remote_option_count); -/* Build the escaped remote-shell command string (the server program path quoted - * as one remote-shell word unless --old-args, followed by ` --stdio` and each +/* Build the escaped remote-shell command string (the server program path always + * quoted as one remote-shell word, followed by ` --stdio` and each * --remote-option value appended as an individually single-quoted shell word). - * Every --remote-option value is individually escaped with the '\'' sequence and + * `old_args` is accepted for CLI/ABI compatibility but no longer disables + * quoting: the path is always escaped so a metacharacter-bearing + * --rsync-path can never be interpreted by the remote shell. Every + * --remote-option value is individually escaped with the '\'' sequence and * values with empty/control characters are rejected at the CLI parse layer. */ char* ssh_build_remote_command(const char* server_path, bool old_args, char* const* remote_options, int remote_option_count); diff --git a/src/shared/transport_tls.c b/src/shared/transport_tls.c index c5f0df4..85bb5a1 100644 --- a/src/shared/transport_tls.c +++ b/src/shared/transport_tls.c @@ -48,6 +48,15 @@ static SSL_CTX* create_ssl_ctx(bool is_server, const char* cert, const char* key return NULL; } + /* Harden the context: never negotiate TLS compression (the CRIME attack + * vector) and never honour a post-handshake renegotiation request. + * SSL_OP_NO_RENEGOTIATION is only available from OpenSSL 1.1.1, so it is + * guarded to keep older headers building. */ + SSL_CTX_set_options(ctx, SSL_OP_NO_COMPRESSION); +#ifdef SSL_OP_NO_RENEGOTIATION + SSL_CTX_set_options(ctx, SSL_OP_NO_RENEGOTIATION); +#endif + if (SSL_CTX_set_min_proto_version(ctx, TLS1_2_VERSION) != 1) { SSL_CTX_free(ctx); return NULL; diff --git a/tests/fuzz/fuzz_config_receive.c b/tests/fuzz/fuzz_config_receive.c new file mode 100644 index 0000000..082359a --- /dev/null +++ b/tests/fuzz/fuzz_config_receive.c @@ -0,0 +1,241 @@ +/* + * Fuzz the binary config-frame receive path: Config* config_receive(int fd). + * + * The frame is a length-prefixed stream of strings/ints/bools, so the receiver + * stops at the first malformed field. Feeding raw fuzz bytes alone therefore + * almost never reaches the deep P8 trailing blocks (--super / --copy-as) or the + * identity-map block, because every preceding wire bool must be exactly 0 or 1. + * + * To exercise those paths we first build one canonical, fully-valid frame with + * the production sender and then feed the receiver four shapes: + * + * 1. raw : the raw fuzz bytes as the whole frame (version gate included). + * 2. general : the valid version-string prefix + the raw fuzz bytes, so the + * fuzzer can walk the early/core/selection blocks from arbitrary + * input while staying past the version gate. + * 3. tail : the valid frame up to its last P8_TAIL_BYTES (super_mode + + * copy-as presence/uid/gid) + the raw fuzz bytes, so the fuzzer + * directly mutates super_mode and the copy-as ids and truncates + * the tail at any byte. + * 4. map : the valid frame up to the --usermap count + the raw fuzz bytes, + * so the fuzzer directly drives the map count (huge/extreme) and + * the map entries. + * + * The canonical frame is captured by running config_send once, writing the + * frame into a pipe whose read end is drained afterwards; the STATUS_OK ack is + * pre-loaded into a second pipe so a single thread suffices. + */ +#include "config.h" +#include "protocol.h" +#include "utils.h" +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* super_mode (4) + copy-as presence (4) + uid (4) + gid (4) = the P8 tail. */ +#define P8_TAIL_BYTES 16 + +/* Distinctive --usermap entry used to locate the map-count field in the + * canonical frame without duplicating the wire layout here. */ +#define MAP_FROM 0x11223344 +#define MAP_TO 0x55667788 + +static unsigned char* g_frame; +static size_t g_frame_len; +static size_t g_version_len; /* length of the leading version-string frame */ +static size_t g_usermap_count_off; /* offset of the usermap count int, 0 = unknown */ +static bool g_frame_ready; + +/* Read the canonical frame from the send peer. The producer shuts down its + * write half first, so a blocking read drains the frame and then sees EOF. */ +static unsigned char* drain_frame(int fd, size_t* out_len) { + size_t cap = 4096; + size_t len = 0; + unsigned char* buf = malloc(cap); + if (!buf) + return NULL; + for (;;) { + if (len == cap) { + size_t grown = cap * 2; + unsigned char* bigger = realloc(buf, grown); + if (!bigger) { + free(buf); + return NULL; + } + buf = bigger; + cap = grown; + } + ssize_t n = read(fd, buf + len, cap - len); + if (n > 0) { + len += (size_t)n; + continue; + } + if (n < 0 && errno == EINTR) + continue; + break; /* 0 (EOF) or error */ + } + *out_len = len; + return buf; +} + +/* Serialize a valid Config with the real sender. The frame is written into a + * pipe (64 KiB kernel buffer, far larger than one config frame) whose read end + * is drained afterwards; the STATUS_OK ack is pre-loaded into a second pipe so + * a single thread suffices (config_send writes the whole frame before it reads + * the ack). */ +static void build_canonical_frame(void) { + g_frame_ready = true; + + Config* cfg = config_create(); + if (!cfg) + return; + cfg->send_directory = str_dup("/src"); + cfg->receive_root_directory = str_dup("/dst"); + /* Force the three P8 tail fields to be present (copy-as requires metadata). */ + cfg->copy_as_set = true; + cfg->copy_as_uid = 0; + cfg->copy_as_gid = 0; + cfg->use_metadata = true; + /* Force one usermap entry with a locatable sentinel. */ + cfg->usermap = malloc(sizeof(IdentityMap)); + if (cfg->usermap) { + cfg->usermap_count = 1; + cfg->usermap[0].from = MAP_FROM; + cfg->usermap[0].to = MAP_TO; + } + if (!cfg->send_directory || !cfg->receive_root_directory || !cfg->usermap) { + config_delete(cfg); + return; + } + + int frame_pipe[2] = {-1, -1}; + int status_pipe[2] = {-1, -1}; + if (pipe(frame_pipe) != 0 || pipe(status_pipe) != 0) + goto out; + + int ack = STATUS_OK; + if (write(status_pipe[1], &ack, sizeof(ack)) != (ssize_t)sizeof(ack)) + goto out; + + io_set_fds(status_pipe[0], frame_pipe[1]); + io_set_bwlimit(0); + bool sent = config_send(frame_pipe[1], cfg); + close(frame_pipe[1]); + frame_pipe[1] = -1; + close(status_pipe[0]); + status_pipe[0] = -1; + close(status_pipe[1]); + status_pipe[1] = -1; + + if (sent) + g_frame = drain_frame(frame_pipe[0], &g_frame_len); + +out: + if (frame_pipe[0] != -1) + close(frame_pipe[0]); + if (frame_pipe[1] != -1) + close(frame_pipe[1]); + if (status_pipe[0] != -1) + close(status_pipe[0]); + if (status_pipe[1] != -1) + close(status_pipe[1]); + config_delete(cfg); + if (!g_frame || g_frame_len == 0) { + free(g_frame); + g_frame = NULL; + g_frame_len = 0; + return; + } + + g_version_len = sizeof(size_t) + strlen(PROTOCOL_VERSION); + if (g_version_len > g_frame_len) + g_version_len = g_frame_len; + + /* Locate the usermap entry sentinel; its count int sits 4 bytes before it. */ + int32_t from = MAP_FROM; + int32_t to = MAP_TO; + unsigned char pattern[8]; + memcpy(pattern, &from, sizeof(from)); + memcpy(pattern + sizeof(from), &to, sizeof(to)); + if (g_frame_len >= sizeof(pattern)) { + for (size_t i = 4; i + sizeof(pattern) <= g_frame_len; i++) { + if (memcmp(g_frame + i, pattern, sizeof(pattern)) == 0) { + g_usermap_count_off = i - sizeof(int32_t); + break; + } + } + } +} + +/* Best-effort non-blocking write: an oversized fuzz input is truncated rather + * than stalling the harness. */ +static void write_best_effort(int fd, const void* data, size_t size) { + const unsigned char* p = data; + size_t off = 0; + while (off < size) { + ssize_t n = write(fd, p + off, size - off); + if (n > 0) { + off += (size_t)n; + continue; + } + if (n < 0 && errno == EINTR) + continue; + break; + } +} + +/* Build prefix ++ data as a stream and drive config_receive over it. */ +static void receive_stream(const unsigned char* prefix, size_t prefix_len, const uint8_t* data, + size_t size) { + int sv[2]; + if (socketpair(AF_UNIX, SOCK_STREAM, 0, sv) != 0) + return; + + int flags = fcntl(sv[0], F_GETFL, 0); + if (flags != -1) + (void)fcntl(sv[0], F_SETFL, flags | O_NONBLOCK); + + if (prefix_len > 0) + write_best_effort(sv[0], prefix, prefix_len); + if (size > 0) + write_best_effort(sv[0], data, size); + /* Signal EOF without closing the read half, so the receiver's STATUS_ERROR + * replies do not hit EPIPE. */ + shutdown(sv[0], SHUT_WR); + + io_set_fds(sv[1], sv[1]); + io_set_bwlimit(0); + Config* cfg = config_receive(sv[1]); + config_delete(cfg); + + close(sv[0]); + close(sv[1]); +} + +int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { + if (!g_frame_ready) + build_canonical_frame(); + + /* Raw bytes as the whole frame (version gate and all). */ + receive_stream(NULL, 0, data, size); + + if (g_frame) { + /* Keep the valid version prefix, fuzz everything after it. */ + receive_stream(g_frame, g_version_len, data, size); + + /* Keep the valid frame up to the P8 tail, fuzz super_mode + copy-as. */ + if (g_frame_len > P8_TAIL_BYTES) + receive_stream(g_frame, g_frame_len - P8_TAIL_BYTES, data, size); + + /* Keep the valid frame up to the usermap count, fuzz the count + entries. */ + if (g_usermap_count_off > 0) + receive_stream(g_frame, g_usermap_count_off, data, size); + } + return 0; +} diff --git a/tests/fuzz/fuzz_identity_parse.c b/tests/fuzz/fuzz_identity_parse.c new file mode 100644 index 0000000..0b9f961 --- /dev/null +++ b/tests/fuzz/fuzz_identity_parse.c @@ -0,0 +1,58 @@ +/* + * Fuzz the CLI-time identity parsers (identity.h): + * - identity_parse_copy_as + * - identity_parse_map (user and group variants) + * - identity_parse_chown + * + * Each parser mutates a Config, so every input gets a fresh config_create() + * (freed afterwards). After a successful parse the shared wire validator and + * the ownership predicate are also exercised on the mutated config. The input + * is NUL-terminated; embedded NULs simply shorten the effective spec, which is + * fine for a parser fuzzer. + */ +#include "config.h" +#include "identity.h" +#include +#include +#include + +static void exercise(Config* c, const char* spec, int which) { + if (!c) + return; + switch (which) { + case 0: + (void)identity_parse_copy_as(c, spec); + break; + case 1: + (void)identity_parse_map(c, spec, false); + break; + case 2: + (void)identity_parse_map(c, spec, true); + break; + default: + (void)identity_parse_chown(c, spec); + break; + } + (void)identity_wire_valid(c); + (void)identity_ownership_requested(c); + config_delete(c); +} + +int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { + if (size == 0) + return 0; + + char* spec = malloc(size + 1); + if (!spec) + return 0; + memcpy(spec, data, size); + spec[size] = '\0'; + + exercise(config_create(), spec, 0); + exercise(config_create(), spec, 1); + exercise(config_create(), spec, 2); + exercise(config_create(), spec, 3); + + free(spec); + return 0; +} diff --git a/tests/integration/test_daemon.py b/tests/integration/test_daemon.py index d0781e0..a77d7bf 100644 --- a/tests/integration/test_daemon.py +++ b/tests/integration/test_daemon.py @@ -66,6 +66,7 @@ def _pw_hash(password): def _write_client_password_file(path, user, password): with open(path, "w") as f: f.write("%s:%s\n" % (user, password)) + os.chmod(path, 0o600) return path @@ -164,6 +165,7 @@ def daemon_env(): f.write("# daemon credential store (Wave B)\n") f.write("alice:%s\n" % _pw_hash(ALICE_PASS)) f.write("bob:%s\n" % _pw_hash(BOB_PASS)) + os.chmod(CRED_FILE, 0o600) # The config's port is a free port chosen per worker; the `daemon` fixture # boots on it (the config-port path) and the --dparam override test boots a @@ -381,6 +383,27 @@ class TestDaemonRejection: refusal happens at the config handshake, before any data lands.""" self._assert_ownership_refused(daemon, "files", ["--super", "--preserve"]) + def test_super_refused_by_no_super_daemon(self): + """A daemon started with the operator --no-super veto must still REFUSE + an explicit client --super on a non-opted module: the veto must not turn + the refusal into a silent accept.""" + port = _find_free_port() + d = DaemonManager() + log_path = os.path.join(TEST_DATA_DIR, "fastsyncd.log") + try: + d.start(CONF_FILE, port_override=port, + extra_args=["--password-file", CRED_FILE, "--no-super"]) + result, _ = run_client(SOURCE_DIR, "127.0.0.1::files", port=d.port, + flags=["--super", "--preserve"]) + assert result.returncode != 0, "the --no-super daemon must refuse --super" + with open(log_path, "rb") as f: + tail = f.read().decode("utf-8", "replace") + assert "client-chosen ownership" in tail, ( + f"daemon did not log the --super refusal: {tail[-400:]!r}" + ) + finally: + d.stop() + 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 @@ -582,6 +605,7 @@ class TestDaemonAuthentication: cred_path = os.path.join(TEST_DATA_DIR, "client_empty.pw") with open(cred_path, "w") as f: f.write("# nothing here\n") + os.chmod(cred_path, 0o600) try: cmd = CLIENT_CMD + ["--source-dir", SOURCE_DIR, "--dest-dir", "127.0.0.1::files", diff --git a/tests/test_client_cli.c b/tests/test_client_cli.c index 5bbbcef..cca77ee 100644 --- a/tests/test_client_cli.c +++ b/tests/test_client_cli.c @@ -104,18 +104,18 @@ static void test_cli_help() { config_delete(cfg); } -/* Test that --archive's config bundle matches rsync -rlptgoD semantics: - * links + metadata + devices + specials, and NOT compression/multithreading. */ +/* Test that the -a short spelling applies --archive's config bundle, matching + * rsync -rlptgoD semantics: links + metadata + devices + specials, and NOT + * compression/multithreading. (--archive itself is covered by + * test_parse_args_archive; this guards the short alias.) */ static void test_cli_archive_flags() { Config* cfg = config_create(); EXPECT_NOT_NULL(cfg); + char* argv[] = {"fastsync", "-a", "/src", "/dst"}; + int positional_args[2]; + int positional_count = 0; - /* Simulate the --archive flag's implied bundle. */ - cfg->follow_symlinks = true; - cfg->use_metadata = true; - cfg->preserve_devices = true; - cfg->preserve_specials = true; - + EXPECT_EQ_INT(parse_args(cfg, 4, argv, positional_args, &positional_count), 0); EXPECT_TRUE(cfg->follow_symlinks); EXPECT_TRUE(cfg->use_metadata); EXPECT_TRUE(cfg->preserve_devices); @@ -3045,13 +3045,30 @@ static void test_parse_args_block_size() { EXPECT_EQ_INT(parse_args(cfg, 4, argv_eq, positional_args, &positional_count), 0); EXPECT_EQ_INT((int)cfg->delta_block_size, 8192); - /* Out of range: parsed, warned, and the default is kept. */ + cfg->delta_block_size = DELTA_BLOCK_SIZE_DEFAULT; + char* argv_delta_eq[] = {"fastsync", "--delta-block=1024", "/src", "/dst"}; + positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 4, argv_delta_eq, positional_args, &positional_count), 0); + EXPECT_EQ_INT((int)cfg->delta_block_size, 1024); + + /* Out of range: parsed, warned, and the default is kept (both spellings). */ cfg->delta_block_size = DELTA_BLOCK_SIZE_DEFAULT; char* argv_bad[] = {"fastsync", "--block-size", "1", "/src", "/dst"}; positional_count = 0; EXPECT_EQ_INT(parse_args(cfg, 5, argv_bad, positional_args, &positional_count), 0); EXPECT_EQ_INT((int)cfg->delta_block_size, (int)DELTA_BLOCK_SIZE_DEFAULT); + cfg->delta_block_size = DELTA_BLOCK_SIZE_DEFAULT; + char* argv_bad_inline[] = {"fastsync", "--delta-block=999999", "/src", "/dst"}; + positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 4, argv_bad_inline, positional_args, &positional_count), 0); + EXPECT_EQ_INT((int)cfg->delta_block_size, (int)DELTA_BLOCK_SIZE_DEFAULT); + + /* A non-numeric value is a hard error for both spellings. */ + char* argv_nan[] = {"fastsync", "--delta-block=abc", "/src", "/dst"}; + positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 4, argv_nan, positional_args, &positional_count), -1); + /* A non-default block size changes the number of signature blocks for identical data: block_count = ceil(size / block_size). */ const char data[10000] = {0}; diff --git a/tests/test_config.c b/tests/test_config.c index 77a7ba0..ade0564 100644 --- a/tests/test_config.c +++ b/tests/test_config.c @@ -1883,13 +1883,13 @@ static void test_privilege_super_permitted_modes() { Config* c = config_create(); EXPECT_NOT_NULL(c); c->super_mode = SUPER_MODE_OFF; - identity_set_active(c); + EXPECT_TRUE(identity_set_active(c)); EXPECT_FALSE(privilege_super_permitted()); c->super_mode = SUPER_MODE_ON; - identity_set_active(c); + EXPECT_TRUE(identity_set_active(c)); EXPECT_TRUE(privilege_super_permitted()); c->super_mode = SUPER_MODE_AUTO; - identity_set_active(c); + EXPECT_TRUE(identity_set_active(c)); EXPECT_TRUE(privilege_super_permitted()); config_delete(c); @@ -1952,10 +1952,10 @@ static void test_super_does_not_imply_numeric() { EXPECT_NOT_NULL(c); c->super_mode = SUPER_MODE_ON; c->use_metadata = true; - identity_set_active(c); + EXPECT_TRUE(identity_set_active(c)); EXPECT_FALSE(identity_active_enabled()); c->numeric_ids = true; - identity_set_active(c); + EXPECT_TRUE(identity_set_active(c)); EXPECT_TRUE(identity_active_enabled()); identity_clear_active(); config_delete(c); diff --git a/tests/test_credentials.c b/tests/test_credentials.c index 10c6888..3ddcf06 100644 --- a/tests/test_credentials.c +++ b/tests/test_credentials.c @@ -33,6 +33,9 @@ static char* make_tmp_file(const char* contents) { return NULL; } fclose(fp); + /* Credential/password files are owner-only; the reader rejects group/other + * permission bits, so create temp files 0600 like the real ones. */ + chmod(path, 0600); return strdup(path); } @@ -353,6 +356,47 @@ static void test_credentials_gate_allows() { free(path); } +static void test_credentials_rejects_group_or_other_accessible() { + char err[512]; + char* path = + make_tmp_file("alice:9b90e524e94995ee4aeae2ee3c428a53405d1e8db147f44facc46797d0caf4c3\n"); + EXPECT_NOT_NULL(path); + + /* 0600 is accepted by the server store loader. */ + EXPECT_EQ_INT(chmod(path, 0600), 0); + CredentialStore* store = credentials_load(path, NULL, err, sizeof(err)); + EXPECT_NOT_NULL(store); + credentials_free(store); + + /* Group-readable and world-readable are both refused, with a clear error. */ + EXPECT_EQ_INT(chmod(path, 0640), 0); + EXPECT_NULL(credentials_load(path, NULL, err, sizeof(err))); + EXPECT_TRUE(strstr(err, "owner-only") != NULL); + EXPECT_EQ_INT(chmod(path, 0604), 0); + EXPECT_NULL(credentials_load(path, NULL, err, sizeof(err))); + + /* The client --password-file reader enforces the same rule. */ + EXPECT_EQ_INT(chmod(path, 0644), 0); + char* user = NULL; + char* password = NULL; + EXPECT_EQ_INT(credentials_read_secret_file(path, &user, &password, err, sizeof(err)), -1); + EXPECT_NULL(user); + EXPECT_NULL(password); + EXPECT_TRUE(strstr(err, "owner-only") != NULL); + + /* An --early-input file is checked too. */ + char* pw = + make_tmp_file("bob:2bb80d537b1da3e38bd30361aa855686bde0eacd7162fef6a25fe97bf527a25b\n"); + EXPECT_NOT_NULL(pw); + EXPECT_EQ_INT(chmod(path, 0644), 0); + EXPECT_NULL(credentials_load(pw, path, err, sizeof(err))); + + rm_temp(pw); + rm_temp(path); + free(pw); + free(path); +} + static void test_credentials_burn() { char secret[32]; memcpy(secret, "supersecretvalue", 17); @@ -374,6 +418,7 @@ void test_credentials(void) { test_credentials_early_input_merge(); test_credentials_read_secret_file(); test_credentials_read_secret_file_bad(); + test_credentials_rejects_group_or_other_accessible(); test_credentials_gate_allows(); test_credentials_burn(); } diff --git a/tests/test_file.c b/tests/test_file.c index 19deef1..fd715fc 100644 --- a/tests/test_file.c +++ b/tests/test_file.c @@ -1370,6 +1370,126 @@ static void test_dir_time_list() { rmdir(root); } +/* -K/--keep-dirlinks secure open: with an authorized root, a destination path + * component that is a symlink to an IN-ROOT directory is used as that directory + * (its referent is opened through a relative O_NOFOLLOW walk from the root fd, + * not by re-opening an absolute realpath() result), while a symlink resolving + * OUTSIDE the root is rejected. With -K off, even the in-root link is not + * followed. */ +static void test_keep_dirlinks_secure_open_impl() { + const char* root = "test_keep_dirlinks_root"; + const char* real = "test_keep_dirlinks_root/realdir"; + const char* link = "test_keep_dirlinks_root/linkdir"; + const char* abslink = "test_keep_dirlinks_root/abslink"; + const char* escape = "test_keep_dirlinks_root/escape"; + const char* outside = "test_keep_dirlinks_outside"; + unlink(link); + unlink(abslink); + unlink(escape); + rmdir(real); + rmdir(root); + rmdir(outside); + EXPECT_EQ_INT(mkdir(root, 0755), 0); + EXPECT_EQ_INT(mkdir(real, 0755), 0); + EXPECT_EQ_INT(mkdir(outside, 0755), 0); + + char root_abs[PATH_MAX]; + char real_abs[PATH_MAX]; + char outside_abs[PATH_MAX]; + EXPECT_NOT_NULL(realpath(root, root_abs)); + EXPECT_NOT_NULL(realpath(real, real_abs)); + EXPECT_NOT_NULL(realpath(outside, outside_abs)); + EXPECT_EQ_INT(symlink("realdir", link), 0); /* relative, in-root */ + EXPECT_EQ_INT(symlink(real_abs, abslink), 0); /* absolute, in-root */ + /* cppcheck-suppress knownConditionTrueFalse */ + EXPECT_EQ_INT(symlink(outside_abs, escape), 0); /* absolute, outside root */ + + int root_fd = open(root_abs, O_RDONLY | O_DIRECTORY | O_CLOEXEC); + EXPECT_TRUE(root_fd >= 0); + // cppcheck-suppress knownConditionTrueFalse + if (root_fd < 0) { + unlink(link); + unlink(abslink); + unlink(escape); + rmdir(real); + rmdir(root); + rmdir(outside); + return; + } + EXPECT_TRUE(file_set_authorized_root(root_fd, root_abs)); + file_set_keep_dirlinks(true); + + struct stat real_st; + EXPECT_EQ_INT(fstatat(root_fd, "realdir", &real_st, 0), 0); + + /* Relative in-root symlink-to-directory: followed to the referent dir. */ + char path[PATH_MAX + 64]; + snprintf(path, sizeof(path), "%s/linkdir/file.txt", root_abs); + char* leaf = NULL; + int parent_fd = file_open_secure_parent(path, &leaf, false); + EXPECT_TRUE(parent_fd >= 0); + EXPECT_NOT_NULL(leaf); + // cppcheck-suppress knownConditionTrueFalse + if (leaf) + EXPECT_EQ_STR(leaf, "file.txt"); + // cppcheck-suppress knownConditionTrueFalse + if (parent_fd >= 0) { + struct stat st; + EXPECT_EQ_INT(fstat(parent_fd, &st), 0); + EXPECT_TRUE(st.st_dev == real_st.st_dev && st.st_ino == real_st.st_ino); + close(parent_fd); + } + free(leaf); + + /* Absolute-but-in-root symlink-to-directory is followed the same way. */ + snprintf(path, sizeof(path), "%s/abslink/file.txt", root_abs); + leaf = NULL; + parent_fd = file_open_secure_parent(path, &leaf, false); + EXPECT_TRUE(parent_fd >= 0); + // cppcheck-suppress knownConditionTrueFalse + if (parent_fd >= 0) { + struct stat st; + EXPECT_EQ_INT(fstat(parent_fd, &st), 0); + EXPECT_TRUE(st.st_dev == real_st.st_dev && st.st_ino == real_st.st_ino); + close(parent_fd); + } + free(leaf); + + /* A symlink resolving outside the authorized root is rejected. */ + snprintf(path, sizeof(path), "%s/escape/file.txt", root_abs); + leaf = NULL; + EXPECT_EQ_INT(file_open_secure_parent(path, &leaf, false), -1); + free(leaf); + + /* With -K off the in-root symlink is not followed either. */ + file_set_keep_dirlinks(false); + snprintf(path, sizeof(path), "%s/linkdir/file.txt", root_abs); + leaf = NULL; + EXPECT_EQ_INT(file_open_secure_parent(path, &leaf, false), -1); + free(leaf); + + file_set_keep_dirlinks(false); + file_set_authorized_root(-1, NULL); + close(root_fd); + unlink(link); + unlink(abslink); + unlink(escape); + rmdir(real); + rmdir(root); + rmdir(outside); +} + +/* Wrapper guarantees the process-wide keep-dirlinks/authorized-root policy is + * cleared even when an EXPECT inside the body returns early (a failing EXPECT + * returns from its own function, so the body's trailing resets may be skipped). */ +static void test_keep_dirlinks_secure_open() { + file_set_authorized_root(-1, NULL); + file_set_keep_dirlinks(false); + test_keep_dirlinks_secure_open_impl(); + file_set_authorized_root(-1, NULL); + file_set_keep_dirlinks(false); +} + void test_file() { test_file_create(); test_file_special_rdev_valid(); @@ -1411,6 +1531,7 @@ void test_file() { } test_file_metadata_create(); test_dir_time_list(); + test_keep_dirlinks_secure_open(); test_inplace_overwrite_clears_special_mode_bits(); test_inplace_overwrite_metadata_strips_special_bits(); test_inplace_overwrite_truncates_shorter_payload(); diff --git a/tests/test_fuzz_smoke.c b/tests/test_fuzz_smoke.c index 9b2c415..50d6c29 100644 --- a/tests/test_fuzz_smoke.c +++ b/tests/test_fuzz_smoke.c @@ -1,16 +1,24 @@ #include "test_fuzz_smoke.h" #include "chunk.h" #include "compression.h" +#include "config.h" #include "data.h" #include "delta.h" #include "metadata.h" +#include "protocol.h" #include "test_utils.h" #include "utils.h" +#include +#include #include #include #include +#include #include +/* P8 config-frame tail: super_mode (4) + copy-as presence (4) + uid (4) + gid (4). */ +#define P8_TAIL_BYTES 16 + /* Smoke test for chunk_deserialize fuzz target */ static void test_fuzz_chunk_deserialize() { /* Create a minimal valid chunk to serialize and deserialize */ @@ -170,6 +178,272 @@ static void test_fuzz_glob_match() { EXPECT_FALSE(glob_match("*.md", "readme.txt")); } +/* ---- Deterministic config-frame receive hardening (P8) ---- + * + * The P8 tail (--super / --copy-as) and the identity-map count only parse after + * the entire preceding frame validates, which random bytes almost never reach. + * These tests capture one valid frame with the production sender and then + * mutate/truncate the exact tail bytes. */ + +/* Serialize cfg with the production sender into a heap buffer. The frame is + * written into a pipe (64 KiB kernel buffer, far larger than one config frame) + * whose read end is drained afterwards; the required STATUS_OK ack is + * pre-loaded into a second pipe, so a single thread suffices. */ +static bool capture_config_frame(const Config* cfg, unsigned char** out, size_t* out_len) { + *out = NULL; + *out_len = 0; + + int frame_pipe[2]; + int status_pipe[2]; + if (pipe(frame_pipe) != 0) + return false; + if (pipe(status_pipe) != 0) { + close(frame_pipe[0]); + close(frame_pipe[1]); + return false; + } + + int ack = STATUS_OK; + bool ok = write(status_pipe[1], &ack, sizeof(ack)) == (ssize_t)sizeof(ack); + if (ok) { + io_set_fds(status_pipe[0], frame_pipe[1]); + io_set_bwlimit(0); + ok = config_send(frame_pipe[1], cfg); + } + close(frame_pipe[1]); + close(status_pipe[0]); + close(status_pipe[1]); + + unsigned char* buf = NULL; + if (ok) { + size_t cap = 4096; + size_t len = 0; + buf = malloc(cap); + if (!buf) { + ok = false; + } + while (ok) { + if (len == cap) { + size_t grown = cap * 2; + unsigned char* bigger = realloc(buf, grown); + if (!bigger) { + ok = false; + break; + } + buf = bigger; + cap = grown; + } + ssize_t n = read(frame_pipe[0], buf + len, cap - len); + if (n > 0) { + len += (size_t)n; + continue; + } + if (n < 0 && errno == EINTR) + continue; + break; + } + if (ok && len > 0) { + *out = buf; + *out_len = len; + buf = NULL; + } + } + close(frame_pipe[0]); + free(buf); + return *out != NULL; +} + +/* Feed a raw config frame to config_receive over a socketpair. The write half + * is shut down (not closed) after the data so the receiver sees EOF but its + * STATUS_ERROR replies do not hit a closed peer. */ +static bool receive_config_frame(const unsigned char* buf, size_t len) { + int sv[2]; + if (socketpair(AF_UNIX, SOCK_STREAM, 0, sv) != 0) + return false; + + size_t off = 0; + while (off < len) { + ssize_t n = write(sv[0], buf + off, len - off); + if (n > 0) { + off += (size_t)n; + continue; + } + if (n < 0 && errno == EINTR) + continue; + break; + } + shutdown(sv[0], SHUT_WR); + io_set_fds(sv[1], sv[1]); + io_set_bwlimit(0); + Config* cfg = config_receive(sv[1]); + bool accepted = cfg != NULL; + config_delete(cfg); + close(sv[0]); + close(sv[1]); + return accepted; +} + +static void put_i32(unsigned char* buf, size_t off, int32_t value) { + memcpy(buf + off, &value, sizeof(value)); +} + +static size_t find_bytes(const unsigned char* haystack, size_t haystack_len, + const unsigned char* needle, size_t needle_len) { + if (needle_len == 0 || haystack_len < needle_len) + return SIZE_MAX; + for (size_t i = 0; i + needle_len <= haystack_len; i++) { + if (memcmp(haystack + i, needle, needle_len) == 0) + return i; + } + return SIZE_MAX; +} + +static Config* make_copy_as_config(void) { + Config* c = config_create(); + if (!c) + return NULL; + c->send_directory = str_dup("/src"); + c->receive_root_directory = str_dup("/dst"); + c->copy_as_set = true; + c->copy_as_uid = 0; + c->copy_as_gid = 0; + c->use_metadata = true; /* --copy-as requires the metadata path */ + return c; +} + +/* The P8 tail must reject an out-of-range super_mode, a negative copy-as id and + * any truncation inside the tail, while the untouched frame is accepted. */ +static void test_fuzz_config_receive_p8_tail() { + Config* c = make_copy_as_config(); + EXPECT_NOT_NULL(c); + + unsigned char* frame = NULL; + size_t len = 0; + bool captured = capture_config_frame(c, &frame, &len); + config_delete(c); + if (!captured || len <= P8_TAIL_BYTES) { + free(frame); + EXPECT_TRUE(false); + return; + } + + /* Baseline: the untouched frame is accepted. */ + EXPECT_TRUE(receive_config_frame(frame, len)); + + unsigned char* mut = malloc(len); + EXPECT_NOT_NULL(mut); + + /* super_mode outside the 0..2 tri-state is refused. */ + memcpy(mut, frame, len); + put_i32(mut, len - P8_TAIL_BYTES, 99); + EXPECT_FALSE(receive_config_frame(mut, len)); + put_i32(mut, len - P8_TAIL_BYTES, -1); + EXPECT_FALSE(receive_config_frame(mut, len)); + + /* A negative (sentinel) and an extreme copy-as uid/gid are refused. */ + memcpy(mut, frame, len); + put_i32(mut, len - P8_TAIL_BYTES, SUPER_MODE_AUTO); + put_i32(mut, len - P8_TAIL_BYTES + 4, 1); + put_i32(mut, len - P8_TAIL_BYTES + 8, -1); + put_i32(mut, len - P8_TAIL_BYTES + 12, 0); + EXPECT_FALSE(receive_config_frame(mut, len)); + put_i32(mut, len - P8_TAIL_BYTES + 8, 0); + put_i32(mut, len - P8_TAIL_BYTES + 12, INT32_MIN); + EXPECT_FALSE(receive_config_frame(mut, len)); + + /* A presence int that is not a wire bool is refused. */ + memcpy(mut, frame, len); + put_i32(mut, len - P8_TAIL_BYTES, SUPER_MODE_AUTO); + put_i32(mut, len - P8_TAIL_BYTES + 4, 2); + EXPECT_FALSE(receive_config_frame(mut, len)); + + /* Truncating anywhere inside the P8 tail is refused. */ + EXPECT_FALSE(receive_config_frame(frame, len - 2)); + EXPECT_FALSE(receive_config_frame(frame, len - P8_TAIL_BYTES)); + + free(mut); + free(frame); +} + +/* A huge or negative --usermap count must be refused up front, never driving a + * giant allocation. The count is located by searching for a sentinel entry. */ +static void test_fuzz_config_receive_huge_map_count() { + Config* c = make_copy_as_config(); + EXPECT_NOT_NULL(c); + int32_t sentinel_from = 0x11223344; + int32_t sentinel_to = 0x55667788; + c->usermap = malloc(sizeof(IdentityMap)); + if (!c->usermap) { + config_delete(c); + EXPECT_TRUE(false); + return; + } + c->usermap_count = 1; + c->usermap[0].from = sentinel_from; + c->usermap[0].to = sentinel_to; + + unsigned char* frame = NULL; + size_t len = 0; + bool captured = capture_config_frame(c, &frame, &len); + config_delete(c); + if (!captured) { + EXPECT_TRUE(false); + return; + } + + unsigned char pattern[8]; + memcpy(pattern, &sentinel_from, sizeof(sentinel_from)); + memcpy(pattern + sizeof(sentinel_from), &sentinel_to, sizeof(sentinel_to)); + size_t entry_off = find_bytes(frame, len, pattern, sizeof(pattern)); + if (entry_off == SIZE_MAX || entry_off < sizeof(int32_t)) { + free(frame); + EXPECT_TRUE(false); + return; + } + size_t count_off = entry_off - sizeof(int32_t); + + /* Baseline accepted. */ + EXPECT_TRUE(receive_config_frame(frame, len)); + + unsigned char* mut = malloc(len); + EXPECT_NOT_NULL(mut); + memcpy(mut, frame, len); + put_i32(mut, count_off, INT_MAX); + EXPECT_FALSE(receive_config_frame(mut, len)); + put_i32(mut, count_off, -1); + EXPECT_FALSE(receive_config_frame(mut, len)); + put_i32(mut, count_off, MAX_IDENTITY_MAP + 1); + EXPECT_FALSE(receive_config_frame(mut, len)); + + free(mut); + free(frame); +} + +/* A mismatched version and a matching version followed by a wrong-order field + * (an int that is not a wire bool) are both refused at/just after the gate. */ +static void test_fuzz_config_receive_version_gate() { + unsigned char buf[64]; + + size_t off = 0; + const char* bad_version = "1.2.3"; + size_t bad_len = strlen(bad_version); + memcpy(buf + off, &bad_len, sizeof(bad_len)); + off += sizeof(bad_len); + memcpy(buf + off, bad_version, bad_len); + off += bad_len; + EXPECT_FALSE(receive_config_frame(buf, off)); + + off = 0; + size_t good_len = strlen(PROTOCOL_VERSION); + memcpy(buf + off, &good_len, sizeof(good_len)); + off += sizeof(good_len); + memcpy(buf + off, PROTOCOL_VERSION, good_len); + off += good_len; + put_i32(buf, off, -1); + off += sizeof(int32_t); + EXPECT_FALSE(receive_config_frame(buf, off)); +} + void test_fuzz_smoke() { test_fuzz_chunk_deserialize(); test_fuzz_compress_decompress(); @@ -177,4 +451,7 @@ void test_fuzz_smoke() { test_fuzz_metadata_from_buf(); test_fuzz_delta_signature_deserialize(); test_fuzz_glob_match(); + test_fuzz_config_receive_p8_tail(); + test_fuzz_config_receive_huge_map_count(); + test_fuzz_config_receive_version_gate(); } diff --git a/tests/test_server.c b/tests/test_server.c index 6fc70ba..82b842e 100644 --- a/tests/test_server.c +++ b/tests/test_server.c @@ -2,6 +2,7 @@ #include "config.h" #include "delta.h" #include "file.h" +#include "log.h" #include "protocol.h" #include "test_utils.h" #include "utils.h" @@ -724,7 +725,44 @@ static void test_receiver_pending_commits_missing_args() { free(root); } +/* A6: an attacker-controlled file path appearing in a log line must be escaped + so a control byte cannot forge a second log record. The socket special-node + branch logs file->path before touching the filesystem, making it a cheap way + to exercise an escaped site. The captured line must contain the escaped path + (`\#012` for the newline), never the raw control byte. */ +static void test_special_socket_path_log_escaped() { + set_log_level(LOG_LEVEL_WARNING); + log_set_8_bit_output(false); + + FILE* capture = tmpfile(); + EXPECT_NOT_NULL(capture); + log_set_file(capture); + + File* file = file_create("evil\npath"); + EXPECT_NOT_NULL(file); + file->is_special = true; + file->metadata = calloc(1, sizeof(FileMetadata)); + EXPECT_NOT_NULL(file->metadata); + file->metadata->mode = S_IFSOCK | 0644; + + FileSaveResult result = file_save_to_disk_full("/tmp/dst", file, NULL); + EXPECT_EQ_INT(result, FILE_SAVE_SKIPPED); + + fflush(capture); + rewind(capture); + char output[512] = {0}; + size_t length = fread(output, 1, sizeof(output) - 1, capture); + output[length] = '\0'; + + log_set_file(NULL); + fclose(capture); + file_destroy(file); + + EXPECT_NOT_NULL(strstr(output, "socket not recreated: evil\\#012path")); +} + void test_server() { + test_special_socket_path_log_escaped(); if (!is_running_under_valgrind()) { test_receive_files_finished(); test_receive_files_single_file(); diff --git a/tests/test_server_cli.c b/tests/test_server_cli.c index 5eea11c..c057ebd 100644 --- a/tests/test_server_cli.c +++ b/tests/test_server_cli.c @@ -164,19 +164,26 @@ static void test_server_cli_invalid() { } static void test_server_cli_password_and_early_input() { - const char* args[] = {"s", "--daemon", "--password-file=/etc/fast.pw", "--early-input", - "/run/secrets"}; + const char* args[] = {"s", + "--daemon", + "--password-file=/etc/fast.pw", + "--early-input", + "/run/secrets", + "--iconv=utf-8"}; ServerCliOptions opts; - EXPECT_EQ_INT(parse_ok(args, 5, &opts), 0); + EXPECT_EQ_INT(parse_ok(args, 6, &opts), 0); EXPECT_EQ_STR(opts.password_file, "/etc/fast.pw"); EXPECT_EQ_STR(opts.early_input_file, "/run/secrets"); + EXPECT_EQ_STR(opts.iconv_spec, "utf-8"); - const char* args2[] = {"s", "--daemon", "--password-file", "/etc/fast.pw", - "--early-input=/secrets"}; + const char* args2[] = { + "s", "--daemon", "--password-file", "/etc/fast.pw", "--early-input=/secrets", + "--iconv", "utf-8,iso-8859-1"}; ServerCliOptions opts2; - EXPECT_EQ_INT(parse_ok(args2, 5, &opts2), 0); + EXPECT_EQ_INT(parse_ok(args2, 7, &opts2), 0); EXPECT_EQ_STR(opts2.password_file, "/etc/fast.pw"); EXPECT_EQ_STR(opts2.early_input_file, "/secrets"); + EXPECT_EQ_STR(opts2.iconv_spec, "utf-8,iso-8859-1"); server_cli_options_free(&opts); server_cli_options_free(&opts2); } diff --git a/tests/test_transport_ssh.c b/tests/test_transport_ssh.c index 7a273ef..94a896a 100644 --- a/tests/test_transport_ssh.c +++ b/tests/test_transport_ssh.c @@ -55,8 +55,14 @@ static void test_ssh_remote_command_argument_modes() { EXPECT_EQ_STR(command, "'fast'\\''sync' --stdio"); free(command); + /* --old-args no longer disables injection-safe quoting: the path is still one + single-quoted word, even when it carries shell metacharacters. */ command = ssh_build_remote_command("fast sync; touch /tmp/pwned", true, NULL, 0); - EXPECT_EQ_STR(command, "fast sync; touch /tmp/pwned --stdio"); + EXPECT_EQ_STR(command, "'fast sync; touch /tmp/pwned' --stdio"); + free(command); + + command = ssh_build_remote_command("fast'sync; rm -rf /", true, NULL, 0); + EXPECT_EQ_STR(command, "'fast'\\''sync; rm -rf /' --stdio"); free(command); } @@ -131,9 +137,9 @@ static void test_ssh_remote_command_with_remote_options() { free(command); free(val); - /* --old-args leaves the server path unquoted but still quotes remote options. */ + /* --old-args still quotes both the server path and the remote options. */ command = ssh_build_remote_command("srv", true, multi, 2); - EXPECT_EQ_STR(command, "srv --stdio '-v' '--allow-delete'"); + EXPECT_EQ_STR(command, "'srv' --stdio '-v' '--allow-delete'"); free(command); } diff --git a/tests/test_transport_tls.c b/tests/test_transport_tls.c index 5e21580..28bbba5 100644 --- a/tests/test_transport_tls.c +++ b/tests/test_transport_tls.c @@ -3,6 +3,7 @@ #include "test_utils.h" #include "transport_tcp.h" #include "transport_tls.h" +#include #include #include @@ -17,6 +18,12 @@ static void test_server_create_tls_without_certs() { bool ok = server_create_tls(s, NULL, NULL, NULL); EXPECT_TRUE(ok); EXPECT_NOT_NULL(s->ssl_ctx); + /* The context must disable TLS compression (CRIME) and renegotiation. */ + SSL_CTX* ctx = (SSL_CTX*)s->ssl_ctx; + EXPECT_TRUE((SSL_CTX_get_options(ctx) & SSL_OP_NO_COMPRESSION) != 0); +#ifdef SSL_OP_NO_RENEGOTIATION + EXPECT_TRUE((SSL_CTX_get_options(ctx) & SSL_OP_NO_RENEGOTIATION) != 0); +#endif server_delete(&s); EXPECT_NULL(s); } diff --git a/tests/test_xattr.c b/tests/test_xattr.c index c5e4316..4997ae2 100644 --- a/tests/test_xattr.c +++ b/tests/test_xattr.c @@ -311,7 +311,7 @@ static void test_fake_super_owner_gate() { /* --no-super: the owner leg is skipped even as root. */ c->super_mode = SUPER_MODE_OFF; - identity_set_active(c); + EXPECT_TRUE(identity_set_active(c)); EXPECT_TRUE(fake_super_restore_fd(fd)); struct stat st; EXPECT_EQ_INT(fstat(fd, &st), 0); @@ -320,7 +320,7 @@ static void test_fake_super_owner_gate() { /* AUTO with an identity policy: the recorded source owner is applied. */ c->super_mode = SUPER_MODE_AUTO; - identity_set_active(c); + EXPECT_TRUE(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, 12345); @@ -331,7 +331,7 @@ static void test_fake_super_owner_gate() { EXPECT_EQ_INT(fchown(fd, 0, 0), 0); c->numeric_ids = false; c->super_mode = SUPER_MODE_ON; - identity_set_active(c); + EXPECT_TRUE(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); @@ -342,7 +342,7 @@ static void test_fake_super_owner_gate() { c->copy_as_set = true; c->copy_as_uid = 777; c->copy_as_gid = 778; - identity_set_active(c); + EXPECT_TRUE(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);