From b1b82589b747cdd79dfa8e583ce1518aab592b9f Mon Sep 17 00:00:00 2001
From: Sangbaek Park <[email protected]>
Date: Mon, 14 Sep 2026 11:59:54 -0700
Subject: [PATCH] media: Disallow hardware-secure CDM off the record

On Windows, the browser-side MediaFoundation CDM plumbing lacked an
off-the-record check: the only Incognito enforcement was renderer-side
key-system filtering. A compromised renderer can bypass renderer-side
filtering and directly invoke media::mojom::InterfaceFactory::CreateCdm
on MediaInterfaceProxy in the browser process.

When this occurred in an off-the-record profile, the browser launched
the out-of-process MediaFoundationService utility process pointing to
the regular profile's MediaFoundationCdmStore directory. This leaked
the regular profile's distinctive identifier (origin_id) and client
token, and persisted DRM state to the regular profile directory,
violating Incognito privacy guarantees.

This CL adds an IsOffTheRecord() check in MediaInterfaceProxy::CreateCdm
for hardware-secure MediaFoundation CDMs. If the hosting BrowserContext
is off-the-record, CDM creation is rejected with kCdmNotSupported.

Bug: 559777100
Change-Id: I50d435073dfdfabbb37694aac0a3f09e593140d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8395341
Reviewed-by: Dale Curtis <[email protected]>
Commit-Queue: Sangbaek Park <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1697400}
---
diff --git a/content/browser/media/media_interface_proxy.cc b/content/browser/media/media_interface_proxy.cc
index e03f4c6..6417168 100644
--- a/content/browser/media/media_interface_proxy.cc
+++ b/content/browser/media/media_interface_proxy.cc
@@ -24,6 +24,7 @@
 #include "content/browser/media/media_web_contents_observer.h"
 #include "content/browser/renderer_host/render_frame_host_delegate.h"
 #include "content/browser/renderer_host/render_frame_host_impl.h"
+#include "content/public/browser/browser_context.h"
 #include "content/public/browser/media_service.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/security_principal.h"
@@ -43,7 +44,6 @@
 #include "mojo/public/cpp/bindings/remote_set.h"
 
 #if BUILDFLAG(ENABLE_CDM_PROVISION_FETCHER)
-#include "content/public/browser/browser_context.h"
 #include "content/public/browser/provision_fetcher_impl.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/storage_partition.h"
@@ -559,6 +559,25 @@
     auto cdm_info = CdmRegistryImpl::GetInstance()->GetCdmInfo(
         cdm_config.key_system, CdmInfo::Robustness::kHardwareSecure);
     if (cdm_info) {
+      // The MediaFoundation CDM path has no off-the-record dimension:
+      // CdmDocumentServiceImpl::GetMediaFoundationCdmData reads the REGULAR
+      // profile's persistent per-origin CDM identity (origin_id /
+      // client_token in prefs::kMediaCdmOriginData) through the incognito
+      // pref overlay, and pre-creates persistent CDM storage under the
+      // REGULAR profile's directory (OffTheRecordProfileImpl::GetPath()
+      // aliases it), which the OS CDM then uses as its durable store —
+      // surviving incognito teardown. Renderer-side incognito key-system
+      // filtering (e.g. AddPlayReady's can_persist_data check) is not a
+      // security boundary, so enforce this browser-side: refuse to launch
+      // the MediaFoundation CDM for off-the-record contexts.
+      if (render_frame_host().GetBrowserContext()->IsOffTheRecord()) {
+        DVLOG(2) << "MediaFoundation CDM not supported off the record";
+        std::move(create_cdm_cb)
+            .Run(mojo::NullRemote(), nullptr,
+                 media::CreateCdmStatus::kCdmNotSupported);
+        return;
+      }
+
       DVLOG(2) << "Get MediaFoundationService with CDM path " << cdm_info->path;
       bool is_cached_factory = mf_interface_factory_remote_.is_bound();
       auto* factory = GetMediaFoundationServiceInterfaceFactory(cdm_info->path);
diff --git a/content/browser/media/media_interface_proxy_unittest.cc b/content/browser/media/media_interface_proxy_unittest.cc
new file mode 100644
index 0000000..f70e58d
--- /dev/null
+++ b/content/browser/media/media_interface_proxy_unittest.cc
@@ -0,0 +1,140 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <memory>
+#include <optional>
+#include <string>
+
+#include "base/command_line.h"
+#include "base/files/file_path.h"
+#include "base/run_loop.h"
+#include "base/test/mock_callback.h"
+#include "build/build_config.h"
+#include "content/browser/media/cdm_registry_impl.h"
+#include "content/browser/renderer_host/render_frame_host_impl.h"
+#include "content/common/pseudonymization_salt.h"
+#include "content/public/common/cdm_info.h"
+#include "content/public/test/test_browser_context.h"
+#include "content/public/test/test_renderer_host.h"
+#include "media/base/cdm_capability.h"
+#include "media/base/cdm_config.h"
+#include "media/mojo/mojom/content_decryption_module.mojom.h"
+#include "media/mojo/mojom/interface_factory.mojom.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
+#include "mojo/public/cpp/bindings/remote.h"
+#include "sandbox/policy/switches.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace content {
+
+namespace {
+
+const char kTestKeySystem[] = "org.chromium.externalclearkey.mediafoundation";
+
+}  // namespace
+
+class MediaInterfaceProxyTest : public RenderViewHostTestHarness {
+ public:
+  void SetUp() override {
+    RenderViewHostTestHarness::SetUp();
+    SetPseudonymizationSalt(12345);
+    base::CommandLine::ForCurrentProcess()->AppendSwitch(
+        sandbox::policy::switches::kNoSandbox);
+
+    // Register a hardware-secure CDM in CdmRegistry.
+    CdmRegistryImpl::GetInstance()->RegisterCdm(CdmInfo(
+        kTestKeySystem, CdmInfo::Robustness::kHardwareSecure, std::nullopt,
+        /*supports_sub_key_systems=*/false, "Test Hardware Secure CDM",
+        media::CdmType(), base::FilePath(FILE_PATH_LITERAL("test_cdm_path"))));
+  }
+
+  void TearDown() override {
+    ResetSaltForTesting();
+    RenderViewHostTestHarness::TearDown();
+  }
+
+  mojo::Remote<media::mojom::InterfaceFactory> GetMediaInterfaceFactory() {
+    mojo::Remote<media::mojom::InterfaceFactory> factory;
+    static_cast<RenderFrameHostImpl*>(main_rfh())
+        ->BindMediaInterfaceFactoryReceiver(
+            factory.BindNewPipeAndPassReceiver());
+    return factory;
+  }
+
+  media::CdmConfig CreateHwSecureCdmConfig() {
+    media::CdmConfig config;
+    config.key_system = kTestKeySystem;
+    config.allow_distinctive_identifier = true;
+    config.allow_persistent_state = true;
+    config.use_hw_secure_codecs = true;
+    return config;
+  }
+};
+
+class MediaInterfaceProxyOffTheRecordTest : public MediaInterfaceProxyTest {
+ protected:
+  std::unique_ptr<BrowserContext> CreateBrowserContext() override {
+    auto browser_context = std::make_unique<TestBrowserContext>();
+    browser_context->set_is_off_the_record(true);
+    return browser_context;
+  }
+};
+
+#if BUILDFLAG(IS_WIN)
+TEST_F(MediaInterfaceProxyOffTheRecordTest,
+       CreateCdm_MediaFoundation_OffTheRecord) {
+  auto factory = GetMediaInterfaceFactory();
+  ASSERT_TRUE(factory.is_bound());
+
+  base::MockCallback<media::mojom::InterfaceFactory::CreateCdmCallback> mock_cb;
+  std::optional<media::CreateCdmStatus> created_status;
+  base::RunLoop run_loop;
+
+  EXPECT_CALL(mock_cb, Run(testing::_, testing::_, testing::_))
+      .WillOnce([&](mojo::PendingRemote<media::mojom::ContentDecryptionModule>,
+                    media::mojom::CdmContextPtr,
+                    media::CreateCdmStatus status) {
+        created_status = status;
+        run_loop.Quit();
+      });
+
+  factory->CreateCdm(CreateHwSecureCdmConfig(), mock_cb.Get());
+  run_loop.Run();
+
+  // In off-the-record contexts, hardware-secure MediaFoundation CDM creation
+  // must be refused to prevent leaking regular profile DRM identity and writing
+  // persistent storage to the regular profile directory.
+  ASSERT_TRUE(created_status.has_value());
+  EXPECT_EQ(created_status.value(), media::CreateCdmStatus::kCdmNotSupported);
+}
+
+TEST_F(MediaInterfaceProxyTest, CreateCdm_MediaFoundation_InvalidConfig) {
+  auto factory = GetMediaInterfaceFactory();
+  ASSERT_TRUE(factory.is_bound());
+
+  media::CdmConfig config = CreateHwSecureCdmConfig();
+  config.allow_persistent_state = false;
+
+  base::MockCallback<media::mojom::InterfaceFactory::CreateCdmCallback> mock_cb;
+  std::optional<media::CreateCdmStatus> created_status;
+  base::RunLoop run_loop;
+
+  EXPECT_CALL(mock_cb, Run(testing::_, testing::_, testing::_))
+      .WillOnce([&](mojo::PendingRemote<media::mojom::ContentDecryptionModule>,
+                    media::mojom::CdmContextPtr,
+                    media::CreateCdmStatus status) {
+        created_status = status;
+        run_loop.Quit();
+      });
+
+  factory->CreateCdm(config, mock_cb.Get());
+  run_loop.Run();
+
+  ASSERT_TRUE(created_status.has_value());
+  EXPECT_EQ(created_status.value(), media::CreateCdmStatus::kInvalidCdmConfig);
+}
+#endif  // BUILDFLAG(IS_WIN)
+
+}  // namespace content
diff --git a/content/test/BUILD.gn b/content/test/BUILD.gn
index e843d9d..830f841 100644
--- a/content/test/BUILD.gn
+++ b/content/test/BUILD.gn
@@ -3503,6 +3503,7 @@
       "../browser/child_process_sandbox_support_win_unittest.cc",
       "../browser/device_posture/device_posture_registry_watcher_win_unittest.cc",
       "../browser/haptics/haptics_manager_impl_win_unittest.cc",
+      "../browser/media/media_interface_proxy_unittest.cc",
       "../browser/renderer_host/direct_manipulation_test_helper_win.cc",
       "../browser/renderer_host/direct_manipulation_test_helper_win.h",
       "../browser/renderer_host/direct_manipulation_win_unittest.cc",

diff --git a/content/browser/media/media_interface_proxy_unittest.cc b/content/browser/media/media_interface_proxy_unittest.cc
new file mode 100644
index 0000000..f70e58d
--- /dev/null
+++ b/content/browser/media/media_interface_proxy_unittest.cc
@@ -0,0 +1,140 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <memory>
+#include <optional>
+#include <string>
+
+#include "base/command_line.h"
+#include "base/files/file_path.h"
+#include "base/run_loop.h"
+#include "base/test/mock_callback.h"
+#include "build/build_config.h"
+#include "content/browser/media/cdm_registry_impl.h"
+#include "content/browser/renderer_host/render_frame_host_impl.h"
+#include "content/common/pseudonymization_salt.h"
+#include "content/public/common/cdm_info.h"
+#include "content/public/test/test_browser_context.h"
+#include "content/public/test/test_renderer_host.h"
+#include "media/base/cdm_capability.h"
+#include "media/base/cdm_config.h"
+#include "media/mojo/mojom/content_decryption_module.mojom.h"
+#include "media/mojo/mojom/interface_factory.mojom.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
+#include "mojo/public/cpp/bindings/remote.h"
+#include "sandbox/policy/switches.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace content {
+
+namespace {
+
+const char kTestKeySystem[] = "org.chromium.externalclearkey.mediafoundation";
+
+}  // namespace
+
+class MediaInterfaceProxyTest : public RenderViewHostTestHarness {
+ public:
+  void SetUp() override {
+    RenderViewHostTestHarness::SetUp();
+    SetPseudonymizationSalt(12345);
+    base::CommandLine::ForCurrentProcess()->AppendSwitch(
+        sandbox::policy::switches::kNoSandbox);
+
+    // Register a hardware-secure CDM in CdmRegistry.
+    CdmRegistryImpl::GetInstance()->RegisterCdm(CdmInfo(
+        kTestKeySystem, CdmInfo::Robustness::kHardwareSecure, std::nullopt,
+        /*supports_sub_key_systems=*/false, "Test Hardware Secure CDM",
+        media::CdmType(), base::FilePath(FILE_PATH_LITERAL("test_cdm_path"))));
+  }
+
+  void TearDown() override {
+    ResetSaltForTesting();
+    RenderViewHostTestHarness::TearDown();
+  }
+
+  mojo::Remote<media::mojom::InterfaceFactory> GetMediaInterfaceFactory() {
+    mojo::Remote<media::mojom::InterfaceFactory> factory;
+    static_cast<RenderFrameHostImpl*>(main_rfh())
+        ->BindMediaInterfaceFactoryReceiver(
+            factory.BindNewPipeAndPassReceiver());
+    return factory;
+  }
+
+  media::CdmConfig CreateHwSecureCdmConfig() {
+    media::CdmConfig config;
+    config.key_system = kTestKeySystem;
+    config.allow_distinctive_identifier = true;
+    config.allow_persistent_state = true;
+    config.use_hw_secure_codecs = true;
+    return config;
+  }
+};
+
+class MediaInterfaceProxyOffTheRecordTest : public MediaInterfaceProxyTest {
+ protected:
+  std::unique_ptr<BrowserContext> CreateBrowserContext() override {
+    auto browser_context = std::make_unique<TestBrowserContext>();
+    browser_context->set_is_off_the_record(true);
+    return browser_context;
+  }
+};
+
+#if BUILDFLAG(IS_WIN)
+TEST_F(MediaInterfaceProxyOffTheRecordTest,
+       CreateCdm_MediaFoundation_OffTheRecord) {
+  auto factory = GetMediaInterfaceFactory();
+  ASSERT_TRUE(factory.is_bound());
+
+  base::MockCallback<media::mojom::InterfaceFactory::CreateCdmCallback> mock_cb;
+  std::optional<media::CreateCdmStatus> created_status;
+  base::RunLoop run_loop;
+
+  EXPECT_CALL(mock_cb, Run(testing::_, testing::_, testing::_))
+      .WillOnce([&](mojo::PendingRemote<media::mojom::ContentDecryptionModule>,
+                    media::mojom::CdmContextPtr,
+                    media::CreateCdmStatus status) {
+        created_status = status;
+        run_loop.Quit();
+      });
+
+  factory->CreateCdm(CreateHwSecureCdmConfig(), mock_cb.Get());
+  run_loop.Run();
+
+  // In off-the-record contexts, hardware-secure MediaFoundation CDM creation
+  // must be refused to prevent leaking regular profile DRM identity and writing
+  // persistent storage to the regular profile directory.
+  ASSERT_TRUE(created_status.has_value());
+  EXPECT_EQ(created_status.value(), media::CreateCdmStatus::kCdmNotSupported);
+}
+
+TEST_F(MediaInterfaceProxyTest, CreateCdm_MediaFoundation_InvalidConfig) {
+  auto factory = GetMediaInterfaceFactory();
+  ASSERT_TRUE(factory.is_bound());
+
+  media::CdmConfig config = CreateHwSecureCdmConfig();
+  config.allow_persistent_state = false;
+
+  base::MockCallback<media::mojom::InterfaceFactory::CreateCdmCallback> mock_cb;
+  std::optional<media::CreateCdmStatus> created_status;
+  base::RunLoop run_loop;
+
+  EXPECT_CALL(mock_cb, Run(testing::_, testing::_, testing::_))
+      .WillOnce([&](mojo::PendingRemote<media::mojom::ContentDecryptionModule>,
+                    media::mojom::CdmContextPtr,
+                    media::CreateCdmStatus status) {
+        created_status = status;
+        run_loop.Quit();
+      });
+
+  factory->CreateCdm(config, mock_cb.Get());
+  run_loop.Run();
+
+  ASSERT_TRUE(created_status.has_value());
+  EXPECT_EQ(created_status.value(), media::CreateCdmStatus::kInvalidCdmConfig);
+}
+#endif  // BUILDFLAG(IS_WIN)
+
+}  // namespace content
diff --git a/content/test/BUILD.gn b/content/test/BUILD.gn
index e843d9d..830f841 100644
--- a/content/test/BUILD.gn
+++ b/content/test/BUILD.gn
@@ -3503,6 +3503,7 @@
       "../browser/child_process_sandbox_support_win_unittest.cc",
       "../browser/device_posture/device_posture_registry_watcher_win_unittest.cc",
       "../browser/haptics/haptics_manager_impl_win_unittest.cc",
+      "../browser/media/media_interface_proxy_unittest.cc",
       "../browser/renderer_host/direct_manipulation_test_helper_win.cc",
       "../browser/renderer_host/direct_manipulation_test_helper_win.h",
       "../browser/renderer_host/direct_manipulation_win_unittest.cc",