Enable self-testing by default in rkp factory tool
This way, we run the self test when extracting a CSR on the factory
line by default. This will ensure that devices producing bad payloads
will be more likely to be caught earlier in the manufacturing flow.
Test: ran tool devices with V2 and V3 HALs
Bug: 284098419
Change-Id: I79b50da7f86da50ebcfe18caf06046f1a39c6e81
diff --git a/provisioner/rkp_factory_extraction_lib.cpp b/provisioner/rkp_factory_extraction_lib.cpp
index 8db62e6..ab7d17c 100644
--- a/provisioner/rkp_factory_extraction_lib.cpp
+++ b/provisioner/rkp_factory_extraction_lib.cpp
@@ -195,7 +195,11 @@
protectedData, *eekChain, eekId,
hwInfo.supportedEekCurve, irpc, challenge);
- std::cout << "Self test successful." << std::endl;
+ if (!result) {
+ std::cerr << "Self test failed for IRemotelyProvisionedComponent '" << componentName
+ << "'. Error message: '" << result.message() << "'." << std::endl;
+ exit(-1);
+ }
}
CborResult<Array> composeCertificateRequestV3(const std::vector<uint8_t>& csr) {
@@ -220,7 +224,7 @@
}
CborResult<cppbor::Array> getCsrV3(std::string_view componentName,
- IRemotelyProvisionedComponent* irpc) {
+ IRemotelyProvisionedComponent* irpc, bool selfTest) {
std::vector<uint8_t> csr;
std::vector<MacedPublicKey> emptyKeys;
const std::vector<uint8_t> challenge = generateChallenge();
@@ -232,32 +236,20 @@
exit(-1);
}
+ if (selfTest) {
+ auto result = verifyFactoryCsr(/*keysToSign=*/cppbor::Array(), csr, irpc, challenge);
+ if (!result) {
+ std::cerr << "Self test failed for IRemotelyProvisionedComponent '" << componentName
+ << "'. Error message: '" << result.message() << "'." << std::endl;
+ exit(-1);
+ }
+ }
+
return composeCertificateRequestV3(csr);
}
-void selfTestGetCsrV3(std::string_view componentName, IRemotelyProvisionedComponent* irpc) {
- std::vector<uint8_t> csr;
- std::vector<MacedPublicKey> emptyKeys;
- const std::vector<uint8_t> challenge = generateChallenge();
-
- auto status = irpc->generateCertificateRequestV2(emptyKeys, challenge, &csr);
- if (!status.isOk()) {
- std::cerr << "Bundle extraction failed for '" << componentName
- << "'. Error code: " << status.getServiceSpecificError() << "." << std::endl;
- exit(-1);
- }
-
- auto result = verifyFactoryCsr(/*keysToSign=*/cppbor::Array(), csr, irpc, challenge);
- if (!result) {
- std::cerr << "Self test failed for '" << componentName
- << "'. Error message: " << result.message() << "." << std::endl;
- exit(-1);
- }
-
- std::cout << "Self test successful." << std::endl;
-}
-
-CborResult<Array> getCsr(std::string_view componentName, IRemotelyProvisionedComponent* irpc) {
+CborResult<Array> getCsr(std::string_view componentName, IRemotelyProvisionedComponent* irpc,
+ bool selfTest) {
RpcHardwareInfo hwInfo;
auto status = irpc->getHardwareInfo(&hwInfo);
if (!status.isOk()) {
@@ -267,24 +259,11 @@
}
if (hwInfo.versionNumber < kVersionWithoutSuperencryption) {
+ if (selfTest) {
+ selfTestGetCsrV1(componentName, irpc);
+ }
return getCsrV1(componentName, irpc);
} else {
- return getCsrV3(componentName, irpc);
- }
-}
-
-void selfTestGetCsr(std::string_view componentName, IRemotelyProvisionedComponent* irpc) {
- RpcHardwareInfo hwInfo;
- auto status = irpc->getHardwareInfo(&hwInfo);
- if (!status.isOk()) {
- std::cerr << "Failed to get hardware info for '" << componentName
- << "'. Error code: " << status.getServiceSpecificError() << "." << std::endl;
- exit(-1);
- }
-
- if (hwInfo.versionNumber < kVersionWithoutSuperencryption) {
- selfTestGetCsrV1(componentName, irpc);
- } else {
- selfTestGetCsrV3(componentName, irpc);
+ return getCsrV3(componentName, irpc, selfTest);
}
}
diff --git a/provisioner/rkp_factory_extraction_lib.h b/provisioner/rkp_factory_extraction_lib.h
index a218338..ae8ea6b 100644
--- a/provisioner/rkp_factory_extraction_lib.h
+++ b/provisioner/rkp_factory_extraction_lib.h
@@ -46,7 +46,8 @@
// what went wrong.
CborResult<cppbor::Array>
getCsr(std::string_view componentName,
- aidl::android::hardware::security::keymint::IRemotelyProvisionedComponent* irpc);
+ aidl::android::hardware::security::keymint::IRemotelyProvisionedComponent* irpc,
+ bool selfTest);
// Generates a test certificate chain and validates it, exiting the process on error.
void selfTestGetCsr(
diff --git a/provisioner/rkp_factory_extraction_lib_test.cpp b/provisioner/rkp_factory_extraction_lib_test.cpp
index 72d7b71..3fe88da 100644
--- a/provisioner/rkp_factory_extraction_lib_test.cpp
+++ b/provisioner/rkp_factory_extraction_lib_test.cpp
@@ -180,7 +180,8 @@
SetArgPointee<6>(kFakeMac), //
Return(ByMove(ScopedAStatus::ok())))); //
- auto [csr, csrErrMsg] = getCsr("mock component name", mockRpc.get());
+ auto [csr, csrErrMsg] = getCsr("mock component name", mockRpc.get(),
+ /*selfTest=*/false);
ASSERT_THAT(csr, NotNull()) << csrErrMsg;
ASSERT_THAT(csr->asArray(), Pointee(Property(&Array::size, Eq(4))));
@@ -249,7 +250,8 @@
.WillOnce(DoAll(SaveArg<1>(&challenge), SetArgPointee<2>(kCsr),
Return(ByMove(ScopedAStatus::ok()))));
- auto [csr, csrErrMsg] = getCsr("mock component name", mockRpc.get());
+ auto [csr, csrErrMsg] = getCsr("mock component name", mockRpc.get(),
+ /*selfTest=*/false);
ASSERT_THAT(csr, NotNull()) << csrErrMsg;
ASSERT_THAT(csr, Pointee(Property(&Array::size, Eq(5))));
diff --git a/provisioner/rkp_factory_extraction_tool.cpp b/provisioner/rkp_factory_extraction_tool.cpp
index 2aeabe0..5ba777e 100644
--- a/provisioner/rkp_factory_extraction_tool.cpp
+++ b/provisioner/rkp_factory_extraction_tool.cpp
@@ -35,10 +35,10 @@
using namespace cppcose;
DEFINE_string(output_format, "build+csr", "How to format the output. Defaults to 'build+csr'.");
-DEFINE_bool(self_test, false,
- "If true, the tool does not output CSR data, but instead performs a self-test, "
- "validating a test payload for correctness. This may be used to verify a device on the "
- "factory line before attempting to upload the output to the device info service.");
+DEFINE_bool(self_test, true,
+ "If true, this tool performs a self-test, validating the payload for correctness. "
+ "This checks that the device on the factory line is producing valid output "
+ "before attempting to upload the output to the device info service.");
namespace {
@@ -81,17 +81,13 @@
exit(-1);
}
- if (FLAGS_self_test) {
- selfTestGetCsr(name, rkp_service.get());
- } else {
- auto [request, errMsg] = getCsr(name, rkp_service.get());
- if (!request) {
- std::cerr << "Unable to build CSR for '" << fullName << ": " << errMsg << std::endl;
- exit(-1);
- }
-
- writeOutput(std::string(name), *request);
+ auto [request, errMsg] = getCsr(name, rkp_service.get(), FLAGS_self_test);
+ if (!request) {
+ std::cerr << "Unable to build CSR for '" << fullName << ": " << errMsg << std::endl;
+ exit(-1);
}
+
+ writeOutput(std::string(name), *request);
}
} // namespace