Hashes revocation review: RevocationInfo not to be optional

Reviewed-by: Pietro Albini <[email protected]>

Ticket: https://ferroussystems.clickup.com/t/86947z6fp

Author: Amanjeev Sethi

crates/criticaltrust/src/keys/pair_aws_kms.rs

@@ -109,6 +109,7 @@ mod tests {
     // "localstack", a local replica of AWS services meant for testing.
 
     #[test]
+    #[ignore = "This test will be enabled in a later release."]
     fn test_roundtrip() {
         let localstack = Localstack::init();
         let key = localstack.create_key(KeySpec::EccNistP256);
@@ -125,7 +126,7 @@ mod tests {
         let signature = keypair.sign(&payload).expect("failed to sign");
         keypair
             .public()
-            .verify(KeyRole::Root, &payload, &signature, None)
+            .verify_without_checking_revocations(KeyRole::Root, &payload, &signature)
             .expect("failed to verify");
     }
 

crates/criticaltrust/src/keys/public.rs

@@ -36,30 +36,27 @@ impl PublicKey {
         role: KeyRole,
         payload: &PayloadBytes<'_>,
         signature: &SignatureBytes<'_>,
-        signed_revocation_info: Option<SignedPayload<RevocationInfo>>,
+        signed_revocation_info: SignedPayload<RevocationInfo>,
     ) -> Result<(), Error> {
         // We need to check if there is revoked content. If the following checks pass, then bail out
         // early with an error.
         //  1. Check if the expiration date has passed.
         //  2. Check whether the `signature` is inside the vector
         //     `RevocationInfo.revoked_content_sha256`.
-        if let Some(revoked_hashes) = signed_revocation_info {
-            let verified_revoked_content = revoked_hashes.get_verified(self)?;
-
-            let expiration_in_days =
-                (verified_revoked_content.expires_at - OffsetDateTime::now_utc()).whole_days();
-            if expiration_in_days < MAX_REVOCATION_INFO_EXPIRATION_DURATION {
-                return Err(Error::VerificationFailed);
-            }
+        let verified_revoked_content = signed_revocation_info.get_verified(self)?;
+        let expiration_in_days =
+            (verified_revoked_content.expires_at - OffsetDateTime::now_utc()).whole_days();
+        if expiration_in_days < MAX_REVOCATION_INFO_EXPIRATION_DURATION {
+            return Err(Error::VerificationFailed);
+        }
 
-            let based_signature =
-                base64::engine::general_purpose::STANDARD.encode(signature.as_bytes());
-            if verified_revoked_content
-                .revoked_content_sha256
-                .contains(&based_signature)
-            {
-                return Err(Error::VerificationFailed);
-            }
+        let based_signature =
+            base64::engine::general_purpose::STANDARD.encode(signature.as_bytes());
+        if verified_revoked_content
+            .revoked_content_sha256
+            .contains(&based_signature)
+        {
+            return Err(Error::VerificationFailed);
         }
 
         self.verify_without_checking_revocations(role, payload, signature)?;
@@ -166,7 +163,7 @@ mod tests {
 
         assert!(key
             .public()
-            .verify(KeyRole::Root, &SAMPLE_PAYLOAD, &signature, None)
+            .verify_without_checking_revocations(KeyRole::Root, &SAMPLE_PAYLOAD, &signature)
             .is_ok())
     }
 
@@ -177,7 +174,7 @@ mod tests {
 
         assert!(key
             .public()
-            .verify(KeyRole::Root, &SAMPLE_PAYLOAD, &signature, None)
+            .verify_without_checking_revocations(KeyRole::Root, &SAMPLE_PAYLOAD, &signature)
             .is_ok());
     }
 
@@ -187,8 +184,11 @@ mod tests {
         let signature = key.sign(&SAMPLE_PAYLOAD).unwrap();
 
         assert!(matches!(
-            key.public()
-                .verify(KeyRole::Packages, &SAMPLE_PAYLOAD, &signature, None),
+            key.public().verify_without_checking_revocations(
+                KeyRole::Packages,
+                &SAMPLE_PAYLOAD,
+                &signature
+            ),
             Err(Error::VerificationFailed)
         ));
     }
@@ -199,8 +199,11 @@ mod tests {
         let signature = key.sign(&SAMPLE_PAYLOAD).unwrap();
 
         assert!(matches!(
-            key.public()
-                .verify(KeyRole::Unknown, &SAMPLE_PAYLOAD, &signature, None),
+            key.public().verify_without_checking_revocations(
+                KeyRole::Unknown,
+                &SAMPLE_PAYLOAD,
+                &signature
+            ),
             Err(Error::VerificationFailed)
         ));
     }
@@ -211,8 +214,11 @@ mod tests {
         let signature = key.sign(&SAMPLE_PAYLOAD).unwrap();
 
         assert!(matches!(
-            key.public()
-                .verify(KeyRole::Root, &SAMPLE_PAYLOAD, &signature, None),
+            key.public().verify_without_checking_revocations(
+                KeyRole::Root,
+                &SAMPLE_PAYLOAD,
+                &signature
+            ),
             Err(Error::VerificationFailed)
         ));
     }
@@ -226,11 +232,10 @@ mod tests {
         *bad_signature.last_mut().unwrap() = bad_signature.last().unwrap().wrapping_add(1);
 
         assert!(matches!(
-            key.public().verify(
+            key.public().verify_without_checking_revocations(
                 KeyRole::Root,
                 &SAMPLE_PAYLOAD,
                 &SignatureBytes::owned(bad_signature),
-                None
             ),
             Err(Error::VerificationFailed)
         ));
@@ -242,11 +247,10 @@ mod tests {
         let signature = key.sign(&SAMPLE_PAYLOAD).unwrap();
 
         assert!(matches!(
-            key.public().verify(
+            key.public().verify_without_checking_revocations(
                 KeyRole::Root,
                 &PayloadBytes::borrowed("Hello world!".as_bytes()),
                 &signature,
-                None
             ),
             Err(Error::VerificationFailed)
         ));
@@ -257,11 +261,10 @@ mod tests {
         let key = generate(KeyRole::Root, None);
 
         assert!(matches!(
-            key.public().verify(
+            key.public().verify_without_checking_revocations(
                 KeyRole::Root,
                 &SAMPLE_PAYLOAD,
                 &SignatureBytes::borrowed(&[]),
-                None
             ),
             Err(Error::VerificationFailed)
         ));
@@ -275,8 +278,11 @@ mod tests {
         let signature = key1.sign(&SAMPLE_PAYLOAD).unwrap();
 
         assert!(matches!(
-            key2.public()
-                .verify(KeyRole::Root, &SAMPLE_PAYLOAD, &signature, None),
+            key2.public().verify_without_checking_revocations(
+                KeyRole::Root,
+                &SAMPLE_PAYLOAD,
+                &signature
+            ),
             Err(Error::VerificationFailed)
         ));
     }
@@ -290,7 +296,7 @@ mod tests {
         public.algorithm = KeyAlgorithm::Unknown;
 
         assert!(matches!(
-            public.verify(KeyRole::Root, &SAMPLE_PAYLOAD, &signature, None),
+            public.verify_without_checking_revocations(KeyRole::Root, &SAMPLE_PAYLOAD, &signature),
             Err(Error::UnsupportedKey)
         ));
     }
@@ -342,11 +348,10 @@ mod tests {
         .unwrap();
 
         // Ensure the key can verify messages signed with the corresponding private key.
-        key.verify(
+        key.verify_without_checking_revocations(
             KeyRole::Root,
             &SAMPLE_PAYLOAD,
             &SignatureBytes::owned(base64_decode(SAMPLE_SIGNATURE).unwrap()),
-            None,
         )
         .unwrap();
     }
@@ -465,7 +470,7 @@ mod tests {
                 KeyRole::Root,
                 &SAMPLE_PAYLOAD,
                 &signature,
-                Some(signed_revocation_info)
+                signed_revocation_info
             ),
             Err(Error::VerificationFailed)
         ));
@@ -496,7 +501,7 @@ mod tests {
                 KeyRole::Root,
                 &SAMPLE_PAYLOAD,
                 &signature,
-                Some(signed_revocation_info.clone())
+                signed_revocation_info.clone()
             ),
             Err(Error::VerificationFailed)
         ));
@@ -506,7 +511,7 @@ mod tests {
                 KeyRole::Revocation,
                 &SAMPLE_PAYLOAD,
                 &signature,
-                Some(signed_revocation_info)
+                signed_revocation_info
             ),
             Err(Error::VerificationFailed)
         ));

crates/criticaltrust/src/manifests.rs

@@ -171,7 +171,7 @@ impl Signable for RevocationInfo {
 pub struct KeysManifest {
     pub version: ManifestVersion<1>,
     pub keys: Vec<SignedPayload<PublicKey>>,
-    pub revoked_signatures: Option<SignedPayload<RevocationInfo>>,
+    pub revoked_signatures: SignedPayload<RevocationInfo>,
 }
 
 #[cfg(test)]

crates/mock-download-server/src/lib.rs

@@ -21,7 +21,7 @@ pub struct AuthenticationToken {
 pub struct Data {
     pub tokens: HashMap<String, AuthenticationToken>,
     pub keys: Vec<SignedPayload<PublicKey>>,
-    pub revoked_signatures: Option<SignedPayload<RevocationInfo>>,
+    pub revoked_signatures: SignedPayload<RevocationInfo>,
     pub release_manifests: HashMap<(String, String), ReleaseManifest>,
 }
 
@@ -30,13 +30,11 @@ pub fn new() -> Builder {
         data: Data {
             tokens: HashMap::new(),
             keys: Vec::new(),
-            revoked_signatures: Some(
-                SignedPayload::new(&RevocationInfo {
-                    revoked_content_sha256: Vec::new(),
-                    expires_at: OffsetDateTime::now_utc(),
-                })
-                .unwrap(),
-            ),
+            revoked_signatures: SignedPayload::new(&RevocationInfo {
+                revoked_content_sha256: Vec::new(),
+                expires_at: OffsetDateTime::now_utc(),
+            })
+            .unwrap(),
             release_manifests: HashMap::new(),
         },
     }