Merge "SecurityCoordinator: cleanup interface (part 1)"

GitOrigin-RevId: bb5f089cdd9edac2b5d310439adbd9d3948ac858

apps/system/components/DebugConsole/kata-shell/Cargo.toml

@@ -9,7 +9,7 @@ cstr_core = "0.2"
 kata-io = { path = "../kata-io" }
 kata-line-reader = { path = "../kata-line-reader" }
 kata-proc-common = { path = "../../ProcessManager/kata-proc-common" }
-kata-security-common = { path = "../../SecurityCoordinator/kata-security-common" }
+kata-security-interface = { path = "../../SecurityCoordinator/kata-security-interface" }
 kata-storage-interface = { path = "../../StorageManager/kata-storage-interface" }
 log = "0.4"
 postcard = { version = "0.7", features = ["alloc"] }

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

@@ -127,18 +127,21 @@ 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_common::*;
+    use kata_security_interface::kata_security_request;
+    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) {
-        SecurityRequestError::SreSuccess => {
+        Ok(_) => {
             writeln!(
                 output,
                 "{}",
                 String::from_utf8_lossy(&reply[..request.len()])
             )?;
         }
-        status => {
+        Err(status) => {
             writeln!(output, "ECHO replied {:?}", status)?;
         }
     }

apps/system/components/ProcessManager/kata-proc-common/Cargo.toml

@@ -4,6 +4,6 @@ version = "0.1.0"
 edition = "2018"
 
 [dependencies]
-kata-security-common = { path = "../../SecurityCoordinator/kata-security-common" }
+kata-security-interface = { path = "../../SecurityCoordinator/kata-security-interface" }
 postcard = { version = "0.7", features = ["alloc"] }
 serde = { version = "1.0", default-features = false, features = ["alloc", "derive"] }

apps/system/components/ProcessManager/kata-proc-manager/Cargo.toml

@@ -7,7 +7,7 @@ edition = "2018"
 [dependencies]
 hashbrown = { version = "0.11", features = ["ahash-compile-time-rng"] }
 kata-proc-common = { path = "../kata-proc-common" }
-kata-security-common = { path = "../../SecurityCoordinator/kata-security-common" }
+kata-security-interface = { path = "../../SecurityCoordinator/kata-security-interface" }
 log = "0.4"
 postcard = "0.7"
 smallstr = "0.2"

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

@@ -6,7 +6,10 @@ extern crate alloc;
 use alloc::string::String;
 use core::slice;
 use kata_proc_common::*;
-use kata_security_common::*;
+use kata_security_interface::kata_security_request;
+use kata_security_interface::SecurityRequest;
+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,14 +98,14 @@ impl ProcessManagerInterface for KataManagerInterface {
             unsafe { slice::from_raw_parts(pkg_buffer, pkg_buffer_size as usize) },
             reply,
         ) {
-            SecurityRequestError::SreSuccess => {
+            Ok(_) => {
                 fn deserialize_failure(e: postcard::Error) -> ProcessManagerError {
                     trace!("install failed: deserialize {:?}", e);
                     ProcessManagerError::BundleDataInvalid
                 }
                 postcard::from_bytes::<String>(reply).map_err(deserialize_failure)
             }
-            status => {
+            Err(status) => {
                 trace!("install failed: {:?}", status);
                 Err(ProcessManagerError::InstallFailed)
             }
@@ -122,8 +125,8 @@ impl ProcessManagerInterface for KataManagerInterface {
         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) {
-            SecurityRequestError::SreSuccess => Ok(()),
+            Ok(_) => Ok(()),
-            status => {
+            Err(status) => {
                 trace!("uninstall failed: {:?}", status);
                 Err(ProcessManagerError::UninstallFailed)
             }

apps/system/components/SecurityCoordinator/Cargo.toml

@@ -1,9 +1,9 @@
 [workspace]
 
 members = [
-    "kata-security-common",
     "kata-security-component",
     "kata-security-coordinator",
+    "kata-security-interface",
 ]
 
 [profile.dev]

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

@@ -7,7 +7,7 @@ edition = "2018"
 kata-allocator = { path = "../../DebugConsole/kata-allocator" }
 kata-logger = { path = "../../DebugConsole/kata-logger" }
 kata-panic = { path = "../../DebugConsole/kata-panic" }
-kata-security-common = { path = "../kata-security-common" }
+kata-security-interface = { path = "../kata-security-interface" }
 kata-security-coordinator = { path = "../kata-security-coordinator" }
 log = "0.4"
 

apps/system/components/SecurityCoordinator/kata-security-component/src/run.rs

@@ -8,8 +8,11 @@ use kata_allocator;
 use kata_logger::KataLogger;
 #[cfg(not(test))]
 extern crate kata_panic;
-use kata_security_common::*;
 use kata_security_coordinator::KATA_SECURITY;
+use kata_security_interface::SecurityCoordinatorInterface;
+use kata_security_interface::SecurityReplyData;
+use kata_security_interface::SecurityRequest;
+use kata_security_interface::SecurityRequestError;
 use log::trace;
 
 #[no_mangle]

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

@@ -10,7 +10,7 @@ sel4 = []
 
 [dependencies]
 hashbrown = { version = "0.11", features = ["ahash-compile-time-rng"] }
-kata-security-common = { path = "../kata-security-common" }
+kata-security-interface = { path = "../kata-security-interface" }
 kata-storage-interface = { path = "../../StorageManager/kata-storage-interface" }
 log = "0.4"
 postcard = "0.7"

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

@@ -3,7 +3,17 @@
 extern crate alloc;
 use alloc::string::{String, ToString};
 use hashbrown::HashMap;
-use kata_security_common::*;
+use kata_security_interface::DeleteKeyRequest;
+use kata_security_interface::GetManifestRequest;
+use kata_security_interface::LoadApplicationRequest;
+use kata_security_interface::LoadModelRequest;
+use kata_security_interface::ReadKeyRequest;
+use kata_security_interface::SecurityCoordinatorInterface;
+use kata_security_interface::SecurityRequest;
+use kata_security_interface::SecurityRequestError;
+use kata_security_interface::SizeBufferRequest;
+use kata_security_interface::UninstallRequest;
+use kata_security_interface::WriteKeyRequest;
 use kata_storage_interface::{KeyValueData, KEY_VALUE_DATA_SIZE};
 use log::trace;
 use postcard;
@@ -123,7 +133,7 @@ impl SecurityCoordinatorInterface for FakeSecurityCoordinator {
                 Ok(())
             }
             SecurityRequest::SrGetManifest => {
-                let request = postcard::from_bytes::<SizeBufferRequest>(&request_buffer[..])
+                let request = postcard::from_bytes::<GetManifestRequest>(&request_buffer[..])
                     .map_err(deserialize_failure)?;
                 trace!("GET MANIFEST bundle_id {}", request.bundle_id);
                 let bundle = self.get_bundle(&request.bundle_id)?;

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

@@ -1,7 +1,16 @@
 //! Kata OS security coordinator seL4 support
 
-extern crate alloc;
+use kata_security_interface::DeleteKeyRequest;
-use kata_security_common::*;
+use kata_security_interface::GetManifestRequest;
+use kata_security_interface::LoadApplicationRequest;
+use kata_security_interface::LoadModelRequest;
+use kata_security_interface::ReadKeyRequest;
+use kata_security_interface::SecurityCoordinatorInterface;
+use kata_security_interface::SecurityRequest;
+use kata_security_interface::SecurityRequestError;
+use kata_security_interface::SizeBufferRequest;
+use kata_security_interface::UninstallRequest;
+use kata_security_interface::WriteKeyRequest;
 use log::trace;
 use postcard;
 
@@ -64,7 +73,7 @@ impl SecurityCoordinatorInterface for SeL4SecurityCoordinator {
                 Err(SreSizeBufferFailed)
             }
             SecurityRequest::SrGetManifest => {
-                let request = postcard::from_bytes::<SizeBufferRequest>(&request_buffer[..])
+                let request = postcard::from_bytes::<GetManifestRequest>(&request_buffer[..])
                     .map_err(deserialize_failure)?;
                 trace!("GET MANIFEST bundle_id {}", request.bundle_id);
                 // TODO(sleffler): fill-in

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

@@ -6,7 +6,9 @@
 
 extern crate alloc;
 use alloc::boxed::Box;
-use kata_security_common::*;
+use kata_security_interface::SecurityCoordinatorInterface;
+use kata_security_interface::SecurityRequest;
+use kata_security_interface::SecurityRequestError;
 
 #[cfg(all(feature = "fake", feature = "sel4"))]
 compile_error!("features \"fake\" and \"sel4\" are mutually exclusive");

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

@@ -1,5 +1,5 @@
 [package]
-name = "kata-security-common"
+name = "kata-security-interface"
 version = "0.1.0"
 edition = "2018"
 

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

@@ -175,7 +175,7 @@ pub fn kata_security_request(
     request: SecurityRequest,
     request_buffer: &[u8],
     reply_buffer: &mut SecurityReplyData,
-) -> SecurityRequestError {
+) -> Result<(), SecurityRequestError> {
     // NB: this assumes the SecurityCoordinator component is named "security".
     extern "C" {
         pub fn security_request(
@@ -185,12 +185,15 @@ pub fn kata_security_request(
             c_reply_buffer: *mut SecurityReplyData,
         ) -> SecurityRequestError;
     }
-    unsafe {
+    match unsafe {
         security_request(
             request,
             request_buffer.len() as u32,
             request_buffer.as_ptr(),
             reply_buffer as *mut _,
         )
+    } {
+        SecurityRequestError::SreSuccess => Ok(()),
+        status => Err(status),
     }
 }

apps/system/components/StorageManager/kata-storage-component/src/run.rs

@@ -11,8 +11,8 @@ use kata_allocator;
 use kata_logger::KataLogger;
 use kata_storage_interface::KeyValueData;
 use kata_storage_interface::StorageError;
-use kata_storage_interface::StorageManagerInterface;
 use kata_storage_interface::StorageManagerError;
+use kata_storage_interface::StorageManagerInterface;
 use kata_storage_manager::KATA_STORAGE;
 use log::trace;
 

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

@@ -5,3 +5,5 @@ edition = "2018"
 
 [dependencies]
 cstr_core = "0.2"
+kata-security-interface = { path = "../../SecurityCoordinator/kata-security-interface" }
+postcard = "0.7"

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

@@ -4,6 +4,8 @@
 
 use core::str;
 use cstr_core::CString;
+use kata_security_interface::SecurityRequestError;
+use postcard;
 
 // TODO(sleffler): temp constraint on value part of key-value pairs
 pub const KEY_VALUE_DATA_SIZE: usize = 100;
@@ -20,12 +22,34 @@ pub enum StorageError {
     KeyInvalid,
     ValueInvalid,
     SerializeFailed,
+    UnknownSecurityError,
     // Generic errors.
     ReadFailed,
     WriteFailed,
     DeleteFailed,
 }
 
+impl From<postcard::Error> for StorageError {
+    fn from(_err: postcard::Error) -> StorageError {
+        StorageError::SerializeFailed
+    }
+}
+impl From<SecurityRequestError> for StorageError {
+    fn from(err: SecurityRequestError) -> StorageError {
+        match err {
+            SecurityRequestError::SreSuccess => StorageError::Success,
+            SecurityRequestError::SreBundleNotFound => StorageError::BundleNotFound,
+            SecurityRequestError::SreKeyNotFound => StorageError::KeyNotFound,
+            SecurityRequestError::SreValueInvalid => StorageError::ValueInvalid,
+            SecurityRequestError::SreKeyInvalid => StorageError::KeyInvalid,
+            SecurityRequestError::SreReadFailed => StorageError::ReadFailed,
+            SecurityRequestError::SreWriteFailed => StorageError::WriteFailed,
+            SecurityRequestError::SreDeleteFailed => StorageError::DeleteFailed,
+            _ => StorageError::UnknownSecurityError,  // NB: cannot happen
+        }
+    }
+}
+
 pub trait StorageManagerInterface {
     fn read(&self, bundle_id: &str, key: &str) -> Result<KeyValueData, StorageError>;
     fn write(&self, bundle_id: &str, key: &str, value: &[u8]) -> Result<(), StorageError>;
@@ -51,17 +75,20 @@ pub enum StorageManagerError {
     SmeDeleteFailed,
 }
 
+impl From<cstr_core::NulError> for StorageManagerError {
+    fn from(_err: cstr_core::NulError) -> StorageManagerError {
+        StorageManagerError::SmeKeyInvalid
+    }
+}
+
 #[inline]
 #[allow(dead_code)]
 pub fn kata_storage_delete(key: &str) -> Result<(), StorageManagerError> {
     // NB: this assumes the StorageManager component is named "storage".
     extern "C" {
-        pub fn storage_delete(
+        pub fn storage_delete(c_key: *const cstr_core::c_char) -> StorageManagerError;
-            c_key: *const cstr_core::c_char
-        ) -> StorageManagerError;
     }
-    let cstr = CString::new(key)
+    let cstr = CString::new(key)?;
-        .map_err(|_| StorageManagerError::SmeKeyInvalid)?;
     match unsafe { storage_delete(cstr.as_ptr()) } {
         StorageManagerError::SmeSuccess => Ok(()),
         status => Err(status),
@@ -77,8 +104,7 @@ pub fn kata_storage_read(key: &str) -> Result<KeyValueData, StorageManagerError>
             c_raw_value: *mut KeyValueData,
         ) -> StorageManagerError;
     }
-    let cstr = CString::new(key)
+    let cstr = CString::new(key)?;
-        .map_err(|_| StorageManagerError::SmeKeyInvalid)?;
     let value = &mut [0u8; KEY_VALUE_DATA_SIZE];
     match unsafe { storage_read(cstr.as_ptr(), value as *mut _) } {
         StorageManagerError::SmeSuccess => Ok(*value),
@@ -96,8 +122,7 @@ pub fn kata_storage_write(key: &str, value: &[u8]) -> Result<(), StorageManagerE
             c_raw_value: *const u8,
         ) -> StorageManagerError;
     }
-    let cstr = CString::new(key)
+    let cstr = CString::new(key)?;
-        .map_err(|_| StorageManagerError::SmeKeyInvalid)?;
     match unsafe { storage_write(cstr.as_ptr(), value.len(), value.as_ptr()) } {
         StorageManagerError::SmeSuccess => Ok(()),
         status => Err(status),

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

@@ -4,7 +4,7 @@ version = "0.1.0"
 edition = "2018"
 
 [dependencies]
-kata-security-common = { path = "../../SecurityCoordinator/kata-security-common" }
+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

@@ -4,16 +4,22 @@
 
 extern crate alloc;
 use alloc::string::String;
-use kata_security_common::*;
+use kata_security_interface::kata_security_request;
-use kata_storage_interface::{KeyValueData, KEY_VALUE_DATA_SIZE};
+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{};
+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
@@ -23,11 +29,6 @@ impl StorageManagerInterface for KataStorageManager {
     fn read(&self, bundle_id: &str, key: &str) -> Result<KeyValueData, StorageError> {
         trace!("read bundle_id:{} key:{}", bundle_id, key);
 
-        fn serialize_failure(e: postcard::Error) -> StorageError {
-            trace!("read: serialize failure {:?}", e);
-            StorageError::SerializeFailed
-        }
-
         // Send request to Security Core via SecurityCoordinator
         let mut request = [0u8; SECURITY_REQUEST_DATA_SIZE];
         let _ = postcard::to_slice(
@@ -36,17 +37,13 @@ impl StorageManagerInterface for KataStorageManager {
                 key: String::from(key),
             },
             &mut request[..],
-        )
+        )?;
-        .map_err(serialize_failure)?;
         let result = &mut [0u8; SECURITY_REPLY_DATA_SIZE];
-        match kata_security_request(SecurityRequest::SrReadKey, &request, result) {
+        let _ = kata_security_request(SecurityRequest::SrReadKey, &request, result)?;
-            SecurityRequestError::SreSuccess => {
+        // NB: must copy into KeyValueData for now
-                let mut keyval = [0u8; KEY_VALUE_DATA_SIZE];
+        let mut keyval = [0u8; KEY_VALUE_DATA_SIZE];
-                keyval.copy_from_slice(&result[..KEY_VALUE_DATA_SIZE]);
+        keyval.copy_from_slice(&result[..KEY_VALUE_DATA_SIZE]);
-                Ok(keyval)
+        Ok(keyval)
-            }
-            e => Err(map_security_request_error(e, StorageError::ReadFailed)),
-        }
     }
     fn write(&self, bundle_id: &str, key: &str, value: &[u8]) -> Result<(), StorageError> {
         trace!(
@@ -56,11 +53,6 @@ impl StorageManagerInterface for KataStorageManager {
             value
         );
 
-        fn serialize_failure(e: postcard::Error) -> StorageError {
-            trace!("write: serialize failure {:?}", e);
-            StorageError::SerializeFailed
-        }
-
         // Send request to Security Core via SecurityCoordinator
         let mut request = [0u8; SECURITY_REQUEST_DATA_SIZE];
         let _ = postcard::to_slice(
@@ -70,22 +62,14 @@ impl StorageManagerInterface for KataStorageManager {
                 value: value,
             },
             &mut request[..],
-        )
+        )?;
-        .map_err(serialize_failure)?;
         let result = &mut [0u8; SECURITY_REPLY_DATA_SIZE];
-        match kata_security_request(SecurityRequest::SrWriteKey, &request, result) {
+        kata_security_request(SecurityRequest::SrWriteKey, &request, result)?;
-            SecurityRequestError::SreSuccess => Ok(()),
+        Ok(())
-            e => Err(map_security_request_error(e, StorageError::WriteFailed)),
-        }
     }
     fn delete(&self, bundle_id: &str, key: &str) -> Result<(), StorageError> {
         trace!("delete bundle_id:{} key:{}", bundle_id, key);
 
-        fn serialize_failure(e: postcard::Error) -> StorageError {
-            trace!("delete: serialize failure {:?}", e);
-            StorageError::SerializeFailed
-        }
-
         // Send request to Security Core via SecurityCoordinator
         let mut request = [0u8; SECURITY_REQUEST_DATA_SIZE];
         let _ = postcard::to_slice(
@@ -94,22 +78,9 @@ impl StorageManagerInterface for KataStorageManager {
                 key: String::from(key),
             },
             &mut request[..],
-        )
+        )?;
-        .map_err(serialize_failure)?;
         let result = &mut [0u8; SECURITY_REPLY_DATA_SIZE];
-        match kata_security_request(SecurityRequest::SrDeleteKey, &request, result) {
+        kata_security_request(SecurityRequest::SrDeleteKey, &request, result)?;
-            SecurityRequestError::SreSuccess => Ok(()),
+        Ok(())
-            e => Err(map_security_request_error(e, StorageError::DeleteFailed)),
-        }
-    }
-}
-
-// Maps a SecuritRequestError to a StorageError.
-fn map_security_request_error(sre: SecurityRequestError, def: StorageError) -> StorageError {
-    match sre {
-        SecurityRequestError::SreSuccess => StorageError::Success,
-        SecurityRequestError::SreBundleNotFound => StorageError::BundleNotFound,
-        SecurityRequestError::SreKeyNotFound => StorageError::KeyNotFound,
-        _ => def,
     }
 }