Opened 5 months ago

Last modified 4 weeks ago

#31091 merge_ready defect

Bug stracktrace when pluggable transport cannot bind to port

Reported by: s7r Owned by: ahf
Priority: Very High Milestone: Tor: 0.4.1.x-final
Component: Core Tor/Tor Version: Tor: unspecified
Severity: Normal Keywords: 042-must, consider-backport-after-0424
Cc: Actual Points:
Parent ID: Points:
Reviewer: nickm Sponsor:

Description

I have just setup some new obfs4 fast bridges running latest obfs4proxy from yawning / master (locally compiled on my server) and Tor 0.4.2.0-alpha-dev and encountered a bug stack trace when started to try with the pluggable transport listening on a port < 1024. This is well known, which is why even in the wiki page it is recommended and properly documented to use setcap in order to grant permission to the PT executable to bind to lower ports, but shouldn't it just warn and exit, instead of offering all this:

Jul 06 05:51:18.000 [warn] Server managed proxy encountered a method error. (obfs4 listen tcp <ipv4>:<port>: bind: permission denied)
Jul 06 05:51:18.000 [warn] Managed proxy at '/usr/local/bin/obfs4proxy' failed the configuration protocol and will be destroyed.
Jul 06 05:51:18.000 [err] tor_assertion_failed_(): Bug: ../src/feature/client/transports.c:1836: managed_proxy_stdout_callback: Assertion mp->conf_state == PT_PROTO_COMPLETED failed; aborting. (on Tor 0.4.2.0-alpha-dev )
Jul 06 05:51:18.000 [err] Bug: Assertion mp->conf_state == PT_PROTO_COMPLETED failed in managed_proxy_stdout_callback at ../src/feature/client/transports.c:1836: . Stack trace: (on Tor 0.4.2.0-alpha-dev )
Jul 06 05:51:18.000 [err] Bug:     /usr/bin/tor(log_backtrace_impl+0x47) [0x559a576f5d87] (on Tor 0.4.2.0-alpha-dev )
Jul 06 05:51:18.000 [err] Bug:     /usr/bin/tor(tor_assertion_failed_+0x147) [0x559a576f0ed7] (on Tor 0.4.2.0-alpha-dev )
Jul 06 05:51:18.000 [err] Bug:     /usr/bin/tor(+0xd4a89) [0x559a575b8a89] (on Tor 0.4.2.0-alpha-dev )
Jul 06 05:51:18.000 [err] Bug:     /usr/bin/tor(+0x1e4e4b) [0x559a576c8e4b] (on Tor 0.4.2.0-alpha-dev )
Jul 06 05:51:18.000 [err] Bug:     /usr/lib/x86_64-linux-gnu/libevent-2.0.so.5(event_base_loop+0x6a0) [0x7f6f343265a0] (on Tor 0.4.2.0-alpha-dev )
Jul 06 05:51:18.000 [err] Bug:     /usr/bin/tor(do_main_loop+0x105) [0x559a57555ed5] (on Tor 0.4.2.0-alpha-dev )
Jul 06 05:51:18.000 [err] Bug:     /usr/bin/tor(tor_run_main+0x1245) [0x559a57543905] (on Tor 0.4.2.0-alpha-dev )
Jul 06 05:51:18.000 [err] Bug:     /usr/bin/tor(tor_main+0x3a) [0x559a57540cfa] (on Tor 0.4.2.0-alpha-dev )
Jul 06 05:51:18.000 [err] Bug:     /usr/bin/tor(main+0x19) [0x559a57540879] (on Tor 0.4.2.0-alpha-dev )
Jul 06 05:51:18.000 [err] Bug:     /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf1) [0x7f6f32b7b2e1] (on Tor 0.4.2.0-alpha-dev )
Jul 06 05:51:18.000 [err] Bug:     /usr/bin/tor(_start+0x2a) [0x559a575408ca] (on Tor 0.4.2.0-alpha-dev )

What if it just stopped here:

Jul 06 05:51:18.000 [warn] Server managed proxy encountered a method error. (obfs4 listen tcp <ip>:<port>: bind: permission denied)
Jul 06 05:51:18.000 [warn] Managed proxy at '/usr/local/bin/obfs4proxy' failed the configuration protocol and will be destroyed.

Or maybe even exit entirely so the operator can know something is really wrong.

Also, I don't know if this is related or not, I will try to make more tests to confirm or infirm this, but under Debian stable the bridges that are not run with setcap don't get a reasonable measured speed. Those with setcap that listen to lower ports have a bw of 2.5 - 3.5 MiB/s and those without setcap have a bw of 12 KiB/s - 70 KiB/s and they are all on the same infrastructure / hardware resources / internet speed. I will do some more digging to confirm this, right now 2 out of 2 obfs4 bridges that were configured without setcap did not get good bandwidth measurement even after 15 days of continuous uptime.

Child Tickets

Change History (11)

comment:1 Changed 4 months ago by nickm

Milestone: Tor: 0.4.2.x-final

comment:2 Changed 2 months ago by nickm

Keywords: 042-must added

comment:3 Changed 2 months ago by nickm

Priority: MediumVery High

Make all 042-must objects "Very High" priority.

comment:4 Changed 2 months ago by ahf

Owner: set to ahf
Status: newassigned

comment:5 Changed 8 weeks ago by ahf

Created PR here: https://github.com/torproject/tor/pull/1365 -- waiting to see what Travis and Appveyor says.

comment:6 Changed 8 weeks ago by ahf

Status: assignedneeds_review

Looks like CI is happy.

comment:7 Changed 8 weeks ago by nickm

Reviewer: nickm
Status: needs_reviewneeds_revision

Looks good. A few questions:

1) Is there any way to test these fixes?

2) Should this branch be based on maint-0.4.0 for possible backports?

3) Now that we no longer crash or log stack traces in these cases ... do we at least log something? If not, we should.

comment:8 Changed 5 weeks ago by ahf

1) These assertions and BUG() statements were added by me to figure out if these cases could ever trigger when we rewrote the process library for s8. I can't think of any easy way to test these without quite some refactoring of the transports.c file (which would be lovely to do though)

2) I have made the new PR against 0.4.0.

3) These cases are legitimate and can happen, so I don't think we should log anything particularly here. The important part being the exit handler is always called.

I have created a new PR against maint-0.4.0 which can be seen here: https://github.com/torproject/tor/pull/1396

comment:9 Changed 5 weeks ago by ahf

Status: needs_revisionneeds_review

Requesting review.

comment:10 Changed 5 weeks ago by nickm

Milestone: Tor: 0.4.2.x-finalTor: 0.4.1.x-final
Status: needs_reviewmerge_ready

LGTM. Merged to 0.4.2 and forward. Marked for backport.

comment:11 Changed 4 weeks ago by teor

Keywords: consider-backport-after-0424 added
Note: See TracTickets for help on using tickets.