1608558 - Add linux sandboxing to socket process

Status: RESOLVED FIXED

(Core :: Networking, enhancement, P2)

Description

[Michael Froman [:mjf]]

The socket process currently has no sandboxing. We can start with Linux first.

Comment 1

[Michael Froman [:mjf]]

My plan is to take most of what works for RDD and then see where I need help from the sandboxing team.

Comment 2

[Dragana Damjanovic [:dragana]]

Just an info: at some point we will move all network operation to the socket process (we will try to do so if we do not have a lot performance penalty). I am writing the note here, so that sandbox team is aware (if not already aware) of it.

Comment 3

[Michael Froman [:mjf]]

Attachment: Bug 1608558 - pt1 - add linux sandboxing to socket process. r?jld!

Comment 4

[Michael Froman [:mjf]]

Jed, I've added most of the linux sandboxing code for the socket process that follows what you did for the linux sandbox on RDD. I still have some mochitest failures and I'm not sure what to change in the sandbox to fix them. Would have time to help troubleshoot? I've submitted a new try run with the patch that I submitted to phabricator:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4aa1638979e838e8dd3c1466f2c2e89bb64305c0

Here is a slightly older run that is already complete, but should be identical code (just in multiple patches while I experimented with it):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2bead52329815626db8eefe6cb3411533060cace&selectedJob=283865740

Comment 5

[Michael Froman [:mjf]]

Here is a concrete failure:
./mach mochitest --keep-open=false --headless dom/media/tests/mochitest/identity/test_peerConnection_asymmetricIsolation.html

This test hangs waiting for network activity when the socket process crashes here[1]. After adding some local instrumentation, we're failing inside of PORT_LoadLibraryFromOrigin[2]. The full path (for me) is /home/mfroman/mozilla/obj/deb/dist/bin/libnss3.so, and newShLibName is libsoftokn3.so.

[1] https://searchfox.org/mozilla-central/source/media/webrtc/signaling/src/peerconnection/MediaTransportHandler.cpp#359
[2] https://searchfox.org/mozilla-central/source/security/nss/lib/util/secload.c#131

Comment 6

[Michael Froman [:mjf]]

More info: after temporarily adding read access to the firefox binary directory like here[1], I hit this:

 1:01.91 GECKO(21318) Sandbox: seccomp sandbox violation: pid 21345, tid 21345, syscall 102, args 140590155486248 140590155490087 0 87 0 0.  Killing process.

[1] https://searchfox.org/mozilla-central/source/security/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp#400-413

Comment 7

[Gian-Carlo Pascutto [:gcp]]

Syscall 102 is getuid. Looks like you'll have to whitelist that call.

https://filippo.io/linux-syscall-table/

Comment 8

[Michael Froman [:mjf]]

Attachment: Bug 1608558 - pt2 - add SandboxBrokerPolicyFactory::GetSocketProcessPolicy to allow access to certs. r?jld!

Depends on D60014

Comment 9

[Michael Froman [:mjf]]

Attachment: Bug 1608558 - pt3 - add EvaluateSocketCall and missing cases to EvaluateSyscall for Socket process sandbox. r?jld!

Depends on D62444

Comment 10

[Michael Froman [:mjf]]

With the current patch sets, I'm getting this failure:

 1:01.97 GECKO(8574) (generic/CRIT) PR_Connect failed: -5980

I've a tried a few things, but nothing has changed that failure. Any thoughts?

Comment 11

[Gian-Carlo Pascutto [:gcp]]

PR_Connect failed: -5980

nsprpub/pr/include/prerr.h
76:#define PR_NETWORK_UNREACHABLE_ERROR (-5980L)

So what could cause the network to be unreachable from within the socket process? The best guess would be that you mistakenly created a new network namespace for the socket process. Some digging through the patch confirms that, I'll point out the mistake in Phabricator.

Comment 12

[Michael Froman [:mjf]]

Good try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e50869285e0f0ebc87914312e419b002c8d0d51d

There is a problem in dt2 on Linux 18.04 x64 debug, but the same tests seem to fail regularly on the parent commit:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=b47e32ff95b8d544ea1e94374e1d5ad8929bba39&selectedJob=288804919

Comment 13

[Michael Froman [:mjf]]

:jld, :gcp,
Who's correct person to review?

Comment 14

[Gian-Carlo Pascutto [:gcp]]

I did a quick first pass, jld will have to do the final review.

What do you think about making the socket process sandbox correspond to level 5? This would allow a quick fallback to 4 if we find problems in the field.

Comment 15

[Gian-Carlo Pascutto [:gcp]]

What do you think about making the socket process sandbox correspond to level 5? This would allow a quick fallback to 4 if we find problems in the field.

jld reminded me the pref we have is for content only. Other sandboxes have to rely on ENV vars. (Maybe they should have prefs too, especially if we think some breakage is possible?)

Comment 16

[Michael Froman [:mjf]]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #15)

What do you think about making the socket process sandbox correspond to level 5? This would allow a quick fallback to 4 if we find problems in the field.

jld reminded me the pref we have is for content only. Other sandboxes have to rely on ENV vars. (Maybe they should have prefs too, especially if we think some breakage is possible?)

Would a simple 'enabled' pref work for this case? That would be fairly easy to add.

Comment 17

[Gian-Carlo Pascutto [:gcp]]

I'd add a "level" one similar to the one for content. If we want to make this sandbox useful we'll have to follow up on this to try to filter the allowed socket types etc, and that's the kind of thing that would be useful to dial back. A level wouldn't be much more complicated than a boolean toggle, I think.

Comment 18

[Michael Froman [:mjf]]

Attachment: Bug 1608558 - pt4 - use security.sandbox.socket.process.level for linux socket process sandbox. r?jld!

Depends on D62445

Comment 19

[Michael Froman [:mjf]]

:gcp, I added a new patch that adds and uses a level pref.

Comment 20

[Gian-Carlo Pascutto [:gcp]]

I added a new patch that adds and uses a level pref.

Looks good. As I commented, I'd set the base level to 1, not 4, and then file a follow-up bug where we try to restrict the allowed connection types and options (which would then increase the level to 2, etc).

Comment 21

[Michael Froman [:mjf]]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #20)

I added a new patch that adds and uses a level pref.

Looks good. As I commented, I'd set the base level to 1, not 4, and then file a follow-up bug where we try to restrict the allowed connection types and options (which would then increase the level to 2, etc).

Sorry, I misunderstood you in Comment 14. I'll thought we were starting at 4 or 5 and going higher. I'll fix.

Comment 22

[Michael Froman [:mjf]]

:gcp I think I've addressed your concerns in pt4. Also, are you satisfied w/ :jld's explanation on pt2, or do you still want me to look into specifically allowing only libsoftokn3.so and its dependencies? If so, can you point me to an example where that type of thing is done?

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/10e01cf10faf
pt1 - add linux sandboxing to socket process. r=gcp
https://hg.mozilla.org/integration/autoland/rev/adc30f2f9a5c
pt2 - add SandboxBrokerPolicyFactory::GetSocketProcessPolicy to allow access to certs. r=jld,gcp
https://hg.mozilla.org/integration/autoland/rev/678c6a3d00ee
pt3 - add EvaluateSocketCall and missing cases to EvaluateSyscall for Socket process sandbox. r=gcp
https://hg.mozilla.org/integration/autoland/rev/43e7de62af7f
pt4 - use security.sandbox.socket.process.level for linux socket process sandbox. r=gcp

Comment 24

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/10e01cf10faf
https://hg.mozilla.org/mozilla-central/rev/adc30f2f9a5c
https://hg.mozilla.org/mozilla-central/rev/678c6a3d00ee
https://hg.mozilla.org/mozilla-central/rev/43e7de62af7f