Add/fix certificates/role checks

Author: stanislav-tkach

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

@@ -35,7 +35,7 @@ use crate::{
             types::{PaymentHistory, RoleNumber, TxInputHash, ValidationSignature},
             utils::Cip0134UriSet,
             validation::{
-                validate_aux, validate_role_signing_key, validate_stake_public_key,
+                validate_aux, validate_role_data, validate_stake_public_key,
                 validate_txn_inputs_hash,
             },
             x509_chunks::X509Chunks,
@@ -161,10 +161,12 @@ impl Cip509 {
             txn.transaction_body.auxiliary_data_hash.as_ref(),
             &cip509.report,
         );
-        // The following checks are only performed for the role 0.
-        if let Some(role_data) = cip509.role_data(RoleNumber::ROLE_0) {
+        if cip509.role_data(RoleNumber::ROLE_0).is_some() {
+            // The following check is only performed for the role 0.
             validate_stake_public_key(txn, cip509.certificate_uris(), &cip509.report);
-            validate_role_signing_key(role_data, cip509.metadata.as_ref(), &cip509.report);
+        }
+        if let Some(metadata) = &cip509.metadata {
+            validate_role_data(metadata, &cip509.report);
         }
 
         Ok(Some(cip509))

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

@@ -41,8 +41,8 @@ pub struct Cip509RbacMetadata {
     pub certificate_uris: Cip0134UriSet,
     /// A list of public keys that can be used instead of storing full certificates.
     ///
-    /// Check [this section] to understand the how certificates and the public keys list
-    /// are related.
+    /// Check [this section] to understand how certificates and the public keys list are
+    /// related.
     ///
     /// [this section]: https://github.com/input-output-hk/catalyst-CIPs/tree/x509-role-registration-metadata/CIP-XXXX#storing-certificates-and-public-key
     pub pub_keys: Vec<SimplePublicKeyType>,

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

@@ -38,14 +38,15 @@ use pallas::{
 
 use super::utils::cip19::compare_key_hash;
 use crate::cardano::cip509::{
-    rbac::Cip509RbacMetadata, types::TxInputHash, Cip0134UriSet, KeyLocalRef, LocalRefInt, RoleData,
+    rbac::Cip509RbacMetadata, types::TxInputHash, C509Cert, Cip0134UriSet, LocalRefInt, RoleData,
+    RoleNumber, X509DerCert,
 };
 
 /// Context-specific primitive type with tag number 6 (`raw_tag` 134) for
 /// uniform resource identifier (URI) in the subject alternative name extension.
 /// Following the ASN.1
 /// <https://www.oss.com/asn1/resources/asn1-made-simple/asn1-quick-reference/asn1-tags.html>
-/// the tag is derive from
+/// the tag is derived from
 /// | Class   (2 bit)    | P/C  (1 bit)   | Tag Number (5 bit) |
 /// |`CONTEXT_SPECIFIC`  | `PRIMITIVE`   `|      6`            |
 /// |`10`                | `0`           `|      00110`        |
@@ -174,45 +175,77 @@ fn extract_stake_addresses(uris: Option<&Cip0134UriSet>) -> Vec<VKeyHash> {
         .collect()
 }
 
-/// Validate role singing key for role 0.
-/// Must reference certificate not the public key
-pub fn validate_role_signing_key(
-    role_data: &RoleData, metadata: Option<&Cip509RbacMetadata>, report: &ProblemReport,
-) {
-    let context = "Cip509 role0 signing key validation";
+/// Checks that only role 0 uses certificates with zero index.
+pub fn validate_role_data(metadata: &Cip509RbacMetadata, report: &ProblemReport) {
+    let context = "Role data validation";
+
+    if matches!(
+        metadata.c509_certs.first(),
+        Some(C509Cert::C509Certificate(_))
+    ) && matches!(metadata.x509_certs.first(), Some(X509DerCert::X509Cert(_)))
+    {
+        report.other("Only one certificate can be defined at index 0", context);
+    }
 
-    let Some(signing_key) = role_data.signing_key() else {
-        report.missing_field("RoleData::signing_key", context);
+    for (number, data) in &metadata.role_data {
+        if number == &RoleNumber::ROLE_0 {
+            validate_role_0(data, metadata, context, report);
+        } else {
+            let Some(signing_key) = data.signing_key() else {
+                // It is ok for other roles to not have a signing key.
+                continue;
+            };
+            if signing_key.key_offset == 0 {
+                report.other(
+                    &format!(
+                        "Only role 0 can reference a certificate with 0 index ({number:?} {data:?})"
+                    ),
+                    context,
+                );
+            }
+        }
+    }
+}
+
+/// Checks that the role 0 data is correct.
+fn validate_role_0(
+    role: &RoleData, metadata: &Cip509RbacMetadata, context: &str, report: &ProblemReport,
+) {
+    let Some(signing_key) = role.signing_key() else {
+        report.missing_field("(Role 0) RoleData::signing_key", context);
         return;
     };
 
-    let Some(metadata) = metadata else {
-        report.other("Missing metadata", context);
+    if signing_key.key_offset != 0 {
+        report.other(
+            &format!("The role 0 must reference a certificate with 0 index ({role:?})"),
+            context,
+        );
         return;
-    };
+    }
 
     match signing_key.local_ref {
         LocalRefInt::X509Certs => {
-            check_key_offset(
-                signing_key,
-                metadata.x509_certs.as_slice(),
-                "X509",
-                context,
-                report,
-            );
+            match metadata.x509_certs.first() {
+                Some(X509DerCert::X509Cert(_)) => {
+                    // All good: role 0 references a valid X509 certificate.
+                }
+                Some(c) => report.other(&format!("Invalid X509 certificate value ({c:?}) for role 0 ({role:?})"), context),
+                None => report.other("Role 0 reference X509 certificate at index 0, but there is no such certificate", context),
+            }
         },
         LocalRefInt::C509Certs => {
-            check_key_offset(
-                signing_key,
-                metadata.c509_certs.as_slice(),
-                "C509",
-                context,
-                report,
-            );
+            match metadata.c509_certs.first() {
+                Some(C509Cert::C509Certificate(_)) => {
+                    // All good: role 0 references a valid C509 certificate.
+                }
+                Some(c) => report.other(&format!("Invalid C509 certificate value ({c:?}) for role 0 ({role:?})"), context),
+                None => report.other("Role 0 reference C509 certificate at index 0, but there is no such certificate", context),
+            }
         },
         LocalRefInt::PubKeys => {
             report.invalid_value(
-                "RoleData::signing_key",
+                "(Role 0) RoleData::signing_key",
                 &format!("{signing_key:?}"),
                 "Role signing key should reference certificate, not public key",
                 context,
@@ -221,31 +254,6 @@ pub fn validate_role_signing_key(
     }
 }
 
-/// Add a problem report entry if the key offset is invalid.
-fn check_key_offset<T>(
-    key: &KeyLocalRef, certificates: &[T], certificate_type: &str, context: &str,
-    report: &ProblemReport,
-) {
-    let Ok(offset) = usize::try_from(key.key_offset) else {
-        report.invalid_value(
-            "RoleData::signing_key",
-            &format!("{key:?}"),
-            "Role signing key offset is too big",
-            context,
-        );
-        return;
-    };
-
-    if offset >= certificates.len() {
-        report.invalid_value(
-            "RoleData::signing_key",
-            &format!("{key:?}"),
-            &format!("Role signing key should reference existing certificate, but there are only {} {} certificates in this registration", certificates.len(), certificate_type),
-            context,
-        );
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

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

@@ -1,4 +1,3 @@
 //! Cardano module
-
 pub mod cip509;
 pub mod transaction;

rust/rbac-registration/src/cardano/transaction/raw_aux_data.rs

@@ -110,7 +110,7 @@ impl RawAuxData {
 
             let _unused = raw_decoded_data.metadata.insert(key, Arc::new(value));
 
-            // Look for End Sentinel IF its an indefinite MAP (which we know because entries is
+            // Look for End Sentinel IF it is an indefinite MAP (which we know because entries is
             // u64::MAX).
             if entries == u64::MAX {
                 match decoder.datatype() {