Update provider to use new version fo TransKeyCtx

This commit updates the TPM provider to use the updated version of the TransientKeyContext which handles key material in the form of public and private fields (instead of an encrypted context). A migration mechanism is also added, to aid in the recovery of keys stored as contexts. It also adds a replace_key_info method to the KIM client, which allows the above-mentioned migration to be done in a data-race safe way. Signed-off-by: Ionut Mihalcea <[email protected]>
parallaxsecond · Sep 17, 2021 · b3bb5ad · b3bb5ad
1 parent 48d55aa
commit b3bb5ad
Show file tree

Hide file tree

Showing 13 changed files with 454 additions and 284 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -29,7 +29,7 @@ log = { version = "0.4.14", features = ["serde"] }
 cryptoki = { version = "0.2.0", optional = true, features = ["psa-crypto-conversions"] }
 picky-asn1-der = { version = "<=0.2.4", optional = true }
 picky-asn1 = { version = ">=0.3.1, <=0.3.1", optional = true }
-tss-esapi = { version = "6.1.0", optional = true }
+tss-esapi = { version = "7.0.0-alpha.1", optional = true }
 bincode = "1.3.1"
 structopt = "0.3.21"
 derivative = "2.2.0"

diff --git a/ci.sh b/ci.sh
@@ -366,4 +366,27 @@ else
         echo "Execute stress tests"
         RUST_BACKTRACE=1 cargo test $TEST_FEATURES --manifest-path ./e2e_tests/Cargo.toml stress_test
 	fi
+
+    # For the TPM provider we check that keys can still be used after a TPM Reset
+    if [ "$PROVIDER_NAME" = "tpm" ]; then
+        # We first create the keys
+        RUST_BACKTRACE=1 cargo test $TEST_FEATURES --manifest-path ./e2e_tests/Cargo.toml before_tpm_reset
+        stop_service
+
+        # In order to reset the TPM, we need to restart the TPM server and send a Startup(CLEAR)
+        pkill tpm_server
+        sleep 1
+
+        tpm_server &
+        TPM_SRV_PID=$!
+        sleep 5
+
+        tpm2_startup -c -T mssim
+
+        # We then spin up the service again and check that the keys can still be used
+        RUST_LOG=error RUST_BACKTRACE=1 cargo run --release $FEATURES -- --config $CONFIG_PATH &
+        wait_for_service
+
+        RUST_BACKTRACE=1 cargo test $TEST_FEATURES --manifest-path ./e2e_tests/Cargo.toml after_tpm_reset
+    fi
 fi
diff --git a/e2e_tests/tests/per_provider/mod.rs b/e2e_tests/tests/per_provider/mod.rs
@@ -3,3 +3,5 @@
 mod key_mappings;
 mod normal_tests;
 mod stress_test;
+#[cfg(feature = "tpm-provider")]
+mod tpm_reset;
diff --git a/e2e_tests/tests/per_provider/normal_tests/asym_sign_verify.rs b/e2e_tests/tests/per_provider/normal_tests/asym_sign_verify.rs
@@ -90,7 +90,7 @@ fn asym_verify_fail_ecc_sha256() -> Result<()> {
 #[test]
 fn only_verify_from_internet() -> Result<()> {
     let mut client = TestClient::new();
-    let key_name = String::from("only_verify");
+    let key_name = String::from("only_verify_from_internet");
     if !client.is_operation_supported(Opcode::PsaImportKey) {
         return Ok(());
     }

diff --git a/e2e_tests/tests/per_provider/normal_tests/import_key.rs b/e2e_tests/tests/per_provider/normal_tests/import_key.rs
@@ -365,43 +365,43 @@ fn failed_imported_key_should_be_removed() -> Result<()> {
     Ok(())
 }
 
-#[cfg(feature = "tpm-provider")]
-#[test]
-fn import_key_pair() {
-    let mut client = TestClient::new();
-    let key_name = String::from("failed_imported_key_should_be_removed");
-
-    client
-        .import_key(
-            key_name,
-            Attributes {
-                lifetime: Lifetime::Persistent,
-                key_type: Type::RsaKeyPair,
-                bits: 1024,
-                policy: Policy {
-                    usage_flags: UsageFlags {
-                        export: false,
-                        copy: false,
-                        cache: false,
-                        encrypt: false,
-                        decrypt: false,
-                        sign_message: true,
-                        sign_hash: true,
-                        verify_message: true,
-                        verify_hash: true,
-                        derive: false,
-                    },
-                    permitted_algorithms: Algorithm::AsymmetricSignature(
-                        AsymmetricSignature::RsaPkcs1v15Sign {
-                            hash_alg: Hash::Sha256.into(),
-                        },
-                    ),
-                },
-            },
-            KEY_PAIR_DATA.to_vec(),
-        )
-        .unwrap();
-}
+// #[cfg(feature = "tpm-provider")]
+// #[test]
+// fn import_key_pair() {
+//     let mut client = TestClient::new();
+//     let key_name = String::from("failed_imported_key_should_be_removed");
+
+//     client
+//         .import_key(
+//             key_name,
+//             Attributes {
+//                 lifetime: Lifetime::Persistent,
+//                 key_type: Type::RsaKeyPair,
+//                 bits: 1024,
+//                 policy: Policy {
+//                     usage_flags: UsageFlags {
+//                         export: false,
+//                         copy: false,
+//                         cache: false,
+//                         encrypt: false,
+//                         decrypt: false,
+//                         sign_message: true,
+//                         sign_hash: true,
+//                         verify_message: true,
+//                         verify_hash: true,
+//                         derive: false,
+//                     },
+//                     permitted_algorithms: Algorithm::AsymmetricSignature(
+//                         AsymmetricSignature::RsaPkcs1v15Sign {
+//                             hash_alg: Hash::Sha256.into(),
+//                         },
+//                     ),
+//                 },
+//             },
+//             KEY_PAIR_DATA.to_vec(),
+//         )
+//         .unwrap();
+// }
 
 #[cfg(any(feature = "mbed-crypto-provider", feature = "cryptoauthlib-provider"))]
 #[test]

diff --git a/e2e_tests/tests/per_provider/tpm_reset.rs b/e2e_tests/tests/per_provider/tpm_reset.rs
@@ -0,0 +1,43 @@
+// Copyright 2021 Contributors to the Parsec project.
+// SPDX-License-Identifier: Apache-2.0
+
+// These tests track a potential regression where the TPM provider
+// was unable to handle stored keys after a TPM reset.
+//
+// `before_tpm_reset` creates keys that should be usable post-TPM-reset,
+// in `after_tpm_reset`.
+//
+// See: https://github.com/parallaxsecond/parsec/issues/504
+use e2e_tests::TestClient;
+
+const RSA_KEY_NAME: &str = "tpm-reset-rsa";
+const ECC_KEY_NAME: &str = "tpm-reset-ecc";
+
+#[test]
+fn before_tpm_reset() {
+    let mut client = TestClient::new();
+    client.do_not_destroy_keys();
+
+    let rsa_key_name = String::from(RSA_KEY_NAME);
+    let ecc_key_name = String::from(ECC_KEY_NAME);
+
+    client.generate_rsa_sign_key(rsa_key_name.clone()).unwrap();
+    client
+        .generate_ecc_key_pair_secpr1_ecdsa_sha256(ecc_key_name.clone())
+        .unwrap();
+}
+
+#[test]
+fn after_tpm_reset() {
+    let mut client = TestClient::new();
+
+    let rsa_key_name = String::from(RSA_KEY_NAME);
+    let ecc_key_name = String::from(ECC_KEY_NAME);
+
+    let _ = client
+        .sign_with_rsa_sha256(rsa_key_name, vec![0xff; 32])
+        .unwrap();
+    let _ = client
+        .sign_with_ecdsa_sha256(ecc_key_name, vec![0xff; 32])
+        .unwrap();
+}
diff --git a/src/key_info_managers/mod.rs b/src/key_info_managers/mod.rs
@@ -261,6 +261,39 @@ impl KeyInfoManagerClient {
         }
     }
 
+    /// Replace the key info saved for a given triple
+    ///
+    /// # Errors
+    ///
+    /// If the key triple doesn't exist in the KIM, PsaErrorDoesNotExist is returned. For
+    /// any other error occurring in the KIM, KeyInfoManagerError is returned.
+    pub fn replace_key_info<T: Serialize>(
+        &self,
+        key_triple: KeyTriple,
+        key_id: &T,
+        attributes: Attributes,
+    ) -> parsec_interface::requests::Result<()> {
+        let mut key_info_manager_impl = self
+            .key_info_manager_impl
+            .write()
+            .expect("Key Info Manager lock poisoned");
+        let key_info = KeyInfo {
+            id: bincode::serialize(key_id)?,
+            attributes,
+        };
+
+        match key_info_manager_impl.insert(key_triple.clone(), key_info) {
+            Ok(None) => {
+                let _ = key_info_manager_impl
+                    .remove(&key_triple)
+                    .map_err(to_response_status)?;
+                Err(ResponseStatus::PsaErrorDoesNotExist)
+            }
+            Ok(Some(_)) => Ok(()),
+            Err(string) => Err(to_response_status(string)),
+        }
+    }
+
     /// Returns a Vec of ApplicationName of clients having keys in the provider.
     ///
     /// # Errors

diff --git a/src/providers/tpm/asym_encryption.rs b/src/providers/tpm/asym_encryption.rs
@@ -1,9 +1,6 @@
 // Copyright 2020 Contributors to the Parsec project.
 // SPDX-License-Identifier: Apache-2.0
-use super::{
-    utils::{self, PasswordContext},
-    Provider,
-};
+use super::{utils, Provider};
 use crate::authenticators::ApplicationName;
 use crate::key_info_managers::KeyTriple;
 use parsec_interface::operations::{psa_asymmetric_decrypt, psa_asymmetric_encrypt};
@@ -19,21 +16,22 @@ impl Provider {
     ) -> Result<psa_asymmetric_encrypt::Result> {
         let key_triple = KeyTriple::new(app_name, ProviderId::Tpm, op.key_name.clone());
 
+        let password_context = self.get_key_ctx(&key_triple)?;
+        let key_attributes = self.key_info_store.get_key_attributes(&key_triple)?;
+
         let mut esapi_context = self
             .esapi_context
             .lock()
             .expect("ESAPI Context lock poisoned");
 
-        let password_context: PasswordContext = self.key_info_store.get_key_id(&key_triple)?;
-        let key_attributes = self.key_info_store.get_key_attributes(&key_triple)?;
-
         op.validate(key_attributes)?;
 
         match esapi_context.rsa_encrypt(
-            password_context.context,
+            password_context.key_material().clone(),
+            utils::parsec_to_tpm_params(key_attributes)?,
             Some(
                 password_context
-                    .auth_value
+                    .auth_value()
                     .try_into()
                     .map_err(utils::to_response_status)?,
             ),
@@ -42,7 +40,6 @@ impl Provider {
                 .clone()
                 .try_into()
                 .map_err(utils::to_response_status)?,
-            utils::convert_asym_scheme_to_tpm(op.alg.into())?,
             match op.salt {
                 Some(salt) => Some(
                     salt.deref()
@@ -71,21 +68,22 @@ impl Provider {
     ) -> Result<psa_asymmetric_decrypt::Result> {
         let key_triple = KeyTriple::new(app_name, ProviderId::Tpm, op.key_name.clone());
 
+        let password_context = self.get_key_ctx(&key_triple)?;
+        let key_attributes = self.key_info_store.get_key_attributes(&key_triple)?;
+
         let mut esapi_context = self
             .esapi_context
             .lock()
             .expect("ESAPI Context lock poisoned");
 
-        let password_context: PasswordContext = self.key_info_store.get_key_id(&key_triple)?;
-        let key_attributes = self.key_info_store.get_key_attributes(&key_triple)?;
-
         op.validate(key_attributes)?;
 
         match esapi_context.rsa_decrypt(
-            password_context.context,
+            password_context.key_material().clone(),
+            utils::parsec_to_tpm_params(key_attributes)?,
             Some(
                 password_context
-                    .auth_value
+                    .auth_value()
                     .try_into()
                     .map_err(utils::to_response_status)?,
             ),
@@ -94,7 +92,6 @@ impl Provider {
                 .clone()
                 .try_into()
                 .map_err(utils::to_response_status)?,
-            utils::convert_asym_scheme_to_tpm(op.alg.into())?,
             match op.salt {
                 Some(salt) => Some(
                     salt.deref()