Merge "Secretkeeper: add AuthGraph key exchange" into main
diff --git a/security/authgraph/aidl/Android.bp b/security/authgraph/aidl/Android.bp
index d94f640..f3d1281 100644
--- a/security/authgraph/aidl/Android.bp
+++ b/security/authgraph/aidl/Android.bp
@@ -34,7 +34,7 @@
platform_apis: true,
},
ndk: {
- apps_enabled: false,
+ enabled: true,
},
rust: {
enabled: true,
diff --git a/security/authgraph/aidl/vts/functional/lib.rs b/security/authgraph/aidl/vts/functional/lib.rs
index da3fa1c..f4c1da9 100644
--- a/security/authgraph/aidl/vts/functional/lib.rs
+++ b/security/authgraph/aidl/vts/functional/lib.rs
@@ -26,6 +26,7 @@
use authgraph_boringssl as boring;
use authgraph_core::{error::Error as AgError, keyexchange as ke};
use coset::CborSerializable;
+use std::{cell::RefCell, rc::Rc};
pub mod sink;
pub mod source;
@@ -34,7 +35,7 @@
pub fn test_ag_participant() -> Result<ke::AuthGraphParticipant, AgError> {
Ok(ke::AuthGraphParticipant::new(
boring::crypto_trait_impls(),
- Box::<boring::test_device::AgDevice>::default(),
+ Rc::new(RefCell::new(boring::test_device::AgDevice::default())),
ke::MAX_OPENED_SESSIONS,
)?)
}
diff --git a/security/authgraph/default/src/fuzzer.rs b/security/authgraph/default/src/fuzzer.rs
index d401777..387d72f 100644
--- a/security/authgraph/default/src/fuzzer.rs
+++ b/security/authgraph/default/src/fuzzer.rs
@@ -22,10 +22,9 @@
use authgraph_nonsecure::LocalTa;
use binder_random_parcel_rs::fuzz_service;
use libfuzzer_sys::fuzz_target;
-use std::sync::{Arc, Mutex};
fuzz_target!(|data: &[u8]| {
let local_ta = LocalTa::new().expect("Failed to create an AuthGraph local TA.");
- let service = AuthGraphService::new_as_binder(Arc::new(Mutex::new(local_ta)));
+ let service = AuthGraphService::new_as_binder(local_ta);
fuzz_service(&mut service.as_binder(), data);
});
diff --git a/security/authgraph/default/src/lib.rs b/security/authgraph/default/src/lib.rs
index 14741aa..1f851b2 100644
--- a/security/authgraph/default/src/lib.rs
+++ b/security/authgraph/default/src/lib.rs
@@ -22,36 +22,62 @@
ta::{AuthGraphTa, Role},
};
use authgraph_hal::channel::SerializedChannel;
-use std::sync::{Arc, Mutex};
+use std::cell::RefCell;
+use std::rc::Rc;
+use std::sync::{mpsc, Mutex};
/// Implementation of the AuthGraph TA that runs locally in-process (and which is therefore
/// insecure).
pub struct LocalTa {
- ta: Arc<Mutex<AuthGraphTa>>,
+ channels: Mutex<Channels>,
+}
+
+struct Channels {
+ in_tx: mpsc::Sender<Vec<u8>>,
+ out_rx: mpsc::Receiver<Vec<u8>>,
}
impl LocalTa {
/// Create a new instance.
pub fn new() -> Result<Self, error::Error> {
- Ok(Self {
- ta: Arc::new(Mutex::new(AuthGraphTa::new(
+ // 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 ta = AuthGraphTa::new(
keyexchange::AuthGraphParticipant::new(
boring::crypto_trait_impls(),
- Box::<boring::test_device::AgDevice>::default(),
+ Rc::new(RefCell::new(boring::test_device::AgDevice::default())),
keyexchange::MAX_OPENED_SESSIONS,
- )?,
+ )
+ .expect("failed to create AG participant"),
Role::Both,
- ))),
+ );
+ // Loop forever processing request messages.
+ loop {
+ let req_data: Vec<u8> = in_rx.recv().expect("failed to receive next req");
+ let rsp_data = ta.process(&req_data);
+ out_tx.send(rsp_data).expect("failed to send out rsp");
+ }
+ });
+ Ok(Self {
+ channels: Mutex::new(Channels { in_tx, out_rx }),
})
}
}
-/// Pretend to be a serialized channel to the TA, but actually just directly invoke the TA with
-/// incoming requests.
impl SerializedChannel for LocalTa {
const MAX_SIZE: usize = usize::MAX;
- fn execute(&mut self, req_data: &[u8]) -> binder::Result<Vec<u8>> {
- Ok(self.ta.lock().unwrap().process(req_data))
+ fn execute(&self, req_data: &[u8]) -> binder::Result<Vec<u8>> {
+ // Serialize across both request and response.
+ let channels = self.channels.lock().unwrap();
+ channels
+ .in_tx
+ .send(req_data.to_vec())
+ .expect("failed to send in request");
+ Ok(channels.out_rx.recv().expect("failed to receive response"))
}
}
diff --git a/security/authgraph/default/src/main.rs b/security/authgraph/default/src/main.rs
index 81f2dd6..ced7567 100644
--- a/security/authgraph/default/src/main.rs
+++ b/security/authgraph/default/src/main.rs
@@ -25,7 +25,6 @@
use authgraph_hal::service;
use authgraph_nonsecure::LocalTa;
use log::{error, info};
-use std::sync::{Arc, Mutex};
static SERVICE_NAME: &str = "android.hardware.security.authgraph.IAuthGraphKeyExchange";
static SERVICE_INSTANCE: &str = "nonsecure";
@@ -65,9 +64,8 @@
binder::ProcessState::start_thread_pool();
// Register the service
- let local_ta =
- LocalTa::new().map_err(|e| format!("Failed to create the TA because: {e:?}"))?;
- let service = service::AuthGraphService::new_as_binder(Arc::new(Mutex::new(local_ta)));
+ let local_ta = LocalTa::new().map_err(|e| format!("Failed to create the TA because: {e:?}"))?;
+ let service = service::AuthGraphService::new_as_binder(local_ta);
let service_name = format!("{}/{}", SERVICE_NAME, SERVICE_INSTANCE);
binder::add_service(&service_name, service.as_binder()).map_err(|e| {
format!(
diff --git a/security/secretkeeper/aidl/Android.bp b/security/secretkeeper/aidl/Android.bp
index c77d299..ac923ca 100644
--- a/security/secretkeeper/aidl/Android.bp
+++ b/security/secretkeeper/aidl/Android.bp
@@ -20,8 +20,15 @@
name: "android.hardware.security.secretkeeper",
vendor_available: true,
srcs: ["android/hardware/security/secretkeeper/*.aidl"],
+ imports: [
+ "android.hardware.security.authgraph-V1",
+ ],
stability: "vintf",
+ frozen: false,
backend: {
+ java: {
+ enabled: false,
+ },
ndk: {
enabled: true,
},
@@ -34,3 +41,44 @@
},
},
}
+
+// cc_defaults that includes the latest Secretkeeper AIDL library.
+// Modules that depend on Secretkeeper directly can include this cc_defaults to avoid
+// managing dependency versions explicitly.
+cc_defaults {
+ name: "secretkeeper_use_latest_hal_aidl_ndk_static",
+ static_libs: [
+ "android.hardware.security.secretkeeper-V1-ndk",
+ ],
+}
+
+cc_defaults {
+ name: "secretkeeper_use_latest_hal_aidl_ndk_shared",
+ shared_libs: [
+ "android.hardware.security.secretkeeper-V1-ndk",
+ ],
+}
+
+cc_defaults {
+ name: "secretkeeper_use_latest_hal_aidl_cpp_static",
+ static_libs: [
+ "android.hardware.security.secretkeeper-V1-cpp",
+ ],
+}
+
+cc_defaults {
+ name: "secretkeeper_use_latest_hal_aidl_cpp_shared",
+ shared_libs: [
+ "android.hardware.security.secretkeeper-V1-cpp",
+ ],
+}
+
+// A rust_defaults that includes the latest Secretkeeper AIDL library.
+// Modules that depend on Secretkeeper directly can include this rust_defaults to avoid
+// managing dependency versions explicitly.
+rust_defaults {
+ name: "secretkeeper_use_latest_hal_aidl_rust",
+ rustlibs: [
+ "android.hardware.security.secretkeeper-V1-rust",
+ ],
+}
diff --git a/security/secretkeeper/aidl/aidl_api/android.hardware.security.secretkeeper/current/android/hardware/security/secretkeeper/ISecretkeeper.aidl b/security/secretkeeper/aidl/aidl_api/android.hardware.security.secretkeeper/current/android/hardware/security/secretkeeper/ISecretkeeper.aidl
index 2eb33c5..023fc8f 100644
--- a/security/secretkeeper/aidl/aidl_api/android.hardware.security.secretkeeper/current/android/hardware/security/secretkeeper/ISecretkeeper.aidl
+++ b/security/secretkeeper/aidl/aidl_api/android.hardware.security.secretkeeper/current/android/hardware/security/secretkeeper/ISecretkeeper.aidl
@@ -34,5 +34,6 @@
package android.hardware.security.secretkeeper;
@VintfStability
interface ISecretkeeper {
+ android.hardware.security.authgraph.IAuthGraphKeyExchange getAuthGraphKe();
byte[] processSecretManagementRequest(in byte[] request);
}
diff --git a/security/secretkeeper/aidl/android/hardware/security/secretkeeper/ISecretkeeper.aidl b/security/secretkeeper/aidl/android/hardware/security/secretkeeper/ISecretkeeper.aidl
index af715a9..1f4768a 100644
--- a/security/secretkeeper/aidl/android/hardware/security/secretkeeper/ISecretkeeper.aidl
+++ b/security/secretkeeper/aidl/android/hardware/security/secretkeeper/ISecretkeeper.aidl
@@ -16,6 +16,8 @@
package android.hardware.security.secretkeeper;
+import android.hardware.security.authgraph.IAuthGraphKeyExchange;
+
@VintfStability
/**
* Secretkeeper service definition.
@@ -29,17 +31,22 @@
* - A completely separate, purpose-built and certified secure CPU.
*
* TODO(b/291224769): Extend the HAL interface to include:
- * 1. Session setup api: This is used to perform cryptographic operations that allow shared keys to
- * be exchanged between session participants, typically (but not necessarily) a pVM instance and
- * Secretkeeper. This session setup is based on public key cryptography.
- * 2. Dice policy operation - These allow sealing of the secrets with a class of Dice chains.
+ * 1. Dice policy operation - These allow sealing of the secrets with a class of Dice chains.
* Typical operations are (securely) updating the dice policy sealing the Secrets above. These
* operations are core to AntiRollback protected secrets - ie, ensuring secrets of a pVM are only
* accessible to same or higher versions of the images.
- * 3. Maintenance api: This is required for removing the Secretkeeper entries for obsolete pvMs.
+ * 2. Maintenance api: This is required for removing the Secretkeeper entries for obsolete pvMs.
*/
interface ISecretkeeper {
/**
+ * Retrieve the instance of the `IAuthGraphKeyExchange` HAL that should be used for shared
+ * session key establishment. These keys are used to perform encryption of messages as
+ * described in SecretManagement.cddl, allowing the client and Secretkeeper to have a
+ * cryptographically secure channel.
+ */
+ IAuthGraphKeyExchange getAuthGraphKe();
+
+ /**
* processSecretManagementRequest method is used for interacting with the Secret Management API
*
* Secret Management API: The clients can use this API to store (& get) 32 bytes of data.
diff --git a/security/secretkeeper/aidl/vts/Android.bp b/security/secretkeeper/aidl/vts/Android.bp
index 6818298..fac16f6 100644
--- a/security/secretkeeper/aidl/vts/Android.bp
+++ b/security/secretkeeper/aidl/vts/Android.bp
@@ -28,6 +28,8 @@
rustlibs: [
"libsecretkeeper_comm_nostd",
"android.hardware.security.secretkeeper-V1-rust",
+ "libauthgraph_core",
+ "libauthgraph_vts_test",
"libbinder_rs",
"liblog_rust",
],
diff --git a/security/secretkeeper/aidl/vts/secretkeeper_test_client.rs b/security/secretkeeper/aidl/vts/secretkeeper_test_client.rs
index 28923f7..70f5da6 100644
--- a/security/secretkeeper/aidl/vts/secretkeeper_test_client.rs
+++ b/security/secretkeeper/aidl/vts/secretkeeper_test_client.rs
@@ -25,6 +25,8 @@
use secretkeeper_comm::data_types::response::Response;
use secretkeeper_comm::data_types::packet::{ResponsePacket, ResponseType};
use android_hardware_security_secretkeeper::aidl::android::hardware::security::secretkeeper::ISecretkeeper::ISecretkeeper;
+use authgraph_vts_test as ag_vts;
+use authgraph_core::key;
const SECRETKEEPER_IDENTIFIER: &str =
"android.hardware.security.secretkeeper.ISecretkeeper/nonsecure";
@@ -42,6 +44,57 @@
}
}
}
+fn authgraph_key_exchange(sk: binder::Strong<dyn ISecretkeeper>) -> [key::AesKey; 2] {
+ let sink = sk.getAuthGraphKe().expect("failed to get AuthGraph");
+ let mut source = ag_vts::test_ag_participant().expect("failed to create a local source");
+ ag_vts::sink::test_mainline(&mut source, sink)
+}
+
+/// Test that the AuthGraph instance returned by SecretKeeper correctly performs
+/// mainline key exchange against a local source implementation.
+#[test]
+fn authgraph_mainline() {
+ let sk = match get_connection() {
+ Some(sk) => sk,
+ None => {
+ warn!("Secretkeeper HAL is unavailable, skipping test");
+ return;
+ }
+ };
+ let _aes_keys = authgraph_key_exchange(sk);
+}
+
+/// Test that the AuthGraph instance returned by SecretKeeper correctly rejects
+/// a corrupted session ID signature.
+#[test]
+fn authgraph_corrupt_sig() {
+ let sk = match get_connection() {
+ Some(sk) => sk,
+ None => {
+ warn!("Secretkeeper HAL is unavailable, skipping test");
+ return;
+ }
+ };
+ let sink = sk.getAuthGraphKe().expect("failed to get AuthGraph");
+ let mut source = ag_vts::test_ag_participant().expect("failed to create a local source");
+ ag_vts::sink::test_corrupt_sig(&mut source, sink);
+}
+
+/// Test that the AuthGraph instance returned by SecretKeeper correctly detects
+/// when corrupted keys are returned to it.
+#[test]
+fn authgraph_corrupt_keys() {
+ let sk = match get_connection() {
+ Some(sk) => sk,
+ None => {
+ warn!("Secretkeeper HAL is unavailable, skipping test");
+ return;
+ }
+ };
+ let sink = sk.getAuthGraphKe().expect("failed to get AuthGraph");
+ let mut source = ag_vts::test_ag_participant().expect("failed to create a local source");
+ ag_vts::sink::test_corrupt_keys(&mut source, sink);
+}
// TODO(b/2797757): Add tests that match different HAL defined objects (like request/response)
// with expected bytes.
diff --git a/security/secretkeeper/default/Android.bp b/security/secretkeeper/default/Android.bp
index 8240b89..a920c6e 100644
--- a/security/secretkeeper/default/Android.bp
+++ b/security/secretkeeper/default/Android.bp
@@ -24,12 +24,19 @@
vendor: true,
installable: false, // install APEX
prefer_rlib: true,
+ defaults: [
+ "authgraph_use_latest_hal_aidl_rust",
+ ],
rustlibs: [
"android.hardware.security.secretkeeper-V1-rust",
"libandroid_logger",
+ "libauthgraph_boringssl",
+ "libauthgraph_core",
+ "libauthgraph_hal",
"libbinder_rs",
"liblog_rust",
"libsecretkeeper_comm_nostd",
+ "libsecretkeeper_hal",
],
srcs: [
"src/main.rs",
diff --git a/security/secretkeeper/default/src/main.rs b/security/secretkeeper/default/src/main.rs
index 2d367c5..4f9302d 100644
--- a/security/secretkeeper/default/src/main.rs
+++ b/security/secretkeeper/default/src/main.rs
@@ -14,80 +14,106 @@
* limitations under the License.
*/
-use binder::{BinderFeatures, Interface};
+//! Non-secure implementation of the Secretkeeper HAL.
+
use log::{error, info, Level};
-use secretkeeper_comm::data_types::error::SecretkeeperError;
-use secretkeeper_comm::data_types::packet::{RequestPacket, ResponsePacket};
-use secretkeeper_comm::data_types::request::Request;
-use secretkeeper_comm::data_types::request_response_impl::{
- GetVersionRequest, GetVersionResponse, Opcode,
-};
-use secretkeeper_comm::data_types::response::Response;
-
+use std::sync::{Arc, Mutex};
+use authgraph_boringssl as boring;
+use authgraph_core::ta::{Role, AuthGraphTa};
+use authgraph_core::keyexchange::{MAX_OPENED_SESSIONS, AuthGraphParticipant};
+use secretkeeper_comm::ta::SecretkeeperTa;
+use secretkeeper_hal::SecretkeeperService;
+use authgraph_hal::channel::SerializedChannel;
use android_hardware_security_secretkeeper::aidl::android::hardware::security::secretkeeper::ISecretkeeper::{
- BnSecretkeeper, BpSecretkeeper, ISecretkeeper,
+ ISecretkeeper, BpSecretkeeper,
};
+use std::cell::RefCell;
+use std::rc::Rc;
+use std::sync::mpsc;
-const CURRENT_VERSION: u64 = 1;
+/// 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>>,
+}
-#[derive(Debug, Default)]
-pub struct NonSecureSecretkeeper;
+/// 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 Interface for NonSecureSecretkeeper {}
+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();
-impl ISecretkeeper for NonSecureSecretkeeper {
- fn processSecretManagementRequest(&self, request: &[u8]) -> binder::Result<Vec<u8>> {
- Ok(self.process_opaque_request(request))
+ // 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 sk_ta = Rc::new(RefCell::new(
+ SecretkeeperTa::new(&mut crypto_impls)
+ .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")
}
}
-impl NonSecureSecretkeeper {
- // A set of requests to Secretkeeper are 'opaque' - encrypted bytes with inner structure
- // described by CDDL. They need to be decrypted, deserialized and processed accordingly.
- fn process_opaque_request(&self, request: &[u8]) -> Vec<u8> {
- // TODO(b/291224769) The request will need to be decrypted & response need to be encrypted
- // with key & related artifacts pre-shared via Authgraph Key Exchange HAL.
- self.process_opaque_request_unhandled_error(request)
- .unwrap_or_else(
- // SecretkeeperError is also a valid 'Response', serialize to a response packet.
- |sk_err| {
- Response::serialize_to_packet(&sk_err)
- .into_bytes()
- .expect("Panicking due to serialization failing")
- },
- )
+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))
}
+}
- fn process_opaque_request_unhandled_error(
- &self,
- request: &[u8],
- ) -> Result<Vec<u8>, SecretkeeperError> {
- let request_packet = RequestPacket::from_bytes(request).map_err(|e| {
- error!("Failed to get Request packet from bytes: {:?}", e);
- SecretkeeperError::RequestMalformed
- })?;
- let response_packet = match request_packet
- .opcode()
- .map_err(|_| SecretkeeperError::RequestMalformed)?
- {
- Opcode::GetVersion => Self::process_get_version_request(request_packet)?,
- _ => todo!("TODO(b/291224769): Unimplemented operations"),
- };
-
- response_packet
- .into_bytes()
- .map_err(|_| SecretkeeperError::UnexpectedServerError)
- }
-
- fn process_get_version_request(
- request: RequestPacket,
- ) -> Result<ResponsePacket, SecretkeeperError> {
- // Deserialization really just verifies the structural integrity of the request such
- // as args being empty.
- let _request = GetVersionRequest::deserialize_from_packet(request)
- .map_err(|_| SecretkeeperError::RequestMalformed)?;
- let response = GetVersionResponse::new(CURRENT_VERSION);
- Ok(response.serialize_to_packet())
+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))
}
}
@@ -104,17 +130,17 @@
error!("{}", panic_info);
}));
- let service = NonSecureSecretkeeper::default();
- let service_binder = BnSecretkeeper::new_binder(service, BinderFeatures::default());
+ 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);
let service_name = format!(
"{}/nonsecure",
<BpSecretkeeper as ISecretkeeper>::get_descriptor()
);
- binder::add_service(&service_name, service_binder.as_binder()).unwrap_or_else(|e| {
- panic!(
- "Failed to register service {} because of {:?}.",
- service_name, e
- );
+ binder::add_service(&service_name, service.as_binder()).unwrap_or_else(|e| {
+ panic!("Failed to register service {service_name} because of {e:?}.",);
});
info!("Registered Binder service, joining threadpool.");
binder::ProcessState::join_thread_pool();