From 3e819c06ee433485f9f7997f714d3717d2a4f8bc Mon Sep 17 00:00:00 2001
From: Nina Satragno <nsatragno@chromium.org>
Date: Thu, 31 Jul 2025 11:14:02 -0700
Subject: [PATCH] [webauthn] Redact sensitive GPM secrets from log

Redact secrets related to end-to-end encryption from device log.

Fixed: 434977743
Change-Id: Ib727476208e8091a23f0450053188e430538625a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6803265
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Commit-Queue: Nina Satragno <nsatragno@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1494968}
---

diff --git a/chrome/browser/webauthn/enclave_authenticator_browsertest.cc b/chrome/browser/webauthn/enclave_authenticator_browsertest.cc
index bf2e825..b6cb868 100644
--- a/chrome/browser/webauthn/enclave_authenticator_browsertest.cc
+++ b/chrome/browser/webauthn/enclave_authenticator_browsertest.cc
@@ -60,6 +60,7 @@
 #include "chrome/grit/generated_resources.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "components/device_event_log/device_event_log.h"
 #include "components/password_manager/core/browser/password_form.h"
 #include "components/password_manager/core/browser/password_store/password_store_interface.h"
 #include "components/password_manager/core/common/password_manager_pref_names.h"
@@ -604,6 +605,13 @@
   }
 }
 
+std::string GetDeviceLog() {
+  return device_event_log::GetAsString(
+      device_event_log::NEWEST_FIRST, /*format=*/"level",
+      /*types=*/"fido",
+      /*max_level=*/device_event_log::LOG_LEVEL_EVENT, /*max_events=*/0);
+}
+
 bool IsMechanismEnclaveCredential(
     const AuthenticatorRequestDialogModel::Mechanism& mechanism) {
   if (std::holds_alternative<
@@ -1013,6 +1021,9 @@
   std::string script_result;
   ASSERT_TRUE(message_queue.WaitForMessage(&script_result));
   EXPECT_EQ(script_result, "\"webauthn: OK\"");
+
+  // Ensure the security domain secret is redacted from logs.
+  EXPECT_THAT(GetDeviceLog(), testing::HasSubstr("\"secret\": \"[redacted]\""));
 }
 
 IN_PROC_BROWSER_TEST_F(EnclaveAuthenticatorBrowserTest, NonWebauthnRequest) {
@@ -1115,6 +1126,9 @@
   EXPECT_TRUE(enabled);
   EXPECT_EQ(first, "none");
   EXPECT_EQ(second, "none");
+
+  // Ensure the PRF is redacted from logs.
+  EXPECT_THAT(GetDeviceLog(), testing::HasSubstr("\"prf\": \"[redacted]\""));
 }
 
 IN_PROC_BROWSER_TEST_F(EnclaveAuthenticatorBrowserTest, GetAssertionWithPrf) {
@@ -4316,6 +4330,10 @@
   histogram_tester.ExpectBucketCount(
       "WebAuthentication.GPM.GetAssertion.LargeBlobSucceeded.Read",
       /*sample=*/true, /*expected_count=*/1);
+
+  // Ensure the large blob is redacted from logs.
+  EXPECT_THAT(GetDeviceLog(),
+              testing::HasSubstr("\"largeBlob\": \"[redacted]\""));
 }
 
 // Disable large blob for GPM feature flag.
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index aba6cb4d..ded37f7 100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -2570,6 +2570,7 @@
       "//components/cookie_config",
       "//components/country_codes",
       "//components/data_sharing/public",
+      "//components/device_event_log",
       "//components/dom_distiller/content/browser",
       "//components/dom_distiller/content/browser:test_support",
       "//components/dom_distiller/content/renderer",
diff --git a/device/fido/enclave/enclave_protocol_utils.cc b/device/fido/enclave/enclave_protocol_utils.cc
index 51883e1..f607cf3 100644
--- a/device/fido/enclave/enclave_protocol_utils.cc
+++ b/device/fido/enclave/enclave_protocol_utils.cc
@@ -232,6 +232,91 @@
   return ret;
 }
 
+// Redacts `path` from `cbor` using the semantics described below for
+// `RedactCbor`. Mutates `cbor` in place.
+void RedactPath(cbor::Value* cbor, base::span<const char*> path) {
+  if (cbor->is_array()) {
+    // Mutate all the elements in the array.
+    cbor::Value::ArrayValue& array =
+        const_cast<cbor::Value::ArrayValue&>(cbor->GetArray());
+    for (cbor::Value& value : array) {
+      RedactPath(&value, path);
+    }
+    return;
+  }
+  if (!cbor->is_map()) {
+    // Only maps and arrays are supported.
+    return;
+  }
+  cbor::Value::MapValue& map =
+      const_cast<cbor::Value::MapValue&>(cbor->GetMap());
+  const char* field = path.take_first_elem();
+  const auto it = map.find(cbor::Value(field));
+  if (it == map.end()) {
+    // Could not find some part of the path, bail out.
+    return;
+  }
+  if (path.empty()) {
+    // Found the leaf, replace the map value regardless of its type.
+    it->second = cbor::Value("[redacted]");
+    return;
+  }
+  RedactPath(&it->second, path);
+}
+
+// Redacts `paths_to_redact` from `cbor` by finding the corresponding keys and
+// replacing them by the cbor string "redacted". Nested paths should correspond
+// to nested maps under the same key name. The redaction is applied to all array
+// elements for a matching key.
+// If a path is not found, a clone of `cbor` is returned.
+//
+// Example:
+//
+// Given a `cbor` value...
+// {
+//   characters: [
+//     {
+//       name: "Reimu",
+//       occupation: ["Shrine maiden"]
+//     },
+//     {
+//       name: "Marisa",
+//       occupation: ["Witch", "Troublemaker"]
+//     }
+//   ]
+// }
+//
+// ...and a `paths_to_redact` value...
+//
+// [
+//   ["characters", "occupation"],
+//   ["characters", "date-of-birth"],
+// ]
+//
+// ...the returned cbor will be:
+//
+// {
+//   characters: [
+//     {
+//       name: "Reimu",
+//       occupation: "redacted"
+//     },
+//     {
+//       name: "Marisa",
+//       occupation: "redacted"
+//     }
+//   ]
+// }
+cbor::Value RedactCbor(
+    const cbor::Value& cbor,
+    base::span<const std::vector<const char*>> paths_to_redact) {
+  cbor::Value response = cbor.Clone();
+  for (std::vector<const char*> field_to_redact : paths_to_redact) {
+    RedactPath(&response, field_to_redact);
+  }
+  return response;
+}
+
 }  // namespace
 
 ErrorResponse::ErrorResponse(std::string error)
@@ -667,4 +752,17 @@
                           std::move(complete_callback)));
 }
 
+cbor::Value RedactEnclaveRequest(const cbor::Value& cbor) {
+  const std::array redacted_fields = {std::vector{"secret"}};
+  return RedactCbor(cbor, redacted_fields);
+}
+
+cbor::Value RedactEnclaveResponse(const cbor::Value& cbor) {
+  const std::array redacted_fields = {
+      std::vector{"ok", "ok", "largeBlob"},
+      std::vector{"ok", "ok", "prf"},
+  };
+  return RedactCbor(cbor, redacted_fields);
+}
+
 }  // namespace device::enclave
diff --git a/device/fido/enclave/enclave_protocol_utils.h b/device/fido/enclave/enclave_protocol_utils.h
index 41772175..e435f04 100644

diff --git a/chrome/browser/webauthn/enclave_authenticator_browsertest.cc b/chrome/browser/webauthn/enclave_authenticator_browsertest.cc
index bf2e825..b6cb868 100644
--- a/chrome/browser/webauthn/enclave_authenticator_browsertest.cc
+++ b/chrome/browser/webauthn/enclave_authenticator_browsertest.cc
@@ -60,6 +60,7 @@
 #include "chrome/grit/generated_resources.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "components/device_event_log/device_event_log.h"
 #include "components/password_manager/core/browser/password_form.h"
 #include "components/password_manager/core/browser/password_store/password_store_interface.h"
 #include "components/password_manager/core/common/password_manager_pref_names.h"
@@ -604,6 +605,13 @@
   }
 }
 
+std::string GetDeviceLog() {
+  return device_event_log::GetAsString(
+      device_event_log::NEWEST_FIRST, /*format=*/"level",
+      /*types=*/"fido",
+      /*max_level=*/device_event_log::LOG_LEVEL_EVENT, /*max_events=*/0);
+}
+
 bool IsMechanismEnclaveCredential(
     const AuthenticatorRequestDialogModel::Mechanism& mechanism) {
   if (std::holds_alternative<
@@ -1013,6 +1021,9 @@
   std::string script_result;
   ASSERT_TRUE(message_queue.WaitForMessage(&script_result));
   EXPECT_EQ(script_result, "\"webauthn: OK\"");
+
+  // Ensure the security domain secret is redacted from logs.
+  EXPECT_THAT(GetDeviceLog(), testing::HasSubstr("\"secret\": \"[redacted]\""));
 }
 
 IN_PROC_BROWSER_TEST_F(EnclaveAuthenticatorBrowserTest, NonWebauthnRequest) {
@@ -1115,6 +1126,9 @@
   EXPECT_TRUE(enabled);
   EXPECT_EQ(first, "none");
   EXPECT_EQ(second, "none");
+
+  // Ensure the PRF is redacted from logs.
+  EXPECT_THAT(GetDeviceLog(), testing::HasSubstr("\"prf\": \"[redacted]\""));
 }
 
 IN_PROC_BROWSER_TEST_F(EnclaveAuthenticatorBrowserTest, GetAssertionWithPrf) {
@@ -4316,6 +4330,10 @@
   histogram_tester.ExpectBucketCount(
       "WebAuthentication.GPM.GetAssertion.LargeBlobSucceeded.Read",
       /*sample=*/true, /*expected_count=*/1);
+
+  // Ensure the large blob is redacted from logs.
+  EXPECT_THAT(GetDeviceLog(),
+              testing::HasSubstr("\"largeBlob\": \"[redacted]\""));
 }
 
 // Disable large blob for GPM feature flag.
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index aba6cb4d..ded37f7 100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -2570,6 +2570,7 @@
       "//components/cookie_config",
       "//components/country_codes",
       "//components/data_sharing/public",
+      "//components/device_event_log",
       "//components/dom_distiller/content/browser",
       "//components/dom_distiller/content/browser:test_support",
       "//components/dom_distiller/content/renderer",
diff --git a/device/fido/enclave/enclave_protocol_utils_unittest.cc b/device/fido/enclave/enclave_protocol_utils_unittest.cc
index cb7e04c..bf22f49 100644
--- a/device/fido/enclave/enclave_protocol_utils_unittest.cc
+++ b/device/fido/enclave/enclave_protocol_utils_unittest.cc
@@ -19,9 +19,11 @@
 #include "base/values.h"
 #include "components/cbor/reader.h"
 #include "components/cbor/values.h"
+#include "components/cbor/writer.h"
 #include "components/device_event_log/device_event_log.h"
 #include "components/sync/protocol/webauthn_credential_specifics.pb.h"
 #include "device/fido/ctap_make_credential_request.h"
+#include "device/fido/enclave/constants.h"
 #include "device/fido/fido_parsing_utils.h"
 #include "device/fido/fido_transport_protocol.h"
 #include "device/fido/json_request.h"
@@ -94,6 +96,18 @@
 constexpr char kMakeCredentialHexResponse[] =
     "81A1626F6BA3677075625F6B657944050607086776657273696F6E0169656E637279707465"
     "644401020304";
+
+// An example response with the top-level "ok" key, dummy large blob and PRF
+// values.
+constexpr char kCompleteGetAssertionHexResponse[] =
+    "A1626F6B81A1626F6BA3637072661904D268726573706F6E7365A3697369676E6174757265"
+    "A2646461746184185318691867186E6474797065664275666665726A7573657248616E646C"
+    "65A2646461746182186118626474797065664275666665727161757468656E74696361746F"
+    "7244617461A2646461746198251118941822188D18A818FD18BD18EE18FD1826181B18D718"
+    "B61859185C18FD187018A50D187018C61840187B18CF01183D18E9186D184E18FB1718DE01"
+    "000000183B647479706566427566666572696C61726765426C6F62A264726561641904D264"
+    "73697A6501";
+
 constexpr int32_t kWrappedSecretVersion = 952;
 
 struct BadResponseTestCase {
@@ -247,8 +261,11 @@
 
   std::vector<uint8_t> wrapped_secret() { return wrapped_secret_; }
 
+  std::vector<uint8_t> secret() { return secret_; }
+
  private:
   const std::vector<uint8_t> wrapped_secret_ = {1, 2, 3, 4, 5};
+  const std::vector<uint8_t> secret_ = {6, 7, 8, 9, 0};
   std::vector<uint8_t> device_id_;
   std::vector<uint8_t> user_id_;
   std::vector<uint8_t> encrypted_passkey_;
@@ -647,5 +664,54 @@
               testing::ElementsAre(1, 2, 3));
 }
 
+TEST_F(EnclaveProtocolUtilsTest, RedactEnclaveRequest) {
+  auto entity = PasskeyEntity();
+  entity.set_rp_id(kRpId);
+  std::optional<base::Value> parsed_json =
+      base::JSONReader::Read(kGetAssertionRequestJson);
+  EXPECT_TRUE(parsed_json);
+  auto json_request =
+      base::MakeRefCounted<JSONRequest>(std::move(*parsed_json));
+  cbor::Value request_cbor = BuildGetAssertionCommand(
+      std::move(entity), json_request, kClientDataJson,
+      /*claimed_pin=*/nullptr, /*wrapped_secret=*/std::nullopt, secret());
+  cbor::Value redacted = RedactEnclaveRequest(request_cbor);
+  ASSERT_TRUE(redacted.is_map());
+  const auto& redacted_map = redacted.GetMap();
+  const auto& redacted_value =
+      redacted_map.find(cbor::Value(kRequestSecretKey))->second;
+  EXPECT_EQ(redacted_value.GetString(), "[redacted]");
+}
+
+TEST_F(EnclaveProtocolUtilsTest, RedactErroneousEnclaveRequest) {
+  cbor::Value request = cbor::Value("not a valid request");
+  EXPECT_EQ(cbor::Writer::Write(RedactEnclaveRequest(request)),
+            cbor::Writer::Write(request));
+}
+
+TEST_F(EnclaveProtocolUtilsTest, RedactEnclaveResponse) {
+  std::vector<uint8_t> response_serialized;
+  CHECK(base::HexStringToBytes(kCompleteGetAssertionHexResponse,
+                               &response_serialized));
+  cbor::Value response_cbor = cbor::Reader::Read(response_serialized).value();
+
+  const cbor::Value redacted = RedactEnclaveResponse(response_cbor);
+  const cbor::Value::MapValue& redacted_outer_map =
+      redacted.GetMap().find(cbor::Value("ok"))->second.GetArray()[0].GetMap();
+  const cbor::Value::MapValue& redacted_map =
+      redacted_outer_map.find(cbor::Value("ok"))->second.GetMap();
+  const auto& large_blob_value =
+      redacted_map.find(cbor::Value("largeBlob"))->second;
+  EXPECT_EQ(large_blob_value.GetString(), "[redacted]");
+  const auto& prf_value = redacted_map.find(cbor::Value("prf"))->second;
+  EXPECT_EQ(prf_value.GetString(), "[redacted]");
+}
+
+TEST_F(EnclaveProtocolUtilsTest, RedactErroneousEnclaveResponse) {
+  cbor::Value response = cbor::Value("not a valid response");
+  EXPECT_EQ(cbor::Writer::Write(RedactEnclaveResponse(response)),
+            cbor::Writer::Write(response));
+}
+
 }  // namespace enclave
 }  // namespace device