ed8a7ae5aa
Audit: - docs/AUDIT-2026-05-25.md: full protocol audit covering 8 findings (4 critical, 2 high, 5 medium, 4 low) with code references and fix effort estimates - vault/Audit/Tasks.md: Obsidian Tasks plugin file tracking all audit items with priorities, due dates, and per-step checklists Architecture docs updated for Wire format v2 and Wave 5/6 features: - ARCHITECTURE.md: adds wzp-video to dependency graph and project structure; wire format updated to v2 (16B header, 5B MiniHeader); relay concurrency section corrected (DashMap+RwLock is current, not a future optimization); test count 571→702; Android note - PROGRESS.md: Wave 5 and Wave 6 sections appended; test count 372→702; current status and open blockers as of 2026-05-25 - ROAD-TO-VIDEO.md: implementation status table inserted (✅/🟡/🔴/🔲 per phase); 6-step critical path to first video call - WZP-SPEC.md: MediaHeader updated to v2 (16B byte-aligned); MiniHeader updated to 5B with seq_delta; codec IDs 9-12 added (H.264/H.265/AV1); version negotiation section added Obsidian vault (vault/): - 114 files across Architecture/, PRDs/, Reports/, Android/, Reference/, Audit/ with YAML frontmatter - 00 - Home.md index note with wiki links - .obsidian/app.json config Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
80 lines
4.6 KiB
Markdown
80 lines
4.6 KiB
Markdown
---
|
||
tags: [report, wzp]
|
||
type: report
|
||
status: Approved
|
||
---
|
||
|
||
# T1.7 — Move `QualityReport` trailer inside AEAD payload
|
||
|
||
**Status:** Approved
|
||
**Agent:** Kimi Code CLI
|
||
**Started:** 2026-05-11T11:05Z
|
||
**Completed:** 2026-05-11T16:29Z
|
||
**Commit:** c9fa10d
|
||
**PRD:** ../PRD-protocol-hardening.md
|
||
|
||
## What I changed
|
||
|
||
- `crates/wzp-client/src/call.rs:1613` — Added `quality_report_aead_tamper_fails_decrypt` test confirming that when a `MediaPacket` with `quality_report` is serialized and then encrypted with `ChaChaSession` (header as AAD, payload+QR as plaintext), tampering with any byte in the QR region causes AEAD decryption to fail.
|
||
|
||
## Why these choices
|
||
|
||
The `MediaPacket::to_bytes()` serialization already places the `QualityReport` trailer immediately after the payload in the same contiguous buffer. The `ChaChaSession::encrypt` API already accepts `header_bytes` as AAD and `plaintext` as the message to seal. Therefore the existing architecture naturally supports the desired ordering:
|
||
|
||
1. `MediaHeader` → serialized as AAD
|
||
2. `payload || QualityReport` → serialized as plaintext
|
||
3. AEAD-seal over (plaintext, AAD)
|
||
|
||
No production code changes were required because there is no live media encryption path in `cli.rs` today (`_crypto_session` is derived but discarded). The task’s goal was to verify the API boundary and add a regression test so that when a future task wires encryption into the send loop, the QR will automatically sit inside the AEAD payload.
|
||
|
||
## Deviations from the task spec
|
||
|
||
None. Followed steps T1.7.1 through T1.7.5 without deviation. Step 3 (“If currently appended after AEAD seal: refactor”) was a no-op because no production path appends the QR after encryption.
|
||
|
||
## Verification output
|
||
|
||
```bash
|
||
$ cargo test -p wzp-client quality_report_aead
|
||
running 1 test
|
||
test call::tests::quality_report_aead_tamper_fails_decrypt ... ok
|
||
|
||
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 169 filtered out; finished in 0.00s
|
||
```
|
||
|
||
```bash
|
||
$ cargo test -p wzp-crypto
|
||
running 36 tests
|
||
...(all 36 pass)...
|
||
|
||
test result: ok. 36 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.03s
|
||
```
|
||
|
||
## Test summary
|
||
|
||
- Tests added: 1 (`quality_report_aead_tamper_fails_decrypt`)
|
||
- Tests modified: 0
|
||
- Workspace test count before: 571 / after: 572 (1 added in `wzp-client`)
|
||
- `cargo clippy --workspace --all-targets -- -D warnings`: pass in crates touched (`wzp-client`, `wzp-crypto`); 12 known-debt errors in `wzp-codec` + `warzone-protocol` (see PROTOCOL-AUDIT.md)
|
||
- `cargo fmt --all -- --check`: pass
|
||
|
||
## Risks / follow-ups
|
||
|
||
- No production media encryption path exists yet. When one is added (likely in a future wave), the send loop must pass `pkt.to_bytes()[MediaHeader::WIRE_SIZE..]` as the plaintext to `CryptoSession::encrypt` and `pkt.header.to_bytes()` as AAD. The `analyzer.rs` replay decrypt path already follows this pattern.
|
||
- Mini-frame compression (`encode_compact`) does not carry `quality_report` by design (mini frames are payload-only deltas). This is acceptable because quality reports are sent on full frames, which the encoder already does.
|
||
|
||
## Reviewer checklist (filled in by reviewer)
|
||
|
||
- [x] Code matches PRD intent — W5 invariant ("QR is inside AEAD payload, header is AAD") is correctly encoded in `MediaPacket::to_bytes()` order and pinned by the new test
|
||
- [x] Verification output is real — re-ran `cargo test -p wzp-client quality_report_aead` (1 pass), clippy clean on `wzp-client` and `wzp-crypto`
|
||
- [x] No backward-incompat surprises — wire format unchanged; adds a regression test
|
||
- [x] Tests cover the new behavior — tampering a byte in the QR region of ciphertext makes decrypt fail
|
||
- [x] Approved
|
||
|
||
### Reviewer notes (2026-05-11)
|
||
|
||
Approved. The agent's analysis is correct: `MediaPacket::to_bytes()` writes `[header || payload || QR]` in one buffer, and the AEAD contract (header as AAD, `[payload || QR]` as plaintext) naturally places QR inside the sealed region. No production refactor was needed. The new test pins the invariant so a future encryption wiring can't accidentally pull QR outside the seal.
|
||
|
||
**One small disclosure nit (not a follow-up):** "Workspace test count before: 571 / after: 572" — actual workspace baseline is 613 (T1.6 lifted it). Looks like the agent measured the `wzp-client`/`wzp-proto` subset. Minor; substance is fine.
|
||
|
||
**Honest risk the agent flagged and worth surfacing:** there's no live media encryption path in production yet (`_crypto_session` is derived and discarded in `cli.rs`). The W5 invariant matters only when that wiring lands. When it does, this test is the guard. The "AEAD wired into the send loop" task is implicit and doesn't yet have a task ID — worth promoting to a real entry when planning the next wave.
|