diff --git a/bin/validator/src/private_record.rs b/bin/validator/src/private_record.rs index a0fad43fd..0b3404d54 100644 --- a/bin/validator/src/private_record.rs +++ b/bin/validator/src/private_record.rs @@ -132,6 +132,29 @@ impl PrivateRecordContext { context.extend_from_slice(&PRIVATE_RECORD_FORMAT_V1.to_be_bytes()); context } + + /// Parses a canonical schema version 1 context produced by [`Self::to_bytes`]. + /// + /// Rejects any input whose domain tag, length, or format version differ from schema + /// version 1, or whose transaction id is not canonical. + pub fn try_from_bytes(bytes: &[u8]) -> Result { + let malformed = || PrivateRecordError::MalformedDecryptionContext; + let (domain, rest) = + bytes.split_at_checked(CONTEXT_DOMAIN_V1.len()).ok_or_else(malformed)?; + let (chain_id, rest) = rest.split_at_checked(32).ok_or_else(malformed)?; + let (key_epoch, rest) = rest.split_at_checked(32).ok_or_else(malformed)?; + let (transaction_id, version) = rest.split_at_checked(32).ok_or_else(malformed)?; + if domain != CONTEXT_DOMAIN_V1 || version != PRIVATE_RECORD_FORMAT_V1.to_be_bytes() { + return Err(malformed()); + } + let transaction_id = + TransactionId::read_from_bytes(transaction_id).map_err(|_error| malformed())?; + Ok(Self::new( + PrivateRecordChainId::new(chain_id.try_into().expect("split yields 32 bytes")), + StorageKeyEpoch::new(key_epoch.try_into().expect("split yields 32 bytes")), + transaction_id, + )) + } } /// Exact record values that an operator must approve before issuing a share. @@ -555,6 +578,9 @@ pub enum PrivateRecordError { /// The request does not carry the record's exact canonical context. #[error("private record decryption context does not match the record")] DecryptionContextMismatch, + /// The decryption context bytes are not a canonical schema version 1 context. + #[error("private record decryption context is malformed")] + MalformedDecryptionContext, /// The authenticated record cipher failed. #[error("failed to encrypt private record")] RecordEncryption, @@ -689,6 +715,57 @@ mod tests { assert_eq!(&bytes[CONTEXT_DOMAIN_V1.len() + 96..], &PRIVATE_RECORD_FORMAT_V1.to_be_bytes(),); } + #[test] + fn context_parses_its_canonical_encoding_and_rejects_others() { + let bytes = context().to_bytes(); + + assert_eq!(PrivateRecordContext::try_from_bytes(&bytes).unwrap(), context()); + + // Truncated, extended, wrong-domain, and wrong-version encodings are all rejected. + let mut extended = bytes.clone(); + extended.push(0); + let mut wrong_domain = bytes.clone(); + wrong_domain[0] ^= 1; + let mut wrong_version = bytes.clone(); + *wrong_version.last_mut().unwrap() ^= 1; + // A transaction id that is the right length but not canonical: an all-ones field element + // exceeds the modulus. + let mut wrong_transaction_id = bytes.clone(); + wrong_transaction_id[CONTEXT_DOMAIN_V1.len() + 64..CONTEXT_DOMAIN_V1.len() + 96].fill(0xff); + + let mut candidates = vec![ + bytes[..bytes.len() - 1].to_vec(), + extended, + wrong_domain, + wrong_version, + wrong_transaction_id, + ]; + // Inputs too short for each successive field, so that every length check is exercised and + // not just the trailing version comparison: nothing at all, then a partial domain tag, + // chain id, key epoch, and transaction id. + candidates.extend( + [ + 0, + CONTEXT_DOMAIN_V1.len() - 1, + CONTEXT_DOMAIN_V1.len() + 16, + CONTEXT_DOMAIN_V1.len() + 48, + CONTEXT_DOMAIN_V1.len() + 80, + ] + .map(|len| bytes[..len].to_vec()), + ); + + for candidate in candidates { + assert!( + matches!( + PrivateRecordContext::try_from_bytes(&candidate), + Err(PrivateRecordError::MalformedDecryptionContext), + ), + "a {}-byte context should not parse", + candidate.len(), + ); + } + } + #[test] fn seal_uses_a_fresh_key_and_nonce_and_wraps_only_the_key() { let plaintext = b"private transaction inputs"; diff --git a/bin/validator/src/server/admin_service/issue_decryption_share.rs b/bin/validator/src/server/admin_service/issue_decryption_share.rs index 93f7b203a..75e26f1a2 100644 --- a/bin/validator/src/server/admin_service/issue_decryption_share.rs +++ b/bin/validator/src/server/admin_service/issue_decryption_share.rs @@ -5,9 +5,9 @@ use axum::extract::State; use rand_core_06::OsRng; use serde::{Deserialize, Serialize}; -use crate::PrivateRecordError; use crate::server::admin_service::error::ApiError; use crate::server::admin_service::{ValidatorAdminService, decode_hex}; +use crate::{PrivateRecordContext, PrivateRecordError}; #[derive(Clone, Debug, Deserialize, Serialize)] pub(super) struct IssueDecryptionShareRequest { @@ -26,6 +26,25 @@ pub(super) async fn issue_decryption_share( ) -> Result, ApiError> { let ciphertext = decode_hex("ciphertext", &request.ciphertext)?; let decryption_context = decode_hex("decryption_context", &request.decryption_context)?; + + // Only issue shares over transactions this validator itself validated. The context is + // cryptographically bound to the ciphertext, so an attacker cannot smuggle an arbitrary + // ciphertext under a validated transaction's context; and because every validator in the quorum + // validated the transaction, cross-validator recovery (combining shares over one validator's + // ciphertext) keeps working. + let context = PrivateRecordContext::try_from_bytes(&decryption_context) + .map_err(|error| ApiError::bad_request(error.to_string()))?; + let validated = service + .reader + .transaction_exists(context.transaction_id()) + .await + .map_err(|_error| ApiError::internal("failed to look up the transaction"))?; + if !validated { + return Err(ApiError::not_found( + "decryption context references a transaction this validator has not validated", + )); + } + let decryption_share = service .operator_key .issue_decryption_share(&mut OsRng, &ciphertext, &decryption_context) @@ -36,11 +55,40 @@ pub(super) async fn issue_decryption_share( })) } +/// Maps a share-issuance failure onto a response. +/// +/// Matched exhaustively rather than through a wildcard, so that a new [`PrivateRecordError`] +/// variant — or a `golden-ehtdh1` upgrade that introduces a new failure — has to be classified +/// here, instead of silently defaulting to an internal error over what may be a malformed +/// request. +/// +/// Only the bad-request arm is reachable today. `issue_decryption_share` rejects, in order, a +/// ciphertext that does not decode, a wrong-sized wrapped content key, and a ciphertext not bound +/// to the supplied context; `MalformedDecryptionContext` comes from this endpoint's own context +/// parsing. Everything in the internal arm belongs to sealing, share combination, or decoding a +/// stored record — none of which this endpoint does — so reaching one is a validator fault rather +/// than the caller's. fn map_share_error(error: &PrivateRecordError) -> ApiError { match error { PrivateRecordError::InvalidGoldenEncoding(_) | PrivateRecordError::InvalidEncryptedRecordKey + | PrivateRecordError::MalformedDecryptionContext | PrivateRecordError::DecryptionContextMismatch => ApiError::bad_request(error.to_string()), - _ => ApiError::internal("failed to issue Golden decryption share"), + PrivateRecordError::KeyEpochMismatch + | PrivateRecordError::RecordIdMismatch + | PrivateRecordError::InvalidValidatorId(_) + | PrivateRecordError::SetupContextMismatch + | PrivateRecordError::RecordEncryption + | PrivateRecordError::ContentKeyEncryption(_) + | PrivateRecordError::InvalidCombinerSetup(_) + | PrivateRecordError::InvalidDecryptionShare(_) + | PrivateRecordError::ShareGeneration(_) + | PrivateRecordError::ShareCombination(_) + | PrivateRecordError::UnsupportedFormat(_) + | PrivateRecordError::InvalidNonceLength { .. } + | PrivateRecordError::InvalidRecordCiphertext + | PrivateRecordError::RecordDecryption => { + ApiError::internal("failed to issue Golden decryption share") + }, } } diff --git a/bin/validator/src/server/admin_service/tests.rs b/bin/validator/src/server/admin_service/tests.rs index 5ef00a70e..e8e32f573 100644 --- a/bin/validator/src/server/admin_service/tests.rs +++ b/bin/validator/src/server/admin_service/tests.rs @@ -395,17 +395,44 @@ async fn get_transaction_returns_the_full_record() { assert_eq!(error.status, StatusCode::BAD_REQUEST); } +/// The share endpoint refuses to act as a decryption oracle: it only issues shares whose decryption +/// context references a transaction this validator itself validated. +#[tokio::test] +async fn share_refused_for_unvalidated_transaction() { + let mut keys = operator_keys(); + let record_owner = keys.pop().unwrap(); + let (_directory, _writer, reader) = test_database().await; + let service = ValidatorAdminService::new(keys.pop().unwrap(), reader); + let transaction_id = TransactionId::from_raw(Word::from([2u32, 4, 6, 8])); + let record = target_record(&record_owner, transaction_id, 30, b"record"); + + let error = issue(&service, share_request(&record)).await.unwrap_err(); + + assert_eq!(error.status, StatusCode::NOT_FOUND); +} + #[tokio::test] async fn shares_for_different_ciphertexts_are_not_reusable() { let mut keys = operator_keys(); let record_owner = keys.pop().unwrap(); - let second = ValidatorAdminService::new(keys.pop().unwrap(), test_database().await.2); - let first = ValidatorAdminService::new(keys.pop().unwrap(), test_database().await.2); let transaction_id = TransactionId::from_raw(Word::from([1u32, 2, 3, 4])); let first_record = target_record(&record_owner, transaction_id, 2, b"same plaintext"); let second_record = target_record(&record_owner, transaction_id, 3, b"same plaintext"); assert_eq!(first_record.context(), second_record.context()); assert_ne!(first_record.encrypted_record_key(), second_record.encrypted_record_key()); + // Each validator has validated (and stored its own record for) the transaction. + let (_first_dir, first_writer, first_reader) = test_database().await; + first_writer + .insert_validated_private_transaction(first_record.clone()) + .await + .unwrap(); + let (_second_dir, second_writer, second_reader) = test_database().await; + second_writer + .insert_validated_private_transaction(second_record.clone()) + .await + .unwrap(); + let second = ValidatorAdminService::new(keys.pop().unwrap(), second_reader); + let first = ValidatorAdminService::new(keys.pop().unwrap(), first_reader); let shares = [ issue(&first, share_request(&first_record)).await.unwrap().decryption_share, @@ -425,10 +452,13 @@ async fn shares_for_different_ciphertexts_are_not_reusable() { #[tokio::test] async fn invalid_share_requests_return_bad_request() { let mut keys = operator_keys(); - let record = - target_record(&keys[0], TransactionId::from_raw(Word::from([1u32, 2, 3, 4])), 4, b"record"); + let transaction_id = TransactionId::from_raw(Word::from([1u32, 2, 3, 4])); + let record = target_record(&keys[0], transaction_id, 4, b"record"); let context = record.context().to_bytes(); - let (_directory, _writer, reader) = test_database().await; + let (_directory, writer, reader) = test_database().await; + // The referenced transaction is validated, so these requests fail on their own defects rather + // than on the validated-transaction check. + writer.insert_validated_private_transaction(record.clone()).await.unwrap(); let invalid_hex = IssueDecryptionShareRequest { ciphertext: "not hex".to_owned(),