pv: Defer CRL downloads until certificate validation succeeds

Perform certificate verification in two stages. First, verify the
certificate chain without CRL checks. Once the chain has been validated,
download the referenced CRLs and repeat the verification with CRL
checking enabled.

Fixes: c6f621d0dc ("rust: Add library for pv tools")
Signed-off-by: Marc Hartmayer <marc@linux.ibm.com>
Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>

rust/pv/src/verify.rs

@@ -5,7 +5,7 @@
 use core::slice;
 use std::path::Path;
 
-use helper::download_first_crl_from_x509;
+use helper::{download_first_crl_from_x509, StoreSetupMode};
 use log::{debug, trace};
 use openssl::error::ErrorStack;
 use openssl::stack::Stack;
@@ -181,25 +181,45 @@ impl CertVerifier {
         Q: AsRef<Path>,
         R: AsRef<Path>,
     {
-        let mut store = helper::store_setup(root_ca_path, crl_paths, cert_paths)?;
         let mut untr_certs = Vec::with_capacity(cert_paths.len());
         for path in cert_paths {
             let mut crt = read_certs(&read_file(path, "certificate")?)?;
-            if !offline {
-                for c in &crt {
-                    if let Some(crl) = download_first_crl_from_x509(c)? {
-                        crl.iter().try_for_each(|c| store.add_crl(c))?;
-                    }
-                }
-            }
             untr_certs.append(&mut crt);
         }
+        let (ibm_z_sign_key, chain) = helper::extract_ibm_sign_key(untr_certs.clone())?;
 
-        // remove the IBM signing certificate from chain.
-        // We have to verify them separately as they are not marked as intermediate certs
-        let (ibm_z_sign_key, chain) = helper::extract_ibm_sign_key(untr_certs)?;
+        // Two-round verification:
+        //
+        // Round 1: Verify chain without CRL checks before downloading files
+        // from URLs from (yet) untrusted certificates.
+        let store_builder = helper::store_setup(
+            root_ca_path.as_ref(),
+            crl_paths,
+            cert_paths,
+            StoreSetupMode::WithoutCrlCheck,
+        )?;
+        helper::verify_chain(
+            &store_builder.build(),
+            &chain,
+            slice::from_ref(&ibm_z_sign_key),
+        )?;
 
-        let store = store.build();
+        // Round 2: Download CRLs and verify again, but this time with CRL checks
+        let mut store_builder = helper::store_setup(
+            root_ca_path,
+            crl_paths,
+            cert_paths,
+            StoreSetupMode::WithCrlCheck,
+        )?;
+        if !offline {
+            for cert in &untr_certs {
+                if let Some(crls) = download_first_crl_from_x509(cert)? {
+                    crls.iter().try_for_each(|c| store_builder.add_crl(c))?;
+                }
+            }
+        }
+
+        let store = store_builder.build();
         helper::verify_chain(&store, &chain, slice::from_ref(&ibm_z_sign_key))?;
 
         Ok(Self {

rust/pv/src/verify/helper.rs

@@ -110,21 +110,27 @@ pub fn verify_crl(crl: &X509CrlRef, issuer: &X509Ref) -> Option<()> {
     }
 }
 
+pub enum StoreSetupMode {
+    WithCrlCheck,
+    WithoutCrlCheck,
+}
+
 /// Setup the x509Store such that it can be used it for verifying certificates
 pub fn store_setup<P: AsRef<Path>, Q: AsRef<Path>, R: AsRef<Path>>(
     root_ca_path: Option<P>,
     crl_paths: &[Q],
     cert_w_crl_paths: &[R],
+    mode: StoreSetupMode,
 ) -> Result<X509StoreBuilder> {
-    let mut x509store = X509StoreBuilder::new()?;
+    let mut x509store_builder = X509StoreBuilder::new()?;
 
     match root_ca_path {
-        None => x509store.set_default_paths()?,
-        Some(p) => load_root_ca(p, &mut x509store)?,
+        None => x509store_builder.set_default_paths()?,
+        Some(p) => load_root_ca(p, &mut x509store_builder)?,
     }
 
     for crl in crl_paths {
-        load_crl_to_store(&mut x509store, crl, true).map_err(|source| Error::X509Load {
+        load_crl_to_store(&mut x509store_builder, crl, true).map_err(|source| Error::X509Load {
             path: crl.as_ref().into(),
             ty: Error::CRL,
             source,
@@ -132,27 +138,35 @@ pub fn store_setup<P: AsRef<Path>, Q: AsRef<Path>, R: AsRef<Path>>(
     }
 
     for crl in cert_w_crl_paths {
-        load_crl_to_store(&mut x509store, crl, false).map_err(|source| Error::X509Load {
-            path: crl.as_ref().into(),
-            ty: Error::CRL,
-            source,
+        load_crl_to_store(&mut x509store_builder, crl, false).map_err(|source| {
+            Error::X509Load {
+                path: crl.as_ref().into(),
+                ty: Error::CRL,
+                source,
+            }
         })?;
     }
     let mut param = X509VerifyParam::new()?;
-    let flags = X509VerifyFlags::X509_STRICT
-        | X509VerifyFlags::CRL_CHECK
-        | X509VerifyFlags::CRL_CHECK_ALL
+    let mut flags = X509VerifyFlags::X509_STRICT
         | X509VerifyFlags::TRUSTED_FIRST
         | X509VerifyFlags::CHECK_SS_SIGNATURE
         | X509VerifyFlags::POLICY_CHECK;
+    match mode {
+        StoreSetupMode::WithCrlCheck => {
+            flags |= X509VerifyFlags::CRL_CHECK | X509VerifyFlags::CRL_CHECK_ALL
+        }
+        StoreSetupMode::WithoutCrlCheck => {
+            // nothing to do
+        }
+    }
 
     param.set_depth(SECURITY_CHAIN_MAX_LEN);
     param.set_auth_level(SECURITY_LEVEL as i32);
     param.set_purpose(X509PurposeId::ANY)?;
     param.set_flags(flags)?;
-    x509store.set_param(&param)?;
+    x509store_builder.set_param(&param)?;
 
-    Ok(x509store)
+    Ok(x509store_builder)
 }
 
 /// Verify that the given IBM signing keys can be trusted

rust/pv/src/verify/test.rs

@@ -9,6 +9,7 @@ use openssl::stack::Stack;
 use super::helper::*;
 use super::{helper, *};
 use crate::test_utils::*;
+use crate::verify::helper::StoreSetupMode;
 use crate::Error;
 use crate::HkdVerifyErrorType::*;
 
@@ -18,7 +19,12 @@ fn store_setup() {
     let inter_path = get_cert_asset_path("inter.crt");
     let crls: [String; 0] = [];
 
-    let store = helper::store_setup(None::<String>, &crls, &[&ibm_path, &inter_path]);
+    let store = helper::store_setup(
+        None::<String>,
+        &crls,
+        &[&ibm_path, &inter_path],
+        StoreSetupMode::WithCrlCheck,
+    );
     assert!(store.is_ok());
 }
 
@@ -41,9 +47,14 @@ fn verify_chain_offline() {
     let root_crt = get_cert_asset_path("root_ca.chained.crt");
     let certs: [String; 0] = [];
 
-    let store = helper::store_setup(Some(&root_crt), &[&inter_crl], &certs)
-        .unwrap()
-        .build();
+    let store = helper::store_setup(
+        Some(&root_crt),
+        &[&inter_crl],
+        &certs,
+        StoreSetupMode::WithCrlCheck,
+    )
+    .unwrap()
+    .build();
 
     let mut sk = Stack::<X509>::new().unwrap();
     sk.push(inter_crt).unwrap();