fix(rust/rbac-registration): Validate witness for stake address and payment address URIs in certificate (#507)

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

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

* chore: fix typo

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

* fix: get addresses

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

* test: add get role address test

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

---------

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 extracted from certificate URIs of a specific role.
     #[must_use]
-    pub fn role_0_stake_addresses(&self) -> HashSet<StakeAddress> {
+    pub fn certificate_addresses(
+        &self,
+        role: usize,
+    ) -> HashSet<Address> {
         self.metadata
             .as_ref()
-            .map(|m| m.certificate_uris.stake_addresses(0))
+            .map(|m| m.certificate_uris.role_addresses(role))
             .unwrap_or_default()
     }
 

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

@@ -72,24 +72,57 @@ 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 role.
     #[must_use]
-    pub fn stake_addresses(
+    pub fn role_addresses(
         &self,
-        index: usize,
-    ) -> HashSet<StakeAddress> {
+        role: usize,
+    ) -> HashSet<Address> {
         let mut result = HashSet::new();
 
-        if let Some(uris) = self.x_uris().get(&index) {
-            result.extend(convert_stake_addresses(uris));
+        if let Some(uris) = self.x_uris().get(&role) {
+            result.extend(uris.iter().map(|uri| uri.address().clone()));
         }
-        if let Some(uris) = self.c_uris().get(&index) {
-            result.extend(convert_stake_addresses(uris));
+        if let Some(uris) = self.c_uris().get(&role) {
+            result.extend(uris.iter().map(|uri| uri.address().clone()));
         }
 
         result
     }
 
+    /// Returns a list of stake addresses by the given role.
+    #[must_use]
+    pub fn role_stake_addresses(
+        &self,
+        role: usize,
+    ) -> HashSet<StakeAddress> {
+        self.role_addresses(role)
+            .iter()
+            .filter_map(|address| {
+                match address {
+                    Address::Stake(a) => Some(a.clone().into()),
+                    _ => None,
+                }
+            })
+            .collect()
+    }
+
+    /// Returns a list of all stake addresses.
+    #[must_use]
+    pub fn stake_addresses(&self) -> HashSet<StakeAddress> {
+        self.x_uris()
+            .values()
+            .chain(self.c_uris().values())
+            .flat_map(|uris| uris.iter())
+            .filter_map(|uri| {
+                match uri.address() {
+                    Address::Stake(a) => Some(a.clone().into()),
+                    _ => None,
+                }
+            })
+            .collect()
+    }
+
     /// Return the updated URIs set.
     ///
     /// The resulting set includes all the data from both the original and a new one. In
@@ -289,18 +322,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 {
 
@@ -323,6 +344,9 @@ mod tests {
         let set = cip509.certificate_uris().unwrap();
         assert!(!set.is_empty());
         assert!(set.c_uris().is_empty());
+        assert_eq!(set.role_addresses(0).len(), 1);
+        assert_eq!(set.role_stake_addresses(0).len(), 1);
+        assert_eq!(set.stake_addresses().len(), 1);
 
         let x_uris = set.x_uris();
         assert_eq!(x_uris.len(), 1);

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 and spendable"
+                    ),
+                    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.role_stake_addresses(0).is_empty() {
+                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

@@ -249,10 +249,10 @@ impl RegistrationChain {
             .and_then(|rdr| rdr.encryption_key_from_rotation(rotation))
     }
 
-    /// Returns a set of role 0 stake addresses.
+    /// Returns all stake addresses associated to this registration.
     #[must_use]
-    pub fn role_0_stake_addresses(&self) -> HashSet<StakeAddress> {
-        self.inner.certificate_uris.stake_addresses(0)
+    pub fn stake_addresses(&self) -> HashSet<StakeAddress> {
+        self.inner.certificate_uris.stake_addresses()
     }
 }