From 39de0cb35be1e4f935a44e2f15d0c86a75e4471b Mon Sep 17 00:00:00 2001
From: Karuna Wadhera <kwadhera@google.com>
Date: Wed, 4 Sep 2024 14:36:09 +0000
Subject: [PATCH] Optionally (dis)allow degenerate DICE chains in verifyCsr

Bug: 323246910
Test: atest libkeymint_remote_prov_support_test & manual testing of
 `rkp_factory_extraction_tool` with/without `allow_degenerate=false` on a
 device with a degenerate DICE chain
Change-Id: Ia1833c0bb6a895ae5b8aefea24850a41cf956f38
---
 .../include/remote_prov/remote_prov_utils.h   |  9 +++-
 .../keymint/support/remote_prov_utils.cpp     | 29 ++++++++----
 .../support/remote_prov_utils_test.cpp        | 44 +++++++++++++++++++
 3 files changed, 72 insertions(+), 10 deletions(-)

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 b7c32ca3bc..141f243cde 100644
--- a/security/keymint/support/include/remote_prov/remote_prov_utils.h
+++ b/security/keymint/support/include/remote_prov/remote_prov_utils.h
@@ -21,6 +21,7 @@
 #include <vector>
 #include "aidl/android/hardware/security/keymint/IRemotelyProvisionedComponent.h"
 
+#include <hwtrust/hwtrust.h>
 #include <keymaster/cppcose/cppcose.h>
 
 namespace aidl::android::hardware::security::keymint::remote_prov {
@@ -176,7 +177,8 @@ ErrMsgOr<std::vector<BccEntryData>> verifyProductionProtectedData(
  */
 ErrMsgOr<std::unique_ptr<cppbor::Array>> verifyFactoryCsr(
         const cppbor::Array& keysToSign, const std::vector<uint8_t>& csr,
-        IRemotelyProvisionedComponent* provisionable, const std::vector<uint8_t>& challenge);
+        IRemotelyProvisionedComponent* provisionable, const std::vector<uint8_t>& challenge,
+        bool allowDegenerate = true);
 /**
  * Verify the CSR as if the device is a final production sample.
  */
@@ -188,4 +190,9 @@ ErrMsgOr<std::unique_ptr<cppbor::Array>> verifyProductionCsr(
 /** Checks whether the CSR has a proper DICE chain. */
 ErrMsgOr<bool> isCsrWithProperDiceChain(const std::vector<uint8_t>& csr);
 
+/** Verify the DICE chain. */
+ErrMsgOr<std::vector<BccEntryData>> validateBcc(const cppbor::Array* bcc,
+                                                hwtrust::DiceChain::Kind kind, bool allowAnyMode,
+                                                bool allowDegenerate);
+
 }  // namespace aidl::android::hardware::security::keymint::remote_prov
diff --git a/security/keymint/support/remote_prov_utils.cpp b/security/keymint/support/remote_prov_utils.cpp
index 646037cf31..16a03dcbef 100644
--- a/security/keymint/support/remote_prov_utils.cpp
+++ b/security/keymint/support/remote_prov_utils.cpp
@@ -26,7 +26,6 @@
 #include <android-base/macros.h>
 #include <android-base/properties.h>
 #include <cppbor.h>
-#include <hwtrust/hwtrust.h>
 #include <json/json.h>
 #include <keymaster/km_openssl/ec_key.h>
 #include <keymaster/km_openssl/ecdsa_operation.h>
@@ -325,7 +324,8 @@ bytevec getProdEekChain(int32_t supportedEekCurve) {
 }
 
 ErrMsgOr<std::vector<BccEntryData>> validateBcc(const cppbor::Array* bcc,
-                                                hwtrust::DiceChain::Kind kind, bool allowAnyMode) {
+                                                hwtrust::DiceChain::Kind kind, bool allowAnyMode,
+                                                bool allowDegenerate) {
     auto encodedBcc = bcc->encode();
 
     // Use ro.build.type instead of ro.debuggable because ro.debuggable=1 for VTS testing
@@ -336,6 +336,11 @@ ErrMsgOr<std::vector<BccEntryData>> validateBcc(const cppbor::Array* bcc,
 
     auto chain = hwtrust::DiceChain::Verify(encodedBcc, kind, allowAnyMode);
     if (!chain.ok()) return chain.error().message();
+
+    if (!allowDegenerate && !chain->IsProper()) {
+        return "DICE chain is degenerate";
+    }
+
     auto keys = chain->CosePublicKeys();
     if (!keys.ok()) return keys.error().message();
     std::vector<BccEntryData> result;
@@ -701,7 +706,8 @@ ErrMsgOr<std::vector<BccEntryData>> verifyProtectedData(
     }
 
     // BCC is [ pubkey, + BccEntry]
-    auto bccContents = validateBcc(bcc->asArray(), hwtrust::DiceChain::Kind::kVsr13, allowAnyMode);
+    auto bccContents = validateBcc(bcc->asArray(), hwtrust::DiceChain::Kind::kVsr13, allowAnyMode,
+                                   /*allowDegenerate=*/true);
     if (!bccContents) {
         return bccContents.message() + "\n" + prettyPrint(bcc.get());
     }
@@ -997,7 +1003,8 @@ ErrMsgOr<hwtrust::DiceChain::Kind> getDiceChainKind() {
 
 ErrMsgOr<bytevec> parseAndValidateAuthenticatedRequest(const std::vector<uint8_t>& request,
                                                        const std::vector<uint8_t>& challenge,
-                                                       bool allowAnyMode = false) {
+                                                       bool allowAnyMode = false,
+                                                       bool allowDegenerate = true) {
     auto [parsedRequest, _, csrErrMsg] = cppbor::parse(request);
     if (!parsedRequest) {
         return csrErrMsg;
@@ -1035,7 +1042,7 @@ ErrMsgOr<bytevec> parseAndValidateAuthenticatedRequest(const std::vector<uint8_t
         return diceChainKind.message();
     }
 
-    auto diceContents = validateBcc(diceCertChain, *diceChainKind, allowAnyMode);
+    auto diceContents = validateBcc(diceCertChain, *diceChainKind, allowAnyMode, allowDegenerate);
     if (!diceContents) {
         return diceContents.message() + "\n" + prettyPrint(diceCertChain);
     }
@@ -1064,7 +1071,8 @@ ErrMsgOr<std::unique_ptr<cppbor::Array>> verifyCsr(const cppbor::Array& keysToSi
                                                    const std::vector<uint8_t>& csr,
                                                    IRemotelyProvisionedComponent* provisionable,
                                                    const std::vector<uint8_t>& challenge,
-                                                   bool isFactory, bool allowAnyMode = false) {
+                                                   bool isFactory, bool allowAnyMode = false,
+                                                   bool allowDegenerate = true) {
     RpcHardwareInfo info;
     provisionable->getHardwareInfo(&info);
     if (info.versionNumber != 3) {
@@ -1072,7 +1080,8 @@ ErrMsgOr<std::unique_ptr<cppbor::Array>> verifyCsr(const cppbor::Array& keysToSi
                ") does not match expected version (3).";
     }
 
-    auto csrPayload = parseAndValidateAuthenticatedRequest(csr, challenge, allowAnyMode);
+    auto csrPayload =
+            parseAndValidateAuthenticatedRequest(csr, challenge, allowAnyMode, allowDegenerate);
     if (!csrPayload) {
         return csrPayload.message();
     }
@@ -1082,8 +1091,10 @@ ErrMsgOr<std::unique_ptr<cppbor::Array>> verifyCsr(const cppbor::Array& keysToSi
 
 ErrMsgOr<std::unique_ptr<cppbor::Array>> verifyFactoryCsr(
         const cppbor::Array& keysToSign, const std::vector<uint8_t>& csr,
-        IRemotelyProvisionedComponent* provisionable, const std::vector<uint8_t>& challenge) {
-    return verifyCsr(keysToSign, csr, provisionable, challenge, /*isFactory=*/true);
+        IRemotelyProvisionedComponent* provisionable, const std::vector<uint8_t>& challenge,
+        bool allowDegenerate) {
+    return verifyCsr(keysToSign, csr, provisionable, challenge, /*isFactory=*/true,
+                     /*allowAnyMode=*/false, allowDegenerate);
 }
 
 ErrMsgOr<std::unique_ptr<cppbor::Array>> verifyProductionCsr(
diff --git a/security/keymint/support/remote_prov_utils_test.cpp b/security/keymint/support/remote_prov_utils_test.cpp
index 89469f11ad..82121cb6a9 100644
--- a/security/keymint/support/remote_prov_utils_test.cpp
+++ b/security/keymint/support/remote_prov_utils_test.cpp
@@ -41,6 +41,41 @@ using ::keymaster::StatusOr;
 using ::testing::ElementsAreArray;
 using byte_view = std::span<const uint8_t>;
 
+inline const std::vector<uint8_t> kDegenerateBcc{
+        0x82, 0xa5, 0x01, 0x01, 0x03, 0x27, 0x04, 0x81, 0x02, 0x20, 0x06, 0x21, 0x58, 0x20, 0xf5,
+        0x5a, 0xfb, 0x28, 0x06, 0x48, 0x68, 0xea, 0x49, 0x3e, 0x47, 0x80, 0x1d, 0xfe, 0x1f, 0xfc,
+        0xa8, 0x84, 0xb3, 0x4d, 0xdb, 0x99, 0xc7, 0xbf, 0x23, 0x53, 0xe5, 0x71, 0x92, 0x41, 0x9b,
+        0x46, 0x84, 0x43, 0xa1, 0x01, 0x27, 0xa0, 0x59, 0x01, 0x75, 0xa9, 0x01, 0x78, 0x28, 0x31,
+        0x64, 0x34, 0x35, 0x65, 0x33, 0x35, 0x63, 0x34, 0x35, 0x37, 0x33, 0x31, 0x61, 0x32, 0x62,
+        0x34, 0x35, 0x36, 0x37, 0x63, 0x33, 0x63, 0x65, 0x35, 0x31, 0x36, 0x66, 0x35, 0x63, 0x31,
+        0x66, 0x34, 0x33, 0x61, 0x62, 0x64, 0x36, 0x30, 0x62, 0x02, 0x78, 0x28, 0x31, 0x64, 0x34,
+        0x35, 0x65, 0x33, 0x35, 0x63, 0x34, 0x35, 0x37, 0x33, 0x31, 0x61, 0x32, 0x62, 0x34, 0x35,
+        0x36, 0x37, 0x63, 0x33, 0x63, 0x65, 0x35, 0x31, 0x36, 0x66, 0x35, 0x63, 0x31, 0x66, 0x34,
+        0x33, 0x61, 0x62, 0x64, 0x36, 0x30, 0x62, 0x3a, 0x00, 0x47, 0x44, 0x50, 0x58, 0x40, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x3a, 0x00, 0x47, 0x44, 0x53, 0x41, 0xa0, 0x3a, 0x00, 0x47, 0x44, 0x52,
+        0x58, 0x40, 0x71, 0xd7, 0x47, 0x9e, 0x61, 0xb5, 0x30, 0xa3, 0xda, 0xe6, 0xac, 0xb2, 0x91,
+        0xa4, 0xf9, 0xcf, 0x7f, 0xba, 0x6b, 0x5f, 0xf9, 0xa3, 0x7f, 0xba, 0xab, 0xac, 0x69, 0xdd,
+        0x0b, 0x04, 0xd6, 0x34, 0xd2, 0x3f, 0x8f, 0x84, 0x96, 0xd7, 0x58, 0x51, 0x1d, 0x68, 0x25,
+        0xea, 0xbe, 0x11, 0x11, 0x1e, 0xd8, 0xdf, 0x4b, 0x62, 0x78, 0x5c, 0xa8, 0xfa, 0xb7, 0x66,
+        0x4e, 0x8d, 0xac, 0x3b, 0x00, 0x4c, 0x3a, 0x00, 0x47, 0x44, 0x54, 0x58, 0x40, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x3a, 0x00, 0x47, 0x44, 0x56, 0x41, 0x00, 0x3a, 0x00, 0x47, 0x44, 0x57, 0x58,
+        0x2d, 0xa5, 0x01, 0x01, 0x03, 0x27, 0x04, 0x81, 0x02, 0x20, 0x06, 0x21, 0x58, 0x20, 0xf5,
+        0x5a, 0xfb, 0x28, 0x06, 0x48, 0x68, 0xea, 0x49, 0x3e, 0x47, 0x80, 0x1d, 0xfe, 0x1f, 0xfc,
+        0xa8, 0x84, 0xb3, 0x4d, 0xdb, 0x99, 0xc7, 0xbf, 0x23, 0x53, 0xe5, 0x71, 0x92, 0x41, 0x9b,
+        0x46, 0x3a, 0x00, 0x47, 0x44, 0x58, 0x41, 0x20, 0x58, 0x40, 0x31, 0x5c, 0x43, 0x87, 0xf9,
+        0xbb, 0xb9, 0x45, 0x09, 0xa8, 0xe8, 0x08, 0x70, 0xc4, 0xd9, 0xdc, 0x4e, 0x5a, 0x19, 0x10,
+        0x4f, 0xa3, 0x21, 0x20, 0x34, 0x05, 0x5b, 0x2e, 0x78, 0x91, 0xd1, 0xe2, 0x39, 0x43, 0x8b,
+        0x50, 0x12, 0x82, 0x37, 0xfe, 0xa4, 0x07, 0xc3, 0xd5, 0xc3, 0x78, 0xcc, 0xf9, 0xef, 0xe1,
+        0x95, 0x38, 0x9f, 0xb0, 0x79, 0x16, 0x4c, 0x4a, 0x23, 0xc4, 0xdc, 0x35, 0x4e, 0x0f};
+
 inline bool equal_byte_views(const byte_view& view1, const byte_view& view2) {
     return std::equal(view1.begin(), view1.end(), view2.begin(), view2.end());
 }
@@ -258,5 +293,14 @@ TEST(RemoteProvUtilsTest, GetProdEcdsaEekChain) {
     EXPECT_THAT(eekPubY, ElementsAreArray(geek->getBstrValue(CoseKey::PUBKEY_Y).value_or(empty)));
 }
 
+TEST(RemoteProvUtilsTest, validateBccDegenerate) {
+    auto [bcc, _, errMsg] = cppbor::parse(kDegenerateBcc);
+    ASSERT_TRUE(bcc) << "Error: " << errMsg;
+
+    EXPECT_TRUE(validateBcc(bcc->asArray(), hwtrust::DiceChain::Kind::kVsr16,
+                            /*allowAnyMode=*/false, /*allowDegenerate=*/true));
+    EXPECT_FALSE(validateBcc(bcc->asArray(), hwtrust::DiceChain::Kind::kVsr16,
+                             /*allowAnyMode=*/false, /*allowDegenerate=*/false));
+}
 }  // namespace
 }  // namespace aidl::android::hardware::security::keymint::remote_prov