Local hardening: bounded Tor cookie read, Core's bitcoin.conf comments, no request dumps in trace logs

[bkeroack]

Problem

Tor cookie read. With -listenonion/-torcontrol, satd reads the SAFECOOKIE cookie from the path the control port names in its PROTOCOLINFO reply, with std::fs::read: to end of file, whatever the file is (node/src/net/tor.rs:152). A path such as /dev/zero is read until memory runs out, and a FIFO blocks startup in open(2) (CONTROL_TIMEOUT covers only the socket). Bitcoin Core reads the cookie with ReadBinaryFile(cookiefile, TOR_COOKIE_SIZE) (src/torcontrol.cpp:616, src/util/readwritefile.cpp:16-35), which never reads more than 32 bytes.

Credentials in trace logs. -loglevel=trace (or RUST_LOG=trace) enables jsonrpsee-server's trace!("{:?}", request) (jsonrpsee-server 0.26 src/server.rs:1026), which runs before satd's auth layer and prints every HTTP request with its headers, authorization: Basic <base64 user:password> included. jsonrpsee-core also logs call parameters and raw request bodies at TRACE (src/server/rpc_module.rs:310, 356, 409), and tungstenite logs every WebSocket frame's payload (src/protocol/frame/mod.rs:228, 257). Bitcoin Core logs neither credentials nor RPC parameters at any level.

Inline # comments in bitcoin.conf. satd skipped only lines starting with # (satd/src/config.rs:8110), so a comment after a value became part of the value, with no warning:

• txindex=1 # c → "1 # c", which is not a boolean, so txindex stayed off;
• rpcport=8332 # c failed to parse and the default port was used;
• rpcpassword=pw # c set the password to pw # c;
• [test] # c was refused as an unknown key.

Core's GetConfigOptions (src/common/config.cpp:41-62) drops everything from the first #, trims " \t\r\n" from the line, key and value, and refuses an rpcpassword line that holds a # at all: parse error on line N, using # in rpcpassword can be ambiguous and should be avoided. sat-cli's own reader (sat-cli/src/conf.rs) copied satd's rule, so it had the same bug. bitcoin-cli reads the file with Core's parser (src/bitcoin-cli.cpp:182).

Debug output of secret-bearing types. Config (rpcpassword, rpcauth, torpassword, esplorauserpass, reorgwebhooksecret), RpcAuthEntry, the reorg WebhookTarget, and satd-auth's CookieCredential / UserPassCredential / RpcAuthCredential (inside OperatorCreds, inside RpcAuth) all derived Debug. Nothing prints them today, but a future {:?} or a panic message would write the secrets to the log.

Fix

• Tor: read_cookie_file opens the cookie with O_NONBLOCK, refuses anything that is not a regular file, and reads at most 33 bytes. A file that is not exactly 32 bytes is still refused, as before (Core would accept a longer file and use its first 32 bytes; Tor always writes 32, and satd keeps its existing stricter check).
• Trace logs: config::request_dump_guard() is a global filter layer installed beside the reloadable EnvFilter in main. It drops TRACE spans and events whose target starts with jsonrpsee or tungstenite, whatever -loglevel, -debug, the logging RPC or RUST_LOG say. It is a separate layer because EnvFilter lets the most specific directive win: a RUST_LOG naming a longer target (jsonrpsee_core::server=trace) would override a jsonrpsee_core=debug directive. Their DEBUG output is unchanged. hyper is built without tracing, h2 leaves header fields out of its frame Debug on purpose, and tower-http's TraceLayer (Esplora) records no headers by default, so they need nothing.
• bitcoin.conf: ConfigFile::parse and sat-cli's ConfFile::parse follow GetConfigOptions: strip from the first #, trim only " \t\r\n", and refuse a # on any line whose key contains rpcpassword, with Core's message. A section header with a trailing comment now opens the section. includeconf files go through the same parser. sat-cli prints the parse error and exits, as bitcoin-cli does.
• Debug: hand-written Debug impls. Config shows the network and datadir only (finish_non_exhaustive); RpcAuthEntry and the satd-auth credentials show the user (or the cookie path) only; WebhookTarget shows the URL and whether a secret is set.
• Manual: the config reference documents comment handling and the TRACE cap; the sat-cli section says the file is read with the same rules.

Tests

Test	Fails without the fix
net::tor::tests::safecookie_refuses_a_fifo_cookie_path	yes: on the old fs::read call site the client blocked in open(2) and the test failed with "authenticate blocked on a FIFO named as the Tor cookie file". With the regular-file check removed: "cookie file … is 0 bytes" instead of "not a regular file".
net::tor::tests::safecookie_refuses_a_device_cookie_path	yes, with the regular-file check removed: "cookie file /dev/zero is more than 32 bytes". (Not run against the unbounded old read, which would exhaust memory.)
net::tor::tests::cookie_read_stops_one_byte_past_the_cookie_length	yes, with the 33-byte bound removed (assertion left == right failed, 1 MiB read).
net::tor::tests::safecookie_refuses_a_cookie_file_longer_than_32_bytes	keeps the existing refusal of a long file.
config::localharden_tests::an_inline_comment_is_not_part_of_the_value	yes: unrecognized key '[test] # the testnet3 section'.
config::localharden_tests::inline_comments_reach_the_running_config_as_core_reads_them	yes: unrecognized key '[regtest] # this chain'.
config::localharden_tests::a_hash_on_an_rpcpassword_line_is_refused_with_cores_message	yes: rpcpassword=pw # old password parsed as ["pw # old password"]. Also fails with only the rpcpassword check disabled.
config::localharden_tests::whitespace_is_trimmed_as_core_trims_it	yes: a trailing vertical tab was trimmed (left: ["x"]).
config::localharden_tests::config_debug_output_carries_no_secrets	yes: "Config Debug shows RpcPasswordMarker" (also with the derive put back).
config::localharden_tests::trace_logging_leaves_out_the_request_dumps	with is_request_dump disabled: "jsonrpsee-server TRACE is logged under -loglevel=trace" (new API, so not run on the old code; the end-to-end test below fails on the old binary). Also checks that their DEBUG output and satd's own TRACE output still get through, and that RUST_LOG-style directives naming the targets cannot re-enable them.
local_hardening::trace_logging_never_writes_rpc_credentials_or_parameters (new test target)	yes: on the old binary the log held TRACE jsonrpsee-server: Request { … headers: {"authorization": "Basic dHJhY2Vsb2d1c2Vy…". Runs a regtest node with -loglevel=trace and user/password auth, calls getblockcount and echo <marker>, then checks the log for the Basic credential, the password and the parameter, and that jsonrpsee's DEBUG lines are still there.
operator::tests::debug_output_names_credentials_without_their_secrets (satd-auth)	yes: "Debug shows CookieTokenMarker".
sat-cli conf::tests::an_inline_comment_is_not_part_of_the_value, a_hash_on_an_rpcpassword_line_is_refused	yes (the port 19000\t# port did not parse; rpcpassword=a#b was accepted). comments_and_bare_keys was edited in place: it asserted the old a#b password.

Local gate: cargo clippy --all-targets --all-features -D warnings, cargo clippy -p node --no-default-features --all-targets -D warnings, node tor tests, satd unit tests, satd-auth, sat-cli, the new local_hardening target, and the regtest sat_cli/logging/sighup/reload/getconfig/log_format/conf tests.

Notes

• umask 077 (not done). Core sets umask(077) at startup (src/common/system.cpp:92-93; current Core has no -sysperms opt-out); satd sets none, so a new datadir is 0755 and mempool.dat, peers.dat and the ban list are 0644 (the cookie and the SV2 key are already 0600). This is left out because shipped packaging relies on group access into the datadir: contrib/systemd/satd.service:11-15, 71-88 (and satd@.service:35-37, 76-80) chmod the cookie to 0640 so satd group members can run sat-cli, and on every chain but mainnet that cookie sits in <datadir>/<network>/, which satd creates. Under umask 077 that directory would be 0700 and group members could no longer reach the cookie. The appliance depends on the same path (contrib/appliance/provision/10-satd.sh:89-92). It needs a decision on how the units grant that access first.
• [ main ] with spaces inside the brackets is still read as [main]; Core would name the section main and ignore it. A bare key without = is still read as key=1, where Core refuses the line. Both are existing satd leniencies and unchanged here, except that a bare rpcpassword line holding a # is refused.
• CliArgs still derives Debug over --rpcpassword and friends; nothing prints it, and a hand-written Debug for the clap struct is not small.

Release notes

satd now reads bitcoin.conf comments as Bitcoin Core does: a # anywhere on a line starts a comment, so a line such as txindex=1 # keep it is no longer silently misread, and an rpcpassword line containing # is refused at startup with Core's error. sat-cli reads the file the same way. -loglevel=trace no longer writes RPC request headers (including the Authorization credentials) or RPC parameters to the log. With Tor control enabled, satd reads at most 33 bytes from the cookie file the control port names and refuses anything that is not a regular file, so a misbehaving control port cannot make startup hang or exhaust memory.

🤖 Generated with Claude Code