From 9a8c9456829013e9665f002917bd83a4664bdbb0 Mon Sep 17 00:00:00 2001 From: Andrew Scull Date: Thu, 22 Dec 2022 12:01:06 +0000 Subject: [PATCH] Use libcert_request_validator to check DICE chain Remove one of the DICE chain validation implementations and replace it with a call to the libcert_request_validator library which has the most complete validation and is planned to be the only implementation we support. VTS will now check both degenerate and proper DICE chain more completely and will be consistent with other tools like `bcc_validator`. P-384 will become a supported key type in the DICE chain. The whole static library is included so that clients that statically link remote_prov_utils don't need to be aware of the dependency. Bug: 254510672 Bug: 265455904 Test: atest VtsHalRemotelyProvisionedComponentTargetTest Change-Id: I067f7e8710e379a4b404ef9d2c04fe6410f73dc4 --- security/keymint/support/Android.bp | 3 + .../include/remote_prov/remote_prov_utils.h | 9 -- .../keymint/support/remote_prov_utils.cpp | 134 ++---------------- 3 files changed, 11 insertions(+), 135 deletions(-) diff --git a/security/keymint/support/Android.bp b/security/keymint/support/Android.bp index efd6fc7800..45e7c04970 100644 --- a/security/keymint/support/Android.bp +++ b/security/keymint/support/Android.bp @@ -66,6 +66,9 @@ cc_library { static_libs: [ "android.hardware.security.rkp-V3-ndk", ], + whole_static_libs: [ + "libcert_request_validator_cxx", + ], shared_libs: [ "libbase", "libbinder_ndk", diff --git a/security/keymint/support/include/remote_prov/remote_prov_utils.h b/security/keymint/support/include/remote_prov/remote_prov_utils.h index 1b94c62d66..79189a1cca 100644 --- a/security/keymint/support/include/remote_prov/remote_prov_utils.h +++ b/security/keymint/support/include/remote_prov/remote_prov_utils.h @@ -108,15 +108,6 @@ struct BccEntryData { bytevec pubKey; }; -/** - * Validates the provided CBOR-encoded BCC, returning a vector of BccEntryData - * structs containing the BCC entry contents. If an entry contains no firmware - * digest, the corresponding BccEntryData.firmwareDigest will have length zero - * (there's no way to distinguish between an empty and missing firmware digest, - * which seems fine). - */ -ErrMsgOr> validateBcc(const cppbor::Array* bcc); - struct JsonOutput { static JsonOutput Ok(std::string json) { return {std::move(json), ""}; } static JsonOutput Error(std::string error) { return {"", std::move(error)}; } diff --git a/security/keymint/support/remote_prov_utils.cpp b/security/keymint/support/remote_prov_utils.cpp index 7e164fd51a..1cb9408145 100644 --- a/security/keymint/support/remote_prov_utils.cpp +++ b/security/keymint/support/remote_prov_utils.cpp @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -289,129 +290,16 @@ bytevec getProdEekChain(int32_t supportedEekCurve) { return chain.encode(); } -ErrMsgOr validatePayloadAndFetchPubKey(const cppbor::Map* payload) { - const auto& issuer = payload->get(kBccPayloadIssuer); - if (!issuer || !issuer->asTstr()) return "Issuer is not present or not a tstr."; - const auto& subject = payload->get(kBccPayloadSubject); - if (!subject || !subject->asTstr()) return "Subject is not present or not a tstr."; - const auto& keyUsage = payload->get(kBccPayloadKeyUsage); - if (!keyUsage || !keyUsage->asBstr()) return "Key usage is not present or not a bstr."; - const auto& serializedKey = payload->get(kBccPayloadSubjPubKey); - if (!serializedKey || !serializedKey->asBstr()) return "Key is not present or not a bstr."; - return serializedKey->asBstr()->value(); -} - -ErrMsgOr verifyAndParseCoseSign1Cwt(const cppbor::Array* coseSign1, - const bytevec& signingCoseKey, const bytevec& aad) { - if (!coseSign1 || coseSign1->size() != kCoseSign1EntryCount) { - return "Invalid COSE_Sign1"; - } - - const cppbor::Bstr* protectedParams = coseSign1->get(kCoseSign1ProtectedParams)->asBstr(); - const cppbor::Map* unprotectedParams = coseSign1->get(kCoseSign1UnprotectedParams)->asMap(); - const cppbor::Bstr* payload = coseSign1->get(kCoseSign1Payload)->asBstr(); - const cppbor::Bstr* signature = coseSign1->get(kCoseSign1Signature)->asBstr(); - - if (!protectedParams || !unprotectedParams || !payload || !signature) { - return "Invalid COSE_Sign1"; - } - - auto [parsedProtParams, _, errMsg] = cppbor::parse(protectedParams); - if (!parsedProtParams) { - return errMsg + " when parsing protected params."; - } - if (!parsedProtParams->asMap()) { - return "Protected params must be a map"; - } - - auto& algorithm = parsedProtParams->asMap()->get(ALGORITHM); - if (!algorithm || !algorithm->asInt() || - (algorithm->asInt()->value() != EDDSA && algorithm->asInt()->value() != ES256)) { - return "Unsupported signature algorithm"; - } - - auto [parsedPayload, __, payloadErrMsg] = cppbor::parse(payload); - if (!parsedPayload) return payloadErrMsg + " when parsing key"; - if (!parsedPayload->asMap()) return "CWT must be a map"; - auto serializedKey = validatePayloadAndFetchPubKey(parsedPayload->asMap()); - if (!serializedKey) { - return "CWT validation failed: " + serializedKey.moveMessage(); - } - - bool selfSigned = signingCoseKey.empty(); - bytevec signatureInput = - cppbor::Array().add("Signature1").add(*protectedParams).add(aad).add(*payload).encode(); - - if (algorithm->asInt()->value() == EDDSA) { - auto key = CoseKey::parseEd25519(selfSigned ? *serializedKey : signingCoseKey); - - if (!key) return "Bad signing key: " + key.moveMessage(); - - if (!ED25519_verify(signatureInput.data(), signatureInput.size(), signature->value().data(), - key->getBstrValue(CoseKey::PUBKEY_X)->data())) { - return "Signature verification failed"; - } - } else { // P256 - auto key = CoseKey::parseP256(selfSigned ? *serializedKey : signingCoseKey); - if (!key || key->getBstrValue(CoseKey::PUBKEY_X)->empty() || - key->getBstrValue(CoseKey::PUBKEY_Y)->empty()) { - return "Bad signing key: " + key.moveMessage(); - } - auto publicKey = key->getEcPublicKey(); - if (!publicKey) return publicKey.moveMessage(); - - auto ecdsaDerSignature = ecdsaCoseSignatureToDer(signature->value()); - if (!ecdsaDerSignature) return ecdsaDerSignature.moveMessage(); - - // convert public key to uncompressed form. - publicKey->insert(publicKey->begin(), 0x04); - - if (!verifyEcdsaDigest(publicKey.moveValue(), sha256(signatureInput), *ecdsaDerSignature)) { - return "Signature verification failed"; - } - } - - return serializedKey.moveValue(); -} - ErrMsgOr> validateBcc(const cppbor::Array* bcc) { - if (!bcc || bcc->size() == 0) return "Invalid BCC"; - + auto encodedBcc = bcc->encode(); + auto chain = cert_request_validator::DiceChain::verify(encodedBcc); + if (!chain.ok()) return chain.error().message(); + auto keys = chain->cose_public_keys(); + if (!keys.ok()) return keys.error().message(); std::vector result; - - const auto& devicePubKey = bcc->get(0); - if (!devicePubKey->asMap()) return "Invalid device public key at the 1st entry in the BCC"; - - bytevec prevKey; - - for (size_t i = 1; i < bcc->size(); ++i) { - const cppbor::Array* entry = bcc->get(i)->asArray(); - if (!entry || entry->size() != kCoseSign1EntryCount) { - return "Invalid BCC entry " + std::to_string(i) + ": " + prettyPrint(entry); - } - auto payload = verifyAndParseCoseSign1Cwt(entry, std::move(prevKey), bytevec{} /* AAD */); - if (!payload) { - return "Failed to verify entry " + std::to_string(i) + ": " + payload.moveMessage(); - } - - auto& certProtParms = entry->get(kCoseSign1ProtectedParams); - if (!certProtParms || !certProtParms->asBstr()) return "Invalid prot params"; - auto [parsedProtParms, _, errMsg] = cppbor::parse(certProtParms->asBstr()->value()); - if (!parsedProtParms || !parsedProtParms->asMap()) return "Invalid prot params"; - - result.push_back(BccEntryData{*payload}); - - // This entry's public key is the signing key for the next entry. - prevKey = payload.moveValue(); - if (i == 1) { - auto [parsedRootKey, _, errMsg] = cppbor::parse(prevKey); - if (!parsedRootKey || !parsedRootKey->asMap()) return "Invalid payload entry in BCC."; - if (*parsedRootKey != *devicePubKey) { - return "Device public key doesn't match BCC root."; - } - } + for (auto& key : *keys) { + result.push_back({std::move(key)}); } - return result; } @@ -683,9 +571,6 @@ ErrMsgOr> verifyProtectedData( if (!bccContents) { return bccContents.message() + "\n" + prettyPrint(bcc.get()); } - if (bccContents->size() == 0U) { - return "The BCC is empty. It must contain at least one entry."; - } auto deviceInfoResult = parseAndValidateDeviceInfo(deviceInfo.deviceInfo, provisionable, isFactory); @@ -977,9 +862,6 @@ ErrMsgOr parseAndValidateAuthenticatedRequest(const std::vectorsize() == 0U) { - return "The DICE chain is empty. It must contain at least one entry."; - } auto& udsPub = diceContents->back().pubKey;