fix: validate all uris in cert that they should be witness

Signed-off-by: bkioshn <[email protected]>

Author: bkioshn

rust/rbac-registration/src/cardano/cip509/cip509.rs

@@ -13,7 +13,7 @@ use cardano_blockchain_types::{
     pallas_addresses::{Address, ShelleyAddress},
     pallas_primitives::{conway, Nullable},
     pallas_traverse::MultiEraTx,
-    MetadatumLabel, MultiEraBlock, StakeAddress, TxnIndex,
+    MetadatumLabel, MultiEraBlock, TxnIndex,
 };
 use catalyst_types::{
     catalyst_id::{role_index::RoleId, CatalystId},
@@ -36,7 +36,7 @@ use crate::cardano::cip509::{
     types::{PaymentHistory, TxInputHash, ValidationSignature},
     utils::Cip0134UriSet,
     validation::{
-        validate_aux, validate_role_data, validate_self_sign_cert, validate_stake_public_key,
+        validate_aux, validate_cert_addrs, validate_role_data, validate_self_sign_cert,
         validate_txn_inputs_hash,
     },
     x509_chunks::X509Chunks,
@@ -172,12 +172,11 @@ impl Cip509 {
             txn.transaction_body.auxiliary_data_hash.as_ref(),
             &cip509.report,
         );
-        if cip509.role_data(RoleId::Role0).is_some() {
-            // The following check is only performed for the role 0.
-            validate_stake_public_key(txn, cip509.certificate_uris(), &cip509.report);
-        }
         if let Some(metadata) = &cip509.metadata {
             cip509.catalyst_id = validate_role_data(metadata, block.network(), &cip509.report);
+            // General check for all roles, check whether the addresses in the certificate URIs are
+            // witnessed in the transaction.
+            validate_cert_addrs(txn, cip509.certificate_uris(), &report);
             validate_self_sign_cert(metadata, &report);
         }
 
@@ -276,12 +275,15 @@ impl Cip509 {
         self.catalyst_id.as_ref()
     }
 
-    /// Returns a list of role 0 stake addresses.
+    /// Returns a list of addresses extract from certificate URIs.
     #[must_use]
-    pub fn role_0_stake_addresses(&self) -> HashSet<StakeAddress> {
+    pub fn certificate_addresses(
+        &self,
+        index: usize,
+    ) -> HashSet<Address> {
         self.metadata
             .as_ref()
-            .map(|m| m.certificate_uris.stake_addresses(0))
+            .map(|m| m.certificate_uris.addresses(index))
             .unwrap_or_default()
     }
 

rust/rbac-registration/src/cardano/cip509/utils/cip134_uri_set.rs

@@ -10,7 +10,7 @@ use c509_certificate::{
     general_names::general_name::{GeneralNameTypeRegistry, GeneralNameValue},
     C509ExtensionType,
 };
-use cardano_blockchain_types::{pallas_addresses::Address, Cip0134Uri, StakeAddress};
+use cardano_blockchain_types::{pallas_addresses::Address, Cip0134Uri};
 use catalyst_types::problem_report::ProblemReport;
 use der_parser::der::parse_der_sequence;
 use tracing::debug;
@@ -72,24 +72,35 @@ impl Cip0134UriSet {
         self.x_uris().is_empty() && self.c_uris().is_empty()
     }
 
-    /// Returns a list of stake addresses by the given index.
+    /// Returns a list of addresses by the given index.
     #[must_use]
-    pub fn stake_addresses(
+    pub fn addresses(
         &self,
         index: usize,
-    ) -> HashSet<StakeAddress> {
+    ) -> HashSet<Address> {
         let mut result = HashSet::new();
 
         if let Some(uris) = self.x_uris().get(&index) {
-            result.extend(convert_stake_addresses(uris));
+            result.extend(uris.iter().map(|uri| uri.address().clone()));
         }
         if let Some(uris) = self.c_uris().get(&index) {
-            result.extend(convert_stake_addresses(uris));
+            result.extend(uris.iter().map(|uri| uri.address().clone()));
         }
 
         result
     }
 
+    /// Return true if the given index contains at least one stake address.
+    #[must_use]
+    pub fn contain_stake_address(
+        &self,
+        index: usize,
+    ) -> bool {
+        self.addresses(index)
+            .iter()
+            .any(|address| matches!(address, Address::Stake(_)))
+    }
+
     /// Return the updated URIs set.
     ///
     /// The resulting set includes all the data from both the original and a new one. In
@@ -289,18 +300,6 @@ fn extract_c509_uris(
     result
 }
 
-/// Converts a list of `Cip0134Uri` to a list of stake addresses.
-fn convert_stake_addresses(uris: &[Cip0134Uri]) -> Vec<StakeAddress> {
-    uris.iter()
-        .filter_map(|uri| {
-            match uri.address() {
-                Address::Stake(a) => Some(a.clone().into()),
-                _ => None,
-            }
-        })
-        .collect()
-}
-
 #[cfg(test)]
 mod tests {
 

rust/rbac-registration/src/cardano/cip509/validation.rs

@@ -117,14 +117,13 @@ pub fn validate_aux(
     }
 }
 
-/// Checks that all public keys extracted from x509 and c509 certificates are present in
-/// the witness set of the transaction.
-pub fn validate_stake_public_key(
+/// Check certificate URI addresses, if any, must be witnessed in the transaction.
+pub fn validate_cert_addrs(
     transaction: &conway::Tx,
     uris: Option<&Cip0134UriSet>,
     report: &ProblemReport,
 ) {
-    let context = "Cip509 stake public key validation";
+    let context = "Cip509 certificate URI address validation";
 
     let transaction = MultiEraTx::Conway(Box::new(Cow::Borrowed(transaction)));
     let witness = match TxnWitness::new(std::slice::from_ref(&transaction)) {
@@ -135,23 +134,17 @@ pub fn validate_stake_public_key(
         },
     };
 
-    let pk_addrs = extract_stake_addresses(uris);
-    if pk_addrs.is_empty() {
-        report.other(
-            "Unable to find stake addresses in Cip509 certificates",
-            context,
-        );
-        return;
-    }
+    let stake_addrs = extract_stake_addresses(uris);
+    let payment_addrs = extract_payment_addresses(uris);
 
-    for (hash, address) in pk_addrs {
+    for (hash, address) in stake_addrs.into_iter().chain(payment_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,
-            );
+                    &format!(
+                        "Address '{address}', key hash (0x{hash}) is not present in the transaction witness set, and can not be verified as owned"
+                    ),
+                    context,
+                );
         }
     }
 }
@@ -184,6 +177,41 @@ fn extract_stake_addresses(uris: Option<&Cip0134UriSet>) -> Vec<(VKeyHash, Strin
         .collect()
 }
 
+/// Extracts all payment addresses from both X509 and C509 certificates containing in the
+/// given `Cip509`. Returns a list of pairs containing verifying public key hash (only the
+/// payment part) and `bech32` string representation of address.
+fn extract_payment_addresses(uris: Option<&Cip0134UriSet>) -> Vec<(VKeyHash, String)> {
+    let Some(uris) = uris else {
+        return Vec::new();
+    };
+
+    uris.x_uris()
+        .iter()
+        .chain(uris.c_uris())
+        .flat_map(|(_index, uris)| uris.iter())
+        .filter_map(|uri| {
+            if let Address::Shelley(a) = uri.address() {
+                match a.payment() {
+                    // Shelley payment part is used to sign the transaction
+                    cardano_blockchain_types::pallas_addresses::ShelleyPaymentPart::Key(hash) => {
+                        match a.to_bech32() {
+                            Ok(bech32) => {
+                                hash.as_slice().try_into().ok().map(|hash| (hash, bech32))
+                            },
+                            Err(_) => None,
+                        }
+                    },
+                    cardano_blockchain_types::pallas_addresses::ShelleyPaymentPart::Script(_) => {
+                        None
+                    },
+                }
+            } else {
+                None
+            }
+        })
+        .collect()
+}
+
 /// Validate self-signed certificates.
 /// All certificates should be self-signed and support only ED25519 signature.
 pub fn validate_self_sign_cert(
@@ -358,6 +386,15 @@ pub fn validate_role_data(
             {
                 report.other("The role 0 certificate must be present", context);
             }
+
+            // Can contain different kind of address URIs, but for role 0
+            // there should be at least 1 stake address
+            if !metadata.certificate_uris.contain_stake_address(0) {
+                report.missing_field(
+                    "The role 0 certificate must have at least one stake address",
+                    context,
+                );
+            }
         } else {
             // For other roles there still must be exactly one certificate at 0 index, but it must
             // have the `undefined` value.

rust/rbac-registration/src/registration/cardano/mod.rs

@@ -2,14 +2,11 @@
 
 mod update_rbac;
 
-use std::{
-    collections::{HashMap, HashSet},
-    sync::Arc,
-};
+use std::{collections::HashMap, sync::Arc};
 
 use anyhow::Context;
 use c509_certificate::c509::C509;
-use cardano_blockchain_types::{hashes::TransactionId, Point, StakeAddress, TxnIndex};
+use cardano_blockchain_types::{hashes::TransactionId, Point, TxnIndex};
 use catalyst_types::{
     catalyst_id::{key_rotation::KeyRotation, role_index::RoleId, CatalystId},
     conversion::zero_out_last_n_bytes,
@@ -249,11 +246,12 @@ impl RegistrationChain {
             .and_then(|rdr| rdr.encryption_key_from_rotation(rotation))
     }
 
-    /// Returns a set of role 0 stake addresses.
-    #[must_use]
-    pub fn role_0_stake_addresses(&self) -> HashSet<StakeAddress> {
-        self.inner.certificate_uris.stake_addresses(0)
-    }
+    // FIXME
+    // /// Returns a set of role 0 stake addresses.
+    // #[must_use]
+    // pub fn role_0_stake_addresses(&self) -> HashSet<StakeAddress> {
+    //     self.inner.certificate_uris.stake_addresses(0)
+    // }
 }
 
 /// Inner structure of registration chain.