serviceability: narrow the sentinel exemption from IP ownership proof validation

CHANGELOG.md

@@ -9,6 +9,8 @@ All notable changes to this project will be documented in this file.
 ### Changes
 
 
+- Serviceability
+  - The RFC-27 sentinel exemption from IP ownership proof validation is narrowed to the shape the shred-oracle actually creates: a `UserType::Multicast` user whose owner is not the transaction payer. Previously any creation paid for by `globalstate.sentinel_authority_pk` could omit a proof, so a compromised sentinel key could bind an arbitrary `client_ip` to a unicast user or to a user it owned itself. Those now require a proof like any other registrant, and the sentinel can obtain one for an address it operates. `doublezero permission audit` reports the exemption and the key holding it whenever `require-ip-ownership-proof` is set, so the remaining privilege is not invisible. (#4215)
 - SDK
   - The Go serviceability `Executor.CreateUser` now emits the RFC-27 `ip_proof` Option discriminant explicitly, so its payload matches the Rust `user_create_args` fixture byte for byte instead of relying on `BorshDeserializeIncremental` to default the missing field. It also refuses to submit when an environment has `require-ip-ownership-proof` set, with a message saying why, rather than spending a transaction per user to collect `IpOwnershipProofRequired` (105): the verifier signs only the address it observes a request originate from, so the synthetic addresses the device-stress orchestrator names cannot be proven at all. Serviceability custom error codes 91-118 gained names, and the RFC-27 rejection classes (105-118) are matchable with `errors.Is` via `ClassifyProgramError`. (#4225)
   - The TypeScript and Python `GlobalState` deserializers expose `ip_verifier_authority_pk`, the RFC-27 trust root the Go SDK and the Rust state already carried, so those consumers can read which key signs IP ownership proofs. The field is appended, so an account written before the upgrade decodes it as the default pubkey rather than failing. (#4231)

e2e/ip_ownership_proof_enforcement_test.go

@@ -132,18 +132,22 @@ func TestE2E_IPOwnershipProof_Enforced(t *testing.T) {
 		requireNoUserForIP(t, dn, unverified.CYOANetworkIP)
 	})
 
-	// The sentinel exemption, which is what keeps enforcement from breaking the shred-oracle.
+	// The bound on the sentinel exemption, after #4215 narrowed it.
 	//
-	// The oracle provisions multicast publishers owned by validators, for addresses the
-	// verification service never sees a request from, so there is no proof it could obtain. The
-	// program waives the *requirement* for a creation paid for by
-	// `globalstate.sentinel_authority_pk`.
+	// The exemption exists so enforcement does not break the shred-oracle, which provisions
+	// multicast publishers owned by validators for addresses the verification service never sees a
+	// request from. It is now scoped to exactly that shape — `UserType::Multicast` with an owner
+	// other than the payer — rather than to any creation the sentinel pays for.
 	//
-	// In this devnet the manager is that authority (`smartcontract_init.go` runs
-	// `authority set --sentinel-authority me`), and `doublezero user create` never attaches a
-	// proof at all, so a manager-side create is the exemption in action. The contrast with
-	// wildcard_pass_without_a_proof is the transaction payer, which is what `is_sentinel` compares.
-	t.Run("sentinel_authority_is_exempt", func(t *testing.T) {
+	// In this devnet the manager is the sentinel authority (`smartcontract_init.go` runs
+	// `authority set --sentinel-authority me`) and `doublezero user create` attaches no proof, owns
+	// the user itself, and is unicast. Before the narrowing that combination was admitted; it is
+	// the privilege #4215 removed, so it must now be refused like any other unproven creation.
+	//
+	// The oracle shape itself is covered onchain by
+	// `test_sentinel_exemption_covers_the_oracle_shape`, which can construct a validator-owned
+	// multicast create directly; reproducing it here would mean standing up the sentinel service.
+	t.Run("sentinel_authority_is_not_exempt_for_a_unicast_self_owned_create", func(t *testing.T) {
 		// An address the manager owns a pass for. It has no container behind it; this subtest is
 		// about whether the creation is admitted, not about tunnels.
 		const sentinelUserIP = "9.0.0.9"
@@ -153,15 +157,18 @@ func TestE2E_IPOwnershipProof_Enforced(t *testing.T) {
 				sentinelUserIP + " --user-payer me"})
 		require.NoError(t, err)
 
-		log.Info("==> Creating a user as the sentinel authority, with no proof")
+		log.Info("==> Creating a unicast user as the sentinel authority, with no proof")
 		out, err := dn.Manager.Exec(t.Context(), []string{"bash", "-c",
 			"doublezero user create --device " + device.Spec.Code + " --client-ip " + sentinelUserIP + " 2>&1"})
-		log.Info("==> User create output", "output", string(out))
-		require.NoError(t, err, "the sentinel authority must be exempt from the proof requirement: %s", string(out))
+		output := string(out)
+		log.Info("==> User create output", "output", output)
+		require.Error(t, err,
+			"a sentinel-paid unicast create owns its own user, so the narrowed exemption must not cover it: %s",
+			output)
+		require.Contains(t, output, "An IP ownership proof is required to create a user",
+			"the refusal must be the program's IpOwnershipProofRequired, not an incidental failure")
 
-		users, err := dn.Manager.Exec(t.Context(), []string{"bash", "-c", "doublezero user list"})
-		require.NoError(t, err)
-		require.Contains(t, string(users), sentinelUserIP)
+		requireNoUserForIP(t, dn, sentinelUserIP)
 	})
 }
 

rfcs/rfc27-ip-verification.md

@@ -258,8 +258,8 @@ The program MUST reject when any of the following holds:
 - no verifier public key is configured in global state;
 - the proof is malformed.
 
-A proof that is *absent* is rejected only when the flag is set and the payer is not the sentinel
-authority; see Backward Compatibility.
+A proof that is *absent* is rejected only when the flag is set and the creation is not the narrow
+oracle shape the sentinel authority is exempt for; see Backward Compatibility.
 
 Each class has its own `DoubleZeroError` variant, so an operator can tell a stale proof from a
 rotated verifier key from a client that never attached the Ed25519 instruction.
@@ -331,19 +331,40 @@ set per environment through the existing `SetFeatureFlags` instruction.
   working. A creation that *does* supply one is still validated in full: a client attaching a broken
   proof is broken now, not at rollout, and letting it through would hide that until the flag flips.
 - **Flag set.** Every user creation requires a valid proof — wildcard and specific-IP passes alike,
-  and on the idempotent rerun path as well as on first creation. One exception: a creation paid for
-  by `globalstate.sentinel_authority_pk` may omit the proof.
+  and on the idempotent rerun path as well as on first creation. One narrow exception: a creation
+  paid for by `globalstate.sentinel_authority_pk` may omit the proof **when it is the shape the
+  shred-oracle produces** — a `UserType::Multicast` user whose owner is not the payer.
 
 **The sentinel exemption.** The shred-oracle provisions multicast publishers owned by validators, so
 the proof would have to name the validator for an address the verification service never sees a
 request from — there is no proof the oracle could obtain, and without the exemption setting the flag
 would break that path outright. Unlike the wildcard-pass gap this RFC closes, the exemption is not
 reachable by a registrant: it requires a DoubleZero-operated key. It waives the *requirement* only;
 a proof the sentinel does attach is still validated in full, so the oracle can start carrying real
-proofs without a program change. The residual risk is that a compromised sentinel key can bind any
-IP, and that `InitGlobalState` seeds `sentinel_authority_pk` to whoever initialized global state, so
-in a fresh environment the exemption belongs to the deployer until the key is rotated. Replacing it
-is tracked in issue #4215.
+proofs without a program change.
+
+**Scoped to the oracle shape (#4215).** The exemption is *not* a blanket waiver for anything the
+sentinel pays for. The predicate is
+
+```rust
+is_sentinel && effective_owner != payer && user_type == UserType::Multicast
+```
+
+which is exactly what `crates/sentinel/src/dz_ledger_writer.rs` builds (`owner` set to the
+validator, `UserType::Multicast`, `publisher: true`). A sentinel-paid creation of any other shape —
+a unicast user, or a multicast user the sentinel owns itself — requires a proof like any other
+registrant, and for those shapes the sentinel operates the address and so can obtain one.
+
+The residual risk that remains is confined to that shape: a compromised sentinel key can bind an
+arbitrary `client_ip` to a validator-owned multicast publisher. It can no longer bind an address to
+a unicast user or to a user it owns. Note also that `InitGlobalState` seeds `sentinel_authority_pk`
+to whoever initialized global state, so in a fresh environment even the narrowed exemption belongs
+to the deployer until the key is rotated. `doublezero permission audit` reports the exemption and
+the key that holds it.
+
+Fully removing it needs the oracle to relay a proof the validator obtained for its own address,
+which is work in `doublezero-shreds` rather than here; the program already validates a proof the
+sentinel supplies, so no program change is required when that lands.
 
 The proof is optional on the wire rather than on the instruction: `BorshDeserializeIncremental`
 decodes an older client's shorter payload as `None`, and whether `None` is acceptable is the flag's

smartcontract/cli/src/permission/audit.rs

@@ -27,6 +27,10 @@ const NON_MIGRATED_SUBSYSTEMS: &[&str] = &[
      via the USER_ADMIN legacy mapping, Permission holders directly — so only the sentinel_authority \
      direct check remains un-migrated; qa_allowlist/foundation_allowlist still bypass device status & \
      seat limits)",
+    "user create RFC-27 IP ownership proof (sentinel_authority may omit a proof for the shred-oracle \
+     shape — UserType::Multicast owned by somebody other than the payer — via a direct \
+     sentinel_authority_pk check, not through authorize(); see the IP ownership proof section above \
+     for whether it is currently active)",
 ];
 
 /// A legacy key that authorizes a migrated instruction today but lacks an equivalent
@@ -48,9 +52,22 @@ struct Gap {
 /// `true` and drop the rendered paragraph when the builder-side append is re-enabled.
 const SDK_ATTACHES_PERMISSION_ACCOUNTS: bool = false;
 
+/// The RFC-27 sentinel exemption from IP ownership proof validation, reported so a
+/// privileged path is never invisible. `None` means enforcement is off, so the
+/// exemption grants nothing today.
+#[derive(Serialize, Debug, PartialEq)]
+struct SentinelIpProofExemption {
+    /// The key the exemption belongs to (`globalstate.sentinel_authority_pk`).
+    key: String,
+    /// The creation shape it covers. Anything else needs a proof.
+    scope: String,
+}
+
 #[derive(Serialize, Debug)]
 struct AuditReport {
     strict_mode_enabled: bool,
+    require_ip_ownership_proof_enabled: bool,
+    sentinel_ip_proof_exemption: Option<SentinelIpProofExemption>,
     /// See [`SDK_ATTACHES_PERMISSION_ACCOUNTS`].
     sdk_attaches_permission_accounts: bool,
     gaps: Vec<Gap>,
@@ -70,6 +87,20 @@ fn build_report(gs: &GlobalState, permissions: &HashMap<Pubkey, Permission>) ->
     let strict_mode_enabled =
         is_feature_enabled(gs.feature_flags, FeatureFlag::RequirePermissionAccounts);
 
+    // RFC-27 (#4215). The exemption is narrowed to the shape the shred-oracle creates, but a
+    // narrowed privilege is still a privilege, so it is reported whenever it can be exercised.
+    let require_ip_ownership_proof_enabled =
+        is_feature_enabled(gs.feature_flags, FeatureFlag::RequireIpOwnershipProof);
+    let sentinel_ip_proof_exemption = (require_ip_ownership_proof_enabled
+        && gs.sentinel_authority_pk != Pubkey::default())
+    .then(|| SentinelIpProofExemption {
+        key: gs.sentinel_authority_pk.to_string(),
+        scope: "may omit an IP ownership proof for a UserType::Multicast creation whose owner is \
+                not the payer (the shred-oracle shape); every other creation it pays for needs a \
+                proof"
+            .to_string(),
+    });
+
     // Coverage gaps for every flag routed through `authorize()`. The set of gated
     // flags and the legacy→key mapping both come from the serviceability crate, so
     // this can never drift from the on-chain authorization.
@@ -133,6 +164,8 @@ fn build_report(gs: &GlobalState, permissions: &HashMap<Pubkey, Permission>) ->
 
     AuditReport {
         strict_mode_enabled,
+        require_ip_ownership_proof_enabled,
+        sentinel_ip_proof_exemption,
         sdk_attaches_permission_accounts: SDK_ATTACHES_PERMISSION_ACCOUNTS,
         gaps,
         permission_admin_holders,
@@ -269,6 +302,33 @@ fn render_text<W: Write>(out: &mut W, report: &AuditReport) -> eyre::Result<()>
         )?;
         writeln!(out)?;
     }
+    writeln!(
+        out,
+        "IP ownership proof (require-ip-ownership-proof): {}",
+        if report.require_ip_ownership_proof_enabled {
+            "ON"
+        } else {
+            "OFF"
+        }
+    )?;
+    match &report.sentinel_ip_proof_exemption {
+        Some(e) => {
+            writeln!(out, "  Sentinel exemption ACTIVE — held by {}", e.key)?;
+            writeln!(out, "  Scope: {}", e.scope)?;
+        }
+        None if report.require_ip_ownership_proof_enabled => {
+            writeln!(out, "  No sentinel authority set, so no exemption is held.")?;
+        }
+        None => {
+            writeln!(
+                out,
+                "  Enforcement is off, so no proof is required of anyone and the sentinel"
+            )?;
+            writeln!(out, "  exemption grants nothing.")?;
+        }
+    }
+    writeln!(out)?;
+
     writeln!(
         out,
         "foundation_allowlist ({} member(s)): {}",
@@ -402,6 +462,46 @@ mod tests {
         assert!(pks.contains(&feed));
     }
 
+    #[test]
+    fn test_sentinel_ip_proof_exemption_is_reported_only_while_enforced() {
+        // RFC-27 (#4215). The exemption is a privileged path, so the audit must name the key that
+        // holds it — but only when the flag makes it exercisable, or the report cries wolf in
+        // every environment that has not turned enforcement on.
+        let sentinel = Pubkey::new_unique();
+
+        // Enforcement off: nothing to report even though a sentinel is set.
+        let mut gs = globalstate_with_foundation(vec![], 0);
+        gs.sentinel_authority_pk = sentinel;
+        let report = build_report(&gs, &HashMap::new());
+        assert!(!report.require_ip_ownership_proof_enabled);
+        assert_eq!(report.sentinel_ip_proof_exemption, None);
+
+        // Enforcement on: the holder is named.
+        let mut gs =
+            globalstate_with_foundation(vec![], FeatureFlag::RequireIpOwnershipProof.to_mask());
+        gs.sentinel_authority_pk = sentinel;
+        let report = build_report(&gs, &HashMap::new());
+        assert!(report.require_ip_ownership_proof_enabled);
+        let exemption = report
+            .sentinel_ip_proof_exemption
+            .as_ref()
+            .expect("an active exemption must be reported");
+        assert_eq!(exemption.key, sentinel.to_string());
+        assert!(exemption.scope.contains("Multicast"));
+
+        // Enforcement on with no sentinel set: nobody holds it.
+        let gs =
+            globalstate_with_foundation(vec![], FeatureFlag::RequireIpOwnershipProof.to_mask());
+        let report = build_report(&gs, &HashMap::new());
+        assert_eq!(report.sentinel_ip_proof_exemption, None);
+
+        // The rendered text has to say so, not just the JSON.
+        let mut out = Vec::new();
+        render_text(&mut out, &report).unwrap();
+        let text = String::from_utf8(out).unwrap();
+        assert!(text.contains("require-ip-ownership-proof"));
+    }
+
     #[test]
     fn test_foundation_not_flagged_for_permission_admin() {
         // A foundation member with no Permission account is safe for PERMISSION_ADMIN

smartcontract/programs/doublezero-serviceability/src/ip_proof.rs

@@ -69,16 +69,22 @@ pub fn split_trailing_instructions_sysvar<'a, 'info>(
 /// enforcement has been switched on, and letting it through would mask that until the flag flips.
 /// With the flag set, a proof is required for every user creation, wildcard and specific-IP passes
 /// alike (#4192 item 3: a legacy path that skips the proof is exactly the hole the RFC exists to
-/// close) — with one exception, `payer_is_sentinel`.
+/// close) — with one narrow exception, `sentinel_exemption`.
 ///
-/// `payer_is_sentinel` waives the *requirement* only, never validation. The shred-oracle
+/// `sentinel_exemption` waives the *requirement* only, never validation. The shred-oracle
 /// provisions multicast publishers owned by validators (`crates/sentinel`), so the proof would have
 /// to name the validator for an address the oracle never observes a request from; there is no proof
 /// the oracle could obtain today. Unlike the wildcard-pass hole this RFC closes, the exemption is
 /// not reachable by a registrant: it requires `globalstate.sentinel_authority_pk`, a
-/// DoubleZero-operated key. The residual risk is that a compromised sentinel key can bind any IP,
-/// which it already could. A proof the sentinel *does* attach is still checked in full, so the
-/// oracle can start carrying real proofs without a program change (#4215).
+/// DoubleZero-operated key. A proof the sentinel *does* attach is still checked in full, so the
+/// oracle can start carrying real proofs without a program change.
+///
+/// #4215 narrowed it. The caller must decide the exemption applies only for the *shape* the oracle
+/// actually creates — a multicast publisher owned by somebody other than the payer — not for any
+/// creation the sentinel happens to pay for. A blanket exemption let a compromised sentinel key
+/// bind any `client_ip` to any user type, including a plain unicast user for itself; narrowing it
+/// leaves only the case that would otherwise break outright. See
+/// `create_user_core` for the predicate.
 ///
 /// `user_type` binds the proof to the account it authorizes: the User PDA is
 /// `f(client_ip, user_type)`, so `client_ip` alone would leave a proof reusable for a different
@@ -92,12 +98,12 @@ pub fn validate_ip_ownership_proof(
     client_ip: &Ipv4Addr,
     user_type: u8,
     current_epoch: u64,
-    payer_is_sentinel: bool,
+    sentinel_exemption: bool,
 ) -> Result<(), ProgramError> {
     let proof = match proof {
         Some(proof) => proof,
         None => {
-            return if payer_is_sentinel {
+            return if sentinel_exemption {
                 Ok(())
             } else if is_feature_enabled(
                 globalstate.feature_flags,