1511438 - add win32k lockdown to Win RDD sandbox

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, enhancement, P1)

Description

[Michael Froman [:mjf]]

From comment 20 from Bug 1498624 [1]:
SUBSYS_WIN32K_LOCKDOWN (similar to GMP's sandbox) should be added to the Win RDD sandbox.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1498624#c20

Comment 1

[Bob Owen (:bobowen)]

First thing that strikes me is that using the child UI loop (same as GPU) is the first problem.

Comment 2

[Bob Owen (:bobowen)]

Try push with just debug failures for broken COM initialization, that I don't think we need:
https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=223579188&revision=f154121a3af0123f9c0db03e6ce61768c457d880

Another media test one with that COM initialization removed:
https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=223579188&revision=1fde80cb5da233a6552e6628b26bc5133911c6db

Comment 3

[Bob Owen (:bobowen)]

Attachment: Part 1: Replace ProcessTypeRequiresWinEventHook with XRE_Win32kCallsAllowed

ProcessTypeRequiresWinEventHook was added when attempting to turn on win32k
lockdown for GMP processes. Having a less specific, but globally accessible,
function will make it more useful while applying win32k lockdown to other
process types.

Comment 4

[Bob Owen (:bobowen)]

Attachment: Part 2: Enable win32k lockdown on RDD process

This stops the use of some win32k calls during start-up that will fail and in some cases cause a crash.

It also moves the MITIGATION_DYNAMIC_CODE_DISABLE to be enabled after start-up.
This is required because the hooks to fake the user32 and gdi32 initialization
are applied as the DLLs load and the dynamic code disable blocks that.

Comment 5

[Jean-Yves Avenard [:jya]]

Comment on attachment 9039103 [details] [diff] [review]
Part 2: Enable win32k lockdown on RDD process

Review of attachment 9039103 [details] [diff] [review]:
-----------------------------------------------------------------

Don't know enough about this code to review it

Comment 6

[Bob Owen (:bobowen)]

(In reply to Jean-Yves Avenard [:jya] from comment #5)

Comment on attachment 9039103 [details] [diff] [review]
Part 2: Enable win32k lockdown on RDD process

Review of attachment 9039103 [details] [diff] [review]:

Don't know enough about this code to review it

Sorry, that's why I added in jimm as well to cover the Windows widget stuff.

I should have made it clear, basically I'm asking if you are OK for me to land this on Fx67 after the merge?

Comment 7

[Jean-Yves Avenard [:jya]]

Sure, as it doesn't impact usage and working of the RDD

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3a9c07fcc2eb
Part 1: Replace ProcessTypeRequiresWinEventHook with XRE_Win32kCallsAllowed. r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/55a7c08b3b67
Part 2: Enable win32k lockdown on RDD process. r=jimm

Comment 9

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/3a9c07fcc2eb
https://hg.mozilla.org/mozilla-central/rev/55a7c08b3b67