diff --git a/security/secretkeeper/aidl/vts/secretkeeper_test_client.rs b/security/secretkeeper/aidl/vts/secretkeeper_test_client.rs index a62546d779..4c0f659960 100644 --- a/security/secretkeeper/aidl/vts/secretkeeper_test_client.rs +++ b/security/secretkeeper/aidl/vts/secretkeeper_test_client.rs @@ -113,8 +113,8 @@ impl SkClient { /// This method is wrapper that use `SkSession::secret_management_request` which handles /// encryption and decryption. - fn secret_management_request(&mut self, req_data: &[u8]) -> Vec { - self.session.secret_management_request(req_data).unwrap() + fn secret_management_request(&mut self, req_data: &[u8]) -> Result, Error> { + Ok(self.session.secret_management_request(req_data)?) } /// Unlike the method [`secret_management_request`], this method directly uses @@ -125,7 +125,7 @@ impl SkClient { req_data: &[u8], req_aad: &[u8], expected_res_aad: &[u8], - ) -> Vec { + ) -> Result, Error> { let aes_gcm = boring::BoringAes; let rng = boring::BoringRng; let request_bytes = cipher::encrypt_message( @@ -136,53 +136,54 @@ impl SkClient { &req_data, req_aad, ) - .unwrap(); + .map_err(|e| secretkeeper_client::Error::CipherError(e))?; // Binder call! - let response_bytes = self.sk.processSecretManagementRequest(&request_bytes).unwrap(); + let response_bytes = self.sk.processSecretManagementRequest(&request_bytes)?; - let response_encrypt0 = CoseEncrypt0::from_slice(&response_bytes).unwrap(); - cipher::decrypt_message( + let response_encrypt0 = CoseEncrypt0::from_slice(&response_bytes)?; + Ok(cipher::decrypt_message( &aes_gcm, self.session.decryption_key(), &response_encrypt0, expected_res_aad, ) - .unwrap() + .map_err(|e| secretkeeper_client::Error::CipherError(e))?) } /// Helper method to store a secret. This uses the default compatible sealing_policy on /// dice_chain. - fn store(&mut self, id: &Id, secret: &Secret) { - let sealing_policy = sealing_policy(self.dice_artifacts.explicit_key_dice_chain().unwrap()); + fn store(&mut self, id: &Id, secret: &Secret) -> Result<(), Error> { + let sealing_policy = sealing_policy( + self.dice_artifacts.explicit_key_dice_chain().ok_or(Error::UnexpectedError)?, + ); let store_request = StoreSecretRequest { id: id.clone(), secret: secret.clone(), sealing_policy }; - let store_request = store_request.serialize_to_packet().to_vec().unwrap(); + let store_request = store_request.serialize_to_packet().to_vec()?; - let store_response = self.secret_management_request(&store_request); - let store_response = ResponsePacket::from_slice(&store_response).unwrap(); + let store_response = self.secret_management_request(&store_request)?; + let store_response = ResponsePacket::from_slice(&store_response)?; - assert_eq!(store_response.response_type().unwrap(), ResponseType::Success); + assert_eq!(store_response.response_type()?, ResponseType::Success); // Really just checking that the response is indeed StoreSecretResponse - let _ = StoreSecretResponse::deserialize_from_packet(store_response).unwrap(); + let _ = StoreSecretResponse::deserialize_from_packet(store_response)?; + Ok(()) } /// Helper method to get a secret. - fn get(&mut self, id: &Id) -> Option { + fn get(&mut self, id: &Id) -> Result { let get_request = GetSecretRequest { id: id.clone(), updated_sealing_policy: None }; - let get_request = get_request.serialize_to_packet().to_vec().unwrap(); + let get_request = get_request.serialize_to_packet().to_vec()?; - let get_response = self.secret_management_request(&get_request); - let get_response = ResponsePacket::from_slice(&get_response).unwrap(); + let get_response = self.secret_management_request(&get_request)?; + let get_response = ResponsePacket::from_slice(&get_response)?; - if get_response.response_type().unwrap() == ResponseType::Success { - let get_response = *GetSecretResponse::deserialize_from_packet(get_response).unwrap(); - Some(Secret(get_response.secret.0)) + if get_response.response_type()? == ResponseType::Success { + let get_response = *GetSecretResponse::deserialize_from_packet(get_response)?; + Ok(Secret(get_response.secret.0)) } else { - // Only expect a not-found failure. - let err = *SecretkeeperError::deserialize_from_packet(get_response).unwrap(); - assert_eq!(err, SecretkeeperError::EntryNotFound); - None + let err = *SecretkeeperError::deserialize_from_packet(get_response)?; + Err(Error::SecretkeeperError(err)) } } @@ -198,6 +199,50 @@ impl SkClient { } } +#[derive(Debug)] +enum Error { + // Errors from Secretkeeper API errors. These are thrown by core SecretManagement and + // not visible without decryption. + SecretkeeperError(SecretkeeperError), + InfraError(secretkeeper_client::Error), + UnexpectedError, +} + +impl From for Error { + fn from(e: secretkeeper_client::Error) -> Self { + Self::InfraError(e) + } +} + +impl From for Error { + fn from(e: SecretkeeperError) -> Self { + Self::SecretkeeperError(e) + } +} + +impl From for Error { + fn from(e: coset::CoseError) -> Self { + Self::InfraError(secretkeeper_client::Error::from(e)) + } +} + +impl From for Error { + fn from(s: binder::Status) -> Self { + Self::InfraError(secretkeeper_client::Error::from(s)) + } +} + +impl From for Error { + fn from(e: secretkeeper_comm::data_types::error::Error) -> Self { + Self::InfraError(secretkeeper_client::Error::from(e)) + } +} + +// Assert that the error is EntryNotFound +fn assert_entry_not_found(res: Result) { + assert!(matches!(res.unwrap_err(), Error::SecretkeeperError(SecretkeeperError::EntryNotFound))) +} + /// Construct a sealing policy on the dice chain. This method uses the following set of /// constraints which are compatible with sample DICE chains used in VTS. /// 1. ExactMatch on AUTHORITY_HASH (non-optional). @@ -272,7 +317,7 @@ fn secret_management_get_version(instance: String) { let request_packet = request.serialize_to_packet(); let request_bytes = request_packet.to_vec().unwrap(); - let response_bytes = sk_client.secret_management_request(&request_bytes); + let response_bytes = sk_client.secret_management_request(&request_bytes).unwrap(); let response_packet = ResponsePacket::from_slice(&response_bytes).unwrap(); assert_eq!(response_packet.response_type().unwrap(), ResponseType::Success); @@ -292,7 +337,7 @@ fn secret_management_malformed_request(instance: String) { // Deform the request request_bytes[0] = !request_bytes[0]; - let response_bytes = sk_client.secret_management_request(&request_bytes); + let response_bytes = sk_client.secret_management_request(&request_bytes).unwrap(); let response_packet = ResponsePacket::from_slice(&response_bytes).unwrap(); assert_eq!(response_packet.response_type().unwrap(), ResponseType::Error); @@ -304,10 +349,10 @@ fn secret_management_malformed_request(instance: String) { fn secret_management_store_get_secret_found(instance: String) { let mut sk_client = SkClient::new(&instance); - sk_client.store(&ID_EXAMPLE, &SECRET_EXAMPLE); + sk_client.store(&ID_EXAMPLE, &SECRET_EXAMPLE).unwrap(); // Get the secret that was just stored - assert_eq!(sk_client.get(&ID_EXAMPLE), Some(SECRET_EXAMPLE)); + assert_eq!(sk_client.get(&ID_EXAMPLE).unwrap(), SECRET_EXAMPLE); } #[rdroidtest(get_instances())] @@ -315,64 +360,63 @@ fn secret_management_store_get_secret_not_found(instance: String) { let mut sk_client = SkClient::new(&instance); // Store a secret (corresponding to an id). - sk_client.store(&ID_EXAMPLE, &SECRET_EXAMPLE); + sk_client.store(&ID_EXAMPLE, &SECRET_EXAMPLE).unwrap(); // Get the secret that was never stored - assert_eq!(sk_client.get(&ID_NOT_STORED), None); + assert_entry_not_found(sk_client.get(&ID_NOT_STORED)); } #[rdroidtest(get_instances())] fn secretkeeper_store_delete_ids(instance: String) { let mut sk_client = SkClient::new(&instance); - sk_client.store(&ID_EXAMPLE, &SECRET_EXAMPLE); - sk_client.store(&ID_EXAMPLE_2, &SECRET_EXAMPLE); + sk_client.store(&ID_EXAMPLE, &SECRET_EXAMPLE).unwrap(); + sk_client.store(&ID_EXAMPLE_2, &SECRET_EXAMPLE).unwrap(); sk_client.delete(&[&ID_EXAMPLE]); - - assert_eq!(sk_client.get(&ID_EXAMPLE), None); - assert_eq!(sk_client.get(&ID_EXAMPLE_2), Some(SECRET_EXAMPLE)); + assert_entry_not_found(sk_client.get(&ID_EXAMPLE)); + assert_eq!(sk_client.get(&ID_EXAMPLE_2).unwrap(), SECRET_EXAMPLE); sk_client.delete(&[&ID_EXAMPLE_2]); - assert_eq!(sk_client.get(&ID_EXAMPLE), None); - assert_eq!(sk_client.get(&ID_EXAMPLE_2), None); + assert_entry_not_found(sk_client.get(&ID_EXAMPLE)); + assert_entry_not_found(sk_client.get(&ID_EXAMPLE_2)); } #[rdroidtest(get_instances())] fn secretkeeper_store_delete_multiple_ids(instance: String) { let mut sk_client = SkClient::new(&instance); - sk_client.store(&ID_EXAMPLE, &SECRET_EXAMPLE); - sk_client.store(&ID_EXAMPLE_2, &SECRET_EXAMPLE); + sk_client.store(&ID_EXAMPLE, &SECRET_EXAMPLE).unwrap(); + sk_client.store(&ID_EXAMPLE_2, &SECRET_EXAMPLE).unwrap(); sk_client.delete(&[&ID_EXAMPLE, &ID_EXAMPLE_2]); - assert_eq!(sk_client.get(&ID_EXAMPLE), None); - assert_eq!(sk_client.get(&ID_EXAMPLE_2), None); + assert_entry_not_found(sk_client.get(&ID_EXAMPLE)); + assert_entry_not_found(sk_client.get(&ID_EXAMPLE_2)); } #[rdroidtest(get_instances())] fn secretkeeper_store_delete_duplicate_ids(instance: String) { let mut sk_client = SkClient::new(&instance); - sk_client.store(&ID_EXAMPLE, &SECRET_EXAMPLE); - sk_client.store(&ID_EXAMPLE_2, &SECRET_EXAMPLE); + sk_client.store(&ID_EXAMPLE, &SECRET_EXAMPLE).unwrap(); + sk_client.store(&ID_EXAMPLE_2, &SECRET_EXAMPLE).unwrap(); // Delete the same secret twice. sk_client.delete(&[&ID_EXAMPLE, &ID_EXAMPLE]); - assert_eq!(sk_client.get(&ID_EXAMPLE), None); - assert_eq!(sk_client.get(&ID_EXAMPLE_2), Some(SECRET_EXAMPLE)); + assert_entry_not_found(sk_client.get(&ID_EXAMPLE)); + assert_eq!(sk_client.get(&ID_EXAMPLE_2).unwrap(), SECRET_EXAMPLE); } #[rdroidtest(get_instances())] fn secretkeeper_store_delete_nonexistent(instance: String) { let mut sk_client = SkClient::new(&instance); - sk_client.store(&ID_EXAMPLE, &SECRET_EXAMPLE); - sk_client.store(&ID_EXAMPLE_2, &SECRET_EXAMPLE); + sk_client.store(&ID_EXAMPLE, &SECRET_EXAMPLE).unwrap(); + sk_client.store(&ID_EXAMPLE_2, &SECRET_EXAMPLE).unwrap(); sk_client.delete(&[&ID_NOT_STORED]); - assert_eq!(sk_client.get(&ID_EXAMPLE), Some(SECRET_EXAMPLE)); - assert_eq!(sk_client.get(&ID_EXAMPLE_2), Some(SECRET_EXAMPLE)); - assert_eq!(sk_client.get(&ID_NOT_STORED), None); + assert_eq!(sk_client.get(&ID_EXAMPLE).unwrap(), SECRET_EXAMPLE); + assert_eq!(sk_client.get(&ID_EXAMPLE_2).unwrap(), SECRET_EXAMPLE); + assert_entry_not_found(sk_client.get(&ID_NOT_STORED)); } // Don't run deleteAll() on a secure device, as it might affect real secrets. @@ -381,22 +425,22 @@ fn secretkeeper_store_delete_nonexistent(instance: String) { fn secretkeeper_store_delete_all(instance: String) { let mut sk_client = SkClient::new(&instance); - sk_client.store(&ID_EXAMPLE, &SECRET_EXAMPLE); - sk_client.store(&ID_EXAMPLE_2, &SECRET_EXAMPLE); + sk_client.store(&ID_EXAMPLE, &SECRET_EXAMPLE).unwrap(); + sk_client.store(&ID_EXAMPLE_2, &SECRET_EXAMPLE).unwrap(); sk_client.delete_all(); - assert_eq!(sk_client.get(&ID_EXAMPLE), None); - assert_eq!(sk_client.get(&ID_EXAMPLE_2), None); + assert_entry_not_found(sk_client.get(&ID_EXAMPLE)); + assert_entry_not_found(sk_client.get(&ID_EXAMPLE_2)); // Store a new secret (corresponding to an id). - sk_client.store(&ID_EXAMPLE, &SECRET_EXAMPLE); + sk_client.store(&ID_EXAMPLE, &SECRET_EXAMPLE).unwrap(); // Get the restored secret. - assert_eq!(sk_client.get(&ID_EXAMPLE), Some(SECRET_EXAMPLE)); + assert_eq!(sk_client.get(&ID_EXAMPLE).unwrap(), SECRET_EXAMPLE); // (Try to) Get the secret that was never stored - assert_eq!(sk_client.get(&ID_NOT_STORED), None); + assert_entry_not_found(sk_client.get(&ID_NOT_STORED)); } // This test checks that Secretkeeper uses the expected [`RequestSeqNum`] as aad while @@ -416,21 +460,21 @@ fn secret_management_replay_protection_seq_num(instance: String) { // Check first request/response is successful let res = ResponsePacket::from_slice( - &sk_client.secret_management_request_custom_aad(&req_1, &seq_0, &seq_0), + &sk_client.secret_management_request_custom_aad(&req_1, &seq_0, &seq_0).unwrap(), ) .unwrap(); assert_eq!(res.response_type().unwrap(), ResponseType::Success); // Check 2nd request/response is successful let res = ResponsePacket::from_slice( - &sk_client.secret_management_request_custom_aad(&req_2, &seq_1, &seq_1), + &sk_client.secret_management_request_custom_aad(&req_2, &seq_1, &seq_1).unwrap(), ) .unwrap(); assert_eq!(res.response_type().unwrap(), ResponseType::Success); // Check 3rd request/response is successful let res = ResponsePacket::from_slice( - &sk_client.secret_management_request_custom_aad(&req_3, &seq_2, &seq_2), + &sk_client.secret_management_request_custom_aad(&req_3, &seq_2, &seq_2).unwrap(), ) .unwrap(); assert_eq!(res.response_type().unwrap(), ResponseType::Success); @@ -452,7 +496,7 @@ fn secret_management_replay_protection_seq_num_per_session(instance: String) { let seq_0 = seq_a.get_then_increment().unwrap(); // Check first request/response is successful let res = ResponsePacket::from_slice( - &sk_client.secret_management_request_custom_aad(&req_1, &seq_0, &seq_0), + &sk_client.secret_management_request_custom_aad(&req_1, &seq_0, &seq_0).unwrap(), ) .unwrap(); assert_eq!(res.response_type().unwrap(), ResponseType::Success); @@ -461,7 +505,7 @@ fn secret_management_replay_protection_seq_num_per_session(instance: String) { let sk_client_diff = SkClient::new(&instance); // Check first request/response is with seq_0 is successful let res = ResponsePacket::from_slice( - &sk_client_diff.secret_management_request_custom_aad(&req_1, &seq_0, &seq_0), + &sk_client_diff.secret_management_request_custom_aad(&req_1, &seq_0, &seq_0).unwrap(), ) .unwrap(); assert_eq!(res.response_type().unwrap(), ResponseType::Success); @@ -470,7 +514,6 @@ fn secret_management_replay_protection_seq_num_per_session(instance: String) { // This test checks that Secretkeeper rejects requests with out of order [`RequestSeqNum`] // TODO(b/317416663): This test fails, when HAL is not present in the device. Refactor to fix this. #[rdroidtest(get_instances())] -#[ignore] fn secret_management_replay_protection_out_of_seq_req_not_accepted(instance: String) { let dice_chain = make_explicit_owned_dice(/*Security version in a node */ 5); let sealing_policy = sealing_policy(dice_chain.explicit_key_dice_chain().unwrap()); @@ -484,11 +527,36 @@ fn secret_management_replay_protection_out_of_seq_req_not_accepted(instance: Str let [seq_0, seq_1, seq_2] = std::array::from_fn(|_| seq_a.get_then_increment().unwrap()); // Assume First request/response is successful - sk_client.secret_management_request_custom_aad(&req_1, &seq_0, &seq_0); + sk_client.secret_management_request_custom_aad(&req_1, &seq_0, &seq_0).unwrap(); // Check 2nd request/response with skipped seq_num in request is a binder error - // This should panic! - sk_client.secret_management_request_custom_aad(&req_2, /*Skipping seq_1*/ &seq_2, &seq_1); + let res = sk_client + .secret_management_request_custom_aad(&req_2, /*Skipping seq_1*/ &seq_2, &seq_1); + let err = res.expect_err("Out of Seq messages accepted!"); + // Incorrect sequence numbers lead to failed decryption. The resultant error should be + // thrown in clear text & wrapped in Binder errors. + assert!(matches!(err, Error::InfraError(secretkeeper_client::Error::BinderStatus(_e)))); +} + +// This test checks DICE policy based access control of Secretkeeper. +#[rdroidtest(get_instances())] +fn secret_management_policy_gate(instance: String) { + let dice_chain = make_explicit_owned_dice(/*Security version in a node */ 100); + let mut sk_client = SkClient::with_identity(&instance, dice_chain); + sk_client.store(&ID_EXAMPLE, &SECRET_EXAMPLE).unwrap(); + + // Start a session with higher security_version & get the stored secret. + let dice_chain_upgraded = make_explicit_owned_dice(/*Security version in a node */ 101); + let mut sk_client_upgraded = SkClient::with_identity(&instance, dice_chain_upgraded); + assert_eq!(sk_client_upgraded.get(&ID_EXAMPLE).unwrap(), SECRET_EXAMPLE); + + // Start a session with lower security_version (This should be denied access to the secret). + let dice_chain_downgraded = make_explicit_owned_dice(/*Security version in a node */ 99); + let mut sk_client_downgraded = SkClient::with_identity(&instance, dice_chain_downgraded); + assert!(matches!( + sk_client_downgraded.get(&ID_EXAMPLE).unwrap_err(), + Error::SecretkeeperError(SecretkeeperError::DicePolicyError) + )); } // Helper method that constructs 3 SecretManagement requests. Callers would usually not care about