From 1de1376e54a8a775eb8c44af02d3cc0b2cd088a7 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sat, 12 Sep 2026 18:08:46 +0200 Subject: [PATCH] fix(a7-auth): final hardening pass on SCRAM auth - burn the store-wide dummy_key in credentials_free() - burn the local mac on hmac_sha256 failure in credentials_get_verifier() - always run the O(store) constant-time scan, even for off-list users, to close the pre-existing off-list timing channel; select the real verifier only when on_list && match - clarify the server_auth_handshake STATUS_AUTH_FAILED comment (failure before success vs. a dropped broken connection while writing the signature) - document accepted anti-enumeration residuals (restart-gated dummy salt; pre-auth-observable iteration count) --- README.md | 6 +++++- RSYNC_COMPAT.md | 2 +- src/server/server.c | 13 ++++++++----- src/shared/credentials.c | 25 +++++++++++++++---------- 4 files changed, 29 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 17bf43f..c6fec51 100644 --- a/README.md +++ b/README.md @@ -522,7 +522,11 @@ 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. +`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. 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 1db9d7d..6d74e25 100644 --- a/RSYNC_COMPAT.md +++ b/RSYNC_COMPAT.md @@ -639,7 +639,7 @@ 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. +- **`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. - **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. diff --git a/src/server/server.c b/src/server/server.c index 3b1aa17..c23dd5b 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -73,8 +73,10 @@ typedef struct ModuleGateContext { * 2.19.0). Sends STATUS_AUTH_CHALLENGE (iteration count, base64 salt, base64 * server nonce), expects STATUS_AUTH_RESPONSE (base64 client nonce, base64 * ClientProof), verifies the proof constant-time and answers STATUS_AUTH_OK - * with the base64 ServerSignature. On any failure it sends exactly one generic - * STATUS_AUTH_FAILED and returns false. The verifier for an unknown/off-list + * with the base64 ServerSignature. On any failure BEFORE the success response + * it sends exactly one generic STATUS_AUTH_FAILED and returns false; a failure + * while writing the success signature cannot send a status and just drops an + * already-broken connection. The verifier for an unknown/off-list * user is a dummy (deterministic per-username salt, store-wide iterations, dummy * keys, found=false) so the same math runs and no user-enumeration/timing oracle * is exposed. */ @@ -368,9 +370,10 @@ static const char* server_module_gate(const Config* config, void* context) { /* Auth-required module (A7, protocol 2.19.0): run the SCRAM challenge/ * response BEFORE the module root is installed and before any data moves. * Fail closed: no store -> refuse (server misconfiguration, STATUS_ERROR); - * a failed handshake writes exactly one STATUS_AUTH_FAILED (on every - * failure path) before signalling ALREADY_TERMINATED. The username may be - * logged (never the password or any derived proof). */ + * a handshake that fails before the success response writes exactly one + * STATUS_AUTH_FAILED before signalling ALREADY_TERMINATED (a failure while + * writing the success signature instead just drops the broken connection). + * The username may be logged (never the password or any derived proof). */ if (g_credentials == NULL) { log_message(LOG_LEVEL_ERROR, "daemon module '%s' requires authentication but no credential store is " diff --git a/src/shared/credentials.c b/src/shared/credentials.c index e305032..5971a26 100644 --- a/src/shared/credentials.c +++ b/src/shared/credentials.c @@ -658,6 +658,9 @@ void credentials_free(CredentialStore* store) { credentials_burn((char*)store->entries[i].server_key, CREDENTIAL_KEY_LEN); free(store->entries[i].user); } + /* The store-wide dummy key is secret (it shapes the miss challenge), so wipe + * it before releasing the store. */ + credentials_burn((char*)store->dummy_key, sizeof(store->dummy_key)); free(store->entries); free(store); } @@ -716,28 +719,30 @@ bool credentials_get_verifier(const CredentialStore* store, const char* user, * reached in production) falls back to the all-zero static key. */ const uint8_t* dummy_key = store ? store->dummy_key : k_dummy_stored_key; uint8_t mac[CREDENTIAL_KEY_LEN]; - if (!hmac_sha256(dummy_key, CREDENTIAL_KEY_LEN, (const uint8_t*)uname, strlen(uname), mac)) + if (!hmac_sha256(dummy_key, CREDENTIAL_KEY_LEN, (const uint8_t*)uname, strlen(uname), mac)) { + credentials_burn((char*)mac, sizeof(mac)); return false; + } memcpy(out->salt, mac, CREDENTIAL_SALT_LEN); credentials_burn((char*)mac, sizeof(mac)); - if (!store || !user || n < 0) - return true; /* Module-list membership: constant-time full scan, no early break, so the * list is not a username-enumeration oracle. */ bool on_list = false; for (int i = 0; i < n; i++) { - if (module_users && module_users[i] && username_secure_equal(module_users[i], user)) - on_list = true; + const char* listed = (module_users && user) ? module_users[i] : NULL; + on_list |= listed ? username_secure_equal(listed, user) : false; } - if (!on_list) - return true; - /* Store lookup is also a constant-time full scan. */ + /* Store lookup is an unconditional constant-time full scan, executed even for + * an off-list user so a probe that is not on the module list still pays the + * same O(store) cost as one that is; skipping it would reopen an off-list + * timing channel. The real verifier is selected only when the user is both + * on the list and matched in the store. */ const CredentialEntry* match = NULL; - for (int i = 0; i < store->count; i++) { + for (int i = 0; store && user && i < store->count; i++) { if (username_secure_equal(store->entries[i].user, user)) match = &store->entries[i]; } - if (match) { + if (on_list && match) { memcpy(out->salt, match->salt, CREDENTIAL_SALT_LEN); out->iters = match->iters; memcpy(out->stored_key, match->stored_key, CREDENTIAL_KEY_LEN);