Index: s390-tools-service/rust/pv/src/verify.rs =================================================================== --- s390-tools-service.orig/rust/pv/src/verify.rs +++ s390-tools-service/rust/pv/src/verify.rs @@ -3,10 +3,11 @@ // Copyright IBM Corp. 2023 use core::slice; -use log::debug; +use log::{debug, trace}; +use openssl::error::ErrorStack; use openssl::stack::Stack; use openssl::x509::store::X509Store; -use openssl::x509::{CrlStatus, X509Ref, X509StoreContext, X509}; +use openssl::x509::{CrlStatus, X509NameRef, X509Ref, X509StoreContext, X509StoreContextRef, X509}; use openssl_extensions::crl::StackableX509Crl; use openssl_extensions::crl::X509StoreContextExtension; @@ -82,8 +83,8 @@ impl HkdVerifier for CertVerifier { if verified_crls.is_empty() { bail_hkd_verify!(NoCrl); } - for crl in &verified_crls { - match crl.get_by_cert(&hkd.to_owned()) { + for crl in verified_crls { + match crl.get_by_serial(hkd.serial_number()) { CrlStatus::NotRevoked => (), _ => bail_hkd_verify!(HdkRevoked), } @@ -94,21 +95,54 @@ impl HkdVerifier for CertVerifier { } impl CertVerifier { + fn quirk_crls( + ctx: &mut X509StoreContextRef, + subject: &X509NameRef, + ) -> Result, ErrorStack> { + match ctx.crls(subject) { + Ok(ret) if !ret.is_empty() => return Ok(ret), + _ => (), + } + + // Armonk/Poughkeepsie fixup + trace!("quirk_crls: Try Locality"); + if let Some(locality_subject) = helper::armonk_locality_fixup(subject) { + match ctx.crls(&locality_subject) { + Ok(ret) if !ret.is_empty() => return Ok(ret), + _ => (), + } + + // reorder + trace!("quirk_crls: Try Locality+Reorder"); + if let Ok(locality_ordered_subject) = helper::reorder_x509_names(&locality_subject) { + match ctx.crls(&locality_ordered_subject) { + Ok(ret) if !ret.is_empty() => return Ok(ret), + _ => (), + } + } + } + + // reorder unchanged loaciliy subject + trace!("quirk_crls: Try Reorder"); + if let Ok(ordered_subject) = helper::reorder_x509_names(subject) { + match ctx.crls(&ordered_subject) { + Ok(ret) if !ret.is_empty() => return Ok(ret), + _ => (), + } + } + // nothing found, return empty stack + Stack::new() + } + ///Download the CLRs that a HKD refers to. pub fn hkd_crls(&self, hkd: &X509Ref) -> Result> { let mut ctx = X509StoreContext::new()?; // Unfortunately we cannot use a dedicated function here and have to use a closure (E0434) // Otherwise, we cannot refer to self + // Search for local CRLs let mut crls = ctx.init_opt(&self.store, None, None, |ctx| { let subject = self.ibm_z_sign_key.subject_name(); - match ctx.crls(subject) { - Ok(crls) => Ok(crls), - _ => { - // reorder the name and try again - let broken_subj = helper::reorder_x509_names(subject)?; - ctx.crls(&broken_subj).or_else(helper::stack_err_hlp) - } - } + Self::quirk_crls(ctx, subject) })?; if !self.offline { Index: s390-tools-service/rust/pv/src/verify/helper.rs =================================================================== --- s390-tools-service.orig/rust/pv/src/verify/helper.rs +++ s390-tools-service/rust/pv/src/verify/helper.rs @@ -13,7 +13,7 @@ use openssl::{ error::ErrorStack, nid::Nid, ssl::SslFiletype, - stack::{Stack, Stackable}, + stack::Stack, x509::{ store::{File, X509Lookup, X509StoreBuilder, X509StoreBuilderRef, X509StoreRef}, verify::{X509VerifyFlags, X509VerifyParam}, @@ -25,6 +25,7 @@ use openssl_extensions::{ akid::{AkidCheckResult, AkidExtension}, crl::X509StoreExtension, }; +use std::str::from_utf8; use std::{cmp::Ordering, ffi::c_int, time::Duration, usize}; /// Minimum security level for the keys/certificates used to establish a chain of @@ -39,7 +40,6 @@ const SECURITY_CHAIN_MAX_LEN: c_int = 2; /// verifies that the HKD /// * has enough security bits /// * is inside its validity period -/// * issuer name is the subject name of the [`sign_key`] /// * the Authority Key ID matches the Signing Key ID of the [`sign_key`] pub fn verify_hkd_options(hkd: &X509Ref, sign_key: &X509Ref) -> Result<()> { let hk_pkey = hkd.public_key()?; @@ -53,9 +53,6 @@ pub fn verify_hkd_options(hkd: &X509Ref, // verify that the hkd is still valid check_validity_period(hkd.not_before(), hkd.not_after())?; - // check if hkd.issuer_name == issuer.subject - check_x509_name_equal(sign_key.subject_name(), hkd.issuer_name())?; - // verify that the AKID of the hkd matches the SKID of the issuer if let Some(akid) = hkd.akid() { if akid.check(sign_key) != AkidCheckResult::OK { @@ -75,9 +72,6 @@ pub fn verify_crl(crl: &X509CrlRef, issu return None; } } - - check_x509_name_equal(crl.issuer_name(), issuer.subject_name()).ok()?; - match crl.verify(issuer.public_key().ok()?.as_ref()).ok()? { true => Some(()), false => None, @@ -207,7 +201,8 @@ pub fn download_crls_into_store(store: & //Asn1StringRef::as_slice aka ASN1_STRING_get0_data gives a string without \0 delimiter const IBM_Z_COMMON_NAME: &[u8; 43usize] = b"International Business Machines Corporation"; const IBM_Z_COUNTRY_NAME: &[u8; 2usize] = b"US"; -const IBM_Z_LOCALITY_NAME: &[u8; 12usize] = b"Poughkeepsie"; +const IBM_Z_LOCALITY_NAME_POUGHKEEPSIE: &[u8; 12usize] = b"Poughkeepsie"; +const IBM_Z_LOCALITY_NAME_ARMONK: &[u8; 6usize] = b"Armonk"; const IBM_Z_ORGANIZATIONAL_UNIT_NAME_SUFFIX: &str = "Key Signing Service"; const IBM_Z_ORGANIZATION_NAME: &[u8; 43usize] = b"International Business Machines Corporation"; const IBM_Z_STATE: &[u8; 8usize] = b"New York"; @@ -226,7 +221,8 @@ fn is_ibm_signing_cert(cert: &X509) -> b if subj.entries().count() != IMB_Z_ENTRY_COUNT || !name_data_eq(subj, Nid::COUNTRYNAME, IBM_Z_COUNTRY_NAME) || !name_data_eq(subj, Nid::STATEORPROVINCENAME, IBM_Z_STATE) - || !name_data_eq(subj, Nid::LOCALITYNAME, IBM_Z_LOCALITY_NAME) + || !(name_data_eq(subj, Nid::LOCALITYNAME, IBM_Z_LOCALITY_NAME_POUGHKEEPSIE) + || name_data_eq(subj, Nid::LOCALITYNAME, IBM_Z_LOCALITY_NAME_ARMONK)) || !name_data_eq(subj, Nid::ORGANIZATIONNAME, IBM_Z_ORGANIZATION_NAME) || !name_data_eq(subj, Nid::COMMONNAME, IBM_Z_COMMON_NAME) { @@ -367,24 +363,6 @@ fn check_validity_period(not_before: &As } } -fn check_x509_name_equal(lhs: &X509NameRef, rhs: &X509NameRef) -> Result<()> { - if lhs.entries().count() != rhs.entries().count() { - bail_hkd_verify!(IssuerMismatch); - } - - for l in lhs.entries() { - // search for the matching value in the rhs names - // found none? -> names are not equal - if !rhs - .entries() - .any(|r| l.data().as_slice() == r.data().as_slice()) - { - bail_hkd_verify!(IssuerMismatch); - } - } - Ok(()) -} - const NIDS_CORRECT_ORDER: [Nid; 6] = [ Nid::COUNTRYNAME, Nid::ORGANIZATIONNAME, @@ -407,13 +385,28 @@ pub fn reorder_x509_names(subject: &X509 Ok(correct_subj.build()) } -pub fn stack_err_hlp( - e: ErrorStack, -) -> std::result::Result, openssl::error::ErrorStack> { - match e.errors().len() { - 0 => Stack::::new(), - _ => Err(e), +/** +* Workaround for potential locality mismatches between CRLs and Certs +* # Return +* fixed subject or none if locality was not Armonk or any OpenSSL error +*/ +pub fn armonk_locality_fixup(subject: &X509NameRef) -> Option { + if !name_data_eq(subject, Nid::LOCALITYNAME, IBM_Z_LOCALITY_NAME_ARMONK) { + return None; + } + + let mut ret = X509Name::builder().ok()?; + for entry in subject.entries() { + match entry.object().nid() { + nid @ Nid::LOCALITYNAME => ret + .append_entry_by_nid(nid, from_utf8(IBM_Z_LOCALITY_NAME_POUGHKEEPSIE).ok()?) + .ok()?, + _ => { + ret.append_entry(entry).ok()?; + } + } } + Some(ret.build()) } #[cfg(test)] @@ -451,20 +444,6 @@ mod test { )); } - #[test] - fn x509_name_equal() { - let sign_crt = load_gen_cert("ibm.crt"); - let hkd = load_gen_cert("host.crt"); - let other = load_gen_cert("inter_ca.crt"); - - assert!(super::check_x509_name_equal(sign_crt.subject_name(), hkd.issuer_name()).is_ok(),); - - assert!(matches!( - super::check_x509_name_equal(other.subject_name(), hkd.subject_name()), - Err(Error::HkdVerify(IssuerMismatch)) - )); - } - #[test] fn is_ibm_z_sign_key() { let ibm_crt = load_gen_cert("ibm.crt"); Index: s390-tools-service/rust/pv/src/verify/test.rs =================================================================== --- s390-tools-service.orig/rust/pv/src/verify/test.rs +++ s390-tools-service/rust/pv/src/verify/test.rs @@ -84,7 +84,6 @@ fn verify_online() { let inter_crt = get_cert_asset_path_string("inter_ca.crt"); let ibm_crt = get_cert_asset_path_string("ibm.crt"); let hkd_revoked = load_gen_cert("host_rev.crt"); - let hkd_inv = load_gen_cert("host_invalid_signing_key.crt"); let hkd_exp = load_gen_cert("host_crt_expired.crt"); let hkd = load_gen_cert("host.crt"); @@ -112,11 +111,6 @@ fn verify_online() { )); assert!(matches!( - verifier.verify(&hkd_inv), - Err(Error::HkdVerify(IssuerMismatch)) - )); - - assert!(matches!( verifier.verify(&hkd_exp), Err(Error::HkdVerify(AfterValidity)) )); @@ -130,7 +124,6 @@ fn verify_offline() { let ibm_crt = get_cert_asset_path_string("ibm.crt"); let ibm_crl = get_cert_asset_path_string("ibm.crl"); let hkd_revoked = load_gen_cert("host_rev.crt"); - let hkd_inv = load_gen_cert("host_invalid_signing_key.crt"); let hkd_exp = load_gen_cert("host_crt_expired.crt"); let hkd = load_gen_cert("host.crt"); @@ -149,11 +142,6 @@ fn verify_offline() { )); assert!(matches!( - verifier.verify(&hkd_inv), - Err(Error::HkdVerify(IssuerMismatch)) - )); - - assert!(matches!( verifier.verify(&hkd_exp), Err(Error::HkdVerify(AfterValidity)) ));