#22794 closed defect (fixed)
Don't open AF_INET/AF_INET6 sockets when AF_LOCAL is configured.
Reported by: | yawning | Owned by: | pospeselr |
---|---|---|---|
Priority: | Medium | Milestone: | |
Component: | Applications/Tor Browser | Version: | |
Severity: | Normal | Keywords: | tbb-security, tbb-sandboxing, TorBrowserTeam201802R |
Cc: | intrigeri, arthuredelstein, mcs | Actual Points: | |
Parent ID: | #20775 | Points: | |
Reviewer: | Sponsor: | Sponsor4 |
Description
Discovered when trying to resolve #20775.
Unsandboxed Tor Browser 7.0.1:
socket(AF_INET, SOCK_STREAM, IPPROTO_IP) = 67 fcntl(67, F_GETFL) = 0x2 (flags O_RDWR) fcntl(67, F_SETFL, O_RDWR|O_NONBLOCK) = 0 socket(AF_INET6, SOCK_STREAM, IPPROTO_IP) = 68 close(68) = 0 socket(AF_INET6, SOCK_STREAM, IPPROTO_IP) = 68 fcntl(68, F_GETFL) = 0x2 (flags O_RDWR) fcntl(68, F_SETFL, O_RDWR|O_NONBLOCK) = 0 close(68) = 0 setsockopt(67, SOL_TCP, TCP_NODELAY, [1], 4) = 0 socket(AF_UNIX, SOCK_STREAM, 0) = 68 fcntl(68, F_GETFL) = 0x2 (flags O_RDWR) fcntl(68, F_SETFL, O_RDWR|O_NONBLOCK) = 0 close(67) = 0 connect(68, {sa_family=AF_UNIX, sun_path="/var/run/tor/socks"}, 106) = 0
If the first socket
(AF_INET
) call fails (as it will due to seccomp-bpf) the AF_LOCAL socket never gets created, and pages don't load. The failure mode doesn't appear to depend on errno
(at least, it didn't make a difference if it was ENOSYS
or EAFNOSUPPORT
).
Using IPC should mean, "Tor Browser uses IPC, and only IPC", and not "Tor Browser refuses to work if non-IPC socket creation fails", because the whole point of using IPC in the first place is so that Tor Browser can be ran in a way that disallows non-IPC connections.
Child Tickets
Attachments (2)
Change History (30)
comment:1 Changed 2 years ago by
comment:2 Changed 2 years ago by
Parent ID: | → #20775 |
---|
Setting a parent so it's obvious why the sandbox doesn't use Tor Browser's IPC support.
comment:3 Changed 2 years ago by
Component: | Applications/Tor Browser Sandbox → Applications/Tor Browser |
---|---|
Owner: | changed from yawning to tbb-team |
It also helps if I file this against the correct component.
comment:4 Changed 2 years ago by
Cc: | intrigeri added |
---|
comment:5 Changed 2 years ago by
Keywords: | TorBrowserTeam201707 added |
---|
Hm, I wonder what is going on here. Putting it on our radar.
comment:6 Changed 2 years ago by
Cc: | arthuredelstein added |
---|
comment:7 Changed 2 years ago by
Keywords: | TorBrowserTeam201708 added; TorBrowserTeam201707 removed |
---|
Moving our Tickets to August.
comment:8 Changed 2 years ago by
Keywords: | TorBrowserTeam201709 added; TorBrowserTeam201708 removed |
---|
Items for September 2017.
comment:9 Changed 2 years ago by
Keywords: | TorBrowserTeam201710 added; TorBrowserTeam201709 removed |
---|
Items for October 2017
comment:10 Changed 2 years ago by
Keywords: | TorBrowserTeam201711 added; TorBrowserTeam201710 removed |
---|
Moving tickets over to November.
comment:12 Changed 2 years ago by
Keywords: | TorBrowserTeam201712 added; TorBrowserTeam201711 removed |
---|
Moving tickets to December 2017, for realz.
comment:13 Changed 2 years ago by
Keywords: | TorBrowserTeam201801 added; TorBrowserTeam201712 removed |
---|
Moving tickets to 2018.
comment:14 Changed 2 years ago by
Owner: | changed from tbb-team to pospeselr |
---|---|
Status: | new → assigned |
comment:15 Changed 2 years ago by
Cc: | mcs added |
---|
comment:16 Changed 2 years ago by
I looked at this a bit more writing an e-mail explaining the issue to pospeselr, and I strongly suspect that at least some of the behavior (the strace output) can be explained by this:
comment:17 Changed 23 months ago by
Apparently someone really wanted to be able to see this with seccomp, so I attached a trivial program that uses libseccomp to initialize a filter that will reject socket(AF_INET[6], ..., ...)
by SIGSYSing the thread.
Usage:
- Compile it.
- Set the required env vars to get Tor Browser to talk to a external tor process over AF_UNIX sockets.
- Run it from the root of a tor browser installation (It calls
system("Browser/start-tor-browser");
because it's a trivial test case and I'm lazy). - Watch in horror as nothing works.
- Run it with strace to see the socket thread traces ending abruptly with
+++ killed by SIGSYS +++
.
The control port appears to work, but any attempt to use the socks port get stomped on, so I'm basically convinced that the root cause is the non-AF_INET
socket support in nsSOCKSIOLayer
being a steaming pile of shit.
comment:18 Changed 23 months ago by
Really though, just look at nsSOCKSIOLayer
.
- All outgoing SOCKS connections made by firefox start out their lives as
AF_INET
sockets. (Assumption, firefox code gives me a headache, but it matches the trace output). - When firefox actually goes to connect to the proxy (
ConnectToProxy
), theFixupAddressFamily
routine is called. FixupAddressFamily
checks to see if the proxy actually is reachable via anAF_INET
socket, and if not, opens a new file descriptor with the correct domain.
What appears to have happened judging for a cursory inspection of the file history was:
- Back in the day, this was only expected to handle
AF_INET
, because "this IPng thing will never happen". - When
AF_INET6
support was required, it was kludged on this way. - When
AF_UNIX
(and Windows pipes or whatever that's also in the code) support was required, the kludge was enhanced.
Which is great if the only reason you want something like AF_UNIX
is to use AF_UNIX
socket for the hell of it, and not so great if you want use something like seccomp to prohibit AF_INET
.
comment:19 Changed 23 months ago by
Keywords: | TorBrowserTeam201802R added; TorBrowserTeam201801 removed |
---|---|
Status: | assigned → needs_review |
comment:20 follow-up: 23 Changed 23 months ago by
Oh hey, I was right.
Regarding the patch:
- I'm not sure if checking that the URI scheme is file is the correct long term fix (eg: #20337), but that bridge can be burnt when someone gets there.
- Should this apply to OSX as well? I do not know how OSX's process sandboxing stuff works, or what options it has for limiting what a process can do.
comment:21 Changed 23 months ago by
macOS legacy sandbox support has:
#include <sandbox.h> ... kSBXProfileNoInternet TCP/IP networking is prohibited. DEPRECATED. kSBXProfileNoNetwork All sockets-based networking is pro- hibited. DEPRECATED.
sandbox-exec is also legacy, but it works from the command line:
https://paolozaino.wordpress.com/2015/10/20/maximum-security-and-privacy-using-mac-os-sandbox-and-tor-browser-bundle/
Unfortunately, the replacement API doesn't seem to distinguish between TCP/IP and unix sockets. We'd need to do some testing.
com.apple.security.network.client Network socket for connecting to other machines com.apple.security.network.server Network socket for listening for incoming connections initiated by other machines
comment:22 Changed 23 months ago by
For what it's worth, I'm of the opinion that the AF_INET6
socket being created is probably fine, as long as the browser isn't rendered non-functional if AF_INET6
isn't supported.
This can be revisited if/when people want to write seccomp rules that will kill (rather than gently reject) proscribed calls, but I don't think that's realistic at the moment.
comment:23 Changed 23 months ago by
Replying to yawning:
Oh hey, I was right.
Regarding the patch:
- I'm not sure if checking that the URI scheme is file is the correct long term fix (eg: #20337), but that bridge can be burnt when someone gets there.
Agreed. Richard, if we keep the "file://"
prefix test, please add a comment to #20337 so we don't forget about this loose end.
- Should this apply to OSX as well? I do not know how OSX's process sandboxing stuff works, or what options it has for limiting what a process can do.
I think we might as well apply the patch to OSX as well since it may be useful there. The not-really-awesome OSX sandboxing prototype that Kathy and I created a while ago does use the older mechanism of the ones that teor discussed (sandbox-exec).
Changed 23 months ago by
Attachment: | 0001-Bug-22794-Don-t-open-AF_INET-AF_INET6-sockets-when-A.patch added |
---|
updated patch to also be enabled on macOS rather than just Linux
comment:24 Changed 23 months ago by
Yeah so long as the seccomp policies for AF_INET6 socket creation doesn't kill the calling thread, the existing Firefox code gracefully handles socket creation failure.
In the event that we want to enable such a strict policy for the sandbox the relevant code in nsSOCKSIOLayer.cpp's nsSOCKSIOLayerAddToSocket() can probably be safely refactored to ignore the IPv6 detection if the desired socket family is AF_LOCAL.
comment:25 Changed 23 months ago by
r=mcs
The latest patch looks good to me, and it compiles and seems to work on macOS.
comment:26 Changed 22 months ago by
Resolution: | → fixed |
---|---|
Status: | needs_review → closed |
Looks good to me and applied to tor-browser-52.6.0esr-8.0-2
(commit eafaa94a613a326bd13234540fe88b86451ee3e9).
Leaving the remaining things to Richard:
1) Comment on #20337
2) Open a bug in Mozilla's bug tracker and trying to get this patch upstreamed for ESR60.
comment:27 Changed 22 months ago by
Sponsor: | → Sponsor4 |
---|
comment:28 Changed 22 months ago by
Bug for uplift has been created here: https://bugzilla.mozilla.org/show_bug.cgi?id=1441327
A minimal/self contained LD_PRELOAD that can reproduce the behavior:
And commenting out the rejection (as in always calling
syscall()
, regardless of the domain), magically makes things start to work.