Passing null for sandbox results in harsh sandboxing

Think CEF could benefit from a new feature or capability? Discuss CEF feature requests here.

Passing null for sandbox results in harsh sandboxing

Postby rcdrone » Wed Mar 04, 2020 2:35 am

The OBS project is trying to implement the new Windows 10 window capture, but it fails when running as admin alongside CEF because they pass NULL sandbox info into CefInitialize. This results in Chromium using MITIGATION_HARDEN_TOKEN_IL_POLICY, which eventually leads to capture failure.

I am able to work around this by linking cef_sandbox.lib and using CefScopedSandboxInfo, but linking against cef_sandbox.lib is not something the OBS project wants to do if they can help it.
rcdrone
Newbie
 
Posts: 4
Joined: Wed Mar 04, 2020 2:17 am

Re: Passing null for sandbox results in harsh sandboxing

Postby magreenblatt » Wed Mar 04, 2020 10:39 am

You should also set CefSettings.no_sandbox (or pass the `--no-sandbox` command-line flag) when disabling the sandbox.
magreenblatt
Site Admin
 
Posts: 12382
Joined: Fri May 29, 2009 6:57 pm

Re: Passing null for sandbox results in harsh sandboxing

Postby rcdrone » Wed Mar 04, 2020 2:44 pm

no_sandbox was already set to true. This is the relevant code block from CefContext::Initialize

sandbox::SandboxInterfaceInfo sandbox_info = {0};
if (windows_sandbox_info == nullptr) {
content::InitializeSandboxInfo(&sandbox_info); // bad for OBS
windows_sandbox_info = &sandbox_info;
settings_.no_sandbox = true;
}

This is the Chromium implementation of content::InitializeSandboxInfo:

void InitializeSandboxInfo(sandbox::SandboxInterfaceInfo* info) {
info->broker_services = sandbox::SandboxFactory::GetBrokerServices();
if (!info->broker_services) {
info->target_services = sandbox::SandboxFactory::GetTargetServices();
} else {
// Ensure the proper mitigations are enforced for the browser process.
sandbox::ApplyProcessMitigationsToCurrentProcess(
sandbox::MITIGATION_DEP | sandbox::MITIGATION_DEP_NO_ATL_THUNK |
sandbox::MITIGATION_HARDEN_TOKEN_IL_POLICY);
// Note: these mitigations are "post-startup". Some mitigations that need
// to be enabled sooner (e.g. MITIGATION_EXTENSION_POINT_DISABLE) are done
// so in Chrome_ELF.
}
}

MITIGATION_HARDEN_TOKEN_IL_POLICY is bad for OBS. The CEF version of InitializeSandboxInfo works better for us, but cef_sandbox.lib is proving problematic to build and link against.

void InitializeSandboxInfo(sandbox::SandboxInterfaceInfo* info) {
info->broker_services = sandbox::SandboxFactory::GetBrokerServices();
if (!info->broker_services) {
info->target_services = sandbox::SandboxFactory::GetTargetServices();
} else {
// Ensure the proper mitigations are enforced for the browser process.
sandbox::ApplyProcessMitigationsToCurrentProcess(
sandbox::MITIGATION_DEP | sandbox::MITIGATION_DEP_NO_ATL_THUNK);
}
}

The current solution they have arrived on is to pass uintptr_t[32]{} as windows_sandbox_info, which I am not a fan of.
rcdrone
Newbie
 
Posts: 4
Joined: Wed Mar 04, 2020 2:17 am

Re: Passing null for sandbox results in harsh sandboxing

Postby magreenblatt » Wed Mar 04, 2020 3:55 pm

The CEF implementation of InitializeSandboxInfo should match the Chromium implementation. Beyond that, it probably doesn't make sense to apply any mitigations for CEF clients if the sandbox is disabled.
magreenblatt
Site Admin
 
Posts: 12382
Joined: Fri May 29, 2009 6:57 pm

Re: Passing null for sandbox results in harsh sandboxing

Postby rcdrone » Wed Mar 04, 2020 4:15 pm

Can you provide a way to call CefInitialize without calling ApplyProcessMitigationsToCurrentProcess?
rcdrone
Newbie
 
Posts: 4
Joined: Wed Mar 04, 2020 2:17 am

Re: Passing null for sandbox results in harsh sandboxing

Postby magreenblatt » Wed Mar 04, 2020 4:17 pm

rcdrone wrote:Can you provide a way to call CefInitialize without calling ApplyProcessMitigationsToCurrentProcess?

You're welcome to do so, and submit a PR.
magreenblatt
Site Admin
 
Posts: 12382
Joined: Fri May 29, 2009 6:57 pm

Re: Passing null for sandbox results in harsh sandboxing

Postby rcdrone » Thu Mar 05, 2020 3:16 am

I don't want to submit a PR until the change has been tested, but it does exist: https://bitbucket.org/rcdrone/cef/commi ... e412da514b

I'm just waiting to see if one of the OBS volunteers can make a CEF build with my change to make sure it works.
rcdrone
Newbie
 
Posts: 4
Joined: Wed Mar 04, 2020 2:17 am


Return to Feature Request Forum

Who is online

Users browsing this forum: No registered users and 15 guests