Skip to content

Commit

Permalink
use kService for V8 proxy resolver
Browse files Browse the repository at this point in the history
Before this CL the V8 proxy resolver was hosted in its own sandbox
type on Windows that was essentially the same as kUtility but denied
dynamic code. On other platforms it was in kUtility.

After this CL the V8 proxy resolver is hosted in the kService sandbox
type on all platforms. Compared to before this denies dynamic code
(so that all platforms now deny dynamic code), and on Windows, enables
Win32 lockdown.

kProxyResolver sandbox is no longer used, so is deleted.

Bug: 696635
Change-Id: I786606a208a1f7c1cfbf72a82c699722df2bf183
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3119839
Commit-Queue: Alex Gough <ajgo@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#915721}
  • Loading branch information
quidity authored and Chromium LUCI CQ committed Aug 26, 2021
1 parent 66d857a commit 4b1dc03
Show file tree
Hide file tree
Showing 11 changed files with 2 additions and 44 deletions.
2 changes: 0 additions & 2 deletions chrome/browser/chrome_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3726,7 +3726,6 @@ std::wstring ChromeContentBrowserClient::GetAppContainerSidForSandboxType(
case sandbox::policy::SandboxType::kPrintCompositor:
case sandbox::policy::SandboxType::kAudio:
case sandbox::policy::SandboxType::kSpeechRecognition:
case sandbox::policy::SandboxType::kProxyResolver:
case sandbox::policy::SandboxType::kPdfConversion:
case sandbox::policy::SandboxType::kService:
case sandbox::policy::SandboxType::kIconReader:
Expand Down Expand Up @@ -3793,7 +3792,6 @@ bool ChromeContentBrowserClient::PreSpawnChild(
case sandbox::policy::SandboxType::kPrintCompositor:
case sandbox::policy::SandboxType::kAudio:
case sandbox::policy::SandboxType::kSpeechRecognition:
case sandbox::policy::SandboxType::kProxyResolver:
case sandbox::policy::SandboxType::kPdfConversion:
case sandbox::policy::SandboxType::kService:
case sandbox::policy::SandboxType::kIconReader:
Expand Down
1 change: 0 additions & 1 deletion content/browser/utility_sandbox_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ UtilitySandboxedProcessLauncherDelegate::
sandbox_type_ ==
sandbox::policy::SandboxType::kNoSandboxAndElevatedPrivileges ||
sandbox_type_ == sandbox::policy::SandboxType::kXrCompositing ||
sandbox_type_ == sandbox::policy::SandboxType::kProxyResolver ||
sandbox_type_ == sandbox::policy::SandboxType::kPdfConversion ||
sandbox_type_ == sandbox::policy::SandboxType::kIconReader ||
sandbox_type_ == sandbox::policy::SandboxType::kMediaFoundationCdm ||
Expand Down
7 changes: 0 additions & 7 deletions content/browser/utility_sandbox_delegate_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -195,13 +195,6 @@ bool UtilitySandboxedProcessLauncherDelegate::PreSpawnTarget(
return false;
}

if (sandbox_type_ == sandbox::policy::SandboxType::kProxyResolver) {
sandbox::MitigationFlags flags = policy->GetDelayedProcessMitigations();
flags |= sandbox::MITIGATION_DYNAMIC_CODE_DISABLE;
if (sandbox::SBOX_ALL_OK != policy->SetDelayedProcessMitigations(flags))
return false;
}

if (sandbox_type_ == sandbox::policy::SandboxType::kSpeechRecognition) {
policy->SetDelayedIntegrityLevel(sandbox::INTEGRITY_LEVEL_LOW);
policy->SetIntegrityLevel(sandbox::INTEGRITY_LEVEL_LOW);
Expand Down
4 changes: 0 additions & 4 deletions sandbox/policy/mojom/sandbox.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,6 @@ enum Sandbox {
[EnableIf=is_win]
kPdfConversion,

// Like kUtility but also disables dynamic code.
[EnableIf=is_win]
kProxyResolver,

// |kXrCompositing| hosts XR Device Service on Windows.
[EnableIf=is_win]
kXrCompositing,
Expand Down
6 changes: 0 additions & 6 deletions sandbox/policy/sandbox_type.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ bool IsUnsandboxedSandboxType(SandboxType sandbox_type) {
return true;
case SandboxType::kXrCompositing:
return !base::FeatureList::IsEnabled(features::kXRSandbox);
case SandboxType::kProxyResolver:
case SandboxType::kPdfConversion:
case SandboxType::kIconReader:
case SandboxType::kMediaFoundationCdm:
Expand Down Expand Up @@ -126,7 +125,6 @@ void SetCommandLineFlagsForSandboxType(base::CommandLine* command_line,
#endif
#if defined(OS_WIN)
case SandboxType::kXrCompositing:
case SandboxType::kProxyResolver:
case SandboxType::kPdfConversion:
case SandboxType::kIconReader:
case SandboxType::kMediaFoundationCdm:
Expand Down Expand Up @@ -263,8 +261,6 @@ std::string StringFromUtilitySandboxType(SandboxType sandbox_type) {
#if defined(OS_WIN)
case SandboxType::kXrCompositing:
return switches::kXrCompositingSandbox;
case SandboxType::kProxyResolver:
return switches::kProxyResolverSandbox;
case SandboxType::kPdfConversion:
return switches::kPdfConversionSandbox;
case SandboxType::kIconReader:
Expand Down Expand Up @@ -339,8 +335,6 @@ SandboxType UtilitySandboxTypeFromString(const std::string& sandbox_string) {
#if defined(OS_WIN)
if (sandbox_string == switches::kXrCompositingSandbox)
return SandboxType::kXrCompositing;
if (sandbox_string == switches::kProxyResolverSandbox)
return SandboxType::kProxyResolver;
if (sandbox_string == switches::kPdfConversionSandbox)
return SandboxType::kPdfConversion;
if (sandbox_string == switches::kIconReaderSandbox)
Expand Down
5 changes: 0 additions & 5 deletions sandbox/policy/sandbox_type.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ enum class SandboxType {
// The XR Compositing process.
kXrCompositing,

// The proxy resolver process.
kProxyResolver,

// The PDF conversion service process used in printing.
kPdfConversion,

Expand Down Expand Up @@ -155,8 +152,6 @@ inline constexpr sandbox::policy::SandboxType MapToSandboxType(
return sandbox::policy::SandboxType::kNoSandboxAndElevatedPrivileges;
case sandbox::mojom::Sandbox::kPdfConversion:
return sandbox::policy::SandboxType::kPdfConversion;
case sandbox::mojom::Sandbox::kProxyResolver:
return sandbox::policy::SandboxType::kProxyResolver;
case sandbox::mojom::Sandbox::kXrCompositing:
return sandbox::policy::SandboxType::kXrCompositing;
#endif // OS_WIN
Expand Down
6 changes: 0 additions & 6 deletions sandbox/policy/sandbox_type_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,6 @@ TEST(SandboxTypeTest, Utility) {
EXPECT_EQ(SandboxType::kXrCompositing,
SandboxTypeFromCommandLine(command_line10));

base::CommandLine command_line11(command_line);
SetCommandLineFlagsForSandboxType(&command_line11,
SandboxType::kProxyResolver);
EXPECT_EQ(SandboxType::kProxyResolver,
SandboxTypeFromCommandLine(command_line11));

base::CommandLine command_line12(command_line);
SetCommandLineFlagsForSandboxType(&command_line12,
SandboxType::kPdfConversion);
Expand Down
1 change: 0 additions & 1 deletion sandbox/policy/switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ const char kVideoCaptureSandbox[] = "video_capture";

#if defined(OS_WIN)
const char kPdfConversionSandbox[] = "pdf_conversion";
const char kProxyResolverSandbox[] = "proxy_resolver";
const char kXrCompositingSandbox[] = "xr_compositing";
const char kIconReaderSandbox[] = "icon_reader";
const char kMediaFoundationCdmSandbox[] = "mf_cdm";
Expand Down
1 change: 0 additions & 1 deletion sandbox/policy/switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ SANDBOX_POLICY_EXPORT extern const char kVideoCaptureSandbox[];

#if defined(OS_WIN)
SANDBOX_POLICY_EXPORT extern const char kPdfConversionSandbox[];
SANDBOX_POLICY_EXPORT extern const char kProxyResolverSandbox[];
SANDBOX_POLICY_EXPORT extern const char kXrCompositingSandbox[];
SANDBOX_POLICY_EXPORT extern const char kIconReaderSandbox[];
SANDBOX_POLICY_EXPORT extern const char kMediaFoundationCdmSandbox[];
Expand Down
2 changes: 0 additions & 2 deletions sandbox/policy/win/sandbox_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1222,8 +1222,6 @@ std::string SandboxWin::GetSandboxTypeInEnglish(SandboxType sandbox_type) {
return "Audio";
case SandboxType::kSpeechRecognition:
return "Speech Recognition";
case SandboxType::kProxyResolver:
return "Proxy Resolver";
case SandboxType::kPdfConversion:
return "PDF Conversion";
case SandboxType::kMediaFoundationCdm:
Expand Down
11 changes: 2 additions & 9 deletions services/proxy_resolver/public/mojom/proxy_resolver.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -81,20 +81,13 @@ interface ProxyResolverRequestClient {
pending_remote<HostResolverRequestClient> client);
};

// Note: On Android this lives in-process in the browser.
[EnableIf=is_win]
const sandbox.mojom.Sandbox kProxyResolverSandbox
= sandbox.mojom.Sandbox.kProxyResolver;
[EnableIfNot=is_win]
const sandbox.mojom.Sandbox kProxyResolverSandbox
= sandbox.mojom.Sandbox.kUtility;

// Creates a ProxyResolver that uses the provided PAC script. The ProxyResolver
// will remain valid even after the ProxyResolverFactory has been destroyed.
//
// Destroying |client| before its ReportResult method is invoked may cancel
// creation of the ProxyResolverFactory.
[ServiceSandbox=kProxyResolverSandbox]
// Note: on Android this lives in the browser process.
[ServiceSandbox=sandbox.mojom.Sandbox.kService]
interface ProxyResolverFactory {
CreateResolver(string pac_script,
pending_receiver<ProxyResolver> receiver,
Expand Down

0 comments on commit 4b1dc03

Please sign in to comment.