Opened 7 weeks ago

Last modified 12 days ago

#24037 needs_review enhancement

Use syscall blacklist rather than whitelist for torsocks

Reported by: cypherpunks Owned by: dgoulet
Priority: Medium Milestone:
Component: Core Tor/Torsocks Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Torsocks hooks network-related syscalls using LD_PRELOAD, by modifying their arguments and forcing them through the syscall() libc function. Currently, this works by disallowing syscalls by default and explicitly whitelisting "safe" syscalls. Unfortunately, this whitelisting must be done explicitly, with matching function signatures, and each time a new syscall is used, torsocks breaks. I can think of two solutions.

Solution one
On 64-bit Linux, all syscalls use exactly 6 arguments, each of type unsigned long, so there is no actual need to copy the function signature. For syscalls which do not take 6 arguments, the extra ones are ignored. As such, syscall(SYS_fork, a0, a1, a2, a3, a4, a5) is valid even though fork() does not take any arguments. This can be exploited by torsocks by only examining the syscall number, and passing all arguments to syscall(). Syscalls which are network-related can be redirected to their torsocks counterparts.

An example tsocks_syscall() replacement that is called whenever a syscall is invoked through its libc wrapper:

static unsigned long handle_syscall(int nr, va_list args)
{
    unsigned long a0, a1, a2, a3, a4, a5;

    /* get the arguments passed to this syscall (must always be 6)
    a0 = va_args(args, unsigned long);
    a1 = va_args(args, unsigned long);
    a2 = va_args(args, unsigned long);
    a3 = va_args(args, unsigned long);
    a4 = va_args(args, unsigned long);
    a5 = va_args(args, unsigned long);

    switch (nr) {
    /* hook networking syscalls */
    case SYS_socket:
        return tsocks_socket(a0, a1, a2);
    case SYS_listen:
        return tsocks_listen(a0, a1);
    case SYS_connect:
        return tsocks_connect(a0, a1, a2);
    case SYS_accept:
        return tsocks_accept(a0, a1, a2);
    /* allow all non-networking syscalls through */
    default:
        return syscall(nr, a0, a1, a2, a3, a4, a5);
    }
}

For additional safety, a check could be placed at low-level libc syscall functions where all calls converge. This would have the advantage of not needing to track new libc functions which may end up using the network, for example res_query() which was added to glibc and took a while to be supported. Tracking only the syscall numbers makes this much easier.

Solution two
An alternative, and potentially easier and more secure technique would be to use libseccomp. This library allows creating filters for mode 2 SECCOMP, a Linux kernel feature which allows whitelisting or blacklisting individual syscalls based on number or argument contents. Even if the application intentional or unintentionally calls a syscall directly (e.g. by using a currently unhooked libc function or by making the syscall in assembly), illegal syscalls still get blocked. The only time they would not be blocked is if they contain safe arguments, or go through torsocks' internal networking functions (which would use AF_UNIX as the domain type, allowing only network access through UNIX domain sockets). All other network-related syscalls would then be safe to call (to the best of my knowledge), because the only valid socket file descriptors would be ones pointing to a UNIX domain socket.

A simple example to set up a syscall filter to deny all calls to socket() which do not use AF_UNIX:

static int enable_seccomp(void)
{
    int rc;
    scmp_filter_ctx ctx;

    /* allow all syscalls by default */
    ctx = seccomp_init(SCMP_ACT_ALLOW);
    if (ctx == NULL)
        return -1;

    /* blacklist socket() if the first argument is not AF_UNIX */
    rc = seccomp_rule_add(ctx, SCMP_ACT_TRAP, SCMP_SYS(socket), 1,
        SCMP_A0(SCMP_CMP_NE, AF_UNIX));
    if (rc < 0)
        goto out;

    /* load the filter and enforce it */
    rc = seccomp_load(ctx);
    if (rc < 0)
        goto out;

out:
    seccomp_release(ctx);
    return -rc;
}

With SCMP_ACT_TRAP, a violation will raise SIGSYS. Torsocks can trap the signal and output a diagnostic message, aiding the debugging of applications which may be unsupported.

Child Tickets

Attachments (1)

0001-use-blacklist-rather-than-whitelist-in-syscall-hijac.patch (15.9 KB) - added by ertosns 4 weeks ago.
implement the first solution

Download all attachments as: .zip

Change History (5)

Changed 4 weeks ago by ertosns

implement the first solution

comment:1 Changed 4 weeks ago by ertosns

Status: newneeds_review

that part probably needs review

  • On an 64 bit *BSD system, syscall(2) should be used for mmap().
  • This is NOT suppose to happen but for protection we deny that call.

comment:2 Changed 13 days ago by cypherpunks

implement the first solution

Nice! 33 insertions, 576 deletions. Great to see an alternative that gets rid of so much bad code. I'm working on the second solution myself (at least once #24400 is resolved so SocksPort doesn't break on UNIX domain sockets with the sandbox), but I think the two solutions can go hand in hand, as the second solution, while more secure, is Linux-specific.

that part probably needs review

I don't think this will be an issue once torsocks no longer uses a whitelist. It seems to be more related to #24116, where the mmap() wrapper requires using mmap() to initialize memory, but can't be initialized until the syscall runs, resulting in a deadlock. There is no danger security-wise to allowing that syscall.

comment:3 Changed 12 days ago by cypherpunks

The seccomp solution is currently blocked by #14132, which is needed if we whitelist AF_UNIX for socket().

comment:4 Changed 12 days ago by cypherpunks

A few comments about the patch:

unsigned long a0, a1, a2, a3, a4, a5;

While this type is certainly correct for 64-bit Linux on x86 systems, it might be better to use something like ptrdiff_t, and maybe even use a macro to choose the number of arguments dynamically (since ptrdiff_t a[NUM] would work perfectly as well), since Linux's isn't the only syscall ABI out there. I can't help but have the nagging feeling that there may be a way to avoid hardcoding the number of arguments at all, and instead use a single va_arg for everything.

tsocks_libc_syscall(number, a0, a1, a2, a3, a4, a5);

This should probably be using a return, otherwise the ret could be used uninitialized.

ret = tsocks_socket(a0, a1, a2);
break;

You could also replacing this (and others like it) with just a return tsocks_socket(a0, a1, a2);, to get rid of that temporary ret entirely and make the code a bit more clean.

Note: See TracTickets for help on using tickets.