Merge pull request #173 from ionut-arm/list-opcodes

Improve handling of list_opcodes

Author: ionut-arm

Cargo.lock

@@ -18,7 +18,7 @@ name = "parsec"
 path = "src/bin/main.rs"
 
 [dependencies]
-parsec-interface = "0.14.1"
+parsec-interface = "0.14.2"
 rand = "0.7.2"
 base64 = "0.10.1"
 uuid = "0.7.4"

Cargo.toml

@@ -18,7 +18,7 @@ picky-asn1-der = "0.2.2"
 picky-asn1 = "0.2.1"
 serde = { version = "1.0", features = ["derive"] }
 sha2 = "0.8.1"
-parsec-client = { version = "0.2.0", features = ["testing"] }
+parsec-client = { version = "0.3.0", features = ["testing"] }
 log = "0.4.8"
 rand = "0.7.3"
 

e2e_tests/Cargo.toml

@@ -22,9 +22,6 @@ fn list_providers() {
 }
 
 #[test]
-// ListOpcodes is not currently supported only by the Core Provider, see
-// parallaxsecond/parsec-operations#9
-#[ignore]
 fn list_opcodes() {
     let mut client = TestClient::new();
     let mut crypto_providers_opcodes = HashSet::new();

e2e_tests/tests/all_providers/mod.rs

@@ -10,6 +10,7 @@ use log::error;
 use parsec_interface::operations::list_providers::ProviderInfo;
 use parsec_interface::operations::{list_opcodes, list_providers, ping};
 use parsec_interface::requests::{Opcode, ProviderID, ResponseStatus, Result};
+use std::collections::{HashMap, HashSet};
 use std::io::{Error, ErrorKind};
 use std::str::FromStr;
 use uuid::Uuid;
@@ -26,37 +27,24 @@ const SUPPORTED_OPCODES: [Opcode; 3] = [Opcode::ListProviders, Opcode::ListOpcod
 pub struct CoreProvider {
     wire_protocol_version_min: u8,
     wire_protocol_version_maj: u8,
-    providers: Vec<ProviderInfo>,
+    provider_info: Vec<ProviderInfo>,
+    provider_opcodes: HashMap<ProviderID, HashSet<Opcode>>,
 }
 
 impl Provide for CoreProvider {
-    fn list_opcodes(&self, _op: list_opcodes::Operation) -> Result<list_opcodes::Result> {
+    fn list_opcodes(&self, op: list_opcodes::Operation) -> Result<list_opcodes::Result> {
         Ok(list_opcodes::Result {
-            opcodes: SUPPORTED_OPCODES.iter().copied().collect(),
+            opcodes: self
+                .provider_opcodes
+                .get(&op.provider_id)
+                .ok_or(ResponseStatus::ProviderNotRegistered)?
+                .clone(),
         })
     }
 
     fn list_providers(&self, _op: list_providers::Operation) -> Result<list_providers::Result> {
         Ok(list_providers::Result {
-            providers: self.providers.clone(),
-        })
-    }
-
-    fn describe(&self) -> Result<ProviderInfo> {
-        let crate_version: Version = Version::from_str(version!()).or_else(|e| {
-            error!("Error parsing the crate version: {}.", e);
-            Err(ResponseStatus::InvalidEncoding)
-        })?;
-
-        Ok(ProviderInfo {
-            // Assigned UUID for this provider: 47049873-2a43-4845-9d72-831eab668784
-            uuid: Uuid::parse_str("47049873-2a43-4845-9d72-831eab668784").or(Err(ResponseStatus::InvalidEncoding))?,
-            description: String::from("Software provider that implements only administrative (i.e. no cryptographic) operations"),
-            vendor: String::new(),
-            version_maj: crate_version.major,
-            version_min: crate_version.minor,
-            version_rev: crate_version.patch,
-            id: ProviderID::Core,
+            providers: self.provider_info.clone(),
         })
     }
 
@@ -75,16 +63,46 @@ impl Provide for CoreProvider {
 pub struct CoreProviderBuilder {
     version_maj: Option<u8>,
     version_min: Option<u8>,
-    providers: Option<Vec<ProviderInfo>>,
+    provider_info: Vec<ProviderInfo>,
+    provider_opcodes: HashMap<ProviderID, HashSet<Opcode>>,
 }
 
 impl CoreProviderBuilder {
-    pub fn new() -> Self {
-        CoreProviderBuilder {
+    pub fn new() -> std::io::Result<Self> {
+        let crate_version: Version = Version::from_str(version!()).or_else(|e| {
+            error!("Error parsing the crate version: {}.", e);
+            Err(Error::new(
+                ErrorKind::InvalidData,
+                "crate version number has invalid format",
+            ))
+        })?;
+
+        let provider_info = vec![ProviderInfo {
+            // Assigned UUID for this provider: 47049873-2a43-4845-9d72-831eab668784
+            uuid: Uuid::parse_str("47049873-2a43-4845-9d72-831eab668784").or_else(|_| Err(Error::new(
+                ErrorKind::InvalidData,
+                "provider UUID is invalid",
+            )))?,
+            description: String::from("Software provider that implements only administrative (i.e. no cryptographic) operations"),
+            vendor: String::new(),
+            version_maj: crate_version.major,
+            version_min: crate_version.minor,
+            version_rev: crate_version.patch,
+            id: ProviderID::Core,
+        }];
+
+        let mut provider_opcodes = HashMap::new();
+        let _ = provider_opcodes.insert(
+            ProviderID::Core,
+            SUPPORTED_OPCODES.iter().copied().collect(),
+        );
+
+        Ok(CoreProviderBuilder {
             version_maj: None,
             version_min: None,
-            providers: None,
-        }
+            provider_info,
+            provider_opcodes,
+        })
     }
 
     pub fn with_wire_protocol_version(mut self, version_min: u8, version_maj: u8) -> Self {
@@ -94,44 +112,29 @@ impl CoreProviderBuilder {
         self
     }
 
-    pub fn with_provider_info(mut self, provider_info: ProviderInfo) -> Self {
-        match self.providers {
-            Some(mut providers) => {
-                providers.push(provider_info);
-                self.providers = Some(providers);
-            }
-            None => {
-                let mut providers = Vec::new();
-                providers.push(provider_info);
-                self.providers = Some(providers);
-            }
-        }
+    pub fn with_provider_details(
+        mut self,
+        provider_info: ProviderInfo,
+        opcodes: HashSet<Opcode>,
+    ) -> Self {
+        let _ = self.provider_opcodes.insert(provider_info.id, opcodes);
+        self.provider_info.push(provider_info);
 
         self
     }
 
     pub fn build(self) -> std::io::Result<CoreProvider> {
-        let mut core_provider = CoreProvider {
+        let core_provider = CoreProvider {
             wire_protocol_version_maj: self
                 .version_maj
                 .ok_or_else(|| Error::new(ErrorKind::InvalidData, "version maj is missing"))?,
             wire_protocol_version_min: self
                 .version_min
                 .ok_or_else(|| Error::new(ErrorKind::InvalidData, "version min is missing"))?,
-            providers: self
-                .providers
-                .ok_or_else(|| Error::new(ErrorKind::InvalidData, "provider info is missing"))?,
+            provider_opcodes: self.provider_opcodes,
+            provider_info: self.provider_info,
         };
 
-        core_provider
-            .providers
-            .push(core_provider.describe().or_else(|_| {
-                Err(Error::new(
-                    ErrorKind::InvalidData,
-                    "error describing Core provider",
-                ))
-            })?);
-
         Ok(core_provider)
     }
 }
@@ -145,7 +148,8 @@ mod tests {
         let provider = CoreProvider {
             wire_protocol_version_min: 8,
             wire_protocol_version_maj: 10,
-            providers: Vec::new(),
+            provider_info: Vec::new(),
+            provider_opcodes: HashMap::new(),
         };
         let op = ping::Operation {};
         let result = provider.ping(op).unwrap();

src/providers/core_provider/mod.rs

@@ -8,8 +8,8 @@ use derivative::Derivative;
 use log::error;
 use parsec_interface::operations::list_providers::ProviderInfo;
 use parsec_interface::operations::{
-    list_opcodes, psa_destroy_key, psa_export_public_key, psa_generate_key, psa_import_key,
-    psa_sign_hash, psa_verify_hash,
+    psa_destroy_key, psa_export_public_key, psa_generate_key, psa_import_key, psa_sign_hash,
+    psa_verify_hash,
 };
 use parsec_interface::requests::{Opcode, ProviderID, ResponseStatus, Result};
 use psa_crypto_binding::psa_key_id_t;
@@ -40,14 +40,13 @@ mod utils;
 
 type LocalIdStore = HashSet<psa_key_id_t>;
 
-const SUPPORTED_OPCODES: [Opcode; 7] = [
+const SUPPORTED_OPCODES: [Opcode; 6] = [
     Opcode::PsaGenerateKey,
     Opcode::PsaDestroyKey,
     Opcode::PsaSignHash,
     Opcode::PsaVerifyHash,
     Opcode::PsaImportKey,
     Opcode::PsaExportPublicKey,
-    Opcode::ListOpcodes,
 ];
 
 #[derive(Derivative)]
@@ -153,14 +152,8 @@ impl MbedProvider {
 }
 
 impl Provide for MbedProvider {
-    fn list_opcodes(&self, _op: list_opcodes::Operation) -> Result<list_opcodes::Result> {
-        Ok(list_opcodes::Result {
-            opcodes: SUPPORTED_OPCODES.iter().copied().collect(),
-        })
-    }
-
-    fn describe(&self) -> Result<ProviderInfo> {
-        Ok(ProviderInfo {
+    fn describe(&self) -> Result<(ProviderInfo, HashSet<Opcode>)> {
+        Ok((ProviderInfo {
             // Assigned UUID for this provider: 1c1139dc-ad7c-47dc-ad6b-db6fdb466552
             uuid: Uuid::parse_str("1c1139dc-ad7c-47dc-ad6b-db6fdb466552").or(Err(ResponseStatus::InvalidEncoding))?,
             description: String::from("User space software provider, based on Mbed Crypto - the reference implementation of the PSA crypto API"),
@@ -169,7 +162,7 @@ impl Provide for MbedProvider {
             version_min: 1,
             version_rev: 0,
             id: ProviderID::MbedCrypto,
-        })
+        }, SUPPORTED_OPCODES.iter().copied().collect()))
     }
 
     fn psa_generate_key(

src/providers/mbed_provider/mod.rs

@@ -6,8 +6,9 @@
 //! are the real implementors of the operations that Parsec claims to support. They map to
 //! functionality in the underlying hardware which allows the PSA Crypto operations to be
 //! backed by a hardware root of trust.
-use parsec_interface::requests::ProviderID;
+use parsec_interface::requests::{Opcode, ProviderID};
 use serde::Deserialize;
+use std::collections::HashSet;
 
 pub mod core_provider;
 
@@ -86,15 +87,19 @@ 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>;
+    fn describe(&self) -> Result<(list_providers::ProviderInfo, HashSet<Opcode>)> {
+        Err(ResponseStatus::PsaErrorNotSupported)
+    }
 
     /// List the providers running in the service.
     fn list_providers(&self, _op: list_providers::Operation) -> Result<list_providers::Result> {
         Err(ResponseStatus::PsaErrorNotSupported)
     }
 
-    /// List the opcodes supported by the current provider.
-    fn list_opcodes(&self, _op: list_opcodes::Operation) -> Result<list_opcodes::Result>;
+    /// List the opcodes supported by the given provider.
+    fn list_opcodes(&self, _op: list_opcodes::Operation) -> Result<list_opcodes::Result> {
+        Err(ResponseStatus::PsaErrorNotSupported)
+    }
 
     /// Execute a Ping operation to get the wire protocol version major and minor information.
     ///

src/providers/mod.rs

@@ -11,8 +11,8 @@ use derivative::Derivative;
 use log::{error, info, warn};
 use parsec_interface::operations::list_providers::ProviderInfo;
 use parsec_interface::operations::{
-    list_opcodes, psa_destroy_key, psa_export_public_key, psa_generate_key, psa_import_key,
-    psa_sign_hash, psa_verify_hash,
+    psa_destroy_key, psa_export_public_key, psa_generate_key, psa_import_key, psa_sign_hash,
+    psa_verify_hash,
 };
 use parsec_interface::requests::{Opcode, ProviderID, ResponseStatus, Result};
 use pkcs11::types::{CKF_OS_LOCKING_OK, CK_C_INITIALIZE_ARGS, CK_SLOT_ID};
@@ -29,14 +29,13 @@ mod asym_sign;
 mod key_management;
 mod utils;
 
-const SUPPORTED_OPCODES: [Opcode; 7] = [
+const SUPPORTED_OPCODES: [Opcode; 6] = [
     Opcode::PsaGenerateKey,
     Opcode::PsaDestroyKey,
     Opcode::PsaSignHash,
     Opcode::PsaVerifyHash,
     Opcode::PsaImportKey,
     Opcode::PsaExportPublicKey,
-    Opcode::ListOpcodes,
 ];
 
 /// Provider for Public Key Cryptography Standard #11
@@ -162,24 +161,23 @@ impl Pkcs11Provider {
 }
 
 impl Provide for Pkcs11Provider {
-    fn list_opcodes(&self, _op: list_opcodes::Operation) -> Result<list_opcodes::Result> {
-        Ok(list_opcodes::Result {
-            opcodes: SUPPORTED_OPCODES.iter().copied().collect(),
-        })
-    }
-
-    fn describe(&self) -> Result<ProviderInfo> {
-        Ok(ProviderInfo {
-            // Assigned UUID for this provider: 30e39502-eba6-4d60-a4af-c518b7f5e38f
-            uuid: Uuid::parse_str("30e39502-eba6-4d60-a4af-c518b7f5e38f")
-                .or(Err(ResponseStatus::InvalidEncoding))?,
-            description: String::from("PKCS #11 provider, interfacing with a PKCS #11 library."),
-            vendor: String::from("OASIS Standard."),
-            version_maj: 0,
-            version_min: 1,
-            version_rev: 0,
-            id: ProviderID::Pkcs11,
-        })
+    fn describe(&self) -> Result<(ProviderInfo, HashSet<Opcode>)> {
+        Ok((
+            ProviderInfo {
+                // Assigned UUID for this provider: 30e39502-eba6-4d60-a4af-c518b7f5e38f
+                uuid: Uuid::parse_str("30e39502-eba6-4d60-a4af-c518b7f5e38f")
+                    .or(Err(ResponseStatus::InvalidEncoding))?,
+                description: String::from(
+                    "PKCS #11 provider, interfacing with a PKCS #11 library.",
+                ),
+                vendor: String::from("OASIS Standard."),
+                version_maj: 0,
+                version_min: 1,
+                version_rev: 0,
+                id: ProviderID::Pkcs11,
+            },
+            SUPPORTED_OPCODES.iter().copied().collect(),
+        ))
     }
 
     fn psa_generate_key(