serviceability: narrow the sentinel exemption from IP ownership proof validation - #4253
Draft
elitegreg wants to merge 1 commit into
Draft
serviceability: narrow the sentinel exemption from IP ownership proof validation#4253elitegreg wants to merge 1 commit into
elitegreg wants to merge 1 commit into
Conversation
… 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4215. Part of RFC-27 (
rfcs/rfc27-ip-verification.md, tracker #4194).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.rsbuilds one thing —ownerset to the validator,UserType::Multicast,publisher: true. The blanketis_sentinelwaiver covered every other shape too. A compromised sentinel key could bind an arbitraryclient_ipto 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:
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.Summary of Changes
create_user_core, so this needs no new accounts and no signature change beyond renamingpayer_is_sentinel→sentinel_exemption.doublezero permission auditreports it. A narrowed privilege is still a privilege, so the report names the holding key and the shape it covers wheneverrequire-ip-ownership-proofis set — and says explicitly that the exemption grants nothing when enforcement is off, so it does not cry wolf in every environment.NON_MIGRATED_SUBSYSTEMSrecords it as a directsentinel_authority_pkcheck that does not route throughauthorize(), per the rule insmartcontract/programs/CLAUDE.md.InitGlobalStatestill 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 createattaches no proof, owns the user itself, and is unicast — precisely the combination the narrowing removes.sentinel_authority_is_exemptbecomessentinel_authority_is_not_exempt_for_a_unicast_self_owned_createand asserts theIpOwnershipProofRequiredrefusal.No change is needed in
crates/sentinel: it already passesip_proof: Nonefor 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_createandtest_sentinel_payer_is_not_exempt_when_it_owns_the_user_itself— the two shapes the narrowing excludes, each assertingIpOwnershipProofRequiredspecifically 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 withIpProofClientIpMismatch, so the waiver cannot become a silent bypass.user_ip_proof_testsuite: 36 passed. Serviceability lib tests: 320 passed.permission::audittests: 8 passed, including one asserting the exemption is reported only while enforced and that the rendered text (not just the JSON) says so.cargo clippyclean on both crates;go vet -tags=e2e ./e2eclean.