Merge "Fixes for the issues found while running Keystore2 client tests on a device with keymaster implementation." into main am: 3dfac14787 am: 4a45b25beb

Original change: https://android-review.googlesource.com/c/platform/system/security/+/2942748

Change-Id: I0c8d292d1fc1a73fb97f1e1699aeab4334e298b7
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

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 "
+            // Handles the case of device-unique attestation chain which is not expected to be
-                       << "OpenSSL error string: " << ERR_error_string(ERR_get_error(), NULL)
+            // self-signed - b/191361618
-                       << '\n'
+            // For device-unique attestation chain `strict_issuer_check` is not set, so ignore the
-                       << cert_data.str();
+            // root certificate signature verification result and in all other cases return the
-            return false;
+            // 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()));

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),

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();
 

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,
+    app_attest_key_feature_exists, delete_app_key, perform_sample_asym_sign_verify_op,
-    perform_sample_sym_key_decrypt_op, perform_sample_sym_key_encrypt_op,
+    perform_sample_hmac_sign_verify_op, perform_sample_sym_key_decrypt_op,
-    verify_certificate_serial_num, verify_certificate_subject_name, SAMPLE_PLAIN_TEXT,
+    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();
 

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,

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(