diff --git a/crates/hashi-guardian-init/src/kp_roster.rs b/crates/hashi-guardian-init/src/kp_roster.rs index f4bcaab1f0..75a62e4629 100644 --- a/crates/hashi-guardian-init/src/kp_roster.rs +++ b/crates/hashi-guardian-init/src/kp_roster.rs @@ -27,7 +27,7 @@ use hashi_types::bitcoin::BitcoinPubkey; use hashi_types::guardian::KPEncryptedShare; use hashi_types::guardian::KPEncryptedShares; use hashi_types::guardian::KPFingerprint; -use hashi_types::guardian::KpShareState; +use hashi_types::guardian::KpShareStateLogMessage; use hashi_types::guardian::PcrAllowlist; use hashi_types::guardian::SecretSharingInstance; use hashi_types::guardian::SecretSharingParams; @@ -162,7 +162,7 @@ impl VerifiedCeremonyState { ceremony_session_id: String, kp_share_session_id: String, secret_sharing_instance: SecretSharingInstance, - kp_share_state: KpShareState, + kp_share_state: KpShareStateLogMessage, btc_master_pubkey: BitcoinPubkey, expected_n: usize, expected_t: usize, @@ -500,7 +500,7 @@ mod tests { "ceremony-session".into(), "kp-share-session".into(), resp.secret_sharing_instance, - KpShareState::new(1, 0, resp.encrypted_shares), + KpShareStateLogMessage::new(1, 0, resp.encrypted_shares), resp.btc_master_pubkey, 3, 2, diff --git a/crates/hashi-guardian-init/src/operator_ceremony.rs b/crates/hashi-guardian-init/src/operator_ceremony.rs index d7e722b4f3..1a81218ce0 100644 --- a/crates/hashi-guardian-init/src/operator_ceremony.rs +++ b/crates/hashi-guardian-init/src/operator_ceremony.rs @@ -20,7 +20,7 @@ use anyhow::ensure; use hashi_guardian::s3_reader::BuildPolicy; use hashi_guardian::s3_reader::GuardianReader; use hashi_types::guardian::GuardianSigned; -use hashi_types::guardian::KpShareState; +use hashi_types::guardian::KpShareStateLogMessage; use hashi_types::guardian::OperatorInitRequest; use hashi_types::guardian::SetupNewKeyRequest; use hashi_types::guardian::SetupNewKeyResponse; @@ -230,7 +230,7 @@ pub async fn run(cfg: Config) -> Result<()> { kp_share_session_id = %live.kp_share_session_id, sharing_seq = live.secret_sharing_instance.sharing_seq(), cert_seq = live.kp_share_cert_seq, - kp_shares_key = %KpShareState::object_key( + kp_shares_key = %KpShareStateLogMessage::object_key( &live.kp_share_session_id, live.secret_sharing_instance.sharing_seq(), live.kp_share_cert_seq diff --git a/crates/hashi-guardian-proxy/src/widlog.rs b/crates/hashi-guardian-proxy/src/widlog.rs index 699b4f512e..f0ba6547a1 100644 --- a/crates/hashi-guardian-proxy/src/widlog.rs +++ b/crates/hashi-guardian-proxy/src/widlog.rs @@ -322,7 +322,7 @@ pub(crate) mod test_store { request_data.seq = seq; let signing_key = GuardianSignKeyPair::from([9u8; 32]); - let mut record = LogRecord::new( + let record = LogRecord::new_at_timestamp( "test-session".to_string(), LogMessage::Withdrawal(Box::new(WithdrawalLogMessage::Success { txid: bitcoin::Txid::from_slice(&[3u8; 32]).unwrap(), @@ -336,9 +336,9 @@ pub(crate) mod test_store { }, })), &signing_key, + timestamp_ms, ); - record.timestamp_ms = timestamp_ms; - let key = record.object_key(); + let key = record.object_key().to_string(); (key, serde_json::to_vec(&record).unwrap()) } diff --git a/crates/hashi-guardian/README.md b/crates/hashi-guardian/README.md index 8a2baca04b..e9a9dc9b21 100644 --- a/crates/hashi-guardian/README.md +++ b/crates/hashi-guardian/README.md @@ -2,6 +2,12 @@ Guardian enclave service that emits immutable S3 logs for audit/state-restart workflows. +The S3 bucket operator is untrusted. Log signatures bind the intent, schema +version, session ID, timestamp, actual object key, and event; readers reject +relocated or non-canonical records. The key is S3 metadata, not JSON. Random +failure suffixes are sampled once before signing. Unsigned OI attestations bind +placement through their Nitro-authenticated signing key and derived session ID. + ## S3 log key format Canonical key layout: diff --git a/crates/hashi-guardian/src/enclave.rs b/crates/hashi-guardian/src/enclave.rs index b11700d801..24d4dbe8b6 100644 --- a/crates/hashi-guardian/src/enclave.rs +++ b/crates/hashi-guardian/src/enclave.rs @@ -575,11 +575,9 @@ impl Enclave { cert_seq: u64, encrypted_shares: KPEncryptedShares, ) -> GuardianResult<()> { - self.write_log(LogMessage::KpShareState(Box::new(KpShareState::new( - sharing_seq, - cert_seq, - encrypted_shares, - )))) + self.write_log(LogMessage::KpShareState(Box::new( + KpShareStateLogMessage::new(sharing_seq, cert_seq, encrypted_shares), + ))) .await } diff --git a/crates/hashi-guardian/src/s3_client.rs b/crates/hashi-guardian/src/s3_client.rs index aaaca2d508..09e296655c 100644 --- a/crates/hashi-guardian/src/s3_client.rs +++ b/crates/hashi-guardian/src/s3_client.rs @@ -105,8 +105,8 @@ impl GuardianS3Client { // ======================================================================== pub async fn write_log_record(&self, log: LogRecord) -> GuardianResult<()> { + let key = log.object_key().to_string(); let object_lock_duration = log.object_lock_duration(); - let key = log.object_key(); self.write_at_key(&key, &log, object_lock_duration).await } @@ -275,7 +275,7 @@ impl GuardianS3Client { /// tampering / lock-expiry beyond what the immutable-log model allows). /// Returned keys are unique and sorted lexicographically. /// - /// Keys-only counterpart to [`Self::list_all_objects_in_dir`]. + /// Keys-only counterpart to [`Self::list_all_log_records_in_dir`]. pub async fn list_all_keys_in_dir(&self, prefix: &str) -> GuardianResult> { self.ensure_no_duplicates_or_deletions(prefix).await } @@ -408,17 +408,18 @@ impl GuardianS3Client { /// Batch read. Callers must ensure that all objects with prefix `dir.to_string()` have /// unexpired compliance-mode object locks. /// - /// Returns: List of objects. - pub async fn list_all_objects_in_dir( + /// Each returned record carries the actual S3 key from which it was read. + pub async fn list_all_log_records_in_dir( &self, dir: &S3HourScopedDirectory, - ) -> GuardianResult> { + ) -> GuardianResult> { let prefix = dir.to_string(); let keys = self.ensure_no_duplicates_or_deletions(&prefix).await?; let mut out = Vec::with_capacity(keys.len()); - for key in &keys { - let obj: T = self.get_object_unsafe(key).await?; - out.push(obj); + for key in keys { + let mut record: LogRecord = self.get_object_unsafe(&key).await?; + record.object_key = key; + out.push(record); } Ok(out) } @@ -473,7 +474,7 @@ impl GuardianS3Client { /// signature rather than S3 immutability (e.g. `kp-shares/`, which carry only a /// short lock that is expected to expire). A tampered object fails the /// caller's signature check; a purged one surfaces as a get error. - pub async fn get_object_no_lock(&self, key: &str) -> GuardianResult { + pub async fn get_log_record_no_lock(&self, key: &str) -> GuardianResult { let response = self .client .get_object() @@ -497,12 +498,14 @@ impl GuardianS3Client { )) })?; - serde_json::from_slice::(&bytes.into_bytes()).map_err(|e| { + let mut record = serde_json::from_slice::(&bytes.into_bytes()).map_err(|e| { S3Error(format!( "Failed to deserialize object {} into target type: {}", key, e )) - }) + })?; + record.object_key = key.to_owned(); + Ok(record) } /// Reads an immutable-log object, asserting its Compliance lock metadata is @@ -519,7 +522,9 @@ impl GuardianS3Client { ))); } - self.get_object_unsafe::(key).await + let mut record = self.get_object_unsafe::(key).await?; + record.object_key = key.to_owned(); + Ok(record) } /// Note: Callers must set signing_pubkey to None only for unsigned messages. @@ -533,7 +538,7 @@ impl GuardianS3Client { if log.session_id != expected_session_id { return Err(S3Error(format!("log session_id mismatch for key {}", key))); } - let (_, _, message) = match signing_pubkey { + let (_, _, _, message) = match signing_pubkey { Some(pk) => log.verify(pk), None => log.verify_unsigned(), }?; diff --git a/crates/hashi-guardian/src/s3_reader.rs b/crates/hashi-guardian/src/s3_reader.rs index 27f81c45ac..4d9782444d 100644 --- a/crates/hashi-guardian/src/s3_reader.rs +++ b/crates/hashi-guardian/src/s3_reader.rs @@ -22,7 +22,7 @@ use hashi_types::guardian::CommitteeUpdateLogMessage; use hashi_types::guardian::GenesisLogMessage; use hashi_types::guardian::GuardianInfo; use hashi_types::guardian::GuardianResult; -use hashi_types::guardian::KpShareState; +use hashi_types::guardian::KpShareStateLogMessage; use hashi_types::guardian::LogMessage; use hashi_types::guardian::LogRecord; use hashi_types::guardian::PcrAllowlist; @@ -126,7 +126,7 @@ impl GuardianReader { ) -> anyhow::Result> { let all_logs = self .s3 - .list_all_objects_in_dir::(dir) + .list_all_log_records_in_dir(dir) .await .with_context(|| format!("failed to list guardian logs in {dir}"))?; @@ -227,13 +227,13 @@ impl GuardianReader { &mut self, sharing_seq: u64, build_policy: BuildPolicy, - ) -> anyhow::Result> { - let prefix = KpShareState::object_prefix(sharing_seq); + ) -> anyhow::Result> { + let prefix = KpShareStateLogMessage::object_prefix(sharing_seq); let keys = self.s3.list_all_keys_in_dir(&prefix).await?; let Some(key) = keys.into_iter().max() else { return Ok(None); }; - let record: LogRecord = self.s3.get_object_no_lock(&key).await?; + let record = self.s3.get_log_record_no_lock(&key).await?; let record = self.cache.verify_record(&self.s3, record).await?; let session_id = record.session_id.clone(); let build_pcrs = record.build_pcrs.clone(); @@ -241,10 +241,6 @@ impl GuardianReader { let LogMessage::KpShareState(msg) = record.message else { anyhow::bail!("expected a kp-shares log at {key}"); }; - let expected_key = KpShareState::object_key(&session_id, msg.sharing_seq, msg.cert_seq); - if key != expected_key { - anyhow::bail!("kp-shares key mismatch: expected {expected_key}, got {key}"); - } if msg.sharing_seq != sharing_seq { anyhow::bail!( "sharing_seq mismatch: {} != {}", @@ -364,13 +360,16 @@ impl GuardianSessionCache { s3: &GuardianS3Client, record: LogRecord, ) -> anyhow::Result { + let object_key = record.object_key.clone(); let session_info = self .get_or_load_session_info(s3, &record.session_id) .await?; record .verify(&session_info.signing_pubkey) - .map(|(session_id, timestamp_ms, message)| { + .map(|(schema_version, session_id, timestamp_ms, message)| { VerifiedLogRecord::new( + schema_version, + object_key, session_id, timestamp_ms, message, diff --git a/crates/hashi-guardian/src/s3_reader/heartbeat_checks.rs b/crates/hashi-guardian/src/s3_reader/heartbeat_checks.rs index a089d43216..300908fe43 100644 --- a/crates/hashi-guardian/src/s3_reader/heartbeat_checks.rs +++ b/crates/hashi-guardian/src/s3_reader/heartbeat_checks.rs @@ -159,6 +159,8 @@ mod tests { fn heartbeat_log(session_id: &str, timestamp_secs: UnixSeconds) -> VerifiedLogRecord { VerifiedLogRecord { + schema_version: hashi_types::guardian::LOG_SCHEMA_VERSION, + object_key: format!("heartbeat/{session_id}.json"), session_id: session_id.to_string(), timestamp_ms: timestamp_secs * 1_000, message: LogMessage::Heartbeat { seq: 0 }, @@ -168,6 +170,8 @@ mod tests { fn non_heartbeat_log() -> VerifiedLogRecord { VerifiedLogRecord { + schema_version: hashi_types::guardian::LOG_SCHEMA_VERSION, + object_key: "init/test-session-pi-enclave-fully-initialized.json".to_string(), session_id: "test-session".to_string(), timestamp_ms: 0, message: LogMessage::Init(Box::new(InitLogMessage::PIEnclaveFullyInitialized { diff --git a/crates/hashi-guardian/src/s3_reader/limiter_recovery.rs b/crates/hashi-guardian/src/s3_reader/limiter_recovery.rs index bbf34a5293..af70d12dc3 100644 --- a/crates/hashi-guardian/src/s3_reader/limiter_recovery.rs +++ b/crates/hashi-guardian/src/s3_reader/limiter_recovery.rs @@ -186,6 +186,8 @@ mod tests { post_state: state_with_seq(next_seq), }; VerifiedLogRecord { + schema_version: hashi_types::guardian::LOG_SCHEMA_VERSION, + object_key: format!("withdraw/success-{next_seq}.json"), session_id: "test-session".to_string(), timestamp_ms: 0, message: LogMessage::Withdrawal(Box::new(msg)), @@ -202,6 +204,8 @@ mod tests { error: GuardianError::RateLimitExceeded, }; VerifiedLogRecord { + schema_version: hashi_types::guardian::LOG_SCHEMA_VERSION, + object_key: "withdraw/failure.json".to_string(), session_id: "test-session".to_string(), timestamp_ms: 0, message: LogMessage::Withdrawal(Box::new(msg)), diff --git a/crates/hashi-types/src/guardian/log/envelope.rs b/crates/hashi-types/src/guardian/log/envelope.rs index 5d5b53d4c7..2e55e74fb9 100644 --- a/crates/hashi-types/src/guardian/log/envelope.rs +++ b/crates/hashi-types/src/guardian/log/envelope.rs @@ -2,8 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 //! The `LogRecord` envelope written to S3: it wraps a `super::message::LogMessage` -//! with the session id, timestamp, and (for signed logs) the guardian signature, -//! and derives the object key + lock duration from the wrapped message. +//! with the session id, timestamp, and (for signed logs) the guardian signature. +//! The object key and lock duration are derived from the wrapped message. use super::S3_OBJECT_LOCK_DURATION_CEREMONY; use super::S3_OBJECT_LOCK_DURATION_COMMITTEE_UPDATE; @@ -13,6 +13,7 @@ use super::S3_OBJECT_LOCK_DURATION_INIT; use super::S3_OBJECT_LOCK_DURATION_KP_SHARES; use super::S3_OBJECT_LOCK_DURATION_WITHDRAW; use super::message::LogMessage; +use super::message::ObjectKeyPattern; use crate::guardian::BuildPcrs; use crate::guardian::GuardianError::InvalidInputs; use crate::guardian::GuardianPubKey; @@ -20,16 +21,28 @@ use crate::guardian::GuardianResult; use crate::guardian::GuardianSignKeyPair; use crate::guardian::GuardianSignature; use crate::guardian::GuardianSigned; +use crate::guardian::IntentType; use crate::guardian::SessionID; +use crate::guardian::SigningIntent; use crate::guardian::UnixMillis; use crate::guardian::now_timestamp_ms; +use crate::guardian::session_id_from_signing_pubkey; use serde::Deserialize; use serde::Serialize; use std::time::Duration; -/// Canonical log record written to S3. +pub const LOG_SCHEMA_VERSION: u8 = 1; + +/// Write: `LogMessage` -> signed `LogRecord` with a finalized key -> JSON body. +/// Read: S3 key + JSON body -> untrusted `LogRecord` -> `VerifiedLogRecord`. #[derive(Serialize, Deserialize, Debug)] pub struct LogRecord { + pub schema_version: u8, + /// Final S3 destination selected before signing. Readers must compare this + /// in-memory key with the actual key returned by S3. S3 already carries the + /// key as object metadata, so it is not duplicated in the JSON body. + #[serde(skip)] + pub object_key: String, pub session_id: SessionID, pub timestamp_ms: UnixMillis, pub message: LogMessage, @@ -37,10 +50,36 @@ pub struct LogRecord { pub signature: Option, } +#[derive(Serialize)] +pub(crate) struct LogSigningPayload { + schema_version: u8, + session_id: SessionID, + object_key: String, + message: LogMessage, +} + +impl SigningIntent for LogSigningPayload { + const INTENT: IntentType = IntentType::LogMessage; + + fn signing_bytes(&self, timestamp_ms: UnixMillis) -> Vec { + bcs::to_bytes(&( + Self::INTENT, + self.schema_version, + &self.session_id, + timestamp_ms, + &self.object_key, + &self.message, + )) + .expect("log signing payload serialization should not fail") + } +} + /// A log record whose message signature and writing session's attestation/PCRs /// have both been verified. #[derive(Debug)] pub struct VerifiedLogRecord { + pub schema_version: u8, + pub object_key: String, pub session_id: SessionID, pub timestamp_ms: UnixMillis, pub message: LogMessage, @@ -49,12 +88,16 @@ pub struct VerifiedLogRecord { impl VerifiedLogRecord { pub fn new( + schema_version: u8, + object_key: String, session_id: SessionID, timestamp_ms: UnixMillis, message: LogMessage, build_pcrs: BuildPcrs, ) -> Self { Self { + schema_version, + object_key, session_id, timestamp_ms, message, @@ -69,20 +112,27 @@ impl LogRecord { message: LogMessage, signing_key: &GuardianSignKeyPair, ) -> Self { - let timestamp_ms = now_timestamp_ms(); + Self::new_at_timestamp(session_id, message, signing_key, now_timestamp_ms()) + } + + pub fn new_at_timestamp( + session_id: SessionID, + message: LogMessage, + signing_key: &GuardianSignKeyPair, + timestamp_ms: UnixMillis, + ) -> Self { + let object_key = message + .object_key_pattern(&session_id, timestamp_ms) + .finalize(); if message.is_allowed_unsigned() { - Self::unsigned(session_id, message, timestamp_ms) + Self::unsigned(session_id, message, timestamp_ms, object_key) } else { - Self::signed(session_id, message, signing_key, timestamp_ms) + Self::signed(session_id, message, signing_key, timestamp_ms, object_key) } } - /// Full S3 object key for this log record (directory + file name). See the - /// `hashi-guardian` README for the canonical per-log-type key layout. - pub fn object_key(&self) -> String { - let dir = self.message.log_dir(self.timestamp_ms); - let log_name = self.message.log_name(&self.session_id); - format!("{}{}", dir, log_name) + pub fn object_key(&self) -> &str { + &self.object_key } pub fn object_lock_duration(&self) -> Duration { @@ -102,22 +152,41 @@ impl LogRecord { message: LogMessage, signing_key: &GuardianSignKeyPair, timestamp_ms: UnixMillis, + object_key: String, ) -> Self { - let signed = GuardianSigned::new(message, signing_key, timestamp_ms); + let signed = GuardianSigned::new( + LogSigningPayload { + schema_version: LOG_SCHEMA_VERSION, + session_id, + object_key, + message, + }, + signing_key, + timestamp_ms, + ); Self { - session_id, + schema_version: signed.data.schema_version, + object_key: signed.data.object_key, + session_id: signed.data.session_id, timestamp_ms: signed.timestamp_ms, - message: signed.data, + message: signed.data.message, signature: Some(signed.signature), } } - fn unsigned(session_id: SessionID, message: LogMessage, timestamp_ms: UnixMillis) -> Self { + fn unsigned( + session_id: SessionID, + message: LogMessage, + timestamp_ms: UnixMillis, + object_key: String, + ) -> Self { assert!( message.is_allowed_unsigned(), "message must be Init(OIAttestationUnsigned)" ); Self { + schema_version: LOG_SCHEMA_VERSION, + object_key, session_id, timestamp_ms, message, @@ -128,7 +197,10 @@ impl LogRecord { pub fn verify( self, pub_key: &GuardianPubKey, - ) -> GuardianResult<(SessionID, UnixMillis, LogMessage)> { + ) -> GuardianResult<(u8, SessionID, UnixMillis, LogMessage)> { + self.validate_object_key()?; + let object_key = self.object_key.clone(); + let schema_version = self.schema_version; let session_id = self.session_id; let timestamp_ms = self.timestamp_ms; let message = self.message; @@ -140,30 +212,86 @@ impl LogRecord { .signature .ok_or_else(|| InvalidInputs("missing log signature".into()))?; GuardianSigned { - data: message, + data: LogSigningPayload { + schema_version: self.schema_version, + session_id: session_id.clone(), + object_key, + message, + }, timestamp_ms, signature, } .verify(pub_key)? + .message }; - Ok((session_id, timestamp_ms, message)) + Ok((schema_version, session_id, timestamp_ms, message)) } - pub fn verify_unsigned(self) -> GuardianResult<(SessionID, UnixMillis, LogMessage)> { + pub fn verify_unsigned(self) -> GuardianResult<(u8, SessionID, UnixMillis, LogMessage)> { if !self.message.is_allowed_unsigned() { return Err(InvalidInputs( "expected unsigned log record but message requires a signature".into(), )); } - Ok((self.session_id, self.timestamp_ms, self.message)) + self.validate_object_key()?; + Ok(( + self.schema_version, + self.session_id, + self.timestamp_ms, + self.message, + )) + } + + fn validate_object_key(&self) -> GuardianResult<()> { + if self.schema_version != LOG_SCHEMA_VERSION { + return Err(InvalidInputs(format!( + "unsupported log schema version: {}", + self.schema_version + ))); + } + match self + .message + .object_key_pattern(&self.session_id, self.timestamp_ms) + { + ObjectKeyPattern::Fixed(expected) if self.object_key != expected => { + return Err(InvalidInputs(format!( + "non-canonical S3 object key: got {}, expected {expected}", + self.object_key + ))); + } + ObjectKeyPattern::RandomSuffix(prefix) if !self.object_key.starts_with(&prefix) => { + return Err(InvalidInputs(format!( + "non-canonical S3 object key: got {}, expected prefix {prefix}", + self.object_key + ))); + } + _ => {} + } + if let LogMessage::Init(init) = &self.message + && let super::message::InitLogMessage::OIAttestationUnsigned { + signing_public_key, .. + } = init.as_ref() + { + let attested_session_id = session_id_from_signing_pubkey(signing_public_key); + if self.session_id != attested_session_id { + return Err(InvalidInputs(format!( + "attestation session ID mismatch: record contains {}, signing public key derives {attested_session_id}", + self.session_id + ))); + } + } + Ok(()) } } #[cfg(test)] mod tests { use super::*; + use crate::guardian::CommitteeUpdateLogMessage; use crate::guardian::GenesisLogMessage; + use crate::guardian::GuardianError; + use crate::guardian::GuardianSigned; use crate::guardian::InitLogMessage; use crate::guardian::KPEncryptedShares; use crate::guardian::LimiterState; @@ -177,23 +305,291 @@ mod tests { use bitcoin::Txid; use bitcoin::hashes::Hash as _; - fn set_timestamp(log: &mut LogRecord, timestamp_ms: UnixMillis) { - log.timestamp_ms = timestamp_ms; + fn signed_heartbeat(timestamp_ms: UnixMillis) -> (String, LogRecord, GuardianSignKeyPair) { + let signing_key = GuardianSignKeyPair::from([13u8; 32]); + let record = LogRecord::new_at_timestamp( + "session-key-binding".to_string(), + LogMessage::Heartbeat { seq: 42 }, + &signing_key, + timestamp_ms, + ); + let object_key = record.object_key().to_string(); + (object_key, record, signing_key) + } + + fn assert_writer_key_is_stable_and_verifies(log: LogRecord, signing_key: &GuardianSignKeyPair) { + let writer_key = log.object_key().to_string(); + for _ in 0..4 { + assert_eq!( + log.object_key(), + writer_key, + "a record must keep the same object key after construction" + ); + } + + let body = serde_json::to_vec(&log).unwrap(); + let mut record_read_from_s3: LogRecord = serde_json::from_slice(&body).unwrap(); + record_read_from_s3.object_key = writer_key; + record_read_from_s3 + .verify(&signing_key.verification_key()) + .expect("the serialized record must verify at the key used by the writer"); + } + + fn assert_heartbeat_relocation_rejected(relocated_key: &str) { + let (_, mut log, signing_key) = signed_heartbeat(1_700_000_000_000); + log.object_key = relocated_key.to_owned(); + let err = log + .verify(&signing_key.verification_key()) + .expect_err("relocated record must be rejected"); + assert!(format!("{err:?}").contains("non-canonical S3 object key")); + } + + #[test] + fn withdrawal_failure_writer_key_is_stable_and_verifies() { + let signing_key = GuardianSignKeyPair::from([16u8; 32]); + let signed_request = StandardWithdrawalRequest::mock_signed_for_testing(Network::Regtest); + let (request_sign, request_data) = signed_request.into_parts(); + let log = LogRecord::new( + "session-withdraw-failure".to_string(), + LogMessage::Withdrawal(Box::new(WithdrawalLogMessage::Failure { + request_data: request_data.into(), + request_sign, + error: GuardianError::RateLimitExceeded, + })), + &signing_key, + ); + + assert_writer_key_is_stable_and_verifies(log, &signing_key); + } + + #[test] + fn committee_update_failure_writer_key_is_stable_and_verifies() { + let signing_key = GuardianSignKeyPair::from([17u8; 32]); + let signed_request = StandardWithdrawalRequest::mock_signed_for_testing(Network::Regtest); + let (request_sign, _) = signed_request.into_parts(); + let log = LogRecord::new( + "session-committee-failure".to_string(), + LogMessage::CommitteeUpdate(Box::new(CommitteeUpdateLogMessage::Failure { + from_epoch: 6, + new_committee: crate::move_types::Committee { + epoch: 7, + members: vec![], + total_weight: 0, + config: crate::move_types::Config::default(), + }, + request_sign, + error: GuardianError::InvalidInputs("test failure".to_string()), + })), + &signing_key, + ); + + assert_writer_key_is_stable_and_verifies(log, &signing_key); + } + + #[test] + fn signed_log_verifies_at_canonical_object_key() { + let (_, log, signing_key) = signed_heartbeat(1_700_000_000_000); + + let (schema_version, _, timestamp_ms, message) = log + .verify(&signing_key.verification_key()) + .expect("record should verify at its intended S3 key"); + + assert_eq!(schema_version, LOG_SCHEMA_VERSION); + assert_eq!(timestamp_ms, 1_700_000_000_000); + assert!(matches!(message, LogMessage::Heartbeat { seq: 42 })); + } + + #[test] + fn object_key_is_supplied_by_s3_not_duplicated_in_json() { + let (object_key, log, signing_key) = signed_heartbeat(1_700_000_000_000); + let json = serde_json::to_value(&log).unwrap(); + assert!(json.get("object_key").is_none()); + + let mut from_s3: LogRecord = serde_json::from_value(json).unwrap(); + assert!(from_s3.object_key().is_empty()); + from_s3.object_key = object_key; + from_s3 + .verify(&signing_key.verification_key()) + .expect("actual S3 key should restore the signed routing context"); + } + + #[test] + fn signed_log_rejects_cross_prefix_relocation() { + assert_heartbeat_relocation_rejected( + "withdraw/2023/11/14/22/session-key-binding-00000000000000000042.json", + ); + } + + #[test] + fn signed_log_rejects_lexicographically_higher_key_relocation() { + assert_heartbeat_relocation_rejected( + "heartbeat/2023/11/14/22/session-key-binding-00000000000000000043.json", + ); + } + + #[test] + fn signed_log_rejects_future_hour_relocation() { + assert_heartbeat_relocation_rejected( + "heartbeat/2023/11/14/23/session-key-binding-00000000000000000042.json", + ); + } + + #[test] + fn signed_log_rejects_changed_session_relocation() { + assert_heartbeat_relocation_rejected( + "heartbeat/2023/11/14/22/aliased-session-00000000000000000042.json", + ); + } + + #[test] + fn signed_log_rejects_tampered_key_derivation_fields() { + let (_, log, signing_key) = signed_heartbeat(1_700_000_000_000); + let mut tampered: LogRecord = + serde_json::from_slice(&serde_json::to_vec(&log).unwrap()).unwrap(); + tampered.message = LogMessage::Heartbeat { seq: 43 }; + let tampered_key = "heartbeat/2023/11/14/22/session-key-binding-00000000000000000043.json"; + tampered.object_key = tampered_key.to_owned(); + + let err = tampered + .verify(&signing_key.verification_key()) + .expect_err("signature must cover the canonical object key and message"); + + assert!(format!("{err:?}").contains("signature invalid")); + } + + #[test] + fn signed_log_rejects_changed_failure_nonce_relocation() { + let signing_key = GuardianSignKeyPair::from([18u8; 32]); + let signed_request = StandardWithdrawalRequest::mock_signed_for_testing(Network::Regtest); + let (request_sign, request_data) = signed_request.into_parts(); + let log = LogRecord::new( + "session-failure-nonce".to_string(), + LogMessage::Withdrawal(Box::new(WithdrawalLogMessage::Failure { + request_data: request_data.into(), + request_sign, + error: GuardianError::RateLimitExceeded, + })), + &signing_key, + ); + let original_key = log.object_key(); + let stem = original_key.strip_suffix(".json").unwrap(); + let (prefix, nonce_hex) = stem.rsplit_once('-').unwrap(); + let nonce = u32::from_str_radix(nonce_hex, 16).unwrap(); + let relocated_key = format!("{prefix}-{:08x}.json", nonce ^ 1); + + let mut record_read_from_s3: LogRecord = + serde_json::from_slice(&serde_json::to_vec(&log).unwrap()).unwrap(); + record_read_from_s3.object_key = relocated_key; + let err = record_read_from_s3 + .verify(&signing_key.verification_key()) + .expect_err("changing only the random failure nonce must invalidate placement"); + + assert!(format!("{err:?}").contains("signature invalid")); + } + + #[test] + fn signed_log_binds_session_even_when_key_does_not_contain_it() { + let signing_key = GuardianSignKeyPair::from([19u8; 32]); + let log = LogRecord::new_at_timestamp( + "original-session".to_string(), + LogMessage::Genesis(Box::new(GenesisLogMessage { + committee: crate::move_types::Committee { + epoch: 0, + members: vec![], + total_weight: 0, + config: crate::move_types::Config::default(), + }, + })), + &signing_key, + 1_700_000_000_000, + ); + let mut aliased: LogRecord = + serde_json::from_slice(&serde_json::to_vec(&log).unwrap()).unwrap(); + aliased.session_id = "aliased-session".to_string(); + aliased.object_key = GenesisLogMessage::object_key(); + + let err = aliased + .verify(&signing_key.verification_key()) + .expect_err("session ID must be part of the signed routing context"); + + assert!(format!("{err:?}").contains("signature invalid")); + } + + #[test] + fn log_rejects_unknown_schema_version() { + let (_, mut log, signing_key) = signed_heartbeat(1_700_000_000_000); + log.schema_version = LOG_SCHEMA_VERSION + 1; + + let err = log + .verify(&signing_key.verification_key()) + .expect_err("unknown schema versions must be rejected"); + + assert!(format!("{err:?}").contains("unsupported log schema version")); + } + + #[test] + fn log_rejects_absent_schema_version() { + let (_, log, _) = signed_heartbeat(1_700_000_000_000); + let mut json = serde_json::to_value(log).unwrap(); + json.as_object_mut().unwrap().remove("schema_version"); + + assert!(serde_json::from_value::(json).is_err()); + } + + #[test] + fn unsigned_log_rejects_replay_at_another_s3_key() { + let signing_key = GuardianSignKeyPair::from([14u8; 32]); + let session_id = session_id_from_signing_pubkey(&signing_key.verification_key()); + let mut log = LogRecord::new_at_timestamp( + session_id, + LogMessage::Init(Box::new(InitLogMessage::OIAttestationUnsigned { + attestation: NitroAttestation::new(vec![1, 2, 3]), + signing_public_key: signing_key.verification_key(), + })), + &signing_key, + 1_700_000_000_000, + ); + + log.object_key = "init/copied-attestation.json".to_string(); + let err = log + .verify_unsigned() + .expect_err("unsigned record copied to another S3 key must be rejected"); + + assert!(format!("{err:?}").contains("non-canonical S3 object key")); + } + + #[test] + fn unsigned_attestation_rejects_session_not_derived_from_signing_key() { + let signing_key = GuardianSignKeyPair::from([15u8; 32]); + let log = LogRecord::new_at_timestamp( + "forged-session".to_string(), + LogMessage::Init(Box::new(InitLogMessage::OIAttestationUnsigned { + attestation: NitroAttestation::new(vec![1, 2, 3]), + signing_public_key: signing_key.verification_key(), + })), + &signing_key, + 1_700_000_000_000, + ); + let err = log + .verify_unsigned() + .expect_err("attestation session ID must come from its signing public key"); + + assert!(format!("{err:?}").contains("attestation session ID mismatch")); } #[test] fn object_key_for_init_attestation_unsigned() { let session_id = "session-a".to_string(); let signing_key = GuardianSignKeyPair::from([7u8; 32]); - let mut log = LogRecord::new( + let log = LogRecord::new_at_timestamp( session_id.clone(), LogMessage::Init(Box::new(InitLogMessage::OIAttestationUnsigned { attestation: NitroAttestation::new(vec![1, 2, 3]), signing_public_key: signing_key.verification_key(), })), &signing_key, + 1_700_000_000_000, ); - set_timestamp(&mut log, 1_700_000_000_000); assert_eq!( log.object_key(), @@ -208,12 +604,12 @@ mod tests { let seq = 42_u64; let timestamp_ms = 1_700_000_000_000; - let mut log = LogRecord::new( + let log = LogRecord::new_at_timestamp( session_id.clone(), LogMessage::Heartbeat { seq }, &signing_key, + timestamp_ms, ); - set_timestamp(&mut log, timestamp_ms); assert_eq!( log.object_key(), @@ -223,20 +619,20 @@ mod tests { #[test] fn object_key_and_lock_for_kp_share_state() { - use crate::guardian::KpShareState; + use crate::guardian::KpShareStateLogMessage; let session_id = "session-d".to_string(); let signing_key = GuardianSignKeyPair::from([10u8; 32]); - let mut log = LogRecord::new( + let log = LogRecord::new_at_timestamp( session_id, - LogMessage::KpShareState(Box::new(KpShareState::new( + LogMessage::KpShareState(Box::new(KpShareStateLogMessage::new( 7, 3, KPEncryptedShares::new(vec![]).unwrap(), ))), &signing_key, + 1_700_000_000_000, ); - set_timestamp(&mut log, 1_700_000_000_000); assert_eq!( log.object_key(), @@ -252,7 +648,7 @@ mod tests { fn object_key_and_lock_for_genesis_is_fixed() { let session_id = "session-g".to_string(); let signing_key = GuardianSignKeyPair::from([12u8; 32]); - let log = LogRecord::new( + let log = LogRecord::new_at_timestamp( session_id, LogMessage::Genesis(Box::new(GenesisLogMessage { committee: crate::move_types::Committee { @@ -263,6 +659,7 @@ mod tests { }, })), &signing_key, + 1_700_000_000_000, ); assert_eq!(log.object_key(), GenesisLogMessage::object_key()); @@ -282,7 +679,7 @@ mod tests { let request_data: StandardWithdrawalRequestWire = request_data.into(); let seq = request_data.seq; - let mut log = LogRecord::new( + let log = LogRecord::new_at_timestamp( session_id.clone(), LogMessage::Withdrawal(Box::new(WithdrawalLogMessage::Success { txid: Txid::from_slice(&[3u8; 32]).expect("valid txid"), @@ -296,8 +693,8 @@ mod tests { }, })), &signing_key, + timestamp_ms, ); - set_timestamp(&mut log, timestamp_ms); assert_eq!( log.object_key(), diff --git a/crates/hashi-types/src/guardian/log/message.rs b/crates/hashi-types/src/guardian/log/message.rs index 9bc4b0b2c5..58e0ddc56d 100644 --- a/crates/hashi-types/src/guardian/log/message.rs +++ b/crates/hashi-types/src/guardian/log/message.rs @@ -40,11 +40,28 @@ pub enum LogMessage { Init(Box), Withdrawal(Box), Ceremony(Box), - KpShareState(Box), + KpShareState(Box), CommitteeUpdate(Box), Genesis(Box), } +pub(super) enum ObjectKeyPattern { + Fixed(String), + /// Complete key prefix through the final `-`; preparation appends the nonce. + RandomSuffix(String), +} + +impl ObjectKeyPattern { + pub(super) fn finalize(self) -> String { + match self { + Self::Fixed(key) => key, + Self::RandomSuffix(prefix) => { + format!("{prefix}{:08x}.json", rand::random::()) + } + } + } +} + /// Operator-trusted bootstrap committee used before any `committee-update/` /// history exists. Written at `genesis/record.json`; callers must source it from /// on-chain state during first deploy. @@ -66,13 +83,13 @@ impl GenesisLogMessage { /// higher `cert_seq` entries for the same `sharing_seq` without changing the /// `ceremony/` instance. #[derive(Debug, Serialize, Deserialize, Clone, PartialEq)] -pub struct KpShareState { +pub struct KpShareStateLogMessage { pub sharing_seq: u64, pub cert_seq: u64, pub encrypted_shares: KPEncryptedShares, } -impl KpShareState { +impl KpShareStateLogMessage { pub fn new(sharing_seq: u64, cert_seq: u64, encrypted_shares: KPEncryptedShares) -> Self { Self { sharing_seq, @@ -244,23 +261,16 @@ impl WithdrawalLogMessage { /// log, which the KP reads to recover limiter state. Failures don't have a /// meaningful seq (the request's seq may be stale), so they use a random /// suffix for dedup. - pub fn log_name(&self, prefix: &str) -> String { + fn object_key_pattern(&self, directory: &str, session_id: &str) -> ObjectKeyPattern { match self { - WithdrawalLogMessage::Success { request_data, .. } => format!( - "success-{:020}-{}-wid{}.json", - request_data.seq, - prefix, - self.wid() - ), - WithdrawalLogMessage::Failure { .. } => { - let random_suffix = rand::random::(); - format!( - "failure-{}-wid{}-{:08x}.json", - prefix, - self.wid(), - random_suffix - ) - } + Self::Success { request_data, .. } => ObjectKeyPattern::Fixed(format!( + "{directory}success-{:020}-{session_id}-wid{}.json", + request_data.seq, request_data.wid, + )), + Self::Failure { request_data, .. } => ObjectKeyPattern::RandomSuffix(format!( + "{directory}failure-{session_id}-wid{}-", + request_data.wid, + )), } } @@ -277,18 +287,16 @@ impl CommitteeUpdateLogMessage { /// is epoch-sorted; failures lead with `failure-` so they sort after /// all successes, leaving the lex-last success key as the latest /// successfully-applied epoch. - pub fn log_name(&self, prefix: &str) -> String { + fn object_key_pattern(&self, directory: &str, session_id: &str) -> ObjectKeyPattern { match self { - CommitteeUpdateLogMessage::Success { new_committee, .. } => { - format!("{:020}-{}.json", new_committee.epoch, prefix) - } - CommitteeUpdateLogMessage::Failure { new_committee, .. } => { - let random_suffix = rand::random::(); - format!( - "failure-{:020}-{}-{:08x}.json", - new_committee.epoch, prefix, random_suffix - ) - } + Self::Success { new_committee, .. } => ObjectKeyPattern::Fixed(format!( + "{directory}{:020}-{session_id}.json", + new_committee.epoch, + )), + Self::Failure { new_committee, .. } => ObjectKeyPattern::RandomSuffix(format!( + "{directory}failure-{:020}-{session_id}-", + new_committee.epoch, + )), } } } @@ -307,7 +315,7 @@ impl LogMessage { } /// The directory under which logs are written. Ends with a slash. - pub fn log_dir(&self, timestamp_ms: UnixMillis) -> String { + fn log_dir(&self, timestamp_ms: UnixMillis) -> String { match self { LogMessage::Init(_) => format!("{}/", S3_DIR_INIT), LogMessage::Heartbeat { .. } => { @@ -325,16 +333,31 @@ impl LogMessage { } } - /// The name of the log. - pub fn log_name(&self, prefix: &str) -> String { + pub(super) fn object_key_pattern( + &self, + session_id: &str, + timestamp_ms: UnixMillis, + ) -> ObjectKeyPattern { + let directory = self.log_dir(timestamp_ms); match self { - LogMessage::Init(init_message) => init_message.log_name(prefix), - LogMessage::Heartbeat { seq } => format!("{}-{:020}.json", prefix, seq), - LogMessage::Withdrawal(withdrawal_message) => withdrawal_message.log_name(prefix), - LogMessage::Ceremony(ss) => format!("{:020}-{}.json", ss.sharing_seq(), prefix), - LogMessage::KpShareState(state) => state.log_name(prefix), - LogMessage::CommitteeUpdate(committee_message) => committee_message.log_name(prefix), - LogMessage::Genesis(_) => GenesisLogMessage::LOG_NAME.to_string(), + Self::Heartbeat { seq } => { + ObjectKeyPattern::Fixed(format!("{directory}{session_id}-{seq:020}.json")) + } + Self::Init(message) => { + ObjectKeyPattern::Fixed(format!("{directory}{}", message.log_name(session_id))) + } + Self::Withdrawal(message) => message.object_key_pattern(&directory, session_id), + Self::Ceremony(message) => ObjectKeyPattern::Fixed(format!( + "{directory}{:020}-{session_id}.json", + message.sharing_seq(), + )), + Self::KpShareState(state) => { + ObjectKeyPattern::Fixed(format!("{directory}{}", state.log_name(session_id))) + } + Self::CommitteeUpdate(message) => message.object_key_pattern(&directory, session_id), + Self::Genesis(_) => { + ObjectKeyPattern::Fixed(format!("{directory}{}", GenesisLogMessage::LOG_NAME)) + } } } diff --git a/crates/hashi-types/src/guardian/signing.rs b/crates/hashi-types/src/guardian/signing.rs index f205f739a6..2bbc5dd8c0 100644 --- a/crates/hashi-types/src/guardian/signing.rs +++ b/crates/hashi-types/src/guardian/signing.rs @@ -11,7 +11,6 @@ use super::GuardianError::InternalError; use super::GuardianError::InvalidInputs; use super::GuardianInfo; use super::GuardianResult; -use super::LogMessage; use super::RotateKpsResponse; use super::SetupNewKeyResponse; use super::SingleProvisionerInitRequest; @@ -33,7 +32,7 @@ use std::path::Path; #[repr(u8)] #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] pub enum IntentType { - /// Intent for all LogMessage's + /// Intent for key-bound guardian log records. LogMessage = 0, /// Intent for SetupNewKeyResponse SetupNewKeyResponse = 1, @@ -48,6 +47,16 @@ pub enum IntentType { /// Trait for types that can be signed, providing domain separation via an intent. pub trait SigningIntent { const INTENT: IntentType; + + /// Canonical bytes covered by this guardian signature. Most payloads use + /// the standard `(intent, data, timestamp)` layout; routing-sensitive + /// payloads may override this while retaining the central intent registry. + fn signing_bytes(&self, timestamp_ms: UnixMillis) -> Vec + where + Self: Serialize, + { + bcs::to_bytes(&(Self::INTENT, self, timestamp_ms)).expect("serialization should not fail") + } } /// All possible KP signing intent types. @@ -68,10 +77,6 @@ pub trait KpSigningIntent { const INTENT: KpSigningIntentType; } -impl SigningIntent for LogMessage { - const INTENT: IntentType = IntentType::LogMessage; -} - impl SigningIntent for SetupNewKeyResponse { const INTENT: IntentType = IntentType::SetupNewKeyResponse; } @@ -115,8 +120,7 @@ impl GuardianSigned { /// Create a new signed payload (used by enclave) /// Includes intent byte for domain separation to prevent cross-type signature attacks pub fn new(data: T, signing_key: &SigningKey, timestamp_ms: UnixMillis) -> Self { - let tuple = (T::INTENT, &data, timestamp_ms); - let signing_payload = bcs::to_bytes(&tuple).expect("serialization should not fail"); + let signing_payload = data.signing_bytes(timestamp_ms); let signature = signing_key.sign(&signing_payload); Self { data, @@ -128,8 +132,7 @@ impl GuardianSigned { /// Verify signature and extract payload /// Checks intent byte to ensure signature is for the correct type pub fn verify(self, pub_key: &VerificationKey) -> GuardianResult { - let tuple = (T::INTENT, &self.data, self.timestamp_ms); - let msg_bytes = bcs::to_bytes(&tuple).expect("serialization should not fail"); + let msg_bytes = self.data.signing_bytes(self.timestamp_ms); pub_key .verify(&self.signature, &msg_bytes) .map_err(|_| InvalidInputs("signature invalid".into()))?;