User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:58.0) Gecko/20100101 Firefox/58.0 Build ID: 20180208173149
Priority: -- → P1
Whiteboard: [tor][tor 22794]
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 on attachment 8954195 [details] [diff] [review] 1441327_00.patch Review of attachment 8954195 [details] [diff] [review]: ----------------------------------------------------------------- not a linux expert
some general guidelines: - 8 lines context for your patches - keep consistent with mozilla code formatting - add comments to your code
Moving this issue to "Core: Networking" for now, which seems the most appropriate component for this.
Component: Untriaged → Networking
Product: Firefox → Core
Assignee: nobody → richard
Whiteboard: [tor][tor 22794] → [tor][tor 22794][necko-triaged]
Created attachment 8956956 [details] [diff] [review] updated patch based on feedback and increased context to 8 lines
Richard, is this new patch meant to be reviewed? If so, please set the review flag to r? on the attachment (perhaps with firstname.lastname@example.org as reviewer). (Just didn't want this getting lost.)
Attachment #8956956 - Flags: review?(daniel)
Attachment #8956956 - Flags: review?(daniel) → review+
Pushed by email@example.com: 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
Status: UNCONFIRMED → RESOLVED
Last Resolved: 10 months ago
status-firefox61: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
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
Attachment #8956956 - Flags: approval-mozilla-beta?
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.
Attachment #8956956 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
You need to log in before you can comment on or make changes to this bug.