Add security review report for security-scan branch
Documents the focused security review of the protocol-versioning migration and the cargo-deny policy addition. Result: no high-confidence vulnerabilities — the versioned_topic XOR transform is entropy-preserving, signature binding uses the raw topic_id consistently, and the ALPN/domain changes are handshake-level compatibility only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,51 @@
|
||||
# Security Review: `security-scan` branch (PeerSpeak)
|
||||
|
||||
_Date: 2026-06-18_
|
||||
|
||||
**Scope:** Protocol-versioning migration (`src/protocol.rs`, `versioned_topic`,
|
||||
ALPN/domain centralization, gossip topic namespacing) and the `deny.toml`
|
||||
supply-chain policy addition.
|
||||
|
||||
## Result: No high-confidence security vulnerabilities found.
|
||||
|
||||
Each plausible attack surface introduced by this branch was investigated and
|
||||
confirmed safe:
|
||||
|
||||
### 1. `versioned_topic` XOR transform — topic secrecy preserved
|
||||
`src/protocol.rs:46`, used at `src/network/gossip.rs:255`
|
||||
|
||||
The room `topic_id` is a uniformly random 32-byte secret (`rand::random()`,
|
||||
`src/core/mod.rs:1012`) acting as the room capability. XOR-ing it with the public
|
||||
constant `GOSSIP_PROTO.to_le_bytes()` cyclically is **bijective and
|
||||
entropy-preserving** — the result is still uniformly random; no byte becomes
|
||||
predictable and no entropy is lost. The room secret is no more recoverable by an
|
||||
observer than before the change (previously the raw `topic_id` was the on-wire
|
||||
topic; now it's a trivial public XOR of it). Bijectivity also preserves room
|
||||
distinctness, so isolation is not weakened. **Not a vulnerability.**
|
||||
|
||||
### 2. Signature topic-binding — no raw/versioned confusion
|
||||
`src/network/gossip.rs`
|
||||
|
||||
`active_topic_bytes` stores the **raw** `ticket.topic_id` (line 293), and both
|
||||
`sign_gossip` and `verify_gossip` bind against that raw value. Only the
|
||||
*subscribed* swarm topic (line 255) uses the versioned value. There is one swarm
|
||||
per join and every peer signs/verifies against the same raw topic, so no second
|
||||
topic exists to enable a raw↔versioned replay/confusion attack. Code matches
|
||||
VERSIONING.md's claim. **Not a vulnerability.**
|
||||
|
||||
### 3. `GOSSIP_SIG_DOMAIN` — moved verbatim
|
||||
Value identical (`"peerspeak-gossip-v1"`, `src/protocol.rs:34`); cross-version
|
||||
cryptographic domain separation preserved. **Not a vulnerability.**
|
||||
|
||||
### 4. ALPN changes — handshake compatibility only
|
||||
Audio `peerspeak-audio` → `peerspeak/audio/1`, friends `/0` → `/1`. No security
|
||||
check keys off the old ALPN strings (audio admission is gated by live room
|
||||
membership per S8, not the ALPN literal); no residual references to old strings
|
||||
in non-test code. **Not a vulnerability.**
|
||||
|
||||
### 5. `deny.toml`
|
||||
Ignores only two *unmaintained* advisories (`RUSTSEC-2024-0436`,
|
||||
`RUSTSEC-2026-0150`) on compile-time/FFI-only crates — documented, and dependency
|
||||
advisories are out of scope. **Not a vulnerability.**
|
||||
|
||||
The versioning migration is a clean, security-preserving change.
|
||||
Reference in New Issue
Block a user