From 1357b92dbc687a43fbb702cd818ac806110769e6 Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Wed, 13 Dec 2023 07:38:57 +0000 Subject: [PATCH] [Secretkeeper] Add maintenance methods Also move error codes from separate `ErrorCode.aidl` file to be inline ERROR_ constants instead. Bug: 291224769 Test: VtsSecretkeeperTargetTest Change-Id: I1b0f3f3b5a7c5e891da3022444bf6c7925850550 --- .../security/secretkeeper/ISecretkeeper.aidl | 5 + .../{ErrorCode.aidl => SecretId.aidl} | 9 +- .../security/secretkeeper/ISecretkeeper.aidl | 30 ++- .../{ErrorCode.aidl => SecretId.aidl} | 16 +- .../secretkeeper/SecretManagement.cddl | 11 +- .../aidl/vts/secretkeeper_test_client.rs | 182 ++++++++++++++++++ security/secretkeeper/default/src/store.rs | 10 + 7 files changed, 236 insertions(+), 27 deletions(-) rename security/secretkeeper/aidl/aidl_api/android.hardware.security.secretkeeper/current/android/hardware/security/secretkeeper/{ErrorCode.aidl => SecretId.aidl} (93%) rename security/secretkeeper/aidl/android/hardware/security/secretkeeper/{ErrorCode.aidl => SecretId.aidl} (69%) 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 023fc8f97f..8ce37cd558 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 @@ -36,4 +36,9 @@ package android.hardware.security.secretkeeper; interface ISecretkeeper { android.hardware.security.authgraph.IAuthGraphKeyExchange getAuthGraphKe(); byte[] processSecretManagementRequest(in byte[] request); + void deleteIds(in android.hardware.security.secretkeeper.SecretId[] ids); + void deleteAll(); + const int ERROR_UNKNOWN_KEY_ID = 1; + const int ERROR_INTERNAL_ERROR = 2; + const int ERROR_REQUEST_MALFORMED = 3; } 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/SecretId.aidl similarity index 93% rename from security/secretkeeper/aidl/aidl_api/android.hardware.security.secretkeeper/current/android/hardware/security/secretkeeper/ErrorCode.aidl rename to security/secretkeeper/aidl/aidl_api/android.hardware.security.secretkeeper/current/android/hardware/security/secretkeeper/SecretId.aidl index cc07f9b3fa..87d0233d03 100644 --- 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/SecretId.aidl @@ -33,10 +33,7 @@ package android.hardware.security.secretkeeper; /* @hide */ -@Backing(type="int") @VintfStability -enum ErrorCode { - OK = 0, - UNKNOWN_KEY_ID = 1, - INTERNAL_ERROR = 2, - REQUEST_MALFORMED = 3, +@VintfStability +parcelable SecretId { + byte[] id; } diff --git a/security/secretkeeper/aidl/android/hardware/security/secretkeeper/ISecretkeeper.aidl b/security/secretkeeper/aidl/android/hardware/security/secretkeeper/ISecretkeeper.aidl index cb3e9b978a..49c3446265 100644 --- a/security/secretkeeper/aidl/android/hardware/security/secretkeeper/ISecretkeeper.aidl +++ b/security/secretkeeper/aidl/android/hardware/security/secretkeeper/ISecretkeeper.aidl @@ -17,6 +17,7 @@ package android.hardware.security.secretkeeper; import android.hardware.security.authgraph.IAuthGraphKeyExchange; +import android.hardware.security.secretkeeper.SecretId; @VintfStability /** @@ -30,14 +31,12 @@ import android.hardware.security.authgraph.IAuthGraphKeyExchange; * - A trusted execution environment such as ARM TrustZone. * - A completely separate, purpose-built and certified secure CPU. * - * TODO(b/291224769): Extend the HAL interface to include: - * 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. - * 2. Maintenance API: This is required for removing the Secretkeeper entries for obsolete pVMs. */ interface ISecretkeeper { + const int ERROR_UNKNOWN_KEY_ID = 1; + const int ERROR_INTERNAL_ERROR = 2; + const int ERROR_REQUEST_MALFORMED = 3; + /** * 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 @@ -60,8 +59,8 @@ interface ISecretkeeper { * 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. + * If an encrypted response cannot be generated, then a service-specific Binder error using one + * of the ERROR_ codes above will be returned. * * Secretkeeper database should guarantee the following properties: * @@ -82,4 +81,19 @@ interface ISecretkeeper { * @return CBOR-encoded ProtectedResponsePacket. See SecretManagement.cddl for its definition */ byte[] processSecretManagementRequest(in byte[] request); + + /** + * Delete the data corresponding to a collection of IDs. + * + * Note that unlike `processSecretManagementRequest`, the contents of this method are in + * plaintext, and no client authentication is required. + * + * @param Secret identifiers to delete. + */ + void deleteIds(in SecretId[] ids); + + /** + * Delete data of all clients. + */ + void deleteAll(); } diff --git a/security/secretkeeper/aidl/android/hardware/security/secretkeeper/ErrorCode.aidl b/security/secretkeeper/aidl/android/hardware/security/secretkeeper/SecretId.aidl similarity index 69% rename from security/secretkeeper/aidl/android/hardware/security/secretkeeper/ErrorCode.aidl rename to security/secretkeeper/aidl/android/hardware/security/secretkeeper/SecretId.aidl index e9cce09f9f..bd982e7c69 100644 --- a/security/secretkeeper/aidl/android/hardware/security/secretkeeper/ErrorCode.aidl +++ b/security/secretkeeper/aidl/android/hardware/security/secretkeeper/SecretId.aidl @@ -17,17 +17,13 @@ package android.hardware.security.secretkeeper; /** - * Secretkeeper unencrypted error code, returned via AIDL as service specific errors in - * EX_SERVICE_SPECIFIC. + * SecretId contains an identifier for a secret held by Secretkeeper. * @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. +parcelable SecretId { + /** + * 64-byte identifier for a secret. + */ + byte[] id; } diff --git a/security/secretkeeper/aidl/android/hardware/security/secretkeeper/SecretManagement.cddl b/security/secretkeeper/aidl/android/hardware/security/secretkeeper/SecretManagement.cddl index 66ca8ed6ba..3d080789f1 100644 --- a/security/secretkeeper/aidl/android/hardware/security/secretkeeper/SecretManagement.cddl +++ b/security/secretkeeper/aidl/android/hardware/security/secretkeeper/SecretManagement.cddl @@ -9,8 +9,8 @@ ProtectedRequestPacket = CryptoPayload CryptoPayload = [ ; 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. + 4 : bstr ; key identifier set to session ID produced + ; by AuthGraph key exchange. }, unprotected: { 5 : bstr .size 12 ; IV @@ -32,8 +32,11 @@ GetVersionOpcode = 1 ; Get version of the SecretManagement API StoreSecretOpcode = 2 ; Store a secret GetSecretOpcode = 3 ; Get the secret +; Retrieve Secretkeeper version. GetVersionParams = () +; Store a secret identified by the given ID, with access to the secret policed +; by the associated sealing policy. StoreSecretParams = ( id : SecretId, secret : Secret, @@ -42,6 +45,9 @@ StoreSecretParams = ( ; INCLUDE DicePolicy.cddl for: DicePolicy +; Retrieve a secret identified by the given ID, policed according to the sealing +; policy that was associated with the secret. If successful, optionally also +; update the sealing policy for the secret. GetSecretParams = ( id : SecretId, ; Retrieving the value of a secret may optionally also update the sealing @@ -68,7 +74,6 @@ ResponsePacket = ; 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. ErrorCode_UnexpectedServerError: 1, diff --git a/security/secretkeeper/aidl/vts/secretkeeper_test_client.rs b/security/secretkeeper/aidl/vts/secretkeeper_test_client.rs index a473bd0eba..b5cca275b2 100644 --- a/security/secretkeeper/aidl/vts/secretkeeper_test_client.rs +++ b/security/secretkeeper/aidl/vts/secretkeeper_test_client.rs @@ -28,6 +28,7 @@ use secretkeeper_comm::data_types::{Id, ID_SIZE, Secret, SECRET_SIZE}; 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 android_hardware_security_secretkeeper::aidl::android::hardware::security::secretkeeper::SecretId::SecretId; use authgraph_vts_test as ag_vts; use authgraph_boringssl as boring; use authgraph_core::key; @@ -52,6 +53,12 @@ const ID_EXAMPLE: [u8; ID_SIZE] = [ 0xCC, 0x24, 0xFD, 0xBF, 0x91, 0x4A, 0x54, 0x84, 0xF9, 0x01, 0x59, 0x25, 0x70, 0x89, 0x38, 0x8D, 0x5E, 0xE6, 0x91, 0xDF, 0x68, 0x60, 0x69, 0x26, 0xBE, 0xFE, 0x79, 0x58, 0xF7, 0xEA, 0x81, 0x7D, ]; +const ID_EXAMPLE_2: [u8; ID_SIZE] = [ + 0x6A, 0xCC, 0xB1, 0xEB, 0xBB, 0xAB, 0xE3, 0xEA, 0x44, 0xBD, 0xDC, 0x75, 0x75, 0x7D, 0xC0, 0xE5, + 0xC7, 0x86, 0x41, 0x56, 0x39, 0x66, 0x96, 0x10, 0xCB, 0x43, 0x10, 0x79, 0x03, 0xDC, 0xE6, 0x9F, + 0x12, 0x2B, 0xEF, 0x28, 0x9C, 0x1E, 0x32, 0x46, 0x5F, 0xA3, 0xE7, 0x8D, 0x53, 0x63, 0xE8, 0x30, + 0x5A, 0x17, 0x6F, 0xEF, 0x42, 0xD6, 0x58, 0x7A, 0xF0, 0xCB, 0xD4, 0x40, 0x58, 0x96, 0x32, 0xF4, +]; const ID_NOT_STORED: [u8; ID_SIZE] = [ 0x56, 0xD0, 0x4E, 0xAA, 0xC1, 0x7B, 0x55, 0x6B, 0xA0, 0x2C, 0x65, 0x43, 0x39, 0x0A, 0x6C, 0xE9, 0x1F, 0xD0, 0x0E, 0x20, 0x3E, 0xFB, 0xF5, 0xF9, 0x3F, 0x5B, 0x11, 0x1B, 0x18, 0x73, 0xF6, 0xBB, @@ -294,6 +301,181 @@ fn secret_management_store_get_secret_not_found() { store_response.response_type().unwrap(), ResponseType::Success ); + // Really just checking that the response is indeed StoreSecretResponse + let _ = StoreSecretResponse::deserialize_from_packet(store_response).unwrap(); + + // Get the secret that was never stored + let get_request = GetSecretRequest { + id: Id(ID_NOT_STORED), + updated_sealing_policy: None, + }; + let get_request = get_request.serialize_to_packet().to_vec().unwrap(); + + let get_response = sk_client.secret_management_request(&get_request); + + // Expect the entry not to be found. + let get_response = ResponsePacket::from_slice(&get_response).unwrap(); + assert_eq!(get_response.response_type().unwrap(), ResponseType::Error); + let err = *SecretkeeperError::deserialize_from_packet(get_response).unwrap(); + assert_eq!(err, SecretkeeperError::EntryNotFound); +} + +#[test] +fn secretkeeper_store_delete_ids() { + let sk_client = match SkClient::new() { + Some(sk) => sk, + None => { + warn!("Secretkeeper HAL is unavailable, skipping test"); + return; + } + }; + + let store_request = StoreSecretRequest { + id: Id(ID_EXAMPLE), + secret: Secret(SECRET_EXAMPLE), + sealing_policy: HYPOTHETICAL_DICE_POLICY.to_vec(), + }; + + let store_request = store_request.serialize_to_packet().to_vec().unwrap(); + let store_response = sk_client.secret_management_request(&store_request); + let store_response = ResponsePacket::from_slice(&store_response).unwrap(); + + assert_eq!( + store_response.response_type().unwrap(), + ResponseType::Success + ); + // Really just checking that the response is indeed StoreSecretResponse + let _ = StoreSecretResponse::deserialize_from_packet(store_response).unwrap(); + + sk_client + .sk + .deleteIds(&[SecretId { + id: ID_EXAMPLE.to_vec(), + }]) + .unwrap(); + + // Try to get the secret that was just stored & deleted + let get_request = GetSecretRequest { + id: Id(ID_EXAMPLE), + updated_sealing_policy: None, + }; + let get_request = get_request.serialize_to_packet().to_vec().unwrap(); + + let get_response = sk_client.secret_management_request(&get_request); + + // Expect the entry not to be found. + let get_response = ResponsePacket::from_slice(&get_response).unwrap(); + assert_eq!(get_response.response_type().unwrap(), ResponseType::Error); + let err = *SecretkeeperError::deserialize_from_packet(get_response).unwrap(); + assert_eq!(err, SecretkeeperError::EntryNotFound); +} + +#[test] +fn secretkeeper_store_delete_all() { + let sk_client = match SkClient::new() { + Some(sk) => sk, + None => { + warn!("Secretkeeper HAL is unavailable, skipping test"); + return; + } + }; + + let store_request = StoreSecretRequest { + id: Id(ID_EXAMPLE), + secret: Secret(SECRET_EXAMPLE), + sealing_policy: HYPOTHETICAL_DICE_POLICY.to_vec(), + }; + + let store_request = store_request.serialize_to_packet().to_vec().unwrap(); + let store_response = sk_client.secret_management_request(&store_request); + let store_response = ResponsePacket::from_slice(&store_response).unwrap(); + + assert_eq!( + store_response.response_type().unwrap(), + ResponseType::Success + ); + // Really just checking that the response is indeed StoreSecretResponse + let _ = StoreSecretResponse::deserialize_from_packet(store_response).unwrap(); + + let store_request = StoreSecretRequest { + id: Id(ID_EXAMPLE_2), + secret: Secret(SECRET_EXAMPLE), + sealing_policy: HYPOTHETICAL_DICE_POLICY.to_vec(), + }; + + let store_request = store_request.serialize_to_packet().to_vec().unwrap(); + let store_response = sk_client.secret_management_request(&store_request); + let store_response = ResponsePacket::from_slice(&store_response).unwrap(); + + assert_eq!( + store_response.response_type().unwrap(), + ResponseType::Success + ); + // Really just checking that the response is indeed StoreSecretResponse + let _ = StoreSecretResponse::deserialize_from_packet(store_response).unwrap(); + + sk_client.sk.deleteAll().unwrap(); + + // Get the first secret that was just stored before deleteAll + let get_request = GetSecretRequest { + id: Id(ID_EXAMPLE), + updated_sealing_policy: None, + }; + let get_request = get_request.serialize_to_packet().to_vec().unwrap(); + + let get_response = sk_client.secret_management_request(&get_request); + + // Expect the entry not to be found. + let get_response = ResponsePacket::from_slice(&get_response).unwrap(); + assert_eq!(get_response.response_type().unwrap(), ResponseType::Error); + let err = *SecretkeeperError::deserialize_from_packet(get_response).unwrap(); + assert_eq!(err, SecretkeeperError::EntryNotFound); + + // Get the second secret that was just stored before deleteAll + let get_request = GetSecretRequest { + id: Id(ID_EXAMPLE_2), + updated_sealing_policy: None, + }; + let get_request = get_request.serialize_to_packet().to_vec().unwrap(); + + let get_response = sk_client.secret_management_request(&get_request); + + // Expect the entry not to be found. + let get_response = ResponsePacket::from_slice(&get_response).unwrap(); + assert_eq!(get_response.response_type().unwrap(), ResponseType::Error); + let err = *SecretkeeperError::deserialize_from_packet(get_response).unwrap(); + assert_eq!(err, SecretkeeperError::EntryNotFound); + + // Store a new secret (corresponding to an id). + let store_request = StoreSecretRequest { + id: Id(ID_EXAMPLE), + secret: Secret(SECRET_EXAMPLE), + sealing_policy: HYPOTHETICAL_DICE_POLICY.to_vec(), + }; + + let store_request = store_request.serialize_to_packet().to_vec().unwrap(); + let store_response = sk_client.secret_management_request(&store_request); + let store_response = ResponsePacket::from_slice(&store_response).unwrap(); + + assert_eq!( + store_response.response_type().unwrap(), + ResponseType::Success + ); + + // Get the restored secret. + let get_request = GetSecretRequest { + id: Id(ID_EXAMPLE), + updated_sealing_policy: None, + }; + let get_request = get_request.serialize_to_packet().to_vec().unwrap(); + + let get_response = sk_client.secret_management_request(&get_request); + let get_response = ResponsePacket::from_slice(&get_response).unwrap(); + + // Get the secret that was just re-stored. + assert_eq!(get_response.response_type().unwrap(), ResponseType::Success); + let get_response = *GetSecretResponse::deserialize_from_packet(get_response).unwrap(); + assert_eq!(get_response.secret.0, SECRET_EXAMPLE); // (Try to) Get the secret that was never stored let get_request = GetSecretRequest { diff --git a/security/secretkeeper/default/src/store.rs b/security/secretkeeper/default/src/store.rs index 7b2d0b94d6..a7fb3b7454 100644 --- a/security/secretkeeper/default/src/store.rs +++ b/security/secretkeeper/default/src/store.rs @@ -33,4 +33,14 @@ impl KeyValueStore for InMemoryStore { let optional_val = self.0.get(key); Ok(optional_val.cloned()) } + + fn delete(&mut self, key: &[u8]) -> Result<(), Error> { + self.0.remove(key); + Ok(()) + } + + fn delete_all(&mut self) -> Result<(), Error> { + self.0.clear(); + Ok(()) + } }