Merge "Secretkeeper: add message encryption" into main am: 57b7d0bc7b
Original change: https://android-review.googlesource.com/c/platform/hardware/interfaces/+/2847414
Change-Id: I4e3fc1dde1ad44c1f90e55d1d4a8f1be76abca7f
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/security/authgraph/aidl/vts/functional/sink.rs b/security/authgraph/aidl/vts/functional/sink.rs
index bb357b8..a331eef 100644
--- a/security/authgraph/aidl/vts/functional/sink.rs
+++ b/security/authgraph/aidl/vts/functional/sink.rs
@@ -29,11 +29,11 @@
}
/// Perform mainline AuthGraph key exchange with the provided sink and local implementation.
-/// Return the agreed AES keys in plaintext.
+/// Return the agreed AES keys in plaintext, together with the session ID.
pub fn test_mainline(
local_source: &mut ke::AuthGraphParticipant,
sink: binder::Strong<dyn IAuthGraphKeyExchange>,
-) -> [key::AesKey; 2] {
+) -> ([key::AesKey; 2], Vec<u8>) {
// Step 1: create an ephemeral ECDH key at the (local) source.
let source_init_info = local_source
.create()
@@ -113,7 +113,7 @@
Ok(array) => array,
Err(_) => panic!("wrong number of decrypted shared key arcs"),
};
- decrypted_shared_keys_array
+ (decrypted_shared_keys_array, sink_info.sessionId)
}
/// Perform mainline AuthGraph key exchange with the provided sink, but provide an invalid
diff --git a/security/authgraph/aidl/vts/functional/source.rs b/security/authgraph/aidl/vts/functional/source.rs
index a1e76b3..019e1e8 100644
--- a/security/authgraph/aidl/vts/functional/source.rs
+++ b/security/authgraph/aidl/vts/functional/source.rs
@@ -29,11 +29,11 @@
}
/// Perform mainline AuthGraph key exchange with the provided source.
-/// Return the agreed AES keys in plaintext.
+/// Return the agreed AES keys in plaintext, together with the session ID.
pub fn test_mainline(
local_sink: &mut ke::AuthGraphParticipant,
source: binder::Strong<dyn IAuthGraphKeyExchange>,
-) -> [key::AesKey; 2] {
+) -> ([key::AesKey; 2], Vec<u8>) {
// Step 1: create an ephemeral ECDH key at the (remote) source.
let source_init_info = source
.create()
@@ -120,7 +120,7 @@
Ok(array) => array,
Err(_) => panic!("wrong number of decrypted shared key arcs"),
};
- decrypted_shared_keys_array
+ (decrypted_shared_keys_array, source_info.sessionId)
}
/// Perform mainline AuthGraph key exchange with the provided source, but provide an invalid session
diff --git a/security/secretkeeper/aidl/aidl_api/android.hardware.security.secretkeeper/current/android/hardware/security/secretkeeper/ErrorCode.aidl b/security/secretkeeper/aidl/aidl_api/android.hardware.security.secretkeeper/current/android/hardware/security/secretkeeper/ErrorCode.aidl
new file mode 100644
index 0000000..cc07f9b
--- /dev/null
+++ b/security/secretkeeper/aidl/aidl_api/android.hardware.security.secretkeeper/current/android/hardware/security/secretkeeper/ErrorCode.aidl
@@ -0,0 +1,42 @@
+/*
+ * 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.
+ */
+///////////////////////////////////////////////////////////////////////////////
+// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. //
+///////////////////////////////////////////////////////////////////////////////
+
+// This file is a snapshot of an AIDL file. Do not edit it manually. There are
+// two cases:
+// 1). this is a frozen version file - do not edit this in any case.
+// 2). this is a 'current' file. If you make a backwards compatible change to
+// the interface (from the latest frozen version), the build system will
+// prompt you to update this file with `m <name>-update-api`.
+//
+// You must not make a backward incompatible change to any AIDL file built
+// with the aidl_interface module type with versions property set. The module
+// type is used to build AIDL files in a way that they can be used across
+// independently updatable components of the system. If a device is shipped
+// with such a backward incompatible change, it has a high risk of breaking
+// later when a module using the interface is updated, e.g., Mainline modules.
+
+package android.hardware.security.secretkeeper;
+/* @hide */
+@Backing(type="int") @VintfStability
+enum ErrorCode {
+ OK = 0,
+ UNKNOWN_KEY_ID = 1,
+ INTERNAL_ERROR = 2,
+ REQUEST_MALFORMED = 3,
+}
diff --git a/security/secretkeeper/aidl/android/hardware/security/secretkeeper/ErrorCode.aidl b/security/secretkeeper/aidl/android/hardware/security/secretkeeper/ErrorCode.aidl
new file mode 100644
index 0000000..e9cce09
--- /dev/null
+++ b/security/secretkeeper/aidl/android/hardware/security/secretkeeper/ErrorCode.aidl
@@ -0,0 +1,33 @@
+/*
+ * 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.
+ */
+
+package android.hardware.security.secretkeeper;
+
+/**
+ * Secretkeeper unencrypted error code, returned via AIDL as service specific errors in
+ * EX_SERVICE_SPECIFIC.
+ * @hide
+ */
+@VintfStability
+@Backing(type="int")
+enum ErrorCode {
+ OK = 0,
+ UNKNOWN_KEY_ID = 1,
+ INTERNAL_ERROR = 2,
+ REQUEST_MALFORMED = 3,
+
+ // TODO(b/291224769): Create a more exhaustive set of error code values.
+}
diff --git a/security/secretkeeper/aidl/android/hardware/security/secretkeeper/ISecretkeeper.aidl b/security/secretkeeper/aidl/android/hardware/security/secretkeeper/ISecretkeeper.aidl
index 1f4768a..cb3e9b9 100644
--- a/security/secretkeeper/aidl/android/hardware/security/secretkeeper/ISecretkeeper.aidl
+++ b/security/secretkeeper/aidl/android/hardware/security/secretkeeper/ISecretkeeper.aidl
@@ -35,7 +35,7 @@
* 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.
- * 2. 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 {
/**
@@ -60,7 +60,11 @@
* Virtual Machines). For this, service (& client) must implement a key exchange protocol, which
* is critical for establishing the secure channel.
*
+ * If an encrypted response cannot be generated, then a service-specific Binder error using an
+ * error code from ErrorCode.aidl will be returned.
+ *
* Secretkeeper database should guarantee the following properties:
+ *
* 1. Confidentiality: No entity (of security privilege lower than Secretkeeper) should
* be able to get a client's data in clear.
*
diff --git a/security/secretkeeper/aidl/android/hardware/security/secretkeeper/SecretManagement.cddl b/security/secretkeeper/aidl/android/hardware/security/secretkeeper/SecretManagement.cddl
index 62a1bd7..66ca8ed 100644
--- a/security/secretkeeper/aidl/android/hardware/security/secretkeeper/SecretManagement.cddl
+++ b/security/secretkeeper/aidl/android/hardware/security/secretkeeper/SecretManagement.cddl
@@ -1,110 +1,73 @@
; CDDL for the Secret Management API.
-; Also see processSecretManagementRequest method in ISecretkeeper.aidl
-; ProtectedRequestPacket is used by client for accessing Secret Management API
-; in Secretkeeper service. The service returns ProtectedResponsePacket of the corresponding type.
+; The input parameter to the `processSecretManagementRequest` operation in
+; `ISecretkeeper.aidl` is always an encrypted request message, CBOR-encoded as a
+; COSE_Encrypt0 object. The encryption uses the first of the keys agreed using
+; the associated AuthGraph instance, referred to as `KeySourceToSink`.
+ProtectedRequestPacket = CryptoPayload<RequestPacket, KeySourceToSink>
-; ProtectedRequestPacket & ProtectedResponsePacket are encrypted wrappers
-; on RequestPacket & ResponsePacket using symmetric keys agreed between Secretkeeper & clients
-; (these are referred to as KeySourceToSink & KeySinkToSource)
-;
-; The API operation required is encoded using 'Opcode', the arguments using 'Params'
-; and returned values as 'Result'.
-
-ProtectedRequestPacket =
- ProtectedGetVersionRequest / ProtectedStoreSecretRequest / ProtectedGetSecretRequest
-ProtectedResponsePacket =
- ProtectedGetVersionResponse / ProtectedStoreSecretResponse / ProtectedGetSecretResponse
-
-ProtectedGetVersionRequest = ProtectedRequestPacket<GetVersionRequestPacket>
-ProtectedGetVersionResponse = ProtectedResponsePacket<GetVersionResponsePacket>
-ProtectedStoreSecretRequest = ProtectedRequestPacket<StoreSecretRequestPacket>
-ProtectedStoreSecretResponse = ProtectedResponsePacket<StoreSecretResponsePacket>
-ProtectedGetSecretRequest = ProtectedRequestPacket<GetSecretRequestPacket>
-ProtectedGetSecretResponse = ProtectedResponsePacket<GetSecretResponsePacket>
-
-GetVersionRequestPacket = RequestPacket<GetVersionOpcode, GetVersionParams>
-GetVersionResponsePacket = ResponsePacket<GetVersionResult>
-StoreSecretRequestPacket = RequestPacket<StoreSecretOpcode, StoreSecretParams>
-StoreSecretResponsePacket = ResponsePacket<StoreSecretResult>
-GetSecretRequestPacket = RequestPacket<GetOpcode, GetSecretParams>
-GetSecretResponsePacket = ResponsePacket<GetSecretResult>
-
-RequestPacket<Opcode, Params> = [
- Opcode,
- Params
-]
-ResponsePacket<Result> = ResponsePacketError / ResponsePacketSuccess<Result>
-
-ResponsePacketSuccess = [
- 0, ; Indicates successful Response
- result : Result
-]
-ResponsePacketError = [
- error_code: ErrorCode, ; Indicate the error
- error_message: tstr ; Additional human-readable context
-]
-
-Opcode = &(
- GetVersionOpcode: 1, ; Get version of the SecretManagement API
- StoreSecretOpcode: 2, ; Store a secret
- GetSecretOpcode: 3, ; Get the secret
-)
-
-GetVersionParams = ()
-GetVersionResult = (version : uint)
-
-StoreSecretParams = (
- id : bstr .size 64 ; Unique identifier of the secret
- secret : bstr .size 32,
- sealing_policy : bstr .cbor DicePolicy, ; See DicePolicy.cddl for definition of DicePolicy
-)
-StoreSecretResult = ()
-
-GetSecretParams = (
- id : bstr .size 64 ; Unique identifier of the secret
- ; Use this to update the sealing policy associated with a secret during GetSecret operation.
- updated_sealing_policy : bstr .cbor DicePolicy / nil,
-)
-GetSecretResult = (secret : bstr .size 32)
-
-
-ProtectedRequestPacket<Payload, Key> = CryptoPayload<Payload, KeySourceToSink>
-ProtectedResponsePacket<Payload, Key> = ProtectedResponseError
- / ProtectedResponseSuccess<Payload>
-
-ProtectedResponseSuccess<Payload> = [
- 0, ; Indicates successful crypto operations. Note: Payload
- ; may contain Error from functional layer.
- message: CryptoPayload<Payload, KeySinkToSource> ; message is the encrypted payload
-]
-
-ProtectedResponseError = [
- error_code: CryptoErrorCode, ; Indicates the error. This is in cleartext & will be
- ; visible to Android. These are errors from crypto
- ; layer & indicates the request could not even be read
- message: tstr ; Additional human-readable context
-]
-
-CryptoPayload<Payload, Key> = [ ; COSE_Encrypt0 (untagged), [RFC 9052 s5.2]
+CryptoPayload<Payload, Key> = [ ; COSE_Encrypt0 (untagged), [RFC 9052 s5.2]
protected: bstr .cbor {
1 : 3, ; Algorithm: AES-GCM mode w/ 256-bit key, 128-bit tag
4 : bstr ; key identifier, uniquely identifies the session
; TODO(b/291228560): Refer to the Key Exchange spec.
},
unprotected: {
- 5 : bstr .size 12 ; IV
+ 5 : bstr .size 12 ; IV
},
- ciphertext : bstr ; AES-GCM-256(Key, bstr .cbor Payload)
- ; AAD for the encryption is CBOR-serialized
- ; Enc_structure (RFC 9052 s5.3) with empty external_aad.
+ ciphertext : bstr ; AES-GCM-256(Key, bstr .cbor Payload)
+ ; AAD for the encryption is CBOR-serialized
+ ; Enc_structure (RFC 9052 s5.3) with empty external_aad.
]
-; TODO(b/291224769): Create a more exhaustive set of CryptoErrorCode
-CryptoErrorCode = &(
- CryptoErrorCode_SessionExpired: 1,
+; Once decrypted, the request packet is an encoded CBOR array holding:
+; - An initial integer indicating which request is present.
+; - Subsequent objects holding the parameters for that specific request.
+RequestPacket =
+ [GetVersionOpcode, GetVersionParams] /
+ [StoreSecretOpcode, StoreSecretParams] /
+ [GetSecretOpcode, GetSecretParams]
+
+GetVersionOpcode = 1 ; Get version of the SecretManagement API
+StoreSecretOpcode = 2 ; Store a secret
+GetSecretOpcode = 3 ; Get the secret
+
+GetVersionParams = ()
+
+StoreSecretParams = (
+ id : SecretId,
+ secret : Secret,
+ sealing_policy : bstr .cbor DicePolicy,
)
+; INCLUDE DicePolicy.cddl for: DicePolicy
+
+GetSecretParams = (
+ id : SecretId,
+ ; Retrieving the value of a secret may optionally also update the sealing
+ ; policy associated with a secret.
+ updated_sealing_policy : bstr .cbor DicePolicy / nil,
+)
+
+SecretId = bstr .size 64 ; Unique identifier of the secret.
+Secret = bstr .size 32 ; The secret value.
+
+; The return value from a successful `processSecretManagementRequest` operation is a
+; response message encrypted with the second of the keys agreed using the associated
+; AuthGraph instance, referred to as `KeySinkToSource`.
+ProtectedResponsePacket = CryptoPayload<ResponsePacket, KeySinkToSource>
+
+; Once decrypted, the inner response message is encoded as a CBOR array holding:
+; - An initial integer return code value.
+; - Subsequently:
+; - If the return code is zero: result value(s).
+; - If the return code is non-zero: an error message.
+ResponsePacket =
+ [0, Result] /
+ [error_code: ErrorCode, error_message: tstr]
+
+; An error code in the inner response message indicates a failure in
+; secret management processing.
; TODO(b/291224769): Create a more exhaustive set of ErrorCodes
ErrorCode = &(
; Use this as if no other error code can be used.
@@ -119,4 +82,16 @@
ErrorCode_DicePolicyError: 5,
)
-; INCLUDE DicePolicy.cddl for: DicePolicy
\ No newline at end of file
+; The particular result variant present is determined by which request
+; message was originally sent.
+Result = &(
+ GetVersionResult,
+ StoreSecretResult,
+ GetSecretResult,
+)
+
+GetVersionResult = (version : uint)
+
+StoreSecretResult = ()
+
+GetSecretResult = (secret : Secret)
diff --git a/security/secretkeeper/aidl/vts/Android.bp b/security/secretkeeper/aidl/vts/Android.bp
index def5d5d..c130a3a 100644
--- a/security/secretkeeper/aidl/vts/Android.bp
+++ b/security/secretkeeper/aidl/vts/Android.bp
@@ -27,7 +27,9 @@
],
rustlibs: [
"libsecretkeeper_comm_nostd",
+ "libsecretkeeper_core_nostd",
"android.hardware.security.secretkeeper-V1-rust",
+ "libauthgraph_boringssl",
"libauthgraph_core",
"libcoset",
"libauthgraph_vts_test",
diff --git a/security/secretkeeper/aidl/vts/secretkeeper_test_client.rs b/security/secretkeeper/aidl/vts/secretkeeper_test_client.rs
index cc606b1..a473bd0 100644
--- a/security/secretkeeper/aidl/vts/secretkeeper_test_client.rs
+++ b/security/secretkeeper/aidl/vts/secretkeeper_test_client.rs
@@ -16,8 +16,9 @@
#[cfg(test)]
use binder::StatusCode;
-use coset::CborSerializable;
+use coset::{CborSerializable, CoseEncrypt0};
use log::warn;
+use secretkeeper_core::cipher;
use secretkeeper_comm::data_types::error::SecretkeeperError;
use secretkeeper_comm::data_types::request::Request;
use secretkeeper_comm::data_types::request_response_impl::{
@@ -28,6 +29,7 @@
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_boringssl as boring;
use authgraph_core::key;
const SECRETKEEPER_IDENTIFIER: &str =
@@ -73,7 +75,50 @@
}
}
}
-fn authgraph_key_exchange(sk: binder::Strong<dyn ISecretkeeper>) -> [key::AesKey; 2] {
+
+/// Secretkeeper client information.
+struct SkClient {
+ sk: binder::Strong<dyn ISecretkeeper>,
+ aes_keys: [key::AesKey; 2],
+ session_id: Vec<u8>,
+}
+
+impl SkClient {
+ fn new() -> Option<Self> {
+ let sk = get_connection()?;
+ let (aes_keys, session_id) = authgraph_key_exchange(sk.clone());
+ Some(Self {
+ sk,
+ aes_keys,
+ session_id,
+ })
+ }
+ /// Wrapper around `ISecretkeeper::processSecretManagementRequest` that handles
+ /// encryption and decryption.
+ fn secret_management_request(&self, req_data: &[u8]) -> Vec<u8> {
+ let aes_gcm = boring::BoringAes;
+ let rng = boring::BoringRng;
+ let request_bytes = cipher::encrypt_message(
+ &aes_gcm,
+ &rng,
+ &self.aes_keys[0],
+ &self.session_id,
+ &req_data,
+ )
+ .unwrap();
+
+ let response_bytes = self
+ .sk
+ .processSecretManagementRequest(&request_bytes)
+ .unwrap();
+
+ let response_encrypt0 = CoseEncrypt0::from_slice(&response_bytes).unwrap();
+ cipher::decrypt_message(&aes_gcm, &self.aes_keys[1], &response_encrypt0).unwrap()
+ }
+}
+
+/// Perform AuthGraph key exchange, returning the session keys and session ID.
+fn authgraph_key_exchange(sk: binder::Strong<dyn ISecretkeeper>) -> ([key::AesKey; 2], Vec<u8>) {
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)
@@ -90,7 +135,7 @@
return;
}
};
- let _aes_keys = authgraph_key_exchange(sk);
+ let (_aes_keys, _session_id) = authgraph_key_exchange(sk);
}
/// Test that the AuthGraph instance returned by SecretKeeper correctly rejects
@@ -130,23 +175,19 @@
#[test]
fn secret_management_get_version() {
- let secretkeeper = match get_connection() {
+ let sk_client = match SkClient::new() {
Some(sk) => sk,
None => {
warn!("Secretkeeper HAL is unavailable, skipping test");
return;
}
};
+
let request = GetVersionRequest {};
let request_packet = request.serialize_to_packet();
let request_bytes = request_packet.to_vec().unwrap();
- // TODO(b/291224769) The request will need to be encrypted & response need to be decrypted
- // with key & related artifacts pre-shared via Authgraph Key Exchange HAL.
-
- let response_bytes = secretkeeper
- .processSecretManagementRequest(&request_bytes)
- .unwrap();
+ let response_bytes = sk_client.secret_management_request(&request_bytes);
let response_packet = ResponsePacket::from_slice(&response_bytes).unwrap();
assert_eq!(
@@ -160,13 +201,14 @@
#[test]
fn secret_management_malformed_request() {
- let secretkeeper = match get_connection() {
+ let sk_client = match SkClient::new() {
Some(sk) => sk,
None => {
warn!("Secretkeeper HAL is unavailable, skipping test");
return;
}
};
+
let request = GetVersionRequest {};
let request_packet = request.serialize_to_packet();
let mut request_bytes = request_packet.to_vec().unwrap();
@@ -174,12 +216,7 @@
// Deform the request
request_bytes[0] = !request_bytes[0];
- // TODO(b/291224769) The request will need to be encrypted & response need to be decrypted
- // with key & related artifacts pre-shared via Authgraph Key Exchange HAL.
-
- let response_bytes = secretkeeper
- .processSecretManagementRequest(&request_bytes)
- .unwrap();
+ let response_bytes = sk_client.secret_management_request(&request_bytes);
let response_packet = ResponsePacket::from_slice(&response_bytes).unwrap();
assert_eq!(
@@ -192,7 +229,7 @@
#[test]
fn secret_management_store_get_secret_found() {
- let secretkeeper = match get_connection() {
+ let sk_client = match SkClient::new() {
Some(sk) => sk,
None => {
warn!("Secretkeeper HAL is unavailable, skipping test");
@@ -207,9 +244,8 @@
};
let store_request = store_request.serialize_to_packet().to_vec().unwrap();
- let store_response = secretkeeper
- .processSecretManagementRequest(&store_request)
- .unwrap();
+
+ let store_response = sk_client.secret_management_request(&store_request);
let store_response = ResponsePacket::from_slice(&store_response).unwrap();
assert_eq!(
@@ -226,9 +262,7 @@
};
let get_request = get_request.serialize_to_packet().to_vec().unwrap();
- let get_response = secretkeeper
- .processSecretManagementRequest(&get_request)
- .unwrap();
+ let get_response = sk_client.secret_management_request(&get_request);
let get_response = ResponsePacket::from_slice(&get_response).unwrap();
assert_eq!(get_response.response_type().unwrap(), ResponseType::Success);
let get_response = *GetSecretResponse::deserialize_from_packet(get_response).unwrap();
@@ -237,7 +271,7 @@
#[test]
fn secret_management_store_get_secret_not_found() {
- let secretkeeper = match get_connection() {
+ let sk_client = match SkClient::new() {
Some(sk) => sk,
None => {
warn!("Secretkeeper HAL is unavailable, skipping test");
@@ -253,9 +287,7 @@
};
let store_request = store_request.serialize_to_packet().to_vec().unwrap();
- let store_response = secretkeeper
- .processSecretManagementRequest(&store_request)
- .unwrap();
+ let store_response = sk_client.secret_management_request(&store_request);
let store_response = ResponsePacket::from_slice(&store_response).unwrap();
assert_eq!(
@@ -269,9 +301,7 @@
updated_sealing_policy: None,
};
let get_request = get_request.serialize_to_packet().to_vec().unwrap();
- let get_response = secretkeeper
- .processSecretManagementRequest(&get_request)
- .unwrap();
+ let get_response = sk_client.secret_management_request(&get_request);
// Check that response is `SecretkeeperError::EntryNotFound`
let get_response = ResponsePacket::from_slice(&get_response).unwrap();