Require attestation app ID.

Bug: 37318025
Test: Manually tested
Change-Id: Iaa992c8d22e0c88c2a2570355199befa484adc19

keymaster/3.0/default/KeymasterDevice.cpp

@@ -519,6 +519,7 @@ Return<void> KeymasterDevice::attestKey(const hidl_vec<uint8_t>& keyToAttest,
 
     hidl_vec<hidl_vec<uint8_t>> resultCertChain;
 
+    bool foundAttestationApplicationId = false;
     for (size_t i = 0; i < attestParams.size(); ++i) {
         switch (attestParams[i].tag) {
         case Tag::ATTESTATION_ID_BRAND:
@@ -532,11 +533,22 @@ Return<void> KeymasterDevice::attestKey(const hidl_vec<uint8_t>& keyToAttest,
             // never perform any device id attestation.
             _hidl_cb(ErrorCode::CANNOT_ATTEST_IDS, resultCertChain);
             return Void();
+
+        case Tag::ATTESTATION_APPLICATION_ID:
+            foundAttestationApplicationId = true;
+            break;
+
         default:
             break;
         }
     }
 
+    // KM3 devices reject missing attest application IDs. KM2 devices do not.
+    if (!foundAttestationApplicationId) {
+        _hidl_cb(ErrorCode::ATTESTATION_APPLICATION_ID_MISSING,
+                 resultCertChain);
+    }
+
     keymaster_cert_chain_t cert_chain{nullptr, 0};
 
     auto kmKeyToAttest = hidlVec2KmKeyBlob(keyToAttest);

keymaster/3.0/vts/functional/attestation_record.cpp

@@ -244,6 +244,8 @@ static ErrorCode extract_auth_list(const KM_AUTH_LIST* record, AuthorizationSet*
     copyAuthTag(record->rsa_public_exponent, TAG_RSA_PUBLIC_EXPONENT, auth_list);
     copyAuthTag(record->usage_expire_date_time, TAG_USAGE_EXPIRE_DATETIME, auth_list);
     copyAuthTag(record->user_auth_type, TAG_USER_AUTH_TYPE, auth_list);
+    copyAuthTag(record->attestation_application_id,
+                TAG_ATTESTATION_APPLICATION_ID, auth_list);
 
     return ErrorCode::OK;
 }

keymaster/3.0/vts/functional/keymaster_hidl_hal_test.cpp

@@ -892,10 +892,10 @@ class KeymasterHidlTest : public ::testing::VtsHalHidlTargetTestBase {
     static hidl_string author_;
 };
 
-bool verify_attestation_record(const string& challenge, AuthorizationSet expected_sw_enforced,
+bool verify_attestation_record(const string& challenge, const string& app_id,
+                               AuthorizationSet expected_sw_enforced,
                                AuthorizationSet expected_tee_enforced,
                                const hidl_vec<uint8_t>& attestation_cert) {
-
     X509_Ptr cert(parse_cert_blob(attestation_cert));
     EXPECT_TRUE(!!cert.get());
     if (!cert.get()) return false;
@@ -912,6 +912,7 @@ bool verify_attestation_record(const string& challenge, AuthorizationSet expecte
     SecurityLevel att_keymaster_security_level;
     HidlBuf att_challenge;
     HidlBuf att_unique_id;
+    HidlBuf att_app_id;
     EXPECT_EQ(ErrorCode::OK,
               parse_attestation_record(attest_rec->data,                 //
                                        attest_rec->length,               //
@@ -930,6 +931,9 @@ bool verify_attestation_record(const string& challenge, AuthorizationSet expecte
         EXPECT_EQ(1U, att_attestation_version);
     }
 
+    expected_sw_enforced.push_back(TAG_ATTESTATION_APPLICATION_ID,
+                                   HidlBuf(app_id));
+
     if (!KeymasterHidlTest::IsSecure()) {
         // SW is KM2
         EXPECT_EQ(att_keymaster_version, 2U);
@@ -3835,17 +3839,43 @@ TEST_F(AttestationTest, RsaAttestation) {
                                              .Authorization(TAG_INCLUDE_UNIQUE_ID)));
 
     hidl_vec<hidl_vec<uint8_t>> cert_chain;
-    EXPECT_EQ(ErrorCode::OK, AttestKey(AuthorizationSetBuilder().Authorization(
-                                           TAG_ATTESTATION_CHALLENGE, HidlBuf("challenge")),
+    EXPECT_EQ(
+        ErrorCode::OK,
+        AttestKey(
+            AuthorizationSetBuilder()
+                .Authorization(TAG_ATTESTATION_CHALLENGE, HidlBuf("challenge"))
+                .Authorization(TAG_ATTESTATION_APPLICATION_ID, HidlBuf("foo")),
             &cert_chain));
     EXPECT_GE(cert_chain.size(), 2U);
     EXPECT_TRUE(verify_chain(cert_chain));
-    EXPECT_TRUE(verify_attestation_record("challenge",                            //
+    EXPECT_TRUE(
+        verify_attestation_record("challenge", "foo",                     //
                                   key_characteristics_.softwareEnforced,  //
                                   key_characteristics_.teeEnforced,       //
                                   cert_chain[0]));
 }
 
+/*
+ * AttestationTest.RsaAttestationRequiresAppId
+ *
+ * Verifies that attesting to RSA requires app ID.
+ */
+TEST_F(AttestationTest, RsaAttestationRequiresAppId) {
+    ASSERT_EQ(ErrorCode::OK,
+              GenerateKey(AuthorizationSetBuilder()
+                              .Authorization(TAG_NO_AUTH_REQUIRED)
+                              .RsaSigningKey(1024, 3)
+                              .Digest(Digest::NONE)
+                              .Padding(PaddingMode::NONE)
+                              .Authorization(TAG_INCLUDE_UNIQUE_ID)));
+
+    hidl_vec<hidl_vec<uint8_t>> cert_chain;
+    EXPECT_EQ(ErrorCode::ATTESTATION_APPLICATION_ID_MISSING,
+              AttestKey(AuthorizationSetBuilder().Authorization(
+                            TAG_ATTESTATION_CHALLENGE, HidlBuf("challenge")),
+                        &cert_chain));
+}
+
 /*
  * AttestationTest.EcAttestation
  *
@@ -3859,18 +3889,43 @@ TEST_F(AttestationTest, EcAttestation) {
                                              .Authorization(TAG_INCLUDE_UNIQUE_ID)));
 
     hidl_vec<hidl_vec<uint8_t>> cert_chain;
-    EXPECT_EQ(ErrorCode::OK, AttestKey(AuthorizationSetBuilder().Authorization(
-                                           TAG_ATTESTATION_CHALLENGE, HidlBuf("challenge")),
+    EXPECT_EQ(
+        ErrorCode::OK,
+        AttestKey(
+            AuthorizationSetBuilder()
+                .Authorization(TAG_ATTESTATION_CHALLENGE, HidlBuf("challenge"))
+                .Authorization(TAG_ATTESTATION_APPLICATION_ID, HidlBuf("foo")),
             &cert_chain));
     EXPECT_GE(cert_chain.size(), 2U);
     EXPECT_TRUE(verify_chain(cert_chain));
 
-    EXPECT_TRUE(verify_attestation_record("challenge",                            //
+    EXPECT_TRUE(
+        verify_attestation_record("challenge", "foo",                     //
                                   key_characteristics_.softwareEnforced,  //
                                   key_characteristics_.teeEnforced,       //
                                   cert_chain[0]));
 }
 
+/*
+ * AttestationTest.EcAttestationRequiresAttestationAppId
+ *
+ * Verifies that attesting to EC keys requires app ID
+ */
+TEST_F(AttestationTest, EcAttestationRequiresAttestationAppId) {
+    ASSERT_EQ(ErrorCode::OK,
+              GenerateKey(AuthorizationSetBuilder()
+                              .Authorization(TAG_NO_AUTH_REQUIRED)
+                              .EcdsaSigningKey(EcCurve::P_256)
+                              .Digest(Digest::SHA_2_256)
+                              .Authorization(TAG_INCLUDE_UNIQUE_ID)));
+
+    hidl_vec<hidl_vec<uint8_t>> cert_chain;
+    EXPECT_EQ(ErrorCode::ATTESTATION_APPLICATION_ID_MISSING,
+              AttestKey(AuthorizationSetBuilder().Authorization(
+                            TAG_ATTESTATION_CHALLENGE, HidlBuf("challenge")),
+                        &cert_chain));
+}
+
 /*
  * AttestationTest.AesAttestation
  *
@@ -3940,14 +3995,17 @@ TEST_F(KeyDeletionTest, DeleteKey) {
     AuthorizationSet begin_out_params;
 
     if (rollback_protected) {
-        EXPECT_EQ(ErrorCode::INVALID_KEY_BLOB,
-                Begin(KeyPurpose::SIGN, key_blob_,
-                        AuthorizationSetBuilder().Digest(Digest::NONE).Padding(PaddingMode::NONE),
+        EXPECT_EQ(
+            ErrorCode::INVALID_KEY_BLOB,
+            Begin(KeyPurpose::SIGN, key_blob_, AuthorizationSetBuilder()
+                                                   .Digest(Digest::NONE)
+                                                   .Padding(PaddingMode::NONE),
                   &begin_out_params, &op_handle_));
     } else {
-        EXPECT_EQ(ErrorCode::OK,
-                Begin(KeyPurpose::SIGN, key_blob_,
-                        AuthorizationSetBuilder().Digest(Digest::NONE).Padding(PaddingMode::NONE),
+        EXPECT_EQ(ErrorCode::OK, Begin(KeyPurpose::SIGN, key_blob_,
+                                       AuthorizationSetBuilder()
+                                           .Digest(Digest::NONE)
+                                           .Padding(PaddingMode::NONE),
                                        &begin_out_params, &op_handle_));
     }
     AbortIfNeeded();
@@ -4014,14 +4072,17 @@ TEST_F(KeyDeletionTest, DeleteAllKeys) {
     AuthorizationSet begin_out_params;
 
     if (rollback_protected) {
-        EXPECT_EQ(ErrorCode::INVALID_KEY_BLOB,
-                Begin(KeyPurpose::SIGN, key_blob_,
-                        AuthorizationSetBuilder().Digest(Digest::NONE).Padding(PaddingMode::NONE),
+        EXPECT_EQ(
+            ErrorCode::INVALID_KEY_BLOB,
+            Begin(KeyPurpose::SIGN, key_blob_, AuthorizationSetBuilder()
+                                                   .Digest(Digest::NONE)
+                                                   .Padding(PaddingMode::NONE),
                   &begin_out_params, &op_handle_));
     } else {
-        EXPECT_EQ(ErrorCode::OK,
-                Begin(KeyPurpose::SIGN, key_blob_,
-                        AuthorizationSetBuilder().Digest(Digest::NONE).Padding(PaddingMode::NONE),
+        EXPECT_EQ(ErrorCode::OK, Begin(KeyPurpose::SIGN, key_blob_,
+                                       AuthorizationSetBuilder()
+                                           .Digest(Digest::NONE)
+                                           .Padding(PaddingMode::NONE),
                                        &begin_out_params, &op_handle_));
     }
     AbortIfNeeded();