Skip to content

fix(audit): frame hash inputs with TLV - #7492

Merged
jmecom merged 1 commit into
mainfrom
jm/fix-hash-input-encodings
Sep 22, 2026
Merged

fix(audit): frame hash inputs with TLV#7492
jmecom merged 1 commit into
mainfrom
jm/fix-hash-input-encodings

Conversation

@jmecom

@jmecom jmecom commented Sep 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Hash new audit entries from a TLV buffer built with one-byte field tags, big-endian u64 byte lengths, and raw values. Absent optional fields are omitted; present empty values retain their tag and zero length. This prevents bytes from moving between fields without restricting object IDs or inferring boundaries from field contents.

Store an explicit hash_version. Migration 0048 labels existing rows as version 1, and new writers use version 2. Historical hashes are unchanged and mixed-version chains verify. The legacy encoding retains its original ambiguity; TLV protects new entries. Apply the migration before deploying the updated writers and verifiers.

Tests cover both boundary collisions, absent versus empty fields, stable digests, unknown versions, mixed-version chains, and stored-row tampering. All 16 audit and 128 database unit tests passed. The six PostgreSQL audit tests passed against both migrated and fresh schemas, and a populated-row migration check preserved the historical hash. Full just ci passed.

Related audit fixes: #4216 and #4249; issue #4173.

@github-actions

github-actions Bot commented Sep 8, 2026 •
edited
Loading

Copy link
Copy Markdown

🔐 Codex Security Review

Status: review required for the current range.

The current range is 77729abfb692b25a0f4ec4a69add86af2e32c0dd...53aa6bed29e7ec06b2f6d94002563431ec23ed7e.
A new review must complete for this exact range. When manual authorization
is required, a Block organization member must comment exactly
@buzz-security-review 53aa6bed29e7ec06b2f6d94002563431ec23ed7e to authorize a new review.
Any previous review applies only to its recorded range.

@jmecom jmecom changed the title fix: remove ambiguous encodings from security hashes fix: remove ambiguous encodings from hash inputs Sep 8, 2026
@jmecom
jmecom marked this pull request as ready for review September 8, 2026 19:27
@jmecom
jmecom requested a review from a team as a code owner September 8, 2026 19:27
@jmecom
jmecom force-pushed the jm/fix-hash-input-encodings branch from daa6f43 to dcd3148 Compare September 8, 2026 19:29
@mfethe1 mfethe1 mentioned this pull request Sep 11, 2026
Open
@jmecom
jmecom force-pushed the jm/fix-hash-input-encodings branch 2 times, most recently from 7cde393 to e07cbeb Compare September 21, 2026 23:23
@jmecom jmecom changed the title fix: remove ambiguous encodings from hash inputs fix(audit): reject ambiguous hash inputs Sep 21, 2026
@jmecom
fix(audit): frame hash inputs with TLV
53aa6be 
Signed-off-by: Jordan Mecom <jm@squareup.com>
@jmecom
jmecom force-pushed the jm/fix-hash-input-encodings branch from e07cbeb to 53aa6be Compare September 21, 2026 23:55
@jmecom jmecom changed the title fix(audit): reject ambiguous hash inputs fix(audit): frame hash inputs with TLV Sep 21, 2026
wpfleger96
wpfleger96 approved these changes Sep 22, 2026
View reviewed changes

@wpfleger96 wpfleger96 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed 53aa6bed29e7ec06b2f6d94002563431ec23ed7e and didn’t find any blocking issues. The TLV framing covers the variable-length boundaries and distinguishes absent from present-empty fields, while the domain prefix separates the new encoding. I also checked that the legacy hash implementation is unchanged, new writes persist version 2 explicitly, and both read paths carry the stored version through verification. Keeping the database default at 1 preserves old-writer inserts during rollout without rewriting historical hashes.

CI at this head ran all 16 audit unit tests and all six PostgreSQL audit tests successfully. I also redirected the production v2 dispatch to legacy hashing in a disposable worktree and ran the full non-ignored audit package suite: both boundary regressions, the v2 digest pin, and the absent-versus-zero previous-hash case failed as expected. The mutation was discarded.

This approval is based on source review, the executed CI coverage, and that mutation check; I haven’t independently exercised a populated production-style rollout. The documented migration-before-writers/verifiers ordering still applies.

@jmecom
jmecom merged commit 7881447 into main Sep 22, 2026
80 checks passed
@jmecom
jmecom deleted the jm/fix-hash-input-encodings branch September 22, 2026 18:19
wpfleger96 pushed a commit that referenced this pull request Sep 22, 2026
Duncan
Merge remote-tracking branch 'origin/main' into wpfleger/deterministi…
f3581d2 
…c-agent-commit-identity

* origin/main:
  fix(desktop): bound startup request bursts and recover quota refusals (#7790)
  fix(audit): frame hash inputs with TLV (#7492)
  fix(admin): allow cold storage worker DB startup (#7770)

Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
brow added a commit that referenced this pull request Sep 22, 2026
@brow
Merge remote-tracking branch 'origin/main' into personal-railway-push…
66d9340 
…-dev

* origin/main:
  fix(desktop): bound startup request bursts and recover quota refusals (#7790)
  fix(audit): frame hash inputs with TLV (#7492)
  fix(admin): allow cold storage worker DB startup (#7770)
  feat(relay): add admin HTTP routes for member restriction management (#7302)
  fix(relay): fire kick live side effects at convergence; persist target; fence re-add race with held lock (#7298)
  feat(relay): add atomic complete read-state snapshots (#7572)
  fix(desktop): register macOS badges for new and existing installs (#7783)
  fix(mobile): avoid opening empty threads on message tap (#7756)
  fix(workflows): make deletion persistent and retryable (#7735)
  fix(mobile): preserve thread replies through refresh failures (#7757)
  fix(mobile): keep iOS message menu actions responsive after rebuilds (#7758)
  fix(relay): exclude ephemeral activity from message quota (#7736)
  release: push gateway chart 0.3.1 (#7749)
  fix(push): label plaintext push gateway service as HTTP (#7717)
  Replace personal and internal data in desktop test fixtures (#7748)
  Add mobile VISION (#7710)
  fix(mobile): keep relay sessions stable during push lease updates (#7745)

Signed-off-by: Tom Brow <tomb@block.xyz>
wpfleger96 pushed a commit that referenced this pull request Sep 22, 2026
Duncan
Merge remote-tracking branch 'origin/main' into wpfleger/desktop-admi…
8ed8a4a 
…n-surface

* origin/main:
  docs(nip-fi): clarify federated identity amendments (#7803)
  fix(desktop): refresh channels after access-revoked closure (#7784)
  fix(desktop): bound startup request bursts and recover quota refusals (#7790)
  fix(audit): frame hash inputs with TLV (#7492)
  fix(admin): allow cold storage worker DB startup (#7770)

Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
wpfleger96 pushed a commit that referenced this pull request Sep 22, 2026
Duncan
Merge remote-tracking branch 'origin/main' into duncan/nip-fi-blossom…
cb7b891 
…-hardening

* origin/main:
  docs(nip-fi): clarify federated identity amendments (#7803)
  fix(desktop): refresh channels after access-revoked closure (#7784)
  fix(desktop): bound startup request bursts and recover quota refusals (#7790)
  fix(audit): frame hash inputs with TLV (#7492)
  fix(admin): allow cold storage worker DB startup (#7770)

Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
wpfleger96 added a commit that referenced this pull request Sep 22, 2026
Hayt @wpfleger96
feat(nip-fi): S4 deny-until-TTL API — full implementation + witnesses
4f7c8c1 
NIP-FI admin disconnect/deny API implementation, rebased onto
origin/main (c213d90) after main advanced past the original merge-base
(77729ab). No buzz-relay production code changed by main's 3 new
commits (#7770, #7492, #7790) — rebase is conflict-free.

Squash of all commits from 2a42ddf through 360150d97:

Production:
- Deny-map JWT verifier and command installer (S4)
- Disconnect endpoint and route wiring
- Admin HTTP routes for deny/undeny, expiry TTL
- WS connection admission: deny-map check wired in
- Audio pre-writer send bounding (1s timeout policy)
- nip_fi_session.rs: bounded pre-registration pairing sends
- handler.rs MINOR: Ok(Ok(())) success check replaces is_err()

Witnesses (Thufir round-5 required corrections):
- R2: drain_terminal!() race witness (before_not_a_member_drain_terminal
  hook + cancel_race_test_hook; falsification: remove lifecycle_cancel()
  → frame-1 timeout RED)
- F3: real admission → 48101 JOIN wire bytes → handle_huddle_liveness_req
  oracle → authoritative generation == wire generation
- F4: valid future deadline + cancel-while-held (not already-expired);
  falsification: already-expired → hook timeout RED
- F5: {"kinds":[13534]} REQ skips DB lookup, reaches after_req_permit_acquired;
  AUTH OK(true) check; real Postgres+Redis pools; canonical denial frame +
  1008 close; zero global+channel refcounts asserted
- F6: ENV_TEST_MUTEX synchronization for Config::from_env()
- F7: 10s bounded consumer threads; entry-hook Drop guards
- R1: reason-published/before-enqueue witness
- F1/F2: LifecycleClosed transition primitives

Evidence inventory corrections per Thufir's requirement:
root raw cancels follow pairing_deny_terminal/auth_deny_terminal;
audio pre-registration pairing sends direct denial/close;
post-registration deny-set/expired paths send directly then raw-cancel;
wrapper cancellation exists before AND after run(control).await.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
wpfleger96 added a commit that referenced this pull request Sep 22, 2026
Hayt @wpfleger96
feat(nip-fi): S4 deny-until-TTL API — full implementation + witnesses
cc5aa3b 
NIP-FI admin disconnect/deny API implementation, rebased onto
origin/main (c213d90) after main advanced past the original merge-base
(77729ab). No buzz-relay production code changed by main's 3 new
commits (#7770, #7492, #7790) — rebase is conflict-free.

Squash of all commits from 2a42ddf through 360150d97:

Production:
- Deny-map JWT verifier and command installer (S4)
- Disconnect endpoint and route wiring
- Admin HTTP routes for deny/undeny, expiry TTL
- WS connection admission: deny-map check wired in
- Audio pre-writer send bounding (1s timeout policy)
- nip_fi_session.rs: bounded pre-registration pairing sends
- handler.rs MINOR: Ok(Ok(())) success check replaces is_err()

Witnesses (Thufir round-5 required corrections):
- R2: drain_terminal!() race witness (before_not_a_member_drain_terminal
  hook + cancel_race_test_hook; falsification: remove lifecycle_cancel()
  → frame-1 timeout RED)
- F3: real admission → 48101 JOIN wire bytes → handle_huddle_liveness_req
  oracle → authoritative generation == wire generation
- F4: valid future deadline + cancel-while-held (not already-expired);
  falsification: already-expired → hook timeout RED
- F5: {"kinds":[13534]} REQ skips DB lookup, reaches after_req_permit_acquired;
  AUTH OK(true) check; real Postgres+Redis pools; canonical denial frame +
  1008 close; zero global+channel refcounts asserted
- F6: ENV_TEST_MUTEX synchronization for Config::from_env()
- F7: 10s bounded consumer threads; entry-hook Drop guards
- R1: reason-published/before-enqueue witness
- F1/F2: LifecycleClosed transition primitives

Evidence inventory corrections per Thufir's requirement:
root raw cancels follow pairing_deny_terminal/auth_deny_terminal;
audio pre-registration pairing sends direct denial/close;
post-registration deny-set/expired paths send directly then raw-cancel;
wrapper cancellation exists before AND after run(control).await.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
wpfleger96 added a commit that referenced this pull request Sep 22, 2026
Hayt @wpfleger96
feat(nip-fi): S4 deny-until-TTL API — full implementation + witnesses
7dbb43c 
NIP-FI admin disconnect/deny API implementation, rebased onto
origin/main (c213d90) after main advanced past the original merge-base
(77729ab). No buzz-relay production code changed by main's 3 new
commits (#7770, #7492, #7790) — rebase is conflict-free.

Squash of all commits from 2a42ddf through 360150d97:

Production:
- Deny-map JWT verifier and command installer (S4)
- Disconnect endpoint and route wiring
- Admin HTTP routes for deny/undeny, expiry TTL
- WS connection admission: deny-map check wired in
- Audio pre-writer send bounding (1s timeout policy)
- nip_fi_session.rs: bounded pre-registration pairing sends
- handler.rs MINOR: Ok(Ok(())) success check replaces is_err()

Witnesses (Thufir round-5 required corrections):
- R2: drain_terminal!() race witness (before_not_a_member_drain_terminal
  hook + cancel_race_test_hook; falsification: remove lifecycle_cancel()
  → frame-1 timeout RED)
- F3: real admission → 48101 JOIN wire bytes → handle_huddle_liveness_req
  oracle → authoritative generation == wire generation
- F4: valid future deadline + cancel-while-held (not already-expired);
  falsification: already-expired → hook timeout RED
- F5: {"kinds":[13534]} REQ skips DB lookup, reaches after_req_permit_acquired;
  AUTH OK(true) check; real Postgres+Redis pools; canonical denial frame +
  1008 close; zero global+channel refcounts asserted
- F6: ENV_TEST_MUTEX synchronization for Config::from_env()
- F7: 10s bounded consumer threads; entry-hook Drop guards
- R1: reason-published/before-enqueue witness
- F1/F2: LifecycleClosed transition primitives

Evidence inventory corrections per Thufir's requirement:
root raw cancels follow pairing_deny_terminal/auth_deny_terminal;
audio pre-registration pairing sends direct denial/close;
post-registration deny-set/expired paths send directly then raw-cancel;
wrapper cancellation exists before AND after run(control).await.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
wpfleger96 added a commit that referenced this pull request Sep 23, 2026
Hayt @wpfleger96
feat(nip-fi): S4 deny-until-TTL API — full implementation + witnesses
cea34b1 
NIP-FI admin disconnect/deny API implementation, rebased onto
origin/main (c213d90) after main advanced past the original merge-base
(77729ab). No buzz-relay production code changed by main's 3 new
commits (#7770, #7492, #7790) — rebase is conflict-free.

Squash of all commits from 2a42ddf through 360150d97:

Production:
- Deny-map JWT verifier and command installer (S4)
- Disconnect endpoint and route wiring
- Admin HTTP routes for deny/undeny, expiry TTL
- WS connection admission: deny-map check wired in
- Audio pre-writer send bounding (1s timeout policy)
- nip_fi_session.rs: bounded pre-registration pairing sends
- handler.rs MINOR: Ok(Ok(())) success check replaces is_err()

Witnesses (Thufir round-5 required corrections):
- R2: drain_terminal!() race witness (before_not_a_member_drain_terminal
  hook + cancel_race_test_hook; falsification: remove lifecycle_cancel()
  → frame-1 timeout RED)
- F3: real admission → 48101 JOIN wire bytes → handle_huddle_liveness_req
  oracle → authoritative generation == wire generation
- F4: valid future deadline + cancel-while-held (not already-expired);
  falsification: already-expired → hook timeout RED
- F5: {"kinds":[13534]} REQ skips DB lookup, reaches after_req_permit_acquired;
  AUTH OK(true) check; real Postgres+Redis pools; canonical denial frame +
  1008 close; zero global+channel refcounts asserted
- F6: ENV_TEST_MUTEX synchronization for Config::from_env()
- F7: 10s bounded consumer threads; entry-hook Drop guards
- R1: reason-published/before-enqueue witness
- F1/F2: LifecycleClosed transition primitives

Evidence inventory corrections per Thufir's requirement:
root raw cancels follow pairing_deny_terminal/auth_deny_terminal;
audio pre-registration pairing sends direct denial/close;
post-registration deny-set/expired paths send directly then raw-cancel;
wrapper cancellation exists before AND after run(control).await.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wpfleger96 wpfleger96 wpfleger96 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jmecom @wpfleger96