Merge "Test validity of device-unique attestation chain" into sc-dev
diff --git a/gnss/aidl/vts/gnss_hal_test_cases.cpp b/gnss/aidl/vts/gnss_hal_test_cases.cpp
index 9d8562d..5964f81 100644
--- a/gnss/aidl/vts/gnss_hal_test_cases.cpp
+++ b/gnss/aidl/vts/gnss_hal_test_cases.cpp
@@ -311,27 +311,42 @@
     EXPECT_TRUE(gnssPowerIndicationCallback->gnss_power_stats_cbq_.retrieve(
             gnssPowerIndicationCallback->last_gnss_power_stats_, kTimeoutSec));
     EXPECT_EQ(gnssPowerIndicationCallback->gnss_power_stats_cbq_.calledCount(), 2);
+
     auto powerStats2 = gnssPowerIndicationCallback->last_gnss_power_stats_;
 
-    // Elapsed realtime must increase
-    EXPECT_GT(powerStats2.elapsedRealtime.timestampNs, powerStats1.elapsedRealtime.timestampNs);
+    if ((gnssPowerIndicationCallback->last_capabilities_ &
+         (int)GnssPowerIndicationCallback::CAPABILITY_TOTAL)) {
+        // Elapsed realtime must increase
+        EXPECT_GT(powerStats2.elapsedRealtime.timestampNs, powerStats1.elapsedRealtime.timestampNs);
 
-    // Total energy must increase
-    EXPECT_GT(powerStats2.totalEnergyMilliJoule, powerStats1.totalEnergyMilliJoule);
+        // Total energy must increase
+        EXPECT_GT(powerStats2.totalEnergyMilliJoule, powerStats1.totalEnergyMilliJoule);
+    }
 
     // At least oone of singleband and multiband acquisition energy must increase
     bool singlebandAcqEnergyIncreased = powerStats2.singlebandAcquisitionModeEnergyMilliJoule >
                                         powerStats1.singlebandAcquisitionModeEnergyMilliJoule;
     bool multibandAcqEnergyIncreased = powerStats2.multibandAcquisitionModeEnergyMilliJoule >
                                        powerStats1.multibandAcquisitionModeEnergyMilliJoule;
-    EXPECT_TRUE(singlebandAcqEnergyIncreased || multibandAcqEnergyIncreased);
+
+    if ((gnssPowerIndicationCallback->last_capabilities_ &
+         (int)GnssPowerIndicationCallback::CAPABILITY_SINGLEBAND_ACQUISITION) ||
+        (gnssPowerIndicationCallback->last_capabilities_ &
+         (int)GnssPowerIndicationCallback::CAPABILITY_MULTIBAND_ACQUISITION)) {
+        EXPECT_TRUE(singlebandAcqEnergyIncreased || multibandAcqEnergyIncreased);
+    }
 
     // At least one of singleband and multiband tracking energy must increase
     bool singlebandTrackingEnergyIncreased = powerStats2.singlebandTrackingModeEnergyMilliJoule >
                                              powerStats1.singlebandTrackingModeEnergyMilliJoule;
     bool multibandTrackingEnergyIncreased = powerStats2.multibandTrackingModeEnergyMilliJoule >
                                             powerStats1.multibandTrackingModeEnergyMilliJoule;
-    EXPECT_TRUE(singlebandTrackingEnergyIncreased || multibandTrackingEnergyIncreased);
+    if ((gnssPowerIndicationCallback->last_capabilities_ &
+         (int)GnssPowerIndicationCallback::CAPABILITY_SINGLEBAND_TRACKING) ||
+        (gnssPowerIndicationCallback->last_capabilities_ &
+         (int)GnssPowerIndicationCallback::CAPABILITY_MULTIBAND_TRACKING)) {
+        EXPECT_TRUE(singlebandTrackingEnergyIncreased || multibandTrackingEnergyIncreased);
+    }
 
     // Clean up
     StopAndClearLocations();
diff --git a/graphics/composer/2.1/vts/OWNERS b/graphics/composer/2.1/vts/OWNERS
index 0b42d2e..ea06752 100644
--- a/graphics/composer/2.1/vts/OWNERS
+++ b/graphics/composer/2.1/vts/OWNERS
@@ -1,6 +1,6 @@
 # Graphics team
+adyabr@google.com
 lpy@google.com
-vhau@google.com
 
 # VTS team
 yim@google.com
diff --git a/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl b/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl
index 4ff4574..58e02b3 100644
--- a/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl
+++ b/security/keymint/aidl/android/hardware/security/keymint/Tag.aidl
@@ -831,14 +831,24 @@
     /**
      * DEVICE_UNIQUE_ATTESTATION is an argument to IKeyMintDevice::attested key generation/import
      * operations.  It indicates that attestation using a device-unique key is requested, rather
-     * than a batch key.  When a device-unique key is used, only the attestation certificate is
-     * returned; no additional chained certificates are provided.  It's up to the caller to
-     * recognize the device-unique signing key.  Only SecurityLevel::STRONGBOX IKeyMintDevices may
-     * support device-unique attestations.  SecurityLevel::TRUSTED_ENVIRONMENT IKeyMintDevices must
-     * return ErrorCode::INVALID_ARGUMENT if they receive DEVICE_UNIQUE_ATTESTATION.
+     * than a batch key.  When a device-unique key is used, the returned chain should contain two
+     * certificates:
+     *    * The attestation certificate, containing the attestation extension, as described in
+            KeyCreationResult.aidl.
+     *    * A self-signed root certificate, signed by the device-unique key.
+     * No additional chained certificates are provided. Only SecurityLevel::STRONGBOX
+     * IKeyMintDevices may support device-unique attestations.  SecurityLevel::TRUSTED_ENVIRONMENT
+     * IKeyMintDevices must return ErrorCode::INVALID_ARGUMENT if they receive
+     * DEVICE_UNIQUE_ATTESTATION.
      * SecurityLevel::STRONGBOX IKeyMintDevices need not support DEVICE_UNIQUE_ATTESTATION, and
      * return ErrorCode::CANNOT_ATTEST_IDS if they do not support it.
      *
+     * The caller needs to obtain the device-unique keys out-of-band and compare them against the
+     * key used to sign the self-signed root certificate.
+     * To ease this process, the IKeyMintDevice implementation should include, both in the subject
+     * and issuer fields of the self-signed root, the unique identifier of the device. Using the
+     * unique identifier will make it straightforward for the caller to link a device to its key.
+     *
      * IKeyMintDevice implementations that support device-unique attestation MUST add the
      * DEVICE_UNIQUE_ATTESTATION tag to device-unique attestations.
      */
diff --git a/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp b/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp
index b0f056a..732d9eb 100644
--- a/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp
+++ b/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp
@@ -42,8 +42,11 @@
 
         EXPECT_TRUE(crypto_params.Contains(TAG_KEY_SIZE, key_size)) << "Key size missing";
 
+        // The device-unique attestation chain should contain exactly two certificates:
+        // * The leaf with the attestation extension.
+        // * A self-signed root, signed using the device-unique key.
+        ASSERT_EQ(cert_chain_.size(), 2);
         EXPECT_TRUE(ChainSignaturesAreValid(cert_chain_));
-        ASSERT_GT(cert_chain_.size(), 0);
 
         AuthorizationSet sw_enforced = SwEnforcedAuthorizations(key_characteristics);
         EXPECT_TRUE(verify_attestation_record("challenge", "foo", sw_enforced, hw_enforced,
diff --git a/security/keymint/support/keymint_utils.cpp b/security/keymint/support/keymint_utils.cpp
index 2dbdfa8..1e0733f 100644
--- a/security/keymint/support/keymint_utils.cpp
+++ b/security/keymint/support/keymint_utils.cpp
@@ -32,10 +32,11 @@
 
 constexpr char kPlatformPatchlevelProp[] = "ro.build.version.security_patch";
 constexpr char kVendorPatchlevelProp[] = "ro.vendor.build.security_patch";
-constexpr char kPatchlevelRegex[] = "^([0-9]{4})-([0-9]{2})-[0-9]{2}$";
+constexpr char kPatchlevelRegex[] = "^([0-9]{4})-([0-9]{2})-([0-9]{2})$";
 constexpr size_t kYearMatch = 1;
 constexpr size_t kMonthMatch = 2;
-constexpr size_t kPatchlevelMatchCount = kMonthMatch + 1;
+constexpr size_t kDayMatch = 3;
+constexpr size_t kPatchlevelMatchCount = kDayMatch + 1;
 
 uint32_t match_to_uint32(const char* expression, const regmatch_t& match) {
     if (match.rm_so == -1) return 0;
@@ -53,8 +54,6 @@
     return prop_value;
 }
 
-}  // anonymous namespace
-
 uint32_t getOsVersion(const char* version_str) {
     regex_t regex;
     if (regcomp(&regex, kPlatformVersionRegex, REG_EXTENDED)) {
@@ -76,12 +75,9 @@
     return (major * 100 + minor) * 100 + subminor;
 }
 
-uint32_t getOsVersion() {
-    std::string version = wait_and_get_property(kPlatformVersionProp);
-    return getOsVersion(version.c_str());
-}
+enum class PatchlevelOutput { kYearMonthDay, kYearMonth };
 
-uint32_t getPatchlevel(const char* patchlevel_str) {
+uint32_t getPatchlevel(const char* patchlevel_str, PatchlevelOutput detail) {
     regex_t regex;
     if (regcomp(&regex, kPatchlevelRegex, REG_EXTENDED) != 0) {
         return 0;
@@ -100,17 +96,35 @@
     if (month < 1 || month > 12) {
         return 0;
     }
-    return year * 100 + month;
+
+    switch (detail) {
+        case PatchlevelOutput::kYearMonthDay: {
+            uint32_t day = match_to_uint32(patchlevel_str, matches[kDayMatch]);
+            if (day < 1 || day > 31) {
+                return 0;
+            }
+            return year * 10000 + month * 100 + day;
+        }
+        case PatchlevelOutput::kYearMonth:
+            return year * 100 + month;
+    }
+}
+
+}  // anonymous namespace
+
+uint32_t getOsVersion() {
+    std::string version = wait_and_get_property(kPlatformVersionProp);
+    return getOsVersion(version.c_str());
 }
 
 uint32_t getOsPatchlevel() {
     std::string patchlevel = wait_and_get_property(kPlatformPatchlevelProp);
-    return getPatchlevel(patchlevel.c_str());
+    return getPatchlevel(patchlevel.c_str(), PatchlevelOutput::kYearMonth);
 }
 
 uint32_t getVendorPatchlevel() {
     std::string patchlevel = wait_and_get_property(kVendorPatchlevelProp);
-    return getPatchlevel(patchlevel.c_str());
+    return getPatchlevel(patchlevel.c_str(), PatchlevelOutput::kYearMonthDay);
 }
 
 }  // namespace aidl::android::hardware::security::keymint
diff --git a/security/secureclock/aidl/vts/functional/SecureClockAidlTest.cpp b/security/secureclock/aidl/vts/functional/SecureClockAidlTest.cpp
index 31f4854..bf332d5 100644
--- a/security/secureclock/aidl/vts/functional/SecureClockAidlTest.cpp
+++ b/security/secureclock/aidl/vts/functional/SecureClockAidlTest.cpp
@@ -114,6 +114,7 @@
     EXPECT_EQ(ErrorCode::OK, result1.error);
     EXPECT_EQ(1U, result1.token.challenge);
     EXPECT_GT(result1.token.timestamp.milliSeconds, 0U);
+    EXPECT_EQ(32U, result1.token.mac.size());
 
     unsigned long time_to_sleep = 200;
     sleep_ms(time_to_sleep);
@@ -123,6 +124,7 @@
     EXPECT_EQ(ErrorCode::OK, result2.error);
     EXPECT_EQ(2U, result2.token.challenge);
     EXPECT_GT(result2.token.timestamp.milliSeconds, 0U);
+    EXPECT_EQ(32U, result2.token.mac.size());
 
     auto host_time_delta = result2_time - result1_time;
 
@@ -153,6 +155,7 @@
     EXPECT_EQ(ErrorCode::OK, result1.error);
     EXPECT_EQ(0U, result1.token.challenge);
     EXPECT_GT(result1.token.timestamp.milliSeconds, 0U);
+    EXPECT_EQ(32U, result1.token.mac.size());
 
     unsigned long time_to_sleep = 200;
     sleep_ms(time_to_sleep);
@@ -162,6 +165,7 @@
     EXPECT_EQ(ErrorCode::OK, result2.error);
     EXPECT_EQ(1U, result2.token.challenge);
     EXPECT_GT(result2.token.timestamp.milliSeconds, 0U);
+    EXPECT_EQ(32U, result2.token.mac.size());
 
     auto host_time_delta = result2_time - result1_time;
 
diff --git a/security/sharedsecret/aidl/vts/functional/SharedSecretAidlTest.cpp b/security/sharedsecret/aidl/vts/functional/SharedSecretAidlTest.cpp
index 8426120..919f882 100644
--- a/security/sharedsecret/aidl/vts/functional/SharedSecretAidlTest.cpp
+++ b/security/sharedsecret/aidl/vts/functional/SharedSecretAidlTest.cpp
@@ -48,6 +48,9 @@
         SharedSecretParameters params;
         auto error = GetReturnErrorCode(sharedSecret->getSharedSecretParameters(&params));
         EXPECT_EQ(ErrorCode::OK, error);
+        EXPECT_TRUE(params.seed.size() == 0 || params.seed.size() == 32);
+        EXPECT_TRUE(params.nonce.size() == 32);
+
         GetParamsResult result;
         result.tie() = std::tie(error, params);
         return result;
@@ -234,6 +237,45 @@
     }
 }
 
+TEST_F(SharedSecretAidlTest, ComputeSharedSecretShortNonce) {
+    auto sharedSecrets = allSharedSecrets();
+    if (sharedSecrets.empty()) {
+        GTEST_SKIP() << "Skipping the test as no shared secret service is found.";
+    }
+    auto fixup_hmac = finally([&]() { computeAllSharedSecrets(getAllSharedSecretParameters()); });
+
+    auto params = getAllSharedSecretParameters();
+    ASSERT_EQ(sharedSecrets.size(), params.size())
+            << "One or more shared secret services failed to provide parameters.";
+
+    // All should be well in the normal case
+    auto responses = computeAllSharedSecrets(params);
+
+    ASSERT_GT(responses.size(), 0U);
+    vector<uint8_t> correct_response = responses[0].sharing_check;
+    verifyResponses(correct_response, responses);
+
+    // Pick a random param and shorten that nonce by one.
+    size_t param_to_tweak = rand() % params.size();
+    auto& to_tweak = params[param_to_tweak].nonce;
+    ASSERT_TRUE(to_tweak.size() == 32);
+    to_tweak.resize(31);
+
+    responses = computeAllSharedSecrets(params);
+    for (size_t i = 0; i < responses.size(); ++i) {
+        if (i == param_to_tweak) {
+            EXPECT_EQ(ErrorCode::INVALID_ARGUMENT, responses[i].error)
+                    << "Shared secret service that provided tweaked param should fail to compute "
+                       "shared secret";
+        } else {
+            EXPECT_EQ(ErrorCode::OK, responses[i].error) << "Others should succeed";
+            EXPECT_NE(correct_response, responses[i].sharing_check)
+                    << "Others should calculate a different shared secret, due to the tweaked "
+                       "nonce.";
+        }
+    }
+}
+
 TEST_F(SharedSecretAidlTest, ComputeSharedSecretCorruptSeed) {
     auto sharedSecrets = allSharedSecrets();
     if (sharedSecrets.empty()) {
@@ -275,6 +317,45 @@
         }
     }
 }
+
+TEST_F(SharedSecretAidlTest, ComputeSharedSecretShortSeed) {
+    auto sharedSecrets = allSharedSecrets();
+    if (sharedSecrets.empty()) {
+        GTEST_SKIP() << "Skipping the test as no shared secret service is found.";
+    }
+    auto fixup_hmac = finally([&]() { computeAllSharedSecrets(getAllSharedSecretParameters()); });
+    auto params = getAllSharedSecretParameters();
+    ASSERT_EQ(sharedSecrets.size(), params.size())
+            << "One or more shared secret service failed to provide parameters.";
+
+    // All should be well in the normal case
+    auto responses = computeAllSharedSecrets(params);
+
+    ASSERT_GT(responses.size(), 0U);
+    vector<uint8_t> correct_response = responses[0].sharing_check;
+    verifyResponses(correct_response, responses);
+
+    // Pick a random param and modify the seed to be of (invalid) length 31.
+    auto param_to_tweak = rand() % params.size();
+    auto& to_tweak = params[param_to_tweak].seed;
+    ASSERT_TRUE(to_tweak.size() == 32 || to_tweak.size() == 0);
+    to_tweak.resize(31);
+
+    responses = computeAllSharedSecrets(params);
+    for (size_t i = 0; i < responses.size(); ++i) {
+        if (i == param_to_tweak) {
+            EXPECT_EQ(ErrorCode::INVALID_ARGUMENT, responses[i].error)
+                    << "Shared secret service that provided tweaked param should fail to compute "
+                       "shared secret";
+        } else {
+            EXPECT_EQ(ErrorCode::OK, responses[i].error) << "Others should succeed";
+            EXPECT_NE(correct_response, responses[i].sharing_check)
+                    << "Others should calculate a different shared secret, due to the tweaked "
+                       "nonce.";
+        }
+    }
+}
+
 }  // namespace aidl::android::hardware::security::sharedsecret::test
 
 int main(int argc, char** argv) {