Review ResponseStatus usage

Cargo.toml

@@ -17,7 +17,7 @@ name = "parsec"
 path = "src/bin/main.rs"
 
 [dependencies]
-parsec-interface = { git = "https://github.com/parallaxsecond/parsec-interface-rs", tag = "0.6.1" }
+parsec-interface = "0.7.1"
 rand = "0.7.2"
 base64 = "0.10.1"
 uuid = "0.7.4"
@@ -39,7 +39,7 @@ derivative = "1.0.3"
 arbitrary = { version = "0.4.0", features = ["derive"], optional = true }
 
 [dev-dependencies]
-parsec-client-test = { git = "https://github.com/parallaxsecond/parsec-client-test", tag = "0.1.14" }
+parsec-client-test = { git = "https://github.com/parallaxsecond/parsec-client-test", tag = "0.1.15" }
 num_cpus = "1.10.1"
 
 [build-dependencies]

src/key_id_managers/mod.rs

@@ -19,7 +19,8 @@
 //! store this mapping using different means but it has to be persistent.
 
 use crate::authenticators::ApplicationName;
-use parsec_interface::requests::ProviderID;
+use log::error;
+use parsec_interface::requests::{ProviderID, ResponseStatus};
 use serde::Deserialize;
 use std::fmt;
 
@@ -72,6 +73,16 @@ impl KeyTriple {
     }
 }
 
+/// Converts the error string returned by the ManageKeyIDs methods to
+/// ResponseStatus::KeyIDManagerError.
+pub fn to_response_status(error_string: String) -> ResponseStatus {
+    error!(
+        "Converting error string \"{}\" to ResponseStatus:KeyIDManagerError.",
+        error_string
+    );
+    ResponseStatus::KeyIDManagerError
+}
+
 pub trait ManageKeyIDs {
     /// Returns a reference to the key ID corresponding to this key triple or `None` if it does not
     /// exist.

src/providers/mbed_provider/mod.rs

@@ -14,6 +14,7 @@
 // limitations under the License.
 use super::Provide;
 use crate::authenticators::ApplicationName;
+use crate::key_id_managers;
 use crate::key_id_managers::{KeyTriple, ManageKeyIDs};
 use constants::PSA_SUCCESS;
 use derivative::Derivative;
@@ -100,11 +101,8 @@ fn get_key_id(key_triple: &KeyTriple, store_handle: &dyn ManageKeyIDs) -> Result
                 Err(ResponseStatus::KeyIDManagerError)
             }
         }
-        Ok(None) => Err(ResponseStatus::KeyDoesNotExist),
-        Err(string) => {
-            error!("Key ID Manager error: {}", string);
-            Err(ResponseStatus::KeyIDManagerError)
-        }
+        Ok(None) => Err(ResponseStatus::PsaErrorDoesNotExist),
+        Err(string) => Err(key_id_managers::to_response_status(string)),
     }
 }
 
@@ -130,10 +128,7 @@ fn create_key_id(
 
             Ok(key_id)
         }
-        Err(string) => {
-            error!("Key ID Manager error: {}", string);
-            Err(ResponseStatus::KeyIDManagerError)
-        }
+        Err(string) => Err(key_id_managers::to_response_status(string)),
     }
 }
 
@@ -148,21 +143,14 @@ fn remove_key_id(
             let _ = local_ids_handle.remove(&key_id);
             Ok(())
         }
-        Err(string) => {
-            error!("Key ID Manager error: {}", string);
-            Err(ResponseStatus::KeyIDManagerError)
-        }
+        Err(string) => Err(key_id_managers::to_response_status(string)),
     }
 }
 
 fn key_id_exists(key_triple: &KeyTriple, store_handle: &dyn ManageKeyIDs) -> Result<bool> {
-    match store_handle.exists(key_triple) {
-        Ok(val) => Ok(val),
-        Err(string) => {
-            error!("Key ID Manager error: {}", string);
-            Err(ResponseStatus::KeyIDManagerError)
-        }
-    }
+    store_handle
+        .exists(key_triple)
+        .or_else(|e| Err(key_id_managers::to_response_status(e)))
 }
 
 impl MbedProvider {
@@ -273,7 +261,7 @@ impl Provide for MbedProvider {
         let mut store_handle = self.key_id_store.write().expect("Key store lock poisoned");
         let mut local_ids_handle = self.local_ids.write().expect("Local ID lock poisoned");
         if key_id_exists(&key_triple, &*store_handle)? {
-            return Err(ResponseStatus::KeyAlreadyExists);
+            return Err(ResponseStatus::PsaErrorAlreadyExists);
         }
         let key_id = create_key_id(
             key_triple.clone(),
@@ -321,7 +309,7 @@ impl Provide for MbedProvider {
         let mut store_handle = self.key_id_store.write().expect("Key store lock poisoned");
         let mut local_ids_handle = self.local_ids.write().expect("Local ID lock poisoned");
         if key_id_exists(&key_triple, &*store_handle)? {
-            return Err(ResponseStatus::KeyAlreadyExists);
+            return Err(ResponseStatus::PsaErrorAlreadyExists);
         }
         let key_id = create_key_id(
             key_triple.clone(),
@@ -407,10 +395,7 @@ impl Provide for MbedProvider {
         if export_status != PSA_SUCCESS {
             error!("Export status: {}", export_status);
             // Safety: same conditions than above.
-            return Err(utils::convert_status(export_status).ok_or_else(|| {
-                error!("Failed to convert error status.");
-                ResponseStatus::PsaErrorGenericError
-            })?);
+            return Err(utils::convert_status(export_status));
         }
 
         buffer.resize(actual_size, 0);
@@ -453,10 +438,7 @@ impl Provide for MbedProvider {
             Ok(ResultDestroyKey {})
         } else {
             error!("Destroy key status: {}", destroy_key_status);
-            Err(utils::convert_status(destroy_key_status).ok_or_else(|| {
-                error!("Failed to convert error status.");
-                ResponseStatus::PsaErrorGenericError
-            })?)
+            Err(utils::convert_status(destroy_key_status))
         }
     }
 
@@ -515,10 +497,7 @@ impl Provide for MbedProvider {
             Ok(res)
         } else {
             error!("Sign status: {}", sign_status);
-            Err(utils::convert_status(sign_status).ok_or_else(|| {
-                error!("Failed to convert error status.");
-                ResponseStatus::PsaErrorGenericError
-            })?)
+            Err(utils::convert_status(sign_status))
         }
     }
 
@@ -562,10 +541,7 @@ impl Provide for MbedProvider {
         if verify_status == PSA_SUCCESS {
             Ok(ResultAsymVerify {})
         } else {
-            Err(utils::convert_status(verify_status).ok_or_else(|| {
-                error!("Failed to convert error status.");
-                ResponseStatus::PsaErrorGenericError
-            })?)
+            Err(utils::convert_status(verify_status))
         }
     }
 }

src/providers/mbed_provider/utils.rs

@@ -87,12 +87,12 @@ pub fn convert_key_bits(key_size: u32) -> psa_key_bits_t {
 /// # Errors
 ///
 /// Only `KeyType::RsaKeypair` and `KeyType::RsaPublicKey` are supported. Returns
-/// ResponseStatus::UnsupportedParameters otherwise.
+/// ResponseStatus::PsaErrorNotSupported otherwise.
 pub fn convert_key_type(key_type: KeyType) -> Result<psa_key_type_t> {
     match key_type {
         KeyType::RsaKeypair => Ok(PSA_KEY_TYPE_RSA_KEYPAIR),
         KeyType::RsaPublicKey => Ok(PSA_KEY_TYPE_RSA_PUBLIC_KEY),
-        _ => Err(ResponseStatus::UnsupportedParameters),
+        _ => Err(ResponseStatus::PsaErrorNotSupported),
     }
 }
 
@@ -136,20 +136,20 @@ pub fn convert_key_usage(operation: &key_attributes::KeyAttributes) -> psa_key_u
 ///
 /// Only `AlgorithmInner::Sign` is supported as algorithm with only the
 /// `SignAlgorithm::RsaPkcs1v15Sign` signing algorithm. Will return
-/// ResponseStatus::UnsupportedParameters otherwise.
+/// ResponseStatus::PsaErrorNotSupported otherwise.
 pub fn convert_algorithm(alg: &Algorithm) -> Result<psa_algorithm_t> {
     let mut algo_val: psa_algorithm_t;
     match alg.inner() {
         AlgorithmInner::Sign(sign, hash) => {
             algo_val = match sign {
                 SignAlgorithm::RsaPkcs1v15Sign => PSA_ALG_RSA_PKCS1V15_SIGN_BASE,
-                _ => return Err(ResponseStatus::UnsupportedParameters),
+                _ => return Err(ResponseStatus::PsaErrorNotSupported),
             };
             if let Some(hash_alg) = hash {
                 algo_val |= convert_hash_algorithm(*hash_alg) & PSA_ALG_HASH_MASK;
             }
         }
-        _ => return Err(ResponseStatus::UnsupportedParameters),
+        _ => return Err(ResponseStatus::PsaErrorNotSupported),
     }
     Ok(algo_val)
 }
@@ -179,14 +179,25 @@ const PSA_STATUS_TO_RESPONSE_STATUS_OFFSET: psa_status_t = 1000;
 
 /// Converts between Mbed Crypto and native status values.
 /// Returns None if the conversion can not happen.
-pub fn convert_status(psa_status: psa_status_t) -> Option<ResponseStatus> {
+pub fn convert_status(psa_status: psa_status_t) -> ResponseStatus {
     // psa_status_t errors are i32, negative values between -132 and -151. To map them to u16
     // ResponseStatus values between 1000 and 1999 (as per the Wire Protocol), they are taken their
     // absolute values and added 1000.
-    let psa_status = psa_status.checked_abs()?;
-    let psa_status = psa_status.checked_add(PSA_STATUS_TO_RESPONSE_STATUS_OFFSET)?;
-    let psa_status = u16::try_from(psa_status).ok()?;
-    Some(psa_status.try_into().ok()?)
+    let psa_status = match psa_status.checked_abs() {
+        Some(status) => status,
+        None => return ResponseStatus::InvalidEncoding,
+    };
+    let psa_status = match psa_status.checked_add(PSA_STATUS_TO_RESPONSE_STATUS_OFFSET) {
+        Some(status) => status,
+        None => return ResponseStatus::InvalidEncoding,
+    };
+    let psa_status = match u16::try_from(psa_status) {
+        Ok(status) => status,
+        Err(_) => return ResponseStatus::InvalidEncoding,
+    };
+    psa_status
+        .try_into()
+        .unwrap_or(ResponseStatus::InvalidEncoding)
 }
 
 macro_rules! bits_to_bytes {
@@ -201,7 +212,7 @@ pub fn psa_asymmetric_sign_output_size(key_attrs: &psa_key_attributes_t) -> Resu
     match key_attrs.core.type_ {
         PSA_KEY_TYPE_RSA_KEYPAIR => Ok(usize::from(bits_to_bytes!(key_attrs.core.bits))),
         PSA_KEY_TYPE_ECC_KEYPAIR_BASE => Ok(usize::from(bits_to_bytes!(key_attrs.core.bits) * 2)),
-        _ => Err(ResponseStatus::UnsupportedParameters),
+        _ => Err(ResponseStatus::PsaErrorNotSupported),
     }
 }
 
@@ -218,7 +229,7 @@ pub fn psa_export_public_key_size(key_attrs: &psa_key_attributes_t) -> Result<us
         PSA_KEY_TYPE_RSA_PUBLIC_KEY | PSA_KEY_TYPE_RSA_KEYPAIR => Ok(usize::from(
             export_asn1_int_max_size!(key_attrs.core.bits) + 11,
         )),
-        _ => Err(ResponseStatus::UnsupportedParameters),
+        _ => Err(ResponseStatus::PsaErrorNotSupported),
     }
 }
 
@@ -277,10 +288,7 @@ impl KeyHandle {
         let open_key_status = psa_crypto_binding::psa_open_key(key_id, &mut key_handle);
         if open_key_status != PSA_SUCCESS {
             error!("Open key status: {}", open_key_status);
-            Err(convert_status(open_key_status).ok_or_else(|| {
-                error!("Failed to convert error status.");
-                ResponseStatus::InvalidEncoding
-            })?)
+            Err(convert_status(open_key_status))
         } else {
             Ok(KeyHandle(key_handle))
         }
@@ -299,10 +307,7 @@ impl KeyHandle {
         let status = psa_crypto_binding::psa_generate_key(attributes, &mut key_handle);
         if status != PSA_SUCCESS {
             error!("Generate key status: {}", status);
-            Err(convert_status(status).ok_or_else(|| {
-                error!("Failed to convert error status.");
-                ResponseStatus::InvalidEncoding
-            })?)
+            Err(convert_status(status))
         } else {
             Ok(KeyHandle(key_handle))
         }
@@ -326,10 +331,7 @@ impl KeyHandle {
         );
         if status != PSA_SUCCESS {
             error!("Import key status: {}", status);
-            Err(convert_status(status).ok_or_else(|| {
-                error!("Failed to convert error status.");
-                ResponseStatus::InvalidEncoding
-            })?)
+            Err(convert_status(status))
         } else {
             Ok(KeyHandle(key_handle))
         }
@@ -347,10 +349,7 @@ impl KeyHandle {
 
         if get_attrs_status != PSA_SUCCESS {
             error!("Get key attributes status: {}", get_attrs_status);
-            Err(convert_status(get_attrs_status).ok_or_else(|| {
-                error!("Failed to convert error status.");
-                ResponseStatus::InvalidEncoding
-            })?)
+            Err(convert_status(get_attrs_status))
         } else {
             Ok(KeyAttributes(key_attrs))
         }
@@ -372,10 +371,7 @@ impl KeyHandle {
 
         if status != PSA_SUCCESS {
             error!("Close key status: {}", status);
-            Err(convert_status(status).ok_or_else(|| {
-                error!("Failed to convert error status.");
-                ResponseStatus::InvalidEncoding
-            })?)
+            Err(convert_status(status))
         } else {
             Ok(())
         }

src/providers/mod.rs

@@ -72,7 +72,7 @@ pub trait Provide {
 
     /// List the providers running in the service.
     fn list_providers(&self, _op: OpListProviders) -> Result<ResultListProviders> {
-        Err(ResponseStatus::UnsupportedOperation)
+        Err(ResponseStatus::PsaErrorNotSupported)
     }
 
     /// List the opcodes supported by the current provider.
@@ -85,17 +85,17 @@ pub trait Provide {
     /// This operation will only fail if not implemented. It will never fail when being called on
     /// the `CoreProvider`.
     fn ping(&self, _op: OpPing) -> Result<ResultPing> {
-        Err(ResponseStatus::UnsupportedOperation)
+        Err(ResponseStatus::PsaErrorNotSupported)
     }
 
     /// Execute a CreateKey operation.
     fn create_key(&self, _app_name: ApplicationName, _op: OpCreateKey) -> Result<ResultCreateKey> {
-        Err(ResponseStatus::UnsupportedOperation)
+        Err(ResponseStatus::PsaErrorNotSupported)
     }
 
     /// Execute a ImportKey operation.
     fn import_key(&self, _app_name: ApplicationName, _op: OpImportKey) -> Result<ResultImportKey> {
-        Err(ResponseStatus::UnsupportedOperation)
+        Err(ResponseStatus::PsaErrorNotSupported)
     }
 
     /// Execute a ExportPublicKey operation.
@@ -104,7 +104,7 @@ pub trait Provide {
         _app_name: ApplicationName,
         _op: OpExportPublicKey,
     ) -> Result<ResultExportPublicKey> {
-        Err(ResponseStatus::UnsupportedOperation)
+        Err(ResponseStatus::PsaErrorNotSupported)
     }
 
     /// Execute a DestroyKey operation.
@@ -113,13 +113,13 @@ pub trait Provide {
         _app_name: ApplicationName,
         _op: OpDestroyKey,
     ) -> Result<ResultDestroyKey> {
-        Err(ResponseStatus::UnsupportedOperation)
+        Err(ResponseStatus::PsaErrorNotSupported)
     }
 
     /// Execute a AsymSign operation. This operation only signs the short digest given but does not
     /// hash it.
     fn asym_sign(&self, _app_name: ApplicationName, _op: OpAsymSign) -> Result<ResultAsymSign> {
-        Err(ResponseStatus::UnsupportedOperation)
+        Err(ResponseStatus::PsaErrorNotSupported)
     }
 
     /// Execute a AsymVerify operation.
@@ -128,6 +128,6 @@ pub trait Provide {
         _app_name: ApplicationName,
         _op: OpAsymVerify,
     ) -> Result<ResultAsymVerify> {
-        Err(ResponseStatus::UnsupportedOperation)
+        Err(ResponseStatus::PsaErrorNotSupported)
     }
 }