Merge "Rename interface method name."
diff --git a/audio/core/all-versions/vts/functional/6.0/AudioPrimaryHidlHalTest.cpp b/audio/core/all-versions/vts/functional/6.0/AudioPrimaryHidlHalTest.cpp
index 0f0cdcf..0ebe4c2 100644
--- a/audio/core/all-versions/vts/functional/6.0/AudioPrimaryHidlHalTest.cpp
+++ b/audio/core/all-versions/vts/functional/6.0/AudioPrimaryHidlHalTest.cpp
@@ -128,7 +128,7 @@
 INSTANTIATE_TEST_CASE_P(SingleConfigOutputStream, SingleConfigOutputStreamTest,
                         ::testing::ValuesIn(getOutputDeviceSingleConfigParameters()),
                         &DeviceConfigParameterToString);
-GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(SingleConfigOutputStream);
+GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(SingleConfigOutputStreamTest);
 
 class SingleConfigInputStreamTest : public InputStreamTest {};
 TEST_P(SingleConfigInputStreamTest, CloseDeviceWithOpenedInputStreams) {
@@ -142,7 +142,7 @@
 INSTANTIATE_TEST_CASE_P(SingleConfigInputStream, SingleConfigInputStreamTest,
                         ::testing::ValuesIn(getInputDeviceSingleConfigParameters()),
                         &DeviceConfigParameterToString);
-GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(SingleConfigInputStream);
+GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(SingleConfigInputStreamTest);
 
 TEST_P(AudioPatchHidlTest, UpdatePatchInvalidHandle) {
     doc::test("Verify that passing an invalid handle to updateAudioPatch is checked");
diff --git a/audio/core/all-versions/vts/functional/7.0/AudioPrimaryHidlHalTest.cpp b/audio/core/all-versions/vts/functional/7.0/AudioPrimaryHidlHalTest.cpp
index ef4daba..7fca610 100644
--- a/audio/core/all-versions/vts/functional/7.0/AudioPrimaryHidlHalTest.cpp
+++ b/audio/core/all-versions/vts/functional/7.0/AudioPrimaryHidlHalTest.cpp
@@ -296,7 +296,7 @@
         InputBufferSizeInvalidConfig, InvalidInputConfigNoFlagsTest,
         ::testing::ValuesIn(getInputDeviceInvalidConfigParameters(false /*generateInvalidFlags*/)),
         &DeviceConfigParameterToString);
-GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(InputBufferSizeInvalidConfig);
+GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(InvalidInputConfigNoFlagsTest);
 
 static const DeviceAddress& getValidInputDeviceAddress() {
     static const DeviceAddress valid = {
@@ -682,9 +682,7 @@
                            ::testing::Values(getValidInputDeviceAddress()),
                            ::testing::ValuesIn(wrapMetadata(getInvalidSinkMetadatas()))),
         &StreamOpenParameterToString);
-GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(InputStreamInvalidConfig);
-GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(InputStreamInvalidAddress);
-GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(InputStreamInvalidMetadata);
+GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(StreamOpenTest);
 
 INSTANTIATE_TEST_CASE_P(
         OutputStreamInvalidConfig, StreamOpenTest,
@@ -706,9 +704,6 @@
                            ::testing::Values(getValidOutputDeviceAddress()),
                            ::testing::ValuesIn(wrapMetadata(getInvalidSourceMetadatas()))),
         &StreamOpenParameterToString);
-GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(OutputStreamInvalidConfig);
-GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(OutputStreamInvalidAddress);
-GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(OutputStreamInvalidMetadata);
 
 #define TEST_SINGLE_CONFIG_IO_STREAM(test_name, documentation, code) \
     TEST_P(SingleConfigInputStreamTest, test_name) {                 \
diff --git a/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp b/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp
index ca57243..ce50f25 100644
--- a/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp
+++ b/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp
@@ -415,7 +415,7 @@
 TEST_P(BroadcastRadioHalTest, FmTune) {
     ASSERT_TRUE(openSession());
 
-    uint64_t freq = 100100;  // 100.1 FM
+    uint64_t freq = 90900;  // 90.9 FM
     auto sel = make_selector_amfm(freq);
 
     /* TODO(b/69958777): there is a race condition between tune() and onCurrentProgramInfoChanged
diff --git a/compatibility_matrices/compatibility_matrix.current.xml b/compatibility_matrices/compatibility_matrix.current.xml
index 5a5c5a6..d371f7c 100644
--- a/compatibility_matrices/compatibility_matrix.current.xml
+++ b/compatibility_matrices/compatibility_matrix.current.xml
@@ -258,14 +258,6 @@
             <instance>default</instance>
         </interface>
     </hal>
-    <hal format="hidl" optional="true">
-        <name>android.hardware.health.storage</name>
-        <version>1.0</version>
-        <interface>
-            <name>IStorage</name>
-            <instance>default</instance>
-        </interface>
-    </hal>
     <hal format="aidl" optional="true">
         <name>android.hardware.health.storage</name>
         <version>1</version>
diff --git a/health/1.0/Android.bp b/health/1.0/Android.bp
index 7845871..7786c08 100644
--- a/health/1.0/Android.bp
+++ b/health/1.0/Android.bp
@@ -5,7 +5,6 @@
     root: "android.hardware",
     srcs: [
         "types.hal",
-        "IHealth.hal",
     ],
     interfaces: [
         "android.hidl.base@1.0",
diff --git a/health/1.0/IHealth.hal b/health/1.0/IHealth.hal
deleted file mode 100644
index 3828589..0000000
--- a/health/1.0/IHealth.hal
+++ /dev/null
@@ -1,56 +0,0 @@
-/*
- * Copyright (C) 2016 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package android.hardware.health@1.0;
-
-interface IHealth {
-    /**
-     * This function lets you change healthd configuration from default if
-     * desired. It must be called exactly once at startup time.
-     *
-     * The configuration values are described in 'struct HealthConfig'.
-     * To use default configuration, simply return without modifying the
-     * fields of the config parameter.
-     *
-     * @param default healthd configuration.
-     */
-    init(HealthConfig config) generates (HealthConfig configOut);
-
-    /**
-     * This function is a hook to update/change device's HealthInfo (as described
-     * in 'struct HealthInfo').
-     *
-     * 'HealthInfo' describes device's battery and charging status, typically
-     * read from kernel. These values may be modified in this call.
-     *
-     * @param   Device Health info as described in 'struct HealthInfo'.
-     * @return  skipLogging Indication to the caller to add 'or' skip logging the health
-     *          information. Return 'true' to skip logging the update.
-     * @return  infoOut HealthInfo to be sent to client code. (May or may
-     *          not be modified).
-     */
-    update(HealthInfo info) generates (bool skipLogging, HealthInfo infoOut);
-
-    /**
-     * This function is called by healthd when framework queries for remaining
-     * energy in the Battery through BatteryManager APIs.
-     *
-     * @return  result Result of querying enery counter for the battery.
-     * @return  energy Battery remaining energy in nanowatt-hours.
-     *          Must be '0' if result is anything other than Result::SUCCESS.
-     */
-    energyCounter() generates (Result result, int64_t energy);
-};
diff --git a/health/1.0/default/include/hal_conversion.h b/health/1.0/default/include/hal_conversion.h
index a92b208..a8ddb73 100644
--- a/health/1.0/default/include/hal_conversion.h
+++ b/health/1.0/default/include/hal_conversion.h
@@ -17,7 +17,7 @@
 #ifndef HARDWARE_INTERFACES_HEALTH_V1_0_DEFAULT_INCLUDE_HAL_CONVERSION_H_
 #define HARDWARE_INTERFACES_HEALTH_V1_0_DEFAULT_INCLUDE_HAL_CONVERSION_H_
 
-#include <android/hardware/health/1.0/IHealth.h>
+#include <android/hardware/health/1.0/types.h>
 #include <healthd/healthd.h>
 
 namespace android {
diff --git a/oemlock/aidl/vts/OWNERS b/oemlock/aidl/vts/OWNERS
new file mode 100644
index 0000000..40d95e4
--- /dev/null
+++ b/oemlock/aidl/vts/OWNERS
@@ -0,0 +1,2 @@
+chengyouho@google.com
+frankwoo@google.com
diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp
index 93a216f..766c02d 100644
--- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp
+++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp
@@ -37,7 +37,7 @@
         os << "(Empty)" << ::std::endl;
     else {
         os << "\n";
-        for (size_t i = 0; i < set.size(); ++i) os << set[i] << ::std::endl;
+        for (auto& entry : set) os << entry << ::std::endl;
     }
     return os;
 }
@@ -131,6 +131,17 @@
         *key_blob = std::move(creationResult.keyBlob);
         *key_characteristics = std::move(creationResult.keyCharacteristics);
         cert_chain_ = std::move(creationResult.certificateChain);
+
+        auto algorithm = key_desc.GetTagValue(TAG_ALGORITHM);
+        EXPECT_TRUE(algorithm);
+        if (algorithm &&
+            (algorithm.value() == Algorithm::RSA || algorithm.value() == Algorithm::EC)) {
+            EXPECT_GE(cert_chain_.size(), 1);
+            if (key_desc.Contains(TAG_ATTESTATION_CHALLENGE)) EXPECT_GT(cert_chain_.size(), 1);
+        } else {
+            // For symmetric keys there should be no certificates.
+            EXPECT_EQ(cert_chain_.size(), 0);
+        }
     }
 
     return GetReturnErrorCode(result);
@@ -162,6 +173,17 @@
         *key_blob = std::move(creationResult.keyBlob);
         *key_characteristics = std::move(creationResult.keyCharacteristics);
         cert_chain_ = std::move(creationResult.certificateChain);
+
+        auto algorithm = key_desc.GetTagValue(TAG_ALGORITHM);
+        EXPECT_TRUE(algorithm);
+        if (algorithm &&
+            (algorithm.value() == Algorithm::RSA || algorithm.value() == Algorithm::EC)) {
+            EXPECT_GE(cert_chain_.size(), 1);
+            if (key_desc.Contains(TAG_ATTESTATION_CHALLENGE)) EXPECT_GT(cert_chain_.size(), 1);
+        } else {
+            // For symmetric keys there should be no certificates.
+            EXPECT_EQ(cert_chain_.size(), 0);
+        }
     }
 
     return GetReturnErrorCode(result);
@@ -195,6 +217,20 @@
         key_blob_ = std::move(creationResult.keyBlob);
         key_characteristics_ = std::move(creationResult.keyCharacteristics);
         cert_chain_ = std::move(creationResult.certificateChain);
+
+        AuthorizationSet allAuths;
+        for (auto& entry : key_characteristics_) {
+            allAuths.push_back(AuthorizationSet(entry.authorizations));
+        }
+        auto algorithm = allAuths.GetTagValue(TAG_ALGORITHM);
+        EXPECT_TRUE(algorithm);
+        if (algorithm &&
+            (algorithm.value() == Algorithm::RSA || algorithm.value() == Algorithm::EC)) {
+            EXPECT_GE(cert_chain_.size(), 1);
+        } else {
+            // For symmetric keys there should be no certificates.
+            EXPECT_EQ(cert_chain_.size(), 0);
+        }
     }
 
     return GetReturnErrorCode(result);
@@ -788,6 +824,24 @@
     return (found == key_characteristics.end()) ? kEmptyAuthList : found->authorizations;
 }
 
+const vector<KeyParameter>& KeyMintAidlTestBase::HwEnforcedAuthorizations(
+        const vector<KeyCharacteristics>& key_characteristics) {
+    auto found =
+            std::find_if(key_characteristics.begin(), key_characteristics.end(), [](auto& entry) {
+                return entry.securityLevel == SecurityLevel::STRONGBOX ||
+                       entry.securityLevel == SecurityLevel::TRUSTED_ENVIRONMENT;
+            });
+    return (found == key_characteristics.end()) ? kEmptyAuthList : found->authorizations;
+}
+
+const vector<KeyParameter>& KeyMintAidlTestBase::SwEnforcedAuthorizations(
+        const vector<KeyCharacteristics>& key_characteristics) {
+    auto found = std::find_if(
+            key_characteristics.begin(), key_characteristics.end(),
+            [](auto& entry) { return entry.securityLevel == SecurityLevel::SOFTWARE; });
+    return (found == key_characteristics.end()) ? kEmptyAuthList : found->authorizations;
+}
+
 }  // namespace test
 
 }  // namespace aidl::android::hardware::security::keymint
diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h
index f36c397..c1a1dd9 100644
--- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h
+++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h
@@ -27,7 +27,11 @@
 
 #include <keymint_support/authorization_set.h>
 
-namespace aidl::android::hardware::security::keymint::test {
+namespace aidl::android::hardware::security::keymint {
+
+::std::ostream& operator<<(::std::ostream& os, const AuthorizationSet& set);
+
+namespace test {
 
 using ::android::sp;
 using Status = ::ndk::ScopedAStatus;
@@ -37,8 +41,6 @@
 
 constexpr uint64_t kOpHandleSentinel = 0xFFFFFFFFFFFFFFFF;
 
-::std::ostream& operator<<(::std::ostream& os, const AuthorizationSet& set);
-
 class KeyMintAidlTestBase : public ::testing::TestWithParam<string> {
   public:
     void SetUp() override;
@@ -173,6 +175,10 @@
     inline const vector<KeyParameter>& SecLevelAuthorizations() {
         return SecLevelAuthorizations(key_characteristics_);
     }
+    const vector<KeyParameter>& HwEnforcedAuthorizations(
+            const vector<KeyCharacteristics>& key_characteristics);
+    const vector<KeyParameter>& SwEnforcedAuthorizations(
+            const vector<KeyCharacteristics>& key_characteristics);
 
   private:
     std::shared_ptr<IKeyMintDevice> keymint_;
@@ -190,4 +196,6 @@
                              testing::ValuesIn(KeyMintAidlTestBase::build_params()), \
                              ::android::PrintInstanceNameToString)
 
-}  // namespace aidl::android::hardware::security::keymint::test
+}  // namespace test
+
+}  // namespace aidl::android::hardware::security::keymint
diff --git a/security/keymint/aidl/vts/functional/KeyMintTest.cpp b/security/keymint/aidl/vts/functional/KeyMintTest.cpp
index bd36b8e..e7c94f3 100644
--- a/security/keymint/aidl/vts/functional/KeyMintTest.cpp
+++ b/security/keymint/aidl/vts/functional/KeyMintTest.cpp
@@ -180,9 +180,280 @@
     void operator()(RSA* p) { RSA_free(p); }
 };
 
-/* TODO(seleneh) add attestation verification codes like verify_chain() and
- * attestation tests after we decided on the keymint 1 attestation changes.
- */
+char nibble2hex[16] = {'0', '1', '2', '3', '4', '5', '6', '7',
+                       '8', '9', 'a', 'b', 'c', 'd', 'e', 'f'};
+
+string bin2hex(const vector<uint8_t>& data) {
+    string retval;
+    retval.reserve(data.size() * 2 + 1);
+    for (uint8_t byte : data) {
+        retval.push_back(nibble2hex[0x0F & (byte >> 4)]);
+        retval.push_back(nibble2hex[0x0F & byte]);
+    }
+    return retval;
+}
+
+X509* parse_cert_blob(const vector<uint8_t>& blob) {
+    const uint8_t* p = blob.data();
+    return d2i_X509(nullptr, &p, blob.size());
+}
+
+bool verify_chain(const vector<Certificate>& chain) {
+    for (size_t i = 0; i < chain.size(); ++i) {
+        X509_Ptr key_cert(parse_cert_blob(chain[i].encodedCertificate));
+        X509_Ptr signing_cert;
+        if (i < chain.size() - 1) {
+            signing_cert.reset(parse_cert_blob(chain[i + 1].encodedCertificate));
+        } else {
+            signing_cert.reset(parse_cert_blob(chain[i].encodedCertificate));
+        }
+        EXPECT_TRUE(!!key_cert.get() && !!signing_cert.get());
+        if (!key_cert.get() || !signing_cert.get()) return false;
+
+        EVP_PKEY_Ptr signing_pubkey(X509_get_pubkey(signing_cert.get()));
+        EXPECT_TRUE(!!signing_pubkey.get());
+        if (!signing_pubkey.get()) return false;
+
+        EXPECT_EQ(1, X509_verify(key_cert.get(), signing_pubkey.get()))
+                << "Verification of certificate " << i << " failed "
+                << "OpenSSL error string: " << ERR_error_string(ERR_get_error(), NULL);
+
+        char* cert_issuer =  //
+                X509_NAME_oneline(X509_get_issuer_name(key_cert.get()), nullptr, 0);
+        char* signer_subj =
+                X509_NAME_oneline(X509_get_subject_name(signing_cert.get()), nullptr, 0);
+        EXPECT_STREQ(cert_issuer, signer_subj) << "Cert " << i << " has wrong issuer.";
+        if (i == 0) {
+            char* cert_sub = X509_NAME_oneline(X509_get_subject_name(key_cert.get()), nullptr, 0);
+            EXPECT_STREQ("/CN=Android Keystore Key", cert_sub)
+                    << "Cert " << i << " has wrong subject.";
+            OPENSSL_free(cert_sub);
+        }
+
+        OPENSSL_free(cert_issuer);
+        OPENSSL_free(signer_subj);
+
+        if (dump_Attestations) std::cout << bin2hex(chain[i].encodedCertificate) << std::endl;
+    }
+
+    return true;
+}
+
+// Extract attestation record from cert. Returned object is still part of cert; don't free it
+// separately.
+ASN1_OCTET_STRING* get_attestation_record(X509* certificate) {
+    ASN1_OBJECT_Ptr oid(OBJ_txt2obj(kAttestionRecordOid, 1 /* dotted string format */));
+    EXPECT_TRUE(!!oid.get());
+    if (!oid.get()) return nullptr;
+
+    int location = X509_get_ext_by_OBJ(certificate, oid.get(), -1 /* search from beginning */);
+    EXPECT_NE(-1, location) << "Attestation extension not found in certificate";
+    if (location == -1) return nullptr;
+
+    X509_EXTENSION* attest_rec_ext = X509_get_ext(certificate, location);
+    EXPECT_TRUE(!!attest_rec_ext)
+            << "Found attestation extension but couldn't retrieve it?  Probably a BoringSSL bug.";
+    if (!attest_rec_ext) return nullptr;
+
+    ASN1_OCTET_STRING* attest_rec = X509_EXTENSION_get_data(attest_rec_ext);
+    EXPECT_TRUE(!!attest_rec) << "Attestation extension contained no data";
+    return attest_rec;
+}
+
+bool tag_in_list(const KeyParameter& entry) {
+    // Attestations don't contain everything in key authorization lists, so we need to filter
+    // the key lists to produce the lists that we expect to match the attestations.
+    auto tag_list = {
+            Tag::BLOB_USAGE_REQUIREMENTS,  //
+            Tag::CREATION_DATETIME,        //
+            Tag::EC_CURVE,
+            Tag::HARDWARE_TYPE,
+            Tag::INCLUDE_UNIQUE_ID,
+    };
+    return std::find(tag_list.begin(), tag_list.end(), entry.tag) != tag_list.end();
+}
+
+AuthorizationSet filtered_tags(const AuthorizationSet& set) {
+    AuthorizationSet filtered;
+    std::remove_copy_if(set.begin(), set.end(), std::back_inserter(filtered), tag_in_list);
+    return filtered;
+}
+
+bool avb_verification_enabled() {
+    char value[PROPERTY_VALUE_MAX];
+    return property_get("ro.boot.vbmeta.device_state", value, "") != 0;
+}
+
+bool verify_attestation_record(const string& challenge,                //
+                               const string& app_id,                   //
+                               AuthorizationSet expected_sw_enforced,  //
+                               AuthorizationSet expected_hw_enforced,  //
+                               SecurityLevel security_level,
+                               const vector<uint8_t>& attestation_cert) {
+    X509_Ptr cert(parse_cert_blob(attestation_cert));
+    EXPECT_TRUE(!!cert.get());
+    if (!cert.get()) return false;
+
+    ASN1_OCTET_STRING* attest_rec = get_attestation_record(cert.get());
+    EXPECT_TRUE(!!attest_rec);
+    if (!attest_rec) return false;
+
+    AuthorizationSet att_sw_enforced;
+    AuthorizationSet att_hw_enforced;
+    uint32_t att_attestation_version;
+    uint32_t att_keymaster_version;
+    SecurityLevel att_attestation_security_level;
+    SecurityLevel att_keymaster_security_level;
+    vector<uint8_t> att_challenge;
+    vector<uint8_t> att_unique_id;
+    vector<uint8_t> att_app_id;
+
+    auto error = parse_attestation_record(attest_rec->data,                 //
+                                          attest_rec->length,               //
+                                          &att_attestation_version,         //
+                                          &att_attestation_security_level,  //
+                                          &att_keymaster_version,           //
+                                          &att_keymaster_security_level,    //
+                                          &att_challenge,                   //
+                                          &att_sw_enforced,                 //
+                                          &att_hw_enforced,                 //
+                                          &att_unique_id);
+    EXPECT_EQ(ErrorCode::OK, error);
+    if (error != ErrorCode::OK) return false;
+
+    EXPECT_GE(att_attestation_version, 3U);
+
+    expected_sw_enforced.push_back(TAG_ATTESTATION_APPLICATION_ID,
+                                   vector<uint8_t>(app_id.begin(), app_id.end()));
+
+    EXPECT_GE(att_keymaster_version, 4U);
+    EXPECT_EQ(security_level, att_keymaster_security_level);
+    EXPECT_EQ(security_level, att_attestation_security_level);
+
+    EXPECT_EQ(challenge.length(), att_challenge.size());
+    EXPECT_EQ(0, memcmp(challenge.data(), att_challenge.data(), challenge.length()));
+
+    char property_value[PROPERTY_VALUE_MAX] = {};
+    // TODO(b/136282179): When running under VTS-on-GSI the TEE-backed
+    // keymaster implementation will report YYYYMM dates instead of YYYYMMDD
+    // for the BOOT_PATCH_LEVEL.
+    if (avb_verification_enabled()) {
+        for (int i = 0; i < att_hw_enforced.size(); i++) {
+            if (att_hw_enforced[i].tag == TAG_BOOT_PATCHLEVEL ||
+                att_hw_enforced[i].tag == TAG_VENDOR_PATCHLEVEL) {
+                std::string date =
+                        std::to_string(att_hw_enforced[i].value.get<KeyParameterValue::dateTime>());
+                // strptime seems to require delimiters, but the tag value will
+                // be YYYYMMDD
+                date.insert(6, "-");
+                date.insert(4, "-");
+                EXPECT_EQ(date.size(), 10);
+                struct tm time;
+                strptime(date.c_str(), "%Y-%m-%d", &time);
+
+                // Day of the month (0-31)
+                EXPECT_GE(time.tm_mday, 0);
+                EXPECT_LT(time.tm_mday, 32);
+                // Months since Jan (0-11)
+                EXPECT_GE(time.tm_mon, 0);
+                EXPECT_LT(time.tm_mon, 12);
+                // Years since 1900
+                EXPECT_GT(time.tm_year, 110);
+                EXPECT_LT(time.tm_year, 200);
+            }
+        }
+    }
+
+    // Check to make sure boolean values are properly encoded. Presence of a boolean tag indicates
+    // true. A provided boolean tag that can be pulled back out of the certificate indicates correct
+    // encoding. No need to check if it's in both lists, since the AuthorizationSet compare below
+    // will handle mismatches of tags.
+    if (security_level == SecurityLevel::SOFTWARE) {
+        EXPECT_TRUE(expected_sw_enforced.Contains(TAG_NO_AUTH_REQUIRED));
+    } else {
+        EXPECT_TRUE(expected_hw_enforced.Contains(TAG_NO_AUTH_REQUIRED));
+    }
+
+    // Alternatively this checks the opposite - a false boolean tag (one that isn't provided in
+    // the authorization list during key generation) isn't being attested to in the certificate.
+    EXPECT_FALSE(expected_sw_enforced.Contains(TAG_TRUSTED_USER_PRESENCE_REQUIRED));
+    EXPECT_FALSE(att_sw_enforced.Contains(TAG_TRUSTED_USER_PRESENCE_REQUIRED));
+    EXPECT_FALSE(expected_hw_enforced.Contains(TAG_TRUSTED_USER_PRESENCE_REQUIRED));
+    EXPECT_FALSE(att_hw_enforced.Contains(TAG_TRUSTED_USER_PRESENCE_REQUIRED));
+
+    if (att_hw_enforced.Contains(TAG_ALGORITHM, Algorithm::EC)) {
+        // For ECDSA keys, either an EC_CURVE or a KEY_SIZE can be specified, but one must be.
+        EXPECT_TRUE(att_hw_enforced.Contains(TAG_EC_CURVE) ||
+                    att_hw_enforced.Contains(TAG_KEY_SIZE));
+    }
+
+    // Test root of trust elements
+    vector<uint8_t> verified_boot_key;
+    VerifiedBoot verified_boot_state;
+    bool device_locked;
+    vector<uint8_t> verified_boot_hash;
+    error = parse_root_of_trust(attest_rec->data, attest_rec->length, &verified_boot_key,
+                                &verified_boot_state, &device_locked, &verified_boot_hash);
+    EXPECT_EQ(ErrorCode::OK, error);
+
+    if (avb_verification_enabled()) {
+        EXPECT_NE(property_get("ro.boot.vbmeta.digest", property_value, ""), 0);
+        string prop_string(property_value);
+        EXPECT_EQ(prop_string.size(), 64);
+        EXPECT_EQ(prop_string, bin2hex(verified_boot_hash));
+
+        EXPECT_NE(property_get("ro.boot.vbmeta.device_state", property_value, ""), 0);
+        if (!strcmp(property_value, "unlocked")) {
+            EXPECT_FALSE(device_locked);
+        } else {
+            EXPECT_TRUE(device_locked);
+        }
+
+        // Check that the device is locked if not debuggable, e.g., user build
+        // images in CTS. For VTS, debuggable images are used to allow adb root
+        // and the device is unlocked.
+        if (!property_get_bool("ro.debuggable", false)) {
+            EXPECT_TRUE(device_locked);
+        } else {
+            EXPECT_FALSE(device_locked);
+        }
+    }
+
+    // Verified boot key should be all 0's if the boot state is not verified or self signed
+    std::string empty_boot_key(32, '\0');
+    std::string verified_boot_key_str((const char*)verified_boot_key.data(),
+                                      verified_boot_key.size());
+    EXPECT_NE(property_get("ro.boot.verifiedbootstate", property_value, ""), 0);
+    if (!strcmp(property_value, "green")) {
+        EXPECT_EQ(verified_boot_state, VerifiedBoot::VERIFIED);
+        EXPECT_NE(0, memcmp(verified_boot_key.data(), empty_boot_key.data(),
+                            verified_boot_key.size()));
+    } else if (!strcmp(property_value, "yellow")) {
+        EXPECT_EQ(verified_boot_state, VerifiedBoot::SELF_SIGNED);
+        EXPECT_NE(0, memcmp(verified_boot_key.data(), empty_boot_key.data(),
+                            verified_boot_key.size()));
+    } else if (!strcmp(property_value, "orange")) {
+        EXPECT_EQ(verified_boot_state, VerifiedBoot::UNVERIFIED);
+        EXPECT_EQ(0, memcmp(verified_boot_key.data(), empty_boot_key.data(),
+                            verified_boot_key.size()));
+    } else if (!strcmp(property_value, "red")) {
+        EXPECT_EQ(verified_boot_state, VerifiedBoot::FAILED);
+    } else {
+        EXPECT_EQ(verified_boot_state, VerifiedBoot::UNVERIFIED);
+        EXPECT_NE(0, memcmp(verified_boot_key.data(), empty_boot_key.data(),
+                            verified_boot_key.size()));
+    }
+
+    att_sw_enforced.Sort();
+    expected_sw_enforced.Sort();
+    EXPECT_EQ(filtered_tags(expected_sw_enforced), filtered_tags(att_sw_enforced));
+
+    att_hw_enforced.Sort();
+    expected_hw_enforced.Sort();
+    EXPECT_EQ(filtered_tags(expected_hw_enforced), filtered_tags(att_hw_enforced));
+
+    return true;
+}
 
 std::string make_string(const uint8_t* data, size_t length) {
     return std::string(reinterpret_cast<const char*>(data), length);
@@ -289,6 +560,51 @@
 }
 
 /*
+ * NewKeyGenerationTest.Rsa
+ *
+ * Verifies that keymint can generate all required RSA key sizes, and that the resulting keys
+ * have correct characteristics.
+ */
+TEST_P(NewKeyGenerationTest, RsaWithAttestation) {
+    for (auto key_size : ValidKeySizes(Algorithm::RSA)) {
+        auto challenge = "hello";
+        auto app_id = "foo";
+
+        vector<uint8_t> key_blob;
+        vector<KeyCharacteristics> key_characteristics;
+        ASSERT_EQ(ErrorCode::OK, GenerateKey(AuthorizationSetBuilder()
+                                                     .RsaSigningKey(key_size, 65537)
+                                                     .Digest(Digest::NONE)
+                                                     .Padding(PaddingMode::NONE)
+                                                     .AttestationChallenge(challenge)
+                                                     .AttestationApplicationId(app_id)
+                                                     .Authorization(TAG_NO_AUTH_REQUIRED),
+                                             &key_blob, &key_characteristics));
+
+        ASSERT_GT(key_blob.size(), 0U);
+        CheckBaseParams(key_characteristics);
+
+        AuthorizationSet crypto_params = SecLevelAuthorizations(key_characteristics);
+
+        EXPECT_TRUE(crypto_params.Contains(TAG_ALGORITHM, Algorithm::RSA));
+        EXPECT_TRUE(crypto_params.Contains(TAG_KEY_SIZE, key_size))
+                << "Key size " << key_size << "missing";
+        EXPECT_TRUE(crypto_params.Contains(TAG_RSA_PUBLIC_EXPONENT, 65537U));
+
+        EXPECT_TRUE(verify_chain(cert_chain_));
+        ASSERT_GT(cert_chain_.size(), 0);
+
+        AuthorizationSet hw_enforced = HwEnforcedAuthorizations(key_characteristics);
+        AuthorizationSet sw_enforced = SwEnforcedAuthorizations(key_characteristics);
+        EXPECT_TRUE(verify_attestation_record(challenge, app_id,  //
+                                              sw_enforced, hw_enforced, SecLevel(),
+                                              cert_chain_[0].encodedCertificate));
+
+        CheckedDeleteKey(&key_blob);
+    }
+}
+
+/*
  * NewKeyGenerationTest.NoInvalidRsaSizes
  *
  * Verifies that keymint cannot generate any RSA key sizes that are designated as invalid.
@@ -3895,16 +4211,6 @@
 
 INSTANTIATE_KEYMINT_AIDL_TEST(AddEntropyTest);
 
-typedef KeyMintAidlTestBase AttestationTest;
-
-/*
- * AttestationTest.RsaAttestation
- *
- * Verifies that attesting to RSA keys works and generates the expected output.
- */
-// TODO(seleneh) add attestation tests back after decided on the new attestation
-// behavior under generateKey and importKey
-
 typedef KeyMintAidlTestBase KeyDeletionTest;
 
 /**
diff --git a/security/keymint/support/include/keymint_support/authorization_set.h b/security/keymint/support/include/keymint_support/authorization_set.h
index 596bb89..1407c5f 100644
--- a/security/keymint/support/include/keymint_support/authorization_set.h
+++ b/security/keymint/support/include/keymint_support/authorization_set.h
@@ -259,6 +259,12 @@
                              size - 1);  // drop the terminating '\0'
     }
 
+    template <Tag tag>
+    AuthorizationSetBuilder& Authorization(TypedTag<TagType::BYTES, tag> ttag,
+                                           const std::string& data) {
+        return Authorization(ttag, reinterpret_cast<const uint8_t*>(data.data()), data.size());
+    }
+
     AuthorizationSetBuilder& Authorizations(const AuthorizationSet& set) {
         for (const auto& entry : set) {
             push_back(entry);
@@ -294,6 +300,20 @@
     AuthorizationSetBuilder& Digest(std::vector<Digest> digests);
     AuthorizationSetBuilder& Padding(std::initializer_list<PaddingMode> paddings);
 
+    AuthorizationSetBuilder& AttestationChallenge(const std::string& challenge) {
+        return Authorization(TAG_ATTESTATION_CHALLENGE, challenge);
+    }
+    AuthorizationSetBuilder& AttestationChallenge(std::vector<uint8_t> challenge) {
+        return Authorization(TAG_ATTESTATION_CHALLENGE, challenge);
+    }
+
+    AuthorizationSetBuilder& AttestationApplicationId(const std::string& id) {
+        return Authorization(TAG_ATTESTATION_APPLICATION_ID, id);
+    }
+    AuthorizationSetBuilder& AttestationApplicationId(std::vector<uint8_t> id) {
+        return Authorization(TAG_ATTESTATION_APPLICATION_ID, id);
+    }
+
     template <typename... T>
     AuthorizationSetBuilder& BlockMode(T&&... a) {
         return BlockMode({std::forward<T>(a)...});