1126437 - Remove socket creation/connect from Desktop Linux content processes

Status: RESOLVED FIXED

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

Description

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

The other half of bug 942698 is sockets.

One known problem here is audio access — one of the possible backends is PulseAudio, which uses sockets to send audio data to the server (typically Unix-domain for a local audio server, but remote audio over IP is possible), and connects to the X11 server to try to read configuration information from root window properties (also possibly remote, and very problematic for security in any case).

Hopefully it will be possible to address these issues with high-level remoting, but it's not completely implausible to add a "connect to local socket" operation to the file broker if need be.

Comment 1

[Julian Hector [:tedd] [:jhector]]

Moving to sblc4 which deals with this issue.

Comment 2

[Jim Mathies [:jimm]]

blocked by media work on cubeb. tbc end of H1.

Comment 3

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

The fix for bug 1384292 doesn't stop the socket()ing, but I'm planning to quietly deny socket() anyway.

Comment 4

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Attachment: Bug 1126437 - Reorganize content sandbox params extracted from libxul APIs.

The end goal is to allow the seccomp-bpf policy to vary based on the
content sandbox level.

Rather than add yet another parameter to SetContentProcessSandbox to
pass down the sandbox level, this collects the values that have to be
computed in libxul into a struct, and moves the code that computes it so
it's not cluttering up ContentChild.

Review commit: https://reviewboard.mozilla.org/r/214778/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/214778/

Comment 5

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Attachment: Bug 1126437 - Add Linux content sandbox level 4 for blocking socket APIs.

Review commit: https://reviewboard.mozilla.org/r/214780/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/214780/

Comment 6

[Code Review Bot [:reviewbot]]

Comment on attachment 8944632 [details]
Bug 1126437 - Add Linux content sandbox level 4 for blocking socket APIs.

https://reviewboard.mozilla.org/r/214780/#review220420


Static analysis found 1 defect in this patch.
 - 1 defect found by clang-tidy

You can run this analysis locally with:
 - `./mach static-analysis check path/to/file.cpp` (C/C++)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: security/sandbox/linux/SandboxFilter.cpp:383
(Diff revision 1)
>  
> +  bool BelowLevel(int aLevel) const {
> +    return mParams.mLevel < aLevel;
> +  }
> +  ResultExpr AllowBelowLevel(int aLevel, ResultExpr aOrElse) const {
> +    return BelowLevel(aLevel) ? Allow() : aOrElse;

Warning: Parameter 'aorelse' is passed by value and only copied once; consider moving it to avoid unnecessary copies [clang-tidy: performance-unnecessary-value-param]

    return BelowLevel(aLevel) ? Allow() : aOrElse;
                                          ^
                                          std::move( )

Comment 7

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8944631 [details]
Bug 1126437 - Reorganize content sandbox params extracted from libxul APIs.

https://reviewboard.mozilla.org/r/214778/#review220564

Comment 8

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8944632 [details]
Bug 1126437 - Add Linux content sandbox level 4 for blocking socket APIs.

https://reviewboard.mozilla.org/r/214780/#review220582

::: browser/app/profile/firefox.js:1099
(Diff revision 1)
>  // its Windows/Mac counterpart, but on Linux it's an integer which means:
>  // 0 -> "no sandbox"
>  // 1 -> "content sandbox using seccomp-bpf when available"
>  // 2 -> "seccomp-bpf + write file broker"
>  // 3 -> "seccomp-bpf + read/write file brokering"
> +// 4 -> 3 + network/socket restrictions

"3" -> "all the above" plus

Comment 9

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Comment on attachment 8944631 [details]
Bug 1126437 - Reorganize content sandbox params extracted from libxul APIs.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/214778/diff/1-2/

Comment 10

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Comment on attachment 8944632 [details]
Bug 1126437 - Add Linux content sandbox level 4 for blocking socket APIs.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/214780/diff/1-2/

Comment 11

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Comment on attachment 8944632 [details]
Bug 1126437 - Add Linux content sandbox level 4 for blocking socket APIs.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/214780/diff/2-3/

Comment 12

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Attachment: Bug 1126437 - Reorganize content sandbox params extracted from libxul APIs.

The end goal is to allow the seccomp-bpf policy to vary based on the
content sandbox level.

Rather than add yet another parameter to SetContentProcessSandbox to
pass down the sandbox level, this collects the values that have to be
computed in libxul into a struct, and moves the code that computes it so
it's not cluttering up ContentChild.

Review commit: https://reviewboard.mozilla.org/r/215192/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/215192/

Comment 13

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Comment on attachment 8945007 [details]
Bug 1126437 - Reorganize content sandbox params extracted from libxul APIs.

MozReview is confused.  This patch is unchanged; carrying over r+.

Comment 14

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Comment on attachment 8945007 [details]
Bug 1126437 - Reorganize content sandbox params extracted from libxul APIs.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/215192/diff/1-2/

Comment 15

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Comment on attachment 8944632 [details]
Bug 1126437 - Add Linux content sandbox level 4 for blocking socket APIs.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/214780/diff/3-4/

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb5e75c2d0c8
Reorganize content sandbox params extracted from libxul APIs. r=gcp
https://hg.mozilla.org/integration/mozilla-inbound/rev/35083f8586e7
Add Linux content sandbox level 4 for blocking socket APIs. r=gcp

Comment 17

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/bb5e75c2d0c8
https://hg.mozilla.org/mozilla-central/rev/35083f8586e7

Comment 18

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Comment on attachment 8945007 [details]
Bug 1126437 - Reorganize content sandbox params extracted from libxul APIs.

See comment #13.