fix(mobile/ios): secure push relay resolution#2744
Conversation
… vectors Add tests/vectors/app_attest_transcripts.json — deterministic known-answer vectors (exact transcript bytes + SHA-256) for all five NIP-PL App Attest operations: enroll, delegate, rotate_endpoint, revoke_delegation, revoke_installation — generated by the gateway's own transcript() encoder, plus in-crate tests asserting the encoder reproduces the fixture byte-for-byte. The fixture is shared ground truth for client-side canonical encoders (the Swift NIP-PL iOS client): a client that fails to reproduce these bytes exactly fails every attested call with invalid_attestation and no diagnostic (Swift JSONSerialization's \/ escaping is the known trap). Any future change to the Rust encoder now breaks a test instead of silently stranding clients. Co-authored-by: Tyler Longwell <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
BuzzPushKit SwiftPM package with a hand-rolled ordered-compact-JSON transcript encoder for all five App Attest domains (enroll / delegate / rotate-endpoint / revoke-delegation / revoke-installation), verified byte-for-byte + SHA-256 against the gateway-generated known-answer vectors in crates/buzz-push-gateway/tests/vectors/app_attest_transcripts.json. JSONSerialization is deliberately not used anywhere near these bytes: it escapes '/' as '\/' and does not guarantee member order, either of which silently strands every iOS client with 401 invalid_attestation. Non-ASCII input fails closed per the NIP-PL ASCII profile. The 'audience' member is a fixed NIP-PL protocol constant (https://push.buzz.xyz/v1/...) hardcoded by the gateway for cross-route domain separation — clients must never derive it from a discovered gateway base URL or relay host. Co-authored-by: Tyler Longwell <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
Co-authored-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co>
Co-authored-by: npub102wg7q285p64ch2fjvstmf2ntn2sz3c4u5hmwatalc76mhsuauysftjtfj <7a9c8f0147a0755c5d499320bda5535cd5014715e52fb7757dfe3dadde1cef09@buzz.block.builderlab.xyz> Signed-off-by: npub102wg7q285p64ch2fjvstmf2ntn2sz3c4u5hmwatalc76mhsuauysftjtfj <7a9c8f0147a0755c5d499320bda5535cd5014715e52fb7757dfe3dadde1cef09@buzz.block.builderlab.xyz>
Co-authored-by: npub102wg7q285p64ch2fjvstmf2ntn2sz3c4u5hmwatalc76mhsuauysftjtfj <7a9c8f0147a0755c5d499320bda5535cd5014715e52fb7757dfe3dadde1cef09@buzz.block.builderlab.xyz> Signed-off-by: npub102wg7q285p64ch2fjvstmf2ntn2sz3c4u5hmwatalc76mhsuauysftjtfj <7a9c8f0147a0755c5d499320bda5535cd5014715e52fb7757dfe3dadde1cef09@buzz.block.builderlab.xyz>
Co-authored-by: npub15uvq03jgm52chkxa5x7f2ud28xh9v48287f5fj5fsuu0wqpm63ns8hn2er <a71807c648dd158bd8dda1bc9571aa39ae5654ea3f9344ca898738f7003bd467@buzz.block.builderlab.xyz> Signed-off-by: npub15uvq03jgm52chkxa5x7f2ud28xh9v48287f5fj5fsuu0wqpm63ns8hn2er <a71807c648dd158bd8dda1bc9571aa39ae5654ea3f9344ca898738f7003bd467@buzz.block.builderlab.xyz>
Co-authored-by: npub15uvq03jgm52chkxa5x7f2ud28xh9v48287f5fj5fsuu0wqpm63ns8hn2er <a71807c648dd158bd8dda1bc9571aa39ae5654ea3f9344ca898738f7003bd467@buzz.block.builderlab.xyz> Signed-off-by: npub15uvq03jgm52chkxa5x7f2ud28xh9v48287f5fj5fsuu0wqpm63ns8hn2er <a71807c648dd158bd8dda1bc9571aa39ae5654ea3f9344ca898738f7003bd467@buzz.block.builderlab.xyz>
brow
left a comment
There was a problem hiding this comment.
🤖 Independent review at 8912fdc (cumulative delta e4b1e81..8912fdc: blockers 1+2, main merge, lifecycle-sync fix, and review-fix commit), second half of the standing dual-review loop. Re-posted from #2732's approval after the PR was reopened here; same branch, same head, no code delta.
Verdict: approve. (Formal state is COMMENT because this PR is authored by the same brow account my review credentials use, and GitHub rejects self-approval. Treat this review as the approval of record for the load-bearing half of the dual-review gate.)
Blockers-1/2 core (4c99a83 + merge 14a529d):
- NIP-98 client matches the relay contract: kind 27235,
u/method/payloadtags, canonical NIP-01 serialization (JSONSerialization+.withoutEscapingSlashes), cross-checked againstcrates/buzz-auth/src/nip98.rs(±60s window, normalized per-tenant URL match) andcrates/buzz-relay/src/api/bridge.rs/queryenforcement (auth + replay + membership + p-gated filters). Theutag (https://<relay-host>/queryfrom the stored https relayUrl) lines up withnip98_expected_url's tenant-host reconstruction and normalization. - Event verification is fail-closed: SHA-256 id recomputation over canonical form + BIP-340 Schnorr over the digest; malformed hex/lengths reject.
- Keychain transfer is symmetric: Runner writes
buzz.push.nse.signingwith the access group from Info.plist ($(AppIdentifierPrefix)$(BUZZ_KEYCHAIN_ACCESS_GROUP)), NSE reads with the same key from its own Info.plist, and both targets' entitlements carry the matchingkeychain-access-groupsentry. Fail-closed on partial writes (delete-all on SecItemAdd failure). - Merge 14a529d verified against
git merge-tree: pbxproj-only conflict resolution (PushNativeState + MediaSanitizer both kept), no other content. - APNs registration buffer: cache/replay semantics with Dart callbacks covered by tests, including replay-after-attach and error replay.
Review-fix commit 8912fdc (both prior findings addressed):
- Auth lockout: every snapshot export routes through
syncCommunitySnapshot— best-effort, observable (pushCommunitySnapshotError+ debug log), never gatesauthProvider. The dedup fingerprint only advances on success, so failed exports stay retryable; sign-out key-revocation export is still attempted but no longer load-bearing. Regression tests cover startup and direct authentication with a throwing writer. - Watermark poisoning:
PushWatermarkclamps persisted timestamps to now+300s, rejects out-of-skew events at resolution, self-repairs already-poisoned stored values before querying, keepssinceinclusive for same-second events, and prunes stale per-community keys (prefix-guarded against unrelated defaults). Cross-community tie-break now matches per-community selection and relay ordering (created_at DESC, id ASC).
Independently validated at this exact head: BuzzPushKit swift test 20/20; flutter test 552 passed / 1 skipped; flutter analyze clean.
Non-blocking (accepted as follow-ups by the coordinator): the BuzzPushKit suite does not run in CI — the Mobile job is ubuntu-only and the package needs CryptoKit/Security — so the new watermark/auth-header tests are local-only signal; a small macOS CI step running swift test in mobile/ios/BuzzPushKit would close that gap. One inline nit on the snapshot fingerprint.
| community.nsec, | ||
| ].join('\u0000'), | ||
| ) | ||
| .join('\u0001'); |
There was a problem hiding this comment.
🤖 Non-blocking (re-posted from #2732; accepted as a follow-up): this fingerprint concatenates every community's nsec and is retained for the provider's lifetime in _lastSuccessfulSnapshot. The keys are already resident in provider state elsewhere, so this is not a new exposure class, but storing a digest (e.g. SHA-256) of the joined string instead would avoid holding one more long-lived plaintext copy of all signing keys.
|
🤖 Review record: this PR replaces #2732 (closed only because it was opened from the wrong GitHub account; identical branch and head 8912fdc). The dual-review loop completed on #2732 at this exact head with three independent approvals: #2732 (review) (goblin), belt & suspenders' approval, and #2732 (review) (load-bearing, one non-blocking inline note). All reviewer agents share this GitHub identity, which authored this PR, so approvals cannot be re-submitted here. |
brow
left a comment
There was a problem hiding this comment.
🤖 Re-recording my approval from #2732 (review 4775825106) on this reopened PR — same branch, identical head 8912fdc. GitHub blocks a formal APPROVE here because this PR was opened by the same account my reviews are submitted from (brow), so this COMMENT review carries the approval record instead; the formal approval on the identical head lives on #2732.
Independent review at this exact head: both round-1 findings (auth lockout via load-bearing snapshot export; NSE watermark poisoning) verified fixed, and all three nits addressed (tie-break alignment, stale watermark key pruning, snapshot write dedup). Independently validated at this head: BuzzPushKit swift test 20 passed; flutter test 552 passed / 1 skipped; flutter analyze clean.
Verdict: APPROVE (formal GitHub approval mechanically impossible from this account).
Summary
Builds on #1924 and addresses review blockers 1 and 2 for the NIP-PL iOS push notification foundation.
Blocker 1: authenticated and verified NSE relay resolution
/queryrequests with NIP-98Blocker 2: APNs callback replay
ValueNotifierstateTests
swift test: 14 passed../bin/flutter test: 487 passed, 1 skippedgit diff --check: cleanStacking note
This branch is one commit on top of the exact #1924 head used during implementation (
e4b1e81312f57edb43fbcf1bfa5fb26eec8aef2e). It intentionally includes that foundation when compared directly withmain.