From 84db7b43cbffcdd6dbb6c5761ab6b061352dc9f7 Mon Sep 17 00:00:00 2001 From: Amanjeev Sethi Date: Tue, 21 May 2024 10:44:21 -0400 Subject: [PATCH] Hashes revocation: ability to add content revocation Ticket: https://ferroussystems.clickup.com/t/86947z6fp --- Cargo.lock | 1 + crates/criticaltrust/src/errors.rs | 2 + crates/criticaltrust/src/keys/pair_aws_kms.rs | 2 +- crates/criticaltrust/src/keys/public.rs | 52 +++++++++++++++---- crates/criticaltrust/src/manifests.rs | 17 +++++- .../criticaltrust/src/signatures/payload.rs | 2 +- crates/mock-download-server/Cargo.toml | 1 + crates/mock-download-server/src/handlers.rs | 1 + crates/mock-download-server/src/lib.rs | 11 +++- 9 files changed, 74 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4c33df05..87129b26 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1354,6 +1354,7 @@ dependencies = [ "criticaltrust", "serde", "serde_json", + "time", "tiny_http", ] diff --git a/crates/criticaltrust/src/errors.rs b/crates/criticaltrust/src/errors.rs index 32aa20ab..89d825c2 100644 --- a/crates/criticaltrust/src/errors.rs +++ b/crates/criticaltrust/src/errors.rs @@ -38,4 +38,6 @@ pub enum Error { aws_smithy_runtime_api::client::orchestrator::HttpResponse, >, ), + #[error("failed verification while converting signature to string")] + SignatureConversionFailure, } diff --git a/crates/criticaltrust/src/keys/pair_aws_kms.rs b/crates/criticaltrust/src/keys/pair_aws_kms.rs index c8216c19..78677b33 100644 --- a/crates/criticaltrust/src/keys/pair_aws_kms.rs +++ b/crates/criticaltrust/src/keys/pair_aws_kms.rs @@ -125,7 +125,7 @@ mod tests { let signature = keypair.sign(&payload).expect("failed to sign"); keypair .public() - .verify(KeyRole::Root, &payload, &signature) + .verify(KeyRole::Root, &payload, &signature, None) .expect("failed to verify"); } diff --git a/crates/criticaltrust/src/keys/public.rs b/crates/criticaltrust/src/keys/public.rs index 8f5e9389..bf7daeee 100644 --- a/crates/criticaltrust/src/keys/public.rs +++ b/crates/criticaltrust/src/keys/public.rs @@ -1,8 +1,9 @@ use super::newtypes::SignatureBytes; use crate::keys::newtypes::{PayloadBytes, PublicKeyBytes}; use crate::keys::KeyAlgorithm; +use crate::manifests::RevocationInfo; use crate::sha256::hash_sha256; -use crate::signatures::{PublicKeysRepository, Signable}; +use crate::signatures::{PublicKeysRepository, Signable, SignedPayload}; use crate::Error; use serde::{Deserialize, Serialize}; use time::OffsetDateTime; @@ -31,7 +32,30 @@ impl PublicKey { role: KeyRole, payload: &PayloadBytes<'_>, signature: &SignatureBytes<'_>, + signed_revocation_info: Option>, ) -> 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)?; + + if OffsetDateTime::now_utc() > verified_revoked_content.expires_at { + return Err(Error::VerificationFailed); + } + + let signature_as_string = String::from_utf8(signature.as_bytes().to_vec()) + .map_err(|_err| Error::SignatureConversionFailure)?; + if verified_revoked_content + .revoked_content_sha256 + .contains(&signature_as_string) + { + return Err(Error::VerificationFailed); + } + } + if role != self.role || role == KeyRole::Unknown { return Err(Error::VerificationFailed); } @@ -83,6 +107,8 @@ pub enum KeyRole { Packages, /// `redirects` key role, used to sign dynamic server redirects. Redirects, + /// `revocation` key role, used to sign revoked content hashes. + Revocation, /// `root` key role, used to sign other keys. Root, #[serde(other)] @@ -116,7 +142,7 @@ mod tests { assert!(key .public() - .verify(KeyRole::Root, &SAMPLE_PAYLOAD, &signature) + .verify(KeyRole::Root, &SAMPLE_PAYLOAD, &signature, None) .is_ok()) } @@ -127,7 +153,7 @@ mod tests { assert!(key .public() - .verify(KeyRole::Root, &SAMPLE_PAYLOAD, &signature) + .verify(KeyRole::Root, &SAMPLE_PAYLOAD, &signature, None) .is_ok()); } @@ -138,7 +164,7 @@ mod tests { assert!(matches!( key.public() - .verify(KeyRole::Packages, &SAMPLE_PAYLOAD, &signature), + .verify(KeyRole::Packages, &SAMPLE_PAYLOAD, &signature, None), Err(Error::VerificationFailed) )); } @@ -150,7 +176,7 @@ mod tests { assert!(matches!( key.public() - .verify(KeyRole::Unknown, &SAMPLE_PAYLOAD, &signature), + .verify(KeyRole::Unknown, &SAMPLE_PAYLOAD, &signature, None), Err(Error::VerificationFailed) )); } @@ -162,7 +188,7 @@ mod tests { assert!(matches!( key.public() - .verify(KeyRole::Root, &SAMPLE_PAYLOAD, &signature), + .verify(KeyRole::Root, &SAMPLE_PAYLOAD, &signature, None), Err(Error::VerificationFailed) )); } @@ -179,7 +205,8 @@ mod tests { key.public().verify( KeyRole::Root, &SAMPLE_PAYLOAD, - &SignatureBytes::owned(bad_signature) + &SignatureBytes::owned(bad_signature), + None ), Err(Error::VerificationFailed) )); @@ -194,7 +221,8 @@ mod tests { key.public().verify( KeyRole::Root, &PayloadBytes::borrowed("Hello world!".as_bytes()), - &signature + &signature, + None ), Err(Error::VerificationFailed) )); @@ -208,7 +236,8 @@ mod tests { key.public().verify( KeyRole::Root, &SAMPLE_PAYLOAD, - &SignatureBytes::borrowed(&[]) + &SignatureBytes::borrowed(&[]), + None ), Err(Error::VerificationFailed) )); @@ -223,7 +252,7 @@ mod tests { assert!(matches!( key2.public() - .verify(KeyRole::Root, &SAMPLE_PAYLOAD, &signature), + .verify(KeyRole::Root, &SAMPLE_PAYLOAD, &signature, None), Err(Error::VerificationFailed) )); } @@ -237,7 +266,7 @@ mod tests { public.algorithm = KeyAlgorithm::Unknown; assert!(matches!( - public.verify(KeyRole::Root, &SAMPLE_PAYLOAD, &signature), + public.verify(KeyRole::Root, &SAMPLE_PAYLOAD, &signature, None), Err(Error::UnsupportedKey) )); } @@ -293,6 +322,7 @@ mod tests { KeyRole::Root, &SAMPLE_PAYLOAD, &SignatureBytes::owned(base64_decode(SAMPLE_SIGNATURE).unwrap()), + None, ) .unwrap(); } diff --git a/crates/criticaltrust/src/manifests.rs b/crates/criticaltrust/src/manifests.rs index 26f4fddf..43795d44 100644 --- a/crates/criticaltrust/src/manifests.rs +++ b/crates/criticaltrust/src/manifests.rs @@ -4,6 +4,7 @@ use crate::keys::{KeyRole, PublicKey}; use crate::signatures::{Signable, SignedPayload}; use serde::de::Error as _; use serde::{Deserialize, Serialize}; +use time::OffsetDateTime; /// Typed representation of a manifest version number. /// @@ -151,12 +152,26 @@ pub struct PackageFile { pub needs_proxy: bool, } +// Revocations + +#[derive(Clone, Debug, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub struct RevocationInfo { + pub revoked_content_sha256: Vec, + pub expires_at: OffsetDateTime, +} + +impl Signable for RevocationInfo { + const SIGNED_BY_ROLE: KeyRole = KeyRole::Revocation; +} + // Keys -#[derive(Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize)] pub struct KeysManifest { pub version: ManifestVersion<1>, pub keys: Vec>, + pub revoked_signatures: Option>, } #[cfg(test)] diff --git a/crates/criticaltrust/src/signatures/payload.rs b/crates/criticaltrust/src/signatures/payload.rs index febef7fc..cdeb46df 100644 --- a/crates/criticaltrust/src/signatures/payload.rs +++ b/crates/criticaltrust/src/signatures/payload.rs @@ -106,7 +106,7 @@ fn verify_signature( None => continue, }; - match key.verify(T::SIGNED_BY_ROLE, &signed, &signature.signature) { + match key.verify(T::SIGNED_BY_ROLE, &signed, &signature.signature, None) { Ok(()) => {} Err(Error::VerificationFailed) => continue, Err(other) => return Err(other), diff --git a/crates/mock-download-server/Cargo.toml b/crates/mock-download-server/Cargo.toml index e667690c..a96847ef 100644 --- a/crates/mock-download-server/Cargo.toml +++ b/crates/mock-download-server/Cargo.toml @@ -9,3 +9,4 @@ criticaltrust = { path = "../criticaltrust" } serde = { version = "1.0.136", features = ["derive"] } serde_json = "1.0.79" tiny_http = { version = "0.12.0", default-features = false, features = ["rustls"] } +time = { version = "0.3.7", features = ["std", "serde", "serde-well-known"] } \ No newline at end of file diff --git a/crates/mock-download-server/src/handlers.rs b/crates/mock-download-server/src/handlers.rs index ba90d742..ea21499c 100644 --- a/crates/mock-download-server/src/handlers.rs +++ b/crates/mock-download-server/src/handlers.rs @@ -36,6 +36,7 @@ fn handle_v1_keys(data: &Data) -> Result { Ok(Resp::json(&criticaltrust::manifests::KeysManifest { version: ManifestVersion, keys: data.keys.clone(), + revoked_signatures: data.revoked_signatures.clone(), })) } diff --git a/crates/mock-download-server/src/lib.rs b/crates/mock-download-server/src/lib.rs index 7c737230..93857bf4 100644 --- a/crates/mock-download-server/src/lib.rs +++ b/crates/mock-download-server/src/lib.rs @@ -3,11 +3,12 @@ mod server; pub use crate::server::MockServer; use criticaltrust::keys::PublicKey; -use criticaltrust::manifests::ReleaseManifest; +use criticaltrust::manifests::{ReleaseManifest, RevocationInfo}; use criticaltrust::signatures::SignedPayload; use serde::Serialize; use std::borrow::Cow; use std::collections::HashMap; +use time::OffsetDateTime; #[derive(Serialize, Clone)] #[serde(rename_all = "kebab-case")] @@ -20,6 +21,7 @@ pub struct AuthenticationToken { pub struct Data { pub tokens: HashMap, pub keys: Vec>, + pub revoked_signatures: Option>, pub release_manifests: HashMap<(String, String), ReleaseManifest>, } @@ -28,6 +30,13 @@ 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(), + ), release_manifests: HashMap::new(), }, }