From 64ad8e07bbe863c3f243cc4ba6915c3c63b351a8 Mon Sep 17 00:00:00 2001 From: Peter Nose Date: Mon, 29 Aug 2022 10:13:19 +0200 Subject: [PATCH] keymanager: Validate latest trust root height in key manager requests --- .changelog/4910.feature.md | 1 + keymanager-api-common/src/api.rs | 26 ++++++++-- keymanager-client/src/client.rs | 55 +++++++++++++++++++--- keymanager-lib/src/kdf.rs | 4 ++ keymanager-lib/src/methods.rs | 36 ++++++++++++-- runtime/src/dispatcher.rs | 4 +- tests/runtimes/simple-keyvalue/src/main.rs | 1 + 7 files changed, 110 insertions(+), 17 deletions(-) create mode 100644 .changelog/4910.feature.md diff --git a/.changelog/4910.feature.md b/.changelog/4910.feature.md new file mode 100644 index 00000000000..31658041f3f --- /dev/null +++ b/.changelog/4910.feature.md @@ -0,0 +1 @@ +keymanager: Validate latest trust root height in key manager requests diff --git a/keymanager-api-common/src/api.rs b/keymanager-api-common/src/api.rs index 6d183ab3007..1c7895effa5 100644 --- a/keymanager-api-common/src/api.rs +++ b/keymanager-api-common/src/api.rs @@ -88,7 +88,14 @@ pub struct SignedInitResponse { /// Key manager replication request. #[derive(Clone, Default, cbor::Encode, cbor::Decode)] pub struct ReplicateRequest { - // Empty. + /// Latest trust root height. + pub height: Option, +} + +impl ReplicateRequest { + pub fn new(height: Option) -> Self { + Self { height } + } } /// Key manager replication response. @@ -103,6 +110,8 @@ pub struct ReplicateResponse { /// from the master secret. They can be generated at any time. #[derive(Clone, Default, cbor::Encode, cbor::Decode)] pub struct LongTermKeyRequest { + /// Latest trust root height. + pub height: Option, /// Runtime ID. pub runtime_id: Namespace, /// Key pair ID. @@ -110,8 +119,9 @@ pub struct LongTermKeyRequest { } impl LongTermKeyRequest { - pub fn new(runtime_id: Namespace, key_pair_id: KeyPairId) -> Self { + pub fn new(height: Option, runtime_id: Namespace, key_pair_id: KeyPairId) -> Self { Self { + height, runtime_id, key_pair_id, } @@ -125,6 +135,8 @@ impl LongTermKeyRequest { /// for the past few epochs relative to the consensus layer state. #[derive(Clone, Default, cbor::Encode, cbor::Decode)] pub struct EphemeralKeyRequest { + /// Latest trust root height. + pub height: Option, /// Runtime ID. pub runtime_id: Namespace, /// Key pair ID. @@ -134,8 +146,14 @@ pub struct EphemeralKeyRequest { } impl EphemeralKeyRequest { - pub fn new(runtime_id: Namespace, key_pair_id: KeyPairId, epoch: EpochTime) -> Self { + pub fn new( + height: Option, + runtime_id: Namespace, + key_pair_id: KeyPairId, + epoch: EpochTime, + ) -> Self { Self { + height, runtime_id, key_pair_id, epoch, @@ -225,6 +243,8 @@ pub enum KeyManagerError { NotAuthorized, #[error("invalid epoch")] InvalidEpoch, + #[error("height is not fresh")] + HeightNotFresh, #[error("key manager is not initialized")] NotInitialized, #[error("key manager state corrupted")] diff --git a/keymanager-client/src/client.rs b/keymanager-client/src/client.rs index bacb96afb08..9ee42e59052 100644 --- a/keymanager-client/src/client.rs +++ b/keymanager-client/src/client.rs @@ -13,7 +13,7 @@ use oasis_core_client::RpcClient; use oasis_core_keymanager_api_common::*; use oasis_core_runtime::{ common::{namespace::Namespace, sgx::EnclaveIdentity}, - consensus::beacon::EpochTime, + consensus::{beacon::EpochTime, verifier::Verifier}, enclave_rpc::session, protocol::Protocol, rak::RAK, @@ -29,6 +29,8 @@ struct Inner { runtime_id: Namespace, /// RPC client. rpc_client: RpcClient, + /// Consensus verifier. + consensus_verifier: Arc, /// Local cache for the long-term and ephemeral private keys fetched from /// get_or_create_keys and get_or_create_ephemeral_keys KeyManager endpoints. private_key_cache: RwLock), KeyPair>>, @@ -43,11 +45,17 @@ pub struct RemoteClient { } impl RemoteClient { - fn new(runtime_id: Namespace, rpc_client: RpcClient, keys_cache_sizes: usize) -> Self { + fn new( + runtime_id: Namespace, + rpc_client: RpcClient, + consensus_verifier: Arc, + keys_cache_sizes: usize, + ) -> Self { Self { inner: Arc::new(Inner { runtime_id, rpc_client, + consensus_verifier, private_key_cache: RwLock::new(LruCache::new(keys_cache_sizes)), public_key_cache: RwLock::new(LruCache::new(keys_cache_sizes)), }), @@ -60,6 +68,7 @@ impl RemoteClient { runtime_id: Namespace, enclaves: Option>, protocol: Arc, + consensus_verifier: Arc, rak: Arc, keys_cache_sizes: usize, ) -> Self { @@ -72,6 +81,7 @@ impl RemoteClient { protocol, KEY_MANAGER_ENDPOINT, ), + consensus_verifier, keys_cache_sizes, ) } @@ -84,6 +94,7 @@ impl RemoteClient { pub fn new_runtime( runtime_id: Namespace, protocol: Arc, + consensus_verifier: Arc, rak: Arc, keys_cache_sizes: usize, signers: TrustedPolicySigners, @@ -111,6 +122,7 @@ impl RemoteClient { runtime_id, enclaves, protocol, + consensus_verifier, rak, keys_cache_sizes, ) @@ -154,12 +166,17 @@ impl KeyManagerClient for RemoteClient { // No entry in cache, fetch from key manager. let inner = self.inner.clone(); Box::pin(async move { + let height = inner + .consensus_verifier + .latest_height() + .map_err(|err| KeyManagerError::Other(err.into()))?; + let keys: KeyPair = inner .rpc_client .call( ctx, METHOD_GET_OR_CREATE_KEYS, - LongTermKeyRequest::new(inner.runtime_id, key_pair_id), + LongTermKeyRequest::new(Some(height), inner.runtime_id, key_pair_id), ) .await .map_err(|err| KeyManagerError::Other(err.into()))?; @@ -185,12 +202,17 @@ impl KeyManagerClient for RemoteClient { // No entry in cache, fetch from key manager. let inner = self.inner.clone(); Box::pin(async move { + let height = inner + .consensus_verifier + .latest_height() + .map_err(|err| KeyManagerError::Other(err.into()))?; + let key: Option = inner .rpc_client .call( ctx, METHOD_GET_PUBLIC_KEY, - LongTermKeyRequest::new(inner.runtime_id, key_pair_id), + LongTermKeyRequest::new(Some(height), inner.runtime_id, key_pair_id), ) .await .map_err(|err| KeyManagerError::Other(err.into()))?; @@ -222,12 +244,17 @@ impl KeyManagerClient for RemoteClient { // No entry in cache, fetch from key manager. let inner = self.inner.clone(); Box::pin(async move { + let height = inner + .consensus_verifier + .latest_height() + .map_err(|err| KeyManagerError::Other(err.into()))?; + let keys: KeyPair = inner .rpc_client .call( ctx, METHOD_GET_OR_CREATE_EPHEMERAL_KEYS, - EphemeralKeyRequest::new(inner.runtime_id, key_pair_id, epoch), + EphemeralKeyRequest::new(Some(height), inner.runtime_id, key_pair_id, epoch), ) .await .map_err(|err| KeyManagerError::Other(err.into()))?; @@ -254,12 +281,17 @@ impl KeyManagerClient for RemoteClient { // No entry in cache, fetch from key manager. let inner = self.inner.clone(); Box::pin(async move { + let height = inner + .consensus_verifier + .latest_height() + .map_err(|err| KeyManagerError::Other(err.into()))?; + let key: Option = inner .rpc_client .call( ctx, METHOD_GET_PUBLIC_EPHEMERAL_KEY, - EphemeralKeyRequest::new(inner.runtime_id, key_pair_id, epoch), + EphemeralKeyRequest::new(Some(height), inner.runtime_id, key_pair_id, epoch), ) .await .map_err(|err| KeyManagerError::Other(err.into()))?; @@ -283,9 +315,18 @@ impl KeyManagerClient for RemoteClient { ) -> BoxFuture, KeyManagerError>> { let inner = self.inner.clone(); Box::pin(async move { + let height = inner + .consensus_verifier + .latest_height() + .map_err(|err| KeyManagerError::Other(err.into()))?; + let rsp: ReplicateResponse = inner .rpc_client - .call(ctx, METHOD_REPLICATE_MASTER_SECRET, ReplicateRequest {}) + .call( + ctx, + METHOD_REPLICATE_MASTER_SECRET, + ReplicateRequest::new(Some(height)), + ) .await .map_err(|err| KeyManagerError::Other(err.into()))?; Ok(Some(rsp.master_secret)) diff --git a/keymanager-lib/src/kdf.rs b/keymanager-lib/src/kdf.rs index c0ac8c75ce1..0dedf3006be 100644 --- a/keymanager-lib/src/kdf.rs +++ b/keymanager-lib/src/kdf.rs @@ -292,6 +292,7 @@ impl Kdf { rctx.runtime_id, Policy::global().may_replicate_from(), rctx.protocol.clone(), + ctx.consensus_verifier.clone(), ctx.rak.clone(), 1, // Not used, doesn't matter. ); @@ -699,10 +700,12 @@ mod tests { let runtime_id = Namespace::from(vec![1u8; 32]); let key_pair_id = KeyPairId::from(vec![1u8; 32]); let epoch = 1; + let height = None; // LongTermKeyRequest's seed should depend on runtime_id and // key_pair_id. let req1 = LongTermKeyRequest { + height, runtime_id, key_pair_id, }; @@ -721,6 +724,7 @@ mod tests { // EphemeralKeyRequest's seed should depend on runtime_id, key_pair_id // and epoch. let req1 = EphemeralKeyRequest { + height, epoch, runtime_id, key_pair_id, diff --git a/keymanager-lib/src/methods.rs b/keymanager-lib/src/methods.rs index 150639ee174..2982f1640ef 100644 --- a/keymanager-lib/src/methods.rs +++ b/keymanager-lib/src/methods.rs @@ -9,12 +9,18 @@ use oasis_core_runtime::{ enclave_rpc::Context as RpcContext, }; -/// Maximum age of ephemeral key in the number of epochs. +/// Maximum age of an ephemeral key in the number of epochs. const MAX_EPHEMERAL_KEY_AGE: EpochTime = 10; +/// Maximum age of a fresh height in the number of blocks. +/// +/// A height is considered fresh if it is not more than specified amount +/// of blocks lower than the height of the latest trust root. +const MAX_FRESH_HEIGHT_AGE: u64 = 50; /// See `Kdf::get_or_create_keys`. pub fn get_or_create_keys(req: &LongTermKeyRequest, ctx: &mut RpcContext) -> Result { authorize_private_key_generation(&req.runtime_id, ctx)?; + validate_height_freshness(req.height, ctx)?; Kdf::global().get_or_create_keys(req) } @@ -39,6 +45,7 @@ pub fn get_or_create_ephemeral_keys( ) -> Result { authorize_private_key_generation(&req.runtime_id, ctx)?; validate_epoch(req.epoch, ctx)?; + validate_height_freshness(req.height, ctx)?; Kdf::global().get_or_create_keys(req) } @@ -59,10 +66,11 @@ pub fn get_public_ephemeral_key( /// See `Kdf::replicate_master_secret`. pub fn replicate_master_secret( - _req: &ReplicateRequest, + req: &ReplicateRequest, ctx: &mut RpcContext, ) -> Result { authorize_master_secret_replication(ctx)?; + validate_height_freshness(req.height, ctx)?; Kdf::global().replicate_master_secret() } @@ -92,14 +100,32 @@ fn authenticate<'a>(ctx: &'a RpcContext) -> Result<&'a EnclaveIdentity> { Ok(&si.verified_quote.identity) } -// Validate that the epoch used for derivation of ephemeral private keys is not -// in the future or too far back in the past. +/// Validate that the epoch used for derivation of ephemeral private keys is not +/// in the future or too far back in the past. fn validate_epoch(epoch: EpochTime, ctx: &RpcContext) -> Result<()> { let consensus_state = ctx.consensus_verifier.latest_state()?; let beacon_state = BeaconState::new(&consensus_state); let consensus_epoch = beacon_state.epoch(Context::create_child(&ctx.io_ctx))?; if consensus_epoch < epoch || consensus_epoch > epoch + MAX_EPHEMERAL_KEY_AGE { - return Err(KeyManagerError::InvalidEpoch.into()); + return Err(anyhow::anyhow!(KeyManagerError::InvalidEpoch)); + } + Ok(()) +} + +/// Validate that given height is fresh, i.e. the height is not more than +/// predefined number of blocks lower than the height of the latest trust root. +/// +/// Key manager should use this validation to detect whether the runtimes +/// querying it have a fresh enough state. +fn validate_height_freshness(height: Option, ctx: &RpcContext) -> Result<()> { + // Outdated key manager clients will not send height in their requests. + // To ensure backwards compatibility we skip check in those cases. + // This should be removed in the future by making height mandatory. + if let Some(height) = height { + let latest_height = ctx.consensus_verifier.latest_height()?; + if latest_height > MAX_FRESH_HEIGHT_AGE && height < latest_height - MAX_FRESH_HEIGHT_AGE { + return Err(anyhow::anyhow!(KeyManagerError::HeightNotFresh)); + } } Ok(()) } diff --git a/runtime/src/dispatcher.rs b/runtime/src/dispatcher.rs index 120d4b9dd5c..5e6e0bf5e48 100644 --- a/runtime/src/dispatcher.rs +++ b/runtime/src/dispatcher.rs @@ -126,7 +126,7 @@ struct TxDispatchState { /// State provided by the protocol upon successful initialization. struct ProtocolState { protocol: Arc, - consensus_verifier: Box, + consensus_verifier: Arc, } /// State held by the dispatcher, shared between all async tasks. @@ -205,6 +205,7 @@ impl Dispatcher { /// Start the dispatcher. pub fn start(&self, protocol: Arc, consensus_verifier: Box) { + let consensus_verifier = Arc::from(consensus_verifier); let mut s = self.state.lock().unwrap(); *s = Some(ProtocolState { protocol, @@ -249,7 +250,6 @@ impl Dispatcher { info!(self.logger, "Starting the runtime dispatcher"); let mut rpc_demux = RpcDemux::new(self.rak.clone()); let mut rpc_dispatcher = RpcDispatcher::default(); - let consensus_verifier: Arc = Arc::from(consensus_verifier); let pre_init_state = PreInitState { protocol: &protocol, rak: &self.rak, diff --git a/tests/runtimes/simple-keyvalue/src/main.rs b/tests/runtimes/simple-keyvalue/src/main.rs index 22f6d506e61..2c7acf731cd 100644 --- a/tests/runtimes/simple-keyvalue/src/main.rs +++ b/tests/runtimes/simple-keyvalue/src/main.rs @@ -358,6 +358,7 @@ pub fn main_with_version(version: Version) { let km_client = Arc::new(oasis_core_keymanager_client::RemoteClient::new_runtime( hi.runtime_id, state.protocol.clone(), + state.consensus_verifier.clone(), state.rak.clone(), 1024, trusted_policy_signers(),