Skip to content

Commit

Permalink
Win32k lockdown: move to chrome://flags, add UMA, add Finch.
Browse files Browse the repository at this point in the history
Supplying via command line is not sustainable because shortcuts are sometimes sanitized, so moving to a flag so a persistent state can be set.

Also add a Finch experiment to enable win32k lockdown, and an UMA metric to report when it's enabled (either by Finch or flags)

BUG=365160

Review URL: https://codereview.chromium.org/792873004

Cr-Commit-Position: refs/heads/master@{#307953}
  • Loading branch information
wfh-chromium authored and Commit bot committed Dec 11, 2014
1 parent 0dd63b3 commit d672334
Show file tree
Hide file tree
Showing 10 changed files with 56 additions and 15 deletions.
6 changes: 6 additions & 0 deletions chrome/app/generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -5807,6 +5807,12 @@ Keep your key file in a safe place. You will need it to create new versions of y
<message name="IDS_FLAGS_DISABLE_DIRECT_WRITE_DESCRIPTION" desc="Description of the 'Disable DirectWrite' lab.">
Disables the use of experimental DirectWrite font rendering system.
</message>
<message name="IDS_FLAGS_ENABLE_WIN32K_RENDERER_LOCKDOWN_NAME" desc="Name of the 'Enable win32k renderer lockdown' lab.">
Enable win32k renderer lockdown
</message>
<message name="IDS_FLAGS_ENABLE_WIN32K_RENDERER_LOCKDOWN_DESCRIPTION" desc="Description of the 'Enable win32k renderer lockdown' lab.">
Enables the win32k renderer lockdown, which is only available on Windows 8 and above.
</message>
</if>
<message name="IDS_FLAGS_ENABLE_EXPERIMENTAL_CANVAS_FEATURES_NAME" desc="Name of the 'Enable experimental canvas features' lab.">
Enable experimental canvas features
Expand Down
8 changes: 8 additions & 0 deletions chrome/browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,14 @@ const Experiment kExperiments[] = {
kOsWin,
SINGLE_VALUE_TYPE(switches::kDisableDirectWrite)
},
{
"enable-win32k-renderer-lockdown",
IDS_FLAGS_ENABLE_WIN32K_RENDERER_LOCKDOWN_NAME,
IDS_FLAGS_ENABLE_WIN32K_RENDERER_LOCKDOWN_DESCRIPTION,
kOsWin,
ENABLE_DISABLE_VALUE_TYPE(switches::kEnableWin32kRendererLockDown,
switches::kDisableWin32kRendererLockDown)
},
#endif
{
"enable-experimental-canvas-features",
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/chrome_browser_main_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,7 @@ int ChromeBrowserMainPartsWin::PreCreateThreads() {
// launched for the plugin does not have the Win32K lockdown mode enabled.
// TODO(ananta)
// Revisit this when the pdf plugin uses skia and stops using GDI.
if (CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableWin32kRendererLockDown) &&
if (switches::IsWin32kRendererLockdownEnabled() &&
base::win::GetVersion() >= base::win::VERSION_WIN8) {
CommandLine::ForCurrentProcess()->AppendSwitch(
switches::kEnableOutOfProcessPdf);
Expand Down Expand Up @@ -275,6 +274,8 @@ void ChromeBrowserMainPartsWin::PostBrowserStart() {
ChromeBrowserMainParts::PostBrowserStart();

UMA_HISTOGRAM_BOOLEAN("Windows.Tablet", base::win::IsTabletDevice());
UMA_HISTOGRAM_BOOLEAN("Windows.Win32kRendererLockdown",
switches::IsWin32kRendererLockdownEnabled());

// Set up a task to verify installed modules in the current process. Use a
// delay to reduce the impact on startup time.
Expand Down
1 change: 1 addition & 0 deletions content/browser/renderer_host/render_process_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1300,6 +1300,7 @@ void RenderProcessHostImpl::PropagateBrowserCommandLineToRenderer(
#if defined(OS_WIN)
switches::kDisableDirectWrite,
switches::kEnableWin32kRendererLockDown,
switches::kDisableWin32kRendererLockDown,
#endif
#if defined(OS_CHROMEOS)
switches::kDisableVaapiAcceleratedVideoEncode,
Expand Down
7 changes: 3 additions & 4 deletions content/common/sandbox_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -620,10 +620,9 @@ base::Process StartSandboxedProcess(
sandbox::MITIGATION_DEP_NO_ATL_THUNK |
sandbox::MITIGATION_SEHOP;

if (base::win::GetVersion() >= base::win::VERSION_WIN8 &&
type_str == switches::kRendererProcess &&
browser_command_line.HasSwitch(
switches::kEnableWin32kRendererLockDown)) {
if (base::win::GetVersion() >= base::win::VERSION_WIN8 &&
type_str == switches::kRendererProcess &&
switches::IsWin32kRendererLockdownEnabled()) {
if (policy->AddRule(sandbox::TargetPolicy::SUBSYS_WIN32K_LOCKDOWN,
sandbox::TargetPolicy::FAKE_USER_GDI_INIT,
NULL) != sandbox::SBOX_ALL_OK) {
Expand Down
27 changes: 22 additions & 5 deletions content/public/common/content_switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "content/public/common/content_switches.h"

#include "base/command_line.h"
#include "base/metrics/field_trial.h"

namespace switches {

Expand Down Expand Up @@ -981,11 +982,13 @@ const char kDeviceScaleFactor[] = "device-scale-factor";
// Disable the Legacy Window which corresponds to the size of the WebContents.
const char kDisableLegacyIntermediateWindow[] = "disable-legacy-window";

// Enable the Win32K process mitigation policy for renderer processes which
// prevents them from invoking user32 and gdi32 system calls which enter
// the kernel. This is only supported on Windows 8 and beyond.
const char kEnableWin32kRendererLockDown[]
= "enable_win32k_renderer_lockdown";
// Enables or disables the Win32K process mitigation policy for renderer
// processes which prevents them from invoking user32 and gdi32 system calls
// which enter the kernel. This is only supported on Windows 8 and beyond.
const char kDisableWin32kRendererLockDown[] =
"disable-win32k-renderer-lockdown";
const char kEnableWin32kRendererLockDown[] =
"enable-win32k-renderer-lockdown";

// DirectWrite FontCache is shared by browser to renderers using shared memory.
// This switch allows specifying suffix to shared memory section name to avoid
Expand All @@ -998,6 +1001,20 @@ const char kFontCacheSharedMemSuffix[] = "font-cache-shared-mem-suffix";
const char kEnablePluginPowerSaver[] = "enable-plugin-power-saver";
#endif

#if defined(OS_WIN)
bool IsWin32kRendererLockdownEnabled() {
const std::string group_name =
base::FieldTrialList::FindFullName("Win32kLockdown");
const base::CommandLine* cmd_line = CommandLine::ForCurrentProcess();
if (cmd_line->HasSwitch(kEnableWin32kRendererLockDown))
return true;
if (cmd_line->HasSwitch(kDisableWin32kRendererLockDown))
return false;
// Default.
return group_name == "Enabled";
}
#endif

// Don't dump stuff here, follow the same order as the header.

} // namespace switches
2 changes: 2 additions & 0 deletions content/public/common/content_switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -293,10 +293,12 @@ CONTENT_EXPORT extern const char kDeviceScaleFactor[];
CONTENT_EXPORT extern const char kDisableLegacyIntermediateWindow[];
// This switch will be removed when we enable the win32K lockdown process
// mitigation.
CONTENT_EXPORT extern const char kDisableWin32kRendererLockDown[];
CONTENT_EXPORT extern const char kEnableWin32kRendererLockDown[];
// Switch to uniquely identify names shared memory section for font cache
// across chromium flavors.
CONTENT_EXPORT extern const char kFontCacheSharedMemSuffix[];
CONTENT_EXPORT bool IsWin32kRendererLockdownEnabled();
#endif

#if defined(ENABLE_PLUGINS)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,7 @@ scoped_ptr<ResourceHost> ContentRendererPepperHostFactory::CreateResourceHost(
// TODO(ananta)
// Look into whether this causes a loss of functionality. From cursory
// testing things seem to work well.
if (CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableWin32kRendererLockDown) &&
if (switches::IsWin32kRendererLockdownEnabled() &&
base::win::GetVersion() >= base::win::VERSION_WIN8) {
image_type = ppapi::PPB_ImageData_Shared::SIMPLE;
}
Expand Down
3 changes: 1 addition & 2 deletions content/renderer/pepper/resource_creation_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,7 @@ PP_Resource ResourceCreationImpl::CreateImageData(PP_Instance instance,
// TODO(ananta)
// Look into whether this causes a loss of functionality. From cursory
// testing things seem to work well.
if (CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableWin32kRendererLockDown) &&
if (switches::IsWin32kRendererLockdownEnabled() &&
base::win::GetVersion() >= base::win::VERSION_WIN8) {
return CreateImageDataSimple(instance, format, size, init_to_zero);
}
Expand Down
9 changes: 9 additions & 0 deletions tools/metrics/histograms/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -41306,6 +41306,13 @@ Therefore, the affected-histogram name has to have at least one dot in it.
<summary>Count of browser launches from a Windows tablet pc.</summary>
</histogram>

<histogram name="Windows.Win32kRendererLockdown" enum="BooleanEnabled">
<owner>wfh@chromium.org</owner>
<summary>
Count of browser launches where Win32k renderer lockdown is enabled.
</summary>
</histogram>

<histogram name="WinJumplist.Action" enum="WinJumplistCategory">
<owner>noms@chromium.org</owner>
<summary>The type of category clicked in the Windows Jumplist</summary>
Expand Down Expand Up @@ -49397,6 +49404,7 @@ To add a new entry, add it with any value and run test to compute valid value.
<int value="-418868128" label="enable-experimental-web-platform-features"/>
<int value="-385337473" label="enable-fast-unload"/>
<int value="-384589459" label="disable-supervised-user-safesites"/>
<int value="-378033324" label="disable-win32k-renderer-lockdown"/>
<int value="-349057743" label="extensions-on-chrome-urls"/>
<int value="-344343842" label="disable-experimental-app-list"/>
<int value="-340622848" label="disable-javascript-harmony-shipping"/>
Expand Down Expand Up @@ -49598,6 +49606,7 @@ To add a new entry, add it with any value and run test to compute valid value.
<int value="2121776031" label="auto-virtual-keyboard"/>
<int value="2122876605" label="enable-bleeding-edge-rendering-fast-paths"/>
<int value="2137347307" label="enable-drive-apps-in-app-list"/>
<int value="2137599770" label="enable-win32k-renderer-lockdown"/>
</enum>

<enum name="LoginFailureReason" type="int">
Expand Down

0 comments on commit d672334

Please sign in to comment.