From 42f01c0968d910bf1a4c0634beaf024a635e8202 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sat, 12 Sep 2026 18:40:21 +0200 Subject: [PATCH 1/6] fix(a7-auth): persist dummy key in owner-only sidecar The store-wide dummy key was regenerated on every credentials_load, so an unknown user's dummy salt changed across daemon restarts while a real user's stored salt stayed stable -- a restart-gated username-enumeration oracle. Persist the 32-byte key in a 0600 .dummykey sidecar next to the credential store. An absent sidecar is created with O_EXCL and fsynced; a present sidecar is read only when it is an owner-only regular file of exactly 32 bytes (otherwise the load fails closed). If the sidecar cannot be created (read-only mount, missing directory) fall back to a transient per-run key with a warning. A NULL store path keeps the key ephemeral. --- README.md | 12 ++- RSYNC_COMPAT.md | 4 +- src/shared/credentials.c | 202 +++++++++++++++++++++++++++++++++++++-- src/shared/credentials.h | 9 ++ tests/test_credentials.c | 151 ++++++++++++++++++++++++++++- 5 files changed, 359 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index c6fec51..3919f51 100644 --- a/README.md +++ b/README.md @@ -522,11 +522,13 @@ deterministic per-username dummy challenge, so probing the daemon cannot enumerate users. Store lines are generated with `fastsync-server --hash-credentials ` (see `RSYNC_COMPAT.md`); redirect that output to an owner-only (mode 0600) file, and note that legacy -`user:SHA256HEX` stores are rejected. Two residuals are accepted: the dummy salt -is stable within one daemon lifetime but changes across restarts, so a -restart-gated enumeration channel remains (persisting a dummy key is out of -scope); and the store iteration count is observable pre-auth by design, since -the miss path must match a hit. +`user:SHA256HEX` stores are rejected. FastSync also maintains an owner-only +(mode 0600) `.dummykey` sidecar next to the store: it holds the store-wide +dummy key, is auto-created on first load, and must be preserved across daemon +restarts so the dummy challenge for an unknown user stays stable (the key is +never regenerated while the sidecar exists). One residual is accepted: the store +iteration count is observable pre-auth by design, since the miss path must match +a hit. TLS provides encrypted TCP transport. Supplying `--ca` enables certificate verification; without it, traffic is encrypted but peer identity is not diff --git a/RSYNC_COMPAT.md b/RSYNC_COMPAT.md index 6d74e25..1383666 100644 --- a/RSYNC_COMPAT.md +++ b/RSYNC_COMPAT.md @@ -639,8 +639,8 @@ now transmits targets (the prior behavior was broken/partial); its status moved - **Module selection & confinement:** the client requests a module with an rsync-style `host::module[/path]` destination. The module name crosses the wire as a trailing string on the config frame (bumping `PROTOCOL_VERSION` 2.14.0 → 2.15.0; the bump is required because the config-frame layout changed and the strict same-version handshake is what prevents a peer from desynchronizing on the new trailing field). The daemon looks the module up in ITS OWN config and uses the module's `path` as the authorized root through the exact same `configure_authorization` confinement the standalone server applies to `--destination-root` (`file_open_secure_parent`, `has_path_traversal`, `path_is_within`); the client never supplies the root, every client-chosen-ownership/super-user request is refused unless the module declares `client owner = yes` (the daemon's per-module opt-in, see below), and the operator `--no-super` veto forces super-user activities off for every daemon connection. The client's `/path` part is relative inside the module and is rejected if absolute or if it contains `..`. Unknown modules are refused before any data moves (the run fails cleanly at the config handshake). An absolute destination and a module request against a non-daemon server are also refused. - **`client owner` (client-chosen-ownership opt-in):** by default a daemon module refuses every request that would let the client pick an owner or ask for super-user activities — `--numeric-ids`, `--chown`, `--usermap`/`--groupmap`, `--fake-super`, `--copy-as`, and an explicit `--super` — at the config handshake (before `STATUS_OK`), because a daemon has no per-module opt-in for client-chosen ownership and any anonymous client could otherwise force arbitrary owner ids inside the module root. `client owner = yes` opts a single module in, allowing those requests within that module's root (the standalone listener and the SSH `--stdio` server always honor them for their single operator-authorized root). Without the opt-in the daemon also forces super-user **device** activity off for that connection — char/block device-node creation (`--devices`) and `--write-devices` — even under the default `AUTO` mode, so a non-opted module can never be made to `mknod` or write a raw device; those entries are skipped (not refused) so an ordinary `-a` push still succeeds without device nodes. The opt-in does **not** lift the privilege requirement: `--copy-as` still needs a root receiver, and the operator `--no-super` veto still forces super-user activities off for every connection. The daemon logs a prominent startup warning for each `client owner = yes` module so the operator's deliberate choice is visible. - **`read only` safe default:** every network transfer FastSync currently supports is a push that writes under the module root, so a `read only` module refuses the connection (clear server log "module is read only"; the client exits non-zero, nothing is transferred). A future pull/list operation can be opened up when it exists; the knob is already stored. -- **`auth users` (A7 SCRAM-SHA-256 authentication):** a module that declares `auth users` requires the client to present credentials. The config frame carries ONLY the username; the daemon answers an auth-required module with `STATUS_AUTH_CHALLENGE` (PBKDF2 iteration count, 16-byte salt, 32-byte server nonce), the client answers with `STATUS_AUTH_RESPONSE` (fresh 32-byte client nonce + a 32-byte ClientProof), and the daemon accepts only when the proof verifies **and** the username is **on the module's `auth users` list** and has a store entry, replying `STATUS_AUTH_OK` with a 32-byte ServerSignature the client verifies before proceeding. Verification is constant-time over fixed 32-byte keys (the compare runs even for a miss), username membership uses a constant-time full-length scan, and an unknown/off-list user still receives a challenge and runs the same math against a dummy verifier: a deterministic per-username salt (`HMAC-SHA256(store dummy key, username)`), the store-wide uniform iteration count and dummy keys. Re-probing the same unknown username therefore yields an identical salt and iteration count while a different username yields a different salt, so there is no user-enumeration or timing oracle. The daemon logs the username but **never the password, proof or keys**. A module WITHOUT `auth users` stays open (legitimate rsync configuration); credentials sent to such a module are ignored. Read-only is orthogonal: even a correctly authenticated push to a `read only` module is still refused (all FastSync network transfers write). Fail-closed policy: a daemon whose config declares `auth users` on any module refuses to start unless a credential store was given (`--password-file` and/or `--early-input`); a missing or empty store is never silently treated as "open". A failed handshake (missing credentials, unknown/off-list user, wrong proof or malformed data) yields a single generic `STATUS_AUTH_FAILED` and the daemon closes before any data moves. Two residuals are accepted: the dummy salt is stable within one daemon lifetime but changes across restarts, leaving a restart-gated enumeration channel (persisting the dummy key is out of scope); and the store iteration count is observable pre-auth by design, since the miss path must match a hit. -- **Credential store format:** server `--password-file`/`--early-input` files are line-based `user:$fastsync$1$pbkdf2-sha256$$$$`, one per line (standard base64; 16-byte salt, 32-byte keys; `iters` in `[100000, 10000000]`, default 600000). Every entry in the resulting store must agree on `iters` (a store whose entries disagree, or where a layered `--early-input` disagrees with `--password-file`, is rejected). Generate lines with `fastsync-server --hash-credentials FILE [--iterations N]`; the emitted lines are secret material, so redirect them to an owner-only (mode 0600) file (the tool warns on stderr if stdout is a group/other-accessible regular file). Blank lines and lines starting with `#`/`;` are comments; the parser is strict (a malformed line fails the whole load, so a typo can never let a different set of users in). **The legacy `user:SHA256HEX` form is hard-rejected** with an actionable "legacy" error; there is no auto-upgrade, so a replayable bearer digest can never be loaded by a 2.19.0 daemon. The client `--password-file` holds `user:password` on its first meaningful line (the literal password, used only for the handshake then burned); keep both files readable only by their owner (mode 0600). Per-username wire length is bounded (256 chars) and every decoded salt/key length is validated. +- **`auth users` (A7 SCRAM-SHA-256 authentication):** a module that declares `auth users` requires the client to present credentials. The config frame carries ONLY the username; the daemon answers an auth-required module with `STATUS_AUTH_CHALLENGE` (PBKDF2 iteration count, 16-byte salt, 32-byte server nonce), the client answers with `STATUS_AUTH_RESPONSE` (fresh 32-byte client nonce + a 32-byte ClientProof), and the daemon accepts only when the proof verifies **and** the username is **on the module's `auth users` list** and has a store entry, replying `STATUS_AUTH_OK` with a 32-byte ServerSignature the client verifies before proceeding. Verification is constant-time over fixed 32-byte keys (the compare runs even for a miss), username membership uses a constant-time full-length scan, and an unknown/off-list user still receives a challenge and runs the same math against a dummy verifier: a deterministic per-username salt (`HMAC-SHA256(store dummy key, username)`), the store-wide uniform iteration count and dummy keys. Re-probing the same unknown username therefore yields an identical salt and iteration count while a different username yields a different salt, so there is no user-enumeration or timing oracle. The daemon logs the username but **never the password, proof or keys**. A module WITHOUT `auth users` stays open (legitimate rsync configuration); credentials sent to such a module are ignored. Read-only is orthogonal: even a correctly authenticated push to a `read only` module is still refused (all FastSync network transfers write). Fail-closed policy: a daemon whose config declares `auth users` on any module refuses to start unless a credential store was given (`--password-file` and/or `--early-input`); a missing or empty store is never silently treated as "open". A failed handshake (missing credentials, unknown/off-list user, wrong proof or malformed data) yields a single generic `STATUS_AUTH_FAILED` and the daemon closes before any data moves. The dummy key is persisted in an owner-only `.dummykey` sidecar (auto-created on first load, mode 0600) so the dummy salt stays stable across daemon restarts, closing the restart-gated enumeration channel. One residual is accepted: the store iteration count is observable pre-auth by design, since the miss path must match a hit. +- **Credential store format:** server `--password-file`/`--early-input` files are line-based `user:$fastsync$1$pbkdf2-sha256$$$$`, one per line (standard base64; 16-byte salt, 32-byte keys; `iters` in `[100000, 10000000]`, default 600000). Every entry in the resulting store must agree on `iters` (a store whose entries disagree, or where a layered `--early-input` disagrees with `--password-file`, is rejected). Generate lines with `fastsync-server --hash-credentials FILE [--iterations N]`; the emitted lines are secret material, so redirect them to an owner-only (mode 0600) file (the tool warns on stderr if stdout is a group/other-accessible regular file). Blank lines and lines starting with `#`/`;` are comments; the parser is strict (a malformed line fails the whole load, so a typo can never let a different set of users in). **The legacy `user:SHA256HEX` form is hard-rejected** with an actionable "legacy" error; there is no auto-upgrade, so a replayable bearer digest can never be loaded by a 2.19.0 daemon. The client `--password-file` holds `user:password` on its first meaningful line (the literal password, used only for the handshake then burned); keep both files readable only by their owner (mode 0600). Per-username wire length is bounded (256 chars) and every decoded salt/key length is validated. Loading the store also maintains an owner-only `.dummykey` sidecar (auto-created, mode 0600, exactly 32 bytes) holding the store-wide dummy key that shapes unknown-user challenges; persist it across daemon restarts so those challenges stay stable, and treat a sidecar with the wrong owner, permissions, size or type as a fatal load error (fail closed). - **Plaintext caveat:** over a plaintext (non-TLS) daemon a sniffer can read the challenge and response and mount an **offline dictionary attack** against a weak password, so use `--tls` for any real deployment. Unlike the old challenge-less exchange there is **no replay**: the proof is bound to the fresh per-connection server nonce, so a captured `STATUS_AUTH_RESPONSE` cannot be reused on another connection (an integration test proxies the daemon and proves this). The daemon logs a warning when an auth-required module is reached over plaintext. TLS client-CN (`--client-cn`) is an independent transport identity check and composes with password auth: both may be required on the same connection. - **Wire/protocol:** the config-frame auth block is now `[int present][str_redacted username]` (the old digest field is gone), and the frame stream gains the challenge/response (`STATUS_AUTH_CHALLENGE` → `STATUS_AUTH_RESPONSE` → `STATUS_AUTH_OK`/`STATUS_AUTH_FAILED`) between the config frame and the `STATUS_OK` ack. Both are wire-layout changes, so `PROTOCOL_VERSION` is bumped **2.18.0 → 2.19.0** (see the A7 note in `src/shared/config.h`); the strict same-version handshake keeps a 2.19 client and a 2.18 server from desynchronizing. - **Client side:** `host::module/path` selects the TCP transport and connects to `--server-port`; `host:path` stays the SSH transport; plain paths stay local TCP. The daemon username comes from `--password-file` (first `user:password` line), and `--password-file` without a `host::module/path` destination is a client error (fail fast). A `user@host::module` form is rejected with a pointer to `--password-file`. The client's plaintext password is wiped from memory (`config_burn_auth`) at transfer teardown. diff --git a/src/shared/credentials.c b/src/shared/credentials.c index 5971a26..36dd004 100644 --- a/src/shared/credentials.c +++ b/src/shared/credentials.c @@ -1,4 +1,5 @@ #include "credentials.h" +#include "log.h" #include "utils.h" #include #include @@ -35,16 +36,22 @@ struct CredentialStore { * challenged with the same count as a hit and the count itself never leaks * membership. Unused (0) for an empty store. */ uint32_t iters; - /* Random secret generated once at load. The dummy salt handed out for an - * unknown/off-list user is HMAC-SHA256(dummy_key, username)[:SALT_LEN], so - * repeated probes of the same username always see an identical challenge - * while different usernames differ -- with no fresh-random tell. */ + /* Store-wide secret loaded from (or created in) the owner-only + * `.dummykey` sidecar, so it also survives a daemon restart. The + * dummy salt handed out for an unknown/off-list user is + * HMAC-SHA256(dummy_key, username)[:SALT_LEN], so repeated probes of the same + * username always see an identical challenge while different usernames differ + * -- with no fresh-random tell, and cross-restart stability hides the + * restart-gated enumeration oracle. */ uint8_t dummy_key[CREDENTIAL_KEY_LEN]; }; /* Exact marker prefix of the new store verifier field. */ #define CREDENTIAL_STORE_PREFIX "$fastsync$1$pbkdf2-sha256$" #define CREDENTIAL_AUTH_PREFIX "FastSync-Auth-v1" +/* Owner-only sidecar holding the persistent store-wide dummy key, placed next to + * the credential store (`.dummykey`). */ +#define CREDENTIAL_DUMMY_KEY_SUFFIX ".dummykey" /* Fixed dummy keys used when a user is unknown or off the module's list. They * can never authenticate because acceptance additionally requires found=true. */ @@ -583,6 +590,178 @@ static CredentialStore* load_store_file(const char* path, char* err, size_t err_ return store; } +/* Validate and read an already-open `.dummykey` sidecar. Fails closed on + * anything that is not an owner-only (0600) regular file of exactly + * CREDENTIAL_KEY_LEN bytes, so a loosened, swapped or truncated file can never + * silently change the dummy challenge. */ +static bool read_dummy_key_fd(int fd, const char* path, uint8_t out[CREDENTIAL_KEY_LEN], char* err, + size_t err_size) { + struct stat st; + if (fstat(fd, &st) != 0) { + set_error(err, err_size, "cannot stat dummy key file '%s': %s", path, strerror(errno)); + return false; + } + if (!S_ISREG(st.st_mode) || st.st_uid != geteuid() || (st.st_mode & (S_IRWXG | S_IRWXO)) != 0 || + st.st_size != (off_t)CREDENTIAL_KEY_LEN) { + set_error(err, err_size, + "refusing to read dummy key file '%s': it must be an owner-only (0600) regular file " + "of exactly %d bytes", + path, CREDENTIAL_KEY_LEN); + return false; + } + size_t got = 0; + while (got < CREDENTIAL_KEY_LEN) { + ssize_t n = read(fd, out + got, CREDENTIAL_KEY_LEN - got); + if (n < 0) { + if (errno == EINTR) + continue; + set_error(err, err_size, "cannot read dummy key file '%s': %s", path, strerror(errno)); + return false; + } + if (n == 0) + break; + got += (size_t)n; + } + if (got != CREDENTIAL_KEY_LEN) { + set_error(err, err_size, "dummy key file '%s' is truncated", path); + return false; + } + return true; +} + +/* Load the persistent dummy key for `store_path` from its `.dummykey` + * sidecar, creating it (mode 0600, 32 random bytes) if absent. A NULL + * store_path (empty store) yields a fresh ephemeral key. Reading an existing + * sidecar fails CLOSED on any validation error; only the CREATE path degrades + * to an ephemeral key (with a warning) when the filesystem cannot hold the + * sidecar (e.g. read-only mount), so a daemon still starts. Returns false only + * when the CSPRNG itself fails (or a present-but-invalid sidecar is found). */ +static bool load_or_create_dummy_key(const char* store_path, uint8_t out[CREDENTIAL_KEY_LEN], + char* err, size_t err_size) { + if (!store_path) { + if (!credentials_random_bytes(out, CREDENTIAL_KEY_LEN)) { + set_error(err, err_size, "failed to generate the credential store dummy key"); + return false; + } + return true; + } + + size_t path_len = strlen(store_path); + size_t suffix_len = sizeof(CREDENTIAL_DUMMY_KEY_SUFFIX); /* includes the NUL */ + if (path_len > SIZE_MAX - suffix_len) { + set_error(err, err_size, "credential store path is too long to build a dummy key path"); + return false; + } + char* sidecar = malloc(path_len + suffix_len); + if (!sidecar) { + set_error(err, err_size, "out of memory building the dummy key path"); + return false; + } + int n = snprintf(sidecar, path_len + suffix_len, "%s%s", store_path, CREDENTIAL_DUMMY_KEY_SUFFIX); + if (n < 0 || (size_t)n >= path_len + suffix_len) { + set_error(err, err_size, "credential store path is too long to build a dummy key path"); + free(sidecar); + return false; + } + + int fd = open(sidecar, O_RDONLY | O_CLOEXEC); + if (fd >= 0) { + bool ok = read_dummy_key_fd(fd, sidecar, out, err, err_size); + close(fd); + free(sidecar); + return ok; + } + if (errno != ENOENT) { + /* The sidecar exists but cannot be opened for reading (e.g. EACCES): fail + * closed rather than substituting a different key. */ + set_error(err, err_size, "cannot open dummy key file '%s': %s", sidecar, strerror(errno)); + free(sidecar); + return false; + } + + uint8_t fresh[CREDENTIAL_KEY_LEN]; + if (!credentials_random_bytes(fresh, CREDENTIAL_KEY_LEN)) { + set_error(err, err_size, "failed to generate the credential store dummy key"); + free(sidecar); + return false; + } + fd = open(sidecar, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0600); + if (fd < 0) { + int open_errno = errno; + if (open_errno == EEXIST) { + /* A racing instance created the sidecar first; adopt its key. */ + int rfd = open(sidecar, O_RDONLY | O_CLOEXEC); + if (rfd < 0) { + set_error(err, err_size, "cannot open dummy key file '%s': %s", sidecar, strerror(errno)); + free(sidecar); + credentials_burn((char*)fresh, sizeof(fresh)); + return false; + } + bool ok = read_dummy_key_fd(rfd, sidecar, out, err, err_size); + close(rfd); + free(sidecar); + credentials_burn((char*)fresh, sizeof(fresh)); + return ok; + } + /* Creation failed for another reason (read-only filesystem, missing + * directory, ...). Warn and fall back to an ephemeral key: unknown-user + * challenges stay deterministic within this daemon lifetime but will change + * on the next restart. */ + char* escaped = output_escape(sidecar, log_get_8_bit_output()); + log_message(LOG_LEVEL_WARNING, + "cannot create dummy key file %s: %s; using a transient dummy key so unknown-user " + "challenges will change across restarts", + escaped ? escaped : sidecar, strerror(open_errno)); + free(escaped); + memcpy(out, fresh, CREDENTIAL_KEY_LEN); + free(sidecar); + credentials_burn((char*)fresh, sizeof(fresh)); + return true; + } + + size_t written = 0; + bool write_ok = true; + while (written < CREDENTIAL_KEY_LEN) { + ssize_t w = write(fd, fresh + written, CREDENTIAL_KEY_LEN - written); + if (w < 0) { + if (errno == EINTR) + continue; + write_ok = false; + break; + } + if (w == 0) { + write_ok = false; + break; + } + written += (size_t)w; + } + int write_errno = errno; + if (write_ok && fsync(fd) != 0) { + write_ok = false; + write_errno = errno; + } + close(fd); + if (!write_ok) { + /* Do not leave a truncated sidecar behind that would fail-closed a later + * restart; fall back to an ephemeral key instead. */ + unlink(sidecar); + char* escaped = output_escape(sidecar, log_get_8_bit_output()); + log_message(LOG_LEVEL_WARNING, + "cannot write dummy key file %s: %s; using a transient dummy key so unknown-user " + "challenges will change across restarts", + escaped ? escaped : sidecar, strerror(write_errno)); + free(escaped); + memcpy(out, fresh, CREDENTIAL_KEY_LEN); + free(sidecar); + credentials_burn((char*)fresh, sizeof(fresh)); + return true; + } + memcpy(out, fresh, CREDENTIAL_KEY_LEN); + free(sidecar); + credentials_burn((char*)fresh, sizeof(fresh)); + return true; +} + CredentialStore* credentials_load(const char* password_file, const char* early_input_file, char* err, size_t err_size) { if (err && err_size) @@ -590,12 +769,15 @@ CredentialStore* credentials_load(const char* password_file, const char* early_i CredentialStore* store = load_store_file(password_file, err, err_size); if (!store) return NULL; - /* Generate the store-wide dummy key once for the final (possibly merged) - * store. It makes an unknown-user challenge deterministic, so fail the load - * if the CSPRNG is unavailable rather than degrading the anti-enumeration - * property. */ - if (!credentials_random_bytes(store->dummy_key, sizeof(store->dummy_key))) { - set_error(err, err_size, "failed to generate the credential store dummy key"); + /* Load (or create) the store-wide dummy key once for the final (possibly + * merged) store. It makes an unknown-user challenge deterministic AND + * stable across daemon restarts, so a restart cannot be used as a + * username-enumeration oracle. It is persisted in an owner-only sidecar next + * to the credential store; a NULL store path (empty store) keeps it + * ephemeral. Fail the load if the CSPRNG is unavailable rather than + * degrading the anti-enumeration property. */ + const char* store_path = password_file ? password_file : early_input_file; + if (!load_or_create_dummy_key(store_path, store->dummy_key, err, err_size)) { credentials_free(store); return NULL; } diff --git a/src/shared/credentials.h b/src/shared/credentials.h index 9742ea9..d6314b2 100644 --- a/src/shared/credentials.h +++ b/src/shared/credentials.h @@ -26,6 +26,15 @@ * hard-rejected with an actionable "legacy" error; there is no auto-upgrade. * Use `fastsync-server --hash-credentials` to generate new-format lines. * + * Alongside the store, credentials_load maintains an owner-only (0600) + * `.dummykey` sidecar holding the store-wide random dummy key. It + * is auto-created on first load and MUST be preserved across restarts: it makes + * the dummy challenge for an unknown user stable for the life of the store, so + * a daemon restart cannot be used as a username-enumeration oracle. A sidecar + * that is not an owner-only regular file of exactly 32 bytes fails the load + * (fail closed); if it cannot be created (e.g. a read-only mount) the daemon + * warns and uses a transient per-run key instead. + * * Client --password-file format: the FIRST meaningful (non-comment, non-blank) * line is `user:password`, holding the literal password. The client keeps it * only for the duration of the handshake and wipes it at teardown; the file diff --git a/tests/test_credentials.c b/tests/test_credentials.c index 425a715..658cc9c 100644 --- a/tests/test_credentials.c +++ b/tests/test_credentials.c @@ -70,8 +70,28 @@ static char* make_tmp_file(const char* contents) { } static void rm_temp(const char* path) { - if (path) - unlink(path); + if (!path) + return; + unlink(path); + /* Every successfully loaded store auto-creates an owner-only + * `.dummykey` sidecar; remove it too so tests leave no stray key. */ + size_t n = strlen(path) + strlen(".dummykey") + 1; + char* sidecar = malloc(n); + if (sidecar) { + snprintf(sidecar, n, "%s.dummykey", path); + unlink(sidecar); + free(sidecar); + } +} + +/* `.dummykey` sidecar path (caller frees). */ +static char* dummy_sidecar_path(const char* store_path) { + size_t n = strlen(store_path) + strlen(".dummykey") + 1; + char* out = malloc(n); + if (!out) + return NULL; + snprintf(out, n, "%s.dummykey", store_path); + return out; } /* Build a valid new-format line for user/password at iters. */ @@ -745,6 +765,130 @@ static void test_credentials_rejects_group_or_other_accessible() { free(path); } +/* Loading a store auto-creates an owner-only `.dummykey` sidecar whose + * key is stable across reloads, so an unknown-user dummy salt is identical + * across two loads (the anti-restart enumeration property). */ +static void test_credentials_dummy_key_persisted() { + char line[CREDENTIAL_MAX_LINE]; + EXPECT_TRUE(make_store_line("alice", KAT_PASSWORD, CREDENTIAL_MIN_ITERS, line, sizeof(line))); + char contents[CREDENTIAL_MAX_LINE + 2]; + snprintf(contents, sizeof(contents), "%s\n", line); + char* path = make_tmp_file(contents); + EXPECT_NOT_NULL(path); + char* sidecar = dummy_sidecar_path(path); + EXPECT_NOT_NULL(sidecar); + + char err[512]; + CredentialStore* store = credentials_load(path, NULL, err, sizeof(err)); + EXPECT_NOT_NULL(store); + + struct stat st; + EXPECT_EQ_INT(stat(sidecar, &st), 0); + EXPECT_TRUE(S_ISREG(st.st_mode)); + EXPECT_TRUE((st.st_mode & (S_IRWXG | S_IRWXO)) == 0); + EXPECT_EQ_INT((int)(st.st_mode & 07777), 0600); + EXPECT_EQ_INT((int)st.st_size, CREDENTIAL_KEY_LEN); + + CredentialVerifier v1; + EXPECT_TRUE(credentials_get_verifier(store, "unknown-user", NULL, 0, &v1)); + EXPECT_FALSE(v1.found); + credentials_free(store); + + store = credentials_load(path, NULL, err, sizeof(err)); + EXPECT_NOT_NULL(store); + CredentialVerifier v2; + EXPECT_TRUE(credentials_get_verifier(store, "unknown-user", NULL, 0, &v2)); + EXPECT_FALSE(v2.found); + EXPECT_TRUE(memcmp(v1.salt, v2.salt, sizeof(v1.salt)) == 0); + credentials_free(store); + + rm_temp(path); + free(path); + free(sidecar); +} + +/* A sidecar that is group/other accessible, the wrong size, or not a regular + * file must fail the load closed. */ +static void test_credentials_dummy_key_rejects_bad_sidecar() { + char err[512]; + char line[CREDENTIAL_MAX_LINE]; + EXPECT_TRUE(make_store_line("alice", KAT_PASSWORD, CREDENTIAL_MIN_ITERS, line, sizeof(line))); + char contents[CREDENTIAL_MAX_LINE + 2]; + snprintf(contents, sizeof(contents), "%s\n", line); + + /* Group/other permission bits on the sidecar. */ + char* path = make_tmp_file(contents); + EXPECT_NOT_NULL(path); + char* sidecar = dummy_sidecar_path(path); + EXPECT_NOT_NULL(sidecar); + uint8_t key[CREDENTIAL_KEY_LEN]; + memset(key, 0x5a, sizeof(key)); + FILE* fp = fopen(sidecar, "wb"); + EXPECT_NOT_NULL(fp); + EXPECT_TRUE(fwrite(key, 1, sizeof(key), fp) == sizeof(key)); + fclose(fp); + EXPECT_EQ_INT(chmod(sidecar, 0640), 0); + EXPECT_NULL(credentials_load(path, NULL, err, sizeof(err))); + EXPECT_TRUE(err[0] != '\0'); + rm_temp(path); + free(path); + free(sidecar); + + /* Wrong size (not exactly 32 bytes). */ + path = make_tmp_file(contents); + EXPECT_NOT_NULL(path); + sidecar = dummy_sidecar_path(path); + EXPECT_NOT_NULL(sidecar); + fp = fopen(sidecar, "wb"); + EXPECT_NOT_NULL(fp); + EXPECT_TRUE(fwrite(key, 1, CREDENTIAL_SALT_LEN, fp) == CREDENTIAL_SALT_LEN); + fclose(fp); + EXPECT_EQ_INT(chmod(sidecar, 0600), 0); + EXPECT_NULL(credentials_load(path, NULL, err, sizeof(err))); + EXPECT_TRUE(err[0] != '\0'); + rm_temp(path); + free(path); + free(sidecar); + + /* Non-regular file (a directory at the sidecar path). */ + path = make_tmp_file(contents); + EXPECT_NOT_NULL(path); + sidecar = dummy_sidecar_path(path); + EXPECT_NOT_NULL(sidecar); + EXPECT_EQ_INT(mkdir(sidecar, 0700), 0); + EXPECT_NULL(credentials_load(path, NULL, err, sizeof(err))); + EXPECT_TRUE(err[0] != '\0'); + rmdir(sidecar); + rm_temp(path); + free(path); + free(sidecar); +} + +/* A NULL store path has nowhere to persist a key, so each load gets a fresh + * ephemeral key (and creates no sidecar). */ +static void test_credentials_dummy_key_null_store_ephemeral() { + char err[512]; + CredentialStore* store = credentials_load(NULL, NULL, err, sizeof(err)); + EXPECT_NOT_NULL(store); + CredentialVerifier v1; + CredentialVerifier v1b; + EXPECT_TRUE(credentials_get_verifier(store, "nobody", NULL, 0, &v1)); + EXPECT_FALSE(v1.found); + /* Within one store the dummy challenge is still deterministic. */ + EXPECT_TRUE(credentials_get_verifier(store, "nobody", NULL, 0, &v1b)); + EXPECT_TRUE(memcmp(v1.salt, v1b.salt, sizeof(v1.salt)) == 0); + credentials_free(store); + + store = credentials_load(NULL, NULL, err, sizeof(err)); + EXPECT_NOT_NULL(store); + CredentialVerifier v2; + EXPECT_TRUE(credentials_get_verifier(store, "nobody", NULL, 0, &v2)); + /* No persistence path, so the second load's random key differs (and with it + * the dummy salt). */ + EXPECT_TRUE(memcmp(v1.salt, v2.salt, sizeof(v1.salt)) != 0); + credentials_free(store); +} + static void test_credentials_burn() { char secret[32]; memcpy(secret, "supersecretvalue", 17); @@ -777,5 +921,8 @@ void test_credentials(void) { test_credentials_read_secret_file_bad(); test_credentials_hash_file(); test_credentials_rejects_group_or_other_accessible(); + test_credentials_dummy_key_persisted(); + test_credentials_dummy_key_rejects_bad_sidecar(); + test_credentials_dummy_key_null_store_ephemeral(); test_credentials_burn(); } From a7a1930e8838057d54dd3f1170dfe3cfaf04760e Mon Sep 17 00:00:00 2001 From: TapTap Date: Sat, 12 Sep 2026 19:02:05 +0200 Subject: [PATCH 2/6] fix(a7-3/s1): require TLS or local transport for daemon auth Daemon modules that declare 'auth users' no longer accept credentials over a remote plaintext connection: server_module_gate refuses at the config gate, before any SCRAM challenge is sent, unless the connection is verified TLS with a client certificate matching --client-cn, or a local/SSH transport (loopback TCP peer or the --stdio pipe). --allow-unauthenticated does not relax this. The TLS client-CN comparison now uses credentials_secure_equal (S2). Clients sending --password-file to a non-loopback daemon must use --tls; validate_config rejects the plaintext case before any network I/O. Adds utils_sockaddr_is_loopback / utils_fd_peer_is_local / utils_host_is_loopback helpers with unit tests, a client validation unit test, and integration tests for the client-side plaintext rejection and the wrong-CN gate refusal. --- README.md | 10 ++++ RSYNC_COMPAT.md | 4 +- src/client/client_validation.c | 10 ++++ src/server/server.c | 22 ++++++-- src/shared/utils.c | 67 ++++++++++++++++++++++++ src/shared/utils.h | 6 +++ tests/integration/test_daemon.py | 88 ++++++++++++++++++++++++++++++-- tests/test_client_cli.c | 37 ++++++++++++++ tests/test_shared_utils.c | 79 ++++++++++++++++++++++++++++ 9 files changed, 312 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index c6fec51..ad93fed 100644 --- a/README.md +++ b/README.md @@ -528,6 +528,16 @@ restart-gated enumeration channel remains (persisting a dummy key is out of scope); and the store iteration count is observable pre-auth by design, since the miss path must match a hit. +An `auth users` module only accepts credentials over an encrypted, verified TLS +connection whose client certificate matches the server's `--client-cn`, or over +a local/SSH transport (a loopback TCP peer or the `--stdio` pipe). A remote +plaintext peer is refused before any challenge is sent, and +`--allow-unauthenticated` does **not** relax this: that flag only relaxes the +standalone plaintext gate. Clients sending daemon credentials with +`--password-file` to a non-loopback daemon must therefore use `--tls`; the +client rejects a non-local plaintext credential destination before any network +I/O. + TLS provides encrypted TCP transport. Supplying `--ca` enables certificate verification; without it, traffic is encrypted but peer identity is not verified. Use certificate verification for deployments where authentication diff --git a/RSYNC_COMPAT.md b/RSYNC_COMPAT.md index 6d74e25..b46af90 100644 --- a/RSYNC_COMPAT.md +++ b/RSYNC_COMPAT.md @@ -639,9 +639,9 @@ now transmits targets (the prior behavior was broken/partial); its status moved - **Module selection & confinement:** the client requests a module with an rsync-style `host::module[/path]` destination. The module name crosses the wire as a trailing string on the config frame (bumping `PROTOCOL_VERSION` 2.14.0 → 2.15.0; the bump is required because the config-frame layout changed and the strict same-version handshake is what prevents a peer from desynchronizing on the new trailing field). The daemon looks the module up in ITS OWN config and uses the module's `path` as the authorized root through the exact same `configure_authorization` confinement the standalone server applies to `--destination-root` (`file_open_secure_parent`, `has_path_traversal`, `path_is_within`); the client never supplies the root, every client-chosen-ownership/super-user request is refused unless the module declares `client owner = yes` (the daemon's per-module opt-in, see below), and the operator `--no-super` veto forces super-user activities off for every daemon connection. The client's `/path` part is relative inside the module and is rejected if absolute or if it contains `..`. Unknown modules are refused before any data moves (the run fails cleanly at the config handshake). An absolute destination and a module request against a non-daemon server are also refused. - **`client owner` (client-chosen-ownership opt-in):** by default a daemon module refuses every request that would let the client pick an owner or ask for super-user activities — `--numeric-ids`, `--chown`, `--usermap`/`--groupmap`, `--fake-super`, `--copy-as`, and an explicit `--super` — at the config handshake (before `STATUS_OK`), because a daemon has no per-module opt-in for client-chosen ownership and any anonymous client could otherwise force arbitrary owner ids inside the module root. `client owner = yes` opts a single module in, allowing those requests within that module's root (the standalone listener and the SSH `--stdio` server always honor them for their single operator-authorized root). Without the opt-in the daemon also forces super-user **device** activity off for that connection — char/block device-node creation (`--devices`) and `--write-devices` — even under the default `AUTO` mode, so a non-opted module can never be made to `mknod` or write a raw device; those entries are skipped (not refused) so an ordinary `-a` push still succeeds without device nodes. The opt-in does **not** lift the privilege requirement: `--copy-as` still needs a root receiver, and the operator `--no-super` veto still forces super-user activities off for every connection. The daemon logs a prominent startup warning for each `client owner = yes` module so the operator's deliberate choice is visible. - **`read only` safe default:** every network transfer FastSync currently supports is a push that writes under the module root, so a `read only` module refuses the connection (clear server log "module is read only"; the client exits non-zero, nothing is transferred). A future pull/list operation can be opened up when it exists; the knob is already stored. -- **`auth users` (A7 SCRAM-SHA-256 authentication):** a module that declares `auth users` requires the client to present credentials. The config frame carries ONLY the username; the daemon answers an auth-required module with `STATUS_AUTH_CHALLENGE` (PBKDF2 iteration count, 16-byte salt, 32-byte server nonce), the client answers with `STATUS_AUTH_RESPONSE` (fresh 32-byte client nonce + a 32-byte ClientProof), and the daemon accepts only when the proof verifies **and** the username is **on the module's `auth users` list** and has a store entry, replying `STATUS_AUTH_OK` with a 32-byte ServerSignature the client verifies before proceeding. Verification is constant-time over fixed 32-byte keys (the compare runs even for a miss), username membership uses a constant-time full-length scan, and an unknown/off-list user still receives a challenge and runs the same math against a dummy verifier: a deterministic per-username salt (`HMAC-SHA256(store dummy key, username)`), the store-wide uniform iteration count and dummy keys. Re-probing the same unknown username therefore yields an identical salt and iteration count while a different username yields a different salt, so there is no user-enumeration or timing oracle. The daemon logs the username but **never the password, proof or keys**. A module WITHOUT `auth users` stays open (legitimate rsync configuration); credentials sent to such a module are ignored. Read-only is orthogonal: even a correctly authenticated push to a `read only` module is still refused (all FastSync network transfers write). Fail-closed policy: a daemon whose config declares `auth users` on any module refuses to start unless a credential store was given (`--password-file` and/or `--early-input`); a missing or empty store is never silently treated as "open". A failed handshake (missing credentials, unknown/off-list user, wrong proof or malformed data) yields a single generic `STATUS_AUTH_FAILED` and the daemon closes before any data moves. Two residuals are accepted: the dummy salt is stable within one daemon lifetime but changes across restarts, leaving a restart-gated enumeration channel (persisting the dummy key is out of scope); and the store iteration count is observable pre-auth by design, since the miss path must match a hit. +- **`auth users` (A7 SCRAM-SHA-256 authentication):** a module that declares `auth users` requires the client to present credentials. The config frame carries ONLY the username; the daemon answers an auth-required module with `STATUS_AUTH_CHALLENGE` (PBKDF2 iteration count, 16-byte salt, 32-byte server nonce), the client answers with `STATUS_AUTH_RESPONSE` (fresh 32-byte client nonce + a 32-byte ClientProof), and the daemon accepts only when the proof verifies **and** the username is **on the module's `auth users` list** and has a store entry, replying `STATUS_AUTH_OK` with a 32-byte ServerSignature the client verifies before proceeding. Verification is constant-time over fixed 32-byte keys (the compare runs even for a miss), username membership uses a constant-time full-length scan, and an unknown/off-list user still receives a challenge and runs the same math against a dummy verifier: a deterministic per-username salt (`HMAC-SHA256(store dummy key, username)`), the store-wide uniform iteration count and dummy keys. Re-probing the same unknown username therefore yields an identical salt and iteration count while a different username yields a different salt, so there is no user-enumeration or timing oracle. The daemon logs the username but **never the password, proof or keys**. A module WITHOUT `auth users` stays open (legitimate rsync configuration); credentials sent to such a module are ignored. Read-only is orthogonal: even a correctly authenticated push to a `read only` module is still refused (all FastSync network transfers write). Fail-closed policy: a daemon whose config declares `auth users` on any module refuses to start unless a credential store was given (`--password-file` and/or `--early-input`); a missing or empty store is never silently treated as "open". A failed handshake (missing credentials, unknown/off-list user, wrong proof or malformed data) yields a single generic `STATUS_AUTH_FAILED` and the daemon closes before any data moves. Two residuals are accepted: the dummy salt is stable within one daemon lifetime but changes across restarts, leaving a restart-gated enumeration channel (persisting the dummy key is out of scope); and the store iteration count is observable pre-auth by design, since the miss path must match a hit. **Transport policy (hardening A7-3/S1):** an auth-required module accepts credentials only over an encrypted, verified TLS connection whose client certificate matches `--client-cn`, or over a local/SSH transport (a loopback TCP peer, or the `--stdio` pipe); a remote plaintext peer is refused at the config gate before any challenge is sent, and `--allow-unauthenticated` does **not** relax this. - **Credential store format:** server `--password-file`/`--early-input` files are line-based `user:$fastsync$1$pbkdf2-sha256$$$$`, one per line (standard base64; 16-byte salt, 32-byte keys; `iters` in `[100000, 10000000]`, default 600000). Every entry in the resulting store must agree on `iters` (a store whose entries disagree, or where a layered `--early-input` disagrees with `--password-file`, is rejected). Generate lines with `fastsync-server --hash-credentials FILE [--iterations N]`; the emitted lines are secret material, so redirect them to an owner-only (mode 0600) file (the tool warns on stderr if stdout is a group/other-accessible regular file). Blank lines and lines starting with `#`/`;` are comments; the parser is strict (a malformed line fails the whole load, so a typo can never let a different set of users in). **The legacy `user:SHA256HEX` form is hard-rejected** with an actionable "legacy" error; there is no auto-upgrade, so a replayable bearer digest can never be loaded by a 2.19.0 daemon. The client `--password-file` holds `user:password` on its first meaningful line (the literal password, used only for the handshake then burned); keep both files readable only by their owner (mode 0600). Per-username wire length is bounded (256 chars) and every decoded salt/key length is validated. -- **Plaintext caveat:** over a plaintext (non-TLS) daemon a sniffer can read the challenge and response and mount an **offline dictionary attack** against a weak password, so use `--tls` for any real deployment. Unlike the old challenge-less exchange there is **no replay**: the proof is bound to the fresh per-connection server nonce, so a captured `STATUS_AUTH_RESPONSE` cannot be reused on another connection (an integration test proxies the daemon and proves this). The daemon logs a warning when an auth-required module is reached over plaintext. TLS client-CN (`--client-cn`) is an independent transport identity check and composes with password auth: both may be required on the same connection. +- **Plaintext caveat:** an auth-required module is refused, **before any challenge is sent**, unless the connection is encrypted and verified TLS whose client certificate matches the server's `--client-cn`, or a local/SSH transport (a loopback TCP peer, or the `--stdio` pipe). A remote plaintext peer never receives a challenge, and `--allow-unauthenticated` does **not** relax this policy (that flag only relaxes the standalone plaintext gate). On the loopback/SSH transports that remain permitted, a local sniffer could still read the challenge and response and mount an **offline dictionary attack** against a weak password, so use `--tls` for any real deployment. Clients sending daemon credentials with `--password-file` to a non-loopback daemon must use `--tls`; the client rejects such a destination before any network I/O. Unlike the old challenge-less exchange there is **no replay**: the proof is bound to the fresh per-connection server nonce, so a captured `STATUS_AUTH_RESPONSE` cannot be reused on another connection (an integration test proxies the daemon and proves this). TLS client-CN (`--client-cn`) is an independent transport identity check and composes with password auth: both may be required on the same connection. - **Wire/protocol:** the config-frame auth block is now `[int present][str_redacted username]` (the old digest field is gone), and the frame stream gains the challenge/response (`STATUS_AUTH_CHALLENGE` → `STATUS_AUTH_RESPONSE` → `STATUS_AUTH_OK`/`STATUS_AUTH_FAILED`) between the config frame and the `STATUS_OK` ack. Both are wire-layout changes, so `PROTOCOL_VERSION` is bumped **2.18.0 → 2.19.0** (see the A7 note in `src/shared/config.h`); the strict same-version handshake keeps a 2.19 client and a 2.18 server from desynchronizing. - **Client side:** `host::module/path` selects the TCP transport and connects to `--server-port`; `host:path` stays the SSH transport; plain paths stay local TCP. The daemon username comes from `--password-file` (first `user:password` line), and `--password-file` without a `host::module/path` destination is a client error (fail fast). A `user@host::module` form is rejected with a pointer to `--password-file`. The client's plaintext password is wiped from memory (`config_burn_auth`) at transfer teardown. - **MOTD (Wave C):** a daemon configured with a global `motd file` sends that file's content as the first server→client string frame after the config-frame STATUS_OK ack (rsync sends the MOTD as the first thing from the server at the start of a daemon connection). Only the daemon listener path (`host::module`) gets a MOTD; the `--stdio` SSH path never sends or reads one. The server reads the file bounded to 4096 bytes and treats an absent/unreadable file as "no MOTD" (an empty frame, never an error). The exchange is server→client only and does **not** bump `PROTOCOL_VERSION`: every 2.15.0 daemon client reads the frame after the ack, so sender and receiver stay in lockstep (see the Wave C note in `src/shared/config.h`). `--no-motd` is the client-side suppression switch: the client still reads (consumes) the frame to keep the stream in sync but does not display it. The MOTD is printed to stdout with control bytes (ESC included) escaped octal-style while newlines/tabs are preserved, so a hostile server cannot inject terminal escape sequences. diff --git a/src/client/client_validation.c b/src/client/client_validation.c index 67e8060..f6a8ad0 100644 --- a/src/client/client_validation.c +++ b/src/client/client_validation.c @@ -3,6 +3,7 @@ #include "delay_updates.h" #include "log.h" #include "usage.h" +#include "utils.h" #include #include @@ -136,6 +137,15 @@ bool validate_config(const Config* config) { return false; } } + /* Daemon credentials (A7, protocol 2.19.0): a --password-file would send the + username in the clear and derive a SCRAM proof a network sniffer could + attack offline, so it is only allowed over TLS (which itself mandates a + verified --cert/--key/--ca set above) or to a loopback destination. A + remote plaintext daemon is refused here, before any network I/O. */ + if (config->password_file && !config->use_tls && !utils_host_is_loopback(config->server_host)) { + log_message(LOG_LEVEL_ERROR, "sending daemon credentials to a non-local server requires --tls"); + return false; + } if (config->delay_updates && config->inplace) { log_message(LOG_LEVEL_ERROR, "--delay-updates does not work with --inplace"); return false; diff --git a/src/server/server.c b/src/server/server.c index c23dd5b..ac0cd6f 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -173,7 +173,7 @@ static bool tls_client_identity_allowed(SSL* ssl) { size_t required_length = strlen(required_client_cn); bool allowed = length >= 0 && (size_t)length == required_length && required_length < sizeof(common_name) && - memcmp(common_name, required_client_cn, required_length) == 0; + credentials_secure_equal(common_name, required_client_cn, required_length); X509_free(certificate); return allowed; } @@ -382,11 +382,23 @@ static const char* server_module_gate(const Config* config, void* context) { return "requested daemon module requires authentication and no credential " "store is configured"; } - if (gate_ctx && !gate_ctx->ssl) { - log_message(LOG_LEVEL_WARNING, - "daemon module '%s' is authenticating over a plaintext connection (no --tls); " - "the credential exchange is not encrypted", + /* Transport policy (A7-3/S1): an auth-required module only accepts + * credentials over an encrypted, verified TLS connection whose client + * certificate matches --client-cn, or over a local/SSH transport (a + * loopback TCP peer, or the --stdio pipe). A remote plaintext peer is + * refused HERE, before the challenge is sent, so an unverified client never + * receives a nonce. --allow-unauthenticated is intentionally NOT consulted: + * that flag relaxes the standalone plaintext gate, never this one. */ + bool tls_ok = gate_ctx && gate_ctx->ssl && SSL_get_verify_result(gate_ctx->ssl) == X509_V_OK && + tls_client_identity_allowed(gate_ctx->ssl); + bool local_ok = gate_ctx && gate_ctx->fd >= 0 && utils_fd_peer_is_local(gate_ctx->fd); + if (!tls_ok && !local_ok) { + log_message(LOG_LEVEL_ERROR, + "daemon module '%s' requires authentication over an encrypted, verified TLS " + "connection (or a local/SSH transport); refusing", config->module); + return "daemon module requires authentication over an encrypted, verified TLS " + "connection"; } if (!gate_ctx || gate_ctx->fd < 0) { log_message(LOG_LEVEL_ERROR, "daemon module '%s': no auth transport available", diff --git a/src/shared/utils.c b/src/shared/utils.c index 6fa85fd..df92cc4 100644 --- a/src/shared/utils.c +++ b/src/shared/utils.c @@ -1,13 +1,16 @@ #include "utils.h" #include "array_list.h" #include "log.h" +#include #include #include #include +#include #include #include #include #include +#include #include #include @@ -543,3 +546,67 @@ bool append_tail_length(unsigned long long old_size, unsigned long long check_si *tail_out = check_size - old_size; return true; } + +/* True when a bound/peer socket address is on the loopback interface: any + 127.0.0.0/8 IPv4 address, IPv6 ::1, or an IPv4-mapped ::ffff:127.x.x.x. This + is the transport-local test the daemon auth gate uses to decide whether a + plaintext connection is a trustworthy local/SSH channel. */ +bool utils_sockaddr_is_loopback(const struct sockaddr* addr) { + if (!addr) + return false; + if (addr->sa_family == AF_INET) { + const struct sockaddr_in* v4 = (const struct sockaddr_in*)addr; + uint32_t host = ntohl(v4->sin_addr.s_addr); + return (host & 0xff000000u) == 0x7f000000u; + } + if (addr->sa_family == AF_INET6) { + const struct sockaddr_in6* v6 = (const struct sockaddr_in6*)addr; + if (IN6_IS_ADDR_LOOPBACK(&v6->sin6_addr)) + return true; + /* An IPv4-mapped ::ffff:127.x.x.x is loopback too. */ + if (IN6_IS_ADDR_V4MAPPED(&v6->sin6_addr) && v6->sin6_addr.s6_addr[12] == 127) + return true; + return false; + } + return false; +} + +/* True when the fd's peer is a local channel: a loopback TCP peer, or a + non-socket descriptor (the --stdio SSH transport is a pipe, so a failed + getpeername with ENOTSOCK counts as local). Any other socket peer is not + local. */ +bool utils_fd_peer_is_local(int fd) { + if (fd < 0) + return false; + struct sockaddr_storage peer; + socklen_t length = sizeof(peer); + if (getpeername(fd, (struct sockaddr*)&peer, &length) != 0) + return errno == ENOTSOCK; + return utils_sockaddr_is_loopback((const struct sockaddr*)&peer); +} + +/* True when a client-supplied host string names a loopback destination: + "localhost", any 127.0.0.0/8 literal, "::1", or "[::1]". */ +bool utils_host_is_loopback(const char* host) { + if (!host || host[0] == '\0') + return false; + if (strcmp(host, "localhost") == 0) + return true; + struct in_addr v4; + if (inet_pton(AF_INET, host, &v4) == 1) + return (ntohl(v4.s_addr) & 0xff000000u) == 0x7f000000u; + struct in6_addr addr6; + if (host[0] == '[') { + size_t len = strlen(host); + if (len < 3 || host[len - 1] != ']') + return false; + /* inet_pton needs the bare address, not the bracketed form. */ + char bare[INET6_ADDRSTRLEN]; + if (len - 2 >= sizeof(bare)) + return false; + memcpy(bare, host + 1, len - 2); + bare[len - 2] = '\0'; + return inet_pton(AF_INET6, bare, &addr6) == 1 && IN6_IS_ADDR_LOOPBACK(&addr6); + } + return inet_pton(AF_INET6, host, &addr6) == 1 && IN6_IS_ADDR_LOOPBACK(&addr6); +} diff --git a/src/shared/utils.h b/src/shared/utils.h index d8dd2a2..b7a6208 100644 --- a/src/shared/utils.h +++ b/src/shared/utils.h @@ -4,6 +4,7 @@ #include "array_list.h" #include #include +#include char* str_dup(const char* string); char* output_escape(const char* string, bool eight_bit_output); @@ -66,5 +67,10 @@ bool format_human_bytes(unsigned long long bytes, char* buffer, size_t buffer_si bool append_resume_eligible(unsigned long long old_size, unsigned long long check_size); bool append_tail_length(unsigned long long old_size, unsigned long long check_size, unsigned long long* tail_out); +/* Loopback / local-transport classification for the daemon auth gate and the + client credential rule. See utils.c for the exact accepted forms. */ +bool utils_sockaddr_is_loopback(const struct sockaddr* addr); +bool utils_fd_peer_is_local(int fd); +bool utils_host_is_loopback(const char* host); #endif diff --git a/tests/integration/test_daemon.py b/tests/integration/test_daemon.py index f8999d0..91bd6ed 100644 --- a/tests/integration/test_daemon.py +++ b/tests/integration/test_daemon.py @@ -766,6 +766,26 @@ class TestDaemonAuthentication: finally: os.unlink(cred_path) + @pytest.mark.ci + def test_remote_plaintext_credentials_rejected_client_side(self): + """A7-3/S1: sending daemon credentials to a clearly non-local daemon + WITHOUT --tls is refused by the client itself, before any network I/O + (192.0.2.0/24 is TEST-NET-1 and never reachable, so a network attempt + would time out instead of failing fast).""" + cred_path = os.path.join(TEST_DATA_DIR, "client_remote.pw") + _write_client_password_file(cred_path, "alice", ALICE_PASS) + try: + cmd = CLIENT_CMD + ["--source-dir", SOURCE_DIR, + "--dest-dir", "192.0.2.1::files", + "--save-to-disk", "--password-file", cred_path, + "--server-port", "873"] + result = subprocess.run(cmd, capture_output=True, text=True, timeout=15) + assert result.returncode != 0 + combined = (result.stderr or "") + (result.stdout or "") + assert "--tls" in combined, combined + finally: + os.unlink(cred_path) + def test_client_empty_password_file_rejected(self): """Client-side: an empty --password-file is rejected (no credentials).""" cred_path = os.path.join(TEST_DATA_DIR, "client_empty.pw") @@ -1001,23 +1021,30 @@ class TestDaemonMotd: d.stop() -def _generate_tls_certs(cert_dir): - """Generate a self-signed CA, server cert (with 127.0.0.1 SAN) and a client - cert signed by that CA, for the TLS+auth composition test.""" +def _generate_tls_certs(cert_dir, extra_san_ips=None): + """Generate a self-signed CA, server cert (with 127.0.0.1 SAN plus any + extra_san_ips) and two client certs signed by that CA: one with the + expected CN (fastsync-client) and one with a WRONG CN, for the TLS+auth + composition and wrong-identity tests.""" os.makedirs(cert_dir, exist_ok=True) ca_key, ca_cert = os.path.join(cert_dir, "ca.key"), os.path.join(cert_dir, "ca.pem") server_key = os.path.join(cert_dir, "server.key") server_cert = os.path.join(cert_dir, "server.pem") client_key = os.path.join(cert_dir, "client.key") client_cert = os.path.join(cert_dir, "client.pem") + wrong_client_key = os.path.join(cert_dir, "wrong_client.key") + wrong_client_cert = os.path.join(cert_dir, "wrong_client.pem") subprocess.run(["openssl", "req", "-x509", "-newkey", "rsa:2048", "-nodes", "-keyout", ca_key, "-out", ca_cert, "-days", "1", "-subj", "/CN=FastSync Test CA"], check=True, capture_output=True) san = os.path.join(cert_dir, "san.conf") + san_ips = ["IP.1 = 127.0.0.1"] + for index, ip in enumerate(extra_san_ips or [], start=2): + san_ips.append("IP.%d = %s" % (index, ip)) with open(san, "w") as f: f.write("[req]\ndistinguished_name = dn\nreq_extensions = v3_req\n\n" "[dn]\nCN = localhost\n\n[v3_req]\nsubjectAltName = @an\n\n" - "[an]\nDNS.1 = localhost\nIP.1 = 127.0.0.1\n") + "[an]\nDNS.1 = localhost\n" + "\n".join(san_ips) + "\n") subprocess.run(["openssl", "req", "-newkey", "rsa:2048", "-nodes", "-keyout", server_key, "-out", os.path.join(cert_dir, "server.csr"), "-subj", "/CN=localhost", "-config", san], check=True, capture_output=True) @@ -1031,12 +1058,20 @@ def _generate_tls_certs(cert_dir): subprocess.run(["openssl", "x509", "-req", "-in", os.path.join(cert_dir, "client.csr"), "-CA", ca_cert, "-CAkey", ca_key, "-CAcreateserial", "-out", client_cert, "-days", "1"], check=True, capture_output=True) + subprocess.run(["openssl", "req", "-newkey", "rsa:2048", "-nodes", + "-keyout", wrong_client_key, "-out", os.path.join(cert_dir, "wrong_client.csr"), + "-subj", "/CN=wrong-client"], check=True, capture_output=True) + subprocess.run(["openssl", "x509", "-req", "-in", os.path.join(cert_dir, "wrong_client.csr"), + "-CA", ca_cert, "-CAkey", ca_key, "-CAcreateserial", + "-out", wrong_client_cert, "-days", "1"], check=True, capture_output=True) return { "ca": ca_cert, "server_cert": server_cert, "server_key": server_key, "client_cert": client_cert, "client_key": client_key, + "wrong_client_cert": wrong_client_cert, + "wrong_client_key": wrong_client_key, } @@ -1077,3 +1112,48 @@ class TestDaemonTLSAuth: d.stop() os.unlink(client_creds) shutil.rmtree(cert_dir, ignore_errors=True) + + @pytest.mark.ci + def test_wrong_client_cn_refused_before_auth_challenge(self): + """A7-3/S1: over a NON-local TLS connection an auth-required module is + refused at the config gate when the CA-valid client certificate does not + match --client-cn -- before any SCRAM challenge is sent and before any + file data moves. The daemon is started WITH --allow-unauthenticated to + prove that flag does not relax the auth-module transport policy.""" + try: + remote_ip = socket.gethostbyname(socket.gethostname()) + except OSError: + pytest.skip("hostname does not resolve") + if remote_ip.startswith("127."): + pytest.skip("host resolves to loopback; no non-loopback interface") + cert_dir = os.path.join(TEST_DATA_DIR, "daemon_tls_certs_wrong") + certs = _generate_tls_certs(cert_dir, extra_san_ips=[remote_ip]) + client_creds = os.path.join(TEST_DATA_DIR, "daemon_tls_wrong_client.pw") + _write_client_password_file(client_creds, "alice", ALICE_PASS) + d = DaemonManager() + port = _find_free_port() + log_path = os.path.join(TEST_DATA_DIR, "fastsyncd.log") + try: + d.start(CONF_FILE, port_override=port, extra_args=[ + "--tls", "--cert", certs["server_cert"], "--key", certs["server_key"], + "--ca", certs["ca"], "--client-cn", "fastsync-client", + "--password-file", CRED_FILE]) + before_files = _tree_file_count(AUTH_MODULE) + log_before = os.path.getsize(log_path) if os.path.exists(log_path) else 0 + tls_flags = ["--tls", + "--cert", certs["wrong_client_cert"], "--key", + certs["wrong_client_key"], "--ca", certs["ca"]] + result, _ = run_client(SOURCE_DIR, "%s::locked" % remote_ip, port=port, + flags=tls_flags, extra_args=["--password-file", client_creds]) + assert result.returncode != 0, "a wrong client CN must be refused" + assert _tree_file_count(AUTH_MODULE) == before_files, \ + "a refused connection wrote file data" + with open(log_path, "rb") as f: + f.seek(log_before) + tail = f.read().decode("utf-8", "replace") + assert "requires authentication over an encrypted, verified TLS connection" in tail, \ + tail[-400:] + finally: + d.stop() + os.unlink(client_creds) + shutil.rmtree(cert_dir, ignore_errors=True) diff --git a/tests/test_client_cli.c b/tests/test_client_cli.c index 8a6c51e..44d6de7 100644 --- a/tests/test_client_cli.c +++ b/tests/test_client_cli.c @@ -69,6 +69,42 @@ static void test_validate_config_tls_requirements() { config_delete(cfg); } +/* A7-3/S1: --password-file sends daemon credentials, so it is only allowed + over TLS (which itself mandates a verified --cert/--key/--ca) or to a + loopback destination. A remote plaintext daemon is refused up front. */ +static void test_validate_config_credentials_require_tls_or_loopback() { + /* Default host is 127.0.0.1 (loopback), so plaintext credentials are fine. */ + Config* cfg = valid_client_config(); + cfg->password_file = str_dup("creds.pw"); + EXPECT_TRUE(validate_config(cfg)); + + /* localhost is loopback too. */ + free(cfg->server_host); + cfg->server_host = str_dup("localhost"); + EXPECT_TRUE(validate_config(cfg)); + + /* A clearly remote host over plaintext is refused before any network I/O. */ + free(cfg->server_host); + cfg->server_host = str_dup("192.0.2.1"); + EXPECT_FALSE(validate_config(cfg)); + + /* TLS makes the remote destination acceptable (cert/key/ca are required). */ + cfg->use_tls = true; + EXPECT_FALSE(validate_config(cfg)); + cfg->tls_cert = str_dup("cert.pem"); + cfg->tls_key = str_dup("key.pem"); + cfg->tls_ca = str_dup("ca.pem"); + EXPECT_TRUE(validate_config(cfg)); + + /* No credentials: the remote plaintext rule does not apply. */ + cfg->use_tls = false; + char* creds = cfg->password_file; + cfg->password_file = NULL; + EXPECT_TRUE(validate_config(cfg)); + cfg->password_file = creds; + config_delete(cfg); +} + static void test_validate_config_delta_sendfile_constraints() { Config* cfg = valid_client_config(); cfg->use_delta = true; @@ -3117,6 +3153,7 @@ void test_client_cli() { test_validate_config_append_verify_rejects_whole_file(); test_validate_config_incompatible_options(); test_validate_config_tls_requirements(); + test_validate_config_credentials_require_tls_or_loopback(); test_validate_config_delta_sendfile_constraints(); test_cli_help(); test_cli_archive_flags(); diff --git a/tests/test_shared_utils.c b/tests/test_shared_utils.c index a2cbc1c..a2c0ec0 100644 --- a/tests/test_shared_utils.c +++ b/tests/test_shared_utils.c @@ -2,12 +2,15 @@ #include "utils.h" #include "protocol.h" #include "test_utils.h" +#include #include #include #include +#include #include #include #include +#include #include #include #include @@ -269,12 +272,88 @@ static int escape_thread(void* arg) { return 0; } +/* A7-3/S1 transport classification: the daemon auth gate and the client + credential rule both key off these helpers, so cover the exact accepted + forms plus the negative cases. */ +static void test_loopback_helpers() { + /* Host strings. */ + EXPECT_TRUE(utils_host_is_loopback("localhost")); + EXPECT_TRUE(utils_host_is_loopback("127.0.0.1")); + EXPECT_TRUE(utils_host_is_loopback("127.255.255.254")); + EXPECT_TRUE(utils_host_is_loopback("127.0.0.0")); + EXPECT_TRUE(utils_host_is_loopback("::1")); + EXPECT_TRUE(utils_host_is_loopback("[::1]")); + EXPECT_FALSE(utils_host_is_loopback("128.0.0.1")); + EXPECT_FALSE(utils_host_is_loopback("10.0.0.1")); + EXPECT_FALSE(utils_host_is_loopback("0.0.0.0")); + EXPECT_FALSE(utils_host_is_loopback("example.com")); + EXPECT_FALSE(utils_host_is_loopback("")); + EXPECT_FALSE(utils_host_is_loopback(NULL)); + + /* Raw sockaddr classification. */ + struct sockaddr_in v4; + memset(&v4, 0, sizeof(v4)); + v4.sin_family = AF_INET; + EXPECT_TRUE(inet_pton(AF_INET, "127.0.0.1", &v4.sin_addr) == 1); + EXPECT_TRUE(utils_sockaddr_is_loopback((const struct sockaddr*)&v4)); + EXPECT_TRUE(inet_pton(AF_INET, "127.5.5.5", &v4.sin_addr) == 1); + EXPECT_TRUE(utils_sockaddr_is_loopback((const struct sockaddr*)&v4)); + EXPECT_TRUE(inet_pton(AF_INET, "128.0.0.1", &v4.sin_addr) == 1); + EXPECT_FALSE(utils_sockaddr_is_loopback((const struct sockaddr*)&v4)); + + struct sockaddr_in6 v6; + memset(&v6, 0, sizeof(v6)); + v6.sin6_family = AF_INET6; + EXPECT_TRUE(inet_pton(AF_INET6, "::1", &v6.sin6_addr) == 1); + EXPECT_TRUE(utils_sockaddr_is_loopback((const struct sockaddr*)&v6)); + EXPECT_TRUE(inet_pton(AF_INET6, "::ffff:127.0.0.1", &v6.sin6_addr) == 1); + EXPECT_TRUE(utils_sockaddr_is_loopback((const struct sockaddr*)&v6)); + EXPECT_TRUE(inet_pton(AF_INET6, "::ffff:127.255.255.254", &v6.sin6_addr) == 1); + EXPECT_TRUE(utils_sockaddr_is_loopback((const struct sockaddr*)&v6)); + EXPECT_TRUE(inet_pton(AF_INET6, "::ffff:10.0.0.1", &v6.sin6_addr) == 1); + EXPECT_FALSE(utils_sockaddr_is_loopback((const struct sockaddr*)&v6)); + + EXPECT_FALSE(utils_sockaddr_is_loopback(NULL)); + + /* A pipe has no socket peer: getpeername fails with ENOTSOCK, which is the + --stdio/SSH case and must count as local. */ + int pipe_fds[2]; + EXPECT_EQ_INT(pipe(pipe_fds), 0); + EXPECT_TRUE(utils_fd_peer_is_local(pipe_fds[0])); + close(pipe_fds[0]); + close(pipe_fds[1]); + EXPECT_FALSE(utils_fd_peer_is_local(-1)); + + /* A real loopback TCP peer is local. */ + int listener = socket(AF_INET, SOCK_STREAM, 0); + EXPECT_TRUE(listener >= 0); + struct sockaddr_in bind_addr; + memset(&bind_addr, 0, sizeof(bind_addr)); + bind_addr.sin_family = AF_INET; + bind_addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); + bind_addr.sin_port = 0; + EXPECT_EQ_INT(bind(listener, (const struct sockaddr*)&bind_addr, sizeof(bind_addr)), 0); + EXPECT_EQ_INT(listen(listener, 1), 0); + socklen_t addr_len = sizeof(bind_addr); + EXPECT_EQ_INT(getsockname(listener, (struct sockaddr*)&bind_addr, &addr_len), 0); + int dialer = socket(AF_INET, SOCK_STREAM, 0); + EXPECT_TRUE(dialer >= 0); + EXPECT_EQ_INT(connect(dialer, (const struct sockaddr*)&bind_addr, sizeof(bind_addr)), 0); + int accepted = accept(listener, NULL, NULL); + EXPECT_TRUE(accepted >= 0); + EXPECT_TRUE(utils_fd_peer_is_local(accepted)); + close(accepted); + close(dialer); + close(listener); +} + void test_shared_utils() { test_walker_removes_extras_keeps_manifest_and_protected(); test_walker_max_delete_exceeded_deletes_nothing(); test_walker_max_delete_exact_bound_deletes(); test_walker_unlimited_deletes_all(); test_walker_hard_bound_all_or_nothing(); + test_loopback_helpers(); /* --append / --append-verify tail-resume math: a resume is eligible only for a shorter existing destination, and the tail length is then the difference. */ From f0381a6b8e588d28136ab89fbe5620ef0e038f3d Mon Sep 17 00:00:00 2001 From: TapTap Date: Sat, 12 Sep 2026 19:16:11 +0200 Subject: [PATCH 3/6] fix(a7-auth): publish dummy-key sidecar atomically and harden reads Address review findings on the persistent dummy-key sidecar: - Publish atomically: write a private same-directory temp file (.dummykey.tmp., 0600), fsync, then link(2) into place; fsync the containing directory and drop the temp name. A concurrent starter can no longer observe a zero/partial sidecar and fail closed. On EEXIST adopt the winner's sidecar; otherwise warn and use a transient ephemeral key. - Harden the read path (initial and EEXIST-adopt) with O_RDONLY|O_NOFOLLOW|O_NONBLOCK|O_CLOEXEC: reject planted symlinks (ELOOP fails closed) and never block on a planted FIFO. - Require the exact owner-only mode (st_mode & 07777) == 0600 and make the rejection message truthful. - Report a clear "short write" instead of a stale strerror(errno) when write() returns 0. - Document the artifact and its creation-failure caveat (FIFO store path, read-only filesystem, missing directory) in README.md and RSYNC_COMPAT.md. - Tests: known-key sidecar adoption (dummy salt KAT + reload), symlink rejection, and the exact-0600 rule (0400 now rejected). --- README.md | 6 +- RSYNC_COMPAT.md | 4 +- src/shared/credentials.c | 168 ++++++++++++++++++++++++++++++--------- tests/test_credentials.c | 119 ++++++++++++++++++++++++++- 4 files changed, 254 insertions(+), 43 deletions(-) diff --git a/README.md b/README.md index 3919f51..4d98112 100644 --- a/README.md +++ b/README.md @@ -526,7 +526,11 @@ redirect that output to an owner-only (mode 0600) file, and note that legacy (mode 0600) `.dummykey` sidecar next to the store: it holds the store-wide dummy key, is auto-created on first load, and must be preserved across daemon restarts so the dummy challenge for an unknown user stays stable (the key is -never regenerated while the sidecar exists). One residual is accepted: the store +never regenerated while the sidecar exists). If the sidecar cannot be created +(process-substitution/FIFO store path such as `/dev/fd/N`, a read-only +filesystem, or a missing directory), the daemon logs a warning and uses a +transient key, so the cross-restart guarantee does not hold for those +deployments. One residual is accepted: the store iteration count is observable pre-auth by design, since the miss path must match a hit. diff --git a/RSYNC_COMPAT.md b/RSYNC_COMPAT.md index 1383666..33cdf38 100644 --- a/RSYNC_COMPAT.md +++ b/RSYNC_COMPAT.md @@ -639,8 +639,8 @@ now transmits targets (the prior behavior was broken/partial); its status moved - **Module selection & confinement:** the client requests a module with an rsync-style `host::module[/path]` destination. The module name crosses the wire as a trailing string on the config frame (bumping `PROTOCOL_VERSION` 2.14.0 → 2.15.0; the bump is required because the config-frame layout changed and the strict same-version handshake is what prevents a peer from desynchronizing on the new trailing field). The daemon looks the module up in ITS OWN config and uses the module's `path` as the authorized root through the exact same `configure_authorization` confinement the standalone server applies to `--destination-root` (`file_open_secure_parent`, `has_path_traversal`, `path_is_within`); the client never supplies the root, every client-chosen-ownership/super-user request is refused unless the module declares `client owner = yes` (the daemon's per-module opt-in, see below), and the operator `--no-super` veto forces super-user activities off for every daemon connection. The client's `/path` part is relative inside the module and is rejected if absolute or if it contains `..`. Unknown modules are refused before any data moves (the run fails cleanly at the config handshake). An absolute destination and a module request against a non-daemon server are also refused. - **`client owner` (client-chosen-ownership opt-in):** by default a daemon module refuses every request that would let the client pick an owner or ask for super-user activities — `--numeric-ids`, `--chown`, `--usermap`/`--groupmap`, `--fake-super`, `--copy-as`, and an explicit `--super` — at the config handshake (before `STATUS_OK`), because a daemon has no per-module opt-in for client-chosen ownership and any anonymous client could otherwise force arbitrary owner ids inside the module root. `client owner = yes` opts a single module in, allowing those requests within that module's root (the standalone listener and the SSH `--stdio` server always honor them for their single operator-authorized root). Without the opt-in the daemon also forces super-user **device** activity off for that connection — char/block device-node creation (`--devices`) and `--write-devices` — even under the default `AUTO` mode, so a non-opted module can never be made to `mknod` or write a raw device; those entries are skipped (not refused) so an ordinary `-a` push still succeeds without device nodes. The opt-in does **not** lift the privilege requirement: `--copy-as` still needs a root receiver, and the operator `--no-super` veto still forces super-user activities off for every connection. The daemon logs a prominent startup warning for each `client owner = yes` module so the operator's deliberate choice is visible. - **`read only` safe default:** every network transfer FastSync currently supports is a push that writes under the module root, so a `read only` module refuses the connection (clear server log "module is read only"; the client exits non-zero, nothing is transferred). A future pull/list operation can be opened up when it exists; the knob is already stored. -- **`auth users` (A7 SCRAM-SHA-256 authentication):** a module that declares `auth users` requires the client to present credentials. The config frame carries ONLY the username; the daemon answers an auth-required module with `STATUS_AUTH_CHALLENGE` (PBKDF2 iteration count, 16-byte salt, 32-byte server nonce), the client answers with `STATUS_AUTH_RESPONSE` (fresh 32-byte client nonce + a 32-byte ClientProof), and the daemon accepts only when the proof verifies **and** the username is **on the module's `auth users` list** and has a store entry, replying `STATUS_AUTH_OK` with a 32-byte ServerSignature the client verifies before proceeding. Verification is constant-time over fixed 32-byte keys (the compare runs even for a miss), username membership uses a constant-time full-length scan, and an unknown/off-list user still receives a challenge and runs the same math against a dummy verifier: a deterministic per-username salt (`HMAC-SHA256(store dummy key, username)`), the store-wide uniform iteration count and dummy keys. Re-probing the same unknown username therefore yields an identical salt and iteration count while a different username yields a different salt, so there is no user-enumeration or timing oracle. The daemon logs the username but **never the password, proof or keys**. A module WITHOUT `auth users` stays open (legitimate rsync configuration); credentials sent to such a module are ignored. Read-only is orthogonal: even a correctly authenticated push to a `read only` module is still refused (all FastSync network transfers write). Fail-closed policy: a daemon whose config declares `auth users` on any module refuses to start unless a credential store was given (`--password-file` and/or `--early-input`); a missing or empty store is never silently treated as "open". A failed handshake (missing credentials, unknown/off-list user, wrong proof or malformed data) yields a single generic `STATUS_AUTH_FAILED` and the daemon closes before any data moves. The dummy key is persisted in an owner-only `.dummykey` sidecar (auto-created on first load, mode 0600) so the dummy salt stays stable across daemon restarts, closing the restart-gated enumeration channel. One residual is accepted: the store iteration count is observable pre-auth by design, since the miss path must match a hit. -- **Credential store format:** server `--password-file`/`--early-input` files are line-based `user:$fastsync$1$pbkdf2-sha256$$$$`, one per line (standard base64; 16-byte salt, 32-byte keys; `iters` in `[100000, 10000000]`, default 600000). Every entry in the resulting store must agree on `iters` (a store whose entries disagree, or where a layered `--early-input` disagrees with `--password-file`, is rejected). Generate lines with `fastsync-server --hash-credentials FILE [--iterations N]`; the emitted lines are secret material, so redirect them to an owner-only (mode 0600) file (the tool warns on stderr if stdout is a group/other-accessible regular file). Blank lines and lines starting with `#`/`;` are comments; the parser is strict (a malformed line fails the whole load, so a typo can never let a different set of users in). **The legacy `user:SHA256HEX` form is hard-rejected** with an actionable "legacy" error; there is no auto-upgrade, so a replayable bearer digest can never be loaded by a 2.19.0 daemon. The client `--password-file` holds `user:password` on its first meaningful line (the literal password, used only for the handshake then burned); keep both files readable only by their owner (mode 0600). Per-username wire length is bounded (256 chars) and every decoded salt/key length is validated. Loading the store also maintains an owner-only `.dummykey` sidecar (auto-created, mode 0600, exactly 32 bytes) holding the store-wide dummy key that shapes unknown-user challenges; persist it across daemon restarts so those challenges stay stable, and treat a sidecar with the wrong owner, permissions, size or type as a fatal load error (fail closed). +- **`auth users` (A7 SCRAM-SHA-256 authentication):** a module that declares `auth users` requires the client to present credentials. The config frame carries ONLY the username; the daemon answers an auth-required module with `STATUS_AUTH_CHALLENGE` (PBKDF2 iteration count, 16-byte salt, 32-byte server nonce), the client answers with `STATUS_AUTH_RESPONSE` (fresh 32-byte client nonce + a 32-byte ClientProof), and the daemon accepts only when the proof verifies **and** the username is **on the module's `auth users` list** and has a store entry, replying `STATUS_AUTH_OK` with a 32-byte ServerSignature the client verifies before proceeding. Verification is constant-time over fixed 32-byte keys (the compare runs even for a miss), username membership uses a constant-time full-length scan, and an unknown/off-list user still receives a challenge and runs the same math against a dummy verifier: a deterministic per-username salt (`HMAC-SHA256(store dummy key, username)`), the store-wide uniform iteration count and dummy keys. Re-probing the same unknown username therefore yields an identical salt and iteration count while a different username yields a different salt, so there is no user-enumeration or timing oracle. The daemon logs the username but **never the password, proof or keys**. A module WITHOUT `auth users` stays open (legitimate rsync configuration); credentials sent to such a module are ignored. Read-only is orthogonal: even a correctly authenticated push to a `read only` module is still refused (all FastSync network transfers write). Fail-closed policy: a daemon whose config declares `auth users` on any module refuses to start unless a credential store was given (`--password-file` and/or `--early-input`); a missing or empty store is never silently treated as "open". A failed handshake (missing credentials, unknown/off-list user, wrong proof or malformed data) yields a single generic `STATUS_AUTH_FAILED` and the daemon closes before any data moves. The dummy key is persisted in an owner-only `.dummykey` sidecar (auto-created on first load, mode 0600) so the dummy salt stays stable across daemon restarts, closing the restart-gated enumeration channel. The sidecar must be preserved across restarts for that guarantee; if it cannot be created (a process-substitution/FIFO store path such as `/dev/fd/N`, a read-only filesystem, or a missing directory), the daemon logs a warning and uses a transient per-run key, so unknown-user challenges change across restarts and the cross-restart guarantee does not hold for that deployment. One residual is accepted: the store iteration count is observable pre-auth by design, since the miss path must match a hit. +- **Credential store format:** server `--password-file`/`--early-input` files are line-based `user:$fastsync$1$pbkdf2-sha256$$$$`, one per line (standard base64; 16-byte salt, 32-byte keys; `iters` in `[100000, 10000000]`, default 600000). Every entry in the resulting store must agree on `iters` (a store whose entries disagree, or where a layered `--early-input` disagrees with `--password-file`, is rejected). Generate lines with `fastsync-server --hash-credentials FILE [--iterations N]`; the emitted lines are secret material, so redirect them to an owner-only (mode 0600) file (the tool warns on stderr if stdout is a group/other-accessible regular file). Blank lines and lines starting with `#`/`;` are comments; the parser is strict (a malformed line fails the whole load, so a typo can never let a different set of users in). **The legacy `user:SHA256HEX` form is hard-rejected** with an actionable "legacy" error; there is no auto-upgrade, so a replayable bearer digest can never be loaded by a 2.19.0 daemon. The client `--password-file` holds `user:password` on its first meaningful line (the literal password, used only for the handshake then burned); keep both files readable only by their owner (mode 0600). Per-username wire length is bounded (256 chars) and every decoded salt/key length is validated. Loading the store also maintains an owner-only `.dummykey` sidecar (auto-created, mode 0600, exactly 32 bytes) holding the store-wide dummy key that shapes unknown-user challenges; persist it across daemon restarts so those challenges stay stable, and treat a sidecar with the wrong owner, a mode other than exactly 0600, the wrong size or the wrong type as a fatal load error (fail closed). If the sidecar cannot be created (e.g. a process-substitution store path such as `/dev/fd/N`, a read-only filesystem, or a missing directory), the daemon logs a warning and uses a transient per-run key, so the cross-restart stability guarantee does not hold there. - **Plaintext caveat:** over a plaintext (non-TLS) daemon a sniffer can read the challenge and response and mount an **offline dictionary attack** against a weak password, so use `--tls` for any real deployment. Unlike the old challenge-less exchange there is **no replay**: the proof is bound to the fresh per-connection server nonce, so a captured `STATUS_AUTH_RESPONSE` cannot be reused on another connection (an integration test proxies the daemon and proves this). The daemon logs a warning when an auth-required module is reached over plaintext. TLS client-CN (`--client-cn`) is an independent transport identity check and composes with password auth: both may be required on the same connection. - **Wire/protocol:** the config-frame auth block is now `[int present][str_redacted username]` (the old digest field is gone), and the frame stream gains the challenge/response (`STATUS_AUTH_CHALLENGE` → `STATUS_AUTH_RESPONSE` → `STATUS_AUTH_OK`/`STATUS_AUTH_FAILED`) between the config frame and the `STATUS_OK` ack. Both are wire-layout changes, so `PROTOCOL_VERSION` is bumped **2.18.0 → 2.19.0** (see the A7 note in `src/shared/config.h`); the strict same-version handshake keeps a 2.19 client and a 2.18 server from desynchronizing. - **Client side:** `host::module/path` selects the TCP transport and connects to `--server-port`; `host:path` stays the SSH transport; plain paths stay local TCP. The daemon username comes from `--password-file` (first `user:password` line), and `--password-file` without a `host::module/path` destination is a client error (fail fast). A `user@host::module` form is rejected with a pointer to `--password-file`. The client's plaintext password is wiped from memory (`config_burn_auth`) at transfer teardown. diff --git a/src/shared/credentials.c b/src/shared/credentials.c index 36dd004..3dbc3e5 100644 --- a/src/shared/credentials.c +++ b/src/shared/credentials.c @@ -601,11 +601,11 @@ static bool read_dummy_key_fd(int fd, const char* path, uint8_t out[CREDENTIAL_K set_error(err, err_size, "cannot stat dummy key file '%s': %s", path, strerror(errno)); return false; } - if (!S_ISREG(st.st_mode) || st.st_uid != geteuid() || (st.st_mode & (S_IRWXG | S_IRWXO)) != 0 || + if (!S_ISREG(st.st_mode) || st.st_uid != geteuid() || (st.st_mode & 07777) != 0600 || st.st_size != (off_t)CREDENTIAL_KEY_LEN) { set_error(err, err_size, - "refusing to read dummy key file '%s': it must be an owner-only (0600) regular file " - "of exactly %d bytes", + "refusing to read dummy key file '%s': it must be an owned regular file with exact " + "mode 0600 and exactly %d bytes", path, CREDENTIAL_KEY_LEN); return false; } @@ -629,12 +629,42 @@ static bool read_dummy_key_fd(int fd, const char* path, uint8_t out[CREDENTIAL_K return true; } +/* fsync the directory containing `path` (best effort). After publishing the + * sidecar with link(2), syncing the directory makes the new name durable so a + * crash cannot leave a restart without the key it just started using. */ +static void fsync_containing_dir(const char* path) { + char* dir = str_dup(path); + if (!dir) + return; + char* slash = strrchr(dir, '/'); + if (!slash) { + free(dir); + dir = str_dup("."); + if (!dir) + return; + } else if (slash == dir) { + slash[1] = '\0'; /* keep the leading '/' */ + } else { + *slash = '\0'; + } + int dfd = open(dir, O_RDONLY | O_DIRECTORY | O_CLOEXEC); + free(dir); + if (dfd < 0) + return; + fsync(dfd); + close(dfd); +} + /* Load the persistent dummy key for `store_path` from its `.dummykey` * sidecar, creating it (mode 0600, 32 random bytes) if absent. A NULL * store_path (empty store) yields a fresh ephemeral key. Reading an existing * sidecar fails CLOSED on any validation error; only the CREATE path degrades * to an ephemeral key (with a warning) when the filesystem cannot hold the - * sidecar (e.g. read-only mount), so a daemon still starts. Returns false only + * sidecar (e.g. read-only mount), so a daemon still starts. + * + * Creation is ATOMIC: the key is written to a private same-directory temp file + * and hard-linked into place, so a concurrent starter (or reader) never observes + * a partial/zero sidecar that would fail the load closed. Returns false only * when the CSPRNG itself fails (or a present-but-invalid sidecar is found). */ static bool load_or_create_dummy_key(const char* store_path, uint8_t out[CREDENTIAL_KEY_LEN], char* err, size_t err_size) { @@ -664,7 +694,10 @@ static bool load_or_create_dummy_key(const char* store_path, uint8_t out[CREDENT return false; } - int fd = open(sidecar, O_RDONLY | O_CLOEXEC); + /* Readers reject a planted symlink (O_NOFOLLOW) and never block on a planted + * FIFO (O_NONBLOCK; fstat rejects the non-regular file before any data read). + * Any open error other than ENOENT fails closed. */ + int fd = open(sidecar, O_RDONLY | O_NOFOLLOW | O_NONBLOCK | O_CLOEXEC); if (fd >= 0) { bool ok = read_dummy_key_fd(fd, sidecar, out, err, err_size); close(fd); @@ -672,48 +705,55 @@ static bool load_or_create_dummy_key(const char* store_path, uint8_t out[CREDENT return ok; } if (errno != ENOENT) { - /* The sidecar exists but cannot be opened for reading (e.g. EACCES): fail - * closed rather than substituting a different key. */ + /* The sidecar exists but cannot be opened for reading (EACCES, or ELOOP + * from a symlink): fail closed rather than substituting a different key. */ set_error(err, err_size, "cannot open dummy key file '%s': %s", sidecar, strerror(errno)); free(sidecar); return false; } - uint8_t fresh[CREDENTIAL_KEY_LEN]; - if (!credentials_random_bytes(fresh, CREDENTIAL_KEY_LEN)) { - set_error(err, err_size, "failed to generate the credential store dummy key"); + /* Publish atomically: write a private same-directory temp file, fsync it, + * then hard-link it into place. A concurrent reader therefore only ever + * sees a complete 32-byte sidecar (or none), never a partial/zero file. */ + char pid_suffix[32]; + int pn = snprintf(pid_suffix, sizeof(pid_suffix), ".tmp.%ld", (long)getpid()); + if (pn < 0 || (size_t)pn >= sizeof(pid_suffix)) { + set_error(err, err_size, "failed to build the dummy key temp path"); free(sidecar); return false; } - fd = open(sidecar, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0600); + size_t sidecar_len = (size_t)n; + size_t tmp_len = sidecar_len + (size_t)pn; + char* tmp = malloc(tmp_len + 1); + if (!tmp) { + set_error(err, err_size, "out of memory building the dummy key temp path"); + free(sidecar); + return false; + } + snprintf(tmp, tmp_len + 1, "%s%s", sidecar, pid_suffix); + + uint8_t fresh[CREDENTIAL_KEY_LEN]; + if (!credentials_random_bytes(fresh, CREDENTIAL_KEY_LEN)) { + set_error(err, err_size, "failed to generate the credential store dummy key"); + free(tmp); + free(sidecar); + return false; + } + + fd = open(tmp, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0600); if (fd < 0) { - int open_errno = errno; - if (open_errno == EEXIST) { - /* A racing instance created the sidecar first; adopt its key. */ - int rfd = open(sidecar, O_RDONLY | O_CLOEXEC); - if (rfd < 0) { - set_error(err, err_size, "cannot open dummy key file '%s': %s", sidecar, strerror(errno)); - free(sidecar); - credentials_burn((char*)fresh, sizeof(fresh)); - return false; - } - bool ok = read_dummy_key_fd(rfd, sidecar, out, err, err_size); - close(rfd); - free(sidecar); - credentials_burn((char*)fresh, sizeof(fresh)); - return ok; - } - /* Creation failed for another reason (read-only filesystem, missing - * directory, ...). Warn and fall back to an ephemeral key: unknown-user - * challenges stay deterministic within this daemon lifetime but will change - * on the next restart. */ - char* escaped = output_escape(sidecar, log_get_8_bit_output()); + /* Creation failed (read-only filesystem, missing directory, ...). Warn and + * fall back to an ephemeral key: unknown-user challenges stay deterministic + * within this daemon lifetime but will change on the next restart. */ + int create_errno = errno; + char* escaped = output_escape(tmp, log_get_8_bit_output()); log_message(LOG_LEVEL_WARNING, "cannot create dummy key file %s: %s; using a transient dummy key so unknown-user " "challenges will change across restarts", - escaped ? escaped : sidecar, strerror(open_errno)); + escaped ? escaped : tmp, strerror(create_errno)); free(escaped); memcpy(out, fresh, CREDENTIAL_KEY_LEN); + free(tmp); free(sidecar); credentials_burn((char*)fresh, sizeof(fresh)); return true; @@ -721,42 +761,92 @@ static bool load_or_create_dummy_key(const char* store_path, uint8_t out[CREDENT size_t written = 0; bool write_ok = true; + int write_errno = 0; while (written < CREDENTIAL_KEY_LEN) { ssize_t w = write(fd, fresh + written, CREDENTIAL_KEY_LEN - written); if (w < 0) { if (errno == EINTR) continue; write_ok = false; + write_errno = errno; break; } if (w == 0) { + /* A zero-length write is not a system error; errno is stale here, so + * report a clear short-write instead of a bogus strerror(errno). */ write_ok = false; + write_errno = 0; break; } written += (size_t)w; } - int write_errno = errno; if (write_ok && fsync(fd) != 0) { write_ok = false; write_errno = errno; } close(fd); if (!write_ok) { - /* Do not leave a truncated sidecar behind that would fail-closed a later - * restart; fall back to an ephemeral key instead. */ - unlink(sidecar); - char* escaped = output_escape(sidecar, log_get_8_bit_output()); + /* Do not leave a truncated temp file behind; fall back to an ephemeral key + * instead of failing closed on the next restart. */ + unlink(tmp); + char* escaped = output_escape(tmp, log_get_8_bit_output()); + const char* why = write_errno != 0 ? strerror(write_errno) : "short write"; log_message(LOG_LEVEL_WARNING, "cannot write dummy key file %s: %s; using a transient dummy key so unknown-user " "challenges will change across restarts", - escaped ? escaped : sidecar, strerror(write_errno)); + escaped ? escaped : tmp, why); free(escaped); memcpy(out, fresh, CREDENTIAL_KEY_LEN); + free(tmp); free(sidecar); credentials_burn((char*)fresh, sizeof(fresh)); return true; } + + if (link(tmp, sidecar) != 0) { + int link_errno = errno; + if (link_errno == EEXIST) { + /* A concurrent starter published first; adopt its key. Read it back + * through the same hardened path (no symlink, no block, exact mode). */ + int rfd = open(sidecar, O_RDONLY | O_NOFOLLOW | O_NONBLOCK | O_CLOEXEC); + if (rfd < 0) { + set_error(err, err_size, "cannot open dummy key file '%s': %s", sidecar, strerror(errno)); + unlink(tmp); + free(tmp); + free(sidecar); + credentials_burn((char*)fresh, sizeof(fresh)); + return false; + } + bool ok = read_dummy_key_fd(rfd, sidecar, out, err, err_size); + close(rfd); + unlink(tmp); + free(tmp); + free(sidecar); + credentials_burn((char*)fresh, sizeof(fresh)); + return ok; + } + /* Linking failed for another reason (e.g. no hard-link support on this + * filesystem). Warn and fall back to an ephemeral key. */ + unlink(tmp); + char* escaped = output_escape(sidecar, log_get_8_bit_output()); + log_message(LOG_LEVEL_WARNING, + "cannot publish dummy key file %s: %s; using a transient dummy key so unknown-user " + "challenges will change across restarts", + escaped ? escaped : sidecar, strerror(link_errno)); + free(escaped); + memcpy(out, fresh, CREDENTIAL_KEY_LEN); + free(tmp); + free(sidecar); + credentials_burn((char*)fresh, sizeof(fresh)); + return true; + } + + /* Published: make the new directory entry durable, then drop the private + * temp name (the sidecar keeps the inode alive). */ + fsync_containing_dir(sidecar); + unlink(tmp); memcpy(out, fresh, CREDENTIAL_KEY_LEN); + free(tmp); free(sidecar); credentials_burn((char*)fresh, sizeof(fresh)); return true; diff --git a/tests/test_credentials.c b/tests/test_credentials.c index 658cc9c..8832bd1 100644 --- a/tests/test_credentials.c +++ b/tests/test_credentials.c @@ -74,7 +74,8 @@ static void rm_temp(const char* path) { return; unlink(path); /* Every successfully loaded store auto-creates an owner-only - * `.dummykey` sidecar; remove it too so tests leave no stray key. */ + * `.dummykey` sidecar; remove it too so tests leave no stray key. The + * atomic-publish temp name is also removed defensively. */ size_t n = strlen(path) + strlen(".dummykey") + 1; char* sidecar = malloc(n); if (sidecar) { @@ -82,6 +83,13 @@ static void rm_temp(const char* path) { unlink(sidecar); free(sidecar); } + n = strlen(path) + strlen(".dummykey.tmp.") + 32; + char* tmp = malloc(n); + if (tmp) { + snprintf(tmp, n, "%s.dummykey.tmp.%ld", path, (long)getpid()); + unlink(tmp); + free(tmp); + } } /* `.dummykey` sidecar path (caller frees). */ @@ -862,6 +870,113 @@ static void test_credentials_dummy_key_rejects_bad_sidecar() { rm_temp(path); free(path); free(sidecar); + + /* Exact-mode rule: 0400 has no group/other bits but is not 0600, so it is + * rejected now (the mode must be exactly owner read+write). */ + path = make_tmp_file(contents); + EXPECT_NOT_NULL(path); + sidecar = dummy_sidecar_path(path); + EXPECT_NOT_NULL(sidecar); + fp = fopen(sidecar, "wb"); + EXPECT_NOT_NULL(fp); + EXPECT_TRUE(fwrite(key, 1, sizeof(key), fp) == sizeof(key)); + fclose(fp); + EXPECT_EQ_INT(chmod(sidecar, 0400), 0); + EXPECT_NULL(credentials_load(path, NULL, err, sizeof(err))); + EXPECT_TRUE(err[0] != '\0'); + rm_temp(path); + free(path); + free(sidecar); +} + +/* A pre-existing valid sidecar is adopted verbatim (no regeneration): the + * unknown-user dummy salt must equal HMAC-SHA256(known key, username), and a + * reload must yield the same salt. */ +static void test_credentials_dummy_key_existing_sidecar_adopted() { + char line[CREDENTIAL_MAX_LINE]; + EXPECT_TRUE(make_store_line("alice", KAT_PASSWORD, CREDENTIAL_MIN_ITERS, line, sizeof(line))); + char contents[CREDENTIAL_MAX_LINE + 2]; + snprintf(contents, sizeof(contents), "%s\n", line); + char* path = make_tmp_file(contents); + EXPECT_NOT_NULL(path); + char* sidecar = dummy_sidecar_path(path); + EXPECT_NOT_NULL(sidecar); + + /* Pre-create a valid owner-only sidecar with a known key. */ + uint8_t key[CREDENTIAL_KEY_LEN]; + memset(key, 0x5a, sizeof(key)); + FILE* fp = fopen(sidecar, "wb"); + EXPECT_NOT_NULL(fp); + EXPECT_TRUE(fwrite(key, 1, sizeof(key), fp) == sizeof(key)); + fclose(fp); + EXPECT_EQ_INT(chmod(sidecar, 0600), 0); + + char err[512]; + CredentialStore* store = credentials_load(path, NULL, err, sizeof(err)); + EXPECT_NOT_NULL(store); + CredentialVerifier v1; + EXPECT_TRUE(credentials_get_verifier(store, "unknown-user", NULL, 0, &v1)); + EXPECT_FALSE(v1.found); + /* HMAC-SHA256(0x5a * 32, "unknown-user")[:16], computed independently. */ + uint8_t expect[CREDENTIAL_SALT_LEN]; + unhex("4b0d2e6b73025cc2fcb41d0a710ff469", expect, sizeof(expect)); + EXPECT_TRUE(memcmp(v1.salt, expect, sizeof(expect)) == 0); + credentials_free(store); + + /* The adopted sidecar still holds exactly the pre-created key (not a fresh + * random one). */ + uint8_t readback[CREDENTIAL_KEY_LEN]; + fp = fopen(sidecar, "rb"); + EXPECT_NOT_NULL(fp); + EXPECT_TRUE(fread(readback, 1, sizeof(readback), fp) == sizeof(readback)); + fclose(fp); + EXPECT_TRUE(memcmp(readback, key, sizeof(key)) == 0); + + /* Persisted across a reload. */ + CredentialVerifier v2; + store = credentials_load(path, NULL, err, sizeof(err)); + EXPECT_NOT_NULL(store); + EXPECT_TRUE(credentials_get_verifier(store, "unknown-user", NULL, 0, &v2)); + EXPECT_TRUE(memcmp(v1.salt, v2.salt, sizeof(v1.salt)) == 0); + credentials_free(store); + + rm_temp(path); + free(path); + free(sidecar); +} + +/* A symlink planted at the sidecar path must fail the load closed (O_NOFOLLOW), + * even when it resolves to a valid owner-only key file. */ +static void test_credentials_dummy_key_symlink_rejected() { + char line[CREDENTIAL_MAX_LINE]; + EXPECT_TRUE(make_store_line("alice", KAT_PASSWORD, CREDENTIAL_MIN_ITERS, line, sizeof(line))); + char contents[CREDENTIAL_MAX_LINE + 2]; + snprintf(contents, sizeof(contents), "%s\n", line); + char* path = make_tmp_file(contents); + EXPECT_NOT_NULL(path); + char* sidecar = dummy_sidecar_path(path); + EXPECT_NOT_NULL(sidecar); + + char target[256]; + snprintf(target, sizeof(target), "/tmp/fs_cred_key_%d_%d", (int)getpid(), g_file_counter++); + uint8_t key[CREDENTIAL_KEY_LEN]; + memset(key, 0x5a, sizeof(key)); + FILE* fp = fopen(target, "wb"); + EXPECT_NOT_NULL(fp); + EXPECT_TRUE(fwrite(key, 1, sizeof(key), fp) == sizeof(key)); + fclose(fp); + EXPECT_EQ_INT(chmod(target, 0600), 0); + EXPECT_EQ_INT(symlink(target, sidecar), 0); + + char err[512]; + EXPECT_NULL(credentials_load(path, NULL, err, sizeof(err))); + EXPECT_TRUE(err[0] != '\0'); + + unlink(sidecar); /* remove the symlink itself, not its target */ + unlink(target); + rm_temp(path); + free(path); + free(sidecar); } /* A NULL store path has nowhere to persist a key, so each load gets a fresh @@ -923,6 +1038,8 @@ void test_credentials(void) { test_credentials_rejects_group_or_other_accessible(); test_credentials_dummy_key_persisted(); test_credentials_dummy_key_rejects_bad_sidecar(); + test_credentials_dummy_key_existing_sidecar_adopted(); + test_credentials_dummy_key_symlink_rejected(); test_credentials_dummy_key_null_store_ephemeral(); test_credentials_burn(); } From d53614d06bbdfde45f0e73fe848833b35cde6db1 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sat, 12 Sep 2026 19:18:29 +0200 Subject: [PATCH 4/6] fix(a7-3/s1): fail closed on non-loopback peers; require plaintext opt-in before challenge utils_fd_peer_is_local now returns true only when getpeername SUCCEEDS and the peer address classifies as loopback. A non-socket descriptor (pipe/socketpair) or any getpeername error is NOT local, so the daemon auth gate fails closed instead of treating an untestable --stdio pipe as trusted (daemon auth modules are --daemon-only and the stdio path never loads a daemon config). server_module_gate now requires --allow-unauthenticated for the loopback plaintext auth path: a plaintext loopback connection without the operator opt-in is refused at the config gate BEFORE server_auth_handshake, so no SCRAM challenge is sent. Remote peers still require verified TLS regardless of the flag; the handler keeps its defense-in-depth checks. Docs state the exact policy (verified TLS with matching --client-cn, or operator-opted-in loopback plaintext), drop the SSH/stdio auth-transport claim (they are daemon-only), and add the loopback trust-boundary relay caveat and the CN-only (no SAN) residual. Adds a unit-test negative for pipe/socketpair and an integration test where a relay observes no challenge when the flag is absent. --- README.md | 29 +++++++++++++++------- RSYNC_COMPAT.md | 4 +-- src/server/server.c | 19 +++++++++------ src/shared/utils.c | 12 +++++---- src/shared/utils.h | 7 +++++- tests/integration/test_daemon.py | 42 +++++++++++++++++++++++++++++++- tests/test_shared_utils.c | 15 +++++++++--- 7 files changed, 99 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index ad93fed..5aa45cf 100644 --- a/README.md +++ b/README.md @@ -528,15 +528,26 @@ restart-gated enumeration channel remains (persisting a dummy key is out of scope); and the store iteration count is observable pre-auth by design, since the miss path must match a hit. -An `auth users` module only accepts credentials over an encrypted, verified TLS -connection whose client certificate matches the server's `--client-cn`, or over -a local/SSH transport (a loopback TCP peer or the `--stdio` pipe). A remote -plaintext peer is refused before any challenge is sent, and -`--allow-unauthenticated` does **not** relax this: that flag only relaxes the -standalone plaintext gate. Clients sending daemon credentials with -`--password-file` to a non-loopback daemon must therefore use `--tls`; the -client rejects a non-local plaintext credential destination before any network -I/O. +An `auth users` module accepts credentials only when one of two conditions +holds: (a) the connection is an encrypted, verified TLS connection whose client +certificate matches the server's `--client-cn`, or (b) the connection is +plaintext from a loopback peer **and** the operator explicitly passed +`--allow-unauthenticated`. A remote plaintext peer is refused before any +challenge is sent, and `--allow-unauthenticated` never permits remote plaintext +auth: remote peers still require verified TLS regardless of the flag. Clients +sending daemon credentials with `--password-file` to a non-loopback daemon must +therefore use `--tls`; the client rejects a non-local plaintext credential +destination before any network I/O. Daemon modules are a `--daemon`-only +feature: the SSH `--stdio` path never loads a daemon config and is not an auth +transport for them. + +Because the loopback allowance trusts whichever peer the kernel reports as +`127.0.0.1`, it assumes nothing relays remote connections to the daemon. A local +TCP forwarder or a TLS-terminating proxy in front of an auth-module listener +makes remote clients appear as loopback and bypasses the mutual-TLS identity +check, so do not front an auth-module listener with such a relay. Note also that +`--client-cn` matches the certificate's CN only (not a subjectAltName), which is +acceptable for a private CA. TLS provides encrypted TCP transport. Supplying `--ca` enables certificate verification; without it, traffic is encrypted but peer identity is not diff --git a/RSYNC_COMPAT.md b/RSYNC_COMPAT.md index b46af90..8b96e8a 100644 --- a/RSYNC_COMPAT.md +++ b/RSYNC_COMPAT.md @@ -639,9 +639,9 @@ now transmits targets (the prior behavior was broken/partial); its status moved - **Module selection & confinement:** the client requests a module with an rsync-style `host::module[/path]` destination. The module name crosses the wire as a trailing string on the config frame (bumping `PROTOCOL_VERSION` 2.14.0 → 2.15.0; the bump is required because the config-frame layout changed and the strict same-version handshake is what prevents a peer from desynchronizing on the new trailing field). The daemon looks the module up in ITS OWN config and uses the module's `path` as the authorized root through the exact same `configure_authorization` confinement the standalone server applies to `--destination-root` (`file_open_secure_parent`, `has_path_traversal`, `path_is_within`); the client never supplies the root, every client-chosen-ownership/super-user request is refused unless the module declares `client owner = yes` (the daemon's per-module opt-in, see below), and the operator `--no-super` veto forces super-user activities off for every daemon connection. The client's `/path` part is relative inside the module and is rejected if absolute or if it contains `..`. Unknown modules are refused before any data moves (the run fails cleanly at the config handshake). An absolute destination and a module request against a non-daemon server are also refused. - **`client owner` (client-chosen-ownership opt-in):** by default a daemon module refuses every request that would let the client pick an owner or ask for super-user activities — `--numeric-ids`, `--chown`, `--usermap`/`--groupmap`, `--fake-super`, `--copy-as`, and an explicit `--super` — at the config handshake (before `STATUS_OK`), because a daemon has no per-module opt-in for client-chosen ownership and any anonymous client could otherwise force arbitrary owner ids inside the module root. `client owner = yes` opts a single module in, allowing those requests within that module's root (the standalone listener and the SSH `--stdio` server always honor them for their single operator-authorized root). Without the opt-in the daemon also forces super-user **device** activity off for that connection — char/block device-node creation (`--devices`) and `--write-devices` — even under the default `AUTO` mode, so a non-opted module can never be made to `mknod` or write a raw device; those entries are skipped (not refused) so an ordinary `-a` push still succeeds without device nodes. The opt-in does **not** lift the privilege requirement: `--copy-as` still needs a root receiver, and the operator `--no-super` veto still forces super-user activities off for every connection. The daemon logs a prominent startup warning for each `client owner = yes` module so the operator's deliberate choice is visible. - **`read only` safe default:** every network transfer FastSync currently supports is a push that writes under the module root, so a `read only` module refuses the connection (clear server log "module is read only"; the client exits non-zero, nothing is transferred). A future pull/list operation can be opened up when it exists; the knob is already stored. -- **`auth users` (A7 SCRAM-SHA-256 authentication):** a module that declares `auth users` requires the client to present credentials. The config frame carries ONLY the username; the daemon answers an auth-required module with `STATUS_AUTH_CHALLENGE` (PBKDF2 iteration count, 16-byte salt, 32-byte server nonce), the client answers with `STATUS_AUTH_RESPONSE` (fresh 32-byte client nonce + a 32-byte ClientProof), and the daemon accepts only when the proof verifies **and** the username is **on the module's `auth users` list** and has a store entry, replying `STATUS_AUTH_OK` with a 32-byte ServerSignature the client verifies before proceeding. Verification is constant-time over fixed 32-byte keys (the compare runs even for a miss), username membership uses a constant-time full-length scan, and an unknown/off-list user still receives a challenge and runs the same math against a dummy verifier: a deterministic per-username salt (`HMAC-SHA256(store dummy key, username)`), the store-wide uniform iteration count and dummy keys. Re-probing the same unknown username therefore yields an identical salt and iteration count while a different username yields a different salt, so there is no user-enumeration or timing oracle. The daemon logs the username but **never the password, proof or keys**. A module WITHOUT `auth users` stays open (legitimate rsync configuration); credentials sent to such a module are ignored. Read-only is orthogonal: even a correctly authenticated push to a `read only` module is still refused (all FastSync network transfers write). Fail-closed policy: a daemon whose config declares `auth users` on any module refuses to start unless a credential store was given (`--password-file` and/or `--early-input`); a missing or empty store is never silently treated as "open". A failed handshake (missing credentials, unknown/off-list user, wrong proof or malformed data) yields a single generic `STATUS_AUTH_FAILED` and the daemon closes before any data moves. Two residuals are accepted: the dummy salt is stable within one daemon lifetime but changes across restarts, leaving a restart-gated enumeration channel (persisting the dummy key is out of scope); and the store iteration count is observable pre-auth by design, since the miss path must match a hit. **Transport policy (hardening A7-3/S1):** an auth-required module accepts credentials only over an encrypted, verified TLS connection whose client certificate matches `--client-cn`, or over a local/SSH transport (a loopback TCP peer, or the `--stdio` pipe); a remote plaintext peer is refused at the config gate before any challenge is sent, and `--allow-unauthenticated` does **not** relax this. +- **`auth users` (A7 SCRAM-SHA-256 authentication):** a module that declares `auth users` requires the client to present credentials. The config frame carries ONLY the username; the daemon answers an auth-required module with `STATUS_AUTH_CHALLENGE` (PBKDF2 iteration count, 16-byte salt, 32-byte server nonce), the client answers with `STATUS_AUTH_RESPONSE` (fresh 32-byte client nonce + a 32-byte ClientProof), and the daemon accepts only when the proof verifies **and** the username is **on the module's `auth users` list** and has a store entry, replying `STATUS_AUTH_OK` with a 32-byte ServerSignature the client verifies before proceeding. Verification is constant-time over fixed 32-byte keys (the compare runs even for a miss), username membership uses a constant-time full-length scan, and an unknown/off-list user still receives a challenge and runs the same math against a dummy verifier: a deterministic per-username salt (`HMAC-SHA256(store dummy key, username)`), the store-wide uniform iteration count and dummy keys. Re-probing the same unknown username therefore yields an identical salt and iteration count while a different username yields a different salt, so there is no user-enumeration or timing oracle. The daemon logs the username but **never the password, proof or keys**. A module WITHOUT `auth users` stays open (legitimate rsync configuration); credentials sent to such a module are ignored. Read-only is orthogonal: even a correctly authenticated push to a `read only` module is still refused (all FastSync network transfers write). Fail-closed policy: a daemon whose config declares `auth users` on any module refuses to start unless a credential store was given (`--password-file` and/or `--early-input`); a missing or empty store is never silently treated as "open". A failed handshake (missing credentials, unknown/off-list user, wrong proof or malformed data) yields a single generic `STATUS_AUTH_FAILED` and the daemon closes before any data moves. Two residuals are accepted: the dummy salt is stable within one daemon lifetime but changes across restarts, leaving a restart-gated enumeration channel (persisting the dummy key is out of scope); and the store iteration count is observable pre-auth by design, since the miss path must match a hit. **Transport policy (hardening A7-3/S1):** an auth-required module accepts credentials only when either (a) the connection is an encrypted, verified TLS connection whose client certificate matches `--client-cn`, or (b) the connection is plaintext from a loopback TCP peer **and** the operator explicitly passed `--allow-unauthenticated`. A remote plaintext peer, and a loopback plaintext peer without that flag, are refused at the config gate before any challenge is sent; `--allow-unauthenticated` never permits remote plaintext auth (remote peers still require verified TLS). Daemon modules are a `--daemon`-only feature — the SSH `--stdio` path never loads a daemon config and is not an auth transport for them. Because the loopback allowance trusts whichever peer the kernel reports as `127.0.0.1`, it assumes nothing relays remote connections to the daemon: a local TCP forwarder or TLS-terminating proxy in front of an auth-module listener makes remote clients appear as loopback and bypasses the mutual-TLS identity check, so do not front an auth-module listener with such a relay. - **Credential store format:** server `--password-file`/`--early-input` files are line-based `user:$fastsync$1$pbkdf2-sha256$$$$`, one per line (standard base64; 16-byte salt, 32-byte keys; `iters` in `[100000, 10000000]`, default 600000). Every entry in the resulting store must agree on `iters` (a store whose entries disagree, or where a layered `--early-input` disagrees with `--password-file`, is rejected). Generate lines with `fastsync-server --hash-credentials FILE [--iterations N]`; the emitted lines are secret material, so redirect them to an owner-only (mode 0600) file (the tool warns on stderr if stdout is a group/other-accessible regular file). Blank lines and lines starting with `#`/`;` are comments; the parser is strict (a malformed line fails the whole load, so a typo can never let a different set of users in). **The legacy `user:SHA256HEX` form is hard-rejected** with an actionable "legacy" error; there is no auto-upgrade, so a replayable bearer digest can never be loaded by a 2.19.0 daemon. The client `--password-file` holds `user:password` on its first meaningful line (the literal password, used only for the handshake then burned); keep both files readable only by their owner (mode 0600). Per-username wire length is bounded (256 chars) and every decoded salt/key length is validated. -- **Plaintext caveat:** an auth-required module is refused, **before any challenge is sent**, unless the connection is encrypted and verified TLS whose client certificate matches the server's `--client-cn`, or a local/SSH transport (a loopback TCP peer, or the `--stdio` pipe). A remote plaintext peer never receives a challenge, and `--allow-unauthenticated` does **not** relax this policy (that flag only relaxes the standalone plaintext gate). On the loopback/SSH transports that remain permitted, a local sniffer could still read the challenge and response and mount an **offline dictionary attack** against a weak password, so use `--tls` for any real deployment. Clients sending daemon credentials with `--password-file` to a non-loopback daemon must use `--tls`; the client rejects such a destination before any network I/O. Unlike the old challenge-less exchange there is **no replay**: the proof is bound to the fresh per-connection server nonce, so a captured `STATUS_AUTH_RESPONSE` cannot be reused on another connection (an integration test proxies the daemon and proves this). TLS client-CN (`--client-cn`) is an independent transport identity check and composes with password auth: both may be required on the same connection. +- **Plaintext caveat:** an auth-required module is refused, **before any challenge is sent**, unless the connection is encrypted and verified TLS whose client certificate matches the server's `--client-cn`, or it is plaintext from a loopback TCP peer **and** the operator passed `--allow-unauthenticated`. A remote plaintext peer, and a loopback plaintext peer without that flag, never receive a challenge, and `--allow-unauthenticated` never permits remote plaintext auth (remote peers still require verified TLS). On the loopback plaintext transport that remains permitted, a local sniffer could still read the challenge and response and mount an **offline dictionary attack** against a weak password, so use `--tls` for any real deployment. `--client-cn` matches the certificate CN only (not a subjectAltName), which is acceptable for a private CA. Clients sending daemon credentials with `--password-file` to a non-loopback daemon must use `--tls`; the client rejects such a destination before any network I/O. Unlike the old challenge-less exchange there is **no replay**: the proof is bound to the fresh per-connection server nonce, so a captured `STATUS_AUTH_RESPONSE` cannot be reused on another connection (an integration test proxies the daemon and proves this). TLS client-CN (`--client-cn`) is an independent transport identity check and composes with password auth: both may be required on the same connection. - **Wire/protocol:** the config-frame auth block is now `[int present][str_redacted username]` (the old digest field is gone), and the frame stream gains the challenge/response (`STATUS_AUTH_CHALLENGE` → `STATUS_AUTH_RESPONSE` → `STATUS_AUTH_OK`/`STATUS_AUTH_FAILED`) between the config frame and the `STATUS_OK` ack. Both are wire-layout changes, so `PROTOCOL_VERSION` is bumped **2.18.0 → 2.19.0** (see the A7 note in `src/shared/config.h`); the strict same-version handshake keeps a 2.19 client and a 2.18 server from desynchronizing. - **Client side:** `host::module/path` selects the TCP transport and connects to `--server-port`; `host:path` stays the SSH transport; plain paths stay local TCP. The daemon username comes from `--password-file` (first `user:password` line), and `--password-file` without a `host::module/path` destination is a client error (fail fast). A `user@host::module` form is rejected with a pointer to `--password-file`. The client's plaintext password is wiped from memory (`config_burn_auth`) at transfer teardown. - **MOTD (Wave C):** a daemon configured with a global `motd file` sends that file's content as the first server→client string frame after the config-frame STATUS_OK ack (rsync sends the MOTD as the first thing from the server at the start of a daemon connection). Only the daemon listener path (`host::module`) gets a MOTD; the `--stdio` SSH path never sends or reads one. The server reads the file bounded to 4096 bytes and treats an absent/unreadable file as "no MOTD" (an empty frame, never an error). The exchange is server→client only and does **not** bump `PROTOCOL_VERSION`: every 2.15.0 daemon client reads the frame after the ack, so sender and receiver stay in lockstep (see the Wave C note in `src/shared/config.h`). `--no-motd` is the client-side suppression switch: the client still reads (consumes) the frame to keep the stream in sync but does not display it. The MOTD is printed to stdout with control bytes (ESC included) escaped octal-style while newlines/tabs are preserved, so a hostile server cannot inject terminal escape sequences. diff --git a/src/server/server.c b/src/server/server.c index ac0cd6f..77817fc 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -383,19 +383,22 @@ static const char* server_module_gate(const Config* config, void* context) { "store is configured"; } /* Transport policy (A7-3/S1): an auth-required module only accepts - * credentials over an encrypted, verified TLS connection whose client - * certificate matches --client-cn, or over a local/SSH transport (a - * loopback TCP peer, or the --stdio pipe). A remote plaintext peer is - * refused HERE, before the challenge is sent, so an unverified client never - * receives a nonce. --allow-unauthenticated is intentionally NOT consulted: - * that flag relaxes the standalone plaintext gate, never this one. */ + * credentials over (a) an encrypted, verified TLS connection whose client + * certificate matches --client-cn, or (b) a plaintext connection from a + * loopback peer that the operator explicitly opted into with + * --allow-unauthenticated. A remote plaintext peer and an un-flagged + * loopback plaintext peer are both refused HERE, before the challenge is + * sent, so an unverified client never receives a nonce. The operator flag + * never permits REMOTE plaintext auth: remote peers still require verified + * TLS regardless of the flag. */ bool tls_ok = gate_ctx && gate_ctx->ssl && SSL_get_verify_result(gate_ctx->ssl) == X509_V_OK && tls_client_identity_allowed(gate_ctx->ssl); - bool local_ok = gate_ctx && gate_ctx->fd >= 0 && utils_fd_peer_is_local(gate_ctx->fd); + bool local_ok = allow_unauthenticated && gate_ctx && gate_ctx->fd >= 0 && + utils_fd_peer_is_local(gate_ctx->fd); if (!tls_ok && !local_ok) { log_message(LOG_LEVEL_ERROR, "daemon module '%s' requires authentication over an encrypted, verified TLS " - "connection (or a local/SSH transport); refusing", + "connection (or an opted-in loopback plaintext transport); refusing", config->module); return "daemon module requires authentication over an encrypted, verified TLS " "connection"; diff --git a/src/shared/utils.c b/src/shared/utils.c index df92cc4..16ea671 100644 --- a/src/shared/utils.c +++ b/src/shared/utils.c @@ -571,17 +571,19 @@ bool utils_sockaddr_is_loopback(const struct sockaddr* addr) { return false; } -/* True when the fd's peer is a local channel: a loopback TCP peer, or a - non-socket descriptor (the --stdio SSH transport is a pipe, so a failed - getpeername with ENOTSOCK counts as local). Any other socket peer is not - local. */ +/* True when the fd's peer is provably a loopback TCP peer: getpeername must + succeed AND the returned address must classify as loopback. Everything else + is NOT local, including a non-socket descriptor (pipe/socketpair): a failed + getpeername (ENOTSOCK, ENOTCONN, ...) fails closed. The daemon auth gate + must not treat "I cannot tell" as "trusted", and daemon auth modules are + daemon-only anyway (the --stdio path never loads a daemon config). */ bool utils_fd_peer_is_local(int fd) { if (fd < 0) return false; struct sockaddr_storage peer; socklen_t length = sizeof(peer); if (getpeername(fd, (struct sockaddr*)&peer, &length) != 0) - return errno == ENOTSOCK; + return false; return utils_sockaddr_is_loopback((const struct sockaddr*)&peer); } diff --git a/src/shared/utils.h b/src/shared/utils.h index b7a6208..4d09c67 100644 --- a/src/shared/utils.h +++ b/src/shared/utils.h @@ -68,7 +68,12 @@ bool append_resume_eligible(unsigned long long old_size, unsigned long long chec bool append_tail_length(unsigned long long old_size, unsigned long long check_size, unsigned long long* tail_out); /* Loopback / local-transport classification for the daemon auth gate and the - client credential rule. See utils.c for the exact accepted forms. */ + client credential rule. utils_sockaddr_is_loopback accepts 127.0.0.0/8, + IPv6 ::1 and IPv4-mapped ::ffff:127.x.x.x; utils_host_is_loopback additionally + accepts the literal "localhost". utils_fd_peer_is_local is fail-closed: it is + true only when getpeername SUCCEEDS and reports a loopback peer -- a non-socket + descriptor (pipe/socketpair) or any getpeername error yields false. See + utils.c for the exact accepted forms. */ bool utils_sockaddr_is_loopback(const struct sockaddr* addr); bool utils_fd_peer_is_local(int fd); bool utils_host_is_loopback(const char* host); diff --git a/tests/integration/test_daemon.py b/tests/integration/test_daemon.py index 91bd6ed..165d72e 100644 --- a/tests/integration/test_daemon.py +++ b/tests/integration/test_daemon.py @@ -597,6 +597,9 @@ class _AuthReplayProxy: self.server.settimeout(20) self.port = self.server.getsockname()[1] self.stolen = None + # Set when a relayed connection received a SCRAM challenge from the + # backend; lets a test assert the daemon refused before any challenge. + self.saw_challenge = False def close(self): try: @@ -634,6 +637,7 @@ class _AuthReplayProxy: if len(buf_s) >= 4: (status,) = struct.unpack_from("= 0); From ac3c4c7c72adbe6108377b614aca313c66faa760 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sat, 12 Sep 2026 19:29:23 +0200 Subject: [PATCH 5/6] fix(a7-auth): harden dummy-key temp creation - Make the atomic-publish temp name unpredictable by appending 16 random hex chars to the pid, so a leftover/planted temp cannot be targeted. - On EEXIST, unlink the stale temp and retry the O_EXCL create once (bounded), so a crash leftover or reused pid cannot silently defeat sidecar persistence. - fchmod the temp fd to 0600 after creation (umask can clear owner bits) and treat failure as a create failure, so the published sidecar is always exactly 0600. - Clarify comments: the sidecar requires exact 0600 while the store and password files only reject group/other bits. - Add a unit test that a restrictive umask still yields an exact 0600 sidecar; clean random-suffixed temps in tests. --- src/shared/credentials.c | 96 +++++++++++++++++++++++++++++----------- src/shared/credentials.h | 15 ++++--- tests/test_credentials.c | 57 ++++++++++++++++++++---- 3 files changed, 129 insertions(+), 39 deletions(-) diff --git a/src/shared/credentials.c b/src/shared/credentials.c index 3dbc3e5..f1a0c98 100644 --- a/src/shared/credentials.c +++ b/src/shared/credentials.c @@ -36,7 +36,7 @@ struct CredentialStore { * challenged with the same count as a hit and the count itself never leaks * membership. Unused (0) for an empty store. */ uint32_t iters; - /* Store-wide secret loaded from (or created in) the owner-only + /* Store-wide secret loaded from (or created in) the exact-mode-0600 * `.dummykey` sidecar, so it also survives a daemon restart. The * dummy salt handed out for an unknown/off-list user is * HMAC-SHA256(dummy_key, username)[:SALT_LEN], so repeated probes of the same @@ -49,8 +49,8 @@ struct CredentialStore { /* Exact marker prefix of the new store verifier field. */ #define CREDENTIAL_STORE_PREFIX "$fastsync$1$pbkdf2-sha256$" #define CREDENTIAL_AUTH_PREFIX "FastSync-Auth-v1" -/* Owner-only sidecar holding the persistent store-wide dummy key, placed next to - * the credential store (`.dummykey`). */ +/* Exact-mode-0600 sidecar holding the persistent store-wide dummy key, placed + * next to the credential store (`.dummykey`). */ #define CREDENTIAL_DUMMY_KEY_SUFFIX ".dummykey" /* Fixed dummy keys used when a user is unknown or off the module's list. They @@ -73,7 +73,10 @@ static bool is_comment_char(char 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 + * permission bit (so 0600 and stricter modes such as 0400 are accepted), + * mirroring the TLS private-key check. This only rejects group/other bits, + * deliberately unlike the dummy-key sidecar which requires EXACT mode 0600. 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. @@ -591,7 +594,7 @@ static CredentialStore* load_store_file(const char* path, char* err, size_t err_ } /* Validate and read an already-open `.dummykey` sidecar. Fails closed on - * anything that is not an owner-only (0600) regular file of exactly + * anything that is not an exact-mode-0600 regular file of exactly * CREDENTIAL_KEY_LEN bytes, so a loosened, swapped or truncated file can never * silently change the dummy challenge. */ static bool read_dummy_key_fd(int fd, const char* path, uint8_t out[CREDENTIAL_KEY_LEN], char* err, @@ -656,7 +659,7 @@ static void fsync_containing_dir(const char* path) { } /* Load the persistent dummy key for `store_path` from its `.dummykey` - * sidecar, creating it (mode 0600, 32 random bytes) if absent. A NULL + * sidecar, creating it (exact mode 0600, 32 random bytes) if absent. A NULL * store_path (empty store) yields a fresh ephemeral key. Reading an existing * sidecar fails CLOSED on any validation error; only the CREATE path degrades * to an ephemeral key (with a warning) when the filesystem cannot hold the @@ -714,12 +717,41 @@ static bool load_or_create_dummy_key(const char* store_path, uint8_t out[CREDENT /* Publish atomically: write a private same-directory temp file, fsync it, * then hard-link it into place. A concurrent reader therefore only ever - * sees a complete 32-byte sidecar (or none), never a partial/zero file. */ - char pid_suffix[32]; - int pn = snprintf(pid_suffix, sizeof(pid_suffix), ".tmp.%ld", (long)getpid()); - if (pn < 0 || (size_t)pn >= sizeof(pid_suffix)) { + * sees a complete 32-byte sidecar (or none), never a partial/zero file. + * + * The temp name carries both the pid and a fresh random suffix, so it is not + * predictable. If the name nevertheless already exists (a SIGKILL/crash + * leftover, pid reuse, or a planted file) the stale temp is removed and the + * O_EXCL create is retried once, so it can never silently defeat persistence + * for this pid. */ + uint8_t fresh[CREDENTIAL_KEY_LEN]; + if (!credentials_random_bytes(fresh, CREDENTIAL_KEY_LEN)) { + set_error(err, err_size, "failed to generate the credential store dummy key"); + free(sidecar); + return false; + } + + uint8_t name_rand[8]; + if (!credentials_random_bytes(name_rand, sizeof(name_rand))) { + set_error(err, err_size, "failed to generate the dummy key temp name"); + free(sidecar); + credentials_burn((char*)fresh, sizeof(fresh)); + return false; + } + char name_hex[sizeof(name_rand) * 2 + 1]; + static const char hex_digits[] = "0123456789abcdef"; + for (size_t i = 0; i < sizeof(name_rand); i++) { + name_hex[2 * i] = hex_digits[name_rand[i] >> 4]; + name_hex[2 * i + 1] = hex_digits[name_rand[i] & 0x0f]; + } + name_hex[sizeof(name_hex) - 1] = '\0'; + + char tmp_suffix[64]; + int pn = snprintf(tmp_suffix, sizeof(tmp_suffix), ".tmp.%ld.%s", (long)getpid(), name_hex); + if (pn < 0 || (size_t)pn >= sizeof(tmp_suffix)) { set_error(err, err_size, "failed to build the dummy key temp path"); free(sidecar); + credentials_burn((char*)fresh, sizeof(fresh)); return false; } size_t sidecar_len = (size_t)n; @@ -728,24 +760,38 @@ static bool load_or_create_dummy_key(const char* store_path, uint8_t out[CREDENT if (!tmp) { set_error(err, err_size, "out of memory building the dummy key temp path"); free(sidecar); + credentials_burn((char*)fresh, sizeof(fresh)); return false; } - snprintf(tmp, tmp_len + 1, "%s%s", sidecar, pid_suffix); + snprintf(tmp, tmp_len + 1, "%s%s", sidecar, tmp_suffix); - uint8_t fresh[CREDENTIAL_KEY_LEN]; - if (!credentials_random_bytes(fresh, CREDENTIAL_KEY_LEN)) { - set_error(err, err_size, "failed to generate the credential store dummy key"); - free(tmp); - free(sidecar); - return false; + /* Bounded create: at most one unlink+retry on EEXIST. The retry keeps + * O_EXCL, so only a stale name is reclaimed and a live peer's temp is never + * truncated. */ + int create_errno = 0; + for (int attempt = 0; attempt < 2; attempt++) { + fd = open(tmp, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0600); + if (fd >= 0) + break; + create_errno = errno; + if (create_errno != EEXIST || attempt == 1) + break; + unlink(tmp); + } + /* umask can clear owner bits from the 0600 create mode while the reader + * requires an exact 0600, so force the mode on the fd before publishing; a + * failure here is treated like any other create failure (warning + ephemeral + * key) so the published sidecar is always exactly 0600. */ + if (fd >= 0 && fchmod(fd, 0600) != 0) { + create_errno = errno; + close(fd); + unlink(tmp); + fd = -1; } - - fd = open(tmp, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0600); if (fd < 0) { - /* Creation failed (read-only filesystem, missing directory, ...). Warn and - * fall back to an ephemeral key: unknown-user challenges stay deterministic - * within this daemon lifetime but will change on the next restart. */ - int create_errno = errno; + /* Creation failed (read-only filesystem, missing directory, fchmod, ...). + * Warn and fall back to an ephemeral key: unknown-user challenges stay + * deterministic within this daemon lifetime but will change on restart. */ char* escaped = output_escape(tmp, log_get_8_bit_output()); log_message(LOG_LEVEL_WARNING, "cannot create dummy key file %s: %s; using a transient dummy key so unknown-user " @@ -862,8 +908,8 @@ CredentialStore* credentials_load(const char* password_file, const char* early_i /* Load (or create) the store-wide dummy key once for the final (possibly * merged) store. It makes an unknown-user challenge deterministic AND * stable across daemon restarts, so a restart cannot be used as a - * username-enumeration oracle. It is persisted in an owner-only sidecar next - * to the credential store; a NULL store path (empty store) keeps it + * username-enumeration oracle. It is persisted in an exact-mode-0600 sidecar + * next to the credential store; a NULL store path (empty store) keeps it * ephemeral. Fail the load if the CSPRNG is unavailable rather than * degrading the anti-enumeration property. */ const char* store_path = password_file ? password_file : early_input_file; diff --git a/src/shared/credentials.h b/src/shared/credentials.h index d6314b2..116ff13 100644 --- a/src/shared/credentials.h +++ b/src/shared/credentials.h @@ -26,14 +26,16 @@ * hard-rejected with an actionable "legacy" error; there is no auto-upgrade. * Use `fastsync-server --hash-credentials` to generate new-format lines. * - * Alongside the store, credentials_load maintains an owner-only (0600) + * Alongside the store, credentials_load maintains an exact-mode-0600 * `.dummykey` sidecar holding the store-wide random dummy key. It * is auto-created on first load and MUST be preserved across restarts: it makes * the dummy challenge for an unknown user stable for the life of the store, so * a daemon restart cannot be used as a username-enumeration oracle. A sidecar - * that is not an owner-only regular file of exactly 32 bytes fails the load - * (fail closed); if it cannot be created (e.g. a read-only mount) the daemon - * warns and uses a transient per-run key instead. + * that is not an exact-mode-0600 regular file of exactly 32 bytes fails the load + * (fail closed); if it cannot be created (e.g. a read-only mount or a restrictive + * umask the fchmod cannot repair) the daemon warns and uses a transient per-run + * key instead. NOTE: the sidecar requires EXACT 0600, whereas the store / + * password files only reject group/other bits (a deliberate difference). * * Client --password-file format: the FIRST meaningful (non-comment, non-blank) * line is `user:password`, holding the literal password. The client keeps it @@ -165,8 +167,9 @@ bool credentials_verify_response(const CredentialVerifier* v, const char* user, bool credentials_hash_store_line(const char* user, const char* password, uint32_t iters, char* out, size_t out_sz, char* err, size_t err_size); -/* Read `user:password` lines from `path` (the same owner-only check as the - * other secret files) and write one new-format store line per entry to `out`. +/* Read `user:password` lines from `path` (the same no-group/other-bits check as + * the other secret files) and write one new-format store line per entry to + * `out`. * Blank/comment lines are skipped; a malformed line fails the whole run. * Returns 0 on success, -1 on error (err filled). Used by * `--hash-credentials`. */ diff --git a/tests/test_credentials.c b/tests/test_credentials.c index 8832bd1..86d4dd6 100644 --- a/tests/test_credentials.c +++ b/tests/test_credentials.c @@ -3,6 +3,7 @@ #include "test_utils.h" #include "utils.h" #include +#include #include #include #include @@ -73,9 +74,10 @@ static void rm_temp(const char* path) { if (!path) return; unlink(path); - /* Every successfully loaded store auto-creates an owner-only + /* Every successfully loaded store auto-creates an exact-mode-0600 * `.dummykey` sidecar; remove it too so tests leave no stray key. The - * atomic-publish temp name is also removed defensively. */ + * atomic-publish temps carry a random suffix, so glob them all and remove any + * that a failing path may have left behind. */ size_t n = strlen(path) + strlen(".dummykey") + 1; char* sidecar = malloc(n); if (sidecar) { @@ -83,12 +85,18 @@ static void rm_temp(const char* path) { unlink(sidecar); free(sidecar); } - n = strlen(path) + strlen(".dummykey.tmp.") + 32; - char* tmp = malloc(n); - if (tmp) { - snprintf(tmp, n, "%s.dummykey.tmp.%ld", path, (long)getpid()); - unlink(tmp); - free(tmp); + n = strlen(path) + strlen(".dummykey.tmp.*") + 1; + char* pattern = malloc(n); + if (pattern) { + snprintf(pattern, n, "%s.dummykey.tmp.*", path); + glob_t matches; + memset(&matches, 0, sizeof(matches)); + if (glob(pattern, 0, NULL, &matches) == 0) { + for (size_t i = 0; i < matches.gl_pathc; i++) + unlink(matches.gl_pathv[i]); + } + globfree(&matches); + free(pattern); } } @@ -815,6 +823,38 @@ static void test_credentials_dummy_key_persisted() { free(sidecar); } +/* A restrictive umask must not leave the freshly published sidecar with owner + * bits cleared: creation forces exact 0600 with fchmod (the reader requires an + * exact 0600), so the daemon cannot lock itself out on the next restart. */ +static void test_credentials_dummy_key_exact_mode_under_umask() { + char line[CREDENTIAL_MAX_LINE]; + EXPECT_TRUE(make_store_line("alice", KAT_PASSWORD, CREDENTIAL_MIN_ITERS, line, sizeof(line))); + char contents[CREDENTIAL_MAX_LINE + 2]; + snprintf(contents, sizeof(contents), "%s\n", line); + char* path = make_tmp_file(contents); + EXPECT_NOT_NULL(path); + char* sidecar = dummy_sidecar_path(path); + EXPECT_NOT_NULL(sidecar); + + /* Clear every permission bit the O_CREAT mode would otherwise provide; only + * the explicit fchmod can restore the exact 0600 the reader demands. */ + mode_t old_umask = umask(0777); + char err[512]; + CredentialStore* store = credentials_load(path, NULL, err, sizeof(err)); + umask(old_umask); + EXPECT_NOT_NULL(store); + + struct stat st; + EXPECT_EQ_INT(stat(sidecar, &st), 0); + EXPECT_EQ_INT((int)(st.st_mode & 07777), 0600); + EXPECT_EQ_INT((int)st.st_size, CREDENTIAL_KEY_LEN); + credentials_free(store); + + rm_temp(path); + free(path); + free(sidecar); +} + /* A sidecar that is group/other accessible, the wrong size, or not a regular * file must fail the load closed. */ static void test_credentials_dummy_key_rejects_bad_sidecar() { @@ -1037,6 +1077,7 @@ void test_credentials(void) { test_credentials_hash_file(); test_credentials_rejects_group_or_other_accessible(); test_credentials_dummy_key_persisted(); + test_credentials_dummy_key_exact_mode_under_umask(); test_credentials_dummy_key_rejects_bad_sidecar(); test_credentials_dummy_key_existing_sidecar_adopted(); test_credentials_dummy_key_symlink_rejected(); From 1b90ee244969f551788a7e0ac7b688f6b67c2fa6 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sat, 12 Sep 2026 19:50:52 +0200 Subject: [PATCH 6/6] fix(review): close loopback TLS auth bypass; align docs and wrong-CN test - server gate: the --allow-unauthenticated loopback allowance now requires an actual plaintext connection (!gate_ctx->ssl), so a loopback TLS client whose cert fails the --client-cn check is refused before any SCRAM challenge instead of falling through the plaintext opt-in. Keep the invalid-fd guard as belt-and-braces (unreachable after the policy check). - test: rewrote test_wrong_client_cn_refused_before_auth_challenge to run deterministically over 127.0.0.1 with --tls + --allow-unauthenticated and a CA-valid wrong-CN client cert, asserting the gate refusal log and an unchanged module tree (no skip). - docs: --client-cn is mandatory with --tls; dummykey sidecar is secret material; document all transient-fallback reasons; qualify --allow-unauthenticated in README and --help so it cannot read as permitting remote plaintext auth. - credentials.h: drop stale restrictive-umask claim (fchmod forces exact 0600; only create/write/fsync/link/fchmod failure degrades to ephemeral). --- README.md | 24 ++++++++++++++---------- RSYNC_COMPAT.md | 6 +++--- src/server/server.c | 26 +++++++++++++++++--------- src/shared/credentials.h | 9 +++++---- tests/integration/test_daemon.py | 24 ++++++++++-------------- 5 files changed, 49 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index c7e5887..8b83ba3 100644 --- a/README.md +++ b/README.md @@ -145,7 +145,7 @@ partial, alternate, and planned behavior. | `--cert ` | TLS certificate file (PEM) | | `--key ` | TLS private key file (PEM) | | `--ca ` | TLS CA certificate file for verification (PEM) | -| `--client-cn ` | Required TLS client certificate common name | +| `--client-cn ` | TLS client certificate common name; mandatory with `--tls` (a TLS connection always verifies the client CN) | ### Server @@ -159,7 +159,7 @@ partial, alternate, and planned behavior. | `--ca ` | TLS CA certificate file for verification (PEM) | | `--destination-root ` | Authorized destination root (default: `.`) | | `--allow-delete` | Permit manifest deletion | -| `--allow-unauthenticated` | Permit plaintext TCP clients | +| `--allow-unauthenticated` | Permit plaintext TCP clients. For an `auth users` module this opts in **loopback plaintext only**; remote auth still requires verified TLS, so the flag never permits remote plaintext auth. | | `-v, --verbose` | Enable debug logging | | `--help` | Show help | @@ -526,11 +526,14 @@ redirect that output to an owner-only (mode 0600) file, and note that legacy (mode 0600) `.dummykey` sidecar next to the store: it holds the store-wide dummy key, is auto-created on first load, and must be preserved across daemon restarts so the dummy challenge for an unknown user stays stable (the key is -never regenerated while the sidecar exists). If the sidecar cannot be created -(process-substitution/FIFO store path such as `/dev/fd/N`, a read-only -filesystem, or a missing directory), the daemon logs a warning and uses a -transient key, so the cross-restart guarantee does not hold for those -deployments. One residual is accepted: the store +never regenerated while the sidecar exists). The sidecar is secret material and +must be protected like the credential store: keep it owner-only (mode 0600) and +include it with the store in backups and credential rotation. If the sidecar +cannot be created (a process-substitution/FIFO store path such as `/dev/fd/N`, a +read-only filesystem, a missing directory, or a create, write, fsync, link, or +fchmod failure), the daemon logs a warning and uses a transient key, so the +cross-restart guarantee does not hold for those deployments. One residual is +accepted: the store iteration count is observable pre-auth by design, since the miss path must match a hit. @@ -551,9 +554,10 @@ Because the loopback allowance trusts whichever peer the kernel reports as `127.0.0.1`, it assumes nothing relays remote connections to the daemon. A local TCP forwarder or a TLS-terminating proxy in front of an auth-module listener makes remote clients appear as loopback and bypasses the mutual-TLS identity -check, so do not front an auth-module listener with such a relay. Note also that -`--client-cn` matches the certificate's CN only (not a subjectAltName), which is -acceptable for a private CA. +check, so do not front an auth-module listener with such a relay. `--tls` always +mandates `--client-cn`, so a TLS connection to an auth-required module always +has its client CN verified (`--client-cn` matches the certificate's CN only, not +a subjectAltName, which is acceptable for a private CA). TLS provides encrypted TCP transport. Supplying `--ca` enables certificate verification; without it, traffic is encrypted but peer identity is not diff --git a/RSYNC_COMPAT.md b/RSYNC_COMPAT.md index 191cb49..ef5ead2 100644 --- a/RSYNC_COMPAT.md +++ b/RSYNC_COMPAT.md @@ -639,9 +639,9 @@ now transmits targets (the prior behavior was broken/partial); its status moved - **Module selection & confinement:** the client requests a module with an rsync-style `host::module[/path]` destination. The module name crosses the wire as a trailing string on the config frame (bumping `PROTOCOL_VERSION` 2.14.0 → 2.15.0; the bump is required because the config-frame layout changed and the strict same-version handshake is what prevents a peer from desynchronizing on the new trailing field). The daemon looks the module up in ITS OWN config and uses the module's `path` as the authorized root through the exact same `configure_authorization` confinement the standalone server applies to `--destination-root` (`file_open_secure_parent`, `has_path_traversal`, `path_is_within`); the client never supplies the root, every client-chosen-ownership/super-user request is refused unless the module declares `client owner = yes` (the daemon's per-module opt-in, see below), and the operator `--no-super` veto forces super-user activities off for every daemon connection. The client's `/path` part is relative inside the module and is rejected if absolute or if it contains `..`. Unknown modules are refused before any data moves (the run fails cleanly at the config handshake). An absolute destination and a module request against a non-daemon server are also refused. - **`client owner` (client-chosen-ownership opt-in):** by default a daemon module refuses every request that would let the client pick an owner or ask for super-user activities — `--numeric-ids`, `--chown`, `--usermap`/`--groupmap`, `--fake-super`, `--copy-as`, and an explicit `--super` — at the config handshake (before `STATUS_OK`), because a daemon has no per-module opt-in for client-chosen ownership and any anonymous client could otherwise force arbitrary owner ids inside the module root. `client owner = yes` opts a single module in, allowing those requests within that module's root (the standalone listener and the SSH `--stdio` server always honor them for their single operator-authorized root). Without the opt-in the daemon also forces super-user **device** activity off for that connection — char/block device-node creation (`--devices`) and `--write-devices` — even under the default `AUTO` mode, so a non-opted module can never be made to `mknod` or write a raw device; those entries are skipped (not refused) so an ordinary `-a` push still succeeds without device nodes. The opt-in does **not** lift the privilege requirement: `--copy-as` still needs a root receiver, and the operator `--no-super` veto still forces super-user activities off for every connection. The daemon logs a prominent startup warning for each `client owner = yes` module so the operator's deliberate choice is visible. - **`read only` safe default:** every network transfer FastSync currently supports is a push that writes under the module root, so a `read only` module refuses the connection (clear server log "module is read only"; the client exits non-zero, nothing is transferred). A future pull/list operation can be opened up when it exists; the knob is already stored. -- **`auth users` (A7 SCRAM-SHA-256 authentication):** a module that declares `auth users` requires the client to present credentials. The config frame carries ONLY the username; the daemon answers an auth-required module with `STATUS_AUTH_CHALLENGE` (PBKDF2 iteration count, 16-byte salt, 32-byte server nonce), the client answers with `STATUS_AUTH_RESPONSE` (fresh 32-byte client nonce + a 32-byte ClientProof), and the daemon accepts only when the proof verifies **and** the username is **on the module's `auth users` list** and has a store entry, replying `STATUS_AUTH_OK` with a 32-byte ServerSignature the client verifies before proceeding. Verification is constant-time over fixed 32-byte keys (the compare runs even for a miss), username membership uses a constant-time full-length scan, and an unknown/off-list user still receives a challenge and runs the same math against a dummy verifier: a deterministic per-username salt (`HMAC-SHA256(store dummy key, username)`), the store-wide uniform iteration count and dummy keys. Re-probing the same unknown username therefore yields an identical salt and iteration count while a different username yields a different salt, so there is no user-enumeration or timing oracle. The daemon logs the username but **never the password, proof or keys**. A module WITHOUT `auth users` stays open (legitimate rsync configuration); credentials sent to such a module are ignored. Read-only is orthogonal: even a correctly authenticated push to a `read only` module is still refused (all FastSync network transfers write). Fail-closed policy: a daemon whose config declares `auth users` on any module refuses to start unless a credential store was given (`--password-file` and/or `--early-input`); a missing or empty store is never silently treated as "open". A failed handshake (missing credentials, unknown/off-list user, wrong proof or malformed data) yields a single generic `STATUS_AUTH_FAILED` and the daemon closes before any data moves. The dummy key is persisted in an owner-only `.dummykey` sidecar (auto-created on first load, mode 0600) so the dummy salt stays stable across daemon restarts, closing the restart-gated enumeration channel. The sidecar must be preserved across restarts for that guarantee; if it cannot be created (a process-substitution/FIFO store path such as `/dev/fd/N`, a read-only filesystem, or a missing directory), the daemon logs a warning and uses a transient per-run key, so unknown-user challenges change across restarts and the cross-restart guarantee does not hold for that deployment. One residual is accepted: the store iteration count is observable pre-auth by design, since the miss path must match a hit. **Transport policy (hardening A7-3/S1):** an auth-required module accepts credentials only when either (a) the connection is an encrypted, verified TLS connection whose client certificate matches `--client-cn`, or (b) the connection is plaintext from a loopback TCP peer **and** the operator explicitly passed `--allow-unauthenticated`. A remote plaintext peer, and a loopback plaintext peer without that flag, are refused at the config gate before any challenge is sent; `--allow-unauthenticated` never permits remote plaintext auth (remote peers still require verified TLS). Daemon modules are a `--daemon`-only feature — the SSH `--stdio` path never loads a daemon config and is not an auth transport for them. Because the loopback allowance trusts whichever peer the kernel reports as `127.0.0.1`, it assumes nothing relays remote connections to the daemon: a local TCP forwarder or TLS-terminating proxy in front of an auth-module listener makes remote clients appear as loopback and bypasses the mutual-TLS identity check, so do not front an auth-module listener with such a relay. -- **Credential store format:** server `--password-file`/`--early-input` files are line-based `user:$fastsync$1$pbkdf2-sha256$$$$`, one per line (standard base64; 16-byte salt, 32-byte keys; `iters` in `[100000, 10000000]`, default 600000). Every entry in the resulting store must agree on `iters` (a store whose entries disagree, or where a layered `--early-input` disagrees with `--password-file`, is rejected). Generate lines with `fastsync-server --hash-credentials FILE [--iterations N]`; the emitted lines are secret material, so redirect them to an owner-only (mode 0600) file (the tool warns on stderr if stdout is a group/other-accessible regular file). Blank lines and lines starting with `#`/`;` are comments; the parser is strict (a malformed line fails the whole load, so a typo can never let a different set of users in). **The legacy `user:SHA256HEX` form is hard-rejected** with an actionable "legacy" error; there is no auto-upgrade, so a replayable bearer digest can never be loaded by a 2.19.0 daemon. The client `--password-file` holds `user:password` on its first meaningful line (the literal password, used only for the handshake then burned); keep both files readable only by their owner (mode 0600). Per-username wire length is bounded (256 chars) and every decoded salt/key length is validated. Loading the store also maintains an owner-only `.dummykey` sidecar (auto-created, mode 0600, exactly 32 bytes) holding the store-wide dummy key that shapes unknown-user challenges; persist it across daemon restarts so those challenges stay stable, and treat a sidecar with the wrong owner, a mode other than exactly 0600, the wrong size or the wrong type as a fatal load error (fail closed). If the sidecar cannot be created (e.g. a process-substitution store path such as `/dev/fd/N`, a read-only filesystem, or a missing directory), the daemon logs a warning and uses a transient per-run key, so the cross-restart stability guarantee does not hold there. -- **Plaintext caveat:** an auth-required module is refused, **before any challenge is sent**, unless the connection is encrypted and verified TLS whose client certificate matches the server's `--client-cn`, or it is plaintext from a loopback TCP peer **and** the operator passed `--allow-unauthenticated`. A remote plaintext peer, and a loopback plaintext peer without that flag, never receive a challenge, and `--allow-unauthenticated` never permits remote plaintext auth (remote peers still require verified TLS). On the loopback plaintext transport that remains permitted, a local sniffer could still read the challenge and response and mount an **offline dictionary attack** against a weak password, so use `--tls` for any real deployment. `--client-cn` matches the certificate CN only (not a subjectAltName), which is acceptable for a private CA. Clients sending daemon credentials with `--password-file` to a non-loopback daemon must use `--tls`; the client rejects such a destination before any network I/O. Unlike the old challenge-less exchange there is **no replay**: the proof is bound to the fresh per-connection server nonce, so a captured `STATUS_AUTH_RESPONSE` cannot be reused on another connection (an integration test proxies the daemon and proves this). TLS client-CN (`--client-cn`) is an independent transport identity check and composes with password auth: both may be required on the same connection. +- **`auth users` (A7 SCRAM-SHA-256 authentication):** a module that declares `auth users` requires the client to present credentials. The config frame carries ONLY the username; the daemon answers an auth-required module with `STATUS_AUTH_CHALLENGE` (PBKDF2 iteration count, 16-byte salt, 32-byte server nonce), the client answers with `STATUS_AUTH_RESPONSE` (fresh 32-byte client nonce + a 32-byte ClientProof), and the daemon accepts only when the proof verifies **and** the username is **on the module's `auth users` list** and has a store entry, replying `STATUS_AUTH_OK` with a 32-byte ServerSignature the client verifies before proceeding. Verification is constant-time over fixed 32-byte keys (the compare runs even for a miss), username membership uses a constant-time full-length scan, and an unknown/off-list user still receives a challenge and runs the same math against a dummy verifier: a deterministic per-username salt (`HMAC-SHA256(store dummy key, username)`), the store-wide uniform iteration count and dummy keys. Re-probing the same unknown username therefore yields an identical salt and iteration count while a different username yields a different salt, so there is no user-enumeration or timing oracle. The daemon logs the username but **never the password, proof or keys**. A module WITHOUT `auth users` stays open (legitimate rsync configuration); credentials sent to such a module are ignored. Read-only is orthogonal: even a correctly authenticated push to a `read only` module is still refused (all FastSync network transfers write). Fail-closed policy: a daemon whose config declares `auth users` on any module refuses to start unless a credential store was given (`--password-file` and/or `--early-input`); a missing or empty store is never silently treated as "open". A failed handshake (missing credentials, unknown/off-list user, wrong proof or malformed data) yields a single generic `STATUS_AUTH_FAILED` and the daemon closes before any data moves. The dummy key is persisted in an owner-only `.dummykey` sidecar (auto-created on first load, mode 0600) so the dummy salt stays stable across daemon restarts, closing the restart-gated enumeration channel. The sidecar is secret material and must be protected like the credential store (owner-only 0600, included with the store in backups and rotation). It must be preserved across restarts for that guarantee; if it cannot be created (a process-substitution/FIFO store path such as `/dev/fd/N`, a read-only filesystem, a missing directory, or a create/write/fsync/link/fchmod failure), the daemon logs a warning and uses a transient per-run key, so unknown-user challenges change across restarts and the cross-restart guarantee does not hold for that deployment. One residual is accepted: the store iteration count is observable pre-auth by design, since the miss path must match a hit. **Transport policy (hardening A7-3/S1):** an auth-required module accepts credentials only when either (a) the connection is an encrypted, verified TLS connection whose client certificate matches `--client-cn`, or (b) the connection is plaintext from a loopback TCP peer **and** the operator explicitly passed `--allow-unauthenticated`. A remote plaintext peer, and a loopback plaintext peer without that flag, are refused at the config gate before any challenge is sent; `--allow-unauthenticated` never permits remote plaintext auth (remote peers still require verified TLS). Daemon modules are a `--daemon`-only feature — the SSH `--stdio` path never loads a daemon config and is not an auth transport for them. Because the loopback allowance trusts whichever peer the kernel reports as `127.0.0.1`, it assumes nothing relays remote connections to the daemon: a local TCP forwarder or TLS-terminating proxy in front of an auth-module listener makes remote clients appear as loopback and bypasses the mutual-TLS identity check, so do not front an auth-module listener with such a relay. +- **Credential store format:** server `--password-file`/`--early-input` files are line-based `user:$fastsync$1$pbkdf2-sha256$$$$`, one per line (standard base64; 16-byte salt, 32-byte keys; `iters` in `[100000, 10000000]`, default 600000). Every entry in the resulting store must agree on `iters` (a store whose entries disagree, or where a layered `--early-input` disagrees with `--password-file`, is rejected). Generate lines with `fastsync-server --hash-credentials FILE [--iterations N]`; the emitted lines are secret material, so redirect them to an owner-only (mode 0600) file (the tool warns on stderr if stdout is a group/other-accessible regular file). Blank lines and lines starting with `#`/`;` are comments; the parser is strict (a malformed line fails the whole load, so a typo can never let a different set of users in). **The legacy `user:SHA256HEX` form is hard-rejected** with an actionable "legacy" error; there is no auto-upgrade, so a replayable bearer digest can never be loaded by a 2.19.0 daemon. The client `--password-file` holds `user:password` on its first meaningful line (the literal password, used only for the handshake then burned); keep both files readable only by their owner (mode 0600). Per-username wire length is bounded (256 chars) and every decoded salt/key length is validated. Loading the store also maintains an owner-only `.dummykey` sidecar (auto-created, mode 0600, exactly 32 bytes) holding the store-wide dummy key that shapes unknown-user challenges; persist it across daemon restarts so those challenges stay stable, and treat a sidecar with the wrong owner, a mode other than exactly 0600, the wrong size or the wrong type as a fatal load error (fail closed). If the sidecar cannot be created (e.g. a process-substitution store path such as `/dev/fd/N`, a read-only filesystem, a missing directory, or a create/write/fsync/link/fchmod failure), the daemon logs a warning and uses a transient per-run key, so the cross-restart stability guarantee does not hold there. +- **Plaintext caveat:** an auth-required module is refused, **before any challenge is sent**, unless the connection is encrypted and verified TLS whose client certificate matches the server's `--client-cn`, or it is plaintext from a loopback TCP peer **and** the operator passed `--allow-unauthenticated`. A remote plaintext peer, and a loopback plaintext peer without that flag, never receive a challenge, and `--allow-unauthenticated` never permits remote plaintext auth (remote peers still require verified TLS). On the loopback plaintext transport that remains permitted, a local sniffer could still read the challenge and response and mount an **offline dictionary attack** against a weak password, so use `--tls` for any real deployment. `--client-cn` matches the certificate CN only (not a subjectAltName), which is acceptable for a private CA. Clients sending daemon credentials with `--password-file` to a non-loopback daemon must use `--tls`; the client rejects such a destination before any network I/O. Unlike the old challenge-less exchange there is **no replay**: the proof is bound to the fresh per-connection server nonce, so a captured `STATUS_AUTH_RESPONSE` cannot be reused on another connection (an integration test proxies the daemon and proves this). TLS client-CN (`--client-cn`) is an independent transport identity check and composes with password auth; because `--tls` already mandates `--client-cn`, a TLS auth connection always verifies the client CN, so both checks necessarily apply together on such a connection. - **Wire/protocol:** the config-frame auth block is now `[int present][str_redacted username]` (the old digest field is gone), and the frame stream gains the challenge/response (`STATUS_AUTH_CHALLENGE` → `STATUS_AUTH_RESPONSE` → `STATUS_AUTH_OK`/`STATUS_AUTH_FAILED`) between the config frame and the `STATUS_OK` ack. Both are wire-layout changes, so `PROTOCOL_VERSION` is bumped **2.18.0 → 2.19.0** (see the A7 note in `src/shared/config.h`); the strict same-version handshake keeps a 2.19 client and a 2.18 server from desynchronizing. - **Client side:** `host::module/path` selects the TCP transport and connects to `--server-port`; `host:path` stays the SSH transport; plain paths stay local TCP. The daemon username comes from `--password-file` (first `user:password` line), and `--password-file` without a `host::module/path` destination is a client error (fail fast). A `user@host::module` form is rejected with a pointer to `--password-file`. The client's plaintext password is wiped from memory (`config_burn_auth`) at transfer teardown. - **MOTD (Wave C):** a daemon configured with a global `motd file` sends that file's content as the first server→client string frame after the config-frame STATUS_OK ack (rsync sends the MOTD as the first thing from the server at the start of a daemon connection). Only the daemon listener path (`host::module`) gets a MOTD; the `--stdio` SSH path never sends or reads one. The server reads the file bounded to 4096 bytes and treats an absent/unreadable file as "no MOTD" (an empty frame, never an error). The exchange is server→client only and does **not** bump `PROTOCOL_VERSION`: every 2.15.0 daemon client reads the frame after the ack, so sender and receiver stay in lockstep (see the Wave C note in `src/shared/config.h`). `--no-motd` is the client-side suppression switch: the client still reads (consumes) the frame to keep the stream in sync but does not display it. The MOTD is printed to stdout with control bytes (ESC included) escaped octal-style while newlines/tabs are preserved, so a hostile server cannot inject terminal escape sequences. diff --git a/src/server/server.c b/src/server/server.c index 77817fc..dce8fee 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -384,16 +384,18 @@ static const char* server_module_gate(const Config* config, void* context) { } /* Transport policy (A7-3/S1): an auth-required module only accepts * credentials over (a) an encrypted, verified TLS connection whose client - * certificate matches --client-cn, or (b) a plaintext connection from a - * loopback peer that the operator explicitly opted into with - * --allow-unauthenticated. A remote plaintext peer and an un-flagged - * loopback plaintext peer are both refused HERE, before the challenge is - * sent, so an unverified client never receives a nonce. The operator flag - * never permits REMOTE plaintext auth: remote peers still require verified - * TLS regardless of the flag. */ + * certificate matches --client-cn, or (b) an actual PLAINTEXT connection + * from a loopback peer that the operator explicitly opted into with + * --allow-unauthenticated. A remote plaintext peer, an un-flagged loopback + * plaintext peer, and a loopback TLS peer whose certificate does not match + * --client-cn are all refused HERE, before the challenge is sent, so an + * unverified client never receives a nonce: the loopback allowance requires + * !gate_ctx->ssl, so --tls + --allow-unauthenticated can never be used to + * bypass the client-CN check. The operator flag never permits REMOTE + * plaintext auth: remote peers still require verified TLS regardless. */ bool tls_ok = gate_ctx && gate_ctx->ssl && SSL_get_verify_result(gate_ctx->ssl) == X509_V_OK && tls_client_identity_allowed(gate_ctx->ssl); - bool local_ok = allow_unauthenticated && gate_ctx && gate_ctx->fd >= 0 && + bool local_ok = allow_unauthenticated && gate_ctx && !gate_ctx->ssl && gate_ctx->fd >= 0 && utils_fd_peer_is_local(gate_ctx->fd); if (!tls_ok && !local_ok) { log_message(LOG_LEVEL_ERROR, @@ -403,6 +405,10 @@ static const char* server_module_gate(const Config* config, void* context) { return "daemon module requires authentication over an encrypted, verified TLS " "connection"; } + /* Belt-and-braces: the transport policy above already guarantees a context + * with a usable socket (verified TLS implies a live SSL object and loopback + * allowance requires gate_ctx->fd >= 0), so this is unreachable today; keep + * the guard so the handshake can never be driven over an invalid fd. */ if (!gate_ctx || gate_ctx->fd < 0) { log_message(LOG_LEVEL_ERROR, "daemon module '%s': no auth transport available", config->module); @@ -756,7 +762,7 @@ static void print_server_usage(void) { printf(" --cert TLS certificate file (PEM)\n"); printf(" --key TLS private key file (PEM)\n"); printf(" --ca TLS CA certificate file (PEM)\n"); - printf(" --client-cn Required TLS client certificate CN\n"); + printf(" --client-cn TLS client certificate CN (mandatory with --tls)\n"); printf(" --destination-root Authorized destination root (default: .)\n"); printf(" --address Bind the listening socket to this address\n"); printf(" -4, --ipv4 Bind an IPv4 socket (default)\n"); @@ -772,6 +778,8 @@ static void print_server_usage(void) { printf(" client's CONVERT_SPEC). A name that cannot be\n"); printf(" represented fails the run cleanly\n"); printf(" --allow-unauthenticated Allow plaintext/anonymous network clients\n"); + printf(" (an auth-required module still accepts only opted-in\n"); + printf(" loopback plaintext; remote auth requires verified TLS)\n"); printf(" --hash-credentials Read 's user:password lines and print\n"); printf(" PBKDF2 credential-store lines to stdout, then exit.\n"); printf(" Use the output as --password-file for --daemon;\n"); diff --git a/src/shared/credentials.h b/src/shared/credentials.h index 116ff13..377fb02 100644 --- a/src/shared/credentials.h +++ b/src/shared/credentials.h @@ -32,10 +32,11 @@ * the dummy challenge for an unknown user stable for the life of the store, so * a daemon restart cannot be used as a username-enumeration oracle. A sidecar * that is not an exact-mode-0600 regular file of exactly 32 bytes fails the load - * (fail closed); if it cannot be created (e.g. a read-only mount or a restrictive - * umask the fchmod cannot repair) the daemon warns and uses a transient per-run - * key instead. NOTE: the sidecar requires EXACT 0600, whereas the store / - * password files only reject group/other bits (a deliberate difference). + * (fail closed); creation forces exact 0600 with fchmod (so a restrictive umask + * cannot leave the sidecar unreadable), and only a create/write/fsync/link or + * fchmod failure degrades to a transient per-run key with a warning. NOTE: the + * sidecar requires EXACT 0600, whereas the store / password files only reject + * group/other bits (a deliberate difference). * * Client --password-file format: the FIRST meaningful (non-comment, non-blank) * line is `user:password`, holding the literal password. The client keeps it diff --git a/tests/integration/test_daemon.py b/tests/integration/test_daemon.py index 165d72e..30410e8 100644 --- a/tests/integration/test_daemon.py +++ b/tests/integration/test_daemon.py @@ -1154,20 +1154,16 @@ class TestDaemonTLSAuth: @pytest.mark.ci def test_wrong_client_cn_refused_before_auth_challenge(self): - """A7-3/S1: over a NON-local TLS connection an auth-required module is - refused at the config gate when the CA-valid client certificate does not - match --client-cn -- before any SCRAM challenge is sent and before any - file data moves. The daemon is started WITH --allow-unauthenticated to - prove that flag never relaxes the remote auth-module transport policy - (it only opts in plaintext from a loopback peer).""" - try: - remote_ip = socket.gethostbyname(socket.gethostname()) - except OSError: - pytest.skip("hostname does not resolve") - if remote_ip.startswith("127."): - pytest.skip("host resolves to loopback; no non-loopback interface") + """A7-3/S1: with --tls AND --allow-unauthenticated, a loopback TLS peer + whose CA-valid client certificate does not match --client-cn is still + refused at the config gate -- before any SCRAM challenge is sent and + before any file data moves. The --allow-unauthenticated flag only opts + in loopback PLAINTEXT; it must never turn a wrong-CN TLS peer into an + accepted auth transport. Runs over 127.0.0.1 so it is deterministic and + never skips; the gate log line (emitted before server_auth_handshake) + plus the unchanged module tree prove the refusal preceded any challenge.""" cert_dir = os.path.join(TEST_DATA_DIR, "daemon_tls_certs_wrong") - certs = _generate_tls_certs(cert_dir, extra_san_ips=[remote_ip]) + certs = _generate_tls_certs(cert_dir) client_creds = os.path.join(TEST_DATA_DIR, "daemon_tls_wrong_client.pw") _write_client_password_file(client_creds, "alice", ALICE_PASS) d = DaemonManager() @@ -1183,7 +1179,7 @@ class TestDaemonTLSAuth: tls_flags = ["--tls", "--cert", certs["wrong_client_cert"], "--key", certs["wrong_client_key"], "--ca", certs["ca"]] - result, _ = run_client(SOURCE_DIR, "%s::locked" % remote_ip, port=port, + result, _ = run_client(SOURCE_DIR, "127.0.0.1::locked", port=port, flags=tls_flags, extra_args=["--password-file", client_creds]) assert result.returncode != 0, "a wrong client CN must be refused" assert _tree_file_count(AUTH_MODULE) == before_files, \