Merge pull request #5458 from oasisprotocol/andrej/bugfix/sgx-cpu-cha…

…nge-err

runtime: Improve error reporting if DeoxysII unsealing fails

.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.

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()))
     }

keymanager/src/policy/cached.rs

@@ -172,10 +172,12 @@ impl Policy {
     fn load_policy(storage: &dyn KeyValue) -> Option<CachedPolicy> {
         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]) {

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<u8> {
 /// 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<Vec<u8>> {
+pub fn unseal(
+    key_policy: Keypolicy,
+    context: &[u8],
+    ciphertext: &[u8],
+) -> Result<Option<Vec<u8>>, 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);
     }
 }

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.

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");