Add docs and some fixes

This commit adds documentation to the structures and methods in the new
Trusted Service provider. A few small fixes are also added, along with a
finer-grained approach to zeroizing the key data in import_key.

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

Author: ionut-arm

src/providers/trusted_service/asym_sign.rs

@@ -20,7 +20,7 @@ impl Provider {
         Ok(psa_sign_hash::Result {
             signature: self
                 .context
-                .asym_sign(key_id, op.hash.to_vec(), op.alg)?
+                .sign_hash(key_id, op.hash.to_vec(), op.alg)?
                 .into(),
         })
     }
@@ -35,7 +35,7 @@ impl Provider {
         let key_id = key_management::get_key_id(&key_triple, &*store_handle)?;
 
         self.context
-            .asym_verify(key_id, op.hash.to_vec(), op.signature.to_vec(), op.alg)?;
+            .verify_hash(key_id, op.hash.to_vec(), op.signature.to_vec(), op.alg)?;
 
         Ok(psa_verify_hash::Result {})
     }

src/providers/trusted_service/context/asym_sign.rs

@@ -8,13 +8,14 @@ use parsec_interface::requests::ResponseStatus;
 use std::convert::TryInto;
 
 impl Context {
-    pub fn asym_sign(
+    /// Sign a hash with an asymmetric key given its ID and the signing algorithm.
+    pub fn sign_hash(
         &self,
         key_id: u32,
         hash: Vec<u8>,
         algorithm: AsymmetricSignature,
     ) -> Result<Vec<u8>, ResponseStatus> {
-        info!("Handling GenerateKey request");
+        info!("Handling SignHash request");
         let proto_req = SignHashIn {
             handle: 0,
             hash,
@@ -25,14 +26,15 @@ impl Context {
         Ok(signature)
     }
 
-    pub fn asym_verify(
+    /// Verify a signature on a hash with an asymmetric key given its ID and the signing algorithm.
+    pub fn verify_hash(
         &self,
         key_id: u32,
         hash: Vec<u8>,
         signature: Vec<u8>,
         algorithm: AsymmetricSignature,
     ) -> Result<(), ResponseStatus> {
-        info!("Handling GenerateKey request");
+        info!("Handling VerifyHash request");
         let proto_req = VerifyHashIn {
             handle: 0,
             hash,

src/providers/trusted_service/context/key_management.rs

@@ -13,6 +13,10 @@ use std::convert::{TryFrom, TryInto};
 use zeroize::Zeroize;
 
 impl Context {
+    /// Generate a key given its attributes and ID
+    ///
+    /// Lifetime flexibility is not supported: the `lifetime` parameter in the key
+    /// attributes is essentially ignored and replaced with `KeyLifetime::Persistent`.
     pub fn generate_key(&self, key_attrs: Attributes, id: u32) -> Result<(), ResponseStatus> {
         info!("Handling GenerateKey request");
         let generate_req = GenerateKeyIn {
@@ -35,38 +39,66 @@ impl Context {
         Ok(())
     }
 
+    /// Import a key given its attributes, ID, and key data.
+    ///
+    /// Lifetime flexibility is not supported: the `lifetime` parameter in the key
+    /// attributes is essentially ignored and replaced with `KeyLifetime::Persistent`.
+    ///
+    /// Key data must be in the format described by the PSA Crypto format.
     pub fn import_key(
         &self,
         key_attrs: Attributes,
         id: u32,
         key_data: &[u8],
     ) -> Result<(), ResponseStatus> {
+        info!("Handling ImportKey request");
+        let mut data = key_data.to_vec();
         let mut import_req = ImportKeyIn {
             attributes: Some(KeyAttributes {
-                r#type: u16::try_from(key_attrs.key_type)? as u32,
-                key_bits: key_attrs.bits.try_into()?,
+                r#type: u16::try_from(key_attrs.key_type).map_err(|e| {
+                    data.zeroize();
+                    e
+                })? as u32,
+                key_bits: key_attrs.bits.try_into().map_err(|e| {
+                    data.zeroize();
+                    e
+                })?,
                 lifetime: KeyLifetime::Persistent as u32,
                 id,
                 policy: Some(KeyPolicy {
-                    usage: key_attrs.policy.usage_flags.try_into()?,
-                    alg: key_attrs.policy.permitted_algorithms.try_into()?,
+                    usage: key_attrs.policy.usage_flags.into(),
+                    alg: key_attrs
+                        .policy
+                        .permitted_algorithms
+                        .try_into()
+                        .map_err(|e| {
+                            data.zeroize();
+                            e
+                        })?,
                 }),
             }),
-            data: key_data.to_vec(),
+            data,
         };
-        let ImportKeyOut { handle } = self.send_request(&import_req)?;
+        let res = self.send_request(&import_req);
         import_req.data.zeroize();
+        let ImportKeyOut { handle } = res?;
 
         let close_req = CloseKeyIn { handle };
         self.send_request(&close_req)?;
 
         Ok(())
     }
 
+    /// Export the public part of a key given its ID.
+    ///
+    /// The public key data is returned in the format specified by the PSA Crypto
+    /// format.
     pub fn export_public_key(&self, id: u32) -> Result<Vec<u8>, ResponseStatus> {
+        info!("Handling ExportPublicKey request");
         Ok(self.send_request_with_key(ExportPublicKeyIn::default(), id)?)
     }
 
+    /// Destroy a key given its ID.
     pub fn destroy_key(&self, key_id: u32) -> Result<(), ResponseStatus> {
         info!("Handling DestroyKey request");
         if !self.check_key_exists(key_id)? {
@@ -80,6 +112,7 @@ impl Context {
         Ok(())
     }
 
+    /// Verify if a key with a given ID exists.
     pub fn check_key_exists(&self, key_id: u32) -> Result<bool, Error> {
         info!("Handling CheckKey request");
         let open_req = OpenKeyIn { id: key_id };

src/providers/trusted_service/context/mod.rs

@@ -50,6 +50,8 @@ mod key_management;
 mod ts_protobuf {
     include!(concat!(env!("OUT_DIR"), "/ts_crypto.rs"));
 
+    /// Trait for associating an Opcode with each operation type
+    /// and obtaining it in a generic way.
     pub trait GetOpcode {
         fn opcode(&self) -> Opcode;
     }
@@ -87,6 +89,8 @@ mod ts_protobuf {
     opcode_impl!(ImportKeyIn, ImportKeyOut, ImportKey);
     opcode_impl!(ExportPublicKeyIn, ExportPublicKeyOut, ExportPublicKey);
 
+    /// Trait allowing the handle of opened-key-dependent operations
+    /// to be set in a generic way.
     pub trait SetHandle {
         fn set_handle(&mut self, handle: u32);
     }
@@ -110,8 +114,28 @@ mod ts_protobuf {
 // TODO:
 // * RPC caller error handling
 // * proper logging
-// * docs
 
+/// Context for interacting with the crypto Trusted Service (TS).
+///
+/// The context maintains the state necessary for calls to be made
+/// and acts as a bridge between the two encoding types: Rust native
+/// PSA Crypto and the IPC mechanism used by the Normal World userland
+/// TS endpoint.
+///
+/// `Context` does not surface the full operation sequence demanded by the
+/// TS. Keys need not be opened before use - only referenced by their creation
+/// ID - and therefore not closed either.
+///
+/// # Safety
+///
+/// `Sync` and `Send` are manually implemented on this type since it
+/// contains pointers to the structures that perform various tasks
+/// and which act as the underlying client of `Context`. The use of
+/// these pointers is not thread-safe, so in order to allow `Context`
+/// to be used across threads, a `Mutex` is used to lock down all calls.
+///
+/// Upon being dropped, all the resources are released and no prior cleanup
+/// is required from the caller.
 #[derive(Debug)]
 pub struct Context {
     rpc_caller: *mut rpc_caller,
@@ -121,6 +145,7 @@ pub struct Context {
 }
 
 impl Context {
+    /// Establish a connection to the Trusted Service to obtain a working context.
     pub fn connect() -> anyhow::Result<Self> {
         // Initialise service locator. Can be called multiple times,
         // but *must* be called at least once.
@@ -171,11 +196,14 @@ impl Context {
         Ok(ctx)
     }
 
+    // Serialize and send a request and deserialize the response back to
+    // the caller. The caller is responsible for explicitly declaring
+    // the response type if its contents are of interest.
     fn send_request<T: Message + Default>(
         &self,
         req: &(impl Message + GetOpcode),
     ) -> Result<T, PsaError> {
-        let _mutex_guard = self.call_mutex.try_lock().expect("Call mutex poisoned");
+        let _mutex_guard = self.call_mutex.lock().expect("Call mutex poisoned");
         info!("Beginning call to Trusted Service");
 
         let mut buf_out = null_mut();
@@ -229,16 +257,21 @@ impl Context {
         Ok(resp)
     }
 
+    // Send a request that requires a key, given the key's ID.
+    // This function is responsible for opening the key, for sending the
+    // request with `send_request` and for closing the key afterwards.
     fn send_request_with_key<T: Message + Default>(
         &self,
         mut req: impl Message + GetOpcode + SetHandle,
         key_id: u32,
     ) -> Result<T, PsaError> {
         let open_req = OpenKeyIn { id: key_id };
         let OpenKeyOut { handle } = self.send_request(&open_req)?;
+
         req.set_handle(handle);
         let res = self.send_request(&req);
         let close_req = CloseKeyIn { handle };
+
         let res_close = self.send_request(&close_req);
         let res = res?;
         res_close?;

src/providers/trusted_service/mod.rs

@@ -5,20 +5,20 @@
 //! This provider is backed by a crypto Trusted Service deployed in TrustZone
 use super::mbed_crypto::key_management as mbed_crypto_key_management;
 use crate::authenticators::ApplicationName;
-use crate::key_info_managers::KeyTriple;
-use crate::key_info_managers::ManageKeyInfo;
+use crate::key_info_managers::{self, KeyTriple, ManageKeyInfo};
 use crate::providers::Provide;
 use context::Context;
 use derivative::Derivative;
 use log::{error, trace};
 use parsec_interface::operations::list_providers::ProviderInfo;
 use parsec_interface::operations::{
-    psa_destroy_key, psa_export_public_key, psa_generate_key, psa_import_key, psa_sign_hash,
-    psa_verify_hash,
+    list_keys, 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, Result};
+use parsec_interface::requests::{Opcode, ProviderID, ResponseStatus, Result};
 use psa_crypto::types::key;
 use std::collections::HashSet;
+use std::ops::Deref;
 use std::sync::{
     atomic::{AtomicU32, Ordering},
     Arc, RwLock,
@@ -29,13 +29,19 @@ mod asym_sign;
 mod context;
 mod key_management;
 
-const SUPPORTED_OPCODES: [Opcode; 2] = [Opcode::PsaDestroyKey, Opcode::PsaGenerateKey];
+const SUPPORTED_OPCODES: [Opcode; 6] = [
+    Opcode::PsaDestroyKey,
+    Opcode::PsaGenerateKey,
+    Opcode::PsaSignHash,
+    Opcode::PsaVerifyHash,
+    Opcode::PsaImportKey,
+    Opcode::PsaExportPublicKey,
+];
 
 /// Trusted Service provider structure
 ///
-/// Currently the provider only supports volatile keys due to limitations in the stack
-/// underneath us. Therefore none of the key information is persisted, being kept instead
-/// in a map for fast access.
+/// Operations for this provider are serviced through an IPC interface that leads
+/// to a Secure World implementation of PSA Crypto.
 #[derive(Derivative)]
 #[derivative(Debug)]
 pub struct Provider {
@@ -53,7 +59,7 @@ pub struct Provider {
 }
 
 impl Provider {
-    /// Creates and initialise a new instance of Provider.
+    /// Creates and initialises a new instance of Provider.
     fn new(
         key_info_store: Arc<RwLock<dyn ManageKeyInfo + Send + Sync>>,
     ) -> anyhow::Result<Provider> {
@@ -64,14 +70,14 @@ impl Provider {
         };
         let mut max_key_id: key::psa_key_id_t = key::PSA_KEY_ID_USER_MIN;
         {
-            // The local scope allows to drop store_handle and local_ids_handle in order to return
+            // The local scope allows dropping store_handle and local_ids_handle in order to return
             // the ts_provider.
             let mut store_handle = ts_provider
                 .key_info_store
                 .write()
                 .expect("Key store lock poisoned");
             let mut to_remove: Vec<KeyTriple> = Vec::new();
-            // Go through all TrustedServiceProvider key triple to key info mappings and check if they are still
+            // Go through all TrustedServiceProvider key triples to key info mappings and check if they are still
             // present.
             // Delete those who are not present and add to the local_store the ones present.
             match store_handle.get_all(ProviderID::TrustedService) {
@@ -129,6 +135,25 @@ impl Provide for Provider {
         }, SUPPORTED_OPCODES.iter().copied().collect()))
     }
 
+    fn list_keys(
+        &self,
+        app_name: ApplicationName,
+        _op: list_keys::Operation,
+    ) -> Result<list_keys::Result> {
+        let store_handle = self.key_info_store.read().expect("Key store lock poisoned");
+        Ok(list_keys::Result {
+            keys: key_info_managers::list_keys(
+                store_handle.deref(),
+                &app_name,
+                ProviderID::TrustedService,
+            )
+            .map_err(|e| {
+                format_error!("Error occurred when fetching key information", e);
+                ResponseStatus::KeyInfoManagerError
+            })?,
+        })
+    }
+
     fn psa_generate_key(
         &self,
         app_name: ApplicationName,