From 22dabe63d57fa2a9544155c36582dad80580daf3 Mon Sep 17 00:00:00 2001 From: Nick Santana Date: Wed, 19 Oct 2022 09:36:06 -0700 Subject: [PATCH 1/2] Add `LoadPolicyInitializer` to `mc-sgx-dcap-quoteverify` Previously the load policy for the quote verification enclave was set via `mc-sgx-dcap-quoteverify::load_policy`. Now the `mc-sgx-dcap-quoteverify::LoadPolicyInitializer` is used to set the load policy. --- CHANGELOG.md | 4 + dcap/ql/src/quote3.rs | 11 ++- dcap/ql/src/quote_enclave.rs | 9 ++- dcap/quoteverify/src/lib.rs | 4 +- dcap/quoteverify/src/quote_enclave.rs | 105 ++++++++++++++++++++++++-- dcap/quoteverify/src/verify.rs | 14 +++- 6 files changed, 126 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0231ee9a..e8e1e22a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `mc-sgx-dcap-ql::set_path` and `mc-sgx-dcap-ql::load_policy` have been replaced with `mc-sgx-dcap-ql::PathInitializer` and `mc-sgx-dcap-ql::LoadPolicyInitializer` +- `mc-sgx-dcap-quoteverify::set_path` and + `mc-sgx-dcap-quoteverify::load_policy` have been replaced + with `mc-sgx-dcap-quoteverify::PathInitializer` and + `mc-sgx-dcap-quoteverify::LoadPolicyInitializer` ## [0.2.1] - 2022-10-14 diff --git a/dcap/ql/src/quote3.rs b/dcap/ql/src/quote3.rs index 9a66cbc4..70a3e0d6 100644 --- a/dcap/ql/src/quote3.rs +++ b/dcap/ql/src/quote3.rs @@ -14,12 +14,11 @@ use mc_sgx_util::ResultInto; pub trait TryFromReport { /// Try to create a [`Quote3`] from the provided [`Report`] /// - /// Note: This will initialize the - /// [`PathInitializer`](crate::PathInitializer) to the defaults if the - /// [`PathInitializer`](crate::PathInitializer) has not been initialized - /// yet. Calling - /// [`PathInitializer::with_paths()`](crate::PathInitializer::with_paths) - /// after calling this function will result in an error. + /// Note: This will initialize the [`PathInitializer`] and + /// [`LoadPolicyInitializer`] to the defaults if they have not been + /// initialized yet. Attempts to initialize [`PathInitializer`] or + /// [`LoadPolicyInitializer`] after calling this function will result in + /// an error. /// /// # Arguments /// * `report` - The report to build the quote from diff --git a/dcap/ql/src/quote_enclave.rs b/dcap/ql/src/quote_enclave.rs index f35adbf4..1241847b 100644 --- a/dcap/ql/src/quote_enclave.rs +++ b/dcap/ql/src/quote_enclave.rs @@ -188,10 +188,11 @@ impl PathInitializer { pub trait QeTargetInfo { /// The target info of the QE(Quoting Enclave) /// - /// Note: This will initialized the [`PathInitializer`] to the - /// defaults if the [`PathInitializer`] has not been initialized yet. - /// Calling [`PathInitializer::with_paths()`] after calling this function - /// will result in an error. + /// Note: This will initialize the [`PathInitializer`] and + /// [`LoadPolicyInitializer`] to the defaults if they have not been + /// initialized yet. Attempts to initialize [`PathInitializer`] or + /// [`LoadPolicyInitializer`] after calling this function will result in + /// an error. /// /// # Errors /// Will return an error if there is a failure from the SGX SDK diff --git a/dcap/quoteverify/src/lib.rs b/dcap/quoteverify/src/lib.rs index 9c222700..69ed8c12 100644 --- a/dcap/quoteverify/src/lib.rs +++ b/dcap/quoteverify/src/lib.rs @@ -7,7 +7,7 @@ mod quote_enclave; mod verify; use mc_sgx_dcap_types::Quote3Error; -pub use quote_enclave::{load_policy, PathInitializer}; +pub use quote_enclave::{LoadPolicyInitializer, PathInitializer}; pub use verify::supplemental_data_size; /// Errors interacting with quote verification library functions @@ -24,7 +24,7 @@ pub enum Error { PathDoesNotExist(String), /// Path length is longer than the 259 character bytes allowed PathLengthTooLong(String), - /// The quoting enclave load policy has already been initialized + /// The quote verification enclave load policy has already been initialized LoadPolicyInitialized, } diff --git a/dcap/quoteverify/src/quote_enclave.rs b/dcap/quoteverify/src/quote_enclave.rs index 2ce58ef6..bfeda88f 100644 --- a/dcap/quoteverify/src/quote_enclave.rs +++ b/dcap/quoteverify/src/quote_enclave.rs @@ -148,14 +148,71 @@ impl PathInitializer { } } -/// Set the load policy +/// Initialization of the load policy for the quote verification enclave /// -/// # Arguments -/// * `policy` - The policy to use for loading quoting enclaves -pub fn load_policy(policy: RequestPolicy) -> Result<()> { - unsafe { mc_sgx_dcap_quoteverify_sys::sgx_qv_set_enclave_load_policy(policy.into()) } - .into_result()?; - Ok(()) +/// This should only be called once during process start up utilizing +/// [`LoadPolicyInitializer::policy()`] or +/// [`LoadPolicyInitializer::try_default()`]. If a consumer of this crate does +/// not explicitly initialize the policy, then it will be the default SGX policy +/// of [`RequestPolicy::Persistent`]. +#[derive(Debug)] +pub struct LoadPolicyInitializer; +static LOAD_POLICY_INITIALIZER: Lazy>> = + Lazy::new(|| Mutex::new(None)); + +impl LoadPolicyInitializer { + /// Try to initialize the quote verification enclave load policy to the + /// default + /// + /// The default is [`RequestPolicy::Persistent`]. + /// + /// # Errors + /// * [`Error::LoadPolicyInitialized`] if the policy has been previously + /// initialized. + /// * [`Error::Sgx`] for any errors setting the policy in the SGX SDK. + pub fn try_default() -> Result<()> { + Self::policy(RequestPolicy::Persistent) + } + + /// Set the load policy to use for the quote verification enclave + /// + /// # Arguments + /// * `policy` - The policy to use for loading the quote verification + /// enclave + /// + /// # Errors + /// * [`Error::LoadPolicyInitialized`] if the policy has been previously + /// initialized. + /// * [`Error::Sgx`] for any errors setting the policy in the SGX SDK. + pub fn policy(policy: RequestPolicy) -> Result<()> { + let mut value = LOAD_POLICY_INITIALIZER + .lock() + .expect("Mutex has been poisoned"); + if value.is_none() { + unsafe { mc_sgx_dcap_quoteverify_sys::sgx_qv_set_enclave_load_policy(policy.into()) } + .into_result()?; + *value = Some(LoadPolicyInitializer); + Ok(()) + } else { + Err(Error::LoadPolicyInitialized) + } + } + + /// Will ensure the load policy has been explicity set + /// + /// If the load policy has already been set does nothing + /// + /// # Errors + /// Will return [`Error::Sgx`] if the load policy has not been initialized + /// and there is an error setting the policy + /// + /// Will *not* return an error if the load policy as previously initialized. + pub(crate) fn ensure_initialized() -> Result<()> { + match Self::try_default() { + Ok(_) | Err(Error::LoadPolicyInitialized) => Ok(()), + Err(e) => Err(e), + } + } } #[cfg(test)] @@ -176,6 +233,17 @@ mod test { *value = None; } + /// Resets the [`LOAD_POLICY_INITIALIZER`] to being uninitialized. + /// Since there is *one* [`LOAD_POLICY_INITIALIZER`] for the entire test + /// process any tests focusing on the functionality of the + /// [`LOAD_POLICY_INITIALIZER`] should be utilizing the `#[serial]` macro. + fn reset_load_policy_initializer() { + let mut value = LOAD_POLICY_INITIALIZER + .lock() + .expect("Mutex has been poisoned"); + *value = None; + } + #[test] fn qve_path_succeeds() { let dir = tempdir().unwrap(); @@ -347,7 +415,28 @@ mod test { persistent = { RequestPolicy::Persistent }, ephemeral = { RequestPolicy::Ephemeral }, )] + #[serial] fn load_policy_succeeds(policy: RequestPolicy) { - assert!(load_policy(policy).is_ok()); + reset_load_policy_initializer(); + assert_eq!(LoadPolicyInitializer::policy(policy), Ok(())); + } + + #[test] + #[serial] + fn load_policy_fails_when_already_initialized() { + reset_load_policy_initializer(); + LoadPolicyInitializer::try_default().unwrap(); + assert_eq!( + LoadPolicyInitializer::try_default(), + Err(Error::LoadPolicyInitialized) + ); + } + + #[test] + #[serial] + fn ensuring_the_policy_is_set_is_ok_when_already_set() { + reset_load_policy_initializer(); + LoadPolicyInitializer::policy(RequestPolicy::Ephemeral).unwrap(); + assert_eq!(LoadPolicyInitializer::ensure_initialized(), Ok(())); } } diff --git a/dcap/quoteverify/src/verify.rs b/dcap/quoteverify/src/verify.rs index a0589338..cf9ab2fe 100644 --- a/dcap/quoteverify/src/verify.rs +++ b/dcap/quoteverify/src/verify.rs @@ -2,12 +2,24 @@ //! This module contains logic to assist in verifying a DCAP quote -use crate::{Error, PathInitializer}; +use crate::{quote_enclave::LoadPolicyInitializer, Error, PathInitializer}; use mc_sgx_util::ResultInto; /// Get the supplemental data size +/// +/// Note: This will initialize the [`PathInitializer`] and +/// [`LoadPolicyInitializer`] to the defaults if they have not been +/// initialized yet. Attempts to initialize [`PathInitializer`] or +/// [`LoadPolicyInitializer`] after calling this function will result in +/// an error. +/// +/// # Errors +/// +/// [`Error::Sgx`] if there is any error retrieving the supplemental size from +/// SGX. pub fn supplemental_data_size() -> Result { PathInitializer::ensure_initialized()?; + LoadPolicyInitializer::ensure_initialized()?; let mut size: u32 = 0; unsafe { mc_sgx_dcap_quoteverify_sys::sgx_qv_get_quote_supplemental_data_size(&mut size) } .into_result()?; From 90554c74fb5c0608f8e2844480965004edd0ce90 Mon Sep 17 00:00:00 2001 From: Nick Santana Date: Thu, 20 Oct 2022 10:08:40 -0700 Subject: [PATCH 2/2] Fix some documentation warnings --- dcap/ql/src/quote3.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dcap/ql/src/quote3.rs b/dcap/ql/src/quote3.rs index 70a3e0d6..5d588406 100644 --- a/dcap/ql/src/quote3.rs +++ b/dcap/ql/src/quote3.rs @@ -5,7 +5,7 @@ //! This functionality requires HW SGX to work correctly otherwise all //! functionality will return errors. -use crate::Error; +use crate::{Error, LoadPolicyInitializer, PathInitializer}; use mc_sgx_core_types::Report; use mc_sgx_dcap_types::Quote3; use mc_sgx_util::ResultInto; @@ -26,8 +26,8 @@ pub trait TryFromReport { /// # Errors /// Will return an [`Error::Sgx`] if there is a failure from the SGX SDK fn try_from_report(report: Report) -> Result>, Error> { - crate::PathInitializer::ensure_initialized()?; - crate::LoadPolicyInitializer::ensure_initialized()?; + PathInitializer::ensure_initialized()?; + LoadPolicyInitializer::ensure_initialized()?; let mut size = 0; unsafe { mc_sgx_dcap_ql_sys::sgx_qe_get_quote_size(&mut size) }.into_result()?;