Merge "Add SCOPED_TRACE to VTS tests that loop over all sensors" into rvc-dev
diff --git a/automotive/vehicle/2.0/types.hal b/automotive/vehicle/2.0/types.hal
index ee34e42..733e7dc 100644
--- a/automotive/vehicle/2.0/types.hal
+++ b/automotive/vehicle/2.0/types.hal
@@ -2813,7 +2813,7 @@
      *
      * To query the association, the Android system gets the property, passing a VehiclePropValue
      * containing the types of associations are being queried, as defined by
-     * UserIdentificationGetRequest. The HAL must return right away, updating the VehiclePropValue
+     * UserIdentificationGetRequest. The HAL must return right away, returning a VehiclePropValue
      * with a UserIdentificationResponse. Notice that user identification should have already
      * happened while system is booting up and the VHAL implementation should only return the
      * already identified association (like the key FOB used to unlock the car), instead of starting
@@ -2828,45 +2828,50 @@
      * For example, to query if the current user (10) is associated with the FOB that unlocked the
      * car and a custom mechanism provided by the OEM, the request would be:
      *
-     * int32[0]: 10  (Android user id)
-     * int32[1]: 0   (Android user flags)
-     * int32[2]: 2   (number of types queried)
-     * int32[3]: 1   (1st type queried, UserIdentificationAssociationType::KEY_FOB)
-     * int32[4]: 101 (2nd type queried, UserIdentificationAssociationType::CUSTOM_1)
+     * int32[0]: 42  // request id
+     * int32[1]: 10  (Android user id)
+     * int32[2]: 0   (Android user flags)
+     * int32[3]: 2   (number of types queried)
+     * int32[4]: 1   (1st type queried, UserIdentificationAssociationType::KEY_FOB)
+     * int32[5]: 101 (2nd type queried, UserIdentificationAssociationType::CUSTOM_1)
      *
      * If the user is associated with the FOB but not with the custom mechanism, the response would
      * be:
      *
-     * int32[9]: 2   (number of associations in the response)
-     * int32[1]: 1   (1st type: UserIdentificationAssociationType::KEY_FOB)
-     * int32[2]: 2   (1st value: UserIdentificationAssociationValue::ASSOCIATED_CURRENT_USER)
-     * int32[3]: 101 (2st type: UserIdentificationAssociationType::CUSTOM_1)
-     * int32[4]: 4   (2nd value: UserIdentificationAssociationValue::NOT_ASSOCIATED_ANY_USER)
+     * int32[0]: 42  // request id
+     * int32[1]: 2   (number of associations in the response)
+     * int32[2]: 1   (1st type: UserIdentificationAssociationType::KEY_FOB)
+     * int32[3]: 2   (1st value: UserIdentificationAssociationValue::ASSOCIATED_CURRENT_USER)
+     * int32[4]: 101 (2st type: UserIdentificationAssociationType::CUSTOM_1)
+     * int32[5]: 4   (2nd value: UserIdentificationAssociationValue::NOT_ASSOCIATED_ANY_USER)
      *
      * Then to associate the user with the custom mechanism, a set request would be made:
      *
-     * int32[0]: 10  (Android user id)
-     * int32[0]: 0   (Android user flags)
-     * int32[1]: 1   (number of associations being set)
-     * int32[2]: 101 (1st type: UserIdentificationAssociationType::CUSTOM_1)
-     * int32[3]: 1   (1st value: UserIdentificationAssociationSetValue::ASSOCIATE_CURRENT_USER)
+     * int32[0]: 42  // request id
+     * int32[1]: 10  (Android user id)
+     * int32[2]: 0   (Android user flags)
+     * int32[3]: 1   (number of associations being set)
+     * int32[4]: 101 (1st type: UserIdentificationAssociationType::CUSTOM_1)
+     * int32[5]: 1   (1st value: UserIdentificationAssociationSetValue::ASSOCIATE_CURRENT_USER)
      *
      * If the request succeeded, the response would be simply:
      *
-     * int32[0]: 2   (number of associations in the response)
-     * int32[1]: 101 (1st type: UserIdentificationAssociationType::CUSTOM_1)
-     * int32[2]: 1   (1st value: UserIdentificationAssociationValue::ASSOCIATED_CURRENT_USER)
+     * int32[0]: 42  // request id
+     * int32[1]: 1   (number of associations in the response)
+     * int32[2]: 101 (1st type: UserIdentificationAssociationType::CUSTOM_1)
+     * int32[3]: 1   (1st value: UserIdentificationAssociationValue::ASSOCIATED_CURRENT_USER)
      *
      * Notice that the set request adds associations, but doesn't remove the existing ones. In the
      * example above, the end state would be 2 associations (FOB and CUSTOM_1). If we wanted to
      * associate the user with just CUSTOM_1 but not FOB, then the request should have been:
      *
-     * int32[0]: 10  (Android user id)
-     * int32[1]: 2   (number of types set)
-     * int32[2]: 1   (1st type: UserIdentificationAssociationType::KEY_FOB)
-     * int32[3]: 2   (1st value: UserIdentificationAssociationValue::DISASSOCIATE_CURRENT_USER)
-     * int32[3]: 101 (2nd type: UserIdentificationAssociationType::CUSTOM_1)
-     * int32[5]: 1   (2nd value: UserIdentificationAssociationValue::ASSOCIATE_CURRENT_USER)
+     * int32[0]: 42  // request id
+     * int32[1]: 10  (Android user id)
+     * int32[2]: 2   (number of types set)
+     * int32[3]: 1   (1st type: UserIdentificationAssociationType::KEY_FOB)
+     * int32[4]: 2   (1st value: UserIdentificationAssociationValue::DISASSOCIATE_CURRENT_USER)
+     * int32[5]: 101 (2nd type: UserIdentificationAssociationType::CUSTOM_1)
+     * int32[6]: 1   (2nd value: UserIdentificationAssociationValue::ASSOCIATE_CURRENT_USER)
      *
      * @change_mode VehiclePropertyChangeMode:ON_CHANGE
      * @access VehiclePropertyAccess:READ_WRITE
@@ -4639,6 +4644,11 @@
  */
 struct UserIdentificationGetRequest {
     /**
+     * Id of the request being responded.
+     */
+    UserRequestId requestId;
+
+    /**
      * Information about the current foreground Android user.
      */
     UserInfo userInfo;
@@ -4662,6 +4672,11 @@
  */
 struct UserIdentificationSetRequest {
     /**
+     * Id of the request being responded.
+     */
+    UserRequestId requestId;
+
+    /**
      * Information about the current foreground Android user.
      */
     UserInfo userInfo;
@@ -4674,7 +4689,7 @@
     /**
      * Associations being set.
      */
-    vec<UserIdentificationAssociationSetValue> associations;
+    vec<UserIdentificationSetAssociation> associations;
 };
 
 /**
@@ -4685,6 +4700,11 @@
  */
 struct UserIdentificationResponse {
     /**
+     * Id of the request being responded.
+     */
+    UserRequestId requestId;
+
+    /**
      * Number of associations being returned.
      */
     int32_t numberAssociation;
diff --git a/graphics/composer/2.4/vts/functional/VtsHalGraphicsComposerV2_4TargetTest.cpp b/graphics/composer/2.4/vts/functional/VtsHalGraphicsComposerV2_4TargetTest.cpp
index 7a00ed2..27b633a 100644
--- a/graphics/composer/2.4/vts/functional/VtsHalGraphicsComposerV2_4TargetTest.cpp
+++ b/graphics/composer/2.4/vts/functional/VtsHalGraphicsComposerV2_4TargetTest.cpp
@@ -204,7 +204,7 @@
     void Test_setActiveConfigWithConstraints(
             const IComposerClient::VsyncPeriodChangeConstraints& constraints, bool refreshMiss);
 
-    void sendRefreshFrame(const VsyncPeriodChangeTimeline&);
+    void sendRefreshFrame(const VsyncPeriodChangeTimeline*);
 
     void waitForVsyncPeriodChange(Display display, const VsyncPeriodChangeTimeline& timeline,
                                   int64_t desiredTimeNanos, int64_t oldPeriodNanos,
@@ -294,7 +294,7 @@
                                            display, config, constraints, &timeline));
 
             if (timeline.refreshRequired) {
-                sendRefreshFrame(timeline);
+                sendRefreshFrame(&timeline);
             }
             waitForVsyncPeriodChange(display, timeline, constraints.desiredTimeNanos, 0,
                                      expectedVsyncPeriodNanos);
@@ -350,7 +350,7 @@
     }
 }
 
-TEST_P(GraphicsComposerHidlTest, setActiveConfigWithConstraints_SeamlessNotAllowed) {
+TEST_P(GraphicsComposerHidlCommandTest, setActiveConfigWithConstraints_SeamlessNotAllowed) {
     VsyncPeriodChangeTimeline timeline;
     IComposerClient::VsyncPeriodChangeConstraints constraints;
 
@@ -365,6 +365,7 @@
                     display, config2, IComposerClient::IComposerClient::Attribute::CONFIG_GROUP);
             if (configGroup1 != configGroup2) {
                 mComposerClient->setActiveConfig(display, config1);
+                sendRefreshFrame(nullptr);
                 EXPECT_EQ(Error::SEAMLESS_NOT_ALLOWED,
                           mComposerClient->setActiveConfigWithConstraints(display, config2,
                                                                           constraints, &timeline));
@@ -377,11 +378,13 @@
     return std::chrono::time_point<std::chrono::steady_clock>(std::chrono::nanoseconds(time));
 }
 
-void GraphicsComposerHidlCommandTest::sendRefreshFrame(const VsyncPeriodChangeTimeline& timeline) {
-    // Refresh time should be before newVsyncAppliedTimeNanos
-    EXPECT_LT(timeline.refreshTimeNanos, timeline.newVsyncAppliedTimeNanos);
+void GraphicsComposerHidlCommandTest::sendRefreshFrame(const VsyncPeriodChangeTimeline* timeline) {
+    if (timeline != nullptr) {
+        // Refresh time should be before newVsyncAppliedTimeNanos
+        EXPECT_LT(timeline->refreshTimeNanos, timeline->newVsyncAppliedTimeNanos);
 
-    std::this_thread::sleep_until(toTimePoint(timeline.refreshTimeNanos));
+        std::this_thread::sleep_until(toTimePoint(timeline->refreshTimeNanos));
+    }
 
     mWriter->selectDisplay(mPrimaryDisplay);
     mComposerClient->setPowerMode(mPrimaryDisplay, V2_1::IComposerClient::PowerMode::ON);
@@ -453,6 +456,7 @@
     for (Display display : mComposerCallback->getDisplays()) {
         forEachTwoConfigs(display, [&](Config config1, Config config2) {
             mComposerClient->setActiveConfig(display, config1);
+            sendRefreshFrame(nullptr);
 
             int32_t vsyncPeriod1 = mComposerClient->getDisplayAttribute_2_4(
                     display, config1, IComposerClient::IComposerClient::Attribute::VSYNC_PERIOD);
@@ -478,7 +482,7 @@
                     // callback
                     std::this_thread::sleep_until(toTimePoint(timeline.refreshTimeNanos) + 100ms);
                 }
-                sendRefreshFrame(timeline);
+                sendRefreshFrame(&timeline);
             }
             waitForVsyncPeriodChange(display, timeline, constraints.desiredTimeNanos, vsyncPeriod1,
                                      vsyncPeriod2);
@@ -493,7 +497,7 @@
 
             if (newTimelime.has_value()) {
                 if (timeline.refreshRequired) {
-                    sendRefreshFrame(newTimelime.value());
+                    sendRefreshFrame(&newTimelime.value());
                 }
                 waitForVsyncPeriodChange(display, newTimelime.value(), constraints.desiredTimeNanos,
                                          vsyncPeriod1, vsyncPeriod2);
diff --git a/keymaster/4.1/support/attestation_record.cpp b/keymaster/4.1/support/attestation_record.cpp
index 63bf854..598b6b5 100644
--- a/keymaster/4.1/support/attestation_record.cpp
+++ b/keymaster/4.1/support/attestation_record.cpp
@@ -296,6 +296,10 @@
 std::tuple<ErrorCode, AttestationRecord> parse_attestation_record(const hidl_vec<uint8_t>& cert) {
     const uint8_t* p = cert.data();
     X509_Ptr x509(d2i_X509(nullptr, &p, cert.size()));
+    if (!x509.get()) {
+        LOG(ERROR) << "Error converting DER";
+        return {ErrorCode::INVALID_ARGUMENT, {}};
+    }
 
     ASN1_OBJECT_Ptr oid(OBJ_txt2obj(kAttestionRecordOid, 1 /* dotted string format */));
     if (!oid.get()) {
diff --git a/keymaster/4.1/vts/functional/DeviceUniqueAttestationTest.cpp b/keymaster/4.1/vts/functional/DeviceUniqueAttestationTest.cpp
index 7ea3275..495de0f 100644
--- a/keymaster/4.1/vts/functional/DeviceUniqueAttestationTest.cpp
+++ b/keymaster/4.1/vts/functional/DeviceUniqueAttestationTest.cpp
@@ -14,6 +14,9 @@
  * limitations under the License.
  */
 
+#define LOG_TAG "keymaster_hidl_hal_test"
+#include <cutils/log.h>
+
 #include "Keymaster4_1HidlTest.h"
 
 #include <cutils/properties.h>
@@ -23,6 +26,10 @@
 #include <keymasterV4_1/attestation_record.h>
 #include <keymasterV4_1/authorization_set.h>
 
+// Not to dump the attestation by default. Can enable by specify the parameter
+// "--dump_attestations" on lunching VTS
+static bool dumpAttestations = false;
+
 namespace android::hardware::keymaster::V4_0 {
 
 bool operator==(const AuthorizationSet& a, const AuthorizationSet& b) {
@@ -57,6 +64,10 @@
     return retval;
 }
 
+inline void dumpContent(string content) {
+    std::cout << content << std::endl;
+}
+
 struct AuthorizationSetDifferences {
     string aName;
     string bName;
@@ -126,6 +137,23 @@
     }
 }
 
+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::INCLUDE_UNIQUE_ID, Tag::BLOB_USAGE_REQUIREMENTS, Tag::EC_CURVE,
+            Tag::HARDWARE_TYPE,     Tag::VENDOR_PATCHLEVEL,       Tag::BOOT_PATCHLEVEL,
+            Tag::CREATION_DATETIME,
+    };
+    return std::find(tag_list.begin(), tag_list.end(), (V4_1::Tag)entry.tag) != tag_list.end();
+}
+
+AuthorizationSet filter_tags(const AuthorizationSet& set) {
+    AuthorizationSet filtered;
+    std::remove_copy_if(set.begin(), set.end(), std::back_inserter(filtered), tag_in_list);
+    return filtered;
+}
+
 void check_attestation_record(AttestationRecord attestation, const HidlBuf& challenge,
                               AuthorizationSet expected_sw_enforced,
                               AuthorizationSet expected_hw_enforced,
@@ -144,9 +172,9 @@
     attestation.software_enforced.Sort();
     attestation.hardware_enforced.Sort();
 
-    EXPECT_EQ(expected_sw_enforced, attestation.software_enforced)
+    EXPECT_EQ(filter_tags(expected_sw_enforced), filter_tags(attestation.software_enforced))
             << DIFFERENCE(expected_sw_enforced, attestation.software_enforced);
-    EXPECT_EQ(expected_hw_enforced, attestation.hardware_enforced)
+    EXPECT_EQ(filter_tags(expected_hw_enforced), filter_tags(attestation.hardware_enforced))
             << DIFFERENCE(expected_hw_enforced, attestation.hardware_enforced);
 }
 
@@ -155,8 +183,8 @@
 using std::string;
 using DeviceUniqueAttestationTest = Keymaster4_1HidlTest;
 
-TEST_P(DeviceUniqueAttestationTest, StrongBoxOnly) {
-    if (SecLevel() != SecurityLevel::STRONGBOX) return;
+TEST_P(DeviceUniqueAttestationTest, NonStrongBoxOnly) {
+    if (SecLevel() == SecurityLevel::STRONGBOX) return;
 
     ASSERT_EQ(ErrorCode::OK, convert(GenerateKey(AuthorizationSetBuilder()
                                                          .Authorization(TAG_NO_AUTH_REQUIRED)
@@ -193,11 +221,11 @@
     if (SecLevel() != SecurityLevel::STRONGBOX) return;
     ASSERT_EQ(ErrorCode::OK,
               convert(GenerateKey(AuthorizationSetBuilder()
-                                          .Authorization(TAG_NO_AUTH_REQUIRED)
-                                          .RsaSigningKey(2048, 65537)
-                                          .Digest(Digest::SHA_2_256)
-                                          .Padding(PaddingMode::RSA_PKCS1_1_5_SIGN)
-                                          .Authorization(TAG_CREATION_DATETIME, 1))));
+                                  .Authorization(TAG_NO_AUTH_REQUIRED)
+                                  .RsaSigningKey(2048, 65537)
+                                  .Digest(Digest::SHA_2_256)
+                                  .Padding(PaddingMode::RSA_PKCS1_1_5_SIGN)
+                                  .Authorization(TAG_INCLUDE_UNIQUE_ID))));
 
     hidl_vec<hidl_vec<uint8_t>> cert_chain;
     HidlBuf challenge("challenge");
@@ -209,14 +237,14 @@
                                         .Authorization(TAG_ATTESTATION_APPLICATION_ID, app_id),
                                 &cert_chain)));
 
-    EXPECT_EQ(1U, cert_chain.size());
+    EXPECT_EQ(2U, cert_chain.size());
+    if (dumpAttestations) dumpContent(bin2hex(cert_chain[0]));
     auto [err, attestation] = parse_attestation_record(cert_chain[0]);
-    EXPECT_EQ(ErrorCode::OK, err);
+    ASSERT_EQ(ErrorCode::OK, err);
 
     check_attestation_record(attestation, challenge,
                              /* sw_enforced */
                              AuthorizationSetBuilder()
-                                     .Authorization(TAG_CREATION_DATETIME, 1)
                                      .Authorization(TAG_ATTESTATION_APPLICATION_ID, app_id),
                              /* hw_enforced */
                              AuthorizationSetBuilder()
@@ -238,7 +266,7 @@
                                           .Authorization(TAG_NO_AUTH_REQUIRED)
                                           .EcdsaSigningKey(256)
                                           .Digest(Digest::SHA_2_256)
-                                          .Authorization(TAG_CREATION_DATETIME, 1))));
+                                          .Authorization(TAG_INCLUDE_UNIQUE_ID))));
 
     hidl_vec<hidl_vec<uint8_t>> cert_chain;
     HidlBuf challenge("challenge");
@@ -250,29 +278,42 @@
                                         .Authorization(TAG_ATTESTATION_APPLICATION_ID, app_id),
                                 &cert_chain)));
 
-    EXPECT_EQ(1U, cert_chain.size());
+    EXPECT_EQ(2U, cert_chain.size());
+    if (dumpAttestations) dumpContent(bin2hex(cert_chain[0]));
     auto [err, attestation] = parse_attestation_record(cert_chain[0]);
-    EXPECT_EQ(ErrorCode::OK, err);
+    ASSERT_EQ(ErrorCode::OK, err);
 
     check_attestation_record(attestation, challenge,
-                             /* sw_enforced */
-                             AuthorizationSetBuilder()
-                                     .Authorization(TAG_CREATION_DATETIME, 1)
-                                     .Authorization(TAG_ATTESTATION_APPLICATION_ID, app_id),
-                             /* hw_enforced */
-                             AuthorizationSetBuilder()
-                                     .Authorization(TAG_DEVICE_UNIQUE_ATTESTATION)
-                                     .Authorization(TAG_NO_AUTH_REQUIRED)
-                                     .EcdsaSigningKey(256)
-                                     .Digest(Digest::SHA_2_256)
-                                     .Authorization(TAG_EC_CURVE, EcCurve::P_256)
-                                     .Authorization(TAG_ORIGIN, KeyOrigin::GENERATED)
-                                     .Authorization(TAG_OS_VERSION, os_version())
-                                     .Authorization(TAG_OS_PATCHLEVEL, os_patch_level()),
-                             SecLevel());
+            /* sw_enforced */
+            AuthorizationSetBuilder().Authorization(TAG_ATTESTATION_APPLICATION_ID, app_id),
+            /* hw_enforced */
+            AuthorizationSetBuilder()
+                    .Authorization(TAG_DEVICE_UNIQUE_ATTESTATION)
+                    .Authorization(TAG_NO_AUTH_REQUIRED)
+                    .EcdsaSigningKey(256)
+                    .Digest(Digest::SHA_2_256)
+                    .Authorization(TAG_EC_CURVE, EcCurve::P_256)
+                    .Authorization(TAG_ORIGIN, KeyOrigin::GENERATED)
+                    .Authorization(TAG_OS_VERSION, os_version())
+                    .Authorization(TAG_OS_PATCHLEVEL, os_patch_level()),
+            SecLevel());
 }
 
 INSTANTIATE_KEYMASTER_4_1_HIDL_TEST(DeviceUniqueAttestationTest);
 
 }  // namespace test
 }  // namespace android::hardware::keymaster::V4_1
+
+int main(int argc, char** argv) {
+    ::testing::InitGoogleTest(&argc, argv);
+    for (int i = 1; i < argc; ++i) {
+        if (argv[i][0] == '-') {
+            if (std::string(argv[i]) == "--dump_attestations") {
+                dumpAttestations = true;
+            }
+        }
+    }
+    int status = RUN_ALL_TESTS();
+    ALOGI("Test result = %d", status);
+    return status;
+}