Merge "secretkeeper: add fuzzer" into main am: a662feeee4 am: 5cb989cb52

Original change: https://android-review.googlesource.com/c/platform/hardware/interfaces/+/2880767

Change-Id: I0dd07bc39968f591a2160ff64fcc19ac68875574
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

security/secretkeeper/default/Android.bp

@@ -18,6 +18,28 @@ package {
     default_applicable_licenses: ["Android-Apache-2.0"],
 }
 
+rust_library {
+    name: "libsecretkeeper_nonsecure",
+    crate_name: "secretkeeper_nonsecure",
+    srcs: [
+        "src/lib.rs",
+    ],
+    vendor_available: true,
+    defaults: [
+        "authgraph_use_latest_hal_aidl_rust",
+    ],
+    rustlibs: [
+        "android.hardware.security.secretkeeper-V1-rust",
+        "libauthgraph_boringssl",
+        "libauthgraph_core",
+        "libauthgraph_hal",
+        "libbinder_rs",
+        "liblog_rust",
+        "libsecretkeeper_core_nostd",
+        "libsecretkeeper_comm_nostd",
+    ],
+}
+
 rust_binary {
     name: "android.hardware.security.secretkeeper-service.nonsecure",
     relative_install_path: "hw",
@@ -30,20 +52,34 @@ rust_binary {
     rustlibs: [
         "android.hardware.security.secretkeeper-V1-rust",
         "libandroid_logger",
-        "libauthgraph_boringssl",
-        "libauthgraph_core",
-        "libauthgraph_hal",
         "libbinder_rs",
         "liblog_rust",
-        "libsecretkeeper_comm_nostd",
-        "libsecretkeeper_core_nostd",
         "libsecretkeeper_hal",
+        "libsecretkeeper_nonsecure",
     ],
     srcs: [
         "src/main.rs",
     ],
 }
 
+rust_fuzz {
+    name: "android.hardware.security.secretkeeper-service.nonsecure_fuzzer",
+    rustlibs: [
+        "libsecretkeeper_hal",
+        "libsecretkeeper_nonsecure",
+        "libbinder_random_parcel_rs",
+        "libbinder_rs",
+    ],
+    srcs: ["src/fuzzer.rs"],
+    fuzz_config: {
+        cc: [
+            "alanstokes@google.com",
+            "drysdale@google.com",
+            "shikhapanwar@google.com",
+        ],
+    },
+}
+
 prebuilt_etc {
     name: "secretkeeper.rc",
     src: "secretkeeper.rc",

security/secretkeeper/default/src/fuzzer.rs

@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#![allow(missing_docs)]
+#![no_main]
+extern crate libfuzzer_sys;
+
+use binder_random_parcel_rs::fuzz_service;
+use libfuzzer_sys::fuzz_target;
+use secretkeeper_hal::SecretkeeperService;
+use secretkeeper_nonsecure::{AuthGraphChannel, LocalTa, SecretkeeperChannel};
+use std::sync::{Arc, Mutex};
+
+fuzz_target!(|data: &[u8]| {
+    let ta = Arc::new(Mutex::new(LocalTa::new()));
+    let ag_channel = AuthGraphChannel(ta.clone());
+    let sk_channel = SecretkeeperChannel(ta.clone());
+
+    let service = SecretkeeperService::new_as_binder(sk_channel, ag_channel);
+    fuzz_service(&mut service.as_binder(), data);
+});

security/secretkeeper/default/src/lib.rs

@@ -0,0 +1,132 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+//! Non-secure implementation of a local Secretkeeper TA.
+
+use authgraph_boringssl as boring;
+use authgraph_core::keyexchange::{AuthGraphParticipant, MAX_OPENED_SESSIONS};
+use authgraph_core::ta::{AuthGraphTa, Role};
+use authgraph_hal::channel::SerializedChannel;
+use log::error;
+use secretkeeper_core::ta::SecretkeeperTa;
+use std::cell::RefCell;
+use std::rc::Rc;
+use std::sync::mpsc;
+use std::sync::{Arc, Mutex};
+
+mod store;
+
+/// Implementation of the Secrekeeper TA that runs locally in-process (and which is therefore
+/// insecure).
+pub struct LocalTa {
+    in_tx: mpsc::Sender<Vec<u8>>,
+    out_rx: mpsc::Receiver<Vec<u8>>,
+}
+
+/// Prefix byte for messages intended for the AuthGraph TA.
+const AG_MESSAGE_PREFIX: u8 = 0x00;
+/// Prefix byte for messages intended for the Secretkeeper TA.
+const SK_MESSAGE_PREFIX: u8 = 0x01;
+
+impl LocalTa {
+    /// Create a new instance.
+    pub fn new() -> Self {
+        // Create a pair of channels to communicate with the TA thread.
+        let (in_tx, in_rx) = mpsc::channel();
+        let (out_tx, out_rx) = mpsc::channel();
+
+        // The TA code expects to run single threaded, so spawn a thread to run it in.
+        std::thread::spawn(move || {
+            let mut crypto_impls = boring::crypto_trait_impls();
+            let storage_impl = Box::new(store::InMemoryStore::default());
+            let sk_ta = Rc::new(RefCell::new(
+                SecretkeeperTa::new(&mut crypto_impls, storage_impl)
+                    .expect("Failed to create local Secretkeeper TA"),
+            ));
+            let mut ag_ta = AuthGraphTa::new(
+                AuthGraphParticipant::new(crypto_impls, sk_ta.clone(), MAX_OPENED_SESSIONS)
+                    .expect("Failed to create local AuthGraph TA"),
+                Role::Sink,
+            );
+
+            // Loop forever processing request messages.
+            loop {
+                let req_data: Vec<u8> = match in_rx.recv() {
+                    Ok(data) => data,
+                    Err(_) => {
+                        error!("local TA failed to receive request!");
+                        break;
+                    }
+                };
+                let rsp_data = match req_data[0] {
+                    AG_MESSAGE_PREFIX => ag_ta.process(&req_data[1..]),
+                    SK_MESSAGE_PREFIX => {
+                        // It's safe to `borrow_mut()` because this code is not a callback
+                        // from AuthGraph (the only other holder of an `Rc`), and so there
+                        // can be no live `borrow()`s in this (single) thread.
+                        sk_ta.borrow_mut().process(&req_data[1..])
+                    }
+                    prefix => panic!("unexpected messageprefix {prefix}!"),
+                };
+                match out_tx.send(rsp_data) {
+                    Ok(_) => {}
+                    Err(_) => {
+                        error!("local TA failed to send out response");
+                        break;
+                    }
+                }
+            }
+            error!("local TA terminating!");
+        });
+        Self { in_tx, out_rx }
+    }
+
+    fn execute_for(&mut self, prefix: u8, req_data: &[u8]) -> Vec<u8> {
+        let mut prefixed_req = Vec::with_capacity(req_data.len() + 1);
+        prefixed_req.push(prefix);
+        prefixed_req.extend_from_slice(req_data);
+        self.in_tx
+            .send(prefixed_req)
+            .expect("failed to send in request");
+        self.out_rx.recv().expect("failed to receive response")
+    }
+}
+
+pub struct AuthGraphChannel(pub Arc<Mutex<LocalTa>>);
+
+impl SerializedChannel for AuthGraphChannel {
+    const MAX_SIZE: usize = usize::MAX;
+    fn execute(&self, req_data: &[u8]) -> binder::Result<Vec<u8>> {
+        Ok(self
+            .0
+            .lock()
+            .unwrap()
+            .execute_for(AG_MESSAGE_PREFIX, req_data))
+    }
+}
+
+pub struct SecretkeeperChannel(pub Arc<Mutex<LocalTa>>);
+
+impl SerializedChannel for SecretkeeperChannel {
+    const MAX_SIZE: usize = usize::MAX;
+    fn execute(&self, req_data: &[u8]) -> binder::Result<Vec<u8>> {
+        Ok(self
+            .0
+            .lock()
+            .unwrap()
+            .execute_for(SK_MESSAGE_PREFIX, req_data))
+    }
+}

security/secretkeeper/default/src/main.rs

@@ -15,112 +15,14 @@
  */
 
 //! Non-secure implementation of the Secretkeeper HAL.
-mod store;
 
-use authgraph_boringssl as boring;
-use authgraph_core::keyexchange::{AuthGraphParticipant, MAX_OPENED_SESSIONS};
-use authgraph_core::ta::{AuthGraphTa, Role};
-use authgraph_hal::channel::SerializedChannel;
 use log::{error, info, Level};
-use secretkeeper_core::ta::SecretkeeperTa;
 use secretkeeper_hal::SecretkeeperService;
-use std::sync::Arc;
+use secretkeeper_nonsecure::{AuthGraphChannel, SecretkeeperChannel, LocalTa};
-use std::sync::Mutex;
+use std::sync::{Arc, Mutex};
-use store::InMemoryStore;
-
 use android_hardware_security_secretkeeper::aidl::android::hardware::security::secretkeeper::ISecretkeeper::{
     BpSecretkeeper, ISecretkeeper,
 };
-use std::cell::RefCell;
-use std::rc::Rc;
-use std::sync::mpsc;
-
-/// Implementation of the Secrekeeper TA that runs locally in-process (and which is therefore
-/// insecure).
-pub struct LocalTa {
-    in_tx: mpsc::Sender<Vec<u8>>,
-    out_rx: mpsc::Receiver<Vec<u8>>,
-}
-
-/// Prefix byte for messages intended for the AuthGraph TA.
-const AG_MESSAGE_PREFIX: u8 = 0x00;
-/// Prefix byte for messages intended for the Secretkeeper TA.
-const SK_MESSAGE_PREFIX: u8 = 0x01;
-
-impl LocalTa {
-    /// Create a new instance.
-    pub fn new() -> Self {
-        // Create a pair of channels to communicate with the TA thread.
-        let (in_tx, in_rx) = mpsc::channel();
-        let (out_tx, out_rx) = mpsc::channel();
-
-        // The TA code expects to run single threaded, so spawn a thread to run it in.
-        std::thread::spawn(move || {
-            let mut crypto_impls = boring::crypto_trait_impls();
-            let storage_impl = Box::new(InMemoryStore::default());
-            let sk_ta = Rc::new(RefCell::new(
-                SecretkeeperTa::new(&mut crypto_impls, storage_impl)
-                    .expect("Failed to create local Secretkeeper TA"),
-            ));
-            let mut ag_ta = AuthGraphTa::new(
-                AuthGraphParticipant::new(crypto_impls, sk_ta.clone(), MAX_OPENED_SESSIONS)
-                    .expect("Failed to create local AuthGraph TA"),
-                Role::Sink,
-            );
-
-            // Loop forever processing request messages.
-            loop {
-                let req_data: Vec<u8> = in_rx.recv().expect("failed to receive next req");
-                let rsp_data = match req_data[0] {
-                    AG_MESSAGE_PREFIX => ag_ta.process(&req_data[1..]),
-                    SK_MESSAGE_PREFIX => {
-                        // It's safe to `borrow_mut()` because this code is not a callback
-                        // from AuthGraph (the only other holder of an `Rc`), and so there
-                        // can be no live `borrow()`s in this (single) thread.
-                        sk_ta.borrow_mut().process(&req_data[1..])
-                    }
-                    prefix => panic!("unexpected messageprefix {prefix}!"),
-                };
-                out_tx.send(rsp_data).expect("failed to send out rsp");
-            }
-        });
-        Self { in_tx, out_rx }
-    }
-
-    fn execute_for(&mut self, prefix: u8, req_data: &[u8]) -> Vec<u8> {
-        let mut prefixed_req = Vec::with_capacity(req_data.len() + 1);
-        prefixed_req.push(prefix);
-        prefixed_req.extend_from_slice(req_data);
-        self.in_tx
-            .send(prefixed_req)
-            .expect("failed to send in request");
-        self.out_rx.recv().expect("failed to receive response")
-    }
-}
-
-pub struct AuthGraphChannel(Arc<Mutex<LocalTa>>);
-impl SerializedChannel for AuthGraphChannel {
-    const MAX_SIZE: usize = usize::MAX;
-    fn execute(&self, req_data: &[u8]) -> binder::Result<Vec<u8>> {
-        Ok(self
-            .0
-            .lock()
-            .unwrap()
-            .execute_for(AG_MESSAGE_PREFIX, req_data))
-    }
-}
-
-pub struct SecretkeeperChannel(Arc<Mutex<LocalTa>>);
-impl SerializedChannel for SecretkeeperChannel {
-    const MAX_SIZE: usize = usize::MAX;
-    fn execute(&self, req_data: &[u8]) -> binder::Result<Vec<u8>> {
-        Ok(self
-            .0
-            .lock()
-            .unwrap()
-            .execute_for(SK_MESSAGE_PREFIX, req_data))
-    }
-}
 
 fn main() {
     // Initialize Android logging.

security/secretkeeper/default/src/store.rs

@@ -13,12 +13,15 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
+//! In-memory store for nonsecure Secretkeeper.
+
 use secretkeeper_comm::data_types::error::Error;
 use secretkeeper_core::store::KeyValueStore;
 use std::collections::HashMap;
 
-/// An in-memory implementation of KeyValueStore. Please note that this is entirely for
+/// An in-memory implementation of [`KeyValueStore`]. Please note that this is entirely for testing
-/// testing purposes. Refer to the documentation of `PolicyGatedStorage` & Secretkeeper HAL for
+/// purposes. Refer to the documentation of `PolicyGatedStorage` and Secretkeeper HAL for
 /// persistence requirements.
 #[derive(Default)]
 pub struct InMemoryStore(HashMap<Vec<u8>, Vec<u8>>);