diff options
author | Rajesh Nyamagoud <nyamagoud@google.com> | 2024-02-01 04:45:41 +0000 |
---|---|---|
committer | Rajesh Nyamagoud <nyamagoud@google.com> | 2024-02-19 20:24:47 +0000 |
commit | 7620921a7f5ea7dcbb810573828ebf2efaece521 (patch) | |
tree | 6fac9a57b5996e19afc97b49d700c4a03a97a88a | |
parent | 53d2763a23872f63657d01b3d6df9777245e4d3c (diff) | |
download | security-7620921a7f5ea7dcbb810573828ebf2efaece521.tar.gz |
Fixes for the issues found while running Keystore2 client tests on a
device with keymaster implementation.
- Ignore INVALID tag in generated key characteristics if keymaster
implementation is present.
- RSA_OAEP_MGF_DIGEST, ATTEST_KEY, USAGE_COUNT_LIMIT are not expected in
generated key characteristics if keymaster implementation is present.
- Corrected device attest ids names.
- Skip device id attestation on device with GSI image and device
first_api_level is less than 34.
- When the DEVICE_UNIQUE_ATTESTATION tag is used in key generation,
root certificate signature verification is ignored during cert-chain
verification.
Bug: 322118247
Test: atest keystore2_client_tests
Change-Id: I42d339a7797114d9139c64bc4d397889b965cb48
-rw-r--r-- | keystore2/test_utils/ffi_test_utils.cpp | 18 | ||||
-rw-r--r-- | keystore2/test_utils/key_generations.rs | 20 | ||||
-rw-r--r-- | keystore2/tests/keystore2_client_attest_key_tests.rs | 7 | ||||
-rw-r--r-- | keystore2/tests/keystore2_client_authorizations_tests.rs | 17 | ||||
-rw-r--r-- | keystore2/tests/keystore2_client_device_unique_attestation_tests.rs | 27 | ||||
-rw-r--r-- | keystore2/tests/keystore2_client_test_utils.rs | 35 |
6 files changed, 101 insertions, 23 deletions
diff --git a/keystore2/test_utils/ffi_test_utils.cpp b/keystore2/test_utils/ffi_test_utils.cpp index 4e781d10..ea030692 100644 --- a/keystore2/test_utils/ffi_test_utils.cpp +++ b/keystore2/test_utils/ffi_test_utils.cpp @@ -155,11 +155,19 @@ bool ChainSignaturesAreValid(const vector<certificate_t>& chain, bool strict_iss } if (!X509_verify(key_cert.get(), signing_pubkey.get())) { - LOG(ERROR) << "Verification of certificate " << i << " failed " - << "OpenSSL error string: " << ERR_error_string(ERR_get_error(), NULL) - << '\n' - << cert_data.str(); - return false; + // Handles the case of device-unique attestation chain which is not expected to be + // self-signed - b/191361618 + // For device-unique attestation chain `strict_issuer_check` is not set, so ignore the + // root certificate signature verification result and in all other cases return the + // error. + bool is_root_cert = (i == chain.size() - 1); + if (strict_issuer_check || !is_root_cert) { + LOG(ERROR) << "Verification of certificate " << i << " failed " + << "OpenSSL error string: " << ERR_error_string(ERR_get_error(), NULL) + << '\n' + << cert_data.str(); + return false; + } } string cert_issuer = x509NameToStr(X509_get_issuer_name(key_cert.get())); diff --git a/keystore2/test_utils/key_generations.rs b/keystore2/test_utils/key_generations.rs index 9ddc87aa..a733be39 100644 --- a/keystore2/test_utils/key_generations.rs +++ b/keystore2/test_utils/key_generations.rs @@ -410,6 +410,11 @@ pub fn check_key_authorizations( ) { // Make sure key authorizations contains only `ALLOWED_TAGS_IN_KEY_AUTHS` authorizations.iter().all(|auth| { + // Ignore `INVALID` tag if the backend is Keymaster and not KeyMint. + // Keymaster allows INVALID tag for unsupported key parameters. + if !has_default_keymint() && auth.keyParameter.tag == Tag::INVALID { + return true; + } assert!( ALLOWED_TAGS_IN_KEY_AUTHS.contains(&auth.keyParameter.tag), "key authorization is not allowed: {:#?}", @@ -427,6 +432,21 @@ pub fn check_key_authorizations( { return true; } + + // Ignore below parameters if the backend is Keymaster and not KeyMint. + // Keymaster does not support these parameters. These key parameters are introduced in + // KeyMint1.0. + if !has_default_keymint() { + if matches!(key_param.tag, Tag::RSA_OAEP_MGF_DIGEST | Tag::USAGE_COUNT_LIMIT) { + return true; + } + if key_param.tag == Tag::PURPOSE + && key_param.value == KeyParameterValue::KeyPurpose(KeyPurpose::ATTEST_KEY) + { + return true; + } + } + if ALLOWED_TAGS_IN_KEY_AUTHS.contains(&key_param.tag) { assert!( check_key_param(authorizations, key_param), diff --git a/keystore2/tests/keystore2_client_attest_key_tests.rs b/keystore2/tests/keystore2_client_attest_key_tests.rs index 3532a350..454248a3 100644 --- a/keystore2/tests/keystore2_client_attest_key_tests.rs +++ b/keystore2/tests/keystore2_client_attest_key_tests.rs @@ -31,12 +31,13 @@ use keystore2_test_utils::{ use keystore2_test_utils::ffi_test_utils::{get_value_from_attest_record, validate_certchain}; use crate::{ - skip_test_if_no_app_attest_key_feature, skip_test_if_no_device_id_attestation_feature, + skip_device_id_attestation_tests, skip_test_if_no_app_attest_key_feature, + skip_test_if_no_device_id_attestation_feature, }; use crate::keystore2_client_test_utils::{ app_attest_key_feature_exists, device_id_attestation_feature_exists, get_attest_id_value, - is_second_imei_id_attestation_required, + is_second_imei_id_attestation_required, skip_device_id_attest_tests, }; /// Generate RSA and EC attestation keys and use them for signing RSA-signing keys. @@ -522,6 +523,8 @@ fn get_attestation_ids(keystore2: &binder::Strong<dyn IKeystoreService>) -> Vec< /// INVALID_TAG` fn generate_attested_key_with_device_attest_ids(algorithm: Algorithm) { skip_test_if_no_device_id_attestation_feature!(); + skip_device_id_attestation_tests!(); + skip_test_if_no_app_attest_key_feature!(); let keystore2 = get_keystore_service(); let sec_level = keystore2.getSecurityLevel(SecurityLevel::TRUSTED_ENVIRONMENT).unwrap(); diff --git a/keystore2/tests/keystore2_client_authorizations_tests.rs b/keystore2/tests/keystore2_client_authorizations_tests.rs index 2291a08f..0fde7aff 100644 --- a/keystore2/tests/keystore2_client_authorizations_tests.rs +++ b/keystore2/tests/keystore2_client_authorizations_tests.rs @@ -41,11 +41,14 @@ use keystore2_test_utils::{ }; use crate::keystore2_client_test_utils::{ - delete_app_key, perform_sample_asym_sign_verify_op, perform_sample_hmac_sign_verify_op, - perform_sample_sym_key_decrypt_op, perform_sample_sym_key_encrypt_op, - verify_certificate_serial_num, verify_certificate_subject_name, SAMPLE_PLAIN_TEXT, + app_attest_key_feature_exists, delete_app_key, perform_sample_asym_sign_verify_op, + perform_sample_hmac_sign_verify_op, perform_sample_sym_key_decrypt_op, + perform_sample_sym_key_encrypt_op, verify_certificate_serial_num, + verify_certificate_subject_name, SAMPLE_PLAIN_TEXT, }; +use crate::{skip_test_if_no_app_attest_key_feature, skip_tests_if_keymaster_impl_present}; + use keystore2_test_utils::ffi_test_utils::get_value_from_attest_record; fn gen_key_including_unique_id( @@ -112,8 +115,9 @@ fn generate_key_and_perform_op_with_max_usage_limit( let auth = key_generations::get_key_auth(&key_metadata.authorizations, Tag::USAGE_COUNT_LIMIT) .unwrap(); - if check_attestation { + if check_attestation && key_generations::has_default_keymint() { // Check usage-count-limit is included in attest-record. + // `USAGE_COUNT_LIMIT` is supported from KeyMint1.0 assert_ne!( gen_params.iter().filter(|kp| kp.tag == Tag::ATTESTATION_CHALLENGE).count(), 0, @@ -443,6 +447,7 @@ fn keystore2_gen_key_auth_usage_expire_datetime_decrypt_op_fail() { /// during creation of an operation using this key. #[test] fn keystore2_gen_key_auth_boot_loader_only_op_fail() { + skip_tests_if_keymaster_impl_present!(); let keystore2 = get_keystore_service(); let sec_level = keystore2.getSecurityLevel(SecurityLevel::TRUSTED_ENVIRONMENT).unwrap(); @@ -472,6 +477,7 @@ fn keystore2_gen_key_auth_boot_loader_only_op_fail() { /// during creation of an operation using this key. #[test] fn keystore2_gen_key_auth_early_boot_only_op_fail() { + skip_tests_if_keymaster_impl_present!(); let keystore2 = get_keystore_service(); let sec_level = keystore2.getSecurityLevel(SecurityLevel::TRUSTED_ENVIRONMENT).unwrap(); @@ -815,6 +821,7 @@ fn keystore2_gen_key_auth_app_id_test_fail() { /// generated attestation-key. #[test] fn keystore2_gen_attested_key_auth_app_id_app_data_test_success() { + skip_test_if_no_app_attest_key_feature!(); let keystore2 = get_keystore_service(); let sec_level = keystore2.getSecurityLevel(SecurityLevel::TRUSTED_ENVIRONMENT).unwrap(); @@ -869,6 +876,7 @@ fn keystore2_gen_attested_key_auth_app_id_app_data_test_success() { /// be provided to generateKey for an attestation key that was generated with them. #[test] fn keystore2_gen_attestation_key_with_auth_app_id_app_data_test_fail() { + skip_test_if_no_app_attest_key_feature!(); let keystore2 = get_keystore_service(); let sec_level = keystore2.getSecurityLevel(SecurityLevel::TRUSTED_ENVIRONMENT).unwrap(); @@ -973,6 +981,7 @@ fn keystore2_flagged_on_get_last_auth_fingerprint_success() { /// generate a key successfully and verify the specified key parameters. #[test] fn keystore2_gen_key_auth_serial_number_subject_test_success() { + skip_tests_if_keymaster_impl_present!(); let keystore2 = get_keystore_service(); let sec_level = keystore2.getSecurityLevel(SecurityLevel::TRUSTED_ENVIRONMENT).unwrap(); diff --git a/keystore2/tests/keystore2_client_device_unique_attestation_tests.rs b/keystore2/tests/keystore2_client_device_unique_attestation_tests.rs index cf88fc54..4f881bcd 100644 --- a/keystore2/tests/keystore2_client_device_unique_attestation_tests.rs +++ b/keystore2/tests/keystore2_client_device_unique_attestation_tests.rs @@ -27,6 +27,8 @@ use crate::keystore2_client_test_utils::{ perform_sample_asym_sign_verify_op, }; +use crate::skip_tests_if_keymaster_impl_present; + /// This macro is used for generating device unique attested EC key with device id attestation. macro_rules! test_ec_key_device_unique_attestation_id { ( $test_name:ident, $tag:expr, $prop_name:expr ) => { @@ -158,6 +160,7 @@ fn generate_device_unique_attested_key_with_device_attest_ids( /// Test should fail to generate a key with error code `INVALID_ARGUMENT` #[test] fn keystore2_gen_key_device_unique_attest_with_default_sec_level_unimplemented() { + skip_tests_if_keymaster_impl_present!(); let keystore2 = get_keystore_service(); let sec_level = keystore2.getSecurityLevel(SecurityLevel::TRUSTED_ENVIRONMENT).unwrap(); @@ -324,32 +327,32 @@ fn keystore2_device_unique_attest_key_fails_with_invalid_attestation_id() { test_ec_key_device_unique_attestation_id!( keystore2_device_unique_attest_ecdsa_attest_id_brand, Tag::ATTESTATION_ID_BRAND, - "ro.product.brand_for_attestation" + "brand" ); test_ec_key_device_unique_attestation_id!( keystore2_device_unique_attest_ecdsa_attest_id_device, Tag::ATTESTATION_ID_DEVICE, - "ro.product.device" + "device" ); test_ec_key_device_unique_attestation_id!( keystore2_device_unique_attest_ecdsa_attest_id_product, Tag::ATTESTATION_ID_PRODUCT, - "ro.product.name_for_attestation" + "name" ); test_ec_key_device_unique_attestation_id!( keystore2_device_unique_attest_ecdsa_attest_id_serial, Tag::ATTESTATION_ID_SERIAL, - "ro.serialno" + "serialno" ); test_ec_key_device_unique_attestation_id!( keystore2_device_unique_attest_ecdsa_attest_id_manufacturer, Tag::ATTESTATION_ID_MANUFACTURER, - "ro.product.manufacturer" + "manufacturer" ); test_ec_key_device_unique_attestation_id!( keystore2_device_unique_attest_ecdsa_attest_id_model, Tag::ATTESTATION_ID_MODEL, - "ro.product.model_for_attestation" + "model" ); test_ec_key_device_unique_attestation_id!( keystore2_device_unique_attest_ecdsa_attest_id_imei, @@ -367,32 +370,32 @@ test_ec_key_device_unique_attestation_id!( test_rsa_key_device_unique_attestation_id!( keystore2_device_unique_attest_rsa_attest_id_brand, Tag::ATTESTATION_ID_BRAND, - "ro.product.brand_for_attestation" + "brand" ); test_rsa_key_device_unique_attestation_id!( keystore2_device_unique_attest_rsa_attest_id_device, Tag::ATTESTATION_ID_DEVICE, - "ro.product.device" + "device" ); test_rsa_key_device_unique_attestation_id!( keystore2_device_unique_attest_rsa_attest_id_product, Tag::ATTESTATION_ID_PRODUCT, - "ro.product.name_for_attestation" + "name" ); test_rsa_key_device_unique_attestation_id!( keystore2_device_unique_attest_rsa_attest_id_serial, Tag::ATTESTATION_ID_SERIAL, - "ro.serialno" + "serialno" ); test_rsa_key_device_unique_attestation_id!( keystore2_device_unique_attest_rsa_attest_id_manufacturer, Tag::ATTESTATION_ID_MANUFACTURER, - "ro.product.manufacturer" + "manufacturer" ); test_rsa_key_device_unique_attestation_id!( keystore2_device_unique_attest_rsa_attest_id_model, Tag::ATTESTATION_ID_MODEL, - "ro.product.model_for_attestation" + "model" ); test_rsa_key_device_unique_attestation_id!( keystore2_device_unique_attest_rsa_attest_id_imei, diff --git a/keystore2/tests/keystore2_client_test_utils.rs b/keystore2/tests/keystore2_client_test_utils.rs index 037482a0..f270297c 100644 --- a/keystore2/tests/keystore2_client_test_utils.rs +++ b/keystore2/tests/keystore2_client_test_utils.rs @@ -15,6 +15,7 @@ use nix::unistd::{Gid, Uid}; use serde::{Deserialize, Serialize}; +use std::path::PathBuf; use std::process::{Command, Output}; use openssl::bn::BigNum; @@ -88,6 +89,22 @@ pub fn device_id_attestation_feature_exists() -> bool { pm.hasSystemFeature(DEVICE_ID_ATTESTATION_FEATURE, 0).expect("hasSystemFeature failed.") } +/// Determines whether to skip device id attestation tests on GSI build with API level < 34. +pub fn skip_device_id_attest_tests() -> bool { + // b/298586194, there are some devices launched with Android T, and they will be receiving + // only system update and not vendor update, newly added attestation properties + // (ro.product.*_for_attestation) reading logic would not be available for such devices + // hence skipping this test for such scenario. + let api_level = std::str::from_utf8(&get_system_prop("ro.board.first_api_level")) + .unwrap() + .parse::<i32>() + .unwrap(); + // This file is only present on GSI builds. + let path_buf = PathBuf::from("/system/system_ext/etc/init/init.gsi.rc"); + + api_level < 34 && path_buf.as_path().is_file() +} + #[macro_export] macro_rules! skip_test_if_no_app_attest_key_feature { () => { @@ -106,6 +123,24 @@ macro_rules! skip_test_if_no_device_id_attestation_feature { }; } +#[macro_export] +macro_rules! skip_device_id_attestation_tests { + () => { + if skip_device_id_attest_tests() { + return; + } + }; +} + +#[macro_export] +macro_rules! skip_tests_if_keymaster_impl_present { + () => { + if !key_generations::has_default_keymint() { + return; + } + }; +} + /// Generate EC key and grant it to the list of users with given access vector. /// Returns the list of granted keys `nspace` values in the order of given grantee uids. pub fn generate_ec_key_and_grant_to_users( |