Merge "More canonicalization checks and canonicalize before signing" am: cf02e97946 am: 8daddefc18

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

Change-Id: I077bddf029ce6a5d2a6532575f6d3e798cc831bc
This commit is contained in:
Max Bires 2022-03-21 17:04:21 +00:00 committed by Automerger Merge Worker
commit 59624afdce

View file

@ -432,10 +432,9 @@ class CertificateRequestTest : public VtsRemotelyProvisionedComponentTests {
auto [deviceInfoMap, __2, deviceInfoErrMsg] = cppbor::parse(deviceInfo.deviceInfo); auto [deviceInfoMap, __2, deviceInfoErrMsg] = cppbor::parse(deviceInfo.deviceInfo);
ASSERT_TRUE(deviceInfoMap) << "Failed to parse deviceInfo: " << deviceInfoErrMsg; ASSERT_TRUE(deviceInfoMap) << "Failed to parse deviceInfo: " << deviceInfoErrMsg;
ASSERT_TRUE(deviceInfoMap->asMap()); ASSERT_TRUE(deviceInfoMap->asMap());
checkDeviceInfo(deviceInfoMap->asMap(), deviceInfo.deviceInfo); checkDeviceInfo(deviceInfoMap->asMap(), deviceInfo.deviceInfo);
auto& signingKey = bccContents->back().pubKey; auto& signingKey = bccContents->back().pubKey;
deviceInfoMap->asMap()->canonicalize();
auto macKey = verifyAndParseCoseSign1(signedMac->asArray(), signingKey, auto macKey = verifyAndParseCoseSign1(signedMac->asArray(), signingKey,
cppbor::Array() // SignedMacAad cppbor::Array() // SignedMacAad
.add(challenge_) .add(challenge_)
@ -467,10 +466,10 @@ class CertificateRequestTest : public VtsRemotelyProvisionedComponentTests {
ASSERT_EQ(val->type(), majorType) << entryName << " has the wrong type."; ASSERT_EQ(val->type(), majorType) << entryName << " has the wrong type.";
switch (majorType) { switch (majorType) {
case cppbor::TSTR: case cppbor::TSTR:
ASSERT_GT(val->asTstr()->value().size(), 0); EXPECT_GT(val->asTstr()->value().size(), 0);
break; break;
case cppbor::BSTR: case cppbor::BSTR:
ASSERT_GT(val->asBstr()->value().size(), 0); EXPECT_GT(val->asBstr()->value().size(), 0);
break; break;
default: default:
break; break;
@ -478,6 +477,8 @@ class CertificateRequestTest : public VtsRemotelyProvisionedComponentTests {
} }
void checkDeviceInfo(const cppbor::Map* deviceInfo, bytevec deviceInfoBytes) { 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);
ASSERT_TRUE(version->asUint()); ASSERT_TRUE(version->asUint());
@ -496,21 +497,21 @@ class CertificateRequestTest : public VtsRemotelyProvisionedComponentTests {
// TODO: Refactor the KeyMint code that validates these fields and include it here. // TODO: Refactor the KeyMint code that validates these fields and include it here.
checkType(deviceInfo, cppbor::TSTR, "vb_state"); checkType(deviceInfo, cppbor::TSTR, "vb_state");
allowList = getAllowedVbStates(); allowList = getAllowedVbStates();
ASSERT_NE(allowList.find(deviceInfo->get("vb_state")->asTstr()->value()), EXPECT_NE(allowList.find(deviceInfo->get("vb_state")->asTstr()->value()),
allowList.end()); allowList.end());
checkType(deviceInfo, cppbor::TSTR, "bootloader_state"); checkType(deviceInfo, cppbor::TSTR, "bootloader_state");
allowList = getAllowedBootloaderStates(); allowList = getAllowedBootloaderStates();
ASSERT_NE(allowList.find(deviceInfo->get("bootloader_state")->asTstr()->value()), EXPECT_NE(allowList.find(deviceInfo->get("bootloader_state")->asTstr()->value()),
allowList.end()); allowList.end());
checkType(deviceInfo, cppbor::BSTR, "vbmeta_digest"); checkType(deviceInfo, cppbor::BSTR, "vbmeta_digest");
checkType(deviceInfo, cppbor::UINT, "system_patch_level"); checkType(deviceInfo, cppbor::UINT, "system_patch_level");
checkType(deviceInfo, cppbor::UINT, "boot_patch_level"); checkType(deviceInfo, cppbor::UINT, "boot_patch_level");
checkType(deviceInfo, cppbor::UINT, "vendor_patch_level"); checkType(deviceInfo, cppbor::UINT, "vendor_patch_level");
checkType(deviceInfo, cppbor::UINT, "fused"); checkType(deviceInfo, cppbor::UINT, "fused");
ASSERT_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"); checkType(deviceInfo, cppbor::TSTR, "security_level");
allowList = getAllowedSecurityLevels(); allowList = getAllowedSecurityLevels();
ASSERT_NE(allowList.find(deviceInfo->get("security_level")->asTstr()->value()), EXPECT_NE(allowList.find(deviceInfo->get("security_level")->asTstr()->value()),
allowList.end()); allowList.end());
if (deviceInfo->get("security_level")->asTstr()->value() == "tee") { if (deviceInfo->get("security_level")->asTstr()->value() == "tee") {
checkType(deviceInfo, cppbor::TSTR, "os_version"); checkType(deviceInfo, cppbor::TSTR, "os_version");
@ -519,20 +520,18 @@ class CertificateRequestTest : public VtsRemotelyProvisionedComponentTests {
case 1: case 1:
checkType(deviceInfo, cppbor::TSTR, "security_level"); checkType(deviceInfo, cppbor::TSTR, "security_level");
allowList = getAllowedSecurityLevels(); allowList = getAllowedSecurityLevels();
ASSERT_NE(allowList.find(deviceInfo->get("security_level")->asTstr()->value()), EXPECT_NE(allowList.find(deviceInfo->get("security_level")->asTstr()->value()),
allowList.end()); allowList.end());
if (version->asUint()->value() == 1) { if (version->asUint()->value() == 1) {
checkType(deviceInfo, cppbor::TSTR, "att_id_state"); checkType(deviceInfo, cppbor::TSTR, "att_id_state");
allowList = getAllowedAttIdStates(); allowList = getAllowedAttIdStates();
ASSERT_NE(allowList.find(deviceInfo->get("att_id_state")->asTstr()->value()), EXPECT_NE(allowList.find(deviceInfo->get("att_id_state")->asTstr()->value()),
allowList.end()); allowList.end());
} }
break; break;
default: default:
FAIL() << "Unrecognized version: " << version->asUint()->value(); FAIL() << "Unrecognized version: " << version->asUint()->value();
} }
ASSERT_EQ(deviceInfo->clone()->asMap()->canonicalize().encode(), deviceInfoBytes)
<< "DeviceInfo ordering is non-canonical.";
} }
bytevec eekId_; bytevec eekId_;