fix(rust/rbac-registration): Validate validation_signature with the latest signing public key of Role 0 (#312)

* bump to the latest mithril-client version 0.12.0

* wip

* fix

* remove commented out code

* fix

* fix comments

* fix(cardano-blockchain-types): add func to get raw aux data from txn index

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

* fix(catalyst-types): add zero out last n bytes func

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

* fix(rbac-registration): add chain root check, should have role 0

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

* fix(rbac-registration): validation sig try into ed25519 sig

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

* fix(rbac-registration): add aux data to rbac test data

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

* fix(rbac-registration): update the registration chain

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

* fix(rbac-registration): store raw aux in cip509

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

* revert rebase change

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

* fix(cardano-blockchain-types): revert change

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

* fix(rbac-registration): add problem report

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

---------

Signed-off-by: bkioshn <[email protected]>
Co-authored-by: Mr-Leshiy <[email protected]>
Co-authored-by: Steven Johnson <[email protected]>

Author: 3 people

rust/catalyst-types/src/conversion.rs

@@ -68,3 +68,13 @@ pub fn vkey_from_bytes(bytes: &[u8]) -> Result<ed25519_dalek::VerifyingKey, VKey
     ed25519_dalek::VerifyingKey::from_bytes(&ed25519)
         .map_err(|source| VKeyFromBytesError::ParseError { source })
 }
+
+/// Zero out the last n bytes
+#[must_use]
+pub fn zero_out_last_n_bytes(data: &[u8], n: usize) -> Vec<u8> {
+    let mut vec = data.to_vec();
+    if let Some(slice) = vec.get_mut(data.len().saturating_sub(n)..) {
+        slice.fill(0);
+    }
+    vec
+}

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

@@ -87,6 +87,8 @@ pub struct Cip509 {
     ///
     /// This field is only present in role 0 registrations.
     catalyst_id: Option<CatalystId>,
+    /// Raw aux data associated with the transaction that CIP509 is attached to,
+    raw_aux_data: Vec<u8>,
     /// A report potentially containing all the issues occurred during `Cip509` decoding
     /// and validation.
     ///
@@ -136,6 +138,7 @@ impl Cip509 {
             Nullable::Some(v) => v.raw_cbor(),
             _ => return Ok(None),
         };
+
         let Some(metadata) = block.txn_metadata(index, MetadatumLabel::CIP509_RBAC) else {
             return Ok(None);
         };
@@ -153,7 +156,17 @@ impl Cip509 {
         let mut cip509 =
             Cip509::decode(&mut decoder, &mut decode_context).context("Failed to decode Cip509")?;
 
+        cip509.raw_aux_data = raw_aux_data.to_vec();
+
         // Perform the validation.
+
+        // Chain root (no previous transaction ID) must contain Role 0
+        if cip509.previous_transaction().is_none() && cip509.role_data(RoleId::Role0).is_none() {
+            cip509
+                .report
+                .missing_field("Chain root role data", "Missing Role 0");
+        }
+
         if let Some(txn_inputs_hash) = &cip509.txn_inputs_hash {
             validate_txn_inputs_hash(txn_inputs_hash, txn, &cip509.report);
         };
@@ -271,6 +284,18 @@ impl Cip509 {
             .unwrap_or_default()
     }
 
+    /// Return validation signature.
+    #[must_use]
+    pub fn validation_signature(&self) -> Option<&ValidationSignature> {
+        self.validation_signature.as_ref()
+    }
+
+    /// Raw aux data associated with the transaction that CIP509 is attached to,
+    #[must_use]
+    pub fn raw_aux_data(&self) -> &[u8] {
+        self.raw_aux_data.as_ref()
+    }
+
     /// Returns `Cip509` fields consuming the structure if it was successfully decoded and
     /// validated otherwise return the problem report that contains all the encountered
     /// issues.
@@ -411,6 +436,7 @@ impl Decode<'_, DecodeContext<'_, '_>> for Cip509 {
             txn_hash,
             origin: decode_context.origin.clone(),
             catalyst_id: None,
+            raw_aux_data: Vec::new(),
             report: decode_context.report.clone(),
         })
     }

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

@@ -1,6 +1,7 @@
 //! A validation signature wrapper.
 
 use anyhow::{anyhow, Error};
+use ed25519_dalek::Signature;
 
 /// A validation signature.
 ///
@@ -20,6 +21,18 @@ impl TryFrom<Vec<u8>> for ValidationSignature {
     }
 }
 
+impl TryInto<Signature> for ValidationSignature {
+    type Error = Error;
+
+    fn try_into(self) -> Result<Signature, Self::Error> {
+        let sig_bytes: [u8; 64] = self
+            .0
+            .try_into()
+            .map_err(|_| anyhow!("Invalid Ed25519 signature length, expect 64"))?;
+        Ok(Signature::from_bytes(&sig_bytes))
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

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

@@ -7,14 +7,16 @@ use std::{
     sync::Arc,
 };
 
-use anyhow::bail;
+use anyhow::{bail, Context};
 use c509_certificate::c509::C509;
 use cardano_blockchain_types::{StakeAddress, TransactionId};
 use catalyst_types::{
     catalyst_id::{key_rotation::KeyRotation, role_index::RoleId, CatalystId},
+    conversion::zero_out_last_n_bytes,
+    problem_report::ProblemReport,
     uuid::UuidV4,
 };
-use ed25519_dalek::VerifyingKey;
+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,
@@ -23,7 +25,7 @@ use x509_cert::certificate::Certificate as X509Certificate;
 
 use crate::cardano::cip509::{
     CertKeyHash, CertOrPk, Cip0134UriSet, Cip509, PaymentHistory, PointData, RoleData,
-    RoleDataRecord,
+    RoleDataRecord, ValidationSignature,
 };
 
 /// Registration chains.
@@ -43,7 +45,7 @@ impl RegistrationChain {
     /// # Errors
     ///
     /// Returns an error if data is invalid
-    pub fn new(cip509: Cip509) -> anyhow::Result<Self> {
+    pub fn new(cip509: &Cip509) -> anyhow::Result<Self> {
         let inner = RegistrationChainInner::new(cip509)?;
 
         Ok(Self {
@@ -59,9 +61,17 @@ impl RegistrationChain {
     /// # Errors
     ///
     /// Returns an error if data is invalid
-    pub fn update(&self, cip509: Cip509) -> anyhow::Result<Self> {
-        let new_inner = self.inner.update(cip509)?;
-
+    pub fn update(&self, cip509: &Cip509) -> anyhow::Result<Self> {
+        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)?
+        } else {
+            cip509.report().missing_field(
+                "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")
+        };
         Ok(Self {
             inner: Arc::new(new_inner),
         })
@@ -264,18 +274,25 @@ impl RegistrationChainInner {
     /// # Errors
     ///
     /// Returns an error if data is invalid
-    fn new(cip509: Cip509) -> anyhow::Result<Self> {
+    fn new(cip509: &Cip509) -> anyhow::Result<Self> {
+        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.");
         };
 
         let point_tx_idx = cip509.origin().clone();
         let current_tx_id_hash = cip509.txn_hash();
-        let (purpose, registration, payment_history) = match cip509.consume() {
+        let validation_signature = cip509.validation_signature().cloned();
+        let raw_aux_data = cip509.raw_aux_data().to_vec();
+        let (purpose, registration, payment_history) = match cip509.clone().consume() {
             Ok(v) => v,
             Err(e) => {
                 let error = format!("Invalid Cip509: {e:?}");
@@ -284,6 +301,42 @@ impl RegistrationChainInner {
             },
         };
 
+        // Role data
+        let mut role_data_history = HashMap::new();
+        let mut role_data_record = HashMap::new();
+
+        update_role_data(
+            &registration,
+            &mut role_data_history,
+            &mut role_data_record,
+            &point_tx_idx,
+        );
+
+        // There should be role 0 since we already check that the chain root (no previous tx id)
+        // 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");
+        };
+        let Some(signing_pk) = role0_data
+            .signing_keys()
+            .last()
+            .and_then(|key| key.data().extract_pk())
+        else {
+            cip509
+                .report()
+                .missing_field("Signing pk for role 0 not found", context);
+            bail!("No valid signing key found for role 0");
+        };
+
+        check_validation_signature(
+            validation_signature,
+            &raw_aux_data,
+            signing_pk,
+            cip509.report(),
+            context,
+        )?;
+
         let purpose = vec![purpose];
         let certificate_uris = registration.certificate_uris.clone();
         let mut x509_certs = HashMap::new();
@@ -306,17 +359,6 @@ impl RegistrationChainInner {
         );
         let revocations = revocations_list(registration.revocation_list.clone(), &point_tx_idx);
 
-        // Role data
-        let mut role_data_history = HashMap::new();
-        let mut role_data_record = HashMap::new();
-
-        update_role_data(
-            &registration,
-            &mut role_data_history,
-            &mut role_data_record,
-            &point_tx_idx,
-        );
-
         Ok(Self {
             catalyst_id,
             current_tx_id_hash,
@@ -340,23 +382,43 @@ impl RegistrationChainInner {
     /// # Errors
     ///
     /// Returns an error if data is invalid
-    fn update(&self, cip509: Cip509) -> anyhow::Result<Self> {
+    fn update(&self, cip509: &Cip509, signing_pk: VerifyingKey) -> anyhow::Result<Self> {
+        let context = "Registration Chain update";
         let mut new_inner = self.clone();
 
         let Some(prv_tx_id) = cip509.previous_transaction() else {
-            bail!("Empty previous transaction ID");
+            cip509
+                .report()
+                .missing_field("previous transaction ID", context);
+            bail!("Missing previous transaction ID");
         };
+
         // Previous transaction ID in the CIP509 should equal to the current transaction ID
-        // or else it is not a part of the chain
         if prv_tx_id == self.current_tx_id_hash {
-            // Update the current transaction ID hash
+            // Perform signature validation
+            // This should be done before updating the signing key
+            check_validation_signature(
+                cip509.validation_signature().cloned(),
+                cip509.raw_aux_data(),
+                signing_pk,
+                cip509.report(),
+                context,
+            )?;
+
+            // If successful, update the chain current transaction ID hash
             new_inner.current_tx_id_hash = cip509.txn_hash();
         } else {
+            cip509.report().invalid_value(
+                "previous transaction ID",
+                &format!("{prv_tx_id:?}"),
+                &format!("{:?}", self.current_tx_id_hash),
+                context,
+            );
             bail!("Invalid previous transaction ID, not a part of this registration chain");
         }
 
         let point_tx_idx = cip509.origin().clone();
-        let (purpose, registration, payment_history) = match cip509.consume() {
+        let (purpose, registration, payment_history) = match cip509.clone().consume() {
             Ok(v) => v,
             Err(e) => {
                 let error = format!("Invalid Cip509: {e:?}");
@@ -403,6 +465,41 @@ impl RegistrationChainInner {
     }
 }
 
+/// Perform a check on the validation signature.
+/// The auxiliary data should be sign with the latest signing public key.
+fn check_validation_signature(
+    validation_signature: Option<ValidationSignature>, raw_aux_data: &[u8],
+    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(|| {
+        report.missing_field("validation signature", context);
+        "Missing validation signature"
+    })?;
+
+    let sig: Signature = validation_sig.clone().try_into().with_context(|| {
+        report.conversion_error(
+            "validation signature",
+            &format!("{validation_sig:?}"),
+            "Ed25519 signature",
+            context,
+        );
+        "Failed to convert validation signature to Ed25519 Signature"
+    })?;
+
+    // Verify the signature using the latest signing public key
+    signing_pk
+        .verify_strict(&unsigned_aux, &sig)
+        .with_context(|| {
+            report.other("Signature validation failed", context);
+            "Signature verification failed"
+        })
+}
+
 #[cfg(test)]
 mod test {
     use catalyst_types::catalyst_id::role_index::RoleId;
@@ -419,7 +516,7 @@ mod test {
         data.assert_valid(&registration);
 
         // Create a chain with the first registration.
-        let chain = RegistrationChain::new(registration).unwrap();
+        let chain = RegistrationChain::new(&registration).unwrap();
         assert_eq!(chain.purpose(), &[data.purpose]);
         assert_eq!(1, chain.x509_certs().len());
         let origin = &chain.x509_certs().get(&0).unwrap().first().unwrap();
@@ -445,7 +542,7 @@ mod test {
             .unwrap();
         assert!(registration.report().is_problematic());
 
-        let error = chain.update(registration).unwrap_err();
+        let error = chain.update(&registration).unwrap_err();
         let error = format!("{error:?}");
         assert!(
             error.contains("Invalid previous transaction ID"),
@@ -459,7 +556,7 @@ mod test {
             .unwrap()
             .unwrap();
         data.assert_valid(&registration);
-        let update = chain.update(registration).unwrap();
+        let update = chain.update(&registration).unwrap();
         // Current tx hash should be equal to the hash from block 4.
         assert_eq!(update.current_tx_id_hash(), data.txn_hash);
         assert!(update.role_data_record().contains_key(&data.role));

rust/rbac-registration/src/utils/test.rs

@@ -169,7 +169,7 @@ pub fn block_4() -> BlockTestData {
     BlockTestData {
         block: block(data),
         slot: 82_004_569.into(),
-        role: RoleId::Role0,
+        role: 4.into(),
         txn_index: 1.into(),
         txn_hash: "eef40a97a4ed1e40c3febd05a84b3ffaa191141b60806c2bba85d9c6879fb378"
             .parse()