Add ListClients and DeleteClient operations

Also adds multitenancy tests for those operations and for the admin
feature. Splits the ApplicationName structure into Application and
ApplicationName to only deal with key names in the Key Info Managers and
not the admin status which is not relevent there.

Signed-off-by: Hugues de Valon <[email protected]>

Author: hug-dev

Cargo.lock

@@ -18,7 +18,7 @@ name = "parsec"
 path = "src/bin/main.rs"
 
 [dependencies]
-parsec-interface = "0.22.0"
+parsec-interface = { git = "https://github.com/parallaxsecond/parsec-interface-rs", rev = "c8a59544fac04df347f51d19323f4a0b5bb9580d" }
 rand = { version = "0.7.3", features = ["small_rng"], optional = true }
 base64 = "0.12.3"
 uuid = "0.8.1"

Cargo.toml

@@ -14,6 +14,7 @@ socket_path = "/tmp/parsec.sock"
 
 [authenticator]
 auth_type = "Direct"
+admins = [ { name = "list_clients test" }, { name = "parsec-client-1" }, { name = "client1" }, { name = "spiffe://example.org/parsec-client-1" } ]
 #workload_endpoint="unix:///tmp/agent.sock"
 
 [[key_manager]]

e2e_tests/provider_cfg/all/config.toml

@@ -53,7 +53,10 @@ impl TestClient {
     /// Creates a TestClient instance.
     pub fn new() -> TestClient {
         // As this method is called in test, it will be called more than once per application.
-        if let Err(_) = env_logger::try_init() {};
+        #[allow(unused_must_use)]
+        {
+            env_logger::try_init();
+        }
 
         let mut basic_client = BasicClient::new_naked();
 
@@ -892,6 +895,18 @@ impl TestClient {
         self.basic_client.list_keys().map_err(convert_error)
     }
 
+    /// Lists the clients.
+    pub fn list_clients(&mut self) -> Result<Vec<String>> {
+        self.basic_client.list_clients().map_err(convert_error)
+    }
+
+    /// Delete a client.
+    pub fn delete_client(&mut self, client: String) -> Result<()> {
+        self.basic_client
+            .delete_client(client)
+            .map_err(convert_error)
+    }
+
     /// Executes a ping operation.
     pub fn ping(&mut self) -> Result<(u8, u8)> {
         self.basic_client.ping().map_err(convert_error)

e2e_tests/src/lib.rs

@@ -330,7 +330,7 @@ impl ServiceChecker {
             .expect("Verification failed");
 
         client
-            .destroy_key(sign_key_name.clone())
+            .destroy_key(sign_key_name)
             .expect("Failed to destroy key");
     }
 
@@ -352,7 +352,7 @@ impl ServiceChecker {
         assert_eq!(plaintext, vec![0xa5; 16]);
 
         client
-            .destroy_key(encr_key_name.clone())
+            .destroy_key(encr_key_name)
             .expect("Failed to destroy key");
     }
 }

e2e_tests/src/stress.rs

@@ -9,6 +9,8 @@ use parsec_client::core::interface::requests::{ProviderID, ResponseStatus};
 // 3. client1_after is executed as parsec-client-1
 //
 // They are executed against all possible authenticators in Parsec.
+//
+// client1 will be configured as an admin.
 
 #[test]
 fn client1_before() {
@@ -23,11 +25,15 @@ fn client1_before() {
         client.set_provider(*provider);
         client.generate_rsa_sign_key(key.clone()).unwrap();
     }
+
+    let clients = client.list_clients().unwrap();
+    assert_eq!(clients.len(), 1);
 }
 
 #[test]
 fn client2() {
     let mut client = TestClient::new();
+    client.do_not_destroy_keys();
     client.set_default_auth(Some("client2".to_string()));
 
     let key = String::from("multitenant");
@@ -49,11 +55,24 @@ fn client2() {
         client.generate_rsa_sign_key(key.clone()).unwrap();
         client.destroy_key(key.clone()).unwrap();
     }
+
+    assert_eq!(
+        client.list_clients().unwrap_err(),
+        ResponseStatus::AdminOperation
+    );
+    assert_eq!(
+        client.delete_client("toto".to_string()).unwrap_err(),
+        ResponseStatus::AdminOperation
+    );
+    client
+        .generate_rsa_sign_key("client2-key".to_string())
+        .unwrap();
 }
 
 #[test]
 fn client1_after() {
     let mut client = TestClient::new();
+    client.do_not_destroy_keys();
     client.set_default_auth(Some("client1".to_string()));
 
     // Verify all keys are still there and can be used
@@ -66,4 +85,18 @@ fn client1_after() {
         client.set_provider(*provider);
         client.destroy_key(key.clone()).unwrap();
     }
+
+    client
+        .generate_rsa_sign_key("client1-key".to_string())
+        .unwrap();
+    let mut clients = client.list_clients().unwrap();
+    assert_eq!(clients.len(), 2);
+    client.delete_client(clients.remove(0)).unwrap();
+    let mut clients = client.list_clients().unwrap();
+    assert_eq!(clients.len(), 1);
+    client.delete_client(clients.remove(0)).unwrap();
+    let clients = client.list_clients().unwrap();
+    assert_eq!(clients.len(), 0);
+    let keys = client.list_keys().unwrap();
+    assert_eq!(keys.len(), 0);
 }

e2e_tests/tests/all_providers/multitenancy.rs

@@ -167,3 +167,49 @@ fn invalid_provider_list_keys() {
         .expect("Failed to read Response");
     assert_eq!(resp.header.status, ResponseStatus::PsaErrorNotSupported);
 }
+
+#[test]
+fn invalid_provider_list_clients() {
+    let mut client = RawRequestClient {};
+    let mut req_hdr = RawHeader::new();
+
+    // Always targeting the Mbed Crypto provider
+    req_hdr.provider = 0x1;
+    req_hdr.opcode = Opcode::ListClients as u32;
+
+    let resp = client
+        .send_raw_request(req_hdr, Vec::new())
+        .expect("Failed to read Response");
+    assert_eq!(resp.header.status, ResponseStatus::PsaErrorNotSupported);
+}
+
+#[test]
+fn list_clients() {
+    let mut client = TestClient::new();
+    client.do_not_destroy_keys();
+    client.set_default_auth(Some("list_clients test".to_string()));
+
+    let clients = client.list_clients().expect("list_clients failed");
+    assert!(!clients.contains(&"list_clients test".to_string()));
+
+    let key1 = String::from("list_clients1");
+    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 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();
+
+    let keys = client.list_keys().expect("list_keys failed");
+
+    assert!(keys.is_empty());
+}

e2e_tests/tests/all_providers/normal.rs

@@ -8,7 +8,7 @@
 //! This authenticator does not offer any security value and should only be used in environments
 //! where all the clients and the service are mutually trustworthy.
 
-use super::{Admin, AdminList, ApplicationName, Authenticate};
+use super::{Admin, AdminList, Application, Authenticate};
 use crate::front::listener::ConnectionMetadata;
 use log::error;
 use parsec_interface::operations::list_authenticators;
@@ -51,7 +51,7 @@ impl Authenticate for DirectAuthenticator {
         &self,
         auth: &RequestAuth,
         _: Option<ConnectionMetadata>,
-    ) -> Result<ApplicationName> {
+    ) -> Result<Application> {
         if auth.buffer.expose_secret().is_empty() {
             error!("The direct authenticator does not expect empty authentication values.");
             Err(ResponseStatus::AuthenticationError)
@@ -60,7 +60,7 @@ impl Authenticate for DirectAuthenticator {
                 Ok(str) => {
                     let app_name = String::from(str);
                     let is_admin = self.admins.is_admin(&app_name);
-                    Ok(ApplicationName::new(app_name, is_admin))
+                    Ok(Application::new(app_name, is_admin))
                 }
                 Err(_) => {
                     error!("Error parsing the authentication value as a UTF-8 string.");
@@ -88,12 +88,12 @@ mod test {
         let req_auth = RequestAuth::new(app_name.clone().into_bytes());
         let conn_metadata = None;
 
-        let auth_name = authenticator
+        let app = authenticator
             .authenticate(&req_auth, conn_metadata)
             .expect("Failed to authenticate");
 
-        assert_eq!(auth_name.get_name(), app_name);
-        assert_eq!(auth_name.is_admin, false);
+        assert_eq!(app.get_name(), app_name);
+        assert_eq!(app.is_admin, false);
     }
 
     #[test]

src/authenticators/direct_authenticator/mod.rs

@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 //! JWT SVID authenticator
 
-use super::{Admin, AdminList, ApplicationName, Authenticate};
+use super::{Admin, AdminList, Application, Authenticate};
 use crate::front::listener::ConnectionMetadata;
 use log::error;
 use parsec_interface::operations::list_authenticators;
@@ -48,7 +48,7 @@ impl Authenticate for JwtSvidAuthenticator {
         &self,
         auth: &RequestAuth,
         _: Option<ConnectionMetadata>,
-    ) -> Result<ApplicationName> {
+    ) -> Result<Application> {
         let svid = Jwt::new(
             str::from_utf8(auth.buffer.expose_secret())
                 .map_err(|e| {
@@ -68,6 +68,6 @@ impl Authenticate for JwtSvidAuthenticator {
         })?;
         let app_name = validate_response.spiffe_id().to_string();
         let is_admin = self.admins.is_admin(&app_name);
-        Ok(ApplicationName::new(app_name, is_admin))
+        Ok(Application::new(app_name, is_admin))
     }
 }

src/authenticators/jwt_svid_authenticator/mod.rs

@@ -27,6 +27,12 @@ use zeroize::Zeroize;
 #[derive(Debug, Clone, Eq, PartialEq, Hash)]
 pub struct ApplicationName {
     name: String,
+}
+
+/// Wrapper for a Parsec application
+#[derive(Debug, Clone)]
+pub struct Application {
+    name: ApplicationName,
     is_admin: bool,
 }
 
@@ -40,7 +46,7 @@ pub trait Authenticate {
     /// operation.
     fn describe(&self) -> Result<list_authenticators::AuthenticatorInfo>;
 
-    /// Authenticates a `RequestAuth` payload and returns the `ApplicationName` if successful. A
+    /// Authenticates a `RequestAuth` payload and returns the `Application` if successful. A
     /// optional `ConnectionMetadata` object is passed in too, since it is sometimes possible to
     /// perform authentication based on the connection's metadata (i.e. as is the case for UNIX
     /// domain sockets with Unix peer credentials).
@@ -52,32 +58,45 @@ pub trait Authenticate {
         &self,
         auth: &RequestAuth,
         meta: Option<ConnectionMetadata>,
-    ) -> Result<ApplicationName>;
+    ) -> Result<Application>;
 }
 
 impl ApplicationName {
-    /// Create a new ApplicationName
-    fn new(name: String, is_admin: bool) -> ApplicationName {
-        ApplicationName { name, is_admin }
-    }
-
     /// Create ApplicationName from name string only
     pub fn from_name(name: String) -> ApplicationName {
-        ApplicationName {
-            name,
-            is_admin: false,
-        }
+        ApplicationName { name }
     }
 
     /// Get a reference to the inner string
     pub fn get_name(&self) -> &str {
         &self.name
     }
+}
+
+impl Application {
+    /// Create a new Application structure
+    pub fn new(name: String, is_admin: bool) -> Application {
+        Application {
+            name: ApplicationName::from_name(name),
+            is_admin,
+        }
+    }
 
     /// Check whether the application is an admin
     pub fn is_admin(&self) -> bool {
         self.is_admin
     }
+
+    /// Get a reference to the inner ApplicationName string
+    pub fn get_name(&self) -> &str {
+        &self.name.name
+    }
+}
+
+impl From<Application> for ApplicationName {
+    fn from(auth: Application) -> Self {
+        auth.name
+    }
 }
 
 impl std::fmt::Display for ApplicationName {