Skip to content

serviceability: narrow the sentinel exemption from IP ownership proof validation - #4253

Draft
elitegreg wants to merge 1 commit into
gm/sdk-go-ip-proof-gatefrom
gm/sentinel-ip-proof-exemption
Draft

serviceability: narrow the sentinel exemption from IP ownership proof validation#4253
elitegreg wants to merge 1 commit into
gm/sdk-go-ip-proof-gatefrom
gm/sentinel-ip-proof-exemption

Conversation

@elitegreg

Copy link
Copy Markdown
Contributor

Closes #4215. Part of RFC-27 (rfcs/rfc27-ip-verification.md, tracker #4194).

Stacked on #4252, which is stacked on #4243. Must not merge before them. The diff shown here is against gm/sdk-go-ip-proof-gate.

Decision: narrow the exemption

The issue offered three options. This takes the middle one, and the evidence for it is that the oracle's shape is specific, not that the exemption is unreachable:

crates/sentinel/src/dz_ledger_writer.rs builds one thing — owner set to the validator, UserType::Multicast, publisher: true. The blanket is_sentinel waiver covered every other shape too. A compromised sentinel key could bind an arbitrary client_ip to a unicast user, or to a multicast user it owned itself. Neither is a shape the oracle creates, and for both the sentinel operates the address, so it could obtain a real proof.

The other two options were not taken here:

  • Validator-supplied proof relayed by the oracle is the right end state, but the oracle lives in doublezero-shreds, not this repo. The program already validates a proof the sentinel supplies, so that work needs no program change and is not blocked by this PR.
  • Verify at access-pass issuance is the alternative RFC-27 already considers and rejects as a complement rather than a substitute.

Summary of Changes

  • The exemption is now a predicate, not a payer check:
    is_sentinel && effective_owner != payer && user_type == UserType::Multicast
    Both values are already in scope at the call site in create_user_core, so this needs no new accounts and no signature change beyond renaming payer_is_sentinelsentinel_exemption.
  • Validation is unchanged. The exemption waives the requirement only; a proof the sentinel does attach is still checked in full.
  • doublezero permission audit reports it. A narrowed privilege is still a privilege, so the report names the holding key and the shape it covers whenever require-ip-ownership-proof is set — and says explicitly that the exemption grants nothing when enforcement is off, so it does not cry wolf in every environment. NON_MIGRATED_SUBSYSTEMS records it as a direct sentinel_authority_pk check that does not route through authorize(), per the rule in smartcontract/programs/CLAUDE.md.
  • RFC-27 amended — the flag-set bullet, the rejection-conditions line, and the exemption section, which now states the predicate, what residual risk remains (an arbitrary IP bound to a validator-owned multicast publisher, and nothing else), and that InitGlobalState still seeds the key to the deployer.

Behavior change worth a reviewer's attention

This inverts an e2e subtest introduced in #4243. In a local devnet the manager is the sentinel authority, and doublezero user create attaches no proof, owns the user itself, and is unicast — precisely the combination the narrowing removes. sentinel_authority_is_exempt becomes sentinel_authority_is_not_exempt_for_a_unicast_self_owned_create and asserts the IpOwnershipProofRequired refusal.

No change is needed in crates/sentinel: it already passes ip_proof: None for the shape that stays exempt.

Testing Verification

  • test_sentinel_exemption_covers_the_oracle_shape — the validator-owned multicast create still succeeds with no proof, so enforcement does not break the oracle path.
  • test_sentinel_payer_is_not_exempt_for_a_unicast_create and test_sentinel_payer_is_not_exempt_when_it_owns_the_user_itself — the two shapes the narrowing excludes, each asserting IpOwnershipProofRequired specifically and that no user account was left behind. Both would have passed before this change, so they measure the narrowing rather than restating existing behavior.
  • test_sentinel_payer_still_has_a_supplied_proof_validated — a bad proof on the exempt shape is still rejected with IpProofClientIpMismatch, so the waiver cannot become a silent bypass.
  • Full user_ip_proof_test suite: 36 passed. Serviceability lib tests: 320 passed. permission::audit tests: 8 passed, including one asserting the exemption is reported only while enforced and that the rendered text (not just the JSON) says so.
  • cargo clippy clean on both crates; go vet -tags=e2e ./e2e clean.

… validation

With require-ip-ownership-proof set, any creation paid for by
globalstate.sentinel_authority_pk could omit a proof. The exemption exists
because the shred-oracle has no proof it could obtain: it provisions multicast
publishers owned by validators, and the verifier signs only the address it
observes a request originate from, so the proof would have to name the validator
for an address the service never sees a request from.

That reasoning covers one shape, but the waiver covered every shape. A
compromised sentinel key could bind an arbitrary client_ip to a unicast user, or
to a multicast user it owned itself -- neither of which the oracle ever creates,
and both of which the sentinel could obtain a real proof for, since it operates
those addresses.

The exemption is now the predicate

    is_sentinel && effective_owner != payer && user_type == UserType::Multicast

which is exactly what crates/sentinel/src/dz_ledger_writer.rs builds. Everything
else a sentinel-paid transaction can create needs a proof like any other
registrant. Validation is unchanged: a proof the sentinel does attach is still
checked in full, so the oracle can start carrying real proofs without a program
change, and fully removing the exemption stays possible without touching this
program.

A narrowed privilege is still a privilege, so `doublezero permission audit`
reports it: the holding key and the shape it covers, whenever enforcement is on.
NON_MIGRATED_SUBSYSTEMS records it as a direct sentinel_authority_pk check that
does not route through authorize().

The e2e subtest that asserted the blanket exemption is inverted -- a manager-paid
unicast create is self-owned and unicast, so it is now refused. The oracle shape
keeps its coverage in test_sentinel_exemption_covers_the_oracle_shape, alongside
two new tests for the shapes the narrowing excludes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant