introduce convenient handling of attestation skip and debug mode secu…

…rity flags (#204) * introduce allow-skipping-attestation storage and cleanup old skip-ias-check flag * add unit tests for Skip RA * add root origin dispatchable for security flags. and unit-test/benchmarking * define secure default values for security flags if unset * clippy
integritee-network · Jul 15, 2023 · 4a8592b · 4a8592b
1 parent 8de46ff
commit 4a8592b
Show file tree

Hide file tree

Showing 14 changed files with 235 additions and 180 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/enclave-bridge/src/mock.rs b/enclave-bridge/src/mock.rs
@@ -152,7 +152,10 @@ pub fn new_test_ext() -> sp_io::TestExternalities {
 	}
 	.assimilate_storage(&mut t)
 	.unwrap();
-	let teerex_config = pallet_teerex::GenesisConfig { allow_sgx_debug_mode: true };
+	let teerex_config = pallet_teerex::GenesisConfig {
+		allow_sgx_debug_mode: true,
+		allow_skipping_attestation: true,
+	};
 	GenesisBuild::<Test>::assimilate_storage(&teerex_config, &mut t).unwrap();
 
 	let mut ext: sp_io::TestExternalities = t.into();

diff --git a/sidechain/Cargo.toml b/sidechain/Cargo.toml
@@ -74,8 +74,5 @@ runtime-benchmarks = [
     "pallet-timestamp/runtime-benchmarks",
     "test-utils",
 ]
-# Allow workers to register without remote attestation for dev purposes.
-# This pallet needs the flag only to run the tests, otherwise skip-ias-check should only be set in the pallet-teerex.
-skip-ias-check = []
 
 try-runtime = ["frame-support/try-runtime"]
diff --git a/sidechain/src/lib.rs b/sidechain/src/lib.rs
@@ -153,6 +153,6 @@ impl<T: Config> Pallet<T> {
 mod benchmarking;
 #[cfg(test)]
 mod mock;
-#[cfg(all(test, not(feature = "skip-ias-check")))]
+#[cfg(test)]
 mod tests;
 pub mod weights;
diff --git a/sidechain/src/mock.rs b/sidechain/src/mock.rs
@@ -162,7 +162,10 @@ pub fn new_test_ext() -> sp_io::TestExternalities {
 	}
 	.assimilate_storage(&mut t)
 	.unwrap();
-	let teerex_config = pallet_teerex::GenesisConfig { allow_sgx_debug_mode: true };
+	let teerex_config = pallet_teerex::GenesisConfig {
+		allow_sgx_debug_mode: true,
+		allow_skipping_attestation: true,
+	};
 	GenesisBuild::<Test>::assimilate_storage(&teerex_config, &mut t).unwrap();
 
 	let mut ext: sp_io::TestExternalities = t.into();

diff --git a/teeracle/src/benchmarking.rs b/teeracle/src/benchmarking.rs
@@ -35,16 +35,9 @@ use test_utils::{
 	test_data::{consts::*, ias::*},
 };
 
-fn ensure_not_skipping_ra_check() {
-	#[cfg(not(test))]
-	if cfg!(feature = "skip-ias-check") {
-		panic!("Benchmark does not allow the `skip-ias-check` flag.");
-	};
-}
 benchmarks! {
 	where_clause {  where T::AccountId: From<[u8; 32]>, T::Hash: From<[u8; 32]> }
 	update_exchange_rate {
-		ensure_not_skipping_ra_check();
 		timestamp::Pallet::<T>::set_timestamp(TEST4_SETUP.timestamp.checked_into().unwrap());
 		let signer: T::AccountId = get_signer(TEST4_SETUP.signer_pub);
 		let trading_pair: TradingPairString =  "DOT/USD".into();
@@ -67,7 +60,6 @@ benchmarks! {
 	}
 
 	update_oracle {
-		ensure_not_skipping_ra_check();
 		timestamp::Pallet::<T>::set_timestamp(TEST4_SETUP.timestamp.checked_into().unwrap());
 		let signer: T::AccountId = get_signer(TEST4_SETUP.signer_pub);
 		let oracle_name = OracleDataName::from("Test_Oracle_Name");

diff --git a/teeracle/src/mock.rs b/teeracle/src/mock.rs
@@ -153,7 +153,10 @@ pub fn new_test_ext() -> sp_io::TestExternalities {
 	}
 	.assimilate_storage(&mut t)
 	.unwrap();
-	let teerex_config = pallet_teerex::GenesisConfig { allow_sgx_debug_mode: true };
+	let teerex_config = pallet_teerex::GenesisConfig {
+		allow_sgx_debug_mode: true,
+		allow_skipping_attestation: true,
+	};
 	GenesisBuild::<Test>::assimilate_storage(&teerex_config, &mut t).unwrap();
 
 	let mut ext: sp_io::TestExternalities = t.into();

diff --git a/teerex/src/benchmarking.rs b/teerex/src/benchmarking.rs
@@ -35,13 +35,6 @@ use test_utils::{
 
 const MAX_SILENCE_TIME: u64 = 172_800_000; // 48h
 
-fn ensure_not_skipping_ra_check() {
-	#[cfg(not(test))]
-	if cfg!(feature = "skip-ias-check") {
-		panic!("Benchmark does not allow the `skip-ias-check` flag.");
-	};
-}
-
 fn generate_accounts<T: Config>(amount: u32) -> Vec<T::AccountId> {
 	(0..amount).map(|n| account("dummy name", n, n)).collect()
 }
@@ -57,7 +50,6 @@ benchmarks! {
 	// Benchmark `register_sgx_enclave` with the worst possible conditions (DCAP sovereign is more involved than Ias or proxied DCAP):
 	// * dcap registration succeeds with `proxied: false`
 	register_sgx_enclave {
-		ensure_not_skipping_ra_check();
 		timestamp::Pallet::<T>::set_timestamp(TEST_VALID_COLLATERAL_TIMESTAMP.checked_into().unwrap());
 		let signer: T::AccountId = get_signer(&TEST1_DCAP_QUOTE_SIGNER);
 
@@ -74,7 +66,6 @@ benchmarks! {
 	// Benchmark `register_quoting_enclave` with the worst possible conditions:
 	// * quoting enclave registration succeeds
 	register_quoting_enclave {
-		ensure_not_skipping_ra_check();
 		timestamp::Pallet::<T>::set_timestamp(TEST_VALID_COLLATERAL_TIMESTAMP.checked_into().unwrap());
 		let signer: T::AccountId = get_signer(&TEST1_DCAP_QUOTE_SIGNER);
 
@@ -87,7 +78,6 @@ benchmarks! {
 	// Benchmark `register_tcb_info` with the worst possible conditions:
 	// * tcb registration succeeds
 	register_tcb_info {
-		ensure_not_skipping_ra_check();
 		timestamp::Pallet::<T>::set_timestamp(TEST_VALID_COLLATERAL_TIMESTAMP.checked_into().unwrap());
 		let signer: T::AccountId = get_signer(&TEST1_DCAP_QUOTE_SIGNER);
 		register_test_quoting_enclave::<T>(signer.clone());
@@ -127,6 +117,13 @@ benchmarks! {
 	verify {
 		assert!(!crate::ProxiedEnclaves::<T>::contains_key(&key0));
 	}
+
+	set_security_flags {
+	}: _(RawOrigin::Root, false, true)
+	verify {
+		assert_eq!(crate::AllowSkippingAttestation::<T>::get(), false);
+		assert_eq!(crate::SgxAllowDebugMode::<T>::get(), true);
+	}
 }
 
 fn add_sovereign_enclaves_to_registry<T: Config>(accounts: &[T::AccountId]) {

diff --git a/teerex/src/lib.rs b/teerex/src/lib.rs
@@ -88,6 +88,10 @@ pub mod pallet {
 		SgxQuotingEnclaveRegistered {
 			quoting_enclave: SgxQuotingEnclave,
 		},
+		UpdatedSecurityFlags {
+			allow_skipping_attestation: bool,
+			sgx_allow_debug_mode: bool,
+		},
 	}
 
 	#[pallet::storage]
@@ -114,20 +118,38 @@ pub mod pallet {
 	pub type SgxTcbInfo<T: Config> =
 		StorageMap<_, Blake2_128Concat, Fmspc, SgxTcbInfoOnChain, ValueQuery>;
 
+	#[pallet::type_value]
+	pub fn DefaultSgxAllowDebugMode<T: Config>() -> bool {
+		false
+	}
+
 	#[pallet::storage]
 	#[pallet::getter(fn allow_sgx_debug_mode)]
-	pub type SgxAllowDebugMode<T: Config> = StorageValue<_, bool, ValueQuery>;
+	pub type SgxAllowDebugMode<T: Config> =
+		StorageValue<_, bool, ValueQuery, DefaultSgxAllowDebugMode<T>>;
+
+	#[pallet::type_value]
+	pub fn DefaultAllowSkippingAttestation<T: Config>() -> bool {
+		false
+	}
+
+	#[pallet::storage]
+	#[pallet::getter(fn allow_skipping_attestation)]
+	pub type AllowSkippingAttestation<T: Config> =
+		StorageValue<_, bool, ValueQuery, DefaultAllowSkippingAttestation<T>>;
 
 	#[pallet::genesis_config]
 	#[cfg_attr(feature = "std", derive(Default))]
 	pub struct GenesisConfig {
 		pub allow_sgx_debug_mode: bool,
+		pub allow_skipping_attestation: bool,
 	}
 
 	#[pallet::genesis_build]
 	impl<T: Config> GenesisBuild<T> for GenesisConfig {
 		fn build(&self) {
 			SgxAllowDebugMode::<T>::put(self.allow_sgx_debug_mode);
+			AllowSkippingAttestation::<T>::put(self.allow_skipping_attestation);
 		}
 	}
 
@@ -231,17 +253,24 @@ pub mod pallet {
 					)
 					.with_attestation_method(SgxAttestationMethod::Dcap { proxied })
 				},
-				SgxAttestationMethod::Skip { proxied } => SgxEnclave::new(
-					SgxReportData::default(),
-					// insert mrenclave if the ra_report represents one, otherwise insert default
-					<MrEnclave>::decode(&mut proof.as_slice()).unwrap_or_default(),
-					MrSigner::default(),
-					<timestamp::Pallet<T>>::get().saturated_into(),
-					SgxBuildMode::default(),
-					SgxStatus::Invalid,
-				)
-				.with_pubkey(sender.encode().as_ref())
-				.with_attestation_method(SgxAttestationMethod::Skip { proxied }),
+				SgxAttestationMethod::Skip { proxied } => {
+					if !Self::allow_skipping_attestation() {
+						log::debug!(target: TEEREX, "skipping attestation not allowed",);
+						return Err(<Error<T>>::SkippingAttestationNotAllowed.into())
+					}
+					log::debug!(target: TEEREX, "skipping attestation verification",);
+					SgxEnclave::new(
+						SgxReportData::default(),
+						// insert mrenclave if the ra_report represents one, otherwise insert default
+						<MrEnclave>::decode(&mut proof.as_slice()).unwrap_or_default(),
+						MrSigner::default(),
+						<timestamp::Pallet<T>>::get().saturated_into(),
+						SgxBuildMode::default(),
+						SgxStatus::Invalid,
+					)
+					.with_pubkey(sender.encode().as_ref())
+					.with_attestation_method(SgxAttestationMethod::Skip { proxied })
+				},
 			};
 
 			if !<SgxAllowDebugMode<T>>::get() && enclave.build_mode == SgxBuildMode::Debug {
@@ -360,6 +389,25 @@ pub mod pallet {
 			Self::deposit_event(Event::SgxTcbInfoRegistered { fmspc, on_chain_info });
 			Ok(().into())
 		}
+
+		#[pallet::call_index(5)]
+		#[pallet::weight((<T as Config>::WeightInfo::set_security_flags(), DispatchClass::Normal, Pays::Yes))]
+		pub fn set_security_flags(
+			origin: OriginFor<T>,
+			allow_skipping_attestation: bool,
+			sgx_allow_debug_mode: bool,
+		) -> DispatchResultWithPostInfo {
+			log::debug!(target: TEEREX, "Called into runtime call set_security_flags()");
+			ensure_root(origin)?;
+			<AllowSkippingAttestation<T>>::set(allow_skipping_attestation);
+			<SgxAllowDebugMode<T>>::set(sgx_allow_debug_mode);
+			log::info!(target: TEEREX, "set security flags");
+			Self::deposit_event(Event::UpdatedSecurityFlags {
+				allow_skipping_attestation,
+				sgx_allow_debug_mode,
+			});
+			Ok(().into())
+		}
 	}
 
 	#[pallet::error]
@@ -385,6 +433,8 @@ pub mod pallet {
 		CollateralInvalid,
 		/// It is not allowed to unregister enclaves with recent activity
 		UnregisterActiveEnclaveNotAllowed,
+		/// skipping attestation not allowed by configuration
+		SkippingAttestationNotAllowed,
 	}
 }
 

diff --git a/teerex/src/mock.rs b/teerex/src/mock.rs
@@ -145,7 +145,8 @@ pub fn new_test_ext() -> sp_io::TestExternalities {
 	}
 	.assimilate_storage(&mut t)
 	.unwrap();
-	let teerex_config = crate::GenesisConfig { allow_sgx_debug_mode: true };
+	let teerex_config =
+		crate::GenesisConfig { allow_sgx_debug_mode: true, allow_skipping_attestation: true };
 	GenesisBuild::<Test>::assimilate_storage(&teerex_config, &mut t).unwrap();
 
 	let mut ext: sp_io::TestExternalities = t.into();
@@ -154,7 +155,6 @@ pub fn new_test_ext() -> sp_io::TestExternalities {
 }
 
 //Build genesis storage for mockup, where RA from enclave compiled in debug mode is NOT allowed
-#[cfg(not(feature = "skip-ias-check"))]
 pub fn new_test_production_ext() -> sp_io::TestExternalities {
 	let mut t = system::GenesisConfig::default().build_storage::<Test>().unwrap();
 	pallet_balances::GenesisConfig::<Test> {
@@ -163,7 +163,8 @@ pub fn new_test_production_ext() -> sp_io::TestExternalities {
 	.assimilate_storage(&mut t)
 	.unwrap();
 
-	let teerex_config = crate::GenesisConfig { allow_sgx_debug_mode: false };
+	let teerex_config =
+		crate::GenesisConfig { allow_sgx_debug_mode: false, allow_skipping_attestation: false };
 	GenesisBuild::<Test>::assimilate_storage(&teerex_config, &mut t).unwrap();
 
 	let mut ext: sp_io::TestExternalities = t.into();
@@ -172,7 +173,6 @@ pub fn new_test_production_ext() -> sp_io::TestExternalities {
 }
 
 /// Helper method for the OnTimestampSet to be called
-#[cfg(not(feature = "skip-ias-check"))]
 pub fn set_timestamp(t: u64) {
 	let _ = timestamp::Pallet::<Test>::set(RuntimeOrigin::none(), t);
 }
diff --git a/teerex/src/tests/mod.rs b/teerex/src/tests/mod.rs
@@ -14,7 +14,4 @@
 	limitations under the License.
 
 */
-#[cfg(feature = "skip-ias-check")]
-mod skip_ias_check_tests;
-#[cfg(not(feature = "skip-ias-check"))]
 mod test_cases;
diff --git a/teerex/src/tests/skip_ias_check_tests.rs b/teerex/src/tests/skip_ias_check_tests.rs