1441327 - Allow for seccomp filtering of socket(AF_INET/AF_INET_6) calls on Linux when using UNIX Domain Sockets for SOCKS Proxy

Status: RESOLVED FIXED

(Core :: Networking, defect, P2)

Description

[morgan (Tor Project)]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20180208173149

Comment 1

[morgan (Tor Project)]

Attachment: 1441327_00.patch

Comment 2

[morgan (Tor Project)]

This patch is prerequisite for blocking network socket creation via seccomp policy in the sandboxed content process.  Currently, with seccomp policy in place to have socket(AF_INET,...) and friends return an error, SOCKS proxy connection fails (even when configured to use a UNIX Domain Socket) because the code first creates an AF_INET socket, and then later on replaces it with the socket type actually required.  Creation failure for this initial dummy socket is not recoverable.

The attached patch adds additional logic for Linux and macOS builds to first check to see if the provided SOCKS proxy host begins with "file://" and if so, assumes it is a path to a UNIX Domain Socket and opens an AF_LOCAL socket with the provided path, rather than the requested family.  Similar logic already exists for named pipes on Windows.

For past discussion about this issue, see tor trac ticket #22794: https://trac.torproject.org/projects/tor/ticket/22794

The attached patch passes all mochitests for linux, linux64 and macOS (apart from 1 which seems to be an infastructure or test bug which has two bugs filed with it): https://treeherder.mozilla.org/testview.html?repo=try&revision=19e83acb73284b03899c788550b34f5efe359bfe

Comment 3

[Honza Bambas (:mayhemer)]

Comment on attachment 8954195 [details] [diff] [review]
1441327_00.patch

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

not a linux expert

Comment 4

[Honza Bambas (:mayhemer)]

some general guidelines:
- 8 lines context for your patches
- keep consistent with mozilla code formatting
- add comments to your code

Comment 5

[Emil Pasca [:emilpasca]]

Moving this issue to "Core: Networking" for now, which seems the most appropriate component for this.

Comment 6

[morgan (Tor Project)]

Attachment: updated patch based on feedback and increased context to 8 lines

Comment 7

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Richard, is this new patch meant to be reviewed?  If so, please set the review flag to r? on the attachment (perhaps with [e-mail] as reviewer).

(Just didn't want this getting lost.)

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd4d107aa919
Allow for seccomp filtering of socket(AF_INET/AF_INET_6) calls on Linux when using UNIX Domain Sockets for SOCKS Proxy. r=bagder

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/fd4d107aa919

Comment 10

[Georg Koppen]

Comment on attachment 8956956 [details] [diff] [review]
updated patch based on feedback and increased context to 8 lines

Approval Request Comment
[Feature/Bug causing the regression]: none
[User impact if declined]: Tor Browser and other browsers based on Firefox 60 ESR would need to ship this patch on their own if it can't make it into the next ESR. Similar functionality is already available on Windows and this patch adds the missing bits for *NIX.
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: It's been a while on Nightly now.
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: The patch is not complicated and basically just covering the missing *NIX part by generalizing the already available Windows related code.
[String changes made/needed]: none

Comment 11

[Julien Cristau [:jcristau]]

Comment on attachment 8956956 [details] [diff] [review]
updated patch based on feedback and increased context to 8 lines

This feels like it can ride the trains.