Skip to content

Commit

Permalink
Introduce a safe-mode for the VideoCapture Service
Browse files Browse the repository at this point in the history 
On macOS, this disables loading 3rd-party DAL plugins, which may
crash upon use.
This relies on the hardened runtime from signed binaries.
When the CHILD_PLUGIN option is passed, the runtime is disabled and
3rd-party DAL plugins are allowed.
For developers convenience who cannot sign their binaries,
an environment variable was identified within the the macOS libraries
to achieve a similar effect and skip loading all DAL plugins.

This is enabled by the RetryGetVideoCaptureDeviceInfos feature which
is also updated accordingly.

The feature can now handle a service crash properly and is compiled
on all platforms, though only macOS will have at most one retry.
On the second attempt, the VideoCaptureService safe-mode will be
enabled.

Traces of the old RetryCount are removed since the value is unused.

Bug: 990381
Change-Id: If042b19006ef4166260941af5485897f65e5e0de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4209899
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Commit-Queue: Florent Castelli <orphis@chromium.org>
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Reviewed-by: Tony Herre <toprice@chromium.org>
Reviewed-by: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1103243}
  • Loading branch information
@Orphis
Orphis authored and Chromium LUCI CQ committed Feb 9, 2023
1 parent 6ef2e9a commit 8764dd4
Show file tree
Hide file tree
Showing 16 changed files with 120 additions and 128 deletions.
2 changes: 0 additions & 2 deletions chrome/browser/ash/camera_presence_notifier_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ class FakeVideoCaptureService
fake_provider_.Bind(std::move(receiver));
}

void SetRetryCount(int32_t count) override {}

void BindControlsForTesting(
mojo::PendingReceiver<video_capture::mojom::TestingControls> receiver)
override {}
Expand Down
3 changes: 2 additions & 1 deletion content/browser/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2124,7 +2124,8 @@ source_set("browser") {
"utility_process_host_receiver_bindings.cc",
"utility_sandbox_delegate.cc",
"utility_sandbox_delegate.h",
"video_capture_service.cc",
"video_capture_service_impl.cc",
"video_capture_service_impl.h",
"wake_lock/wake_lock_context_host.cc",
"wake_lock/wake_lock_context_host.h",
"wake_lock/wake_lock_service_impl.cc",
Expand Down
4 changes: 0 additions & 4 deletions content/browser/renderer_host/media/ref_counted_video_source_provider.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ RefCountedVideoSourceProvider::GetWeakPtr() {
return weak_ptr_factory_.GetWeakPtr();
}

void RefCountedVideoSourceProvider::SetRetryCount(int32_t count) {
GetVideoCaptureService().SetRetryCount(count);
}

void RefCountedVideoSourceProvider::ReleaseProviderForTesting() {
source_provider_.reset();
}
Expand Down
1 change: 0 additions & 1 deletion content/browser/renderer_host/media/ref_counted_video_source_provider.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ class CONTENT_EXPORT RefCountedVideoSourceProvider
return source_provider_;
}

void SetRetryCount(int32_t count);
void ReleaseProviderForTesting();

private:
Expand Down
106 changes: 49 additions & 57 deletions content/browser/renderer_host/media/service_video_capture_provider.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,14 @@

#include "base/functional/bind.h"
#include "base/functional/callback_helpers.h"
#include "base/metrics/histogram_macros.h"
#include "base/trace_event/common/trace_event_common.h"
#include "build/build_config.h"
#include "build/chromeos_buildflags.h"
#include "content/browser/child_process_host_impl.h"
#include "content/browser/renderer_host/media/service_video_capture_device_launcher.h"
#include "content/browser/renderer_host/media/virtual_video_capture_devices_changed_observer.h"
#include "content/browser/video_capture_service_impl.h"
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/gpu_data_manager.h"
Expand All @@ -29,13 +32,6 @@
#include "content/public/browser/chromeos/delegate_to_browser_gpu_service_accelerator_factory.h"
#endif // BUILDFLAG(IS_CHROMEOS_ASH)

#if BUILDFLAG(IS_MAC)
#include "base/mac/mac_util.h"
#include "base/metrics/histogram_macros.h"
#include "base/strings/string_util.h"
#include "base/trace_event/common/trace_event_common.h"
#endif

namespace {

#if BUILDFLAG(IS_CHROMEOS_ASH)
Expand All @@ -46,10 +42,6 @@ CreateAcceleratorFactory() {
}
#endif // BUILDFLAG(IS_CHROMEOS_ASH)

#if BUILDFLAG(IS_MAC)
static const int kMaxRetriesForGetDeviceInfos = 1;
#endif

} // anonymous namespace

namespace content {
Expand Down Expand Up @@ -88,8 +80,10 @@ class ServiceVideoCaptureProvider::ServiceProcessObserver
}

void OnServiceProcessCrashed(const ServiceProcessInfo& info) override {
if (info.IsService<video_capture::mojom::VideoCaptureService>())
if (info.IsService<video_capture::mojom::VideoCaptureService>()) {
LOG(WARNING) << "Detected crash of video capture service";
io_task_runner_->PostTask(FROM_HERE, base::BindOnce(stop_callback_));
}
}

const scoped_refptr<base::TaskRunner> io_task_runner_;
Expand Down Expand Up @@ -148,7 +142,9 @@ void ServiceVideoCaptureProvider::GetDeviceInfosAsync(
GetDeviceInfosCallback result_callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
emit_log_message_cb_.Run("ServiceVideoCaptureProvider::GetDeviceInfosAsync");
GetDeviceInfosAsyncForRetry(std::move(result_callback), 0);
get_device_infos_retried_ = false;
get_device_infos_pending_callbacks_.push_back(std::move(result_callback));
GetDeviceInfosAsyncForRetry();
}

std::unique_ptr<VideoCaptureDeviceLauncher>
Expand Down Expand Up @@ -177,17 +173,15 @@ void ServiceVideoCaptureProvider::OnServiceStarted() {
}

void ServiceVideoCaptureProvider::OnServiceStopped() {
#if BUILDFLAG(IS_MAC)
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
if (stashed_result_callback_for_retry_) {
if (!get_device_infos_pending_callbacks_.empty()) {
// The service stopped during a device info query.
TRACE_EVENT_INSTANT0(
TRACE_DISABLED_BY_DEFAULT("video_and_image_capture"),
"Video capture service has shut down. Retrying GetDeviceInfos.",
TRACE_EVENT_SCOPE_PROCESS);
GetDeviceInfosAsyncForRetry(std::move(stashed_result_callback_for_retry_),
stashed_retry_count_ + 1);
GetDeviceInfosAsyncForRetry();
}
#endif
}

void ServiceVideoCaptureProvider::OnLauncherConnectingToSourceProvider(
Expand Down Expand Up @@ -235,6 +229,13 @@ ServiceVideoCaptureProvider::LazyConnectToService() {
#endif

mojo::Remote<video_capture::mojom::VideoSourceProvider> source_provider;
#if BUILDFLAG(IS_MAC)
if (get_device_infos_retried_) {
// If the service crashed once during a device info query, enable the
// safe-mode VideoCaptureService.
EnableVideoCaptureServiceSafeMode();
}
#endif
GetVideoCaptureService().ConnectToVideoSourceProvider(
source_provider.BindNewPipeAndPassReceiver());
source_provider.set_disconnect_handler(base::BindOnce(
Expand All @@ -249,64 +250,55 @@ ServiceVideoCaptureProvider::LazyConnectToService() {
return result;
}

void ServiceVideoCaptureProvider::GetDeviceInfosAsyncForRetry(
GetDeviceInfosCallback result_callback,
int retry_count) {
void ServiceVideoCaptureProvider::GetDeviceInfosAsyncForRetry() {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
auto service_connection = LazyConnectToService();
service_connection->SetRetryCount(retry_count);
// Make sure that |result_callback| gets invoked with an empty result in case
// Make sure that the callback gets invoked with an empty result in case
// that the service drops the request.
auto split_callback = base::SplitOnceCallback(std::move(result_callback));
service_connection->source_provider()->GetSourceInfos(
mojo::WrapCallbackWithDropHandler(
base::BindOnce(&ServiceVideoCaptureProvider::OnDeviceInfosReceived,
weak_ptr_factory_.GetWeakPtr(), service_connection,
std::move(split_callback.first), retry_count),
weak_ptr_factory_.GetWeakPtr(), service_connection),
base::BindOnce(
&ServiceVideoCaptureProvider::OnDeviceInfosRequestDropped,
weak_ptr_factory_.GetWeakPtr(), service_connection,
std::move(split_callback.second), retry_count)));
weak_ptr_factory_.GetWeakPtr(), service_connection)));
}

void ServiceVideoCaptureProvider::OnDeviceInfosReceived(
scoped_refptr<RefCountedVideoSourceProvider> service_connection,
GetDeviceInfosCallback result_callback,
int retry_count,
const std::vector<media::VideoCaptureDeviceInfo>& infos) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
#if BUILDFLAG(IS_MAC)
std::string model = base::mac::GetModelIdentifier();
if (base::FeatureList::IsEnabled(
features::kRetryGetVideoCaptureDeviceInfos) &&
base::StartsWith(model, "MacBook",
base::CompareCase::INSENSITIVE_ASCII)) {
if (infos.empty() && retry_count < kMaxRetriesForGetDeviceInfos &&
!stashed_result_callback_for_retry_) {
TRACE_EVENT_INSTANT0(TRACE_DISABLED_BY_DEFAULT("video_and_image_capture"),
"Waiting for video capture service to shut down.",
TRACE_EVENT_SCOPE_PROCESS);
stashed_result_callback_for_retry_ = std::move(result_callback);
stashed_retry_count_ = retry_count;

// We may try again once |OnServiceStopped()| is invoked via our
// ServiceProcessHost observer.
return;
}
for (GetDeviceInfosCallback& callback : get_device_infos_pending_callbacks_) {
std::move(callback).Run(media::mojom::DeviceEnumerationResult::kSuccess,
infos);
}
#endif
std::move(result_callback)
.Run(media::mojom::DeviceEnumerationResult::kSuccess, infos);
get_device_infos_pending_callbacks_.clear();
}

void ServiceVideoCaptureProvider::OnDeviceInfosRequestDropped(
scoped_refptr<RefCountedVideoSourceProvider> service_connection,
GetDeviceInfosCallback result_callback,
int retry_count) {
scoped_refptr<RefCountedVideoSourceProvider> service_connection) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
std::move(result_callback)
.Run(media::mojom::DeviceEnumerationResult::kErrorCaptureServiceCrash,
std::vector<media::VideoCaptureDeviceInfo>());
if (base::FeatureList::IsEnabled(
features::kRetryGetVideoCaptureDeviceInfos)) {
if (!get_device_infos_retried_) {
get_device_infos_retried_ = true;
// Do nothing, OnServiceStopped will retry the query automatically when
// the service has been torn down.
return;
}
LOG(WARNING) << "Too many GetDeviceInfos() retries";
emit_log_message_cb_.Run(
"ServiceVideoCaptureProvider::OnDeviceInfosRequestDropped: Too many "
"retries");
}

// After too many retries, we just return an empty list
for (GetDeviceInfosCallback& callback : get_device_infos_pending_callbacks_) {
std::move(callback).Run(
media::mojom::DeviceEnumerationResult::kErrorCaptureServiceCrash,
std::vector<media::VideoCaptureDeviceInfo>());
}
get_device_infos_pending_callbacks_.clear();
}

void ServiceVideoCaptureProvider::OnLostConnectionToSourceProvider() {
Expand Down
17 changes: 6 additions & 11 deletions content/browser/renderer_host/media/service_video_capture_provider.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#ifndef CONTENT_BROWSER_RENDERER_HOST_MEDIA_SERVICE_VIDEO_CAPTURE_PROVIDER_H_
#define CONTENT_BROWSER_RENDERER_HOST_MEDIA_SERVICE_VIDEO_CAPTURE_PROVIDER_H_

#include <vector>

#include "base/threading/sequence_bound.h"
#include "base/threading/thread_checker.h"
#include "base/time/time.h"
Expand Down Expand Up @@ -69,17 +71,12 @@ class CONTENT_EXPORT ServiceVideoCaptureProvider
[[nodiscard]] scoped_refptr<RefCountedVideoSourceProvider>
LazyConnectToService();

void GetDeviceInfosAsyncForRetry(GetDeviceInfosCallback result_callback,
int retry_count);
void GetDeviceInfosAsyncForRetry();
void OnDeviceInfosReceived(
scoped_refptr<RefCountedVideoSourceProvider> service_connection,
GetDeviceInfosCallback result_callback,
int retry_count,
const std::vector<media::VideoCaptureDeviceInfo>& infos);
void OnDeviceInfosRequestDropped(
scoped_refptr<RefCountedVideoSourceProvider> service_connection,
GetDeviceInfosCallback result_callback,
int retry_count);
scoped_refptr<RefCountedVideoSourceProvider> service_connection);
void OnLostConnectionToSourceProvider();
void OnServiceConnectionClosed(ReasonForDisconnect reason);

Expand All @@ -94,10 +91,8 @@ class CONTENT_EXPORT ServiceVideoCaptureProvider
base::TimeTicks time_of_last_connect_;
base::TimeTicks time_of_last_uninitialize_;

#if BUILDFLAG(IS_MAC)
GetDeviceInfosCallback stashed_result_callback_for_retry_;
int stashed_retry_count_;
#endif
std::vector<GetDeviceInfosCallback> get_device_infos_pending_callbacks_;
bool get_device_infos_retried_ = false;

// We own this but it must operate on the UI thread.
class ServiceProcessObserver;
Expand Down
51 changes: 34 additions & 17 deletions content/browser/video_capture_service.cc → ...ent/browser/video_capture_service_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "content/public/browser/video_capture_service.h"
#include "content/browser/video_capture_service_impl.h"

#include "base/no_destructor.h"
#include "base/task/thread_pool.h"
Expand All @@ -13,6 +13,7 @@
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/child_process_host.h"
#include "content/public/browser/service_process_host.h"
#include "content/public/browser/video_capture_service.h"
#include "content/public/common/content_features.h"
#include "content/public/common/content_switches.h"
#include "mojo/public/cpp/bindings/receiver_set.h"
Expand All @@ -31,6 +32,7 @@
namespace content {

namespace {
std::atomic<bool> g_use_safe_mode(false);

video_capture::mojom::VideoCaptureService* g_service_override = nullptr;

Expand Down Expand Up @@ -76,6 +78,11 @@ void BindProxyRemoteOnUIThread(

} // namespace

void EnableVideoCaptureServiceSafeMode() {
LOG(WARNING) << "Enabling safe mode VideoCaptureService";
g_use_safe_mode = true;
}

video_capture::mojom::VideoCaptureService& GetVideoCaptureService() {
if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
static base::SequenceLocalStorageSlot<
Expand All @@ -90,8 +97,9 @@ video_capture::mojom::VideoCaptureService& GetVideoCaptureService() {
return *remote.get();
}

if (g_service_override)
if (g_service_override) {
return *g_service_override;
}

auto& remote = GetUIThreadRemote();
if (!remote.is_bound()) {
Expand All @@ -105,22 +113,31 @@ video_capture::mojom::VideoCaptureService& GetVideoCaptureService() {
FROM_HERE,
base::BindOnce(&BindInProcessInstance, std::move(receiver)));
} else {
ServiceProcessHost::Launch(
std::move(receiver),
ServiceProcessHost::Options()
.WithDisplayName("Video Capture")
ServiceProcessHost::Options options;
options.WithDisplayName("Video Capture");
#if BUILDFLAG(IS_MAC)
// On Mac, the service requires a CFRunLoop which is provided by a
// UI message loop. See https://crbug.com/834581.
.WithExtraCommandLineSwitches({switches::kMessageLoopTypeUi})
// On Mac, the service also needs to have a different set of
// entitlements, the reason being that some virtual cameras
// are not signed or are signed by a different Team ID. Hence,
// library validation has to be disabled (see
// http://crbug.com/990381#c21).
.WithChildFlags(ChildProcessHost::CHILD_PLUGIN)
// On Mac, the service requires a CFRunLoop which is provided by a
// UI message loop. See https://crbug.com/834581.
options.WithExtraCommandLineSwitches({switches::kMessageLoopTypeUi});
if (g_use_safe_mode) {
// When safe-mode is enabled, we keep the original entitlements and the
// hardened runtime to only load safe DAL plugins and reduce crash risk
// from third-party DAL plugins.
// As this is not possible to do with unsigned developer builds, we use
// an undocumented environment variable that macOS CMIO module checks to
// prevent loading any plugins.
setenv("CMIO_DAL_Ignore_Standard_PlugIns", "", 1);
} else {
// On Mac, the service also needs to have a different set of
// entitlements, the reason being that some virtual cameras DAL plugins
// are not signed or are signed by a different Team ID. Hence,
// library validation has to be disabled (see
// http://crbug.com/990381#c21).
options.WithChildFlags(ChildProcessHost::CHILD_PLUGIN);
}
#endif
.Pass());

ServiceProcessHost::Launch(std::move(receiver), options.Pass());

#if !BUILDFLAG(IS_ANDROID)
// On Android, we do not use automatic service shutdown, because when
Expand All @@ -144,7 +161,7 @@ video_capture::mojom::VideoCaptureService& GetVideoCaptureService() {
return *remote.get();
}

void OverrideVideoCaptureServiceForTesting(
void OverrideVideoCaptureServiceForTesting( // IN-TEST
video_capture::mojom::VideoCaptureService* service) {
g_service_override = service;
}
Expand Down
20 changes: 20 additions & 0 deletions content/browser/video_capture_service_impl.h
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CONTENT_BROWSER_VIDEO_CAPTURE_SERVICE_IMPL_H_
#define CONTENT_BROWSER_VIDEO_CAPTURE_SERVICE_IMPL_H_

#include "content/common/content_export.h"
#include "services/video_capture/public/mojom/video_capture_service.mojom-forward.h"

namespace content {

// Enables a safe-mode VideoCaptureService.
// On macOS, this disables 3rd-party DAL plugins from being loaded.
// It currently has no effect on other platforms.
void EnableVideoCaptureServiceSafeMode();

} // namespace content

#endif // CONTENT_BROWSER_VIDEO_CAPTURE_SERVICE_IMPL_H_

0 comments on commit 8764dd4

Please sign in to comment.