Fix DeviceInfo encoding and checks
- Make the default implementation include the DeviceInfo as a map, not
a bstr-holding-a-map, to match the spec.
- Check the signature of the signed MAC even in test mode.
- Include the DeviceInfo in the data that the signature covers.
Test: VtsHalRemotelyProvisionedComponentTargetTest
Change-Id: I9084343c1273c16a9cbd5a1156e7057a1c54a860
diff --git a/security/keymint/aidl/default/RemotelyProvisionedComponent.cpp b/security/keymint/aidl/default/RemotelyProvisionedComponent.cpp
index ca06abc..5b02729 100644
--- a/security/keymint/aidl/default/RemotelyProvisionedComponent.cpp
+++ b/security/keymint/aidl/default/RemotelyProvisionedComponent.cpp
@@ -358,12 +358,13 @@
bcc = bcc_.clone();
}
- deviceInfo->deviceInfo = createDeviceInfo();
+ std::unique_ptr<cppbor::Map> deviceInfoMap = createDeviceInfo();
+ deviceInfo->deviceInfo = deviceInfoMap->encode();
auto signedMac = constructCoseSign1(devicePrivKey /* Signing key */, //
ephemeralMacKey /* Payload */,
cppbor::Array() /* AAD */
.add(challenge)
- .add(deviceInfo->deviceInfo)
+ .add(std::move(deviceInfoMap))
.encode());
if (!signedMac) return Status(signedMac.moveMessage());
@@ -409,8 +410,24 @@
return result;
}
-bytevec RemotelyProvisionedComponent::createDeviceInfo() const {
- return cppbor::Map().encode();
+std::unique_ptr<cppbor::Map> RemotelyProvisionedComponent::createDeviceInfo() const {
+ auto result = std::make_unique<cppbor::Map>(cppbor::Map());
+
+ // The following placeholders show how the DeviceInfo map would be populated.
+ // result->add(cppbor::Tstr("brand"), cppbor::Tstr("Google"));
+ // result->add(cppbor::Tstr("manufacturer"), cppbor::Tstr("Google"));
+ // result->add(cppbor::Tstr("product"), cppbor::Tstr("Fake"));
+ // result->add(cppbor::Tstr("model"), cppbor::Tstr("Imaginary"));
+ // result->add(cppbor::Tstr("board"), cppbor::Tstr("Chess"));
+ // result->add(cppbor::Tstr("vb_state"), cppbor::Tstr("orange"));
+ // result->add(cppbor::Tstr("bootloader_state"), cppbor::Tstr("unlocked"));
+ // result->add(cppbor::Tstr("os_version"), cppbor::Tstr("SC"));
+ // result->add(cppbor::Tstr("system_patch_level"), cppbor::Uint(20210331));
+ // result->add(cppbor::Tstr("boot_patch_level"), cppbor::Uint(20210331));
+ // result->add(cppbor::Tstr("vendor_patch_level"), cppbor::Uint(20210331));
+
+ result->canonicalize();
+ return result;
}
std::pair<bytevec /* privKey */, cppbor::Array /* BCC */>
diff --git a/security/keymint/aidl/default/RemotelyProvisionedComponent.h b/security/keymint/aidl/default/RemotelyProvisionedComponent.h
index 65b1bbc..8185e26 100644
--- a/security/keymint/aidl/default/RemotelyProvisionedComponent.h
+++ b/security/keymint/aidl/default/RemotelyProvisionedComponent.h
@@ -45,7 +45,7 @@
private:
// TODO(swillden): Move these into an appropriate Context class.
std::vector<uint8_t> deriveBytesFromHbk(const std::string& context, size_t numBytes) const;
- std::vector<uint8_t> createDeviceInfo() const;
+ std::unique_ptr<cppbor::Map> createDeviceInfo() const;
std::pair<std::vector<uint8_t>, cppbor::Array> generateBcc();
std::vector<uint8_t> macKey_ = deriveBytesFromHbk("Key to MAC public keys", 32);
diff --git a/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp b/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp
index e4c4a22..516be3b 100644
--- a/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp
+++ b/security/keymint/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp
@@ -370,7 +370,7 @@
}
}
- void checkProtectedData(bool testMode, const cppbor::Array& keysToSign,
+ void checkProtectedData(const DeviceInfo& deviceInfo, const cppbor::Array& keysToSign,
const bytevec& keysToSignMac, const ProtectedData& protectedData) {
auto [parsedProtectedData, _, protDataErrMsg] = cppbor::parse(protectedData.protectedData);
ASSERT_TRUE(parsedProtectedData) << protDataErrMsg;
@@ -404,11 +404,16 @@
ASSERT_TRUE(bccContents) << "\n" << bccContents.message() << "\n" << prettyPrint(bcc.get());
ASSERT_GT(bccContents->size(), 0U);
+ auto [deviceInfoMap, __2, deviceInfoErrMsg] = cppbor::parse(deviceInfo.deviceInfo);
+ ASSERT_TRUE(deviceInfoMap) << "Failed to parse deviceInfo: " << deviceInfoErrMsg;
+ ASSERT_TRUE(deviceInfoMap->asMap());
+
auto& signingKey = bccContents->back().pubKey;
- auto macKey = verifyAndParseCoseSign1(testMode, signedMac->asArray(), signingKey,
- cppbor::Array() // DeviceInfo
+ auto macKey = verifyAndParseCoseSign1(/* ignore_signature = */ false, signedMac->asArray(),
+ signingKey,
+ cppbor::Array() // SignedMacAad
.add(challenge_)
- .add(cppbor::Map())
+ .add(std::move(deviceInfoMap))
.encode());
ASSERT_TRUE(macKey) << macKey.message();
@@ -451,7 +456,7 @@
&protectedData, &keysToSignMac);
ASSERT_TRUE(status.isOk()) << status.getMessage();
- checkProtectedData(testMode, cppbor::Array(), keysToSignMac, protectedData);
+ checkProtectedData(deviceInfo, cppbor::Array(), keysToSignMac, protectedData);
}
}
@@ -499,7 +504,7 @@
&keysToSignMac);
ASSERT_TRUE(status.isOk()) << status.getMessage();
- checkProtectedData(testMode, cborKeysToSign_, keysToSignMac, protectedData);
+ checkProtectedData(deviceInfo, cborKeysToSign_, keysToSignMac, protectedData);
}
}