Merge branch 'gdemay/CRP-1929-SecretKeyStoreCspVault' into 'master'

test(crypto): CRP-1929 delegation proptest for `SecretKeyStoreCspVault`

Ensures by a `proptest` that the remote CSP vault delegates to the local CSP vault when called on any method of the trait `SecretKeyStoreCspVault`. 

See merge request dfinity-lab/public/ic!12569

Author: gregorydemay

rs/crypto/internal/crypto_service_provider/csp_proptest_utils/src/lib.rs

@@ -17,6 +17,7 @@ pub use csp_multi_signature_error::arb_csp_multi_signature_error;
 pub use csp_multi_signature_keygen_error::arb_csp_multi_signature_keygen_error;
 pub use csp_pop::arb_csp_pop;
 pub use csp_public_key::arb_csp_public_key;
+pub use csp_secret_key_store_contains_error::arb_csp_secret_key_store_contains_error;
 pub use csp_signature::arb_csp_signature;
 pub use csp_threshold_sign_error::arb_csp_threshold_sign_error;
 
@@ -272,3 +273,12 @@ mod csp_threshold_sign_error {
         InternalError => {internal_error in ".*"}
     );
 }
+
+mod csp_secret_key_store_contains_error {
+    use super::*;
+    use ic_crypto_internal_csp::vault::api::CspSecretKeyStoreContainsError;
+
+    proptest_strategy_for_enum!(CspSecretKeyStoreContainsError;
+        InternalError => {internal_error in ".*"}
+    );
+}

rs/crypto/internal/crypto_service_provider/csp_proptest_utils/src/tests.rs

@@ -142,3 +142,12 @@ should_have_a_strategy_for_each_variant!(
     MalformedSecretKey { .. },
     InternalError { .. }
 );
+
+use ic_crypto_internal_csp::vault::api::CspSecretKeyStoreContainsError;
+should_have_a_strategy_for_each_variant!(
+    CspSecretKeyStoreContainsError,
+    CspSecretKeyStoreContainsError::InternalError {
+        internal_error: "dummy error to match upon".to_string(),
+    },
+    InternalError { .. }
+);

rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/secret_key_store/tests.rs

@@ -1,19 +1,89 @@
+#![allow(clippy::unwrap_used)]
 //! Verifies the implementation of SecretKeyStoreCspVault for LocalCspVault.
-use crate::vault::test_utils;
+use crate::vault::api::BasicSignatureCspVault;
+use crate::vault::api::SecretKeyStoreCspVault;
+use crate::vault::api::TlsHandshakeCspVault;
+use crate::KeyId;
 use crate::LocalCspVault;
+use ic_types_test_utils::ids::node_test_id;
+
+const NODE_1: u64 = 4241;
+const NOT_AFTER: &str = "99991231235959Z";
 
 #[test]
 fn key_should_be_present_only_after_generation() {
-    test_utils::sks::sks_should_contain_keys_only_after_generation(
-        LocalCspVault::builder_for_test().build_into_arc(),
-        LocalCspVault::builder_for_test().build_into_arc(),
+    let csp_vault1 = LocalCspVault::builder_for_test().build_into_arc();
+    let csp_vault2 = LocalCspVault::builder_for_test().build_into_arc();
+    let public_key1 = csp_vault1
+        .gen_node_signing_key_pair()
+        .expect("Test setup failed: Failed to generate keys");
+    let key_id1 = KeyId::try_from(&public_key1).unwrap();
+    assert!(
+        csp_vault1.sks_contains(&key_id1).expect("SKS call failed"),
+        "Key should be present after generation."
+    );
+    assert!(
+        !csp_vault2.sks_contains(&key_id1).expect("SKS call failed"),
+        "Key should be absent if not generated in the CSP."
+    );
+
+    let public_key2 = csp_vault2
+        .gen_node_signing_key_pair()
+        .expect("Test setup failed: Failed to generate keys");
+    let key_id2 = KeyId::try_from(&public_key2).unwrap();
+    assert_ne!(
+        key_id1, key_id2,
+        "Test failure: Key IDs from different CSPs were the same.  Check random number generation."
+    );
+    assert!(
+        csp_vault2.sks_contains(&key_id2).expect("SKS call failed"),
+        "Key should be present in the CSP that generated it."
+    );
+    assert!(
+        !csp_vault2.sks_contains(&key_id1).expect("SKS call failed"),
+        "The second CSP should not contain the keys of the first."
+    );
+    assert!(
+        !csp_vault1.sks_contains(&key_id2).expect("SKS call failed"),
+        "Key first CSP should not contain the keys of the second."
     );
 }
 
 #[test]
 fn tls_key_should_be_present_only_after_generation() {
-    test_utils::sks::sks_should_contain_tls_keys_only_after_generation(
-        LocalCspVault::builder_for_test().build_into_arc(),
-        LocalCspVault::builder_for_test().build_into_arc(),
+    let csp_vault1 = LocalCspVault::builder_for_test().build_into_arc();
+    let csp_vault2 = LocalCspVault::builder_for_test().build_into_arc();
+    let public_key_cert1 = csp_vault1
+        .gen_tls_key_pair(node_test_id(NODE_1), NOT_AFTER)
+        .expect("error generating TLS key pair");
+    let key_id1 = KeyId::try_from(&public_key_cert1).unwrap();
+    assert!(
+        csp_vault1.sks_contains(&key_id1).expect("SKS call failed"),
+        "TLS key should be present after generation."
+    );
+    assert!(
+        !csp_vault2.sks_contains(&key_id1).expect("SKS call failed"),
+        "TLS key should be absent if not generated in the CSP."
+    );
+
+    let public_key_cert2 = csp_vault2
+        .gen_tls_key_pair(node_test_id(NODE_1), NOT_AFTER)
+        .expect("error generating TLS key pair");
+    let key_id2 = KeyId::try_from(&public_key_cert2).unwrap();
+    assert_ne!(
+        key_id1, key_id2,
+        "Test failure: Key IDs from different CSPs were the same.  Check random number generation."
+    );
+    assert!(
+        csp_vault2.sks_contains(&key_id2).expect("SKS call failed"),
+        "TLS key should be present in the CSP that generated it."
+    );
+    assert!(
+        !csp_vault2.sks_contains(&key_id1).expect("SKS call failed"),
+        "The second CSP should not contain the TLS keys of the first."
+    );
+    assert!(
+        !csp_vault1.sks_contains(&key_id2).expect("SKS call failed"),
+        "Key first CSP should not contain the TLS keys of the second."
     );
 }

rs/crypto/internal/crypto_service_provider/src/vault/remote_csp_vault/tests.rs

@@ -90,34 +90,6 @@ mod timeout {
     }
 }
 
-mod secret_key_store {
-    use super::*;
-
-    #[test]
-    fn key_should_be_present_only_after_generation() {
-        let tokio_rt = new_tokio_runtime();
-        let (vault_1, vault_2) = new_csp_vaults_for_test(tokio_rt.handle());
-
-        test_utils::sks::sks_should_contain_keys_only_after_generation(vault_1, vault_2);
-    }
-
-    #[test]
-    fn tls_key_should_be_present_only_after_generation() {
-        let tokio_rt = new_tokio_runtime();
-        let (vault_1, vault_2) = new_csp_vaults_for_test(tokio_rt.handle());
-
-        test_utils::sks::sks_should_contain_tls_keys_only_after_generation(vault_1, vault_2);
-    }
-
-    fn new_csp_vaults_for_test(
-        rt_handle: &tokio::runtime::Handle,
-    ) -> (Arc<dyn CspVault>, Arc<dyn CspVault>) {
-        let csp_vault_1 = new_remote_csp_vault(rt_handle);
-        let csp_vault_2 = new_remote_csp_vault(rt_handle);
-        (csp_vault_1, csp_vault_2)
-    }
-}
-
 mod ni_dkg {
     use super::*;
     use crate::public_key_store::mock_pubkey_store::MockPublicKeyStore;

rs/crypto/internal/crypto_service_provider/src/vault/test_utils/sks.rs

@@ -1,104 +1,10 @@
-#![allow(clippy::unwrap_used)]
 use crate::secret_key_store::mock_secret_key_store::MockSecretKeyStore;
 use crate::secret_key_store::temp_secret_key_store::TempSecretKeyStore;
 use crate::secret_key_store::SecretKeyStoreInsertionError;
 use crate::types::CspSecretKey;
-use crate::vault::api::CspVault;
 use crate::{KeyId, SecretKeyStore};
 use ic_crypto_internal_tls::keygen::TlsEd25519SecretKeyDerBytes;
-use ic_types_test_utils::ids::node_test_id;
 use openssl::pkey::PKey;
-use std::sync::Arc;
-
-const NODE_1: u64 = 4241;
-const NOT_AFTER: &str = "99991231235959Z";
-
-/// Key should be present only after key generation.
-///
-/// Note:  Theoretically the invariant is: The key should be present only in the
-/// CSP that generated it, and only after generation and before deletion, if
-/// deletion is supported for that key type.  Thus ideally there should be a
-/// test that generates many sequences of events and verifies that this
-/// invariant holds, regardless of the sequence of events, the number or type of
-/// keys in the CSP and so on.  Making such a test is hard, so this is just one
-/// sequence of events.
-pub fn sks_should_contain_keys_only_after_generation(
-    csp_vault1: Arc<dyn CspVault>,
-    csp_vault2: Arc<dyn CspVault>,
-) {
-    let public_key1 = csp_vault1
-        .gen_node_signing_key_pair()
-        .expect("Test setup failed: Failed to generate keys");
-    let key_id1 = KeyId::try_from(&public_key1).unwrap();
-    assert!(
-        csp_vault1.sks_contains(&key_id1).expect("SKS call failed"),
-        "Key should be present after generation."
-    );
-    assert!(
-        !csp_vault2.sks_contains(&key_id1).expect("SKS call failed"),
-        "Key should be absent if not generated in the CSP."
-    );
-
-    let public_key2 = csp_vault2
-        .gen_node_signing_key_pair()
-        .expect("Test setup failed: Failed to generate keys");
-    let key_id2 = KeyId::try_from(&public_key2).unwrap();
-    assert_ne!(
-        key_id1, key_id2,
-        "Test failure: Key IDs from different CSPs were the same.  Check random number generation."
-    );
-    assert!(
-        csp_vault2.sks_contains(&key_id2).expect("SKS call failed"),
-        "Key should be present in the CSP that generated it."
-    );
-    assert!(
-        !csp_vault2.sks_contains(&key_id1).expect("SKS call failed"),
-        "The second CSP should not contain the keys of the first."
-    );
-    assert!(
-        !csp_vault1.sks_contains(&key_id2).expect("SKS call failed"),
-        "Key first CSP should not contain the keys of the second."
-    );
-}
-
-pub fn sks_should_contain_tls_keys_only_after_generation(
-    csp_vault1: Arc<dyn CspVault>,
-    csp_vault2: Arc<dyn CspVault>,
-) {
-    let public_key_cert1 = csp_vault1
-        .gen_tls_key_pair(node_test_id(NODE_1), NOT_AFTER)
-        .expect("error generating TLS key pair");
-    let key_id1 = KeyId::try_from(&public_key_cert1).unwrap();
-    assert!(
-        csp_vault1.sks_contains(&key_id1).expect("SKS call failed"),
-        "TLS key should be present after generation."
-    );
-    assert!(
-        !csp_vault2.sks_contains(&key_id1).expect("SKS call failed"),
-        "TLS key should be absent if not generated in the CSP."
-    );
-
-    let public_key_cert2 = csp_vault2
-        .gen_tls_key_pair(node_test_id(NODE_1), NOT_AFTER)
-        .expect("error generating TLS key pair");
-    let key_id2 = KeyId::try_from(&public_key_cert2).unwrap();
-    assert_ne!(
-        key_id1, key_id2,
-        "Test failure: Key IDs from different CSPs were the same.  Check random number generation."
-    );
-    assert!(
-        csp_vault2.sks_contains(&key_id2).expect("SKS call failed"),
-        "TLS key should be present in the CSP that generated it."
-    );
-    assert!(
-        !csp_vault2.sks_contains(&key_id1).expect("SKS call failed"),
-        "The second CSP should not contain the TLS keys of the first."
-    );
-    assert!(
-        !csp_vault1.sks_contains(&key_id2).expect("SKS call failed"),
-        "Key first CSP should not contain the TLS keys of the second."
-    );
-}
 
 pub fn secret_key_store_with_duplicated_key_id_error_on_insert(
     duplicated_key_id: KeyId,

rs/crypto/internal/crypto_service_provider/tests/remote_csp_vault.rs

@@ -461,6 +461,36 @@ mod threshold_signature_csp_vault {
     }
 }
 
+mod secret_key_store_csp_vault {
+    use super::*;
+    use ic_crypto_internal_csp_proptest_utils::arb_csp_secret_key_store_contains_error;
+    use ic_crypto_internal_csp_proptest_utils::arb_key_id;
+
+    proptest! {
+        #![proptest_config(proptest_config_for_delegation())]
+        #[test]
+        fn should_delegate_for_sks_contains(
+            key_id in arb_key_id(),
+            expected_result in maybe_err(any::<bool>(), arb_csp_secret_key_store_contains_error())
+        ) {
+            let mut local_vault = MockLocalCspVault::new();
+            local_vault
+                .expect_sks_contains()
+                .times(1)
+                .withf(move |key_id_| {
+                     *key_id_ == key_id
+                })
+                .return_const(expected_result.clone());
+            let env = RemoteVaultEnvironment::start_server_with_local_csp_vault(Arc::new(local_vault));
+            let remote_vault = env.new_vault_client();
+
+            let result = remote_vault.sks_contains(&key_id);
+
+            prop_assert_eq!(result, expected_result);
+        }
+    }
+}
+
 fn local_vault_in_temp_dir() -> (
     LocalCspVault<OsRng, ProtoSecretKeyStore, ProtoSecretKeyStore, ProtoPublicKeyStore>,
     TempDir,