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

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(