diff --git a/zeronym/shim/deploy/EXPECTED_SHA256 b/zeronym/shim/deploy/EXPECTED_SHA256 index 4e093b2a..7de770fc 100644 --- a/zeronym/shim/deploy/EXPECTED_SHA256 +++ b/zeronym/shim/deploy/EXPECTED_SHA256 @@ -1 +1 @@ -da0b12a6901f19ab6e9e38846039196eccc39ea62090da172426b3e426b8babe +dae86efed5dbbce185539afa0bdb033ff8a61aec306e83d44c4c28ba00f132db diff --git a/zeronym/shim/deploy/README.md b/zeronym/shim/deploy/README.md index 9a035509..3d1db086 100644 --- a/zeronym/shim/deploy/README.md +++ b/zeronym/shim/deploy/README.md @@ -310,7 +310,8 @@ below names which deployment each superseded binary belongs to, where one does. | | binary sha256 | what it was built from | |---|---|---| -| **current**, the zaino-proto 0.6.0 stack (zaino 0.10.0 re-vendor) | `da0b12a6901f19ab6e9e38846039196eccc39ea62090da172426b3e426b8babe` | the zaino 0.10.0 re-vendor moves the path-dependency `zaino-proto` 0.4.0 to 0.6.0. No source change in the shim and no recipe change: `src/` and `Cargo.toml` are untouched, `Cargo.lock` moves by exactly the one path-dep version line, and the hash moved because the compiled proto crate did. Same class as the `77fa2dc4…` row below, which was the 0.8.0 pull's 0.4.0 bump, except that one also needed two `.into()` call sites and this one needs none: the `Bytes` payload type carried over unchanged. Measured across FOUR cold builds on TWO independent x86_64 runners (two runs of two builds each), all agreeing, with `zebra/` and `zaino/` clean. | +| **current**, the queue-hit sentinel relayed rather than refused | `dae86efed5dbbce185539afa0bdb033ff8a61aec306e83d44c4c28ba00f132db` | one source file. `intercept::get_transaction` takes the hub's queue-hit sentinel, `Found { data: empty, height: 0 }`, in its own arm ahead of the L4 byte guard. The guard deserializes the returned bytes to check them against the queried txid; an empty body does not deserialize, so it failed every queued migration into NOT_FOUND, which is the entire pending signal a stateless shim has. No recipe change, no dependency change: `Cargo.toml` and `Cargo.lock` are untouched, and the tests and `smoke.sh` that moved with the fix are not compiled inputs. **No new strings**, because the change is control flow: `strings` cannot tell this binary from `da0b12a6...`, so unlike the rows below the distinguisher is the test suite and `smoke-local.sh` rather than inspection. `the hub returned a transaction whose txid does not match the query` is still present exactly once, as it must be, since a real mismatch is still refused. Measured across FOUR cold builds on TWO machines and TWO architectures, all agreeing: two on a native x86_64 CI runner and two locally on an arm64 Mac under Rosetta, with `zebra/` and `zaino/` clean. 27773904 bytes, `ELF 64-bit LSB pie executable, x86-64, static-pie linked`, and zero occurrences of any host path. | +| superseded, the zaino-proto 0.6.0 stack (zaino 0.10.0 re-vendor) | `da0b12a6901f19ab6e9e38846039196eccc39ea62090da172426b3e426b8babe` | the zaino 0.10.0 re-vendor moves the path-dependency `zaino-proto` 0.4.0 to 0.6.0. No source change in the shim and no recipe change: `src/` and `Cargo.toml` are untouched, `Cargo.lock` moves by exactly the one path-dep version line, and the hash moved because the compiled proto crate did. Same class as the `77fa2dc4…` row below, which was the 0.8.0 pull's 0.4.0 bump, except that one also needed two `.into()` call sites and this one needs none: the `Bytes` payload type carried over unchanged. Measured across FOUR cold builds on TWO independent x86_64 runners (two runs of two builds each), all agreeing, with `zebra/` and `zaino/` clean. | | superseded, the Hornby-review hardening on the zaino-proto 0.4.0 stack | `7375176ddcf482ead8b726f2fff70a48fad0f61f8da22e81a3576539aae23591` | the two rows below merged, no compiled change of its own: the Hornby-review bounds and refusals (`1646a1b7…`) rebuilt against the zaino 0.8.0 subtree's zaino-proto 0.4.0 (`77fa2dc4…`). Measured across FOUR cold builds on the x86_64 runner: two independent runs of two builds each, hours apart, all agreeing. | | superseded, the Hornby-review bounds and refusals | `1646a1b720903d6ded261641baf8d8e7743705a3123cad1c597c5ffb16e5b13d` | three commits touching `src/`, answering findings from Taylor Hornby's review. (1) `e37e2a7` bounds the inbound listener at 256 connections, the permit held for the connection's LIFE so an idle socket still costs a slot -- per-stream was capped at 4 MiB and nothing capped the aggregate, which reached OOM against a 2048 MB enclave at roughly 512 concurrent requests -- and adds `--require-diversion`, so a shim can refuse to start forward-only instead of silently resolving an unset `ZIS_HUB_NYM` into "No privacy". (2) `48cd321` refuses an empty transaction before it costs a mixnet frame, tells an unrecognised consensus branch id apart from garbage and reports it once per process, warns at startup when more than one hub is configured naming each, reports what each teardown path abandons, and publishes `address_generation` on `/nym-status`. (3) `e000886` changed only comments, and moved the hash anyway: Rust panic locations carry `file!()`/`line!()`, so the binary embeds `src/nym.rs` and shifting its lines shifts the binary. Two cold builds on this host agree, and `strings` finds one `zero-indexer-shim: empty transaction`, one `connection refused: every in-flight slot is held`, one `--require-diversion is set but no hub transport is configured` and three `invalid consensus branch id`, none of which the `b91fa275…` binary contains. 27772832 bytes, `ELF 64-bit LSB pie executable, x86-64, static-pie linked`, and zero occurrences of any host path. CI's native x86_64 double-build is the cross-machine check. | | superseded, zaino-proto 0.4.0 Bytes payloads | `77fa2dc49fab22f1cbf7059d8bba819e7b4ebb45d9de36619bd977245cfded0f` | the zaino 0.8.0 subtree pull bumps the path-dependency `zaino-proto` to 0.4.0, which serves `RawTransaction.data` as prost `Bytes` instead of `Vec`; the shim's two `RawTransaction` construction sites gain `.into()` (`Vec` to `Bytes` is a zero-copy move). The classifier predicate and every route are unchanged: the hash moved because the compiled proto crate and those two sites did. `zeronym-shim-reproduce` reports SELF-CONSISTENT across two cold builds on the x86_64 runner, measured from the tree that vendors zaino 0.8.0. | diff --git a/zeronym/shim/src/intercept.rs b/zeronym/shim/src/intercept.rs index 89af2077..7bcb875e 100644 --- a/zeronym/shim/src/intercept.rs +++ b/zeronym/shim/src/intercept.rs @@ -368,6 +368,29 @@ pub(crate) async fn get_transaction( } match diversion.hub.get_transaction(&filter.hash).await { + // The hub's queue-hit sentinel: found, height 0, no bytes. Since + // 45e408f0ff the hub answers a lookup for a QUEUED migration this way, + // because the lookup is unauthenticated on both transports and serving + // a not-yet-published migration's bytes to whoever asks would let a + // third party broadcast it first. Relaying the sentinel is the whole + // point of it: height 0 is the mempool sentinel, and it is the + // existence-and-status signal this stateless shim has nothing else to + // answer from. A wallet renders "pending" from it. + // + // It must not go through the L4 guard below. The guard verifies the + // RETURNED BYTES against the queried txid, and there are none here; it + // would deserialize an empty body, fail, and turn every queued + // migration into NOT_FOUND. Nor is there anything for it to protect: + // the attack L4 exists to stop is a hub substituting a DIFFERENT + // transaction's bytes, which an empty body cannot do. + // + // Height 0 only. A mined transaction always has bytes, so an empty body + // at a nonzero height is not a queue hit and is not something to hand a + // wallet as a transaction; it falls through to the arm below, where the + // guard refuses it. + Ok(Lookup::Found { data, height }) if data.is_empty() && height == 0 => { + Ok(get_transaction_response(&data, height)) + } Ok(Lookup::Found { data, height }) => { // L4: verify the hub returned the transaction that was ASKED for. A // hub, buggy or hostile, that answers a query with a DIFFERENT @@ -455,9 +478,11 @@ fn not_found_message(wire_hash: &[u8]) -> String { ) } -/// A synthesized `GetTransaction` reply carrying the transaction the hub -/// returned. Height 0 (from a queue hit) is the mempool sentinel; a mined -/// transaction relays the indexer's height. +/// A synthesized `GetTransaction` reply carrying what the hub returned. Height 0 +/// is the mempool sentinel; a mined transaction relays the indexer's height. The +/// bytes are the hub's verbatim, and for a queue hit there are none: the hub +/// withholds a queued migration's bytes, and the wallet that sent it already has +/// them. fn get_transaction_response(tx_bytes: &[u8], height: u64) -> Response { let message = RawTransaction { data: tx_bytes.to_vec().into(), diff --git a/zeronym/shim/tests/divert.rs b/zeronym/shim/tests/divert.rs index d2a802bd..ed80c0a6 100644 --- a/zeronym/shim/tests/divert.rs +++ b/zeronym/shim/tests/divert.rs @@ -256,6 +256,110 @@ async fn a_get_transaction_is_answered_by_the_hub_and_the_operator_is_never_dial ); } +#[tokio::test] +async fn a_queue_hit_with_no_bytes_is_relayed_as_pending() { + // The hub answers a lookup for a QUEUED migration "found, height 0, no + // bytes": it withholds the bytes of a transaction it has not published yet. + // That sentinel IS the answer -- it is the only existence-and-status signal + // a stateless shim has, and a wallet renders "pending" from it -- so the + // shim must relay it rather than run it through the L4 byte guard, which + // has nothing to verify and would turn every queued migration into + // NOT_FOUND. + let looked_up = Arc::new(Mutex::new(None)); + let hub = spawn_mock_hub_full( + "unused", + HubLookup::Found { + data: Vec::new(), + height: 0, + }, + Arc::new(Mutex::new(None)), + looked_up.clone(), + ) + .await; + let backend_conns = Arc::new(AtomicUsize::new(0)); + let backend = spawn_counting_backend(backend_conns.clone()).await; + let shim = spawn_diverting_shim(backend, hub).await; + + let mut sender = connect_h2(shim).await; + let hash = wire_hash(V6_MIGRATION); + let reply = get_transaction(&mut sender, shim, &hash).await; + + assert_eq!(reply.status, 0, "a queue hit is a success, not NOT_FOUND"); + let raw = decode_raw_transaction(&reply.body); + assert!( + raw.data.is_empty(), + "the hub's withheld body is relayed as-is" + ); + assert_eq!(raw.height, 0, "height 0 is the mempool sentinel"); + + assert_eq!(looked_up.lock().unwrap().as_deref(), Some(&hash[..])); + assert_eq!( + backend_conns.load(Ordering::SeqCst), + 0, + "a hub-served GetTransaction must not dial the operator" + ); +} + +#[tokio::test] +async fn a_hub_reply_for_a_different_txid_is_refused_not_served() { + // L4, and the check that accepting the empty-body sentinel above did not + // widen it: a hub that answers with a transaction OTHER than the one + // queried must not have it served to the wallet under the queried txid. + let backend_conns = Arc::new(AtomicUsize::new(0)); + let backend = spawn_counting_backend(backend_conns.clone()).await; + let hub = spawn_mock_hub_full( + "unused", + HubLookup::Found { + data: V6_MIGRATION.to_vec(), + height: 0, + }, + Arc::new(Mutex::new(None)), + Arc::new(Mutex::new(None)), + ) + .await; + let shim = spawn_diverting_shim(backend, hub).await; + + let mut sender = connect_h2(shim).await; + // Query a hash that is NOT V6_MIGRATION's txid; the hub returns V6_MIGRATION. + let reply = get_transaction(&mut sender, shim, &[0x11u8; 32]).await; + + assert_eq!( + reply.status, 5, + "a mismatched lookup reply is refused as NOT_FOUND, not served" + ); + assert_eq!( + backend_conns.load(Ordering::SeqCst), + 0, + "refusing a mismatched reply must not fall back to the operator" + ); +} + +#[tokio::test] +async fn an_empty_body_at_a_mined_height_is_refused() { + // The sentinel is height 0 ONLY. A mined transaction always has bytes, so an + // empty body at a nonzero height is not a queue hit and is not anything to + // hand a wallet as a transaction: it goes through the guard and fails it. + let backend_conns = Arc::new(AtomicUsize::new(0)); + let backend = spawn_counting_backend(backend_conns.clone()).await; + let hub = spawn_mock_hub_full( + "unused", + HubLookup::Found { + data: Vec::new(), + height: 424_242, + }, + Arc::new(Mutex::new(None)), + Arc::new(Mutex::new(None)), + ) + .await; + let shim = spawn_diverting_shim(backend, hub).await; + + let mut sender = connect_h2(shim).await; + let reply = get_transaction(&mut sender, shim, &wire_hash(V6_MIGRATION)).await; + + assert_eq!(reply.status, 5, "an empty body off the chain is refused"); + assert_eq!(backend_conns.load(Ordering::SeqCst), 0); +} + #[tokio::test] async fn get_transaction_height_from_the_hub_is_relayed() { let hub = spawn_mock_hub_full( diff --git a/zeronym/shim/tests/divert_nym.rs b/zeronym/shim/tests/divert_nym.rs index d48bf747..e1b1330e 100644 --- a/zeronym/shim/tests/divert_nym.rs +++ b/zeronym/shim/tests/divert_nym.rs @@ -361,6 +361,71 @@ async fn a_get_transaction_is_answered_over_the_mixnet_and_the_operator_is_never ); } +#[tokio::test] +async fn a_queue_hit_with_no_bytes_is_relayed_as_pending() { + // The mixnet twin of the clearnet case: the hub answers a QUEUED migration + // "found, height 0, no bytes", withholding the bytes of a transaction it has + // not published yet. The sentinel IS the answer -- the only + // existence-and-status signal a stateless shim has, and what a wallet + // renders "pending" from -- so it must be relayed rather than run through + // the L4 byte guard, which has nothing to verify and would turn every queued + // migration into NOT_FOUND. + let backend_conns = Arc::new(AtomicUsize::new(0)); + let backend = spawn_counting_backend(backend_conns.clone()).await; + let (shim, seen) = spawn_nym_shim( + backend, + OnSubmit::Accept, + OnLookup::Found { + data: Vec::new(), + height: 0, + }, + ) + .await; + + let mut sender = connect_h2(shim).await; + let hash = wire_hash(V6_MIGRATION); + let reply = get_transaction(&mut sender, shim, &hash).await; + + assert_eq!(reply.status, 0, "a queue hit is a success, not NOT_FOUND"); + let raw = decode_raw_transaction(&reply.body); + assert!( + raw.data.is_empty(), + "the hub's withheld body is relayed as-is" + ); + assert_eq!(raw.height, 0, "height 0 is the mempool sentinel"); + + assert_eq!(seen.lookups.lock().unwrap().as_slice(), &[hash.clone()]); + assert_eq!( + backend_conns.load(Ordering::SeqCst), + 0, + "a hub-served GetTransaction must not dial the operator" + ); +} + +#[tokio::test] +async fn an_empty_body_at_a_mined_height_is_refused() { + // The sentinel is height 0 ONLY. A mined transaction always has bytes, so an + // empty body at a nonzero height is not a queue hit and is not anything to + // hand a wallet as a transaction: it goes through the guard and fails it. + let backend_conns = Arc::new(AtomicUsize::new(0)); + let backend = spawn_counting_backend(backend_conns.clone()).await; + let (shim, _) = spawn_nym_shim( + backend, + OnSubmit::Accept, + OnLookup::Found { + data: Vec::new(), + height: 424_242, + }, + ) + .await; + + let mut sender = connect_h2(shim).await; + let reply = get_transaction(&mut sender, shim, &wire_hash(V6_MIGRATION)).await; + + assert_eq!(reply.status, 5, "an empty body off the chain is refused"); + assert_eq!(backend_conns.load(Ordering::SeqCst), 0); +} + #[tokio::test] async fn a_mined_height_from_the_hub_is_relayed() { let backend = spawn_counting_backend(Arc::new(AtomicUsize::new(0))).await; diff --git a/zeronym/smoke.sh b/zeronym/smoke.sh index 2a9690e0..a4eb90e3 100644 --- a/zeronym/smoke.sh +++ b/zeronym/smoke.sh @@ -27,6 +27,8 @@ # Environment overrides: # SMOKE_LOOKUP_MAX_SECS GetTransaction must finish within this (default 15) # SMOKE_LOOKUP_HARD_SECS curl's own ceiling on the two heavy calls (default 90) +# SMOKE_DIVERT_SETTLE_SECS how long the divert lookup retries a NOT_FOUND +# while the submit crosses the mixnet (default 10) # SMOKE_HTTP_TIMEOUT ceiling on the small JSON calls (default 20) # SMOKE_BLOCK_START first height of the GetBlockRange check (default 3444100) # SMOKE_FIXTURE path to v6_migration.bin (default: alongside this script) @@ -38,6 +40,11 @@ set -u SMOKE_LOOKUP_MAX_SECS=${SMOKE_LOOKUP_MAX_SECS:-15} SMOKE_LOOKUP_HARD_SECS=${SMOKE_LOOKUP_HARD_SECS:-90} +SMOKE_DIVERT_SETTLE_SECS=${SMOKE_DIVERT_SETTLE_SECS:-10} +# The gap between divert-lookup attempts. Not a knob: the budget above is the +# thing an operator would ever want to change, and a finer step would only add +# packets to a mixnet that is already the slow part. +DIVERT_RETRY_STEP_SECS=2 SMOKE_HTTP_TIMEOUT=${SMOKE_HTTP_TIMEOUT:-20} SMOKE_BLOCK_START=${SMOKE_BLOCK_START:-3444100} BLOCK_COUNT=50 @@ -628,16 +635,56 @@ assert len(txid) == 32, "a txid is 32 bytes" msg = b"\x1a" + bytes([len(txid)]) + txid # TxFilter.hash, field 3 sys.stdout.buffer.write(b"\x00" + len(msg).to_bytes(4, "big") + msg) PY - grpc_call GetTransaction "$_req2" "$SMOKE_LOOKUP_HARD_SECS" + # RETRY A NOT_FOUND, BOUNDED. This is about mixnet ARRIVAL ORDER, not about a + # slow hub. Over the mixnet the submit is fire-and-forget: the shim answers + # the wallet the moment the frame is handed to its Nym client, with a + # locally-computed txid, so the submit and this lookup are two independent + # Sphinx packets with no ordering guarantee between them. A single-shot lookup + # therefore measures packet luck as much as it measures the hub: on 2026-09-15 + # the hub logged the admit and the lookup miss 44 ms apart. + # + # Retrying inside a short budget removes the luck and weakens nothing. A hub + # that never queued the transaction still fails, with the same message; the + # latency assertion is still made against one call, not the sum; and this is a + # retry, not a sleep, so a run where the packets arrive in order pays nothing. + # + # NOT_FOUND ONLY. Retrying any non-zero status would spend the budget on + # failures that arrival order cannot explain and then describe them as a + # missing queue entry: UNAVAILABLE (14) is the hub being unreachable, which a + # wallet needs told immediately, and INVALID_ARGUMENT (3) can never become + # true by waiting. Only a 5 is ambiguous between "not queued" and "not queued + # YET", and only a 5 is retried. A guard refusal is also a 5, identical on the + # wire by design, so it spends the budget too; the note below names it. + _waited=0 + while :; do + grpc_call GetTransaction "$_req2" "$SMOKE_LOOKUP_HARD_SECS" + [ "$CURL_RC" = 0 ] || break + [ "$(header_value "$HDRS" grpc-status)" = 5 ] || break + [ "$_waited" -lt "$SMOKE_DIVERT_SETTLE_SECS" ] || break + # The last step is capped at what is left, so the budget is never overrun. + _step=$((SMOKE_DIVERT_SETTLE_SECS - _waited)) + [ "$_step" -le "$DIVERT_RETRY_STEP_SECS" ] || _step=$DIVERT_RETRY_STEP_SECS + sleep "$_step" + _waited=$((_waited + _step)) + done _measured="submit ${_submit_secs}s, lookup ${SECS}s, $BYTES bytes" + if [ "$_waited" != 0 ]; then + _measured="$_measured, ${_waited}s of mixnet settle" + fi if [ "$CURL_RC" != 0 ]; then fail shim:divert "lookup got no reply within ${SMOKE_LOOKUP_HARD_SECS}s: ${CURL_ERR:-curl exit $CURL_RC}" return fi if [ "$CODE" != 200 ] || grpc_status_bad "$HDRS"; then fail shim:divert "lookup: $CODE, $(grpc_status_text "$HDRS"), $_measured" - note "NOT_FOUND means the transaction is not in the hub's queue: it was never diverted there," - note "or a flush has already dropped it (it is consensus-invalid, so a flush always will)" + # The queue explanation belongs to NOT_FOUND alone. Printing it under an + # UNAVAILABLE would send a deployer to look for a flush that never happened. + if [ "$(header_value "$HDRS" grpc-status)" = 5 ]; then + note "NOT_FOUND, still, after ${_waited}s of retries: the transaction is not in the" + note "hub's queue. It was never diverted there, or a flush has already dropped it (it is" + note "consensus-invalid, so a flush always will). The shim also answers NOT_FOUND when its txid" + note "guard refuses a hub reply; its log names that case (\"does not match the query\")" + fi return fi # The reply must be the fixture BYTE FOR BYTE at height 0. Height 0 is the