Opened 3 months ago

Closed 2 months ago

#22976 closed defect (implemented)

disallow tor exec'ing

Reported by: dawuud Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.2.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: sandbox, review-group-22
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Hello from PETS2017. I recently chatted with Nick Mathewson who suggested that it would be very easy to patch tor such that exec'ing programs could optionally be disallowed. Currently there are three torrc config options that can cause tor to exec:

  1. PortForwardingHelper
  2. ClientTransportPlugin
  3. ServerTransportPlugin

Of course these can be used via the control port which is precisely why it was important to the Subgraph OS project to have a decent Tor control port filter; we were mainly concerned with preventing sandbox escapes. I wrote Roflcoptor for this purpose:

https://github.com/subgraph/roflcoptor

A few other projects have also written their own Tor control port filter daemons. I will not list them here. Even with this feature addition to tor, these Tor control port filter daemons will still be useful for limiting the authority delegated by access to the tor control port.

Child Tickets

Change History (13)

comment:1 Changed 3 months ago by dgoulet

Keywords: sandbox added
Milestone: Tor: 0.3.2.x-final

Do we really have a way to remove a syscall from the sandbox filters at runtime?

That is, tor realizes at runtime that ClientTransportPlugin is in used for instance so we would need a way to tell the sandbox "Oh, this is set, ok don't filter execv()".

comment:2 Changed 3 months ago by dawuud

No that is really not the intent of the ticket at all; in Subgraph OS tor is running outside the sandbox. This ticket/feature specified is not concerned with syscall filtering but merely "telling tor to not ever exec via a config option".

Last edited 3 months ago by dawuud (previous) (diff)

comment:3 in reply to:  2 Changed 3 months ago by dgoulet

Replying to dawuud:

No that is really not the intent of the ticket at all; in Subgraph OS tor is running outside the sandbox. This ticket/feature specified is not concerned with syscall filtering but merely "telling tor to not ever exec via a config option".

Ah! So a torrc option that would be "do not exec never ever" so if any option that does that is enabled through control port, it's denied I guess.

comment:4 Changed 3 months ago by nickm

We could allow a transition in one direction (allow->disallow) but not the other.

comment:5 in reply to:  1 Changed 3 months ago by yawning

Replying to dgoulet:

Do we really have a way to remove a syscall from the sandbox filters at runtime?

That would be trivial to add because the bpf is runtime generated with libseccomp.

comment:6 Changed 2 months ago by nickm

Implementation in my feature22976 branch.

This isn't fundamentally about seccomp; it's about disabling functionality. Our sandbox already disables all exec calls.

comment:7 Changed 2 months ago by nickm

Owner: set to nickm
Status: newaccepted

comment:8 Changed 2 months ago by nickm

Status: acceptedneeds_review

comment:9 Changed 2 months ago by nickm

Keywords: review-group-22 added

comment:10 Changed 2 months ago by dgoulet

Status: needs_reviewmerge_ready

This isn't fundamentally about seccomp; it's about disabling functionality. Our sandbox already disables all exec calls.

Not sure it does... All the exeve() calls are disabled by #if 0.

The changes file has this weird sentence:

    - Added a new NoExec option to . When this option is set to 1,

Apart from that, lgtm;

comment:11 in reply to:  10 Changed 2 months ago by nickm

Replying to dgoulet:

This isn't fundamentally about seccomp; it's about disabling functionality. Our sandbox already disables all exec calls.

Not sure it does... All the exeve() calls are disabled by #if 0.

Ah, but that code is for _enabling_ execve. Unless a syscall is specifically enabled, the sandbox code doesn't allow it.

The changes file has this weird sentence:

    - Added a new NoExec option to . When this option is set to 1,

Oops. Will fix.

Apart from that, lgtm;

comment:12 Changed 2 months ago by nickm

fixed, squashed, merged!

comment:13 Changed 2 months ago by nickm

Resolution: implemented
Status: merge_readyclosed
Note: See TracTickets for help on using tickets.