Don't open AF_INET/AF_INET6 sockets when AF_LOCAL is configured.

[yawning]

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.

[yawning]

A minimal/self contained LD_PRELOAD that can reproduce the behavior:

#include <sys/types.h>
#include <sys/socket.h>
#include <errno.h>
#include <stdio.h>

#define _GNU_SOURCE
#include <unistd.h>
#include <syscall.h>

int socket(int domain, int type, int protocol) {
  fprintf(stderr, "stub: socket(%d, 0x%08x, %d)\n", domain, type, protocol);
  if (domain != AF_LOCAL) {
    fprintf(stderr, "stub: domain is not AF_LOCAL, rejecting\n");
    errno = EAFNOSUPPORT;
    return -1;
  }
  return syscall(SYS_socket, domain, type, protocol);
}

And commenting out the rejection (as in always calling syscall(), regardless of the domain), magically makes things start to work.

[yawning]

Setting a parent so it's obvious why the sandbox doesn't use Tor Browser's IPC support.

[yawning]

It also helps if I file this against the correct component.

[gk]

Hm, I wonder what is going on here. Putting it on our radar.

[gk]

Moving our Tickets to August.

[gk]

Items for September 2017.

[gk]

Items for October 2017

[gk]

Moving tickets over to November.

[gk]

Moving tickets to December 2017

[gk]

Moving tickets to December 2017, for realz.

[gk]

Moving tickets to 2018.

[yawning]

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:

​https://github.com/mozilla/gecko-dev/blob/1d9f00cb8d2088fccb7a611a34be655b9fa9fec1/netwerk/socket/nsSOCKSIOLayer.cpp#L601

[yawning]

seccomp based test case.

[yawning]

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:

1. Compile it.
2. Set the required env vars to get Tor Browser to talk to a external tor process over AF_UNIX sockets.
3. 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).
4. Watch in horror as nothing works.
5. 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.

[yawning]

Really though, just look at nsSOCKSIOLayer.

1. 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).
2. When firefox actually goes to connect to the proxy (ConnectToProxy), the FixupAddressFamily routine is called.
3. FixupAddressFamily checks to see if the proxy actually is reachable via an AF_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:

1. Back in the day, this was only expected to handle AF_INET, because "this IPng thing will never happen".
2. When AF_INET6 support was required, it was kludged on this way.
3. 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.

[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.
• 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.

[teor]

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

​https://developer.apple.com/library/content/documentation/Miscellaneous/Reference/EntitlementKeyReference/Chapters/EnablingAppSandbox.html#//apple_ref/doc/uid/TP40011195-CH4-SW9

[yawning]

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.

[mcs]

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).

[pospeselr]

updated patch to also be enabled on macOS rather than just Linux

[pospeselr]

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.

[mcs]

r=mcs
The latest patch looks good to me, and it compiles and seems to work on macOS.

[gk]

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.

[pospeselr]

Bug for uplift has been created here: ​https://bugzilla.mozilla.org/show_bug.cgi?id=1441327