Merge "Clarify error message in DeviceInfo check." am: 0068fccb41 am: 3e50762f9e am: 7a556052fd am: 1c83972fb8

Original change: https://android-review.googlesource.com/c/platform/hardware/interfaces/+/2210300

Change-Id: I60f8fbe78317225e624140b465e1490cc5d55602
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp b/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp
index 7184613..2e282e0 100644
--- a/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp
+++ b/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp
@@ -30,6 +30,7 @@
 #include <openssl/ec_key.h>
 #include <openssl/x509.h>
 #include <remote_prov/remote_prov_utils.h>
+#include <optional>
 #include <set>
 #include <vector>
 
@@ -73,6 +74,10 @@
     return {"locked", "open"};
 }
 
+std::set<std::string> getAttestationIdEntrySet() {
+    return {"brand", "manufacturer", "product", "model", "device"};
+}
+
 bytevec string_to_bytevec(const char* s) {
     const uint8_t* p = reinterpret_cast<const uint8_t*>(s);
     return bytevec(p, p + strlen(s));
@@ -431,7 +436,7 @@
         auto [deviceInfoMap, __2, deviceInfoErrMsg] = cppbor::parse(deviceInfo.deviceInfo);
         ASSERT_TRUE(deviceInfoMap) << "Failed to parse deviceInfo: " << deviceInfoErrMsg;
         ASSERT_TRUE(deviceInfoMap->asMap());
-        checkDeviceInfo(deviceInfoMap->asMap(), deviceInfo.deviceInfo);
+        checkDeviceInfo(*deviceInfoMap->asMap(), deviceInfo.deviceInfo);
         auto& signingKey = bccContents->back().pubKey;
         deviceInfoMap->asMap()->canonicalize();
         auto macKey = verifyAndParseCoseSign1(signedMac->asArray(), signingKey,
@@ -459,72 +464,92 @@
         }
     }
 
-    void checkType(const cppbor::Map* devInfo, uint8_t majorType, std::string entryName) {
-        const auto& val = devInfo->get(entryName);
-        ASSERT_TRUE(val) << entryName << " does not exist";
-        ASSERT_EQ(val->type(), majorType) << entryName << " has the wrong type.";
+    std::optional<std::string> assertAttribute(const cppbor::Map& devInfo,
+                                               cppbor::MajorType majorType, std::string entryName) {
+        const auto& val = devInfo.get(entryName);
+        if (!val) return entryName + " is missing.\n";
+        if (val->type() != majorType) return entryName + " has the wrong type.\n";
         switch (majorType) {
             case cppbor::TSTR:
-                EXPECT_GT(val->asTstr()->value().size(), 0);
+                if (val->asTstr()->value().size() <= 0) {
+                    return entryName + " is present but the value is empty.\n";
+                }
                 break;
             case cppbor::BSTR:
-                EXPECT_GT(val->asBstr()->value().size(), 0);
+                if (val->asBstr()->value().size() <= 0) {
+                    return entryName + " is present but the value is empty.\n";
+                }
                 break;
             default:
                 break;
         }
+        return {};
     }
 
-    void checkDeviceInfo(const cppbor::Map* deviceInfo, bytevec deviceInfoBytes) {
-        EXPECT_EQ(deviceInfo->clone()->asMap()->canonicalize().encode(), deviceInfoBytes)
+    void checkType(const cppbor::Map& devInfo, cppbor::MajorType majorType, std::string entryName) {
+        if (auto error = assertAttribute(devInfo, majorType, entryName)) {
+            FAIL() << *error;
+        }
+    }
+
+    void checkDeviceInfo(const cppbor::Map& deviceInfo, bytevec deviceInfoBytes) {
+        EXPECT_EQ(deviceInfo.clone()->asMap()->canonicalize().encode(), deviceInfoBytes)
                 << "DeviceInfo ordering is non-canonical.";
-        const auto& version = deviceInfo->get("version");
+        const auto& version = deviceInfo.get("version");
         ASSERT_TRUE(version);
         ASSERT_TRUE(version->asUint());
         RpcHardwareInfo info;
         provisionable_->getHardwareInfo(&info);
         ASSERT_EQ(version->asUint()->value(), info.versionNumber);
         std::set<std::string> allowList;
+        std::string problemEntries;
         switch (version->asUint()->value()) {
             // These fields became mandated in version 2.
             case 2:
-                checkType(deviceInfo, cppbor::TSTR, "brand");
-                checkType(deviceInfo, cppbor::TSTR, "manufacturer");
-                checkType(deviceInfo, cppbor::TSTR, "product");
-                checkType(deviceInfo, cppbor::TSTR, "model");
-                checkType(deviceInfo, cppbor::TSTR, "device");
+                for (auto attId : getAttestationIdEntrySet()) {
+                    if (auto errMsg = assertAttribute(deviceInfo, cppbor::TSTR, attId)) {
+                        problemEntries += *errMsg;
+                    }
+                }
+                EXPECT_EQ("", problemEntries)
+                        << problemEntries
+                        << "Attestation IDs are missing or malprovisioned. If this test is being "
+                           "run against an early proto or EVT build, this error is probably WAI "
+                           "and indicates that Device IDs were not provisioned in the factory. If "
+                           "this error is returned on a DVT or later build revision, then "
+                           "something is likely wrong with the factory provisioning process.";
                 // TODO: Refactor the KeyMint code that validates these fields and include it here.
                 checkType(deviceInfo, cppbor::TSTR, "vb_state");
                 allowList = getAllowedVbStates();
-                EXPECT_NE(allowList.find(deviceInfo->get("vb_state")->asTstr()->value()),
+                EXPECT_NE(allowList.find(deviceInfo.get("vb_state")->asTstr()->value()),
                           allowList.end());
                 checkType(deviceInfo, cppbor::TSTR, "bootloader_state");
                 allowList = getAllowedBootloaderStates();
-                EXPECT_NE(allowList.find(deviceInfo->get("bootloader_state")->asTstr()->value()),
+                EXPECT_NE(allowList.find(deviceInfo.get("bootloader_state")->asTstr()->value()),
                           allowList.end());
                 checkType(deviceInfo, cppbor::BSTR, "vbmeta_digest");
                 checkType(deviceInfo, cppbor::UINT, "system_patch_level");
                 checkType(deviceInfo, cppbor::UINT, "boot_patch_level");
                 checkType(deviceInfo, cppbor::UINT, "vendor_patch_level");
                 checkType(deviceInfo, cppbor::UINT, "fused");
-                EXPECT_LT(deviceInfo->get("fused")->asUint()->value(), 2);  // Must be 0 or 1.
+                EXPECT_LT(deviceInfo.get("fused")->asUint()->value(), 2);  // Must be 0 or 1.
                 checkType(deviceInfo, cppbor::TSTR, "security_level");
                 allowList = getAllowedSecurityLevels();
-                EXPECT_NE(allowList.find(deviceInfo->get("security_level")->asTstr()->value()),
+                EXPECT_NE(allowList.find(deviceInfo.get("security_level")->asTstr()->value()),
                           allowList.end());
-                if (deviceInfo->get("security_level")->asTstr()->value() == "tee") {
+                if (deviceInfo.get("security_level")->asTstr()->value() == "tee") {
                     checkType(deviceInfo, cppbor::TSTR, "os_version");
                 }
                 break;
             case 1:
                 checkType(deviceInfo, cppbor::TSTR, "security_level");
                 allowList = getAllowedSecurityLevels();
-                EXPECT_NE(allowList.find(deviceInfo->get("security_level")->asTstr()->value()),
+                EXPECT_NE(allowList.find(deviceInfo.get("security_level")->asTstr()->value()),
                           allowList.end());
                 if (version->asUint()->value() == 1) {
                     checkType(deviceInfo, cppbor::TSTR, "att_id_state");
                     allowList = getAllowedAttIdStates();
-                    EXPECT_NE(allowList.find(deviceInfo->get("att_id_state")->asTstr()->value()),
+                    EXPECT_NE(allowList.find(deviceInfo.get("att_id_state")->asTstr()->value()),
                               allowList.end());
                 }
                 break;