onionwire/docs/SECURITY_AUDIT.md

OnionWire Security Audit

Commit: 2b42864eba (2b42864) Baseline: cargo test --locked pass (100 passed, 3 ignored), cargo clippy --locked --all-targets -- -D warnings pass Auditor: rust-dev (no access to running hidden services / live Monero wallet) Tree: worktree wt/t_d85060fb at /home/lancelot/Projects/onionwire/.worktrees/t_d85060fb Remote: origin/main = same SHA (https://forgejo.siriusdevops.com/sirius/onionwire.git)

Ignored tests (needs live Tor network): rotate_hs, tor_hs, two_node. Not re-run.

Severity key

Critical = remote key compromise or plaintext disclosure High = local key/plaintext disclosure, authn bypass, or payment forgery Medium = DoS, nonce/IV weakness, metadata leak Low = hygiene, error-path leakage, docs mismatch Info = observation

Findings

F1 — Incoming receipt verified=1 on unrelated wallet history [High]

Location: src/wallet.rs:142-148, used at src/node.rs:667-683

Evidence: confirmation is not “this txid paid this amount to this address”.

pub fn transfers_match(rows: &[TransferRow], txid: &str, amount: &str, address: &str) -> bool {
    rows.iter().any(|r| {
        r.txid == txid
            || (!address.is_empty()
                && r.address == address
                && (amount.is_empty() || r.amount == amount))
    })
}

ingest_receipt verifies the ed25519 on the rcp frame (so the friend signed it), inserts verified: false, then flips verified if transfers_match is true (src/node.rs:671-682). Replica of that predicate against two wallet rows {txid:aaa111, amount:1000, addr:4AAA…} and {txid:bbb222, amount:5, addr:8BBB…}:

txid-only match (wrong amount+addr): True
addr+amount match (wrong txid): True
honest miss: False

Impact: a friend who completed Noise IK can send a signed receipt for an arbitrary amount/address and get the TUI line [receipt] N XMR (verified) if either (a) txid appears anywhere in get_transfers in/pending, or (b) some inbound row already has that address and amount. That is payment forgery against the local “verified” bit. It is not a third-party wire injection: the frame still has to decrypt under the pinned session. docs/THREAT_MODEL.md:33 says “Never trust a rcp frame without RPC confirmation (verified stays 0)” — the code does promote verified, and the RPC check does not bind amount+address+txid together. Tests encode the store default (tests/pay.rs incoming_receipt_is_not_verified) but never exercise transfers_match against mismatched amount.

Fix: require txid == row.txid && amount == row.amount && address == row.address (and reject empty fields). Do not OR. Keep verified=0 if RPC is down.

F2 — Chat AEAD has empty AAD; ciphertext rows are interchangeable [Medium]

Location: src/backup.rs:98-108 (aead_encrypt), src/store.rs:649 / 670

Evidence: bodies are nonce || ChaCha20-Poly1305(key, nonce, pt) with no associated data. The same 32-byte msg_key wraps every row. A DB writer who cannot open the passphrase can still swap messages.plaintext blobs. Throwaway against this tree (/tmp/ow-audit-repro, CARGO_TARGET_DIR = this worktree target):

SWAP: alice sees "secret-for-bob"
SWAP: bob sees "secret-for-alice"

Both list_messages calls returned Ok; Poly1305 verified. dir / friend_id / id / created_at are plaintext columns and are not in the MAC.

Impact: anyone with write access to onionwire.db (same uid, stolen unlocked file, or a bug that writes sqlite) can reattribute ciphertext across friends and in/out without the passphrase. This is not remote plaintext disclosure. Identity secret keys are already plaintext in self (threat model says so); this is extra: the body encryption does not bind a row to its owner. Nonces are 96-bit random per generate_nonce — reuse across restarts/wipe-all (new key) / first-unlock rewrap is not the failure mode here.

Fix: encrypt as Aead::encrypt with AAD = friend_id || dir || row_id (or a committed header), or include those fields in the plaintext that is MACed. Reject decrypt if AAD does not match the row.

F3 — Invite sig is over concatenated strings, not length-prefixed fields [Medium]

Location: src/qr.rs:67-72 (sign_msg), src/qr.rs:36-64 (decode)

Evidence: sig covers k.as_bytes() || onion.as_bytes() || spk.as_bytes() with no delimiters or lengths. k is 64 hex chars after the 32-byte check, so it cannot shift. o and spk can. Encode a real v3 onion + 32-byte spk, then move the first 8 hex chars of spk onto o, keep k and sig. qr::decode accepts the mutant:

CONCAT: mutated invite accepted
CONCAT: onion_changed=true
CONCAT: onion=abcdefghijklmnopqrstuvwxyz234567abcdefghijklmnopqrstuvwxyz234567.onionabababab
CONCAT: spk_len=28 (want 32)
CONCAT: pubkey_unchanged=true

decode does not require signed_prekey.len()==32 or a v3 onion. Node::add_friend_payload then upsert_friend (same k replaces locator) and set_friend_prekey. Duplicate/unknown fields are rejected (src/qr.rs:85-106); all four fields are required. Empty sig fails from_hex. This is not a classic steal-the-identity concat: k stays the signer.

Impact: a mutated invite still verifies under the real identity key. F3-paste (or a same-k rescan) can poison friends.onion / prekey for that pubkey. The shifted onion is not an arbitrary attacker HS (you can only append a hex prefix of the original spk), so this is roster integrity / availability, not a silent MITM. Handshake then fails (prekey length ≠ 32 at node.rs:388). No panic on this path. from_hex has no size cap: a 4,000,000-char hex string decoded to 2,000,000 bytes in 0.34s in CPython; decode allocates that before pubkey.len()!=32 rejects.

Fix: sign a domain-separated encoding (k || 0x00 || o || 0x00 || spk, or length prefixes). Reject spk ≠ 32 bytes and onion ≠ v3. Cap invite length before from_hex (a few KiB).

F4 — Monero address check is prefix+length, not checksum [Medium]

Location: src/pay.rs:38-48; tests require the junk form to pass (tests/pay.rs:15-50)

Evidence:

pub fn check_address(addr: &str) -> Result<()> {
    let ok = match addr.as_bytes().first() {
        Some(b'4') if addr.len() == 95 || addr.len() == 106 => true,
        Some(b'8') if addr.len() == 95 => true,
        _ => false,
    };
    if ok && !addr.contains('\n') {
        Ok(())
    } else {
        Err(Error("invalid Monero address".into()))
    }
}

4 + 'A' * 94 is accepted (len 95). No network byte, no Keccak checksum, no alphabet check. profile::check_fields (src/profile.rs:118-128) does not call check_address at all — xmr_addr may be "4abc" (tests/profile.rs). /tip does call check_address on the stored profile address (src/node.rs:331). /pay invoices are signed over that address (src/node.rs:290).

Impact: OnionWire will persist and sign invoices/receipts for strings no Monero wallet should pay. A live monero-wallet-rpc will usually reject checksum failures on transfer, so this is not by itself silent theft. It is a local fail-open: garbage becomes a signed inv/rcp payload and a payments row. Combined with F1, a verified receipt can cite such an address.

Fix: decode base58, check network prefix + checksum. Empty profile xmr_addr stays allowed; non-empty must pass the same check.

F5 — Wallet RPC is unauthenticated HTTP and can mark receipts from its full transfer list [Medium]

Location: src/wallet.rs:151-171, 223-237, 117-138; README documents ONIONWIRE_WALLET_RPC=http://127.0.0.1:18083

Evidence: URL parser requires http:// (no TLS), host must be loopback or .onion (allowed_host, tested in tests/wallet.rs refuse_non_loopback_non_onion_host). There is no user:pass / Digest / header. http_post is raw TcpStream + read_to_end with a 5s timeout, no body size cap. .onion hosts are allowed but TcpStream::connect((onion, port)) does not go through Arti — so an onion RPC URL fail-closes at connect (not examined live). eprintln!("ONIONWIRE_WALLET_RPC: {e}") prints the parse error, not the URL, on bad env.

Impact: the documented operator setup is “HTTP to loopback, no login”. Any local process that can reach that port can transfer (spend) and get_transfers (the same list F1 trusts). OnionWire never holds spend keys (threat model — true); it also never authenticates to the process that does. This is local, not remote, if the operator actually bound loopback. Code cannot express --rpc-login.

Fix: require digest (or a unix socket). Refuse URLs without credentials. Cap RPC read size. If onion RPC is a goal, dial it through the Arti client, not TcpStream.

F6 — /wipe does not touch payments; sqlite is not secure_delete [Low]

Location: src/store.rs:677-685, 122-125

Evidence: wipe_messages overwrites messages.plaintext with zeroblob(length), DELETE FROM messages, VACUUM. No PRAGMA secure_delete. No DELETE FROM payments. WAL is required (journal_mode = WAL fail-closed). Threat model (docs/THREAT_MODEL.md:21) says “/wipe overwrites message bodies and vacuums” — that part matches. It does not say payments go away; they do not. Identity / friend pubkeys / onions stay plaintext (disclosed, not a finding). onionwire.db itself is never chmod 0600; the home and arti/ dirs are 0700 after create_dir_all (src/store.rs:753-758, tests/store.rs:61-62). Backup files are 0o600 at create (src/node.rs:145-150).

Impact: /wipe is not a forensic erase (SSD wear-leveling, WAL snapshots, payments table, roster). A seized disk after /wipe still has who you pay and who you talk to. /wipe-all is remove_dir_all — same disk caveat.

Fix: if /wipe should mean “chat history gone”, also drop payments (and checkpoint WAL). Document that /wipe is not crypto-shred. Optional secure_delete is still not a guarantee on flash.

F7 — HS publish logs the onion; dangerously_trust_everyone is Arti-only [Low]

Location: src/hs.rs:31-36, src/hs.rs:95, src/node.rs:95

Evidence: wait_until_published(..., &onion, &onion) uses the unredacted onion as label. eprintln!("{label} hs status: {state:?}") and probe lines go to stderr. onion_string uses display_unredacted (src/hs.rs:70) — required to persist the locator; the leak is the log. client_config calls builder.storage().permissions().dangerously_trust_everyone() after create_dir_all on the Arti state/cache paths. That API is fs-mistrust for Arti’s directories, not sqlite. Store::open_at_with_passphrase mkdir_700s home and home/arti first; home/cache is created by Arti’s create_dir_all without 0700. cbtmintimeout / cbtinitialtimeout = 20s is a circuit-build floor (perf / publish reliability), not an auth bypass.

Impact: journald/script logs contain the current v3 locator. Arti state/cache may be created 0755 until something else tightens them; sqlite lives under the 0700 home. A world-readable Arti cache is descriptor/consensus metadata, not chat bodies.

Fix: log a redacted onion (safelog). mkdir_700 the cache dir before client_config. Keep dangerously_trust_everyone scoped to Arti storage; do not reuse it for onionwire.db.

F8 — Threat model overstates receipt verification and omits F1–F3 [Low]

Location: docs/THREAT_MODEL.md:17-21, :33, :39-41

Evidence: TM correctly describes live-only send, loc rules, identity-vs-locator, passphrase-wrapped message key, plaintext identity/roster, experimental Arti, global 30/60s burst-10 token bucket, backup = identity. It claims RPC confirmation keeps verified at 0 unless the chain view agrees — F1 shows the matcher is not that. It does not mention empty AEAD AAD, invite concat, shape-only XMR addresses, or env passphrase (ONIONWIRE_STORE_PASSPHRASE in src/store.rs:745-750, visible in /proc/<pid>/environ).

Impact: an operator who treats TM as the capability list will believe “verified receipt ⇒ wallet saw that payment”.

Fix: either implement F1’s conjunctive match or change the sentence to “incoming rcp is displayed; verified is best-effort and must not be trusted in v0.2”.

Verified correct

• Noise pattern is actually Noise_IK_25519_ChaChaPoly_BLAKE2s with prologue onionwire-v1 (src/session.rs:8-9, 111-122). Initiator sets remote_public_key to the QR/roster x25519 prekey (src/session.rs:189). Responder takes remote static from snow (get_remote_static) and looks up the friend (src/session.rs:255-260, src/node.rs:550-555).
• Mutual identity proofs are ed25519_sign(handshake_hash) with the 32-byte pubkey prefix; verify_proof calls VerifyingKey::verify (src/session.rs:289-310). Initiator compares proof prefix to the pinned identity (src/session.rs:219-222); responder to the roster id (src/session.rs:273-276). Mismatch is Error::mismatch → send hard-fails (src/node.rs:490).
• Transport nonces are snow TransportState counters (Noise spec: increment, reject reuse). Application code does not set ChaCha nonces on the wire. Session keys include ephemeral DH → compromise of long-term static does not decrypt past transport; it does allow impersonation forward. That is IK, not a bug.
• Loc frames: apply_loc verifies ed25519 against the session peer_identity, requires the pubkey already in friends, and requires ts > onion_updated_at (src/store.rs:452-480, src/loc.rs:38-55, src/node.rs:561-568). A peer cannot silently move you to an onion they control unless they hold that identity key (Noise proof + loc sig). An old loc with smaller ts cannot rewind after a later rotate. (F3 invite paste can still overwrite locator; that is out-of-band.)
• Unknown typed prefixes (xyz ) are Kind::Drop, not fatal (src/dispatch.rs:30-38). Unparseable loc/inv/rcp are ignored (src/node.rs:562, 610-611, 643-644). Handshake/frame errors print incoming: {e} and the rend task ends; the accept loop continues (src/node.rs:723-729).
• Frames are length-prefixed, MAX_FRAME = 65535, checked before allocating the body (src/frame.rs:5, 63-67).
• Token bucket is one global TokenBucket::default() = 30 tokens / 60s, burst 10 (src/ratelimit.rs:41-44, src/node.rs:86, 712-720). Matches TM; one flood can starve every friend (availability, not auth).
• Backup: Argon2id v0x13, m=19456 KiB, t=2, p=1 (src/backup.rs:52-53) = OWASP 2023 minimum. Per-export 16-byte salt + 12-byte random nonce in the file. open AEAD-fails with a single error before replace_identity_keys (src/backup.rs:91-95, src/node.rs:155-168). Onion is not in the blob (tests/backup.rs). Wrong passphrase does not write keys.
• First-run message key: 32 random bytes, wrapped with the same KDF/AEAD, stored in store_meta (src/store.rs:281-293). Empty passphrase refused. Integrity check fail-closed (src/store.rs:191-194).
• Amounts on the pay path are decimal integer piconero (src/pay.rs:51-59); xmr_to_atomic pads a ≤12-digit fraction without float (src/pay.rs:62-85). /tip then parses to u64 for RPC (src/node.rs:332-334).
• Incoming inv/rcp are verified against session peer identity, not a field inside the frame (src/node.rs:613, 646).
• Locked product decisions (no server/XMPP/MAM/DHT, live-only send, identity=pubkey, onion=locator, no dual-host grace) match the code. Not findings.

Not examined / out of scope

• Live hidden-service reachability, IPT/HsDir, and two-node Tor tests (ignored; no HS from this auditor).
• Live monero-wallet-rpc (auth defaults, get_transfers JSON shape vs json_amount, unlock/spend confirm).
• snow 0.10 internals beyond the Builder/TransportState API used here (constant-time, rekey at 2^64).
• Arti keystore encryption at rest, fs-mistrust semantics of dangerously_trust_everyone beyond “it is called on Arti storage”.
• Timing of Argon2 / ed25519 verify (failed backup passphrase is one error string; KDF still runs).
• TUI rendering of hostile chat (ratatui text; no HTML).
• Traffic analysis / HS existence (TM already declines that).
• cargo audit: not installed (which cargo-audit empty). Lockfile inspected by hand; no RustSec lookup was executed against this Cargo.lock.

Dependencies (Cargo.lock)

Crate	Lock version	Cargo.toml
snow	0.10.0	0.10
chacha20poly1305	0.10.1	0.10
argon2	0.5.3	0.5
ed25519-dalek	2.2.0	2
x25519-dalek	2.0.1	2
rusqlite	0.36.0	0.36 (bundled)
arti-client	0.46.0	0.46 + onion-service-client + onion-service-service
tor-hsservice	0.46.0	0.46

Caret reqs are not =; cargo update can move 0.10.x / 0.46.x without a Cargo.toml edit. --locked CI is the real pin. Arti onion services are still experimental upstream (TM + onionwire skill); this tree fail-closes, no C-tor fallback (src/hs.rs:55).

Open questions for Lance

• Is a “verified” receipt allowed to mean anything in v0.2, or should the UI only ever show “unverified” until F1 is conjunctive and covered by a test?
• Invite encoding: length-prefix / 0x00 separators now, or wait for onionwire:v2 (v1 strings stay in the wild)?
• Store passphrase: keep ONIONWIRE_STORE_PASSPHRASE (proc-visible) or prompt / kernel keyring?
• Wallet RPC: document “loopback + --rpc-login you type into a wrapper”, or teach OnionWire digest?

Stop / go (auditor, not a ship decision)

Go for friends-only chat under the written seizure model (identity keys plaintext; bodies encrypted; Tor relays are not a server). Do not treat verified receipts as money moved. Do not treat F3-paste of a string you did not copy yourself as an integrity-checked locator. No Critical remote key/plaintext bug found in this revision.