diff --git a/audit-registry/src/lib.rs b/audit-registry/src/lib.rs index aa2ebc8..e190c92 100644 --- a/audit-registry/src/lib.rs +++ b/audit-registry/src/lib.rs @@ -1,5 +1,99 @@ #![no_std] -use soroban_sdk::{contract, contractimpl, symbol_short, Env, Symbol}; +//! Attestation registry. +//! +//! This crate is also the reference implementation of the workspace's +//! cross-contract authorization pattern — see +//! [`docs/cross-contract-authorization.md`](../docs/cross-contract-authorization.md). +//! `usage-meter` reads from here, and the rules below are what make that read +//! safe to build on. + +use soroban_sdk::{ + contract, contracterror, contractevent, contractimpl, contracttype, panic_with_error, Address, + BytesN, Env, IntoVal, Symbol, +}; + +/// Persistent entries are bumped to ~30 days, renewed once inside ~15 days. +/// Attestations are the audit trail, so they are persistent rather than +/// temporary; callers renew by writing, and a lapsed one can be resubmitted +/// under the same id only after it has actually been archived. +const PERSISTENT_TTL: u32 = 518_400; +const PERSISTENT_THRESHOLD: u32 = 259_200; + +#[contracterror] +#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] +#[repr(u32)] +pub enum Error { + AlreadyInitialized = 1, + NotInitialized = 2, + /// A contract tried to act on a subject's behalf without being on the + /// allowlist. This is the privilege-escalation guard. + CallerNotAllowed = 3, + AttestationExists = 4, + AttestationNotFound = 5, + AlreadySuperseded = 6, + /// The replacement attestation names a different subject than the one it + /// claims to supersede. + SubjectMismatch = 7, + SelfSupersede = 8, +} + +#[contracttype] +#[derive(Clone)] +pub enum DataKey { + Admin, + /// Presence of this key is the allowlist entry; the value is unused. + AllowedCaller(Address), + Attestation(BytesN<32>), +} + +#[contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Attestation { + pub id: BytesN<32>, + /// The account the attestation is *about* — the party that will be billed + /// against it. Authorization is anchored here, never on the submitter. + pub subject: Address, + /// Who actually submitted it. Informational: it carries no authority. + pub submitter: Address, + pub model_version: Symbol, + pub policy_ref: BytesN<32>, + pub ledger: u32, + pub superseded_by: Option>, +} + +/// A contract was admitted to the caller allowlist. +#[contractevent] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct CallerAllowed { + #[topic] + pub contract: Address, +} + +/// A contract was removed from the caller allowlist. +#[contractevent] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct CallerRevoked { + #[topic] + pub contract: Address, +} + +#[contractevent] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct AttestationRecorded { + #[topic] + pub id: BytesN<32>, + #[topic] + pub subject: Address, + pub submitter: Address, +} + +#[contractevent] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct AttestationSuperseded { + #[topic] + pub old_id: BytesN<32>, + pub new_id: BytesN<32>, +} /// Stores signed inference attestations. #[contract] @@ -7,24 +101,237 @@ pub struct AuditRegistry; #[contractimpl] impl AuditRegistry { - /// One-time initialization (scaffold — replace with auth in production). - pub fn initialize(env: Env, admin: Symbol) { - if env.storage().instance().has(&symbol_short!("admin")) { - panic!("already initialized"); + /// One-time initialization. `admin` is a real `Address` and signs for it, + /// so the deployer cannot install an admin that never consented. + pub fn initialize(env: Env, admin: Address) { + if env.storage().instance().has(&DataKey::Admin) { + panic_with_error!(&env, Error::AlreadyInitialized); } + admin.require_auth(); + env.storage().instance().set(&DataKey::Admin, &admin); + Self::bump_instance(&env); + } + + // ----------------------------------------------------------------------- + // Caller allowlist + // + // Soroban has no `msg.sender`. A callee cannot discover which contract + // invoked it, so "only usage-meter may call this" has to be built rather + // than assumed: the caller passes its own address, proves it by + // authorizing as itself, and is then checked against this list. + // ----------------------------------------------------------------------- + + /// Admit a contract to the allowlist of callers permitted to act on a + /// subject's behalf. + pub fn allow_caller(env: Env, contract: Address) { + Self::admin(&env).require_auth(); env.storage() .instance() - .set(&symbol_short!("admin"), &admin); + .set(&DataKey::AllowedCaller(contract.clone()), &()); + Self::bump_instance(&env); + CallerAllowed { contract }.publish(&env); } - /// Protocol ping — extend with domain logic. - pub fn ping(env: Env, marker: Symbol) -> Symbol { - let _ = env; - marker + /// Remove a contract from the allowlist. Takes effect immediately; already + /// written attestations are unaffected, since the authority that created + /// them was the subject's, not the caller's. + pub fn revoke_caller(env: Env, contract: Address) { + Self::admin(&env).require_auth(); + env.storage() + .instance() + .remove(&DataKey::AllowedCaller(contract.clone())); + Self::bump_instance(&env); + CallerRevoked { contract }.publish(&env); + } + + pub fn is_caller_allowed(env: Env, contract: Address) -> bool { + env.storage() + .instance() + .has(&DataKey::AllowedCaller(contract)) + } + + // ----------------------------------------------------------------------- + // Attestations + // ----------------------------------------------------------------------- + + /// Record an attestation for `subject`. + /// + /// Two independent checks, and both matter: + /// + /// 1. `subject.require_auth_for_args(...)` binds the subject's signature to + /// *these exact arguments*. Plain `require_auth()` would let any entry + /// the subject signed for this contract be replayed against different + /// arguments by an intermediate — which is the whole escalation this + /// guards against. + /// 2. When `caller` is not the subject, the caller is acting on someone + /// else's behalf, so it must both prove it is who it says and be on the + /// allowlist. `require_auth` on a contract address is satisfied + /// implicitly only for the *immediate invoker*, so a contract cannot + /// name a different, enrolled contract here and inherit its permission + /// — which is the failure mode that would make the allowlist worthless. + /// + /// Neither check subsumes the other. The allowlist alone would let an + /// approved contract forge attestations for any subject; the subject's + /// signature alone would let any contract at all relay it. + pub fn submit_attestation( + env: Env, + caller: Address, + subject: Address, + id: BytesN<32>, + model_version: Symbol, + policy_ref: BytesN<32>, + ) { + subject.require_auth_for_args( + (id.clone(), model_version.clone(), policy_ref.clone()).into_val(&env), + ); + + if caller != subject { + caller.require_auth(); + if !env + .storage() + .instance() + .has(&DataKey::AllowedCaller(caller.clone())) + { + panic_with_error!(&env, Error::CallerNotAllowed); + } + } + + let key = DataKey::Attestation(id.clone()); + if env.storage().persistent().has(&key) { + panic_with_error!(&env, Error::AttestationExists); + } + + let attestation = Attestation { + id: id.clone(), + subject: subject.clone(), + submitter: caller.clone(), + model_version, + policy_ref, + ledger: env.ledger().sequence(), + superseded_by: None, + }; + env.storage().persistent().set(&key, &attestation); + env.storage() + .persistent() + .extend_ttl(&key, PERSISTENT_THRESHOLD, PERSISTENT_TTL); + Self::bump_instance(&env); + + AttestationRecorded { + id, + subject, + submitter: caller, + } + .publish(&env); + } + + /// Mark `old_id` as replaced by `new_id`. + /// + /// Authorized by the subject of the existing attestation, not by whoever + /// submitted it — a submitter that has since been removed from the + /// allowlist must not retain the power to invalidate the record. + pub fn supersede_attestation( + env: Env, + caller: Address, + old_id: BytesN<32>, + new_id: BytesN<32>, + ) { + if old_id == new_id { + panic_with_error!(&env, Error::SelfSupersede); + } + + let mut old = Self::load(&env, &old_id); + if old.superseded_by.is_some() { + panic_with_error!(&env, Error::AlreadySuperseded); + } + + let new = Self::load(&env, &new_id); + if new.subject != old.subject { + panic_with_error!(&env, Error::SubjectMismatch); + } + + old.subject + .require_auth_for_args((old_id.clone(), new_id.clone()).into_val(&env)); + + if caller != old.subject { + caller.require_auth(); + if !env + .storage() + .instance() + .has(&DataKey::AllowedCaller(caller)) + { + panic_with_error!(&env, Error::CallerNotAllowed); + } + } + + old.superseded_by = Some(new_id.clone()); + let key = DataKey::Attestation(old_id.clone()); + env.storage().persistent().set(&key, &old); + env.storage() + .persistent() + .extend_ttl(&key, PERSISTENT_THRESHOLD, PERSISTENT_TTL); + + AttestationSuperseded { old_id, new_id }.publish(&env); + } + + // ----------------------------------------------------------------------- + // Reads + // + // Deliberately outside the allowlist. A read carries no authority: it + // returns what the ledger already exposes to anyone who can decode it, so + // gating it would buy no confidentiality and would instead couple every + // future reader to an admin transaction. `usage-meter` depends on + // `verify_attestation` being callable without being enrolled here. + // ----------------------------------------------------------------------- + + pub fn get_attestation(env: Env, id: BytesN<32>) -> Attestation { + Self::load(&env, &id) + } + + /// The predicate `usage-meter` bills against: exists, not superseded, and + /// belongs to the claimed subject. Returns `false` rather than trapping on + /// an unknown id so a caller can price unattested usage instead of having + /// its whole transaction fail. + pub fn verify_attestation(env: Env, id: BytesN<32>, subject: Address) -> bool { + match env + .storage() + .persistent() + .get::<_, Attestation>(&DataKey::Attestation(id)) + { + Some(a) => a.superseded_by.is_none() && a.subject == subject, + None => false, + } } - /// Contract ABI / deployment marker for integrators. pub fn version(_env: Env) -> u32 { - 1 + 2 + } + + // ----------------------------------------------------------------------- + + fn admin(env: &Env) -> Address { + match env.storage().instance().get(&DataKey::Admin) { + Some(a) => a, + None => panic_with_error!(env, Error::NotInitialized), + } + } + + fn load(env: &Env, id: &BytesN<32>) -> Attestation { + match env + .storage() + .persistent() + .get(&DataKey::Attestation(id.clone())) + { + Some(a) => a, + None => panic_with_error!(env, Error::AttestationNotFound), + } + } + + fn bump_instance(env: &Env) { + env.storage() + .instance() + .extend_ttl(PERSISTENT_THRESHOLD, PERSISTENT_TTL); } } + +#[cfg(test)] +mod test; diff --git a/audit-registry/src/test.rs b/audit-registry/src/test.rs new file mode 100644 index 0000000..09079fb --- /dev/null +++ b/audit-registry/src/test.rs @@ -0,0 +1,592 @@ +#![cfg(test)] +//! Authorization tests for the cross-contract pattern. +//! +//! **Nothing in this file calls `mock_all_auths`.** These tests exist to prove +//! that an unauthorized intermediate contract cannot act on a user's behalf, +//! and `mock_all_auths` approves every `require_auth` in the transaction — it +//! would make every one of them pass regardless of what the contract does. +//! Every signature here is granted deliberately, with `mock_auths`, scoped to +//! one address, one entrypoint and one exact argument list. + +use super::*; +use soroban_sdk::{ + auth::{ContractContext, InvokerContractAuthEntry, SubContractInvocation}, + testutils::{Address as _, MockAuth, MockAuthInvoke}, + vec, Env, IntoVal, +}; + +// --------------------------------------------------------------------------- +// Two stand-in intermediates, mirroring how `usage-meter` will call this +// contract — one that plays by the rules, one that does not. +// --------------------------------------------------------------------------- + +/// Behaves the way an approved intermediate is supposed to: it authorizes as +/// itself for exactly the sub-invocation it is about to make, and it passes +/// its own address as `caller` rather than pretending to be the subject. +#[contract] +pub struct HonestRelay; + +#[contractimpl] +impl HonestRelay { + pub fn relay( + env: Env, + registry: Address, + subject: Address, + id: BytesN<32>, + model_version: Symbol, + policy_ref: BytesN<32>, + ) { + let me = env.current_contract_address(); + env.authorize_as_current_contract(vec![ + &env, + InvokerContractAuthEntry::Contract(SubContractInvocation { + context: ContractContext { + contract: registry.clone(), + fn_name: Symbol::new(&env, "submit_attestation"), + args: ( + me.clone(), + subject.clone(), + id.clone(), + model_version.clone(), + policy_ref.clone(), + ) + .into_val(&env), + }, + sub_invocations: vec![&env], + }), + ]); + + AuditRegistryClient::new(&env, ®istry).submit_attestation( + &me, + &subject, + &id, + &model_version, + &policy_ref, + ); + } + + /// Passes an address that is *not* this contract as `caller` — the shape + /// an impersonation attempt takes, where an unenrolled contract borrows an + /// enrolled one's standing by simply naming it. + pub fn relay_claiming( + env: Env, + registry: Address, + claimed_caller: Address, + subject: Address, + id: BytesN<32>, + model_version: Symbol, + policy_ref: BytesN<32>, + ) { + AuditRegistryClient::new(&env, ®istry).submit_attestation( + &claimed_caller, + &subject, + &id, + &model_version, + &policy_ref, + ); + } +} + +// --------------------------------------------------------------------------- +// Fixture +// --------------------------------------------------------------------------- + +struct Fixture { + env: Env, + registry: Address, + admin: Address, +} + +fn setup() -> Fixture { + let env = Env::default(); + let registry = env.register(AuditRegistry, ()); + let admin = Address::generate(&env); + + // Scoped to this one call: the admin signs for initialize and nothing else. + env.mock_auths(&[MockAuth { + address: &admin, + invoke: &MockAuthInvoke { + contract: ®istry, + fn_name: "initialize", + args: (admin.clone(),).into_val(&env), + sub_invokes: &[], + }, + }]); + AuditRegistryClient::new(&env, ®istry).initialize(&admin); + + Fixture { + env, + registry, + admin, + } +} + +impl Fixture { + fn client(&self) -> AuditRegistryClient<'_> { + AuditRegistryClient::new(&self.env, &self.registry) + } + + fn allow(&self, contract: &Address) { + self.env.mock_auths(&[MockAuth { + address: &self.admin, + invoke: &MockAuthInvoke { + contract: &self.registry, + fn_name: "allow_caller", + args: (contract.clone(),).into_val(&self.env), + sub_invokes: &[], + }, + }]); + self.client().allow_caller(contract); + } + + /// The subject's signature over the exact arguments `submit_attestation` + /// binds it to — id, model version and policy ref, and nothing else. + fn sign_submit(&self, subject: &Address, id: &BytesN<32>, policy: &BytesN<32>) { + self.env.mock_auths(&[MockAuth { + address: subject, + invoke: &MockAuthInvoke { + contract: &self.registry, + fn_name: "submit_attestation", + args: (id.clone(), Symbol::new(&self.env, "gpt_4o"), policy.clone()) + .into_val(&self.env), + sub_invokes: &[], + }, + }]); + } +} + +/// The contract raises errors with `panic_with_error!`, so the generated +/// client surfaces them as host errors carrying the contract error code rather +/// than as the typed enum. +fn raised(e: Error) -> soroban_sdk::Error { + soroban_sdk::Error::from_contract_error(e as u32) +} + +fn bytes(env: &Env, b: u8) -> BytesN<32> { + BytesN::from_array(env, &[b; 32]) +} + +fn model(env: &Env) -> Symbol { + Symbol::new(env, "gpt_4o") +} + +// --------------------------------------------------------------------------- +// The subject's own authority +// --------------------------------------------------------------------------- + +#[test] +fn test_subject_submitting_for_itself_succeeds() { + let f = setup(); + let subject = Address::generate(&f.env); + let id = bytes(&f.env, 1); + let policy = bytes(&f.env, 9); + + f.sign_submit(&subject, &id, &policy); + f.client() + .submit_attestation(&subject, &subject, &id, &model(&f.env), &policy); + + let recorded = f.client().get_attestation(&id); + assert_eq!(recorded.subject, subject); + assert_eq!(recorded.superseded_by, None); + assert!(f.client().verify_attestation(&id, &subject)); +} + +#[test] +fn test_signature_over_different_args_is_not_reusable() { + let f = setup(); + let subject = Address::generate(&f.env); + let id = bytes(&f.env, 1); + + // Signed for policy 9 — submitted with policy 10. + f.sign_submit(&subject, &id, &bytes(&f.env, 9)); + let result = f.client().try_submit_attestation( + &subject, + &subject, + &id, + &model(&f.env), + &bytes(&f.env, 10), + ); + + // This is the reason for require_auth_for_args over plain require_auth: a + // signature is spendable only on the arguments it was given for. + assert!(result.is_err()); +} + +#[test] +fn test_a_third_party_cannot_submit_for_a_subject_that_never_signed() { + let f = setup(); + let subject = Address::generate(&f.env); + let stranger = Address::generate(&f.env); + let id = bytes(&f.env, 2); + let policy = bytes(&f.env, 9); + + // The stranger signs for itself. Nobody signs for the subject. + f.env.mock_auths(&[MockAuth { + address: &stranger, + invoke: &MockAuthInvoke { + contract: &f.registry, + fn_name: "submit_attestation", + args: ( + stranger.clone(), + subject.clone(), + id.clone(), + model(&f.env), + policy.clone(), + ) + .into_val(&f.env), + sub_invokes: &[], + }, + }]); + + let result = + f.client() + .try_submit_attestation(&stranger, &subject, &id, &model(&f.env), &policy); + assert!(result.is_err()); +} + +// --------------------------------------------------------------------------- +// The caller allowlist +// --------------------------------------------------------------------------- + +#[test] +fn test_unallowlisted_contract_is_rejected_even_with_a_valid_subject_signature() { + let f = setup(); + let relay = f.env.register(HonestRelay, ()); + let subject = Address::generate(&f.env); + let id = bytes(&f.env, 3); + let policy = bytes(&f.env, 9); + + // The subject genuinely signed, and the relay genuinely authorizes as + // itself. The only thing missing is enrolment — and that alone must stop it. + f.sign_submit(&subject, &id, &policy); + let result = HonestRelayClient::new(&f.env, &relay).try_relay( + &f.registry, + &subject, + &id, + &model(&f.env), + &policy, + ); + + assert!(result.is_err()); + assert!(!f.client().is_caller_allowed(&relay)); +} + +#[test] +fn test_allowlisted_contract_relaying_a_signed_submission_succeeds() { + let f = setup(); + let relay = f.env.register(HonestRelay, ()); + f.allow(&relay); + + let subject = Address::generate(&f.env); + let id = bytes(&f.env, 4); + let policy = bytes(&f.env, 9); + + f.sign_submit(&subject, &id, &policy); + HonestRelayClient::new(&f.env, &relay).relay( + &f.registry, + &subject, + &id, + &model(&f.env), + &policy, + ); + + let recorded = f.client().get_attestation(&id); + assert_eq!(recorded.subject, subject); + // The submitter is recorded as the relay, and carries no authority of its own. + assert_eq!(recorded.submitter, relay); +} + +#[test] +fn test_allowlisted_contract_cannot_forge_for_a_subject_that_never_signed() { + let f = setup(); + let relay = f.env.register(HonestRelay, ()); + f.allow(&relay); + + let subject = Address::generate(&f.env); + let id = bytes(&f.env, 5); + let policy = bytes(&f.env, 9); + + // Enrolment is not authority over other people. No subject signature is + // granted here, so the relay's own standing must not substitute for one. + f.env.mock_auths(&[]); + let result = HonestRelayClient::new(&f.env, &relay).try_relay( + &f.registry, + &subject, + &id, + &model(&f.env), + &policy, + ); + + assert!(result.is_err()); +} + +#[test] +fn test_a_contract_cannot_borrow_an_allowlisted_contract_s_standing() { + let f = setup(); + let enrolled = f.env.register(HonestRelay, ()); + let impostor = f.env.register(HonestRelay, ()); + f.allow(&enrolled); + assert!(!f.client().is_caller_allowed(&impostor)); + + let subject = Address::generate(&f.env); + let id = bytes(&f.env, 6); + let policy = bytes(&f.env, 9); + + // This is the escalation the allowlist would be worthless against if + // `caller` were taken at face value: an unenrolled contract passes the + // enrolled contract's address and inherits its permission. + // + // It fails because `require_auth` on a contract address is satisfied + // implicitly only for the *immediate invoker*. The impostor is the + // invoker here, so authorizing for `enrolled` is not something it can do, + // and no signature for `enrolled` exists in the transaction. + f.sign_submit(&subject, &id, &policy); + let result = HonestRelayClient::new(&f.env, &impostor).try_relay_claiming( + &f.registry, + &enrolled, + &subject, + &id, + &model(&f.env), + &policy, + ); + + assert!(result.is_err()); +} + +#[test] +fn test_revoking_a_caller_takes_effect_immediately() { + let f = setup(); + let relay = f.env.register(HonestRelay, ()); + f.allow(&relay); + assert!(f.client().is_caller_allowed(&relay)); + + f.env.mock_auths(&[MockAuth { + address: &f.admin, + invoke: &MockAuthInvoke { + contract: &f.registry, + fn_name: "revoke_caller", + args: (relay.clone(),).into_val(&f.env), + sub_invokes: &[], + }, + }]); + f.client().revoke_caller(&relay); + assert!(!f.client().is_caller_allowed(&relay)); + + let subject = Address::generate(&f.env); + let id = bytes(&f.env, 7); + let policy = bytes(&f.env, 9); + f.sign_submit(&subject, &id, &policy); + + let result = HonestRelayClient::new(&f.env, &relay).try_relay( + &f.registry, + &subject, + &id, + &model(&f.env), + &policy, + ); + assert!(result.is_err()); +} + +#[test] +fn test_allowlist_administration_requires_the_admin() { + let f = setup(); + let relay = f.env.register(HonestRelay, ()); + let impostor = Address::generate(&f.env); + + f.env.mock_auths(&[MockAuth { + address: &impostor, + invoke: &MockAuthInvoke { + contract: &f.registry, + fn_name: "allow_caller", + args: (relay.clone(),).into_val(&f.env), + sub_invokes: &[], + }, + }]); + + assert!(f.client().try_allow_caller(&relay).is_err()); + assert!(!f.client().is_caller_allowed(&relay)); +} + +// --------------------------------------------------------------------------- +// Attestation lifecycle +// --------------------------------------------------------------------------- + +#[test] +fn test_duplicate_attestation_id_is_rejected() { + let f = setup(); + let subject = Address::generate(&f.env); + let id = bytes(&f.env, 8); + let policy = bytes(&f.env, 9); + + f.sign_submit(&subject, &id, &policy); + f.client() + .submit_attestation(&subject, &subject, &id, &model(&f.env), &policy); + + f.sign_submit(&subject, &id, &policy); + let result = + f.client() + .try_submit_attestation(&subject, &subject, &id, &model(&f.env), &policy); + + assert_eq!(result, Err(Ok(raised(Error::AttestationExists)))); +} + +#[test] +fn test_verify_returns_false_for_unknown_superseded_and_mismatched_subject() { + let f = setup(); + let subject = Address::generate(&f.env); + let other = Address::generate(&f.env); + let old_id = bytes(&f.env, 20); + let new_id = bytes(&f.env, 21); + let policy = bytes(&f.env, 9); + + // Unknown id fails closed rather than trapping, so a caller can price + // unattested usage instead of losing the whole transaction. + assert!(!f.client().verify_attestation(&old_id, &subject)); + + f.sign_submit(&subject, &old_id, &policy); + f.client() + .submit_attestation(&subject, &subject, &old_id, &model(&f.env), &policy); + f.sign_submit(&subject, &new_id, &policy); + f.client() + .submit_attestation(&subject, &subject, &new_id, &model(&f.env), &policy); + + // Right id, wrong subject. + assert!(!f.client().verify_attestation(&old_id, &other)); + + f.env.mock_auths(&[MockAuth { + address: &subject, + invoke: &MockAuthInvoke { + contract: &f.registry, + fn_name: "supersede_attestation", + args: (old_id.clone(), new_id.clone()).into_val(&f.env), + sub_invokes: &[], + }, + }]); + f.client().supersede_attestation(&subject, &old_id, &new_id); + + assert!(!f.client().verify_attestation(&old_id, &subject)); + assert!(f.client().verify_attestation(&new_id, &subject)); +} + +#[test] +fn test_supersede_is_authorized_by_the_subject_not_the_original_submitter() { + let f = setup(); + let relay = f.env.register(HonestRelay, ()); + f.allow(&relay); + + let subject = Address::generate(&f.env); + let old_id = bytes(&f.env, 30); + let new_id = bytes(&f.env, 31); + let policy = bytes(&f.env, 9); + + f.sign_submit(&subject, &old_id, &policy); + HonestRelayClient::new(&f.env, &relay).relay( + &f.registry, + &subject, + &old_id, + &model(&f.env), + &policy, + ); + f.sign_submit(&subject, &new_id, &policy); + HonestRelayClient::new(&f.env, &relay).relay( + &f.registry, + &subject, + &new_id, + &model(&f.env), + &policy, + ); + + // The relay submitted both, and signs here for itself — but the record is + // the subject's, so the submitter cannot invalidate it. + f.env.mock_auths(&[MockAuth { + address: &relay, + invoke: &MockAuthInvoke { + contract: &f.registry, + fn_name: "supersede_attestation", + args: (old_id.clone(), new_id.clone()).into_val(&f.env), + sub_invokes: &[], + }, + }]); + assert!(f + .client() + .try_supersede_attestation(&relay, &old_id, &new_id) + .is_err()); + assert!(f.client().verify_attestation(&old_id, &subject)); +} + +#[test] +fn test_supersede_rejects_self_reference_and_double_supersede() { + let f = setup(); + let subject = Address::generate(&f.env); + let old_id = bytes(&f.env, 40); + let new_id = bytes(&f.env, 41); + let policy = bytes(&f.env, 9); + + f.sign_submit(&subject, &old_id, &policy); + f.client() + .submit_attestation(&subject, &subject, &old_id, &model(&f.env), &policy); + f.sign_submit(&subject, &new_id, &policy); + f.client() + .submit_attestation(&subject, &subject, &new_id, &model(&f.env), &policy); + + assert_eq!( + f.client() + .try_supersede_attestation(&subject, &old_id, &old_id), + Err(Ok(raised(Error::SelfSupersede))) + ); + + let sign_supersede = |old: &BytesN<32>, new: &BytesN<32>| { + f.env.mock_auths(&[MockAuth { + address: &subject, + invoke: &MockAuthInvoke { + contract: &f.registry, + fn_name: "supersede_attestation", + args: (old.clone(), new.clone()).into_val(&f.env), + sub_invokes: &[], + }, + }]); + }; + + sign_supersede(&old_id, &new_id); + f.client().supersede_attestation(&subject, &old_id, &new_id); + + sign_supersede(&old_id, &new_id); + assert_eq!( + f.client() + .try_supersede_attestation(&subject, &old_id, &new_id), + Err(Ok(raised(Error::AlreadySuperseded))) + ); +} + +#[test] +fn test_supersede_rejects_a_replacement_for_a_different_subject() { + let f = setup(); + let subject = Address::generate(&f.env); + let other = Address::generate(&f.env); + let old_id = bytes(&f.env, 50); + let new_id = bytes(&f.env, 51); + let policy = bytes(&f.env, 9); + + f.sign_submit(&subject, &old_id, &policy); + f.client() + .submit_attestation(&subject, &subject, &old_id, &model(&f.env), &policy); + f.sign_submit(&other, &new_id, &policy); + f.client() + .submit_attestation(&other, &other, &new_id, &model(&f.env), &policy); + + assert_eq!( + f.client() + .try_supersede_attestation(&subject, &old_id, &new_id), + Err(Ok(raised(Error::SubjectMismatch))) + ); +} + +#[test] +fn test_double_initialize_is_rejected() { + let f = setup(); + let other = Address::generate(&f.env); + assert_eq!( + f.client().try_initialize(&other), + Err(Ok(raised(Error::AlreadyInitialized))) + ); +} diff --git a/docs/cross-contract-authorization.md b/docs/cross-contract-authorization.md new file mode 100644 index 0000000..67199d3 --- /dev/null +++ b/docs/cross-contract-authorization.md @@ -0,0 +1,156 @@ +# Cross-contract authorization + +`usage-meter` will call `audit-registry`, and `payment-router` will call +`usage-meter`. This document fixes the rules for those calls before they are +wired, so that "contract A may do X on behalf of user U" is something the code +enforces rather than something the call graph implies. + +`audit-registry` is the reference implementation. New cross-contract edges +should follow the same shape. + +--- + +## The failure this prevents + +Soroban has no `msg.sender`. A contract cannot discover who invoked it, so a +callee that wants to restrict its callers has to be *told* who is calling and +then verify the claim. The tempting shortcut — trust the address passed in, or +trust that only the intended contract knows the entrypoint exists — produces a +contract that looks permission-checked and is not. + +The second, subtler failure is authority laundering. A user signs one +authorization for a transaction; an intermediate contract then spends that +authorization on a different action than the user agreed to. Plain +`require_auth()` is vulnerable to this whenever an intermediate chooses the +arguments. + +Both are privilege escalation, and neither shows up under `mock_all_auths`. + +--- + +## The pattern + +Every entrypoint reachable by another contract takes an explicit +`caller: Address` and applies three checks in this order: + +| # | Check | Guards against | +|---|---|---| +| 1 | `subject.require_auth_for_args(...)` over the arguments that determine the effect | An intermediate spending the user's signature on different arguments | +| 2 | `caller.require_auth()` when `caller != subject` | A contract naming a different contract as the caller | +| 3 | `caller` present in the admin-managed allowlist | An arbitrary contract relaying for users at all | + +None of the three is redundant: + +- Without **1**, an approved intermediate could take a signature the subject + gave for one attestation and submit a different one. +- Without **2**, any contract could pass an allowlisted contract's address and + inherit its standing. This one is load-bearing and non-obvious: a contract + address satisfies `require_auth` implicitly *only for the immediate invoker*, + which is exactly why claiming someone else's address fails. +- Without **3**, the subject's signature alone would let any contract on the + network act as a relay. + +### Why the authority is anchored on the subject + +The subject is the party that gets billed against the record. The submitter is +recorded but carries no authority: removing a contract from the allowlist must +not strip anyone of records already validly created, and a former submitter must +not retain the power to invalidate them. `supersede_attestation` is therefore +authorized by the subject of the existing attestation, never by whoever +originally submitted it. + +### Reads are deliberately not gated + +`get_attestation` and `verify_attestation` are callable by anyone. A read +carries no authority and returns what the ledger already exposes to anyone able +to decode it, so gating it would buy no confidentiality while coupling every +future reader to an admin transaction. `usage-meter` depends on +`verify_attestation` being callable without enrolment. + +The allowlist covers **writes on behalf of a subject**, and nothing else. + +--- + +## Authorization tree per flow + +### Flow 1 — user submits directly + +``` +tx root: audit-registry.submit_attestation(caller=U, subject=U, id, model, policy) +└── auth: U → require_auth_for_args([id, model, policy]) +``` + +`caller == subject`, so checks 2 and 3 do not apply. One signature, scoped to +the three arguments that define the record. + +### Flow 2 — user goes through an enrolled intermediate + +``` +tx root: usage-meter.record_usage(...) +└── sub-invocation: audit-registry.submit_attestation(caller=M, subject=U, ...) + ├── auth: U → require_auth_for_args([id, model, policy]) (signed by the user) + └── auth: M → require_auth() (implicit: M is the invoker) + └── allowlist: M enrolled by admin +``` + +The user's entry is rooted at the `audit-registry` invocation, not at +`usage-meter`'s — the user authorizes the effect, not the route to it. An +intermediate that changes any of `id`, `model` or `policy` between receiving the +call and making it invalidates the entry. + +### Flow 3 — payment-router → usage-meter + +Not yet wired. When it is, it takes the same shape: `usage-meter` gains an +allowlist, `payment-router` is enrolled in it, and the party whose funds move is +the subject whose `require_auth_for_args` is checked. + +--- + +## `require_auth` vs `require_auth_for_args` + +Use `require_auth_for_args` wherever an intermediate contract chooses or can +alter the arguments — which is every entrypoint reachable cross-contract. The +argument list passed to it should be the values that determine the effect, not +the full parameter list: including `caller` would make the subject's signature +depend on which relay was used, so re-signing would be needed for a routing +change that does not alter what the subject agreed to. + +Plain `require_auth` is enough for: + +- admin operations, where the admin is invoking directly and the identity being + checked is the sole authority for the action +- proving a `caller` is the immediate invoker, where the address itself is the + entire claim + +--- + +## Testing rules + +**Any test that is about authorization must not call `mock_all_auths`.** It +approves every `require_auth` in the transaction, so it turns every one of these +checks green regardless of what the contract does. `audit-registry/src/test.rs` +uses `mock_auths` throughout, granting one signature at a time, scoped to a +single address, entrypoint and argument list. + +The escalation attempts that must stay covered when this pattern is extended: + +1. A signature for one argument set replayed against another +2. A third party submitting for a subject that never signed +3. An unenrolled contract relaying a genuinely signed submission +4. An enrolled contract forging for a subject that never signed +5. An unenrolled contract naming an enrolled contract as `caller` +6. Revocation taking effect immediately +7. A submitter attempting to invalidate a record it does not own + +--- + +## Review record + +| Date | Reviewer | Scope | Outcome | +|---|---|---|---| +| — | *pending* | Pattern as described above, and its implementation in `audit-registry` | *open* | + +The acceptance criterion "pattern peer-reviewed and the review recorded" +explicitly requires review by someone who did not design it, so it cannot be +closed from inside this change. The table is here to be filled in by the +reviewer on this PR; please add the row rather than approving silently.