diff --git a/.changelog/5458.bugfix.md b/.changelog/5458.bugfix.md new file mode 100644 index 00000000000..c1fe19c3636 --- /dev/null +++ b/.changelog/5458.bugfix.md @@ -0,0 +1,6 @@ +runtime: Improve error reporting if DeoxysII unsealing fails + +Previously, if the CPU changed between runs of the Oasis node, the error +reported was a cryptic "ciphertext is corrupted" (because the sealed SGX +secrets were invalidated). +Now we add a bit more context to make it easier for the end-user. diff --git a/keymanager/src/crypto/kdf.rs b/keymanager/src/crypto/kdf.rs index 61cc288b9f2..5de032e06ab 100644 --- a/keymanager/src/crypto/kdf.rs +++ b/keymanager/src/crypto/kdf.rs @@ -780,7 +780,7 @@ impl Kdf { let d2 = new_deoxysii(Keypolicy::MRENCLAVE, MASTER_SECRET_SEAL_CONTEXT); let plaintext = d2 .open(&nonce, ciphertext.to_vec(), additional_data) - .expect("persisted state is corrupted"); + .unwrap(); Some(Secret(plaintext.try_into().unwrap())) } diff --git a/keymanager/src/policy/cached.rs b/keymanager/src/policy/cached.rs index 7a9c7072cd9..2ee07a1130c 100644 --- a/keymanager/src/policy/cached.rs +++ b/keymanager/src/policy/cached.rs @@ -172,10 +172,12 @@ impl Policy { fn load_policy(storage: &dyn KeyValue) -> Option { let ciphertext = storage.get(POLICY_STORAGE_KEY.to_vec()).unwrap(); - unseal(Keypolicy::MRENCLAVE, POLICY_SEAL_CONTEXT, &ciphertext).map(|plaintext| { - // Deserialization failures are fatal, because it is state corruption. - CachedPolicy::parse_raw(&plaintext).expect("failed to deserialize persisted policy") - }) + unseal(Keypolicy::MRENCLAVE, POLICY_SEAL_CONTEXT, &ciphertext) + .unwrap() + .map(|plaintext| { + // Deserialization failures are fatal, because it is state corruption. + CachedPolicy::parse_raw(&plaintext).expect("failed to deserialize persisted policy") + }) } fn save_raw_policy(storage: &dyn KeyValue, raw_policy: &[u8]) { diff --git a/runtime/src/common/sgx/seal.rs b/runtime/src/common/sgx/seal.rs index d320cdac02c..96c1f337e37 100644 --- a/runtime/src/common/sgx/seal.rs +++ b/runtime/src/common/sgx/seal.rs @@ -1,4 +1,5 @@ //! Wrappers for sealing secrets to the enclave in cold storage. +use anyhow::{format_err, Error}; use rand::{rngs::OsRng, Rng}; use sgx_isa::Keypolicy; use zeroize::Zeroize; @@ -27,20 +28,18 @@ pub fn seal(key_policy: Keypolicy, context: &[u8], data: &[u8]) -> Vec { /// Unseal a previously sealed secret to the enclave. /// /// The `context` field is a domain separation tag. -/// -/// # Panics -/// -/// All parsing and authentication errors of the ciphertext are fatal and -/// will result in a panic. -pub fn unseal(key_policy: Keypolicy, context: &[u8], ciphertext: &[u8]) -> Option> { +pub fn unseal( + key_policy: Keypolicy, + context: &[u8], + ciphertext: &[u8], +) -> Result>, Error> { let ct_len = ciphertext.len(); if ct_len == 0 { - return None; + return Ok(None); + } + if ct_len < TAG_SIZE + NONCE_SIZE { + return Err(format_err!("ciphertext is corrupted: invalid size")); } - assert!( - ct_len >= TAG_SIZE + NONCE_SIZE, - "ciphertext is corrupted, invalid size" - ); let ct_len = ct_len - NONCE_SIZE; // Split the ciphertext || tag || nonce. @@ -49,11 +48,11 @@ pub fn unseal(key_policy: Keypolicy, context: &[u8], ciphertext: &[u8]) -> Optio let ciphertext = &ciphertext[..ct_len]; let d2 = new_deoxysii(key_policy, context); - let plaintext = d2 - .open(&nonce, ciphertext.to_vec(), vec![]) - .expect("ciphertext is corrupted"); - Some(plaintext) + match d2.open(&nonce, ciphertext.to_vec(), vec![]) { + Ok(plaintext) => Ok(Some(plaintext)), + Err(_) => Err(format_err!("ciphertext is corrupted")), + } } /// Creates a new Deoxys-II instance initialized with an SGX sealing key derived @@ -77,37 +76,37 @@ mod tests { // Test different policies. let sealed_a = seal(Keypolicy::MRSIGNER, b"MRSIGNER", b"Mr. Signer"); let unsealed_a = unseal(Keypolicy::MRSIGNER, b"MRSIGNER", &sealed_a); - assert_eq!(unsealed_a, Some(b"Mr. Signer".to_vec())); + assert_eq!(unsealed_a.unwrap(), Some(b"Mr. Signer".to_vec())); let sealed_b = seal(Keypolicy::MRENCLAVE, b"MRENCLAVE", b"Mr. Enclave"); let unsealed_b = unseal(Keypolicy::MRENCLAVE, b"MRENCLAVE", &sealed_b); - assert_eq!(unsealed_b, Some(b"Mr. Enclave".to_vec())); + assert_eq!(unsealed_b.unwrap(), Some(b"Mr. Enclave".to_vec())); // Test zero-length ciphertext. let unsealed_c = unseal(Keypolicy::MRENCLAVE, b"MRENCLAVE", b""); - assert_eq!(unsealed_c, None); + assert_eq!(unsealed_c.unwrap(), None); } #[test] - #[should_panic] fn test_incorrect_context() { // Test incorrect context. let sealed_b = seal(Keypolicy::MRENCLAVE, b"MRENCLAVE1", b"Mr. Enclave"); - unseal(Keypolicy::MRENCLAVE, b"MRENCLAVE2", &sealed_b); + let unsealed_b = unseal(Keypolicy::MRENCLAVE, b"MRENCLAVE2", &sealed_b); + assert_eq!(unsealed_b.is_err(), true); } #[test] - #[should_panic] fn test_incorrect_ciphertext_a() { let sealed_b = seal(Keypolicy::MRENCLAVE, b"MRENCLAVE", b"Mr. Enclave"); - unseal(Keypolicy::MRENCLAVE, b"MRENCLAVE", &sealed_b[..2]); + let unsealed_b = unseal(Keypolicy::MRENCLAVE, b"MRENCLAVE", &sealed_b[..2]); + assert_eq!(unsealed_b.is_err(), true); } #[test] - #[should_panic] fn test_incorrect_ciphertext_b() { let mut sealed_b = seal(Keypolicy::MRENCLAVE, b"MRENCLAVE", b"Mr. Enclave"); sealed_b[0] = sealed_b[0].wrapping_add(1); - unseal(Keypolicy::MRENCLAVE, b"MRENCLAVE", &sealed_b); + let unsealed_b = unseal(Keypolicy::MRENCLAVE, b"MRENCLAVE", &sealed_b); + assert_eq!(unsealed_b.is_err(), true); } } diff --git a/runtime/src/consensus/tendermint/verifier/mod.rs b/runtime/src/consensus/tendermint/verifier/mod.rs index 74fe358a74e..3df0ff6ff77 100644 --- a/runtime/src/consensus/tendermint/verifier/mod.rs +++ b/runtime/src/consensus/tendermint/verifier/mod.rs @@ -646,9 +646,17 @@ impl Verifier { // Build a light client using the embedded trust root or trust root // stored in the local store. info!(self.logger, "Loading trusted state"); - let trusted_state: TrustedState = self + let trusted_state = self .trusted_state_store - .load(self.runtime_version, &self.trust_root)?; + .load(self.runtime_version, &self.trust_root); + + let trusted_state: TrustedState = match trusted_state { + Ok(state) => state, + Err(err) => { + error!(self.logger, "failed to load trusted state (if running in SGX mode, check if the CPU had changed; if yes, wipe 'worker-local-storage.badger.db' and restart the node)"); + return Err(err); + } + }; // Verify if we can trust light blocks from a new chain if the consensus // chain context changes. diff --git a/runtime/src/consensus/tendermint/verifier/store/state.rs b/runtime/src/consensus/tendermint/verifier/store/state.rs index ef2fccbe520..e79e37a9029 100644 --- a/runtime/src/consensus/tendermint/verifier/store/state.rs +++ b/runtime/src/consensus/tendermint/verifier/store/state.rs @@ -147,7 +147,9 @@ impl TrustedStateStore { TRUSTED_STATE_CONTEXT, &untrusted_value, ) + .map_err(|_| Error::TrustedStateLoadingFailed)? .unwrap(); + let trusted_state: TrustedState = cbor::from_slice(&raw).expect("corrupted sealed trusted state");