From dc989ab413e79fd13c3bb8943feba23c719818c6 Mon Sep 17 00:00:00 2001 From: Mathieu Amiot Date: Wed, 20 Dec 2023 16:11:29 +0100 Subject: [PATCH 01/23] feat: Tighten Send+Sync requirements to allow PkiEnvironment to be Send+Sync as well --- certval/src/environment/pki_environment.rs | 50 ---------------------- 1 file changed, 50 deletions(-) diff --git a/certval/src/environment/pki_environment.rs b/certval/src/environment/pki_environment.rs index fe5ce56..8505df2 100644 --- a/certval/src/environment/pki_environment.rs +++ b/certval/src/environment/pki_environment.rs @@ -74,34 +74,19 @@ pub struct PkiEnvironment { //Storage and retrieval interfaces //-------------------------------------------------------------------------- /// List of trait objects that provide access to trust anchors - #[cfg(feature = "std")] trust_anchor_sources: Vec>, - #[cfg(not(feature = "std"))] - trust_anchor_sources: Vec>, /// List of trait objects that provide access to certificates - #[cfg(feature = "std")] certificate_sources: Vec>, - #[cfg(not(feature = "std"))] - certificate_sources: Vec>, /// List of trait objects that provide access to CRLs - #[cfg(feature = "std")] crl_sources: Vec>, - #[cfg(not(feature = "std"))] - crl_sources: Vec>, /// List of trait objects that provide access to cached revocation status determinations - #[cfg(feature = "std")] revocation_cache: Vec>, - #[cfg(not(feature = "std"))] - revocation_cache: Vec>, /// List of trait objects that provide access to blocklist and last modified info - #[cfg(feature = "std")] check_remote: Vec>, - #[cfg(not(feature = "std"))] - check_remote: Vec>, //-------------------------------------------------------------------------- //Miscellaneous interfaces @@ -283,17 +268,10 @@ impl PkiEnvironment { } /// add_trust_anchor_source adds a [`TrustAnchorSource`] object to the list used by get_trust_anchor. - #[cfg(feature = "std")] pub fn add_trust_anchor_source(&mut self, c: Box<(dyn TrustAnchorSource + Send + Sync)>) { self.trust_anchor_sources.push(c); } - /// add_trust_anchor_source adds a [`TrustAnchorSource`] object to the list used by get_trust_anchor. - #[cfg(not(feature = "std"))] - pub fn add_trust_anchor_source(&mut self, c: Box<(dyn TrustAnchorSource)>) { - self.trust_anchor_sources.push(c); - } - /// clear_trust_anchor_sources clears the list of [`TrustAnchorSource`] objects used by get_trust_anchor. pub fn clear_trust_anchor_sources(&mut self) { self.trust_anchor_sources.clear(); @@ -370,34 +348,20 @@ impl PkiEnvironment { } /// add_certificate_source adds a [`CertificateSource`] object to the list. - #[cfg(feature = "std")] pub fn add_certificate_source(&mut self, c: Box<(dyn CertificateSource + Send + Sync)>) { self.certificate_sources.push(c); } - /// add_certificate_source adds a [`CertificateSource`] object to the list. - #[cfg(not(feature = "std"))] - pub fn add_certificate_source(&mut self, c: Box<(dyn CertificateSource)>) { - self.certificate_sources.push(c); - } - /// clear_certificate_sources clears the list of [`CertificateSource`] objects. pub fn clear_certificate_sources(&mut self) { self.certificate_sources.clear(); } /// add_crl_source adds a [`CrlSource`] object to the list. - #[cfg(feature = "std")] pub fn add_crl_source(&mut self, c: Box<(dyn CrlSource + Send + Sync)>) { self.crl_sources.push(c); } - /// add_crl_source adds a [`CrlSource`] object to the list. - #[cfg(not(feature = "std"))] - pub fn add_crl_source(&mut self, c: Box<(dyn CrlSource)>) { - self.crl_sources.push(c); - } - /// clear_crl_sources clears the list of [`CrlSource`] objects. pub fn clear_crl_sources(&mut self) { self.crl_sources.clear(); @@ -434,17 +398,10 @@ impl PkiEnvironment { } /// add_revocation_cache adds a [`RevocationStatusCache`] object to the list. - #[cfg(feature = "std")] pub fn add_revocation_cache(&mut self, c: Box<(dyn RevocationStatusCache + Send + Sync)>) { self.revocation_cache.push(c); } - /// add_revocation_cache adds a [`RevocationStatusCache`] object to the list. - #[cfg(not(feature = "std"))] - pub fn add_revocation_cache(&mut self, c: Box<(dyn RevocationStatusCache)>) { - self.revocation_cache.push(c); - } - /// clear_revocation_cache clears the list of [`CertificateSource`] objects. pub fn clear_revocation_cache(&mut self) { self.revocation_cache.clear(); @@ -524,17 +481,10 @@ impl PkiEnvironment { } /// add_check_remote adds a [`CheckRemoteResource`] object to the list. - #[cfg(feature = "std")] pub fn add_check_remote(&mut self, c: Box<(dyn CheckRemoteResource + Send + Sync)>) { self.check_remote.push(c); } - /// add_check_remote adds a [`CheckRemoteResource`] object to the list. - #[cfg(not(feature = "std"))] - pub fn add_check_remote(&mut self, c: Box<(dyn CheckRemoteResource)>) { - self.check_remote.push(c); - } - /// clear_check_remote_callbacks clears the list of [`CheckRemoteResource`] objects. pub fn clear_check_remote_callbacks(&mut self) { self.check_remote.clear(); From 130cfee8d7a3391e3e9669e73fa89573fd904439 Mon Sep 17 00:00:00 2001 From: Mathieu Amiot Date: Wed, 20 Dec 2023 18:11:49 +0100 Subject: [PATCH 02/23] fix: Serde stuff --- certval/Cargo.toml | 2 +- certval/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/certval/Cargo.toml b/certval/Cargo.toml index 6aeb2e2..bb324db 100644 --- a/certval/Cargo.toml +++ b/certval/Cargo.toml @@ -83,7 +83,7 @@ tokio-test = "0.4.2" # webpki can be paired with any other feature and simply adds a means of initializing a TaSource from the webpki-roots crate [features] default = ["remote", "webpki"] -revocation = ["ndarray"] +revocation = ["ndarray", "serde/std", "serde/rc"] std = ["ndarray", "tokio", "base64ct", "walkdir", "url", "serde_json", "serde/rc", "flagset/serde", "regex", "lazy_static/spin", "bitvec", "cidr", "der/std"] remote = ["revocation", "std", "reqwest", "lazy_static/spin"] pqc = ["pqcrypto-internals", "pqcrypto-dilithium", "pqcrypto-falcon", "pqcrypto-sphincsplus", "pqcrypto", "pqcrypto-traits", "pqckeys"] diff --git a/certval/src/lib.rs b/certval/src/lib.rs index 0cb13a5..297dec2 100644 --- a/certval/src/lib.rs +++ b/certval/src/lib.rs @@ -2,7 +2,7 @@ #![doc = include_str!("../README.md")] #![forbid(unsafe_code)] #![warn(missing_docs, rust_2018_idioms)] -#![cfg_attr(not(feature = "std"), no_std)] +// #![cfg_attr(not(feature = "std"), no_std)] pub mod asn1; pub mod environment; From 2944b8af037065dfd7d949310e335bc36a83eb8d Mon Sep 17 00:00:00 2001 From: Mathieu Amiot Date: Thu, 21 Dec 2023 14:55:05 +0100 Subject: [PATCH 03/23] Added support for retrieving TAs and Inter CAs --- certval/src/environment/pki_environment.rs | 23 +++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/certval/src/environment/pki_environment.rs b/certval/src/environment/pki_environment.rs index 8505df2..45aa890 100644 --- a/certval/src/environment/pki_environment.rs +++ b/certval/src/environment/pki_environment.rs @@ -40,7 +40,7 @@ use alloc::{vec, vec::Vec}; use der::asn1::ObjectIdentifier; use spki::{AlgorithmIdentifierOwned, SubjectPublicKeyInfoOwned}; -use x509_cert::{certificate::Raw, crl::CertificateList}; +use x509_cert::{certificate::Raw, crl::CertificateList, name::Name}; use crate::PathValidationStatus::RevocationStatusNotDetermined; use crate::{ @@ -327,6 +327,27 @@ impl PkiEnvironment { Err(Error::Unrecognized) } + /// Retrieves a trust anchor for a given Name + pub fn get_trust_anchor_by_name(&'_ self, name: &Name) -> Result<&PDVTrustAnchorChoice> { + for f in &self.trust_anchor_sources { + if let Ok(r) = f.get_trust_anchor_by_name(name) { + return Ok(r); + } + } + + Err(Error::Unrecognized) + } + + /// Retrieves a set of certificates from certificate sources (i.e. intermediate CAs) matching a certain name + pub fn get_cert_by_name(&'_ self, name: &Name) -> Vec<&PDVCertificate> { + self.certificate_sources.iter().fold(vec![], |mut acc, f| { + if let Ok(mut r) = f.get_certificates_for_name(name) { + acc.append(&mut r); + } + acc + }) + } + /// is_cert_a_trust_anchor takes a target certificate indication if cert is a trust anchor. pub fn is_cert_a_trust_anchor(&self, target: &PDVCertificate) -> Result<()> { for f in &self.trust_anchor_sources { From 949a0bf942964581caf9c1582e27d47d1e40d5c6 Mon Sep 17 00:00:00 2001 From: Mathieu Amiot Date: Mon, 22 Jan 2024 14:55:14 +0100 Subject: [PATCH 04/23] fix: Misc fixes for Inter CA source --- certval/src/revocation/crl.rs | 5 ++--- certval/src/source/cert_source.rs | 14 +++++++------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/certval/src/revocation/crl.rs b/certval/src/revocation/crl.rs index 9dab73f..eb95887 100644 --- a/certval/src/revocation/crl.rs +++ b/certval/src/revocation/crl.rs @@ -849,9 +849,8 @@ fn verify_crl( issuer_cert: &CertificateInner, cpr: &mut CertificationPathResults, ) -> Result<()> { - let defer_crl = match DeferDecodeSigned::from_der(crl_buf) { - Ok(crl) => crl, - Err(_e) => return Err(Error::Unrecognized), + let Ok(defer_crl) = DeferDecodeSigned::from_der(crl_buf) else { + return Err(Error::Unrecognized); }; let r = pe.verify_signature_message( diff --git a/certval/src/source/cert_source.rs b/certval/src/source/cert_source.rs index a79041c..db2c87c 100644 --- a/certval/src/source/cert_source.rs +++ b/certval/src/source/cert_source.rs @@ -965,7 +965,7 @@ impl CertSource { /// index_certs prepares internally used key identifier and name maps after the caller has modified /// the buffers_and_paths and certs fields. - fn index_certs(&mut self) { + pub fn index_certs(&mut self) { for (i, cert) in self.certs.iter().enumerate() { if let Some(cert) = cert { let hex_skid = hex_skid_from_cert(cert); @@ -1531,9 +1531,9 @@ impl CertificateSource for CertSource { fn get_certificates_for_skid(&self, skid: &[u8]) -> Result> { let hex_skid = buffer_to_hex(skid); let mut retval = vec![]; - if self.skid_map.contains_key(hex_skid.as_str()) { - for i in &self.skid_map[&hex_skid] { - if let Some(cert) = &self.certs[*i] { + if let Some(skid_map) = self.skid_map.get(hex_skid.as_str()) { + for i in skid_map { + if let Some(Some(cert)) = &self.certs.get(*i) { retval.push(cert); } } @@ -1549,9 +1549,9 @@ impl CertificateSource for CertSource { fn get_certificates_for_name(&self, name: &Name) -> Result> { let name_str = name_to_string(name); let mut retval = vec![]; - if self.name_map.contains_key(name_str.as_str()) { - for i in &self.name_map[&name_str] { - if let Some(cert) = &self.certs[*i] { + if let Some(name_map) = self.name_map.get(name_str.as_str()) { + for i in name_map { + if let Some(Some(cert)) = &self.certs.get(*i) { retval.push(cert); } } From ceef98370d3f79f70e247345103f7e7358b4b1b0 Mon Sep 17 00:00:00 2001 From: Mathieu Amiot Date: Thu, 18 Jan 2024 17:54:17 +0100 Subject: [PATCH 05/23] Added support for id-Ed25519 --- Cargo.lock | 70 ++++++++++++++++++++++++++++++++ Cargo.toml | 2 + certval/Cargo.toml | 3 +- certval/src/util/crypto.rs | 26 +++++++++++- certval/src/util/pdv_alg_oids.rs | 7 ++++ 5 files changed, 105 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4f39e06..5bb3daa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -273,6 +273,7 @@ dependencies = [ "const-oid", "der", "ecdsa", + "ed25519-dalek", "flagset", "hex-literal", "lazy_static", @@ -499,6 +500,31 @@ dependencies = [ "memchr", ] +[[package]] +name = "curve25519-dalek" +version = "4.1.3" +source = "git+https://github.com/dalek-cryptography/curve25519-dalek.git?branch=rustcrypto-new-releases#44508ba8652ae3445608ad3c56b63ef528ddfb93" +dependencies = [ + "cfg-if", + "cpufeatures", + "curve25519-dalek-derive", + "digest", + "fiat-crypto", + "rustc_version", + "subtle", + "zeroize", +] + +[[package]] +name = "curve25519-dalek-derive" +version = "0.1.1" +source = "git+https://github.com/dalek-cryptography/curve25519-dalek.git?branch=rustcrypto-new-releases#44508ba8652ae3445608ad3c56b63ef528ddfb93" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.52", +] + [[package]] name = "der" version = "0.8.0-rc.0" @@ -583,6 +609,29 @@ dependencies = [ "signature", ] +[[package]] +name = "ed25519" +version = "2.3.0-pre.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "62bcc0730fbd27c8619332efad3dfa1de229dc5859a31495ab674e0ac0f9996b" +dependencies = [ + "pkcs8", + "signature", +] + +[[package]] +name = "ed25519-dalek" +version = "2.2.0-pre" +source = "git+https://github.com/dalek-cryptography/curve25519-dalek.git?branch=rustcrypto-new-releases#44508ba8652ae3445608ad3c56b63ef528ddfb93" +dependencies = [ + "curve25519-dalek", + "ed25519", + "serde", + "sha2", + "subtle", + "zeroize", +] + [[package]] name = "elliptic-curve" version = "0.14.0-pre.6" @@ -642,6 +691,12 @@ dependencies = [ "subtle", ] +[[package]] +name = "fiat-crypto" +version = "0.2.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "28dea519a9695b9977216879a3ebfddf92f1c08c05d984f8996aecd6ecdc811d" + [[package]] name = "flagset" version = "0.4.6" @@ -1811,6 +1866,15 @@ version = "0.1.23" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d626bb9dae77e28219937af045c257c28bfd3f69333c512553507f5f9798cb76" +[[package]] +name = "rustc_version" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bfa0f585226d2e68097d4f95d113b15b83a82e819ab25717ec0590d9584ef366" +dependencies = [ + "semver", +] + [[package]] name = "rustix" version = "0.38.31" @@ -1899,6 +1963,12 @@ dependencies = [ "libc", ] +[[package]] +name = "semver" +version = "1.0.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "61697e0a1c7e512e84a621326239844a24d8207b4669b41bc18b32ea5cbf988b" + [[package]] name = "serde" version = "1.0.201" diff --git a/Cargo.toml b/Cargo.toml index cccdece..81ea74e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,8 @@ debug = true cms = { git = "https://github.com/RustCrypto/formats.git" } x509-ocsp = { git = "https://github.com/RustCrypto/formats.git" } x509-cert = { git = "https://github.com/RustCrypto/formats.git" } +# FIXME: https://github.com/dalek-cryptography/curve25519-dalek/pull/676 +ed25519-dalek = { git = "https://github.com/dalek-cryptography/curve25519-dalek.git", branch = "rustcrypto-new-releases" } [patch.'https://github.com/carl-wallace/pqckeys'] pqckeys = { git = "https://github.com/baloo/pqckeys.git", branch = "baloo/pre-releases" } diff --git a/certval/Cargo.toml b/certval/Cargo.toml index bb324db..aa5c206 100644 --- a/certval/Cargo.toml +++ b/certval/Cargo.toml @@ -32,6 +32,7 @@ pkiprocmacros = { path = "../pkiprocmacros"} ecdsa = {version = "0.17.0-pre.7", default-features = false, features = ["der"]} p256 = {version = "0.14.0-pre.1", default-features = false, features = ["ecdsa", "ecdsa-core"]} p384 = {version = "0.14.0-pre.1", default-features = false, features = ["ecdsa"]} +ed25519-dalek = { version = "2.2.0-pre", default-features = false, features = ["fast", "zeroize", "pkcs8"] } rsa = {version = "0.10.0-pre.2", default-features = false} sha1 = {version = "0.11.0-pre.4", default-features = false} sha2 = {version = "0.11.0-pre.4", default-features = false, features = ["oid"] } @@ -84,7 +85,7 @@ tokio-test = "0.4.2" [features] default = ["remote", "webpki"] revocation = ["ndarray", "serde/std", "serde/rc"] -std = ["ndarray", "tokio", "base64ct", "walkdir", "url", "serde_json", "serde/rc", "flagset/serde", "regex", "lazy_static/spin", "bitvec", "cidr", "der/std"] +std = ["ndarray", "tokio", "base64ct", "walkdir", "ed25519-dalek/std", "url", "serde_json", "serde/rc", "flagset/serde", "regex", "lazy_static/spin", "bitvec", "cidr", "der/std"] remote = ["revocation", "std", "reqwest", "lazy_static/spin"] pqc = ["pqcrypto-internals", "pqcrypto-dilithium", "pqcrypto-falcon", "pqcrypto-sphincsplus", "pqcrypto", "pqcrypto-traits", "pqckeys"] webpki = ["webpki-roots"] diff --git a/certval/src/util/crypto.rs b/certval/src/util/crypto.rs index a68d0c7..981ebff 100644 --- a/certval/src/util/crypto.rs +++ b/certval/src/util/crypto.rs @@ -69,6 +69,10 @@ pub(crate) fn is_ecdsa(oid: &ObjectIdentifier) -> bool { || *oid == PKIXALG_ECDSA_WITH_SHA512 } +pub(crate) fn is_eddsa(oid: &ObjectIdentifier) -> bool { + *oid == PKIXALG_ED25519 +} + #[cfg(feature = "pqc")] pub(crate) fn is_ml_dsa_44_ipd(oid: &ObjectIdentifier) -> bool { *oid == ML_DSA_44_IPD @@ -257,8 +261,8 @@ pub fn verify_signature_message_rust_crypto( }; macro_rules! verify_with_ecdsa { ($crypto_root:ident) => {{ - use ::ecdsa::signature::hazmat::PrehashVerifier; - use ::$crypto_root::ecdsa; + use ecdsa::signature::hazmat::PrehashVerifier; + use $crypto_root::ecdsa; let verifying_key = ecdsa::VerifyingKey::from_sec1_bytes(spki.subject_public_key.raw_bytes()) .map_err(|_err| { @@ -289,7 +293,25 @@ pub fn verify_signature_message_rust_crypto( Err(Error::Unrecognized) } }; + } else if is_eddsa(&signature_alg.oid) { + let Ok(verifying_key) = + ed25519_dalek::VerifyingKey::try_from(spki.subject_public_key.raw_bytes()) + else { + error!("Could not decode verifying key"); + return Err(Error::PathValidation(PathValidationStatus::EncodingError)); + }; + let Ok(s) = ed25519_dalek::Signature::from_slice(signature) else { + error!("Could not decode signature"); + return Err(Error::PathValidation(PathValidationStatus::EncodingError)); + }; + verifying_key + .verify_strict(message_to_verify, &s) + .map_err(|_| { + Error::PathValidation(PathValidationStatus::SignatureVerificationFailure) + })?; + return Ok(()); } + debug!("Unrecognized signature algorithm: {}", signature_alg.oid); Err(Error::Unrecognized) } diff --git a/certval/src/util/pdv_alg_oids.rs b/certval/src/util/pdv_alg_oids.rs index 99d93f1..c07c995 100644 --- a/certval/src/util/pdv_alg_oids.rs +++ b/certval/src/util/pdv_alg_oids.rs @@ -2,6 +2,11 @@ use der::asn1::ObjectIdentifier; +// id-edwards-curve-algs OBJECT IDENTIFIER ::= { 1 3 101 } + +/// id-Ed25519 OBJECT IDENTIFIER ::= { id-edwards-curve-algs 112 } +pub const PKIXALG_ED25519: ObjectIdentifier = ed25519_dalek::pkcs8::ALGORITHM_OID; + // ------------------------------------------------------------------------------------------------- // OIDs from PKIXAlgs-2009 // ------------------------------------------------------------------------------------------------- @@ -12,6 +17,8 @@ use der::asn1::ObjectIdentifier; pub const PKIXALG_RSA_ENCRYPTION: ObjectIdentifier = ObjectIdentifier::new_unwrap("1.2.840.113549.1.1.1"); +// 1.3.101.112 + /// id-ecPublicKey OBJECT IDENTIFIER ::= { /// iso(1) member-body(2) us(840) ansi-X9-62(10045) keyType(2) 1 } pub const PKIXALG_EC_PUBLIC_KEY: ObjectIdentifier = From 6ff1da89f2690215da5412a228d00b12e911bf78 Mon Sep 17 00:00:00 2001 From: Mathieu Amiot Date: Tue, 6 Feb 2024 14:00:18 +0100 Subject: [PATCH 06/23] feat: Added support for forbidding self-signed EE certs --- certval/src/util/error.rs | 3 +++ certval/src/validator/path_settings.rs | 3 +++ certval/src/validator/path_validator.rs | 29 ++++++++++++++++++++++++- 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/certval/src/util/error.rs b/certval/src/util/error.rs index 03669fa..32f3701 100644 --- a/certval/src/util/error.rs +++ b/certval/src/util/error.rs @@ -85,6 +85,8 @@ pub enum PathValidationStatus { RevocationStatusNotAvailable, /// A configuration error was detected. See textual log output for more details. Misconfiguration, + /// An End-Identity certificate was self-signed, but it is forbidden + SelfSignedEndIdentity, } /// Error type @@ -183,6 +185,7 @@ impl fmt::Display for PathValidationStatus { write!(f, "RevocationStatusNotAvailable") } PathValidationStatus::Misconfiguration => write!(f, "Misconfiguration"), + PathValidationStatus::SelfSignedEndIdentity => write!(f, "SelfSignedEndIdentity"), } } } diff --git a/certval/src/validator/path_settings.rs b/certval/src/validator/path_settings.rs index 0e8cca7..e805b40 100644 --- a/certval/src/validator/path_settings.rs +++ b/certval/src/validator/path_settings.rs @@ -336,6 +336,8 @@ pub static PS_CBOR_TA_STORE: &str = "psCborTaStore"; pub static PS_REQUIRE_TA_STORE: &str = "psRequireTaStore"; /// PS_USE_POLICY_GRAPH is used to indicate that the validator should use policy graph-based certificate policy processing. pub static PS_USE_POLICY_GRAPH: &str = "psUsePolicyGraph"; +/// PS_FORBID_SELF_SIGNED_EE is used to forbid allowing self-signed end-identity certificates +pub static PS_FORBID_SELF_SIGNED_EE: &str = "psForbidSelfSignedEE"; //----------------------------------------------------------------------------------------------- // Getters/setters for settings @@ -672,6 +674,7 @@ cps_gets_and_sets!(PS_PERM_COUNTRIES, Strings); cps_gets_and_sets!(PS_EXCL_COUNTRIES, Strings); cps_gets_and_sets_with_default!(PS_REQUIRE_TA_STORE, bool, true); cps_gets_and_sets_with_default!(PS_USE_POLICY_GRAPH, bool, false); +cps_gets_and_sets_with_default!(PS_FORBID_SELF_SIGNED_EE, bool, false); impl CertificationPathSettings { /// `get_target_key_usage` retrieves the `PS_KEY_USAGE` value from a diff --git a/certval/src/validator/path_validator.rs b/certval/src/validator/path_validator.rs index cbcd1b1..4e73a3b 100644 --- a/certval/src/validator/path_validator.rs +++ b/certval/src/validator/path_validator.rs @@ -112,7 +112,7 @@ pub fn validate_path_rfc5280( /// It uses values from the [`PS_INITIAL_PATH_LENGTH_CONSTRAINT`] item in the [`CertificationPathSettings`] /// and the path_len_constraint field of basicConstraints extensions. pub fn check_basic_constraints( - _pe: &PkiEnvironment, + pe: &PkiEnvironment, cps: &CertificationPathSettings, cp: &mut CertificationPath, cpr: &mut CertificationPathResults, @@ -182,6 +182,33 @@ pub fn check_basic_constraints( } } + if get_forbid_self_signed_ee(cps) { + let pdv_ext: Option<&PDVExtension> = cp.target.get_extension(&ID_CE_BASIC_CONSTRAINTS)?; + let bc = match pdv_ext { + Some(PDVExtension::BasicConstraints(bc)) => bc, + _ => { + log_error_for_ca(&cp.target, "missing basic constraints"); + set_validation_status(cpr, PathValidationStatus::MissingBasicConstraints); + return Err(Error::PathValidation( + PathValidationStatus::MissingBasicConstraints, + )); + } + }; + + let is_ee = !bc.ca; + + if is_ee && is_self_signed(pe, &cp.target) { + log_error_for_ca( + &cp.target, + "End-identity certificate is self-signed, but it is forbidden", + ); + set_validation_status(cpr, PathValidationStatus::SelfSignedEndIdentity); + return Err(Error::PathValidation( + PathValidationStatus::SelfSignedEndIdentity, + )); + } + } + Ok(()) } From 563870afee8a85e53856d955d0161ceb4df1330c Mon Sep 17 00:00:00 2001 From: Mathieu Amiot Date: Tue, 6 Feb 2024 15:32:33 +0100 Subject: [PATCH 07/23] fix: Improve performance + additional self-emission check --- certval/src/util/crypto.rs | 6 ++---- certval/src/util/pdv_utilities.rs | 11 ++++------- certval/src/validator/path_validator.rs | 4 ++-- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/certval/src/util/crypto.rs b/certval/src/util/crypto.rs index 981ebff..8ee5b31 100644 --- a/certval/src/util/crypto.rs +++ b/certval/src/util/crypto.rs @@ -187,8 +187,7 @@ pub fn verify_signature_digest_rust_crypto( signature_alg: &AlgorithmIdentifierOwned, // signature algorithm spki: &SubjectPublicKeyInfoOwned, // public key ) -> Result<()> { - let enc_spki = spki.to_der(); - if let Ok(enc_spki) = enc_spki { + if let Ok(enc_spki) = spki.to_der() { if is_rsa(&signature_alg.oid) { let rsa = RsaPublicKey::from_public_key_der(&enc_spki); if let Ok(rsa) = rsa { @@ -231,9 +230,8 @@ pub fn verify_signature_message_rust_crypto( signature_alg: &AlgorithmIdentifierOwned, // signature algorithm spki: &SubjectPublicKeyInfoOwned, // public key ) -> Result<()> { - let enc_spki = spki.to_der(); if is_rsa(&signature_alg.oid) { - if let Ok(enc_spki) = enc_spki { + if let Ok(enc_spki) = spki.to_der() { let rsa = RsaPublicKey::from_public_key_der(&enc_spki); if let Ok(rsa) = rsa { let hash_alg = get_hash_alg_from_sig_alg(&signature_alg.oid)?; diff --git a/certval/src/util/pdv_utilities.rs b/certval/src/util/pdv_utilities.rs index 070d9a4..16d678b 100644 --- a/certval/src/util/pdv_utilities.rs +++ b/certval/src/util/pdv_utilities.rs @@ -50,18 +50,15 @@ pub fn is_self_signed_with_buffer( enc_cert: &[u8], ) -> bool { match DeferDecodeSigned::from_der(enc_cert) { - Ok(defer_cert) => { - let r = pe.verify_signature_message( + Ok(defer_cert) => pe + .verify_signature_message( pe, &defer_cert.tbs_field, cert.signature.raw_bytes(), &cert.tbs_certificate.signature, &cert.tbs_certificate.subject_public_key_info, - ); - //TODO is it worth making metadata a RefCell to save the result of checks like this? - //If not, ditch metadata and replace with String field for locator - matches!(r, Ok(_e)) - } + ) + .is_ok(), Err(e) => { error!( "Failed to defer decode certificate in is_self_signed with: {}", diff --git a/certval/src/validator/path_validator.rs b/certval/src/validator/path_validator.rs index 4e73a3b..d4648fa 100644 --- a/certval/src/validator/path_validator.rs +++ b/certval/src/validator/path_validator.rs @@ -197,10 +197,10 @@ pub fn check_basic_constraints( let is_ee = !bc.ca; - if is_ee && is_self_signed(pe, &cp.target) { + if is_ee && (is_self_issued(&cp.target.decoded_cert) || is_self_signed(pe, &cp.target)) { log_error_for_ca( &cp.target, - "End-identity certificate is self-signed, but it is forbidden", + "End-identity certificate is self-signed or self-issued, but it is forbidden", ); set_validation_status(cpr, PathValidationStatus::SelfSignedEndIdentity); return Err(Error::PathValidation( From 062dbce62b510d54b7ae2cc2180cc8a9e411fe96 Mon Sep 17 00:00:00 2001 From: Mathieu Amiot Date: Tue, 6 Feb 2024 18:11:28 +0100 Subject: [PATCH 08/23] added test --- certval/tests/examples/ee_alice_ss_test.pem | 14 +++++++++ certval/tests/path_validator.rs | 35 +++++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 certval/tests/examples/ee_alice_ss_test.pem diff --git a/certval/tests/examples/ee_alice_ss_test.pem b/certval/tests/examples/ee_alice_ss_test.pem new file mode 100644 index 0000000..c0505ba --- /dev/null +++ b/certval/tests/examples/ee_alice_ss_test.pem @@ -0,0 +1,14 @@ +-----BEGIN CERTIFICATE----- +MIICFjCCAcigAwIBAgIDALWAMAUGAytlcDAqMRQwEgYDVQQDDAthbGljZSBTbWl0 +aDESMBAGA1UECgwJd29ybGQuY29tMB4XDTI0MDIwNjE2NDIyM1oXDTI0MDIwNzE2 +NDIyM1owKjEUMBIGA1UEAwwLYWxpY2UgU21pdGgxEjAQBgNVBAoMCXdvcmxkLmNv +bTAqMAUGAytlcAMhALHBqDPXfzoo93krCe7ZhyZoxcnRYt1g84BVcrwVG+Dgo4IB +DzCCAQswHQYDVR0OBBYEFFFwmrsOPEqQm4fQzKRHhv6QN6J5MB8GA1UdIwQYMBaA +FFFwmrsOPEqQm4fQzKRHhv6QN6J5MAwGA1UdEwEB/wQCMAAwDgYDVR0PAQH/BAQD +AgbAMBYGA1UdJQEB/wQMMAoGCCsGAQUFBwMCMGgGA1UdEQRhMF+GIXdpcmVhcHA6 +Ly8lNDBhbGljZV93aXJlQHdvcmxkLmNvbYY6d2lyZWFwcDovL0xoVzkzU1psUUlT +REpaMHdocktpUEEhOTI3NjM5Y2NjMzI2YmJkYUB3aXJlLmNvbTApBgNVHR8EIjAg +MB6gHKAahhhodHRwOi8vd29ybGQuY29tL2NybC5kZXIwBQYDK2VwA0EAsHZsyBLh +1D3H/TF0V63Msw24/Pk72gjySGsptP3/3w8fWpcahKJgs7H5hPce9P9sUCzVqCbh +4QBWKWfrMQTlBw== +-----END CERTIFICATE----- diff --git a/certval/tests/path_validator.rs b/certval/tests/path_validator.rs index df4e392..a28fd3f 100644 --- a/certval/tests/path_validator.rs +++ b/certval/tests/path_validator.rs @@ -138,3 +138,38 @@ fn is_trust_anchor_test() { let ta = PDVTrustAnchorChoice::try_from(der_encoded_ta.as_slice()).unwrap(); assert!(pe.is_trust_anchor(&ta).is_ok()); } + +#[test] +fn denies_self_signed_ee() { + let mut pe = PkiEnvironment::default(); + let ta_source = TaSource::default(); + let cert_source = CertSource::default(); + pe.add_trust_anchor_source(Box::new(ta_source.clone())); + pe.add_certificate_source(Box::new(cert_source.clone())); + + populate_5280_pki_environment(&mut pe); + + let pem_encoded_cert = include_bytes!("../tests/examples/ee_alice_ss_test.pem"); + use der::DecodePem as _; + let cert = x509_cert::Certificate::from_pem(pem_encoded_cert).unwrap(); + let cert = PDVCertificate::try_from(cert).unwrap(); + + let mut cps = CertificationPathSettings::default(); + set_forbid_self_signed_ee(&mut cps, true); + let mut paths = vec![]; + pe.get_paths_for_target(&pe, &cert, &mut paths, 0, get_time_of_interest(&cps)) + .unwrap(); + + if paths.is_empty() { + return; + } + + for path in &mut paths { + let mut cpr = CertificationPathResults::new(); + if validate_path_rfc5280(&pe, &cps, path, &mut cpr).is_err() { + return; + } + } + + panic!("EE cert was accepted"); +} From 1e26f0128b48e9e3a34168d40c594d841998c8d5 Mon Sep 17 00:00:00 2001 From: Mathieu Amiot Date: Wed, 7 Feb 2024 17:35:52 +0100 Subject: [PATCH 09/23] feat: Add support for retrieving intermediates --- certval/src/environment/pki_environment.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/certval/src/environment/pki_environment.rs b/certval/src/environment/pki_environment.rs index 45aa890..f558b34 100644 --- a/certval/src/environment/pki_environment.rs +++ b/certval/src/environment/pki_environment.rs @@ -378,6 +378,17 @@ impl PkiEnvironment { self.certificate_sources.clear(); } + /// gives all the intermediate certificates + pub fn get_intermediates(&self) -> Result> { + for f in &self.certificate_sources { + let r = f.get_certificates(); + if let Ok(r) = r { + return Ok(r); + } + } + Err(Error::Unrecognized) + } + /// add_crl_source adds a [`CrlSource`] object to the list. pub fn add_crl_source(&mut self, c: Box<(dyn CrlSource + Send + Sync)>) { self.crl_sources.push(c); From dfbaf5bd1ec90709d721252052ed11f2c121806c Mon Sep 17 00:00:00 2001 From: Mathieu Amiot Date: Thu, 8 Feb 2024 15:59:51 +0100 Subject: [PATCH 10/23] Added test to assert that the Wire environment actually works --- Cargo.lock | 52 +++++++++++- certval/Cargo.toml | 1 + certval/src/source/cert_source.rs | 2 + certval/src/validator/path_validator.rs | 6 +- certval/tests/examples/wire_certchain/ee.pem | 14 +++ .../examples/wire_certchain/intermediate.pem | 14 +++ certval/tests/examples/wire_certchain/ta.pem | 12 +++ certval/tests/path_validator.rs | 85 +++++++++++++++++-- 8 files changed, 175 insertions(+), 11 deletions(-) create mode 100644 certval/tests/examples/wire_certchain/ee.pem create mode 100644 certval/tests/examples/wire_certchain/intermediate.pem create mode 100644 certval/tests/examples/wire_certchain/ta.pem diff --git a/Cargo.lock b/Cargo.lock index 5bb3daa..789052c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -290,6 +290,7 @@ dependencies = [ "pqcrypto-internals", "pqcrypto-sphincsplus", "pqcrypto-traits", + "pretty_env_logger", "readonly", "regex", "reqwest", @@ -659,6 +660,19 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "env_logger" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4cd405aab171cb85d6735e5c8d9db038c17d3ca007a4d2c25f337935c3d90580" +dependencies = [ + "humantime", + "is-terminal", + "log", + "regex", + "termcolor", +] + [[package]] name = "equivalent" version = "1.0.1" @@ -880,6 +894,12 @@ version = "0.3.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d231dfb89cfffdbc30e7fc41579ed6066ad03abda9e567ccafae602b97ec5024" +[[package]] +name = "hermit-abi" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fbf6a919d6cf397374f7dfeeea91d974c7c0a7221d0d0f4f20d859d329e53fcc" + [[package]] name = "hex-literal" version = "0.4.1" @@ -1031,6 +1051,17 @@ version = "2.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f518f335dce6725a761382244631d86cf0ccb2863413590b31338feb467f9c3" +[[package]] +name = "is-terminal" +version = "0.4.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "261f68e344040fbd0edea105bef17c66edf46f984ddb1115b775ce31be948f4b" +dependencies = [ + "hermit-abi 0.4.0", + "libc", + "windows-sys 0.52.0", +] + [[package]] name = "itoa" version = "1.0.10" @@ -1276,7 +1307,7 @@ version = "1.16.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4161fcb6d602d4d2081af7c3a45852d875a03dd337a6bfdd6e06407b61342a43" dependencies = [ - "hermit-abi", + "hermit-abi 0.3.9", "libc", ] @@ -1672,6 +1703,16 @@ dependencies = [ "termtree", ] +[[package]] +name = "pretty_env_logger" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "865724d4dbe39d9f3dd3b52b88d859d66bcb2d6a0acfd5ea68a65fb66d4bdc1c" +dependencies = [ + "env_logger", + "log", +] + [[package]] name = "primeorder" version = "0.14.0-pre.1" @@ -2202,6 +2243,15 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "termcolor" +version = "1.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06794f8f6c5c898b3275aebefa6b8a1cb24cd2c6c79397ab15774837a0bc5755" +dependencies = [ + "winapi-util", +] + [[package]] name = "termtree" version = "0.4.1" diff --git a/certval/Cargo.toml b/certval/Cargo.toml index aa5c206..f21b979 100644 --- a/certval/Cargo.toml +++ b/certval/Cargo.toml @@ -73,6 +73,7 @@ webpki-roots = {version = "0.25.1", optional = true} tempfile = "3.8.1" hex-literal = "0.4.1" tokio-test = "0.4.2" +pretty_env_logger = "0.5" # There are five feature gates: # - no-default-features (i.e., no-std) provides full path validation without file system support, network or thread safety (and no revocation support) diff --git a/certval/src/source/cert_source.rs b/certval/src/source/cert_source.rs index db2c87c..072fd3d 100644 --- a/certval/src/source/cert_source.rs +++ b/certval/src/source/cert_source.rs @@ -1397,6 +1397,8 @@ impl CertificateSource for CertSource { if !akid_hex.is_empty() { let partial_paths = &self.buffers_and_paths.partial_paths; + dbg!(&*partial_paths); + for p in partial_paths.iter() { if p.contains_key(&akid_hex) { let indices_vec = &p[&akid_hex]; diff --git a/certval/src/validator/path_validator.rs b/certval/src/validator/path_validator.rs index d4648fa..af93541 100644 --- a/certval/src/validator/path_validator.rs +++ b/certval/src/validator/path_validator.rs @@ -182,13 +182,13 @@ pub fn check_basic_constraints( } } - if get_forbid_self_signed_ee(cps) { + if cps.get_forbid_self_signed_ee() { let pdv_ext: Option<&PDVExtension> = cp.target.get_extension(&ID_CE_BASIC_CONSTRAINTS)?; let bc = match pdv_ext { Some(PDVExtension::BasicConstraints(bc)) => bc, _ => { log_error_for_ca(&cp.target, "missing basic constraints"); - set_validation_status(cpr, PathValidationStatus::MissingBasicConstraints); + cpr.set_validation_status(PathValidationStatus::MissingBasicConstraints); return Err(Error::PathValidation( PathValidationStatus::MissingBasicConstraints, )); @@ -202,7 +202,7 @@ pub fn check_basic_constraints( &cp.target, "End-identity certificate is self-signed or self-issued, but it is forbidden", ); - set_validation_status(cpr, PathValidationStatus::SelfSignedEndIdentity); + cpr.set_validation_status(PathValidationStatus::SelfSignedEndIdentity); return Err(Error::PathValidation( PathValidationStatus::SelfSignedEndIdentity, )); diff --git a/certval/tests/examples/wire_certchain/ee.pem b/certval/tests/examples/wire_certchain/ee.pem new file mode 100644 index 0000000..21a35ba --- /dev/null +++ b/certval/tests/examples/wire_certchain/ee.pem @@ -0,0 +1,14 @@ +-----BEGIN CERTIFICATE----- +MIICJTCCAdegAwIBAgICY4EwBQYDK2VwMD8xKTAnBgNVBAMMIFdvcmxkIERvbWlu +YXRpb24gSW50ZXJtZWRpYXRlIENBMRIwEAYDVQQKDAl3b3JsZC5jb20wHhcNMjQw +MjA4MTAwODEzWhcNMjQwMjA5MTAwODEzWjAkMQ4wDAYDVQQDDAVhbGljZTESMBAG +A1UECgwJd29ybGQuY29tMCowBQYDK2VwAyEAaLFURMk54SzsvyospqSdDajBN05i +7/H/hru6p9KNXpKjggEQMIIBDDAdBgNVHQ4EFgQURT1grcplwsHDbrjcKG5lI175 +wmwwHwYDVR0jBBgwFoAUZ7XAzZy/AgTIXbV23QWmC0fkPm0wDAYDVR0TAQH/BAIw +ADAOBgNVHQ8BAf8EBAMCBsAwFgYDVR0lAQH/BAwwCgYIKwYBBQUHAwIwaQYDVR0R +BGIwYIYhd2lyZWFwcDovLyU0MGFsaWNlX3dpcmVAd29ybGQuY29thjt3aXJlYXBw +Oi8vaVU5WDJwMEZUME9GQjNGWmRhQ3BKUSEzZTdkNzUzNjEwZTNiOWQwQHdvcmxk +LmNvbTApBgNVHR8EIjAgMB6gHKAahhhodHRwOi8vd29ybGQuY29tL2NybC5kZXIw +BQYDK2VwA0EAiZCoOy6TYH+4KZIzDRSMZKHXujrYzfeQkxp6d9P3FrJ9sEmdw6uw +6FeVOrOy/KfMrXE/lpNZmoDoZUCAElVfBg== +-----END CERTIFICATE----- diff --git a/certval/tests/examples/wire_certchain/intermediate.pem b/certval/tests/examples/wire_certchain/intermediate.pem new file mode 100644 index 0000000..3d82543 --- /dev/null +++ b/certval/tests/examples/wire_certchain/intermediate.pem @@ -0,0 +1,14 @@ +-----BEGIN CERTIFICATE----- +MIICFjCCAcigAwIBAgIDAPhEMAUGAytlcDA3MSEwHwYDVQQDDBhXb3JsZCBEb21p +bmF0aW9uIFJvb3QgQ0ExEjAQBgNVBAoMCXdvcmxkLmNvbTAeFw0yNDAyMDgxMDA4 +MTNaFw0yNDAyMDkxMDA4MTNaMD8xKTAnBgNVBAMMIFdvcmxkIERvbWluYXRpb24g +SW50ZXJtZWRpYXRlIENBMRIwEAYDVQQKDAl3b3JsZC5jb20wKjAFBgMrZXADIQC2 +JnfxbzRMMu6JGdW2/UbKZJ00WOVOaZxc5PJNIJaCO6OB7jCB6zAdBgNVHQ4EFgQU +Z7XAzZy/AgTIXbV23QWmC0fkPm0wHwYDVR0jBBgwFoAU+RgSMrMip3POiLXQVdm7 +9hC4BfswEgYDVR0TAQH/BAgwBgEB/wIBATAOBgNVHQ8BAf8EBAMCAQYwDAYDVR0l +AQH/BAIwADAUBgNVHREEDTALggl3b3JsZC5jb20wNgYDVR0eAQH/BCwwKqAoMAyG +Ci53b3JsZC5jb20wC4YJd29ybGQuY29tMAuCCXdvcmxkLmNvbTApBgNVHR8EIjAg +MB6gHKAahhhodHRwOi8vd29ybGQuY29tL2NybC5kZXIwBQYDK2VwA0EAGCv0VhjG +wukXyeykvTvbY4UuFfROVavNOioTRrCcUb47QxR5uqC5gHlgRTYEfs2ZzoPYUWQb +a5KA4jKBCOw1Cw== +-----END CERTIFICATE----- diff --git a/certval/tests/examples/wire_certchain/ta.pem b/certval/tests/examples/wire_certchain/ta.pem new file mode 100644 index 0000000..9b55a86 --- /dev/null +++ b/certval/tests/examples/wire_certchain/ta.pem @@ -0,0 +1,12 @@ +-----BEGIN CERTIFICATE----- +MIIB0jCCAYSgAwIBAgICNaIwBQYDK2VwMDcxITAfBgNVBAMMGFdvcmxkIERvbWlu +YXRpb24gUm9vdCBDQTESMBAGA1UECgwJd29ybGQuY29tMB4XDTI0MDIwODEwMDgx +M1oXDTI0MDIwOTEwMDgxM1owNzEhMB8GA1UEAwwYV29ybGQgRG9taW5hdGlvbiBS +b290IENBMRIwEAYDVQQKDAl3b3JsZC5jb20wKjAFBgMrZXADIQCvOTnU9T7DoKBf +Xuv13HqkhatDHlIk97k2O53su7K36qOBszCBsDAdBgNVHQ4EFgQU+RgSMrMip3PO +iLXQVdm79hC4BfswDwYDVR0TAQH/BAUwAwEB/zAOBgNVHQ8BAf8EBAMCAQYwHwYD +VR0jBBgwFoAU+RgSMrMip3POiLXQVdm79hC4BfswDAYDVR0lAQH/BAIwADAUBgNV +HREEDTALggl3b3JsZC5jb20wKQYDVR0fBCIwIDAeoBygGoYYaHR0cDovL3dvcmxk +LmNvbS9jcmwuZGVyMAUGAytlcANBAENjvG0I5KI+ZhggbdW/86+Wtm6LYRs/SIX/ +NnqjXCxgIHha7Q5+X/jbM4K8WhuShcdA4VaizNDPzdNR9/7RIA8= +-----END CERTIFICATE----- diff --git a/certval/tests/path_validator.rs b/certval/tests/path_validator.rs index a28fd3f..4dd25f2 100644 --- a/certval/tests/path_validator.rs +++ b/certval/tests/path_validator.rs @@ -1,11 +1,14 @@ //use certval::asn1::cryptographic_message_syntax2004::*; + use certval::environment::pki_environment::PkiEnvironment; use certval::path_settings::*; use certval::validator::path_validator::*; use certval::*; -use der::Decode; +use der::{Decode, DecodePem, Encode}; use x509_cert::*; +use self::certificate::{CertificateInner, Raw}; + #[test] fn prehash_required() { let enc_ca_cert = include_bytes!("examples/prehash_ca.der"); @@ -141,23 +144,28 @@ fn is_trust_anchor_test() { #[test] fn denies_self_signed_ee() { + let _ = pretty_env_logger::try_init(); + + let time_of_interest: TimeOfInterest = TimeOfInterest::from_unix_secs(1707264000).unwrap(); let mut pe = PkiEnvironment::default(); let ta_source = TaSource::default(); let cert_source = CertSource::default(); - pe.add_trust_anchor_source(Box::new(ta_source.clone())); - pe.add_certificate_source(Box::new(cert_source.clone())); + pe.add_trust_anchor_source(Box::new(ta_source)); + pe.add_certificate_source(Box::new(cert_source)); - populate_5280_pki_environment(&mut pe); + pe.populate_5280_pki_environment(); let pem_encoded_cert = include_bytes!("../tests/examples/ee_alice_ss_test.pem"); use der::DecodePem as _; - let cert = x509_cert::Certificate::from_pem(pem_encoded_cert).unwrap(); + let cert = CertificateInner::::from_pem(pem_encoded_cert).unwrap(); let cert = PDVCertificate::try_from(cert).unwrap(); let mut cps = CertificationPathSettings::default(); - set_forbid_self_signed_ee(&mut cps, true); + cps.set_forbid_self_signed_ee(true); + cps.set_time_of_interest(time_of_interest); + let mut paths = vec![]; - pe.get_paths_for_target(&pe, &cert, &mut paths, 0, get_time_of_interest(&cps)) + pe.get_paths_for_target(&cert, &mut paths, 0, cps.get_time_of_interest()) .unwrap(); if paths.is_empty() { @@ -173,3 +181,66 @@ fn denies_self_signed_ee() { panic!("EE cert was accepted"); } + +#[test] +fn wire_certchain_works() { + let _ = pretty_env_logger::try_init(); + + let time_of_interest: TimeOfInterest = TimeOfInterest::from_unix_secs(1707402960).unwrap(); + + let mut pe = certval::environment::PkiEnvironment::default(); + pe.populate_5280_pki_environment(); + + let mut cps = CertificationPathSettings::new(); + cps.set_time_of_interest(time_of_interest); + + // Make a TrustAnchor source + let mut trust_anchors = TaSource::new(); + + let root = + x509_cert::Certificate::from_pem(include_bytes!("examples/wire_certchain/ta.pem")).unwrap(); + + trust_anchors.push(certval::CertFile { + filename: format!("TrustAnchor #1"), + bytes: root.to_der().unwrap(), + }); + + trust_anchors.initialize().unwrap(); + pe.add_trust_anchor_source(Box::new(trust_anchors)); + + // Make a Certificate source for intermediate CA certs + let mut cert_source = CertSource::new(); + let cert = x509_cert::Certificate::from_pem(include_bytes!( + "examples/wire_certchain/intermediate.pem" + )) + .unwrap(); + cert_source.push(certval::CertFile { + filename: format!("Intermediate CA #1 [{}]", cert.tbs_certificate.subject), + bytes: cert.to_der().unwrap(), + }); + + cert_source.initialize(&cps).unwrap(); + cert_source.find_all_partial_paths(&pe, &cps); + pe.add_certificate_source(Box::new(cert_source)); + + cps.set_require_ta_store(true); + cps.set_forbid_self_signed_ee(true); + + let mut end_identity_cert = PDVCertificate::try_from( + CertificateInner::::from_pem(include_bytes!("examples/wire_certchain/ee.pem")) + .unwrap(), + ) + .unwrap(); + end_identity_cert.parse_extensions(EXTS_OF_INTEREST); + + let mut paths = vec![]; + pe.get_paths_for_target(&end_identity_cert, &mut paths, 0, time_of_interest) + .unwrap(); + + assert!(!paths.is_empty(), "No paths detected"); + + for path in &mut paths { + let mut cpr = CertificationPathResults::new(); + let _ = validate_path_rfc5280(&pe, &cps, path, &mut cpr).unwrap(); + } +} From 2fd9f1204e1d45ea352a3da499bc5bfbf04e697c Mon Sep 17 00:00:00 2001 From: Mathieu Amiot Date: Thu, 8 Feb 2024 16:53:48 +0100 Subject: [PATCH 11/23] fix: Validation works without the std feature --- certval/Cargo.toml | 6 +- certval/src/builder/graph_builder.rs | 154 +++++++++--------- certval/src/revocation/crl.rs | 109 +++++++------ certval/src/source/cert_source.rs | 2 - certval/src/util/pdv_utilities.rs | 8 +- certval/src/validator/name_constraints_set.rs | 151 ++++++----------- certval/tests/examples/wire_certchain/ee.pem | 16 +- .../examples/wire_certchain/intermediate.pem | 20 +-- certval/tests/examples/wire_certchain/ta.pem | 20 +-- certval/tests/path_validator.rs | 4 +- 10 files changed, 226 insertions(+), 264 deletions(-) diff --git a/certval/Cargo.toml b/certval/Cargo.toml index f21b979..923b759 100644 --- a/certval/Cargo.toml +++ b/certval/Cargo.toml @@ -74,6 +74,8 @@ tempfile = "3.8.1" hex-literal = "0.4.1" tokio-test = "0.4.2" pretty_env_logger = "0.5" +tokio = { version = "1.33.0", features = ["full", "time", "rt-multi-thread"] } + # There are five feature gates: # - no-default-features (i.e., no-std) provides full path validation without file system support, network or thread safety (and no revocation support) @@ -85,8 +87,8 @@ pretty_env_logger = "0.5" # webpki can be paired with any other feature and simply adds a means of initializing a TaSource from the webpki-roots crate [features] default = ["remote", "webpki"] -revocation = ["ndarray", "serde/std", "serde/rc"] -std = ["ndarray", "tokio", "base64ct", "walkdir", "ed25519-dalek/std", "url", "serde_json", "serde/rc", "flagset/serde", "regex", "lazy_static/spin", "bitvec", "cidr", "der/std"] +revocation = ["dep:ndarray", "serde/std", "serde/rc", "dep:url", "dep:regex"] +std = ["dep:ndarray", "dep:base64ct", "dep:walkdir", "ed25519-dalek/std", "dep:url", "dep:serde_json", "serde/rc", "flagset/serde", "dep:regex", "lazy_static/spin", "dep:bitvec", "dep:cidr", "der/std"] remote = ["revocation", "std", "reqwest", "lazy_static/spin"] pqc = ["pqcrypto-internals", "pqcrypto-dilithium", "pqcrypto-falcon", "pqcrypto-sphincsplus", "pqcrypto", "pqcrypto-traits", "pqckeys"] webpki = ["webpki-roots"] diff --git a/certval/src/builder/graph_builder.rs b/certval/src/builder/graph_builder.rs index 0e80957..ceed5b2 100644 --- a/certval/src/builder/graph_builder.rs +++ b/certval/src/builder/graph_builder.rs @@ -182,78 +182,84 @@ pub fn read_cbor(filename: &Option) -> Vec { vec![] } -#[cfg(feature = "std")] -#[tokio::test] -async fn non_existent_dir() { - let ta_store_folder = format!( - "{}{}", - env!("CARGO_MANIFEST_DIR"), - "/tests/examples/ta_store_with_bad" - ); - let ca_store_folder = format!( - "{}{}", - env!("CARGO_MANIFEST_DIR"), - "/tests/examples/cert_store_with_expired" - ); - let nonexistent = format!( - "{}{}", - env!("CARGO_MANIFEST_DIR"), - "/tests/examples/nonexistent" - ); - - let mut pe = PkiEnvironment::default(); - pe.populate_5280_pki_environment(); - - let mut ta_store = TaSource::new(); - ta_folder_to_vec( - &pe, - &ta_store_folder, - &mut ta_store, - TimeOfInterest::disabled(), - ) - .unwrap(); - ta_store.initialize().unwrap(); - - let mut cps = CertificationPathSettings::default(); - let r = build_graph(&pe, &cps).await; - assert!(r.is_err()); - let r = r.err(); - assert_eq!(Some(Error::NotFound), r); - - let r = build_graph(&pe, &cps).await; - assert!(r.is_err()); - let r = r.err(); - assert_eq!(Some(Error::NotFound), r); - - cps.set_certification_authority_folder(nonexistent.clone()); - let r = build_graph(&pe, &cps).await; - assert!(r.is_err()); - let r = r.err(); - assert_eq!(Some(Error::NotFound), r); - - cps.set_retrieve_from_aia_sia_http(false); - cps.set_certification_authority_folder(ca_store_folder.clone()); - pe.add_trust_anchor_source(Box::new(ta_store.clone())); - let cbor = build_graph(&pe, &cps).await; - assert!(cbor.is_ok()); - let cert_source = match CertSource::new_from_cbor(cbor.unwrap().as_slice()) { - Ok(cbor_data) => cbor_data, - Err(e) => { - panic!("Failed to parse CBOR file: {}", e) - } - }; - assert_eq!(3, cert_source.len()); - - // serialize as TA store (so no partial paths) - cps.set_cbor_ta_store(true); - cps.set_certification_authority_folder(ca_store_folder.clone()); - let cbor = build_graph(&pe, &cps).await; - assert!(cbor.is_ok()); - let cert_source = match CertSource::new_from_cbor(cbor.unwrap().as_slice()) { - Ok(cbor_data) => cbor_data, - Err(e) => { - panic!("Failed to parse CBOR file: {}", e) - } - }; - assert_eq!(3, cert_source.len()); +#[cfg(test)] +mod tests { + use crate::builder::file_utils::ta_folder_to_vec; + use crate::*; + + #[cfg(feature = "std")] + #[tokio::test] + async fn non_existent_dir() { + let ta_store_folder = format!( + "{}{}", + env!("CARGO_MANIFEST_DIR"), + "/tests/examples/ta_store_with_bad" + ); + let ca_store_folder = format!( + "{}{}", + env!("CARGO_MANIFEST_DIR"), + "/tests/examples/cert_store_with_expired" + ); + let nonexistent = format!( + "{}{}", + env!("CARGO_MANIFEST_DIR"), + "/tests/examples/nonexistent" + ); + + let mut pe = PkiEnvironment::default(); + pe.populate_5280_pki_environment(); + + let mut ta_store = TaSource::new(); + ta_folder_to_vec( + &pe, + &ta_store_folder, + &mut ta_store, + TimeOfInterest::disabled(), + ) + .unwrap(); + ta_store.initialize().unwrap(); + + let mut cps = CertificationPathSettings::default(); + let r = build_graph(&pe, &cps).await; + assert!(r.is_err()); + let r = r.err(); + assert_eq!(Some(Error::NotFound), r); + + let r = build_graph(&pe, &cps).await; + assert!(r.is_err()); + let r = r.err(); + assert_eq!(Some(Error::NotFound), r); + + cps.set_certification_authority_folder(nonexistent.clone()); + let r = build_graph(&pe, &cps).await; + assert!(r.is_err()); + let r = r.err(); + assert_eq!(Some(Error::NotFound), r); + + cps.set_retrieve_from_aia_sia_http(false); + cps.set_certification_authority_folder(ca_store_folder.clone()); + pe.add_trust_anchor_source(Box::new(ta_store.clone())); + let cbor = build_graph(&pe, &cps).await; + assert!(cbor.is_ok()); + let cert_source = match CertSource::new_from_cbor(cbor.unwrap().as_slice()) { + Ok(cbor_data) => cbor_data, + Err(e) => { + panic!("Failed to parse CBOR file: {}", e) + } + }; + assert_eq!(3, cert_source.len()); + + // serialize as TA store (so no partial paths) + cps.set_cbor_ta_store(true); + cps.set_certification_authority_folder(ca_store_folder.clone()); + let cbor = build_graph(&pe, &cps).await; + assert!(cbor.is_ok()); + let cert_source = match CertSource::new_from_cbor(cbor.unwrap().as_slice()) { + Ok(cbor_data) => cbor_data, + Err(e) => { + panic!("Failed to parse CBOR file: {}", e) + } + }; + assert_eq!(3, cert_source.len()); + } } diff --git a/certval/src/revocation/crl.rs b/certval/src/revocation/crl.rs index eb95887..a2140ee 100644 --- a/certval/src/revocation/crl.rs +++ b/certval/src/revocation/crl.rs @@ -1182,56 +1182,63 @@ pub(crate) async fn check_revocation_crl_remote( target_status } -#[cfg(feature = "remote")] -#[tokio::test] -async fn fetch_crl_test() { - use crate::{CrlSourceFolders, RemoteStatus, RevocationCache}; - use std::path::PathBuf; - let mut pe = PkiEnvironment::default(); - pe.clear_all_callbacks(); - pe.populate_5280_pki_environment(); - - let d = PathBuf::from(env!("CARGO_MANIFEST_DIR")); - let f = d.join("tests/examples/fetch_crl_test"); - let crl_source = CrlSourceFolders::new(f.as_path().to_str().unwrap()); - if crl_source - .index_crls(TimeOfInterest::from_unix_secs(1647011592).unwrap()) - .is_err() - { - panic!("Failed to index CRLs") - } - pe.add_crl_source(Box::new(crl_source)); - pe.add_revocation_cache(Box::new(RevocationCache::new())); - pe.add_check_remote(Box::new(RemoteStatus::new(f.as_path().to_str().unwrap()))); - - let r = fetch_crl(&pe, "ldap://ldap.scheme/", Duration::from_secs(60)).await; - assert!(r.is_err()); - assert_eq!(Some(Error::InvalidUriScheme), r.err()); - pe.add_to_blocklist("http://blocklist.test"); - let r = fetch_crl(&pe, "http://blocklist.test", Duration::from_secs(60)).await; - assert!(r.is_err()); - assert_eq!(Some(Error::UriOnBlocklist), r.err()); - - let f = d.join("tests/examples/fetch_crl_test/last_modified_map.json"); - if std::path::Path::exists(&f) { - tokio::fs::remove_file(f.to_str().unwrap()).await.unwrap(); - } +#[cfg(test)] +mod tests { + use crate::*; + + #[cfg(feature = "remote")] + #[tokio::test] + async fn fetch_crl_test() { + use crate::{CrlSourceFolders, RemoteStatus, RevocationCache}; + use std::{path::PathBuf, time::Duration}; + + use self::crl::fetch_crl; + let mut pe = PkiEnvironment::default(); + pe.clear_all_callbacks(); + pe.populate_5280_pki_environment(); + + let d = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + let f = d.join("tests/examples/fetch_crl_test"); + let crl_source = CrlSourceFolders::new(f.as_path().to_str().unwrap()); + if crl_source + .index_crls(TimeOfInterest::from_unix_secs(1647011592).unwrap()) + .is_err() + { + panic!("Failed to index CRLs") + } + pe.add_crl_source(Box::new(crl_source)); + pe.add_revocation_cache(Box::new(RevocationCache::new())); + pe.add_check_remote(Box::new(RemoteStatus::new(f.as_path().to_str().unwrap()))); + + let r = fetch_crl(&pe, "ldap://ldap.scheme/", Duration::from_secs(60)).await; + assert!(r.is_err()); + assert_eq!(Some(Error::InvalidUriScheme), r.err()); + pe.add_to_blocklist("http://blocklist.test"); + let r = fetch_crl(&pe, "http://blocklist.test", Duration::from_secs(60)).await; + assert!(r.is_err()); + assert_eq!(Some(Error::UriOnBlocklist), r.err()); + + let f = d.join("tests/examples/fetch_crl_test/last_modified_map.json"); + if std::path::Path::exists(&f) { + tokio::fs::remove_file(f.to_str().unwrap()).await.unwrap(); + } - let r = fetch_crl( - &pe, - "http://crl.sectigo.com/SectigoRSAOrganizationValidationSecureServerCA.crl", - Duration::from_secs(60), - ) - .await; - assert!(r.is_ok()); - let r = fetch_crl( - &pe, - "http://crl.sectigo.com/SectigoRSAOrganizationValidationSecureServerCA.crl", - Duration::from_secs(60), - ) - .await; - assert!(r.is_err()); - assert_eq!(Some(Error::ResourceUnchanged), r.err()); - - let _ = tokio::fs::remove_file(f.to_str().unwrap()).await; + let r = fetch_crl( + &pe, + "http://crl.sectigo.com/SectigoRSAOrganizationValidationSecureServerCA.crl", + Duration::from_secs(60), + ) + .await; + assert!(r.is_ok()); + let r = fetch_crl( + &pe, + "http://crl.sectigo.com/SectigoRSAOrganizationValidationSecureServerCA.crl", + Duration::from_secs(60), + ) + .await; + assert!(r.is_err()); + assert_eq!(Some(Error::ResourceUnchanged), r.err()); + + let _ = tokio::fs::remove_file(f.to_str().unwrap()).await; + } } diff --git a/certval/src/source/cert_source.rs b/certval/src/source/cert_source.rs index 072fd3d..db2c87c 100644 --- a/certval/src/source/cert_source.rs +++ b/certval/src/source/cert_source.rs @@ -1397,8 +1397,6 @@ impl CertificateSource for CertSource { if !akid_hex.is_empty() { let partial_paths = &self.buffers_and_paths.partial_paths; - dbg!(&*partial_paths); - for p in partial_paths.iter() { if p.contains_key(&akid_hex) { let indices_vec = &p[&akid_hex]; diff --git a/certval/src/util/pdv_utilities.rs b/certval/src/util/pdv_utilities.rs index 16d678b..3a971f9 100644 --- a/certval/src/util/pdv_utilities.rs +++ b/certval/src/util/pdv_utilities.rs @@ -7,10 +7,7 @@ use core::str::FromStr; use log::{debug, error}; -#[cfg(feature = "std")] use lazy_static::lazy_static; - -#[cfg(feature = "std")] use regex::Regex; use const_oid::db::rfc2256::STATE_OR_PROVINCE_NAME; @@ -326,7 +323,6 @@ pub(crate) const EMAIL_PATTERN: &str = // TODO implement to support name constraints for no-std /// `descended_from_rfc822` returns true if new_name is equal to or descended from prev_name and false otherwise. -#[cfg(feature = "std")] pub fn descended_from_host(prev_name: &Ia5String, cand: &str, is_uri: bool) -> bool { let base = prev_name.to_string(); @@ -371,7 +367,7 @@ pub fn descended_from_host(prev_name: &Ia5String, cand: &str, is_uri: bool) -> b // TODO implement to support name constraints for no-std /// `is_email` returns true if addr matches the regular expression defined by [`EMAIL_PATTERN`]. -#[cfg(feature = "std")] + pub(crate) fn is_email(addr: &str) -> bool { lazy_static! { static ref EMAIL_RE: Regex = Regex::new( @@ -389,7 +385,7 @@ pub(crate) fn is_email(addr: &str) -> bool { // TODO implement to support name constraints for no-std /// `descended_from_rfc822` returns true if new_name is equal to or descended from prev_name and false otherwise. -#[cfg(feature = "std")] + pub(crate) fn descended_from_rfc822(prev_name: &Ia5String, new_name: &Ia5String) -> bool { let cand = new_name.to_string(); let base = prev_name.to_string(); diff --git a/certval/src/validator/name_constraints_set.rs b/certval/src/validator/name_constraints_set.rs index 7c73c7b..7bb36db 100644 --- a/certval/src/validator/name_constraints_set.rs +++ b/certval/src/validator/name_constraints_set.rs @@ -11,7 +11,7 @@ use core::str::FromStr; use serde::{Deserialize, Serialize}; -#[cfg(feature = "std")] +#[cfg(any(feature = "std", feature = "revocation"))] use url::Url; #[cfg(feature = "std")] @@ -128,24 +128,14 @@ impl NameConstraintsSet { // as None signifies a failure. match gn { GeneralName::Rfc822Name(_rfc822) => { - #[cfg(feature = "std")] if !self.rfc822_name_null { self.rfc822_name.push(subtree.clone()); } - #[cfg(not(feature = "std"))] - { - self.rfc822_name_null = true; - } } GeneralName::DnsName(_dns) => { - #[cfg(feature = "std")] if !self.dns_name_null { self.dns_name.push(subtree.clone()); } - #[cfg(not(feature = "std"))] - { - self.dns_name_null = true; - } } GeneralName::DirectoryName(_dn) => { if !self.directory_name_null { @@ -153,14 +143,9 @@ impl NameConstraintsSet { } } GeneralName::UniformResourceIdentifier(_uri) => { - #[cfg(feature = "std")] if !self.uniform_resource_identifier_null { self.uniform_resource_identifier.push(subtree.clone()); } - #[cfg(not(feature = "std"))] - { - self.uniform_resource_identifier_null = true; - } } GeneralName::IpAddress(_ip) => { #[cfg(feature = "std")] @@ -270,8 +255,6 @@ impl NameConstraintsSet { } let mut rfc822_ok = false; - - #[cfg(feature = "std")] for gn_state in &self.rfc822_name { if let GeneralName::Rfc822Name(rfc822_state) = &gn_state.base { if descended_from_rfc822(rfc822_state, rfc822_san) { @@ -296,8 +279,6 @@ impl NameConstraintsSet { } let mut dns_ok = false; - - #[cfg(feature = "std")] for gn_state in &self.dns_name { if let GeneralName::DnsName(dns_state) = &gn_state.base { if descended_from_host(dns_state, dns_san.as_str(), false) { @@ -322,8 +303,6 @@ impl NameConstraintsSet { } let mut uri_ok = false; - - #[cfg(feature = "std")] for gn_state in &self.uniform_resource_identifier { if let GeneralName::UniformResourceIdentifier(uri_state) = &gn_state.base @@ -483,7 +462,6 @@ impl NameConstraintsSet { continue; } - #[cfg(feature = "std")] for gn_state in &self.dns_name { if let GeneralName::DnsName(dns_state) = &gn_state.base { if descended_from_host(dns_state, dns_san.as_str(), false) { @@ -503,7 +481,6 @@ impl NameConstraintsSet { continue; } - #[cfg(feature = "std")] for gn_state in &self.uniform_resource_identifier { if let GeneralName::UniformResourceIdentifier(uri_state) = &gn_state.base @@ -574,38 +551,30 @@ impl NameConstraintsSet { return; } - #[cfg(not(feature = "std"))] - { - self.rfc822_name_null = true; - } - - #[cfg(feature = "std")] - { - let mut new_set = Vec::new(); + let mut new_set = Vec::new(); - for new_name in new_names { - if let GeneralName::Rfc822Name(new_rfc822) = &new_name.base { - if self.rfc822_name.is_empty() { - new_set.push(new_name.clone()); - } else { - for prev_name in &self.rfc822_name { - if let GeneralName::Rfc822Name(prev_rfc822) = &prev_name.base { - if new_name == prev_name - || descended_from_rfc822(prev_rfc822, new_rfc822) - { - new_set.push(prev_name.clone()); - } + for new_name in new_names { + if let GeneralName::Rfc822Name(new_rfc822) = &new_name.base { + if self.rfc822_name.is_empty() { + new_set.push(new_name.clone()); + } else { + for prev_name in &self.rfc822_name { + if let GeneralName::Rfc822Name(prev_rfc822) = &prev_name.base { + if new_name == prev_name + || descended_from_rfc822(prev_rfc822, new_rfc822) + { + new_set.push(prev_name.clone()); } } } } } + } - if !new_set.is_empty() { - self.rfc822_name = new_set; - } else { - self.rfc822_name_null = true; - } + if !new_set.is_empty() { + self.rfc822_name = new_set; + } else { + self.rfc822_name_null = true; } } @@ -615,38 +584,30 @@ impl NameConstraintsSet { return; } - #[cfg(not(feature = "std"))] - { - self.dns_name_null = true; - } - - #[cfg(feature = "std")] - { - let mut new_set = Vec::new(); + let mut new_set = Vec::new(); - for new_name in new_names { - if let GeneralName::DnsName(new_dns) = &new_name.base { - if self.dns_name.is_empty() { - new_set.push(new_name.clone()); - } else { - for prev_name in &self.dns_name { - if let GeneralName::DnsName(prev_dns) = &prev_name.base { - if new_name == prev_name - || descended_from_host(prev_dns, new_dns.as_str(), false) - { - new_set.push(prev_name.clone()); - } + for new_name in new_names { + if let GeneralName::DnsName(new_dns) = &new_name.base { + if self.dns_name.is_empty() { + new_set.push(new_name.clone()); + } else { + for prev_name in &self.dns_name { + if let GeneralName::DnsName(prev_dns) = &prev_name.base { + if new_name == prev_name + || descended_from_host(prev_dns, new_dns.as_str(), false) + { + new_set.push(prev_name.clone()); } } } } } + } - if !new_set.is_empty() { - self.dns_name = new_set; - } else { - self.dns_name_null = true; - } + if !new_set.is_empty() { + self.dns_name = new_set; + } else { + self.dns_name_null = true; } } @@ -694,40 +655,30 @@ impl NameConstraintsSet { return; } - #[cfg(not(feature = "std"))] - { - self.uniform_resource_identifier_null = true; - } - - #[cfg(feature = "std")] - { - let mut new_set = Vec::new(); + let mut new_set = Vec::new(); - for new_name in new_names { - if let GeneralName::UniformResourceIdentifier(new_uri) = &new_name.base { - if self.uniform_resource_identifier.is_empty() { - new_set.push(new_name.clone()); - } else { - for prev_name in &self.uniform_resource_identifier { - if let GeneralName::UniformResourceIdentifier(prev_uri) = - &prev_name.base + for new_name in new_names { + if let GeneralName::UniformResourceIdentifier(new_uri) = &new_name.base { + if self.uniform_resource_identifier.is_empty() { + new_set.push(new_name.clone()); + } else { + for prev_name in &self.uniform_resource_identifier { + if let GeneralName::UniformResourceIdentifier(prev_uri) = &prev_name.base { + if new_name == prev_name + || descended_from_host(prev_uri, new_uri.as_str(), true) { - if new_name == prev_name - || descended_from_host(prev_uri, new_uri.as_str(), true) - { - new_set.push(prev_name.clone()); - } + new_set.push(prev_name.clone()); } } } } } + } - if !new_set.is_empty() { - self.uniform_resource_identifier = new_set; - } else { - self.uniform_resource_identifier_null = true; - } + if !new_set.is_empty() { + self.uniform_resource_identifier = new_set; + } else { + self.uniform_resource_identifier_null = true; } } diff --git a/certval/tests/examples/wire_certchain/ee.pem b/certval/tests/examples/wire_certchain/ee.pem index 21a35ba..8a14ed6 100644 --- a/certval/tests/examples/wire_certchain/ee.pem +++ b/certval/tests/examples/wire_certchain/ee.pem @@ -1,14 +1,14 @@ -----BEGIN CERTIFICATE----- -MIICJTCCAdegAwIBAgICY4EwBQYDK2VwMD8xKTAnBgNVBAMMIFdvcmxkIERvbWlu +MIICJTCCAdegAwIBAgICHlwwBQYDK2VwMD8xKTAnBgNVBAMMIFdvcmxkIERvbWlu YXRpb24gSW50ZXJtZWRpYXRlIENBMRIwEAYDVQQKDAl3b3JsZC5jb20wHhcNMjQw -MjA4MTAwODEzWhcNMjQwMjA5MTAwODEzWjAkMQ4wDAYDVQQDDAVhbGljZTESMBAG -A1UECgwJd29ybGQuY29tMCowBQYDK2VwAyEAaLFURMk54SzsvyospqSdDajBN05i -7/H/hru6p9KNXpKjggEQMIIBDDAdBgNVHQ4EFgQURT1grcplwsHDbrjcKG5lI175 -wmwwHwYDVR0jBBgwFoAUZ7XAzZy/AgTIXbV23QWmC0fkPm0wDAYDVR0TAQH/BAIw +MjA4MTUwMjQ1WhcNMjQwMjA5MTUwMjQ1WjAkMQ4wDAYDVQQDDAVhbGljZTESMBAG +A1UECgwJd29ybGQuY29tMCowBQYDK2VwAyEA0G4TeiZCFMraoLDqtJh+zmoV7Hb6 +CtuvjqbGLWz5MZijggEQMIIBDDAdBgNVHQ4EFgQU6459bMyGJtPLefC6zHYeDLIY +r54wHwYDVR0jBBgwFoAUFwQfJ5eF8CQwbAdZI9gdFKomhm8wDAYDVR0TAQH/BAIw ADAOBgNVHQ8BAf8EBAMCBsAwFgYDVR0lAQH/BAwwCgYIKwYBBQUHAwIwaQYDVR0R BGIwYIYhd2lyZWFwcDovLyU0MGFsaWNlX3dpcmVAd29ybGQuY29thjt3aXJlYXBw -Oi8vaVU5WDJwMEZUME9GQjNGWmRhQ3BKUSEzZTdkNzUzNjEwZTNiOWQwQHdvcmxk +Oi8vbG4ybVp6ZktTTUNEWktVWTBkMnJLUSFlMzE5ZDNmYTZkOWU4NjRiQHdvcmxk LmNvbTApBgNVHR8EIjAgMB6gHKAahhhodHRwOi8vd29ybGQuY29tL2NybC5kZXIw -BQYDK2VwA0EAiZCoOy6TYH+4KZIzDRSMZKHXujrYzfeQkxp6d9P3FrJ9sEmdw6uw -6FeVOrOy/KfMrXE/lpNZmoDoZUCAElVfBg== +BQYDK2VwA0EAT6O/VmMasdZppGWSOV8cn184ShJrC5Y0+8Lm/RHGIXgtnkJFzTfo +8xRq8Ei/g6BZonXhYEiLRAUUH5W5/sq/AQ== -----END CERTIFICATE----- diff --git a/certval/tests/examples/wire_certchain/intermediate.pem b/certval/tests/examples/wire_certchain/intermediate.pem index 3d82543..ceb2274 100644 --- a/certval/tests/examples/wire_certchain/intermediate.pem +++ b/certval/tests/examples/wire_certchain/intermediate.pem @@ -1,14 +1,14 @@ -----BEGIN CERTIFICATE----- -MIICFjCCAcigAwIBAgIDAPhEMAUGAytlcDA3MSEwHwYDVQQDDBhXb3JsZCBEb21p -bmF0aW9uIFJvb3QgQ0ExEjAQBgNVBAoMCXdvcmxkLmNvbTAeFw0yNDAyMDgxMDA4 -MTNaFw0yNDAyMDkxMDA4MTNaMD8xKTAnBgNVBAMMIFdvcmxkIERvbWluYXRpb24g -SW50ZXJtZWRpYXRlIENBMRIwEAYDVQQKDAl3b3JsZC5jb20wKjAFBgMrZXADIQC2 -JnfxbzRMMu6JGdW2/UbKZJ00WOVOaZxc5PJNIJaCO6OB7jCB6zAdBgNVHQ4EFgQU -Z7XAzZy/AgTIXbV23QWmC0fkPm0wHwYDVR0jBBgwFoAU+RgSMrMip3POiLXQVdm7 -9hC4BfswEgYDVR0TAQH/BAgwBgEB/wIBATAOBgNVHQ8BAf8EBAMCAQYwDAYDVR0l +MIICFjCCAcigAwIBAgIDAP8GMAUGAytlcDA3MSEwHwYDVQQDDBhXb3JsZCBEb21p +bmF0aW9uIFJvb3QgQ0ExEjAQBgNVBAoMCXdvcmxkLmNvbTAeFw0yNDAyMDgxNTAy +NDVaFw0yNDAyMDkxNTAyNDVaMD8xKTAnBgNVBAMMIFdvcmxkIERvbWluYXRpb24g +SW50ZXJtZWRpYXRlIENBMRIwEAYDVQQKDAl3b3JsZC5jb20wKjAFBgMrZXADIQCP +4meiDjZPFghug7xNW6jXEkmms5+mt5iAfPaHxoBOnKOB7jCB6zAdBgNVHQ4EFgQU +FwQfJ5eF8CQwbAdZI9gdFKomhm8wHwYDVR0jBBgwFoAUFxoIqSsBxqoHJFxoiTDv +1nTd+90wEgYDVR0TAQH/BAgwBgEB/wIBATAOBgNVHQ8BAf8EBAMCAQYwDAYDVR0l AQH/BAIwADAUBgNVHREEDTALggl3b3JsZC5jb20wNgYDVR0eAQH/BCwwKqAoMAyG Ci53b3JsZC5jb20wC4YJd29ybGQuY29tMAuCCXdvcmxkLmNvbTApBgNVHR8EIjAg -MB6gHKAahhhodHRwOi8vd29ybGQuY29tL2NybC5kZXIwBQYDK2VwA0EAGCv0VhjG -wukXyeykvTvbY4UuFfROVavNOioTRrCcUb47QxR5uqC5gHlgRTYEfs2ZzoPYUWQb -a5KA4jKBCOw1Cw== +MB6gHKAahhhodHRwOi8vd29ybGQuY29tL2NybC5kZXIwBQYDK2VwA0EAPEp3rWHR +V/jqBNKnoEHHQwBrdYQHUdAywMejVLM+t94gtkFOKCuhimSxBCxEA50obY22qz0A +cBkxtwRbULCrBA== -----END CERTIFICATE----- diff --git a/certval/tests/examples/wire_certchain/ta.pem b/certval/tests/examples/wire_certchain/ta.pem index 9b55a86..31094c1 100644 --- a/certval/tests/examples/wire_certchain/ta.pem +++ b/certval/tests/examples/wire_certchain/ta.pem @@ -1,12 +1,12 @@ -----BEGIN CERTIFICATE----- -MIIB0jCCAYSgAwIBAgICNaIwBQYDK2VwMDcxITAfBgNVBAMMGFdvcmxkIERvbWlu -YXRpb24gUm9vdCBDQTESMBAGA1UECgwJd29ybGQuY29tMB4XDTI0MDIwODEwMDgx -M1oXDTI0MDIwOTEwMDgxM1owNzEhMB8GA1UEAwwYV29ybGQgRG9taW5hdGlvbiBS -b290IENBMRIwEAYDVQQKDAl3b3JsZC5jb20wKjAFBgMrZXADIQCvOTnU9T7DoKBf -Xuv13HqkhatDHlIk97k2O53su7K36qOBszCBsDAdBgNVHQ4EFgQU+RgSMrMip3PO -iLXQVdm79hC4BfswDwYDVR0TAQH/BAUwAwEB/zAOBgNVHQ8BAf8EBAMCAQYwHwYD -VR0jBBgwFoAU+RgSMrMip3POiLXQVdm79hC4BfswDAYDVR0lAQH/BAIwADAUBgNV -HREEDTALggl3b3JsZC5jb20wKQYDVR0fBCIwIDAeoBygGoYYaHR0cDovL3dvcmxk -LmNvbS9jcmwuZGVyMAUGAytlcANBAENjvG0I5KI+ZhggbdW/86+Wtm6LYRs/SIX/ -NnqjXCxgIHha7Q5+X/jbM4K8WhuShcdA4VaizNDPzdNR9/7RIA8= +MIIB0zCCAYWgAwIBAgIDAPoOMAUGAytlcDA3MSEwHwYDVQQDDBhXb3JsZCBEb21p +bmF0aW9uIFJvb3QgQ0ExEjAQBgNVBAoMCXdvcmxkLmNvbTAeFw0yNDAyMDgxNTAy +NDVaFw0yNDAyMDkxNTAyNDVaMDcxITAfBgNVBAMMGFdvcmxkIERvbWluYXRpb24g +Um9vdCBDQTESMBAGA1UECgwJd29ybGQuY29tMCowBQYDK2VwAyEAgbE5S4EEPUMp +OygHNYD+6HwICVkobV3vDxu5fLTeKvCjgbMwgbAwHQYDVR0OBBYEFBcaCKkrAcaq +ByRcaIkw79Z03fvdMA8GA1UdEwEB/wQFMAMBAf8wDgYDVR0PAQH/BAQDAgEGMB8G +A1UdIwQYMBaAFBcaCKkrAcaqByRcaIkw79Z03fvdMAwGA1UdJQEB/wQCMAAwFAYD +VR0RBA0wC4IJd29ybGQuY29tMCkGA1UdHwQiMCAwHqAcoBqGGGh0dHA6Ly93b3Js +ZC5jb20vY3JsLmRlcjAFBgMrZXADQQAxEx7MGOXv60LuKOyKaAu+PGCoZJ+Tr4or +sp4PuUAlGreciWB+3AbqVqyTXtAwNv2tOyP6VOMEOkbr8dUSDcsG -----END CERTIFICATE----- diff --git a/certval/tests/path_validator.rs b/certval/tests/path_validator.rs index 4dd25f2..72b216d 100644 --- a/certval/tests/path_validator.rs +++ b/certval/tests/path_validator.rs @@ -186,7 +186,7 @@ fn denies_self_signed_ee() { fn wire_certchain_works() { let _ = pretty_env_logger::try_init(); - let time_of_interest: TimeOfInterest = TimeOfInterest::from_unix_secs(1707402960).unwrap(); + let time_of_interest: TimeOfInterest = TimeOfInterest::from_unix_secs(1707405529).unwrap(); let mut pe = certval::environment::PkiEnvironment::default(); pe.populate_5280_pki_environment(); @@ -242,5 +242,7 @@ fn wire_certchain_works() { for path in &mut paths { let mut cpr = CertificationPathResults::new(); let _ = validate_path_rfc5280(&pe, &cps, path, &mut cpr).unwrap(); + let validation_status = cpr.get_validation_status().unwrap(); + assert_eq!(validation_status, PathValidationStatus::Valid); } } From cfa8531672772e75839690218a032c629c8d3410 Mon Sep 17 00:00:00 2001 From: Mathieu Amiot Date: Wed, 14 Feb 2024 16:55:34 +0100 Subject: [PATCH 12/23] fix: Return expired cert error when computing paths instead of nothing --- certval/Cargo.toml | 3 +-- certval/src/environment/pki_environment.rs | 12 +++++++----- certval/src/revocation/crl.rs | 8 ++++---- certval/src/source/cert_source.rs | 4 ++-- certval/src/util/error.rs | 10 ++++++++++ certval/src/util/pdv_utilities.rs | 3 +++ certval/tests/path_validator.rs | 5 +++-- pittv3/tests/pittv3.rs | 12 ++++++++++-- 8 files changed, 40 insertions(+), 17 deletions(-) diff --git a/certval/Cargo.toml b/certval/Cargo.toml index 923b759..cfceaf5 100644 --- a/certval/Cargo.toml +++ b/certval/Cargo.toml @@ -52,7 +52,6 @@ log = {version = "0.4.20", default-features = false} ndarray = {version = "0.15.6", optional = true, default-features = false} reqwest = { version = "0.11.22", features = ["blocking"], optional = true} serde_json = {version = "1.0.108", optional = true, default-features = false, features = ["alloc"] } -tokio = { version = "1.33.0", features = ["full", "time", "rt-multi-thread"], optional = true } url = {version = "2.4.1", optional = true} walkdir = { version = "2.4.0", optional = true} @@ -74,7 +73,7 @@ tempfile = "3.8.1" hex-literal = "0.4.1" tokio-test = "0.4.2" pretty_env_logger = "0.5" -tokio = { version = "1.33.0", features = ["full", "time", "rt-multi-thread"] } +tokio = { version = "1.33.0", features = ["full", "macros", "time", "rt-multi-thread"] } # There are five feature gates: diff --git a/certval/src/environment/pki_environment.rs b/certval/src/environment/pki_environment.rs index f558b34..be47215 100644 --- a/certval/src/environment/pki_environment.rs +++ b/certval/src/environment/pki_environment.rs @@ -476,17 +476,19 @@ impl PkiEnvironment { time_of_interest: TimeOfInterest, ) -> Result<()> { let mut some_valid = false; + let mut last_error = Error::Unrecognized; for f in &self.certificate_sources { - if f.get_paths_for_target(self, target, paths, threshold, time_of_interest) - .is_ok() - { - some_valid = true; + match f.get_paths_for_target(self, target, paths, threshold, time_of_interest) { + Ok(_) => some_valid = true, + Err(e) => { + last_error = e; + } } } if some_valid { Ok(()) } else { - Err(Error::Unrecognized) + Err(last_error) } } diff --git a/certval/src/revocation/crl.rs b/certval/src/revocation/crl.rs index a2140ee..aaeb7eb 100644 --- a/certval/src/revocation/crl.rs +++ b/certval/src/revocation/crl.rs @@ -1184,15 +1184,15 @@ pub(crate) async fn check_revocation_crl_remote( #[cfg(test)] mod tests { - use crate::*; + use crate::crl::fetch_crl; + use crate::util::Error; + use crate::PkiEnvironment; #[cfg(feature = "remote")] #[tokio::test] async fn fetch_crl_test() { - use crate::{CrlSourceFolders, RemoteStatus, RevocationCache}; + use crate::{CrlSourceFolders, RemoteStatus, RevocationCache, TimeOfInterest}; use std::{path::PathBuf, time::Duration}; - - use self::crl::fetch_crl; let mut pe = PkiEnvironment::default(); pe.clear_all_callbacks(); pe.populate_5280_pki_environment(); diff --git a/certval/src/source/cert_source.rs b/certval/src/source/cert_source.rs index db2c87c..f27b303 100644 --- a/certval/src/source/cert_source.rs +++ b/certval/src/source/cert_source.rs @@ -1359,13 +1359,13 @@ impl CertificateSource for CertSource { threshold: usize, time_of_interest: TimeOfInterest, ) -> Result<()> { - if let Err(_e) = valid_at_time(&target.decoded_cert.tbs_certificate, time_of_interest, true) + if let Err(e) = valid_at_time(&target.decoded_cert.tbs_certificate, time_of_interest, true) { error!( "No paths found because target is not valid at indicated time of interest ({})", time_of_interest ); - return Ok(()); + return Err(e); } let ta = pe.get_trust_anchor_for_target(target); diff --git a/certval/src/util/error.rs b/certval/src/util/error.rs index 32f3701..1784316 100644 --- a/certval/src/util/error.rs +++ b/certval/src/util/error.rs @@ -130,6 +130,16 @@ pub enum Error { StdIoError(std::io::ErrorKind), } +impl Error { + /// Returns true if the error returned is for an expired certificate + pub fn is_certificate_expired_error(&self) -> bool { + matches!( + self, + Self::PathValidation(PathValidationStatus::InvalidNotAfterDate) + ) + } +} + impl From for Error { fn from(err: der::Error) -> Error { Error::Asn1Error(err) diff --git a/certval/src/util/pdv_utilities.rs b/certval/src/util/pdv_utilities.rs index 3a971f9..7020f16 100644 --- a/certval/src/util/pdv_utilities.rs +++ b/certval/src/util/pdv_utilities.rs @@ -128,6 +128,8 @@ pub fn valid_at_time( } let nb = target.validity.not_before; + dbg!(toi.as_unix_secs()); + dbg!(nb.to_unix_duration().as_secs()); if nb > toi { if !stifle_log { log_error_for_name(&target.subject, "certificate is not yet valid, i.e., not_before is prior to the configured time of interest"); @@ -138,6 +140,7 @@ pub fn valid_at_time( } let na = target.validity.not_after; + dbg!(na.to_unix_duration().as_secs()); if na < toi { if !stifle_log { log_error_for_name( diff --git a/certval/tests/path_validator.rs b/certval/tests/path_validator.rs index 72b216d..6a36e5f 100644 --- a/certval/tests/path_validator.rs +++ b/certval/tests/path_validator.rs @@ -165,8 +165,9 @@ fn denies_self_signed_ee() { cps.set_time_of_interest(time_of_interest); let mut paths = vec![]; - pe.get_paths_for_target(&cert, &mut paths, 0, cps.get_time_of_interest()) - .unwrap(); + if let Err(e) = pe.get_paths_for_target(&cert, &mut paths, 0, cps.get_time_of_interest()) { + assert!(e.is_certificate_expired_error()); + } if paths.is_empty() { return; diff --git a/pittv3/tests/pittv3.rs b/pittv3/tests/pittv3.rs index 646e6a3..789721d 100644 --- a/pittv3/tests/pittv3.rs +++ b/pittv3/tests/pittv3.rs @@ -417,6 +417,7 @@ fn empty_cbor1() -> Result<(), Box> { // Try static building with empty CBOR to affirm it fails let mut cmd = Command::cargo_bin("pittv3")?; cmd.arg("--cbor").arg("tests/examples/empty.cbor"); + cmd.arg("--time-of-interest").arg("1674179800"); cmd.arg("-t").arg("tests/examples/ta_store_one"); cmd.arg("-e") .arg("tests/examples/end_entities/from_email_CA_63.der"); @@ -475,6 +476,7 @@ fn empty_cbor2() -> Result<(), Box> { // Try static building with empty CBOR to affirm it fails let mut cmd = Command::cargo_bin("pittv3")?; cmd.arg("--cbor").arg("tests/examples/empty.cbor"); + cmd.arg("--time-of-interest").arg("1674179800"); cmd.arg("-t").arg("tests/examples/ta_store_one"); cmd.arg("-e") .arg("tests/examples/end_entities/from_email_CA_63.der"); @@ -534,6 +536,7 @@ fn empty_cbor3() -> Result<(), Box> { // Try static building with empty CBOR to affirm it fails let mut cmd = Command::cargo_bin("pittv3")?; cmd.arg("--cbor").arg("tests/examples/empty.cbor"); + cmd.arg("--time-of-interest").arg("1674179800"); cmd.arg("-t").arg("tests/examples/ta_store_one"); cmd.arg("-e") .arg("tests/examples/end_entities/from_email_CA_63.der"); @@ -593,6 +596,7 @@ fn empty_cbor4() -> Result<(), Box> { // Try static building with empty CBOR to affirm it fails let mut cmd = Command::cargo_bin("pittv3")?; cmd.arg("--cbor").arg("tests/examples/empty.cbor"); + cmd.arg("--time-of-interest").arg("1674179800"); cmd.arg("-t").arg("tests/examples/ta_store_one"); cmd.arg("-e") .arg("tests/examples/end_entities/from_email_CA_63.der"); @@ -640,6 +644,7 @@ fn absent_cbor1() -> Result<(), Box> { // Try static building with empty CBOR to affirm it fails let mut cmd = Command::cargo_bin("pittv3")?; cmd.arg("-t").arg("tests/examples/ta_store_one"); + cmd.arg("--time-of-interest").arg("1674179800"); cmd.arg("-e") .arg("tests/examples/end_entities/from_email_CA_63.der"); cmd.assert().stdout(predicate::str::contains( @@ -687,6 +692,7 @@ fn absent_cbor2() -> Result<(), Box> { { // Try static building with empty CBOR to affirm it fails let mut cmd = Command::cargo_bin("pittv3")?; + cmd.arg("--time-of-interest").arg("1674179800"); cmd.arg("-t").arg("tests/examples/ta_store_one"); cmd.arg("-e") .arg("tests/examples/end_entities/from_email_CA_63.der"); @@ -736,6 +742,7 @@ fn absent_cbor3() -> Result<(), Box> { { // Try static building with empty CBOR to affirm it fails let mut cmd = Command::cargo_bin("pittv3")?; + cmd.arg("--time-of-interest").arg("1674179800"); cmd.arg("-t").arg("tests/examples/ta_store_one"); cmd.arg("-e") .arg("tests/examples/end_entities/from_email_CA_63.der"); @@ -784,6 +791,7 @@ fn absent_cbor4() -> Result<(), Box> { { // Try static building with empty CBOR to affirm it fails let mut cmd = Command::cargo_bin("pittv3")?; + cmd.arg("--time-of-interest").arg("1674179800"); cmd.arg("-t").arg("tests/examples/ta_store_one"); cmd.arg("-e") .arg("tests/examples/end_entities/from_email_CA_63.der"); @@ -1168,7 +1176,7 @@ fn generate_then_validate_with_expired() -> Result<(), Box Result<(), Box> { cmd.assert() .stdout(predicate::str::contains("Total paths found: 0")); cmd.assert().stdout(predicate::str::contains( - "Failed to find any certification paths for target", + "Failed to find certification paths for target with error PathValidation(InvalidNotAfterDate)", )); } Ok(()) From 04c5568c440a3568f492933b4b10c2d8e79f5656 Mon Sep 17 00:00:00 2001 From: Mathieu Amiot Date: Mon, 19 Feb 2024 12:19:32 +0100 Subject: [PATCH 13/23] feat: Added accessor for intermediates by SKI --- certval/src/environment/pki_environment.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/certval/src/environment/pki_environment.rs b/certval/src/environment/pki_environment.rs index be47215..a08c1aa 100644 --- a/certval/src/environment/pki_environment.rs +++ b/certval/src/environment/pki_environment.rs @@ -389,6 +389,17 @@ impl PkiEnvironment { Err(Error::Unrecognized) } + /// Fetches all intermediate certs matching a particular skid + pub fn get_intermediates_by_skid(&self, skid: &[u8]) -> Result> { + for f in &self.certificate_sources { + let r = f.get_certificates_for_skid(skid); + if let Ok(r) = r { + return Ok(r); + } + } + Err(Error::Unrecognized) + } + /// add_crl_source adds a [`CrlSource`] object to the list. pub fn add_crl_source(&mut self, c: Box<(dyn CrlSource + Send + Sync)>) { self.crl_sources.push(c); From 3966c2f517c5111e18879eb2f1684956c222ca73 Mon Sep 17 00:00:00 2001 From: Mathieu Amiot Date: Mon, 19 Feb 2024 18:36:39 +0100 Subject: [PATCH 14/23] fix: Set validation status as Valid upon revocation check --- certval/src/revocation/check_revocation.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/certval/src/revocation/check_revocation.rs b/certval/src/revocation/check_revocation.rs index 374f81f..f153aa9 100644 --- a/certval/src/revocation/check_revocation.rs +++ b/certval/src/revocation/check_revocation.rs @@ -410,6 +410,7 @@ pub fn check_revocation( cpr.set_validation_status(RevocationStatusNotDetermined); Err(Error::PathValidation(RevocationStatusNotDetermined)) } else { + set_validation_status(cpr, Valid); Ok(()) } } From 7ca6851ee4c5ee04783f8c3ae8c2d2b87a3a2ef9 Mon Sep 17 00:00:00 2001 From: Mathieu Amiot Date: Tue, 20 Feb 2024 12:16:24 +0100 Subject: [PATCH 15/23] fix: Don't fail on missing BasicConstraints in EE certs --- certval/src/validator/path_validator.rs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/certval/src/validator/path_validator.rs b/certval/src/validator/path_validator.rs index af93541..9ee5f55 100644 --- a/certval/src/validator/path_validator.rs +++ b/certval/src/validator/path_validator.rs @@ -184,19 +184,12 @@ pub fn check_basic_constraints( if cps.get_forbid_self_signed_ee() { let pdv_ext: Option<&PDVExtension> = cp.target.get_extension(&ID_CE_BASIC_CONSTRAINTS)?; - let bc = match pdv_ext { - Some(PDVExtension::BasicConstraints(bc)) => bc, - _ => { - log_error_for_ca(&cp.target, "missing basic constraints"); - cpr.set_validation_status(PathValidationStatus::MissingBasicConstraints); - return Err(Error::PathValidation( - PathValidationStatus::MissingBasicConstraints, - )); - } + let is_ee = if let Some(PDVExtension::BasicConstraints(bc)) = pdv_ext { + !bc.ca + } else { + true }; - let is_ee = !bc.ca; - if is_ee && (is_self_issued(&cp.target.decoded_cert) || is_self_signed(pe, &cp.target)) { log_error_for_ca( &cp.target, From a5ef377815904d73292855bed9ce76ba0c512370 Mon Sep 17 00:00:00 2001 From: Mathieu Amiot Date: Wed, 6 Mar 2024 10:12:35 +0100 Subject: [PATCH 16/23] fix: Use more robust CRL serial comparison --- certval/src/revocation/crl.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/certval/src/revocation/crl.rs b/certval/src/revocation/crl.rs index aaeb7eb..e5d17dd 100644 --- a/certval/src/revocation/crl.rs +++ b/certval/src/revocation/crl.rs @@ -15,7 +15,7 @@ use const_oid::db::rfc5912::{ ID_CE_FRESHEST_CRL, ID_CE_HOLD_INSTRUCTION_CODE, ID_CE_INVALIDITY_DATE, ID_CE_ISSUING_DISTRIBUTION_POINT, ID_CE_KEY_USAGE, }; -use der::{Decode, Encode}; +use der::{Decode, DerOrd, Encode}; use x509_cert::ext::pkix::crl::dp::ReasonFlags; use x509_cert::ext::pkix::{ crl::dp::DistributionPoint, @@ -1071,7 +1071,12 @@ pub(crate) fn process_crl( return Err(Error::UnsupportedIndirectCrl); } - if rc.serial_number == target_cert.decoded_cert.tbs_certificate.serial_number { + if rc + .serial_number + .der_cmp(&target_cert.decoded_cert.tbs_certificate.serial_number) + .map(|ordering| matches!(ordering, std::cmp::Ordering::Equal)) + .unwrap_or_default() + { // this is probably not a useful check. will change ultimate error from revoked to // status not determined, most likely. if let Err(_e) = check_entry_extensions(&rc) { @@ -1184,14 +1189,15 @@ pub(crate) async fn check_revocation_crl_remote( #[cfg(test)] mod tests { - use crate::crl::fetch_crl; use crate::util::Error; use crate::PkiEnvironment; #[cfg(feature = "remote")] #[tokio::test] async fn fetch_crl_test() { - use crate::{CrlSourceFolders, RemoteStatus, RevocationCache, TimeOfInterest}; + use crate::{ + crl::fetch_crl, CrlSourceFolders, RemoteStatus, RevocationCache, TimeOfInterest, + }; use std::{path::PathBuf, time::Duration}; let mut pe = PkiEnvironment::default(); pe.clear_all_callbacks(); From 6c2607272ab55647a3ef305869ea1bf50a6dc1ae Mon Sep 17 00:00:00 2001 From: Mathieu Amiot Date: Mon, 11 Mar 2024 16:33:12 +0100 Subject: [PATCH 17/23] chore: Misc fixes --- certval/src/revocation/crl.rs | 18 ++++++++++++++---- certval/src/source/ta_source.rs | 10 +++++++--- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/certval/src/revocation/crl.rs b/certval/src/revocation/crl.rs index e5d17dd..f8d1dda 100644 --- a/certval/src/revocation/crl.rs +++ b/certval/src/revocation/crl.rs @@ -519,14 +519,24 @@ pub(crate) fn get_crl_info(crl: &CertificateList) -> Result { match &idp.distribution_point { Some(DistributionPointName::FullName(gns)) => { for gn in gns { - if let GeneralName::DirectoryName(dn) = gn { - idp_name = Some(name_to_string(dn)); + match gn { + GeneralName::DirectoryName(dn) => { + idp_name.replace(name_to_string(dn)); + } + GeneralName::UniformResourceIdentifier(uri) => { + let uri_str: &str = uri.as_ref(); + idp_name.replace(uri_str.to_string()); + } + _ => {} + } + + if idp_name.is_some() { break; } } if idp_name.is_none() { - // not supporting non-DN DPs - return Err(Error::Unrecognized); + // not supporting non-DN/URI DPs + return Err(Error::Unrecognized.into()); } } Some(DistributionPointName::NameRelativeToCRLIssuer(_unsupported)) => { diff --git a/certval/src/source/ta_source.rs b/certval/src/source/ta_source.rs index d577340..2cbc806 100644 --- a/certval/src/source/ta_source.rs +++ b/certval/src/source/ta_source.rs @@ -334,12 +334,12 @@ impl TrustAnchorSource for TaSource { &self, target: &PDVCertificate, ) -> Result<&PDVTrustAnchorChoice> { - let mut akid_hex = "".to_string(); + let mut akid_hex = None; let mut name_vec = vec![&target.decoded_cert.tbs_certificate.issuer]; let akid_ext = target.get_extension(&ID_CE_AUTHORITY_KEY_IDENTIFIER); if let Ok(Some(PDVExtension::AuthorityKeyIdentifier(akid))) = akid_ext { if let Some(kid) = &akid.key_identifier { - akid_hex = buffer_to_hex(kid.as_bytes()); + akid_hex.replace(buffer_to_hex(kid.as_bytes())); } else if let Some(names) = &akid.authority_cert_issuer { for n in names { if let GeneralName::DirectoryName(dn) = n { @@ -348,7 +348,8 @@ impl TrustAnchorSource for TaSource { } } } - if !akid_hex.is_empty() { + + if let Some(akid_hex) = akid_hex { match self.get_trust_anchor_by_hex_skid(&akid_hex) { Ok(s) => return Ok(s), Err(_e) => { @@ -356,9 +357,12 @@ impl TrustAnchorSource for TaSource { } } } + for n in name_vec { let r = self.get_trust_anchor_by_name(n); + if r.is_ok() { + info!("Found trust anchor by name: {n}"); return r; } } From 6eac197ccd8caaa50e37d906320a7f3bb57f01f2 Mon Sep 17 00:00:00 2001 From: Mathieu Amiot Date: Wed, 13 Mar 2024 14:37:50 +0100 Subject: [PATCH 18/23] feat: Added ability to list all CRLs in-store --- certval/src/environment/pki_environment.rs | 13 ++++++++++++ .../src/environment/pki_environment_traits.rs | 2 ++ certval/src/source/crl_source.rs | 20 +++++++++++++++++++ certval/src/util/error.rs | 3 +++ certval/src/util/pdv_utilities.rs | 3 --- 5 files changed, 38 insertions(+), 3 deletions(-) diff --git a/certval/src/environment/pki_environment.rs b/certval/src/environment/pki_environment.rs index a08c1aa..c4e534a 100644 --- a/certval/src/environment/pki_environment.rs +++ b/certval/src/environment/pki_environment.rs @@ -410,6 +410,19 @@ impl PkiEnvironment { self.crl_sources.clear(); } + /// Retrieves all the CRLs made available by the various [`CrlSource`] objects + pub fn get_all_crls(&self) -> Result>> { + let mut retval = vec![]; + for f in &self.crl_sources { + let Ok(mut crls) = f.get_all_crls() else { + continue; + }; + retval.append(&mut crls); + } + retval.dedup(); + Ok(retval) + } + /// Retrieves CRLs for given certificate from store pub fn get_crls(&self, cert: &PDVCertificate) -> Result>> { let mut retval = vec![]; diff --git a/certval/src/environment/pki_environment_traits.rs b/certval/src/environment/pki_environment_traits.rs index 64b68ab..7512c32 100644 --- a/certval/src/environment/pki_environment_traits.rs +++ b/certval/src/environment/pki_environment_traits.rs @@ -146,6 +146,8 @@ pub enum CertificationPathBuilderFormats { /// The [`CrlSource`] trait defines the interface for storing and retrieving CRLs in support of certification path validation. pub trait CrlSource { + /// Lists all CRLs available + fn get_all_crls(&self) -> Result>>; /// Retrieves CRLs for given certificate from store fn get_crls(&self, cert: &PDVCertificate) -> Result>>; /// Adds a CRL to the store diff --git a/certval/src/source/crl_source.rs b/certval/src/source/crl_source.rs index c9b5562..a7eae12 100644 --- a/certval/src/source/crl_source.rs +++ b/certval/src/source/crl_source.rs @@ -116,6 +116,22 @@ impl CrlSourceFolders { ) } + fn read_all_crls(&self) -> Result>> { + let crl_info_guard = self.inner.read().map_err(|_| Error::LockGuardError)?; + let crl_info = crl_info_guard.crl_info.as_slice(); + let mut retval = vec![]; + for ci in crl_info.iter() { + let Some(filename) = &ci.filename else { + continue; + }; + + if let Ok(crl_buf) = get_file_as_byte_vec_pem(Path::new(filename.as_str())) { + retval.push(crl_buf); + } + } + Ok(retval) + } + fn read_crl_at_index(&self, index: usize) -> Option> { let inner = self.inner.read().ok()?; let ci = &inner.crl_info[index]; @@ -259,6 +275,10 @@ impl CheckRemoteResource for RemoteStatus { } impl CrlSource for CrlSourceFolders { + fn get_all_crls(&self) -> Result>> { + self.read_all_crls() + } + fn add_crl(&self, crl_buf: &[u8], crl: &CertificateList, uri: &str) -> Result<()> { let mut cur_crl_info = get_crl_info(crl)?; let digest = Sha256::digest(uri).to_vec(); diff --git a/certval/src/util/error.rs b/certval/src/util/error.rs index 1784316..6291e18 100644 --- a/certval/src/util/error.rs +++ b/certval/src/util/error.rs @@ -128,6 +128,8 @@ pub enum Error { /// Error encapsulates an error derived from [std::io::ErrorKind] #[cfg(feature = "std")] StdIoError(std::io::ErrorKind), + /// Failed to obtain lock guard + LockGuardError, } impl Error { @@ -221,6 +223,7 @@ impl fmt::Display for Error { Error::ResourceUnchanged => write!(f, "ResourceUnchanged"), #[cfg(feature = "std")] Error::StdIoError(err) => write!(f, "StdError: {:?}", err), + &Error::LockGuardError => write!(f, "LockGuardError"), } } } diff --git a/certval/src/util/pdv_utilities.rs b/certval/src/util/pdv_utilities.rs index 7020f16..3a971f9 100644 --- a/certval/src/util/pdv_utilities.rs +++ b/certval/src/util/pdv_utilities.rs @@ -128,8 +128,6 @@ pub fn valid_at_time( } let nb = target.validity.not_before; - dbg!(toi.as_unix_secs()); - dbg!(nb.to_unix_duration().as_secs()); if nb > toi { if !stifle_log { log_error_for_name(&target.subject, "certificate is not yet valid, i.e., not_before is prior to the configured time of interest"); @@ -140,7 +138,6 @@ pub fn valid_at_time( } let na = target.validity.not_after; - dbg!(na.to_unix_duration().as_secs()); if na < toi { if !stifle_log { log_error_for_name( From 0540b8ec662435abd6311950f836dc08f5d5b37a Mon Sep 17 00:00:00 2001 From: Mathieu Amiot Date: Wed, 13 Mar 2024 14:38:33 +0100 Subject: [PATCH 19/23] fix: Use revocation cached state on not std too --- certval/src/revocation/check_revocation.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/certval/src/revocation/check_revocation.rs b/certval/src/revocation/check_revocation.rs index f153aa9..3c08f3a 100644 --- a/certval/src/revocation/check_revocation.rs +++ b/certval/src/revocation/check_revocation.rs @@ -308,6 +308,12 @@ pub fn check_revocation( // check revocation status cache let mut cur_status = pe.get_status(cur_cert, toi); + if CertificateRevoked == cur_status { + info!("Determined revocation status (revoked) using cached status for certificate issued to {}", cur_cert_subject); + set_validation_status(cpr, revoked_error); + return Err(Error::PathValidation(revoked_error)); + } + if let Ok(Some(PDVExtension::OcspNoCheck(_nc))) = ca_cert_ref.get_extension(&ID_PKIX_OCSP_NOCHECK) { From f1a2d46d3491d7a0951747c69388620ef00cdb67 Mon Sep 17 00:00:00 2001 From: Mathieu Amiot Date: Tue, 26 Mar 2024 13:22:19 +0100 Subject: [PATCH 20/23] feat: Added support for P521 --- Cargo.lock | 21 +++++++++++++++++++++ certval/Cargo.toml | 1 + certval/src/util/crypto.rs | 4 ++++ 3 files changed, 26 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 789052c..6adc37c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -281,6 +281,7 @@ dependencies = [ "ndarray", "p256", "p384", + "p521", "pem-rfc7468 1.0.0-rc.1", "pkiprocmacros", "pqckeys", @@ -1403,6 +1404,20 @@ dependencies = [ "sha2", ] +[[package]] +name = "p521" +version = "0.14.0-pre.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9ec5d919bea930a34a522bb1c95a89f559925deab255db2c2ffa174fc48df664" +dependencies = [ + "base16ct", + "ecdsa", + "elliptic-curve", + "primefield", + "primeorder", + "sha2", +] + [[package]] name = "parking_lot" version = "0.12.1" @@ -1713,6 +1728,12 @@ dependencies = [ "log", ] +[[package]] +name = "primefield" +version = "0.14.0-pre.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d3f2ce0fa9cccdaf216230d151ce51a15298aef50ad76081a830128ecbc6428a" + [[package]] name = "primeorder" version = "0.14.0-pre.1" diff --git a/certval/Cargo.toml b/certval/Cargo.toml index cfceaf5..8301eca 100644 --- a/certval/Cargo.toml +++ b/certval/Cargo.toml @@ -32,6 +32,7 @@ pkiprocmacros = { path = "../pkiprocmacros"} ecdsa = {version = "0.17.0-pre.7", default-features = false, features = ["der"]} p256 = {version = "0.14.0-pre.1", default-features = false, features = ["ecdsa", "ecdsa-core"]} p384 = {version = "0.14.0-pre.1", default-features = false, features = ["ecdsa"]} +p521 = {version = "0.14.0-pre.1", default-features = false, features = ["ecdsa"]} ed25519-dalek = { version = "2.2.0-pre", default-features = false, features = ["fast", "zeroize", "pkcs8"] } rsa = {version = "0.10.0-pre.2", default-features = false} sha1 = {version = "0.11.0-pre.4", default-features = false} diff --git a/certval/src/util/crypto.rs b/certval/src/util/crypto.rs index 8ee5b31..4c8b7d1 100644 --- a/certval/src/util/crypto.rs +++ b/certval/src/util/crypto.rs @@ -249,6 +249,7 @@ pub fn verify_signature_message_rust_crypto( let hash_to_verify = match signature_alg.oid { PKIXALG_ECDSA_WITH_SHA256 => Sha256::digest(message_to_verify).to_vec(), PKIXALG_ECDSA_WITH_SHA384 => Sha384::digest(message_to_verify).to_vec(), + PKIXALG_ECDSA_WITH_SHA512 => Sha512::digest(message_to_verify).to_vec(), _ => { error!( "Unrecognized or unsupported signature algorithm: {}", @@ -286,6 +287,9 @@ pub fn verify_signature_message_rust_crypto( PKIXALG_SECP384R1 => { verify_with_ecdsa!(p384) } + PKIXALG_SECP521R1 => { + verify_with_ecdsa!(p521) + } _ => { error!("Unrecognized or unsupported named curve: {}", named_curve); Err(Error::Unrecognized) From 4c549417456bf31bc20f67f9490eb5a36f2f81c9 Mon Sep 17 00:00:00 2001 From: Mathieu Amiot Date: Tue, 9 Apr 2024 18:08:02 +0200 Subject: [PATCH 21/23] fix: Correctly handle EcDSA OIDs --- certval/Cargo.toml | 2 +- certval/src/util/crypto.rs | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/certval/Cargo.toml b/certval/Cargo.toml index 8301eca..79c1fc2 100644 --- a/certval/Cargo.toml +++ b/certval/Cargo.toml @@ -30,7 +30,7 @@ pem-rfc7468 = { version="1.0.0-pre.0", features = ["alloc"]} pkiprocmacros = { path = "../pkiprocmacros"} ecdsa = {version = "0.17.0-pre.7", default-features = false, features = ["der"]} -p256 = {version = "0.14.0-pre.1", default-features = false, features = ["ecdsa", "ecdsa-core"]} +p256 = {version = "0.14.0-pre.1", default-features = false, features = ["ecdsa"]} p384 = {version = "0.14.0-pre.1", default-features = false, features = ["ecdsa"]} p521 = {version = "0.14.0-pre.1", default-features = false, features = ["ecdsa"]} ed25519-dalek = { version = "2.2.0-pre", default-features = false, features = ["fast", "zeroize", "pkcs8"] } diff --git a/certval/src/util/crypto.rs b/certval/src/util/crypto.rs index 4c8b7d1..2564985 100644 --- a/certval/src/util/crypto.rs +++ b/certval/src/util/crypto.rs @@ -63,7 +63,8 @@ pub(crate) fn is_rsa(oid: &ObjectIdentifier) -> bool { /// is_ecdsa returns true is the presented OID is one of [`PKIXALG_ECDSA_WITH_SHA224`], /// [`PKIXALG_ECDSA_WITH_SHA256`], [`PKIXALG_ECDSA_WITH_SHA384`] or [`PKIXALG_ECDSA_WITH_SHA512`] and false otherwise. pub(crate) fn is_ecdsa(oid: &ObjectIdentifier) -> bool { - *oid == PKIXALG_ECDSA_WITH_SHA256 + *oid == PKIXALG_EC_PUBLIC_KEY + || *oid == PKIXALG_ECDSA_WITH_SHA256 || *oid == PKIXALG_ECDSA_WITH_SHA384 || *oid == PKIXALG_ECDSA_WITH_SHA224 || *oid == PKIXALG_ECDSA_WITH_SHA512 @@ -246,6 +247,7 @@ pub fn verify_signature_message_rust_crypto( } } else if is_ecdsa(&signature_alg.oid) { let named_curve = get_named_curve_parameter(&spki.algorithm)?; + let hash_to_verify = match signature_alg.oid { PKIXALG_ECDSA_WITH_SHA256 => Sha256::digest(message_to_verify).to_vec(), PKIXALG_ECDSA_WITH_SHA384 => Sha384::digest(message_to_verify).to_vec(), From 7584d3350967987a5c7e673a9fd1c36ee6b1c3cf Mon Sep 17 00:00:00 2001 From: beltram Date: Tue, 16 Apr 2024 15:16:50 +0200 Subject: [PATCH 22/23] feat: add feature to exclude RSA crate --- certval/Cargo.toml | 3 ++- certval/src/util/crypto.rs | 51 +++++++++++++++++++++++++++----------- 2 files changed, 38 insertions(+), 16 deletions(-) diff --git a/certval/Cargo.toml b/certval/Cargo.toml index 79c1fc2..745b0a9 100644 --- a/certval/Cargo.toml +++ b/certval/Cargo.toml @@ -34,7 +34,7 @@ p256 = {version = "0.14.0-pre.1", default-features = false, features = ["ecdsa"] p384 = {version = "0.14.0-pre.1", default-features = false, features = ["ecdsa"]} p521 = {version = "0.14.0-pre.1", default-features = false, features = ["ecdsa"]} ed25519-dalek = { version = "2.2.0-pre", default-features = false, features = ["fast", "zeroize", "pkcs8"] } -rsa = {version = "0.10.0-pre.2", default-features = false} +rsa = {version = "0.10.0-pre.2", default-features = false, optional=true} sha1 = {version = "0.11.0-pre.4", default-features = false} sha2 = {version = "0.11.0-pre.4", default-features = false, features = ["oid"] } @@ -92,6 +92,7 @@ std = ["dep:ndarray", "dep:base64ct", "dep:walkdir", "ed25519-dalek/std", "dep:u remote = ["revocation", "std", "reqwest", "lazy_static/spin"] pqc = ["pqcrypto-internals", "pqcrypto-dilithium", "pqcrypto-falcon", "pqcrypto-sphincsplus", "pqcrypto", "pqcrypto-traits", "pqckeys"] webpki = ["webpki-roots"] +rsa = ["dep:rsa"] [package.metadata.docs.rs] all-features = true diff --git a/certval/src/util/crypto.rs b/certval/src/util/crypto.rs index 2564985..d577f34 100644 --- a/certval/src/util/crypto.rs +++ b/certval/src/util/crypto.rs @@ -8,15 +8,13 @@ use log::{debug, error}; #[cfg(feature = "pqc")] use der::Decode; use der::{asn1::ObjectIdentifier, AnyRef, Encode}; -use rsa::pkcs8::DecodePublicKey; -use rsa::{Pkcs1v15Sign, RsaPublicKey}; + use sha2::{Digest, Sha224, Sha256, Sha384, Sha512}; use spki::{AlgorithmIdentifierOwned, SubjectPublicKeyInfoOwned}; use crate::util::error::{Error, PathValidationStatus, Result}; use crate::{ environment::pki_environment::*, util::pdv_alg_oids::*, - util::pdv_utilities::get_hash_alg_from_sig_alg, }; #[cfg(feature = "pqc")] @@ -40,12 +38,13 @@ use pqcrypto_traits::sign::{DetachedSignature, PublicKey as OtherPublicKey}; /// At present, only the PKCS1v15Sign passing scheme is supported, relative to the /// [`PKIXALG_SHA224_WITH_RSA_ENCRYPTION`], [`PKIXALG_SHA256_WITH_RSA_ENCRYPTION`], /// [`PKIXALG_SHA384_WITH_RSA_ENCRYPTION`] and [`PKIXALG_SHA512_WITH_RSA_ENCRYPTION`] algorithm identifiers. -pub fn get_padding_scheme(signature_alg: &AlgorithmIdentifierOwned) -> Result { +#[cfg(feature = "rsa")] +pub fn get_padding_scheme(signature_alg: &AlgorithmIdentifierOwned) -> Result { match signature_alg.oid { - PKIXALG_SHA256_WITH_RSA_ENCRYPTION => Ok(Pkcs1v15Sign::new::()), - PKIXALG_SHA384_WITH_RSA_ENCRYPTION => Ok(Pkcs1v15Sign::new::()), - PKIXALG_SHA224_WITH_RSA_ENCRYPTION => Ok(Pkcs1v15Sign::new::()), - PKIXALG_SHA512_WITH_RSA_ENCRYPTION => Ok(Pkcs1v15Sign::new::()), + PKIXALG_SHA256_WITH_RSA_ENCRYPTION => Ok(rsa::Pkcs1v15Sign::new::()), + PKIXALG_SHA384_WITH_RSA_ENCRYPTION => Ok(rsa::Pkcs1v15Sign::new::()), + PKIXALG_SHA224_WITH_RSA_ENCRYPTION => Ok(rsa::Pkcs1v15Sign::new::()), + PKIXALG_SHA512_WITH_RSA_ENCRYPTION => Ok(rsa::Pkcs1v15Sign::new::()), _ => Err(Error::Unrecognized), } } @@ -93,30 +92,37 @@ pub(crate) fn is_ml_dsa_87_ipd(oid: &ObjectIdentifier) -> bool { pub(crate) fn is_falcon512(oid: &ObjectIdentifier) -> bool { *oid == OQ_FALCON_512 } + #[cfg(feature = "pqc")] pub(crate) fn is_falcon1024(oid: &ObjectIdentifier) -> bool { *oid == OQ_FALCON_1024 } + #[cfg(feature = "pqc")] pub(crate) fn is_slh_dsa_sha2_128f_ipd(oid: &ObjectIdentifier) -> bool { *oid == SLH_DSA_SHA2_128F_IPD } + #[cfg(feature = "pqc")] pub(crate) fn is_slh_dsa_sha2_128s_ipd(oid: &ObjectIdentifier) -> bool { *oid == SLH_DSA_SHA2_128S_IPD } + #[cfg(feature = "pqc")] pub(crate) fn is_slh_dsa_sha2_192f_ipd(oid: &ObjectIdentifier) -> bool { *oid == SLH_DSA_SHA2_192F_IPD } + #[cfg(feature = "pqc")] pub(crate) fn is_slh_dsa_sha2_192s_ipd(oid: &ObjectIdentifier) -> bool { *oid == SLH_DSA_SHA2_192S_IPD } + #[cfg(feature = "pqc")] pub(crate) fn is_slh_dsa_sha2_256f_ipd(oid: &ObjectIdentifier) -> bool { *oid == SLH_DSA_SHA2_256F_IPD } + #[cfg(feature = "pqc")] pub(crate) fn is_slh_dsa_sha2_256s_ipd(oid: &ObjectIdentifier) -> bool { *oid == SLH_DSA_SHA2_256S_IPD @@ -126,22 +132,27 @@ pub(crate) fn is_slh_dsa_sha2_256s_ipd(oid: &ObjectIdentifier) -> bool { pub(crate) fn is_slh_dsa_shake_128f_ipd(oid: &ObjectIdentifier) -> bool { *oid == SLH_DSA_SHAKE_128F_IPD } + #[cfg(feature = "pqc")] pub(crate) fn is_slh_dsa_shake_128s_ipd(oid: &ObjectIdentifier) -> bool { *oid == SLH_DSA_SHAKE_128S_IPD } + #[cfg(feature = "pqc")] pub(crate) fn is_slh_dsa_shake_192f_ipd(oid: &ObjectIdentifier) -> bool { *oid == SLH_DSA_SHAKE_192F_IPD } + #[cfg(feature = "pqc")] pub(crate) fn is_slh_dsa_shake_192s_ipd(oid: &ObjectIdentifier) -> bool { *oid == SLH_DSA_SHAKE_192S_IPD } + #[cfg(feature = "pqc")] pub(crate) fn is_slh_dsa_shake_256f_ipd(oid: &ObjectIdentifier) -> bool { *oid == SLH_DSA_SHAKE_256F_IPD } + #[cfg(feature = "pqc")] pub(crate) fn is_slh_dsa_shake_256s_ipd(oid: &ObjectIdentifier) -> bool { *oid == SLH_DSA_SHAKE_256S_IPD @@ -181,6 +192,7 @@ pub fn calculate_hash_rust_crypto( /// implementations from the [Rust Crypto](https://github.com/RustCrypto) project. /// /// Only RSA is supported by this function. To verify ECDSA signatures, use [`verify_signature_message_rust_crypto`]. +#[allow(unused_variables)] pub fn verify_signature_digest_rust_crypto( _pe: &PkiEnvironment, hash_to_verify: &[u8], // buffer to verify @@ -189,8 +201,10 @@ pub fn verify_signature_digest_rust_crypto( spki: &SubjectPublicKeyInfoOwned, // public key ) -> Result<()> { if let Ok(enc_spki) = spki.to_der() { + #[cfg(feature = "rsa")] if is_rsa(&signature_alg.oid) { - let rsa = RsaPublicKey::from_public_key_der(&enc_spki); + use rsa::pkcs8::DecodePublicKey as _; + let rsa = rsa::RsaPublicKey::from_public_key_der(&enc_spki); if let Ok(rsa) = rsa { let ps = get_padding_scheme(signature_alg)?; let x = rsa.verify(ps, hash_to_verify, signature); @@ -224,6 +238,7 @@ fn get_named_curve_parameter(alg_id: &AlgorithmIdentifierOwned) -> Result Result<()> { if is_rsa(&signature_alg.oid) { + #[cfg(feature = "rsa")] if let Ok(enc_spki) = spki.to_der() { - let rsa = RsaPublicKey::from_public_key_der(&enc_spki); + use rsa::pkcs8::DecodePublicKey as _; + let rsa = rsa::RsaPublicKey::from_public_key_der(&enc_spki); if let Ok(rsa) = rsa { - let hash_alg = get_hash_alg_from_sig_alg(&signature_alg.oid)?; + let hash_alg = crate::util::get_hash_alg_from_sig_alg(&signature_alg.oid)?; let hash_to_verify = calculate_hash_rust_crypto(pe, &hash_alg, message_to_verify)?; let ps = get_padding_scheme(signature_alg)?; return rsa @@ -300,10 +317,10 @@ pub fn verify_signature_message_rust_crypto( } else if is_eddsa(&signature_alg.oid) { let Ok(verifying_key) = ed25519_dalek::VerifyingKey::try_from(spki.subject_public_key.raw_bytes()) - else { - error!("Could not decode verifying key"); - return Err(Error::PathValidation(PathValidationStatus::EncodingError)); - }; + else { + error!("Could not decode verifying key"); + return Err(Error::PathValidation(PathValidationStatus::EncodingError)); + }; let Ok(s) = ed25519_dalek::Signature::from_slice(signature) else { error!("Could not decode signature"); return Err(Error::PathValidation(PathValidationStatus::EncodingError)); @@ -319,14 +336,17 @@ pub fn verify_signature_message_rust_crypto( debug!("Unrecognized signature algorithm: {}", signature_alg.oid); Err(Error::Unrecognized) } + #[cfg(feature = "pqc")] fn is_explicit_composite(oid: ObjectIdentifier) -> bool { ENTU_DILITHIUM3_ECDSA_P256 == oid } + #[cfg(feature = "pqc")] fn is_generic_composite(oid: ObjectIdentifier) -> bool { ENTU_COMPOSITE_SIG == oid } + #[cfg(feature = "pqc")] fn is_composite(oid: ObjectIdentifier) -> bool { is_explicit_composite(oid) || is_generic_composite(oid) @@ -636,6 +656,7 @@ fn test_calculate_hash() { hex!("BA7816BF8F01CFEA414140DE5DAE2223B00361A396177A9CB410FF61F20015AD") ); } + #[test] fn test_verify_signature_digest() { use crate::{DeferDecodeSigned, PkiEnvironment}; From e07dfa1c02854ce72155f5e4b23af30df51fed98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Augusto=20C=C3=A9sar=20Dias?= Date: Wed, 28 Aug 2024 16:42:09 +0200 Subject: [PATCH 23/23] test: test should run only when rsa is enabled. fmt --- certval/src/environment/pki_environment.rs | 5 ++--- certval/src/util/crypto.rs | 13 ++++++------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/certval/src/environment/pki_environment.rs b/certval/src/environment/pki_environment.rs index c4e534a..56c71ed 100644 --- a/certval/src/environment/pki_environment.rs +++ b/certval/src/environment/pki_environment.rs @@ -230,9 +230,8 @@ impl PkiEnvironment { spki: &SubjectPublicKeyInfoOwned, // public key ) -> Result<()> { for f in &self.verify_signature_digest_callbacks { - let r = f(pe, hash_to_verify, signature, signature_alg, spki); - if let Ok(r) = r { - return Ok(r); + if f(pe, hash_to_verify, signature, signature_alg, spki).is_ok() { + return Ok(()); } } Err(Error::Unrecognized) diff --git a/certval/src/util/crypto.rs b/certval/src/util/crypto.rs index d577f34..04419f0 100644 --- a/certval/src/util/crypto.rs +++ b/certval/src/util/crypto.rs @@ -13,9 +13,7 @@ use sha2::{Digest, Sha224, Sha256, Sha384, Sha512}; use spki::{AlgorithmIdentifierOwned, SubjectPublicKeyInfoOwned}; use crate::util::error::{Error, PathValidationStatus, Result}; -use crate::{ - environment::pki_environment::*, util::pdv_alg_oids::*, -}; +use crate::{environment::pki_environment::*, util::pdv_alg_oids::*}; #[cfg(feature = "pqc")] use pqckeys::composite::*; @@ -317,10 +315,10 @@ pub fn verify_signature_message_rust_crypto( } else if is_eddsa(&signature_alg.oid) { let Ok(verifying_key) = ed25519_dalek::VerifyingKey::try_from(spki.subject_public_key.raw_bytes()) - else { - error!("Could not decode verifying key"); - return Err(Error::PathValidation(PathValidationStatus::EncodingError)); - }; + else { + error!("Could not decode verifying key"); + return Err(Error::PathValidation(PathValidationStatus::EncodingError)); + }; let Ok(s) = ed25519_dalek::Signature::from_slice(signature) else { error!("Could not decode signature"); return Err(Error::PathValidation(PathValidationStatus::EncodingError)); @@ -658,6 +656,7 @@ fn test_calculate_hash() { } #[test] +#[cfg(feature = "rsa")] fn test_verify_signature_digest() { use crate::{DeferDecodeSigned, PkiEnvironment}; use der::Decode;