diff --git a/rust/rbac-registration/src/cardano/cip509/cip509.rs b/rust/rbac-registration/src/cardano/cip509/cip509.rs index 6f354d9dd7b..4e599b5e66f 100644 --- a/rust/rbac-registration/src/cardano/cip509/cip509.rs +++ b/rust/rbac-registration/src/cardano/cip509/cip509.rs @@ -465,7 +465,7 @@ fn payment_history( report: &ProblemReport, ) -> HashMap> { let hash = MultiEraTx::Conway(Box::new(Cow::Borrowed(txn))).hash(); - let context = format!("Populating payment history for Cip509, transaction hash = {hash:?}"); + let context = format!("Populating payment history for Cip509, transaction = {hash}"); let mut result: HashMap<_, _> = track_payment_addresses .iter() diff --git a/rust/rbac-registration/src/cardano/cip509/types/role_data.rs b/rust/rbac-registration/src/cardano/cip509/types/role_data.rs index 1337e25232e..2f788da9d50 100644 --- a/rust/rbac-registration/src/cardano/cip509/types/role_data.rs +++ b/rust/rbac-registration/src/cardano/cip509/types/role_data.rs @@ -11,9 +11,7 @@ use pallas::ledger::{ }; use crate::cardano::cip509::{ - rbac::role_data::CborRoleData, - utils::cip19::{compare_key_hash, extract_key_hash}, - KeyLocalRef, + rbac::role_data::CborRoleData, utils::cip19::extract_key_hash, KeyLocalRef, }; /// A role data. @@ -132,20 +130,31 @@ fn convert_payment_key( return None; }, }; - validate_payment_output(address, &witness, context, report); - match Address::from_bytes(address) { - Ok(Address::Shelley(a)) => Some(a), - Ok(a) => { + let address = match Address::from_bytes(address) { + Ok(a) => a, + Err(e) => { report.other( - &format!("Unsupported address type ({a:?}) in payment key index ({index})"), + &format!("Invalid address in payment key index ({index}): {e}"), + context, + ); + return None; + }, + }; + validate_payment_output(&address, &witness, context, report); + + match address { + Address::Shelley(a) => Some(a), + Address::Byron(_) => { + report.other( + &format!("Unsupported Byron address type in payment key index ({index})"), context, ); None }, - Err(e) => { + Address::Stake(_) => { report.other( - &format!("Invalid address in payment key index ({index}): {e:?}"), + &format!("Unsupported Stake address type in payment key index ({index})"), context, ); None @@ -155,12 +164,13 @@ fn convert_payment_key( /// Helper function for validating payment output key. fn validate_payment_output( - output_address: &[u8], + address: &Address, witness: &TxnWitness, context: &str, report: &ProblemReport, ) { - let Some(key) = extract_key_hash(output_address) else { + let bytes = address.to_vec(); + let Some(key) = extract_key_hash(&bytes) else { report.other("Failed to extract payment key hash from address", context); return; }; @@ -168,10 +178,10 @@ fn validate_payment_output( // Set transaction index to 0 because the list of transaction is manually constructed // for TxWitness -> &[txn.clone()], so we can assume that the witness contains only // the witness within this transaction. - if let Err(e) = compare_key_hash(&[key], witness, 0.into()) { + if !witness.check_witness_in_tx(&key, 0.into()) { report.other( &format!( - "Unable to find payment output key ({key:?}) in the transaction witness set: {e:?}" + "Payment Key {address} (0x{key}) is not present in the transaction witness set, and can not be verified as owned and spendable." ), context, ); diff --git a/rust/rbac-registration/src/cardano/cip509/utils/cip19.rs b/rust/rbac-registration/src/cardano/cip509/utils/cip19.rs index 1e15e3aac29..a7bb80007c1 100644 --- a/rust/rbac-registration/src/cardano/cip509/utils/cip19.rs +++ b/rust/rbac-registration/src/cardano/cip509/utils/cip19.rs @@ -1,33 +1,9 @@ //! Utility functions for CIP-19 address. -use anyhow::bail; -use cardano_blockchain_types::{TxnIndex, TxnWitness, VKeyHash}; +use cardano_blockchain_types::VKeyHash; /// Extract the first 28 bytes from the given key /// Refer to for more information. pub(crate) fn extract_key_hash(key: &[u8]) -> Option { key.get(1..29).and_then(|v| v.try_into().ok()) } - -/// Compare the given public key bytes with the transaction witness set. -pub(crate) fn compare_key_hash( - pk_addrs: &[VKeyHash], - witness: &TxnWitness, - txn_idx: TxnIndex, -) -> anyhow::Result<()> { - if pk_addrs.is_empty() { - bail!("No public key addresses provided"); - } - - pk_addrs.iter().try_for_each(|pk_addr| { - // Key hash not found in the transaction witness set - if !witness.check_witness_in_tx(pk_addr, txn_idx) { - bail!( - "Public key hash not found in transaction witness set given {:?}", - pk_addr - ); - } - - Ok(()) - }) -} diff --git a/rust/rbac-registration/src/cardano/cip509/validation.rs b/rust/rbac-registration/src/cardano/cip509/validation.rs index 88c54ba7699..f20ca4d5ae8 100644 --- a/rust/rbac-registration/src/cardano/cip509/validation.rs +++ b/rust/rbac-registration/src/cardano/cip509/validation.rs @@ -23,13 +23,11 @@ use pallas::{ }; use x509_cert::{der::Encode as X509Encode, Certificate as X509}; -use super::{ - extract_key::{c509_key, x509_key}, - utils::cip19::compare_key_hash, -}; use crate::cardano::cip509::{ - rbac::Cip509RbacMetadata, types::TxInputHash, C509Cert, Cip0134UriSet, LocalRefInt, RoleData, - SimplePublicKeyType, X509DerCert, + rbac::Cip509RbacMetadata, + types::TxInputHash, + utils::extract_key::{c509_key, x509_key}, + C509Cert, Cip0134UriSet, LocalRefInt, RoleData, SimplePublicKeyType, X509DerCert, }; /// Context-specific primitive type with tag number 6 (`raw_tag` 134) for @@ -109,7 +107,9 @@ pub fn validate_aux( let hash = Blake2b256Hash::new(raw_aux_data); if hash != auxiliary_data_hash { report.other( - &format!("Incorrect transaction auxiliary data hash = '{hash:?}', expected = '{auxiliary_data_hash:?}'"), + &format!("Incorrect transaction auxiliary data hash = '0x{hash}', expected = '0x{auxiliary_data_hash}'. \ + This metadata does not belong with this transaction. \ + Catalyst metadata may not be transcribed to any other transaction body, and is therefore invalid."), context, ); } @@ -142,17 +142,22 @@ pub fn validate_stake_public_key( return; } - if let Err(e) = compare_key_hash(&pk_addrs, &witness, 0.into()) { - report.other( - &format!("Failed to compare public keys with witnesses: {e:?}"), - context, - ); + for (hash, address) in pk_addrs { + if !witness.check_witness_in_tx(&hash, 0.into()) { + report.other( + &format!( + "Payment Key '{address}' (0x{hash}) is not present in the transaction witness set, and can not be verified as owned and spendable." + ), + context, + ); + } } } /// Extracts all stake addresses from both X509 and C509 certificates containing in the -/// given `Cip509` and converts their hashes to bytes. -fn extract_stake_addresses(uris: Option<&Cip0134UriSet>) -> Vec { +/// given `Cip509`. Returns a list of pairs containing verifying public key hash and +/// `bech32` string representation of address. +fn extract_stake_addresses(uris: Option<&Cip0134UriSet>) -> Vec<(VKeyHash, String)> { let Some(uris) = uris else { return Vec::new(); }; @@ -163,7 +168,13 @@ fn extract_stake_addresses(uris: Option<&Cip0134UriSet>) -> Vec { .flat_map(|(_index, uris)| uris.iter()) .filter_map(|uri| { if let Address::Stake(a) = uri.address() { - a.payload().as_hash().as_slice().try_into().ok() + let bech32 = uri.address().to_string(); + a.payload() + .as_hash() + .as_slice() + .try_into() + .ok() + .map(|hash| (hash, bech32)) } else { None } @@ -547,7 +558,7 @@ mod tests { let report = registration.consume().unwrap_err(); assert!(report.is_problematic()); let report = format!("{report:?}"); - assert!(report.contains("Public key hash not found in transaction witness set")); + assert!(report.contains("is not present in the transaction witness set, and can not be verified as owned and spendable")); } #[test] @@ -617,7 +628,7 @@ mod tests { let addresses = extract_stake_addresses(cip509.certificate_uris()); assert_eq!(1, addresses.len()); - assert_eq!(addresses.first().unwrap(), &hash); + assert_eq!(addresses.first().unwrap().0, hash); } // Verify that we are able to parse `Cip509` with legacy transaction output type. diff --git a/rust/rbac-registration/src/registration/cardano/mod.rs b/rust/rbac-registration/src/registration/cardano/mod.rs index a8beab9a88d..ff959890b7b 100644 --- a/rust/rbac-registration/src/registration/cardano/mod.rs +++ b/rust/rbac-registration/src/registration/cardano/mod.rs @@ -7,7 +7,7 @@ use std::{ sync::Arc, }; -use anyhow::{bail, Context}; +use anyhow::Context; use c509_certificate::c509::C509; use cardano_blockchain_types::{Point, StakeAddress, TransactionId, TxnIndex}; use catalyst_types::{ @@ -17,7 +17,6 @@ use catalyst_types::{ uuid::UuidV4, }; use ed25519_dalek::{Signature, VerifyingKey}; -use tracing::error; use update_rbac::{ revocations_list, update_c509_certs, update_public_keys, update_role_data, update_x509_certs, }; @@ -47,10 +46,11 @@ impl RegistrationChain { /// # Errors /// /// Returns an error if data is invalid - pub fn new(cip509: Cip509) -> anyhow::Result { + #[must_use] + pub fn new(cip509: Cip509) -> Option { let inner = RegistrationChainInner::new(cip509)?; - Ok(Self { + Some(Self { inner: Arc::new(inner), }) } @@ -63,10 +63,11 @@ impl RegistrationChain { /// # Errors /// /// Returns an error if data is invalid + #[must_use] pub fn update( &self, cip509: Cip509, - ) -> anyhow::Result { + ) -> Option { let latest_signing_pk = self.get_latest_signing_pk_for_role(&RoleId::Role0); let new_inner = if let Some((signing_pk, _)) = latest_signing_pk { self.inner.update(cip509, signing_pk)? @@ -75,9 +76,9 @@ impl RegistrationChain { "latest signing key for role 0", "cannot perform signature validation during Registration Chain update", ); - bail!("No latest signing key found for role 0, cannot perform signature validation") + return None; }; - Ok(Self { + Some(Self { inner: Arc::new(new_inner), }) } @@ -301,18 +302,18 @@ impl RegistrationChainInner { /// # Errors /// /// Returns an error if data is invalid - fn new(cip509: Cip509) -> anyhow::Result { + #[must_use] + fn new(cip509: Cip509) -> Option { let context = "Registration Chain new"; // Should be chain root, return immediately if not if cip509.previous_transaction().is_some() { cip509 .report() .invalid_value("previous transaction ID", "None", "Some", context); - bail!("Invalid chain root, previous transaction ID should be None."); } let Some(catalyst_id) = cip509.catalyst_id().cloned() else { cip509.report().missing_field("catalyst id", context); - bail!("Invalid chain root, catalyst id should be present."); + return None; }; let point_tx_idx = cip509.origin().clone(); @@ -337,7 +338,7 @@ impl RegistrationChainInner { // must contain role 0 let Some(role0_data) = role_data_record.get(&RoleId::Role0) else { cip509.report().missing_field("Role 0", context); - bail!("Role 0 not found"); + return None; }; let Some(signing_pk) = role0_data .signing_keys() @@ -347,7 +348,7 @@ impl RegistrationChainInner { cip509 .report() .missing_field("Signing pk for role 0 not found", context); - bail!("No valid signing key found for role 0"); + return None; }; check_validation_signature( @@ -356,15 +357,10 @@ impl RegistrationChainInner { signing_pk, cip509.report(), context, - )?; - - let (purpose, registration, payment_history) = match cip509.consume() { - Ok(v) => v, - Err(e) => { - let error = format!("Invalid Cip509: {e:?}"); - error!(error); - bail!(error); - }, + ); + + let Ok((purpose, registration, payment_history)) = cip509.consume() else { + return None; }; let purpose = vec![purpose]; @@ -389,7 +385,7 @@ impl RegistrationChainInner { ); let revocations = revocations_list(registration.revocation_list.clone(), &point_tx_idx); - Ok(Self { + Some(Self { catalyst_id, current_tx_id_hash, purpose, @@ -412,11 +408,12 @@ impl RegistrationChainInner { /// # Errors /// /// Returns an error if data is invalid + #[must_use] fn update( &self, cip509: Cip509, signing_pk: VerifyingKey, - ) -> anyhow::Result { + ) -> Option { let context = "Registration Chain update"; let mut new_inner = self.clone(); @@ -424,7 +421,7 @@ impl RegistrationChainInner { cip509 .report() .missing_field("previous transaction ID", context); - bail!("Missing previous transaction ID"); + return None; }; // Previous transaction ID in the CIP509 should equal to the current transaction ID @@ -437,7 +434,7 @@ impl RegistrationChainInner { signing_pk, cip509.report(), context, - )?; + ); // If successful, update the chain current transaction ID hash new_inner.current_tx_id_hash = @@ -449,17 +446,12 @@ impl RegistrationChainInner { &format!("{:?}", self.current_tx_id_hash), context, ); - bail!("Invalid previous transaction ID, not a part of this registration chain"); + return None; } let point_tx_idx = cip509.origin().clone(); - let (purpose, registration, payment_history) = match cip509.consume() { - Ok(v) => v, - Err(e) => { - let error = format!("Invalid Cip509: {e:?}"); - error!(error); - bail!(error); - }, + let Ok((purpose, registration, payment_history)) = cip509.consume() else { + return None; }; // Add purpose to the chain, if not already exist @@ -496,7 +488,7 @@ impl RegistrationChainInner { &point_tx_idx, ); - Ok(new_inner) + Some(new_inner) } } @@ -508,34 +500,37 @@ fn check_validation_signature( signing_pk: VerifyingKey, report: &ProblemReport, context: &str, -) -> anyhow::Result<()> { +) { let context = &format!("Check Validation Signature in {context}"); // Note that the validation signature can be in the range of 1 - 64 bytes // But since we allow only Ed25519, it should be 64 bytes let unsigned_aux = zero_out_last_n_bytes(raw_aux_data, Signature::BYTE_SIZE); - let validation_sig = validation_signature.with_context(|| { + let Some(validation_sig) = validation_signature else { report.missing_field("validation signature", context); - "Missing validation signature" - })?; + return; + }; - let sig: Signature = validation_sig.clone().try_into().with_context(|| { + let Ok(sig) = validation_sig.clone().try_into() else { report.conversion_error( "validation signature", &format!("{validation_sig:?}"), "Ed25519 signature", context, ); - "Failed to convert validation signature to Ed25519 Signature" - })?; + return; + }; // Verify the signature using the latest signing public key - signing_pk + if let Err(e) = signing_pk .verify_strict(&unsigned_aux, &sig) .with_context(|| { report.other("Signature validation failed", context); "Signature verification failed" }) + { + report.functional_validation(&format!("Signature validation failed: {e}"), context); + } } #[cfg(test)] @@ -580,12 +575,13 @@ mod test { .unwrap(); assert!(registration.report().is_problematic()); - let error = chain.update(registration).unwrap_err(); - let error = format!("{error:?}"); + let report = registration.report().to_owned(); + assert!(chain.update(registration).is_none()); + let report = format!("{report:?}"); assert!( - error.contains("Invalid previous transaction ID"), + report.contains("kind: InvalidValue { field: \"previous transaction ID\""), "{}", - error + report ); // Add the second registration.