Improve mandatory Provide methods

This commit changes the way some methods on the Provide trait are
implemented. Given that in some cases the methods _must_ be implemented
by the providers (such as `list_keys`), there should be no default
implementation for them and the tests must verify that they operate
correctly.

Signed-off-by: Ionut Mihalcea <[email protected]>

Author: ionut-arm

e2e_tests/tests/all_providers/normal.rs

@@ -128,16 +128,19 @@ fn list_keys() {
 
     assert!(keys.is_empty());
 
-    let key1 = String::from("list_keys1");
-    let key2 = String::from("list_keys2");
-    let key3 = String::from("list_keys3");
-
-    client.set_provider(ProviderID::MbedCrypto);
-    client.generate_rsa_sign_key(key1.clone()).unwrap();
-    client.set_provider(ProviderID::Pkcs11);
-    client.generate_rsa_sign_key(key2.clone()).unwrap();
-    client.set_provider(ProviderID::Tpm);
-    client.generate_rsa_sign_key(key3.clone()).unwrap();
+    let providers = client.list_providers().expect("Failed to list providers");
+    let mut num_keys = providers.len();
+
+    for provider in providers.iter() {
+        client.set_provider(provider.id);
+        if !client.is_operation_supported(Opcode::PsaGenerateKey) {
+            num_keys -= 1;
+            continue;
+        }
+        client
+            .generate_rsa_sign_key(format!("list_keys_{}", provider.id))
+            .unwrap();
+    }
 
     let key_names: Vec<(String, ProviderID)> = client
         .list_keys()
@@ -146,10 +149,11 @@ fn list_keys() {
         .map(|k| (k.name, k.provider_id))
         .collect();
 
-    assert_eq!(key_names.len(), 3);
-    assert!(key_names.contains(&(key1.clone(), ProviderID::MbedCrypto)));
-    assert!(key_names.contains(&(key2.clone(), ProviderID::Pkcs11)));
-    assert!(key_names.contains(&(key3.clone(), ProviderID::Tpm)));
+    assert_eq!(key_names.len(), num_keys);
+
+    for provider in providers.iter() {
+        assert!(key_names.contains(&(format!("list_keys_{}", provider.id), provider.id)));
+    }
 }
 
 #[test]
@@ -187,27 +191,47 @@ fn invalid_provider_list_clients() {
 fn list_and_delete_clients() {
     let mut client = TestClient::new();
     client.do_not_destroy_keys();
-    client.set_default_auth(Some("list_clients test".to_string()));
+
+    let all_providers_user = "list_clients test".to_string();
+    client.set_default_auth(Some(all_providers_user.clone()));
 
     let clients = client.list_clients().expect("list_clients failed");
-    assert!(!clients.contains(&"list_clients test".to_string()));
+    assert!(!clients.contains(&all_providers_user));
+
+    let providers = client.list_providers().expect("Failed to list providers");
+    let mut num_users = providers.len() + 1;
 
-    let key1 = String::from("list_clients1");
-    let key2 = String::from("list_keys2");
-    let key3 = String::from("list_keys3");
+    for provider in providers.iter() {
+        client.set_provider(provider.id);
+        if !client.is_operation_supported(Opcode::PsaGenerateKey) {
+            num_users -= 1;
+            continue;
+        }
 
-    client.set_provider(ProviderID::MbedCrypto);
-    client.generate_rsa_sign_key(key1.clone()).unwrap();
-    client.set_provider(ProviderID::Pkcs11);
-    client.generate_rsa_sign_key(key2.clone()).unwrap();
-    client.set_provider(ProviderID::Tpm);
-    client.generate_rsa_sign_key(key3.clone()).unwrap();
+        client.set_default_auth(Some(all_providers_user.clone()));
+        client
+            .generate_rsa_sign_key("all-providers-user-key".to_string())
+            .unwrap();
+
+        client.set_default_auth(Some(format!("user_{}", provider.id)));
+        client
+            .generate_rsa_sign_key(format!("user_{}-key", provider.id))
+            .unwrap();
+    }
+
+    client.set_default_auth(Some(all_providers_user.clone()));
 
     let clients = client.list_clients().expect("list_clients failed");
-    assert!(clients.contains(&"list_clients test".to_string()));
-    client
-        .delete_client("list_clients test".to_string())
-        .unwrap();
+    assert_eq!(clients.len(), num_users);
+
+    assert!(clients.contains(&all_providers_user));
+    client.delete_client(all_providers_user).unwrap();
+
+    for provider in providers.iter() {
+        let username = format!("user_{}", provider.id);
+        assert!(clients.contains(&username));
+        client.delete_client(username).unwrap();
+    }
 
     let keys = client.list_keys().expect("list_keys failed");
 

e2e_tests/tests/per_provider/normal_tests/hash.rs

@@ -27,16 +27,14 @@ fn hash_not_supported() {
     let mut client = TestClient::new();
     if !client.is_operation_supported(Opcode::PsaHashCompute) {
         assert_eq!(
-            client.hash_compute(Hash::Sha256, &vec![],).unwrap_err(),
+            client.hash_compute(Hash::Sha256, &[],).unwrap_err(),
             ResponseStatus::PsaErrorNotSupported
         );
     }
 
     if !client.is_operation_supported(Opcode::PsaHashCompare) {
         assert_eq!(
-            client
-                .hash_compare(Hash::Sha256, &vec![], &vec![])
-                .unwrap_err(),
+            client.hash_compare(Hash::Sha256, &[], &[]).unwrap_err(),
             ResponseStatus::PsaErrorNotSupported
         );
     }

e2e_tests/tests/per_provider/normal_tests/key_agreement.rs

@@ -70,11 +70,7 @@ fn simple_raw_key_agreement() {
         .generate_ecc_pair_secp_r1_key(key_name.clone())
         .unwrap();
     let _shared_secret = client
-        .raw_key_agreement(
-            RawKeyAgreement::Ecdh,
-            key_name.clone(),
-            &PEER_PUBLIC_KEY_SECPR1,
-        )
+        .raw_key_agreement(RawKeyAgreement::Ecdh, key_name, &PEER_PUBLIC_KEY_SECPR1)
         .unwrap();
 }
 
@@ -91,11 +87,7 @@ fn raw_key_agreement_secpr1() {
         .import_ecc_pair_secp_r1_key(key_name.clone(), OUR_KEY_DATA_SECPR1.to_vec())
         .unwrap();
     let shared_secret = client
-        .raw_key_agreement(
-            RawKeyAgreement::Ecdh,
-            key_name.clone(),
-            &PEER_PUBLIC_KEY_SECPR1,
-        )
+        .raw_key_agreement(RawKeyAgreement::Ecdh, key_name, &PEER_PUBLIC_KEY_SECPR1)
         .unwrap();
 
     assert_eq!(&EXPECTED_OUTPUT_SECPR1, shared_secret.as_slice());

src/providers/core/mod.rs

@@ -142,6 +142,10 @@ impl Provide for Provider {
 
         Ok(result)
     }
+
+    fn describe(&self) -> Result<(ProviderInfo, HashSet<Opcode>)> {
+        unreachable!()
+    }
 }
 
 /// Builder for CoreProvider

src/providers/mod.rs

@@ -116,14 +116,14 @@ use parsec_interface::requests::{ResponseStatus, Result};
 ///
 /// Definition of the interface that a provider must implement to
 /// be linked into the service through a backend handler.
+///
+/// The methods with no default are used on a service-level by the
+/// core provider and so must be supported by all providers.
 pub trait Provide {
     /// Return a description of the current provider.
     ///
     /// The descriptions are gathered in the Core Provider and returned for a ListProviders operation.
-    fn describe(&self) -> Result<(list_providers::ProviderInfo, HashSet<Opcode>)> {
-        trace!("describe ingress");
-        Err(ResponseStatus::PsaErrorNotSupported)
-    }
+    fn describe(&self) -> Result<(list_providers::ProviderInfo, HashSet<Opcode>)>;
 
     /// List the providers running in the service.
     fn list_providers(&self, _op: list_providers::Operation) -> Result<list_providers::Result> {
@@ -151,16 +151,10 @@ pub trait Provide {
         &self,
         _app_name: ApplicationName,
         _op: list_keys::Operation,
-    ) -> Result<list_keys::Result> {
-        trace!("list_keys ingress");
-        Err(ResponseStatus::PsaErrorNotSupported)
-    }
+    ) -> Result<list_keys::Result>;
 
     /// Lists all clients currently having data in the service.
-    fn list_clients(&self, _op: list_clients::Operation) -> Result<list_clients::Result> {
-        trace!("list_clients ingress");
-        Err(ResponseStatus::PsaErrorNotSupported)
-    }
+    fn list_clients(&self, _op: list_clients::Operation) -> Result<list_clients::Result>;
 
     /// Delete all data a client has in the service..
     fn delete_client(&self, _op: delete_client::Operation) -> Result<delete_client::Result> {