diff --git a/contracts/ip_registry/src/lib.rs b/contracts/ip_registry/src/lib.rs index a4e6199a..7e97417d 100644 --- a/contracts/ip_registry/src/lib.rs +++ b/contracts/ip_registry/src/lib.rs @@ -219,6 +219,33 @@ pub const METADATA_FIELD_ACCESS_LEVEL: u32 = 1; /// This is a placeholder - should be set during contract initialization pub const NOTARY_PUBLIC_KEY: &[u8] = b"notary_public_key_placeholder"; +/// Version byte that prefixes every notary signing message (#1149). +/// The original untagged `ip_id || timestamp` format is no longer accepted. +pub const NOTARY_MESSAGE_VERSION: u8 = 1; + +/// Domain-separation tag for notary signing messages (#1149). +pub const NOTARY_DOMAIN_TAG: &[u8] = b"AtomicIP/notary"; + +/// Build the exact bytes a notary signs to notarize `ip_id` at `timestamp` +/// for the deployment at `contract` (#1149). Layout, all concatenated: +/// +/// 1. version byte (`NOTARY_MESSAGE_VERSION`, 1 byte) +/// 2. domain tag (`NOTARY_DOMAIN_TAG`) +/// 3. network id (32 bytes, `Ledger::network_id`) +/// 4. contract address (XDR encoding) +/// 5. `ip_id` (8 bytes, big-endian) +/// 6. `timestamp` (8 bytes, big-endian) +pub fn notary_signing_message(env: &Env, contract: &Address, ip_id: u64, timestamp: u64) -> Bytes { + let mut message = Bytes::new(env); + message.push_back(NOTARY_MESSAGE_VERSION); + message.append(&Bytes::from_slice(env, NOTARY_DOMAIN_TAG)); + message.append(&Bytes::from(env.ledger().network_id())); + message.append(&contract.clone().to_xdr(env)); + message.append(&Bytes::from_array(env, &ip_id.to_be_bytes())); + message.append(&Bytes::from_array(env, ×tamp.to_be_bytes())); + message +} + /// Issue #437: Number of storage shards for commitment distribution. pub const NUM_SHARDS: u32 = 16; @@ -4007,17 +4034,56 @@ impl IpRegistry { /// /// # Panics /// - /// Panics with `Unauthorized` if `access_level` is 0 or > 3, or if caller is not the owner. + /// Panics with `InvalidAccessLevel` if `access_level` is 0 or > 3 (#1150), + /// or with `Unauthorized` if caller is not the owner. pub fn grant_ip_access(env: Env, ip_id: u64, grantee: Address, access_level: u32) { + Self::grant_ip_access_with_expiry(env, ip_id, grantee, access_level, None); + } + + /// Grant access like `grant_ip_access`, with an optional expiry (#1151). + /// + /// `expires_at` is a ledger timestamp. Once `ledger.timestamp() >= expires_at` + /// the grant no longer passes `check_ip_access`. `None` means no expiry. + /// + /// # Panics + /// + /// Panics if: + /// * The IP does not exist (IpNotFound error) + /// * The caller is not the owner (Unauthorized error) + /// * `access_level` is 0 or > 3 (InvalidAccessLevel error) + /// * `grantee` is the owner (SelfGrantNotAllowed error) + /// * `expires_at` is not in the future (InvalidExpiry error) + pub fn grant_ip_access_with_expiry( + env: Env, + ip_id: u64, + grantee: Address, + access_level: u32, + expires_at: Option, + ) { let record = require_ip_exists(&env, ip_id); record.owner.require_auth(); if !(1..=3).contains(&access_level) { env.panic_with_error(Error::from_contract_error( - ContractError::Unauthorized as u32, + ContractError::InvalidAccessLevel as u32, + )); + } + + // #1151: owner already has full access; a self-grant only adds noise. + if grantee == record.owner { + env.panic_with_error(Error::from_contract_error( + ContractError::SelfGrantNotAllowed as u32, )); } + if let Some(expiry) = expires_at { + if expiry <= env.ledger().timestamp() { + env.panic_with_error(Error::from_contract_error( + ContractError::InvalidExpiry as u32, + )); + } + } + let mut grants: Vec = env .storage() .persistent() @@ -4032,6 +4098,7 @@ impl IpRegistry { IpAccessGrant { grantee: grantee.clone(), access_level, + expires_at, }, ); found = true; @@ -4042,6 +4109,7 @@ impl IpRegistry { grants.push_back(IpAccessGrant { grantee: grantee.clone(), access_level, + expires_at, }); } @@ -4113,6 +4181,7 @@ impl IpRegistry { /// a grantee with level 3 satisfies a check for level 1 or 2. /// /// Returns `true` if access is granted, `false` otherwise. + /// Expired grants (#1151) are treated as absent. pub fn check_ip_access(env: Env, ip_id: u64, grantee: Address, required_level: u32) -> bool { let record = require_ip_exists(&env, ip_id); // Owner always has full access @@ -4124,8 +4193,14 @@ impl IpRegistry { .persistent() .get(&DataKey::IpAccessGrants(ip_id)) .unwrap_or(Vec::new(&env)); + let now = env.ledger().timestamp(); for grant in grants.iter() { if grant.grantee == grantee { + if let Some(expiry) = grant.expires_at { + if now >= expiry { + return false; + } + } return grant.access_level >= required_level; } } @@ -4169,20 +4244,34 @@ impl IpRegistry { /// Notarize an IP timestamp with a notary Ed25519 signature. /// - /// The notary must sign the message `ip_id_be_bytes || timestamp_be_bytes` - /// (8 bytes each, big-endian) with the private key corresponding to the - /// stored notary public key. The 64-byte signature is verified on-chain. + /// The notary must sign the message built by `notary_signing_message` (see + /// `docs/notary-signature-format.md`) with the private key corresponding to + /// the stored notary public key. The 64-byte signature is verified on-chain. + /// The message is bound to this contract's address and network, so a + /// signature issued for one deployment is rejected by any other (#1149). + /// + /// # Panics + /// + /// Notarization is write-once: a second call for the same IP is rejected + /// rather than silently replacing the first signature (#1148). /// /// # Panics /// /// Panics if: /// * The IP does not exist (IpNotFound error) + /// * The IP is already notarized (AlreadyNotarized error) /// * The notary public key has not been set (Unauthorized error) /// * The signature is not exactly 64 bytes (Unauthorized error) /// * The Ed25519 signature verification fails (Unauthorized error) pub fn notarize_ip_timestamp(env: Env, ip_id: u64, notary_signature: Bytes) { let mut record = require_ip_exists(&env, ip_id); + if record.notary_signature.is_some() { + env.panic_with_error(Error::from_contract_error( + ContractError::AlreadyNotarized as u32, + )); + } + // Require notary public key to be configured let public_key: BytesN<32> = match env.storage().persistent().get(&DataKey::NotaryPublicKey) { @@ -4209,10 +4298,12 @@ impl IpRegistry { } }; - // Message: ip_id (8 bytes BE) || timestamp (8 bytes BE) - let mut message = Bytes::new(&env); - message.append(&Bytes::from_array(&env, &ip_id.to_be_bytes())); - message.append(&Bytes::from_array(&env, &record.timestamp.to_be_bytes())); + let message = notary_signing_message( + &env, + &env.current_contract_address(), + ip_id, + record.timestamp, + ); // Verify Ed25519 signature — panics if invalid env.crypto().ed25519_verify(&public_key, &message, &sig); diff --git a/contracts/ip_registry/src/notary_access_tests.rs b/contracts/ip_registry/src/notary_access_tests.rs new file mode 100644 index 00000000..e4793b6e --- /dev/null +++ b/contracts/ip_registry/src/notary_access_tests.rs @@ -0,0 +1,235 @@ +//! Regression tests for the notary and access-grant hardening issues: +//! - #1148: notarization is write-once +//! - #1149: notary signatures are domain-separated +//! - #1150: invalid access levels return `InvalidAccessLevel` +//! - #1151: access grants can expire and self-grants are rejected + +#[cfg(test)] +mod notary_access_tests { + use ed25519_dalek::{Signer, SigningKey}; + use soroban_sdk::{testutils::Address as _, Address, Bytes, BytesN, Env}; + + use crate::{IpRegistry, IpRegistryClient}; + + /// Fixed notary key used by every test that signs a notarization. + fn notary_key() -> SigningKey { + SigningKey::from_bytes(&[42u8; 32]) + } + + /// Register the contract, initialize it and configure the notary key. + /// Returns the contract address and the admin/owner address. + fn setup(env: &Env) -> (Address, Address) { + env.mock_all_auths(); + let contract_id = env.register(IpRegistry, ()); + let owner = Address::generate(env); + let client = IpRegistryClient::new(env, &contract_id); + client.initialize(&owner); + let public_key = BytesN::from_array(env, notary_key().verifying_key().as_bytes()); + client.set_notary_public_key(&public_key); + (contract_id, owner) + } + + /// Sign the current notarization message bound to `contract`. + fn sign_for(env: &Env, contract: &Address, ip_id: u64, timestamp: u64) -> Bytes { + let msg = crate::notary_signing_message(env, contract, ip_id, timestamp); + let mut buf = [0u8; 256]; + let len = msg.len() as usize; + msg.copy_into_slice(&mut buf[..len]); + let sig = notary_key().sign(&buf[..len]); + Bytes::from_slice(env, &sig.to_bytes()) + } + + /// Sign the pre-#1149 untagged message `ip_id || timestamp`. + fn sign_legacy(env: &Env, ip_id: u64, timestamp: u64) -> Bytes { + let mut msg = [0u8; 16]; + msg[..8].copy_from_slice(&ip_id.to_be_bytes()); + msg[8..].copy_from_slice(×tamp.to_be_bytes()); + let sig = notary_key().sign(&msg); + Bytes::from_slice(env, &sig.to_bytes()) + } + + // ── #1148: write-once notarization ─────────────────────────────────────── + + #[test] + #[should_panic(expected = "Error(Contract, #60)")] + fn notarize_twice_is_rejected() { + let env = Env::default(); + let (contract_id, owner) = setup(&env); + let client = IpRegistryClient::new(&env, &contract_id); + let ip_id = client.commit_ip(&owner, &BytesN::from_array(&env, &[7u8; 32]), &0u32); + let timestamp = client.get_ip(&ip_id).timestamp; + + let first = sign_for(&env, &contract_id, ip_id, timestamp); + client.notarize_ip_timestamp(&ip_id, &first); + + // A second, valid signature must be refused, not overwrite the first. + let second = sign_for(&env, &contract_id, ip_id, timestamp); + client.notarize_ip_timestamp(&ip_id, &second); + } + + // ── #1149: domain-separated notary signatures ──────────────────────────── + + #[test] + fn notarize_with_current_format_succeeds() { + let env = Env::default(); + let (contract_id, owner) = setup(&env); + let client = IpRegistryClient::new(&env, &contract_id); + let ip_id = client.commit_ip(&owner, &BytesN::from_array(&env, &[9u8; 32]), &0u32); + let timestamp = client.get_ip(&ip_id).timestamp; + + let sig = sign_for(&env, &contract_id, ip_id, timestamp); + client.notarize_ip_timestamp(&ip_id, &sig); + assert_eq!(client.get_ip_notary_signature(&ip_id), Some(sig)); + } + + #[test] + #[should_panic] + fn signature_from_another_deployment_is_rejected() { + let env = Env::default(); + // Two deployments share the same notary key. + let (contract_a, owner_a) = setup(&env); + let (contract_b, _owner_b) = setup(&env); + let client_a = IpRegistryClient::new(&env, &contract_a); + let ip_id = client_a.commit_ip(&owner_a, &BytesN::from_array(&env, &[10u8; 32]), &0u32); + let timestamp = client_a.get_ip(&ip_id).timestamp; + + // Signed for deployment B, replayed against deployment A. + let replayed = sign_for(&env, &contract_b, ip_id, timestamp); + client_a.notarize_ip_timestamp(&ip_id, &replayed); + } + + #[test] + #[should_panic] + fn legacy_untagged_signature_is_rejected() { + let env = Env::default(); + let (contract_id, owner) = setup(&env); + let client = IpRegistryClient::new(&env, &contract_id); + let ip_id = client.commit_ip(&owner, &BytesN::from_array(&env, &[11u8; 32]), &0u32); + let timestamp = client.get_ip(&ip_id).timestamp; + + let legacy = sign_legacy(&env, ip_id, timestamp); + client.notarize_ip_timestamp(&ip_id, &legacy); + } + + #[test] + fn first_notarization_is_kept() { + let env = Env::default(); + let (contract_id, owner) = setup(&env); + let client = IpRegistryClient::new(&env, &contract_id); + let ip_id = client.commit_ip(&owner, &BytesN::from_array(&env, &[8u8; 32]), &0u32); + let timestamp = client.get_ip(&ip_id).timestamp; + + let first = sign_for(&env, &contract_id, ip_id, timestamp); + client.notarize_ip_timestamp(&ip_id, &first); + + assert_eq!(client.get_ip_notary_signature(&ip_id), Some(first)); + } + + // ── #1150: dedicated error for invalid access levels ───────────────────── + + #[test] + #[should_panic(expected = "Error(Contract, #61)")] + fn grant_level_zero_returns_invalid_access_level() { + let env = Env::default(); + let (contract_id, owner) = setup(&env); + let client = IpRegistryClient::new(&env, &contract_id); + let ip_id = client.commit_ip(&owner, &BytesN::from_array(&env, &[12u8; 32]), &0u32); + client.grant_ip_access(&ip_id, &Address::generate(&env), &0u32); + } + + #[test] + #[should_panic(expected = "Error(Contract, #61)")] + fn grant_level_above_three_returns_invalid_access_level() { + let env = Env::default(); + let (contract_id, owner) = setup(&env); + let client = IpRegistryClient::new(&env, &contract_id); + let ip_id = client.commit_ip(&owner, &BytesN::from_array(&env, &[13u8; 32]), &0u32); + client.grant_ip_access(&ip_id, &Address::generate(&env), &4u32); + } + + #[test] + fn grant_each_valid_level_is_accepted() { + let env = Env::default(); + let (contract_id, owner) = setup(&env); + let client = IpRegistryClient::new(&env, &contract_id); + let ip_id = client.commit_ip(&owner, &BytesN::from_array(&env, &[14u8; 32]), &0u32); + let grantee = Address::generate(&env); + for level in 1u32..=3 { + client.grant_ip_access(&ip_id, &grantee, &level); + assert!(client.check_ip_access(&ip_id, &grantee, &level)); + } + } + + // ── #1151: grant expiry and self-grant guard ───────────────────────────── + + #[test] + #[should_panic(expected = "Error(Contract, #62)")] + fn owner_cannot_grant_to_self() { + let env = Env::default(); + let (contract_id, owner) = setup(&env); + let client = IpRegistryClient::new(&env, &contract_id); + let ip_id = client.commit_ip(&owner, &BytesN::from_array(&env, &[15u8; 32]), &0u32); + client.grant_ip_access(&ip_id, &owner, &1u32); + } + + #[test] + #[should_panic(expected = "Error(Contract, #11)")] + fn expiry_in_the_past_is_rejected() { + let env = Env::default(); + let (contract_id, owner) = setup(&env); + let client = IpRegistryClient::new(&env, &contract_id); + let ip_id = client.commit_ip(&owner, &BytesN::from_array(&env, &[16u8; 32]), &0u32); + env.ledger().set_timestamp(1_000); + client.grant_ip_access_with_expiry( + &ip_id, + &Address::generate(&env), + &1u32, + &Some(500u64), + ); + } + + #[test] + fn expired_grant_is_not_honoured() { + let env = Env::default(); + let (contract_id, owner) = setup(&env); + let client = IpRegistryClient::new(&env, &contract_id); + let ip_id = client.commit_ip(&owner, &BytesN::from_array(&env, &[17u8; 32]), &0u32); + let grantee = Address::generate(&env); + + env.ledger().set_timestamp(1_000); + client.grant_ip_access_with_expiry(&ip_id, &grantee, &3u32, &Some(2_000u64)); + assert!(client.check_ip_access(&ip_id, &grantee, &3u32)); + + // Boundary: at exactly `expires_at` the grant is already expired. + env.ledger().set_timestamp(2_000); + assert!(!client.check_ip_access(&ip_id, &grantee, &1u32)); + } + + #[test] + fn grant_without_expiry_never_lapses() { + let env = Env::default(); + let (contract_id, owner) = setup(&env); + let client = IpRegistryClient::new(&env, &contract_id); + let ip_id = client.commit_ip(&owner, &BytesN::from_array(&env, &[18u8; 32]), &0u32); + let grantee = Address::generate(&env); + + client.grant_ip_access(&ip_id, &grantee, &2u32); + env.ledger().set_timestamp(u32::MAX as u64 * 10); + assert!(client.check_ip_access(&ip_id, &grantee, &2u32)); + } + + #[test] + fn stored_grant_records_its_expiry() { + let env = Env::default(); + let (contract_id, owner) = setup(&env); + let client = IpRegistryClient::new(&env, &contract_id); + let ip_id = client.commit_ip(&owner, &BytesN::from_array(&env, &[19u8; 32]), &0u32); + let grantee = Address::generate(&env); + + env.ledger().set_timestamp(1_000); + client.grant_ip_access_with_expiry(&ip_id, &grantee, &1u32, &Some(5_000u64)); + let grants = client.get_ip_access_grants(&ip_id); + assert_eq!(grants.get(0).unwrap().expires_at, Some(5_000u64)); + } +} + diff --git a/contracts/ip_registry/src/test.rs b/contracts/ip_registry/src/test.rs index af4aa7c2..560d3e69 100644 --- a/contracts/ip_registry/src/test.rs +++ b/contracts/ip_registry/src/test.rs @@ -1944,11 +1944,12 @@ mod tests { client.set_notary_public_key(&public_key); let record = client.get_ip(&ip_id); - let mut msg_bytes = [0u8; 16]; - msg_bytes[..8].copy_from_slice(&ip_id.to_be_bytes()); - msg_bytes[8..].copy_from_slice(&record.timestamp.to_be_bytes()); + let msg = crate::notary_signing_message(&env, &contract_id, ip_id, record.timestamp); + let mut msg_bytes = [0u8; 256]; + let msg_len = msg.len() as usize; + msg.copy_into_slice(&mut msg_bytes[..msg_len]); - let sig = signing_key.sign(&msg_bytes); + let sig = signing_key.sign(&msg_bytes[..msg_len]); let notary_sig = soroban_sdk::Bytes::from_slice(&env, &sig.to_bytes()); client.notarize_ip_timestamp(&ip_id, ¬ary_sig); diff --git a/contracts/ip_registry/src/types.rs b/contracts/ip_registry/src/types.rs index f9904fac..e54db574 100644 --- a/contracts/ip_registry/src/types.rs +++ b/contracts/ip_registry/src/types.rs @@ -30,6 +30,9 @@ pub const ACCESS_TRANSFER: u32 = 3; // Can initiate transfer (view + verify + tr pub struct IpAccessGrant { pub grantee: Address, pub access_level: u32, // 1 = view, 2 = verify, 3 = transfer + /// Ledger timestamp at which the grant stops being valid (#1151). + /// `None` means the grant never expires. + pub expires_at: Option, } /// Cached proof path tied to the owner's current Merkle root. diff --git a/docs/notary-signature-format.md b/docs/notary-signature-format.md new file mode 100644 index 00000000..be8050af --- /dev/null +++ b/docs/notary-signature-format.md @@ -0,0 +1,57 @@ +# Notary Signature Message Format + +Status: **version 1** (current). Introduced by #1149. + +The IP Registry's `notarize_ip_timestamp` verifies an Ed25519 signature made +with the notary key configured by `set_notary_public_key`. The signed message +is built by `notary_signing_message` in `contracts/ip_registry/src/lib.rs`. + +## Why a domain tag + +The earlier format signed only `ip_id || timestamp`. Any notary key shared +between two deployments let a signature issued for one deployment be replayed +on the other. The v1 message binds each signature to one deployment (contract +address) on one network, and to a domain tag that marks it as a notary +signature. + +## Layout + +All fields are concatenated in this order: + +| # | Field | Size | Encoding | +|---|-------|------|----------| +| 1 | Version byte | 1 byte | `0x01` | +| 2 | Domain tag | 15 bytes | ASCII `AtomicIP/notary` | +| 3 | Network id | 32 bytes | `SHA-256(network passphrase)`, as returned by `Ledger::network_id` | +| 4 | Contract address | 40 bytes | XDR encoding of the `Address` (`ScAddress`) of the IP Registry deployment | +| 5 | `ip_id` | 8 bytes | big-endian `u64` | +| 6 | `timestamp` | 8 bytes | big-endian `u64`, the IP record's `timestamp` | + +The signature is a 64-byte Ed25519 signature over the full message. The +contract checks it with `ed25519_verify` against the stored notary public key. + +## Version history + +| Version | Message | Status | +|---------|---------|--------| +| 0 (implicit) | `ip_id \|\| timestamp` (16 bytes, no tag) | **Rejected** since v1 | +| 1 | As in the table above | Current | + +Version 0 signatures never verify under v1. Notaries must re-sign every IP +they want to notarize against a v1 deployment. + +## Rules for implementers + +- Build the message with the same field order and encodings. Do not reuse a + signature across deployments or networks. +- To add a new format, increment the version byte and document the new layout + here. Do not change the layout of an existing version. + +## Test coverage + +`contracts/ip_registry/src/notary_access_tests.rs` covers: + +- a v1 signature from the correct deployment is accepted; +- a v1 signature issued for a different deployment is rejected; +- a legacy (version 0) signature is rejected; +- a second notarization for the same IP is rejected (see #1148).