Opened 4 years ago

Closed 7 months ago

#16106 closed defect (wontfix)

Sandbox causes crash when creating a hidden service through the control port

Reported by: micahlee Owned by:
Priority: Low Milestone: Tor: unspecified
Component: Core Tor/Tor Version: Tor: 0.2.5.12
Severity: Normal Keywords: tor-hs, client, crash, sandbox, review-group-31, 034-triage-20180328, 034-removed-20180328
Cc: weasel, segfault@… Actual Points:
Parent ID: Points:
Reviewer: dgoulet Sponsor:

Description

I'm trying to squash a bug with running OnionShare in Tails and I've narrowed it down to a bug in the Tor server in sandbox mode. Here's the related OnionShare issue: https://github.com/micahflee/onionshare/issues/179

Here's a simple script that creates a hidden service using the Tor control port, with stem and flask:

import os
from stem.control import Controller
from flask import Flask
 
def main():
    # set up flask
    app = Flask("example")
 
    @app.route('/')
    def index():
        return "<h1>Testing Tor sandbox!</h1>"
 
    # set up hidden service
    controller = Controller.from_port()
    controller.authenticate()
 
    hs_dir = '/tmp/bugtest'
    print "Creating our hidden service in %s" % hs_dir
 
    controller.set_options([
        ('HiddenServiceDir', hs_dir),
        ('HiddenServicePort', '80 127.0.0.1:5000')
    ])
 
    onion = open(hs_dir + "/hostname", "r").read().strip()
    print 'Running on {0}'.format(onion)
 
    # start web app
    app.run(port=5000)
 
if __name__ == '__main__':
    main()

(Note that you need to manually delete /tmp/bugtest before running this script a second time.)

If you set "Sandbox 0" in torrc and run this script, it works great, and the output looks like this:

user@dev:~/code/tor-sandbox-hs-bug$ sudo python tor-sandbox-hs-bug.py 
Creating our hidden service in /tmp/bugtest
Running on 3ekculjvzye6zr6s.onion
 * Running on http://127.0.0.1:5000/
127.0.0.1 - - [18/May/2015 15:37:56] "GET / HTTP/1.1" 200 -
127.0.0.1 - - [18/May/2015 15:37:59] "GET /favicon.ico HTTP/1.1" 404 -

But if you set "Sandbox 1" in torrc and run the same script again, the script throws an exception and tor crashes:

user@dev:~/code/tor-sandbox-hs-bug$ sudo python tor-sandbox-hs-bug.py 
Creating our hidden service in /tmp/bugtest
Traceback (most recent call last):
  File "tor-sandbox-hs-bug.py", line 32, in <module>
    main()
  File "tor-sandbox-hs-bug.py", line 22, in main
    ('HiddenServicePort', '80 127.0.0.1:5000')
  File "/usr/lib/python2.7/dist-packages/stem/control.py", line 1859, in set_options
    response = self.msg(query)
  File "/usr/lib/python2.7/dist-packages/stem/control.py", line 469, in msg
    raise exc
stem.SocketClosed: Received empty socket content.

Here's what ends up in the tor log:

May 18 15:38:48.000 [notice] New control connection opened from 127.0.0.1.
May 18 15:38:48.000 [notice] Tor 0.2.5.12 (git-3731dd5c3071dcba) opening log file.
May 18 15:38:48.000 [warn] sandbox_intern_string(): Bug: No interned sandbox parameter found for /tmp/bugtest
May 18 15:38:48.000 [warn] sandbox_intern_string(): Bug: No interned sandbox parameter found for /tmp/bugtest/private_key
May 18 15:38:48.000 [warn] sandbox_intern_string(): Bug: No interned sandbox parameter found for /tmp/bugtest/private_key.tmp

============================================================ T= 1431988728
(Sandbox) Caught a bad syscall attempt (syscall open)
/usr/bin/tor(+0x128176)[0x7f1391729176]
/lib/x86_64-linux-gnu/libpthread.so.0(open64+0x10)[0x7f13901fa1d0]
/lib/x86_64-linux-gnu/libpthread.so.0(open64+0x10)[0x7f13901fa1d0]
/usr/bin/tor(tor_open_cloexec+0x40)[0x7f1391715380]
/usr/bin/tor(start_writing_to_file+0xf2)[0x7f1391724182]
/usr/bin/tor(+0x1232eb)[0x7f13917242eb]
/usr/bin/tor(+0x123438)[0x7f1391724438]
/usr/bin/tor(crypto_pk_write_private_key_to_filename+0xcb)[0x7f1391731b6b]
/usr/bin/tor(init_key_from_file+0x172)[0x7f1391668302]
/usr/bin/tor(+0x5a36e)[0x7f139165b36e]
/usr/bin/tor(rend_service_load_all_keys+0x81)[0x7f139165d451]
/usr/bin/tor(set_options+0xc5f)[0x7f13916bff5f]
/usr/bin/tor(options_trial_assign+0xbb)[0x7f13916c14fb]
/usr/bin/tor(+0xdbe2e)[0x7f13916dce2e]
/usr/bin/tor(connection_control_process_inbuf+0x776)[0x7f13916e1056]
/usr/bin/tor(+0xcbd95)[0x7f13916ccd95]
/usr/bin/tor(+0x34a21)[0x7f1391635a21]
/usr/lib/x86_64-linux-gnu/libevent-2.0.so.5(event_base_loop+0x7fc)[0x7f1390c8a3dc]
/usr/bin/tor(do_main_loop+0x194)[0x7f1391637204]
/usr/bin/tor(tor_main+0x1705)[0x7f139163a035]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5)[0x7f138fc5fb45]
/usr/bin/tor(+0x3279b)[0x7f139163379b]

Child Tickets

Change History (36)

comment:1 Changed 4 years ago by nickm

Milestone: Tor: 0.2.7.x-final

comment:2 Changed 4 years ago by nickm

Hm. Probably the right fix here is that we can't allow ADDONION in this case, since we can't grant ourselves permission to open these files.

comment:3 Changed 4 years ago by dgoulet

Keywords: TorCore201508 added

comment:4 in reply to:  2 Changed 4 years ago by yawning

Replying to nickm:

Hm. Probably the right fix here is that we can't allow ADDONION in this case, since we can't grant ourselves permission to open these files.

ADD_ONION doesn't create a directory (it doesn't touch the file system at all as a matter of fact), so it should work just fine.

That said, we should warn instead of crashing if someone tries to use SETCONF to add a new HS to a running sandboxed tor instance.

comment:5 Changed 4 years ago by micahlee

So starting a hidden service with SETCONF won't work at all in sandboxed Tor, but starting one with ADD_ONION should because it doesn't touch the filesystem?

I want to make OnionShare use ADD_ONION in the future [1], but I need to wait until Tor 0.2.7.1 is in Tor Browser before releasing it.

[1] https://github.com/micahflee/onionshare/issues/178

comment:6 in reply to:  5 Changed 4 years ago by yawning

Replying to micahlee:

So starting a hidden service with SETCONF won't work at all in sandboxed Tor, but starting one with ADD_ONION should because it doesn't touch the filesystem?

Correct. Part of the design goals behind ADD_ONION is that it is entirely stateless, and you can even have Tor handle key generation, and not tell you the private key.

I want to make OnionShare use ADD_ONION in the future [1], but I need to wait until Tor 0.2.7.1 is in Tor Browser before releasing it.

[1] https://github.com/micahflee/onionshare/issues/178

Understood. Might be a while, probably when 0.2.7.x is stable.

comment:7 Changed 4 years ago by arma

Ok, so onionshare is broken with the Tor in debian now, since the debian Tor enables Sandbox 1; and onionshare can't use addonion yet because Tor Browser users don't have 0.2.7.x yet? That combination is sad.

That said, I'm confused -- can't onionshare check the Tor version and use addonion if available, else if the version is too old, and getconf sandbox says it's on, then abort rather than setconfing a hidden service that will trigger a sandbox violation?

All of this said, in parallel, maybe the Tor deb should have a canonical location that its sandbox rules allow it to write to, which the user can read, and then onionshare can just use a directory in this canonical allowed location?

comment:8 Changed 4 years ago by arma

Cc: weasel added

cc'ing weasel so he can read the last paragraph of the previous comment

comment:9 Changed 4 years ago by weasel

That which user can read?

comment:10 Changed 4 years ago by nickm

Keywords: PostFreeze027 added

I'd merge patches for these for 0.2.7 if they come in on time. In some cases, that will require figuring out an as-yet-unsolved bugs.

comment:11 Changed 4 years ago by dgoulet

Keywords: TorCore201509 added; TorCore201508 removed

comment:12 Changed 4 years ago by dgoulet

I've worked on this one and here it is. In the branch below, I've made a patch to allow tor to return an EACCES error and continue execution in case of a bad syscall (for now, just for open()). This is good, we don't exit abruptly *BUT* there is another issue here which is that options_act() also makes us exit because we are unable to set the hidden service properly. See this in set_options():

  if (options_act(old_options) < 0) { /* acting on the options failed. die. */
    log_err(LD_BUG,
            "Acting on config options left us in a broken state. Dying.");
    exit(1);
  }

This is powerfully baked in tor right now, that is if an error occured on a valid configuration option (here valid but unable to complete), tor will exit everytime. Maybe it's time to change that function for which it could return different error code and allow us to recover (especially with options coming from the control port.)

I propose we split this ticket into two. One for a way to continue execution for a bad syscall (see branch as a first PoC) and second one to allow us to recover from good configuration options that went bad because of OS restrictions for example.

Branch: bug16106_027_01.

That being said, for 0.2.7, this is getting a bit big so we should move all of this for 0.2.8.

Thoughts?

comment:13 Changed 4 years ago by nickm

Keywords: PostFreeze027 removed
Milestone: Tor: 0.2.7.x-finalTor: 0.2.8.x-final

Hm. I think that delaying to 0.2.8 is fine, assuming that really is a good fix -- tweaking the sandbox code can cast a long shadow.

If anybody thinks this is 0.2.7 critical, please let me know.

comment:14 Changed 4 years ago by dgoulet

Keywords: TorCore201509 removed
Points: small/medium

comment:15 Changed 3 years ago by nickm

Milestone: Tor: 0.2.8.x-finalTor: 0.2.9.x-final

Throw most 0.2.8 "NEW" tickets into 0.2.9. I expect that many of them will subsequently get triaged out.

comment:16 Changed 3 years ago by nickm

Priority: MediumLow

comment:17 Changed 3 years ago by isabela

Milestone: Tor: 0.2.9.x-finalTor: 0.2.???

tickets market to be removed from milestone 029

comment:18 Changed 3 years ago by segfault

Cc: segfault@… added
Severity: Blocker

Huh? I didn't touch the severity. And I don't know which value it had before.

Last edited 3 years ago by segfault (previous) (diff)

comment:19 Changed 3 years ago by nickm

Severity: BlockerNormal

comment:20 Changed 3 years ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:21 Changed 3 years ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:22 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:23 Changed 2 years ago by nickm

Keywords: tor-hs client crash sandbox added
Status: newneeds_revision

If we decide to to this, there's a much easier way: instead of using SCMP_ACT_KILL for open, use SCMP_ACT_ERRNO instead.

comment:24 Changed 22 months ago by cypherpunks

Milestone: Tor: unspecifiedTor: 0.3.3.x-final

comment:25 Changed 22 months ago by yawning

Is this still even realistically an issue, beyond the fact that it crashes when it shouldn't? Supporting creating HSes with Tor's sandbox enabled the hard way (ie: without ADD_ONION) is non-trivial for exactly the same reasons why #22605 is hard.

More to the point, ADD_ONION works fine and is available on all currently supported tor releases except for the 0.2.5.x series...

comment:26 Changed 17 months ago by nickm

Milestone: Tor: 0.3.3.x-finalTor: 0.3.4.x-final
Status: needs_revisionneeds_review

I have a tinier patch against master as bug16106_02_nm. It changes the default behavior for unpermitted open calls from "crash" to EACCES.

comment:27 Changed 17 months ago by micahlee

This issue is no longer relevant for OnionShare, because it just uses ADD_ONION in all cases except for very old versions of Tor.

comment:28 Changed 17 months ago by nickm

Keywords: review-group-31 added

comment:29 Changed 17 months ago by dgoulet

Points: small/medium
Reviewer: dgoulet
Status: needs_reviewmerge_ready

lgtm;

comment:30 Changed 17 months ago by nickm

Resolution: implemented
Status: merge_readyclosed

merged to master

comment:31 Changed 17 months ago by nickm

Resolution: implemented
Status: closedreopened

comment:32 Changed 17 months ago by nickm

Okay, that didn't work; it caused #25115. Reverted in master.

comment:33 Changed 15 months ago by nickm

Keywords: 034-triage-20180328 added

comment:34 Changed 15 months ago by nickm

Keywords: 034-removed-20180328 added

Per our triage process, these tickets are pending removal from 0.3.4.

comment:35 Changed 15 months ago by nickm

Milestone: Tor: 0.3.4.x-finalTor: unspecified

These tickets, tagged with 034-removed-*, are no longer in-scope for 0.3.4. We can reconsider any of them, if time permits.

comment:36 Changed 7 months ago by dgoulet

Resolution: wontfix
Status: reopenedclosed

We can't have the Sandbox and control port create an hidden service because of the simple fact that we can't, at runtime, tell the sandbox to add a directory path (HiddenServiceDir) so it will always fail.

IMO, either we get a nicer sandbox somehow (if Linux even has a nice interface now?) or the only realistic thing short term is document it in the manpage (#28560).

Note: See TracTickets for help on using tickets.