SecurityCoordinator: cleanup interface (part 2)

- low serialize of request parameters into kata_security_request
- add InstallRequest that passes the package buffer as an opaque ptr
- add EchoRequest for SecurityRequestEcho
- purge (now) unused SecurityRequestData type alias and hide
  SECURITY_REQUEST_DATA_SIZE (only used in crate)
- use &str instead of String in serialize requests (from mattharvey@)
- add SreSerializeFailed

Change-Id: Iac1930c0b2fead0f96b87da5d116280865031be2
GitOrigin-RevId: 37df6cd1969b3be2628e2e34f3de8fd129fdbc1b

apps/system/components/DebugConsole/kata-shell/src/lib.rs

@@ -128,12 +128,19 @@ fn echo_command(cmdline: &str, output: &mut dyn io::Write) -> Result<(), Command
 /// Implements an "scecho" command that sends arguments to the Security Core's echo service.
 fn scecho_command(cmdline: &str, output: &mut dyn io::Write) -> Result<(), CommandError> {
     use kata_security_interface::kata_security_request;
+    use kata_security_interface::EchoRequest;
     use kata_security_interface::SecurityRequest;
     use kata_security_interface::SECURITY_REPLY_DATA_SIZE;
 
     let (_, request) = cmdline.split_at(7); // 'scecho'
     let reply = &mut [0u8; SECURITY_REPLY_DATA_SIZE];
-    match kata_security_request(SecurityRequest::SrEcho, request.as_bytes(), reply) {
+    match kata_security_request(
+        SecurityRequest::SrEcho,
+        &EchoRequest {
+            value: request.as_bytes(),
+        },
+        reply,
+    ) {
         Ok(_) => {
             writeln!(
                 output,

apps/system/components/ProcessManager/kata-proc-manager/src/lib.rs

@@ -4,12 +4,12 @@
 
 extern crate alloc;
 use alloc::string::String;
-use core::slice;
 use kata_proc_common::*;
 use kata_security_interface::kata_security_request;
+use kata_security_interface::InstallRequest;
 use kata_security_interface::SecurityRequest;
+use kata_security_interface::UninstallRequest;
 use kata_security_interface::SECURITY_REPLY_DATA_SIZE;
-use kata_security_interface::SECURITY_REQUEST_DATA_SIZE;
 use log::trace;
 use postcard;
 use spin::Mutex;
@@ -95,7 +95,10 @@ impl ProcessManagerInterface for KataManagerInterface {
         let reply = &mut [0u8; SECURITY_REPLY_DATA_SIZE];
         match kata_security_request(
             SecurityRequest::SrInstall,
-            unsafe { slice::from_raw_parts(pkg_buffer, pkg_buffer_size as usize) },
+            &InstallRequest {
+                pkg_buffer_size: pkg_buffer_size,
+                pkg_buffer: pkg_buffer,
+            },
             reply,
         ) {
             Ok(_) => {
@@ -112,19 +115,18 @@ impl ProcessManagerInterface for KataManagerInterface {
         }
     }
     fn uninstall(&mut self, bundle_id: &str) -> Result<(), ProcessManagerError> {
-        fn serialize_failure(e: postcard::Error) -> ProcessManagerError {
-            trace!("uninstall failed: serialize {:?}", e);
-            ProcessManagerError::UninstallFailed
-        }
-
         // NB: the caller has already checked no running application exists
         // NB: the Security Core is assumed to invalidate/remove any kv store
 
         // This is handled by the SecurityCoordinator.
-        let mut request_data = [0u8; SECURITY_REQUEST_DATA_SIZE];
-        let _ = postcard::to_slice(&bundle_id, &mut request_data).map_err(serialize_failure)?;
         let reply = &mut [0u8; SECURITY_REPLY_DATA_SIZE];
-        match kata_security_request(SecurityRequest::SrUninstall, &request_data, reply) {
+        match kata_security_request(
+            SecurityRequest::SrUninstall,
+            &UninstallRequest {
+                bundle_id: &bundle_id,
+            },
+            reply,
+        ) {
             Ok(_) => Ok(()),
             Err(status) => {
                 trace!("uninstall failed: {:?}", status);

apps/system/components/SecurityCoordinator/kata-security-coordinator/src/fakeimpl/mod.rs

@@ -174,7 +174,7 @@ impl SecurityCoordinatorInterface for FakeSecurityCoordinator {
                     request.key,
                 );
                 let bundle = self.get_bundle(&request.bundle_id)?;
-                match bundle.keys.get(&request.key) {
+                match bundle.keys.get(request.key) {
                     Some(value) => {
                         // TODO(sleffler): return values are fixed size unless we serialize
                         reply_buffer[..value.len()].copy_from_slice(&value[..]);
@@ -196,7 +196,7 @@ impl SecurityCoordinatorInterface for FakeSecurityCoordinator {
                 // TODO(sleffler): optimnize with entry
                 let mut value = [0u8; KEY_VALUE_DATA_SIZE];
                 value[..request.value.len()].copy_from_slice(request.value);
-                let _ = bundle.keys.insert(request.key, value);
+                let _ = bundle.keys.insert(request.key.to_string(), value);
                 Ok(())
             }
             SecurityRequest::SrDeleteKey => {
@@ -209,7 +209,7 @@ impl SecurityCoordinatorInterface for FakeSecurityCoordinator {
                 );
                 let bundle = self.get_bundle_mut(&request.bundle_id)?;
                 // TODO(sleffler): error if no entry?
-                let _ = bundle.keys.remove(&request.key);
+                let _ = bundle.keys.remove(request.key);
                 Ok(())
             }
         }

apps/system/components/SecurityCoordinator/kata-security-interface/Cargo.toml

@@ -4,5 +4,6 @@ version = "0.1.0"
 edition = "2018"
 
 [dependencies]
+postcard = "0.7"
 serde = { version = "1.0", default-features = false, features = ["alloc", "derive"] }
 serde-big-array = "0.3"

apps/system/components/SecurityCoordinator/kata-security-interface/src/lib.rs

@@ -2,9 +2,8 @@
 
 #![cfg_attr(not(test), no_std)]
 
-extern crate alloc;
-use alloc::string::String;
 use core::str;
+use postcard;
 use serde::{Deserialize, Serialize};
 
 // NB: serde helper for arrays w/ >32 elements
@@ -17,8 +16,7 @@ big_array! { BigArray; }
 // and also by it being allocated on the stack of the rpc glue code.
 // So we need to balance these against being able to handle all values.
 
-pub const SECURITY_REQUEST_DATA_SIZE: usize = 2048;
+const SECURITY_REQUEST_DATA_SIZE: usize = 2048;
-pub type SecurityRequestData = [u8; SECURITY_REQUEST_DATA_SIZE];
 
 pub const SECURITY_REPLY_DATA_SIZE: usize = 2048;
 pub type SecurityReplyData = [u8; SECURITY_REPLY_DATA_SIZE];
@@ -26,6 +24,25 @@ pub type SecurityReplyData = [u8; SECURITY_REPLY_DATA_SIZE];
 // NB: struct's marked repr(C) are processed by cbindgen to get a .h file
 //   used in camkes C interfaces.
 
+mod ptr_helper {
+    use super::*;
+    use serde::{Deserializer, Serializer};
+
+    pub fn serialize<T, S>(ptr: &*const T, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        (*ptr as usize).serialize(serializer)
+    }
+
+    pub fn deserialize<'de, T, D>(deserializer: D) -> Result<*const T, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        Ok(usize::deserialize(deserializer)? as *const T)
+    }
+}
+
 mod mut_ptr_helper {
     use super::*;
     use serde::{Deserializer, Serializer};
@@ -45,32 +62,45 @@ mod mut_ptr_helper {
     }
 }
 
-// TODO(sleffler): convert String to &str
+// SecurityRequestEcho
+#[derive(Debug, Serialize, Deserialize)]
+pub struct EchoRequest<'a> {
+    pub value: &'a [u8],
+}
 
-// NB: SecurityRequestInstall is handled specially.
+// SecurityRequestInstall
+#[derive(Debug, Serialize, Deserialize)]
+pub struct InstallRequest {
+    pub pkg_buffer_size: u32,
+
+    // Gather-list of shared memory pages holding package data.
+    // TODO(sleffler) gather list
+    #[serde(with = "ptr_helper")]
+    pub pkg_buffer: *const u8,
+}
 
 // SecurityRequestUninstall
 #[derive(Debug, Serialize, Deserialize)]
-pub struct UninstallRequest {
+pub struct UninstallRequest<'a> {
-    pub bundle_id: String,
+    pub bundle_id: &'a str,
 }
 
 // SecurityRequestSizeBuffer
 #[derive(Debug, Serialize, Deserialize)]
-pub struct SizeBufferRequest {
+pub struct SizeBufferRequest<'a> {
-    pub bundle_id: String,
+    pub bundle_id: &'a str,
 }
 
 // SecurityRequestGetManifest
 #[derive(Debug, Serialize, Deserialize)]
-pub struct GetManifestRequest {
+pub struct GetManifestRequest<'a> {
-    pub bundle_id: String,
+    pub bundle_id: &'a str,
 }
 
 // SecurityRequestLoadApplication
 #[derive(Debug, Serialize, Deserialize)]
-pub struct LoadApplicationRequest {
+pub struct LoadApplicationRequest<'a> {
-    pub bundle_id: String,
+    pub bundle_id: &'a str,
 
     // Scatter-list of shared memory pages where application should be loaded.
     // TODO(sleffler) scatter list
@@ -80,9 +110,9 @@ pub struct LoadApplicationRequest {
 
 // SecurityRequestLoadModel
 #[derive(Debug, Serialize, Deserialize)]
-pub struct LoadModelRequest {
+pub struct LoadModelRequest<'a> {
-    pub bundle_id: String,
+    pub bundle_id: &'a str,
-    pub model_id: String,
+    pub model_id: &'a str,
 
     // Scatter-list of shared memory pages where model should be loaded.
     // TODO(sleffler) scatter list
@@ -92,24 +122,24 @@ pub struct LoadModelRequest {
 
 // SecurityRequestReadKey
 #[derive(Debug, Serialize, Deserialize)]
-pub struct ReadKeyRequest {
+pub struct ReadKeyRequest<'a> {
-    pub bundle_id: String,
+    pub bundle_id: &'a str,
-    pub key: String,
+    pub key: &'a str,
 }
 
 // SecurityRequestWriteKey
 #[derive(Debug, Serialize, Deserialize)]
 pub struct WriteKeyRequest<'a> {
-    pub bundle_id: String,
+    pub bundle_id: &'a str,
-    pub key: String,
+    pub key: &'a str,
     pub value: &'a [u8],
 }
 
 // SecurityRequestDeleteKey
 #[derive(Debug, Serialize, Deserialize)]
-pub struct DeleteKeyRequest {
+pub struct DeleteKeyRequest<'a> {
-    pub bundle_id: String,
+    pub bundle_id: &'a str,
-    pub key: String,
+    pub key: &'a str,
 }
 
 // NB: this is the union of InstallInterface & StorageInterface because
@@ -126,6 +156,7 @@ pub enum SecurityRequestError {
     SrePackageBufferLenInvalid,
     SreValueInvalid,
     SreKeyInvalid,
+    SreSerializeFailed,
     // Generic errors, mostly used in unit tests
     SreEchoFailed,
     SreInstallFailed,
@@ -171,9 +202,9 @@ pub trait SecurityCoordinatorInterface {
 // TODO(sleffler): try kata_security_request<T> to lower serde work
 #[inline]
 #[allow(dead_code)]
-pub fn kata_security_request(
+pub fn kata_security_request<T: Serialize>(
     request: SecurityRequest,
-    request_buffer: &[u8],
+    request_args: &T,
     reply_buffer: &mut SecurityReplyData,
 ) -> Result<(), SecurityRequestError> {
     // NB: this assumes the SecurityCoordinator component is named "security".
@@ -185,6 +216,9 @@ pub fn kata_security_request(
             c_reply_buffer: *mut SecurityReplyData,
         ) -> SecurityRequestError;
     }
+    let mut request_buffer = [0u8; SECURITY_REQUEST_DATA_SIZE];
+    let _ = postcard::to_slice(request_args, &mut request_buffer[..])
+        .map_err(|_| SecurityRequestError::SreSerializeFailed)?;
     match unsafe {
         security_request(
             request,

apps/system/components/StorageManager/kata-storage-manager/Cargo.toml

@@ -7,4 +7,3 @@ edition = "2018"
 kata-security-interface = { path = "../../SecurityCoordinator/kata-security-interface" }
 kata-storage-interface = { path = "../kata-storage-interface" }
 log = "0.4"
-postcard = "0.7"

apps/system/components/StorageManager/kata-storage-manager/src/lib.rs

@@ -2,44 +2,35 @@
 
 #![cfg_attr(not(test), no_std)]
 
-extern crate alloc;
-use alloc::string::String;
 use kata_security_interface::kata_security_request;
 use kata_security_interface::DeleteKeyRequest;
 use kata_security_interface::ReadKeyRequest;
 use kata_security_interface::SecurityRequest;
 use kata_security_interface::WriteKeyRequest;
 use kata_security_interface::SECURITY_REPLY_DATA_SIZE;
-use kata_security_interface::SECURITY_REQUEST_DATA_SIZE;
 use kata_storage_interface::StorageError;
 use kata_storage_interface::StorageManagerInterface;
 use kata_storage_interface::{KeyValueData, KEY_VALUE_DATA_SIZE};
 use log::trace;
-use postcard;
 
-// NB: KATA_STORAGE cannot be used before setup is completed with a call to init()
 #[cfg(not(test))]
 pub static mut KATA_STORAGE: KataStorageManager = KataStorageManager {};
 
-// KataStorageManager bundles an instance of the StorageManager that operates
-// on KataOS interfaces. There is a two-step dance to setup an instance because
-// we want KATA_STORAGE static and there is no const Box::new variant.
 pub struct KataStorageManager;
 impl StorageManagerInterface for KataStorageManager {
     fn read(&self, bundle_id: &str, key: &str) -> Result<KeyValueData, StorageError> {
         trace!("read bundle_id:{} key:{}", bundle_id, key);
 
         // Send request to Security Core via SecurityCoordinator
-        let mut request = [0u8; SECURITY_REQUEST_DATA_SIZE];
-        let _ = postcard::to_slice(
-            &ReadKeyRequest {
-                bundle_id: String::from(bundle_id),
-                key: String::from(key),
-            },
-            &mut request[..],
-        )?;
         let result = &mut [0u8; SECURITY_REPLY_DATA_SIZE];
-        let _ = kata_security_request(SecurityRequest::SrReadKey, &request, result)?;
+        kata_security_request(
+            SecurityRequest::SrReadKey,
+            &ReadKeyRequest {
+                bundle_id: bundle_id,
+                key: key,
+            },
+            result,
+        )?;
         // NB: must copy into KeyValueData for now
         let mut keyval = [0u8; KEY_VALUE_DATA_SIZE];
         keyval.copy_from_slice(&result[..KEY_VALUE_DATA_SIZE]);
@@ -54,33 +45,31 @@ impl StorageManagerInterface for KataStorageManager {
         );
 
         // Send request to Security Core via SecurityCoordinator
-        let mut request = [0u8; SECURITY_REQUEST_DATA_SIZE];
+        let result = &mut [0u8; SECURITY_REPLY_DATA_SIZE];
-        let _ = postcard::to_slice(
+        kata_security_request(
+            SecurityRequest::SrWriteKey,
             &WriteKeyRequest {
-                bundle_id: String::from(bundle_id),
+                bundle_id: bundle_id,
-                key: String::from(key),
+                key: key,
                 value: value,
             },
-            &mut request[..],
+            result,
         )?;
-        let result = &mut [0u8; SECURITY_REPLY_DATA_SIZE];
-        kata_security_request(SecurityRequest::SrWriteKey, &request, result)?;
         Ok(())
     }
     fn delete(&self, bundle_id: &str, key: &str) -> Result<(), StorageError> {
         trace!("delete bundle_id:{} key:{}", bundle_id, key);
 
         // Send request to Security Core via SecurityCoordinator
-        let mut request = [0u8; SECURITY_REQUEST_DATA_SIZE];
-        let _ = postcard::to_slice(
-            &DeleteKeyRequest {
-                bundle_id: String::from(bundle_id),
-                key: String::from(key),
-            },
-            &mut request[..],
-        )?;
         let result = &mut [0u8; SECURITY_REPLY_DATA_SIZE];
-        kata_security_request(SecurityRequest::SrDeleteKey, &request, result)?;
+        kata_security_request(
+            SecurityRequest::SrDeleteKey,
+            &DeleteKeyRequest {
+                bundle_id: bundle_id,
+                key: key,
+            },
+            result,
+        )?;
         Ok(())
     }
 }