Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#6274 closed defect (fixed)

config parsing does not abort if ServerTransportPlugin but no ORPort

Reported by: asn Owned by:
Priority: Medium Milestone: Tor: 0.2.3.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: pt tor-client
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Jul 03 00:42:52.632 [notice] Tor v0.2.3.12-alpha-dev (git-e1ac458802b3cd18) running on Linux x86_64.
Jul 03 00:42:52.632 [notice] Tor can't help you if you use it wrong! Learn how to be safe at https://www.torproject.org/download/download#warning
Jul 03 00:42:52.632 [notice] This version is not a stable Tor release. Expect more bugs than usual.
Jul 03 00:42:52.638 [notice] Initialized libevent version 2.0.16-stable using method epoll (with changelist). Good.
Jul 03 00:42:52.638 [notice] Opening Socks listener on 127.0.0.1:0
Jul 03 00:42:52.638 [notice] Socks listener listening on port 56396.
Jul 03 00:42:52.000 [warn] You are running Tor as root. You don't need to, and you probably shouldn't.
Jul 03 00:42:52.000 [notice] No AES engine found; using AES_* functions.
Jul 03 00:42:52.000 [notice] This OpenSSL has a good implementation of counter mode; using it.
Jul 03 00:42:52.000 [notice] OpenSSL OpenSSL 1.0.1 14 Mar 2012 looks like version 0.9.8m or later; I will try SSL_OP to enable renegotiation
Jul 03 00:42:52.000 [notice] Reloaded microdescriptor cache.  Found 2869 descriptors.
Jul 03 00:42:52.000 [notice] I learned some more directory information, but not enough to build a circuit: We have no usable consensus.

Program received signal SIGSEGV, Segmentation fault.
0x0000000000419510 in create_managed_proxy_environment (mp=0x7599e0) at transports.c:1191
1191        smartlist_add_asprintf(envs, "TOR_PT_ORPORT=127.0.0.1:%s",
(gdb) bt full
#0  0x0000000000419510 in create_managed_proxy_environment (mp=0x7599e0) at transports.c:1191
        options = 0x757850
        envs = 0x75d2b0
        merged_env_vars = 0x8db6d0
        env = <optimized out>
#1  launch_managed_proxy (mp=0x7599e0) at transports.c:500
        retval = <optimized out>
        env = 0x7fffffffe330
#2  configure_proxy (mp=0x7599e0) at transports.c:651
        r = <optimized out>
        stdout_buf = "\f\000\000\000\000\000\000\000\000\274u\000\000\000\000\000\000\235u", '\000' <repeats 13 times>"\250, \346R", '\000' <repeats 14 times>"\235, u\000\000\000\000\000\000\274u\000\000\000\000\000\000\274u", '\000' <repeats 14 times>"\274, u\000$
#3  pt_configure_remaining_proxies () at transports.c:571
        mp_sl_idx = <optimized out>
        mp_sl_len = <optimized out>
        mp = 0x7599e0
        tmp = 0x78c4c0
        __PRETTY_FUNCTION__ = "pt_configure_remaining_proxies"

Child Tickets

TicketStatusOwnerSummaryComponent
#6455closed#6274 introduced minor memleakCore Tor/Tor

Change History (9)

comment:1 Changed 6 years ago by asn

Status: newneeds_review
Summary: segfault if ServerTransportPlugin without an ORPortconfig parsing does not abort if ServerTransportPlugin but no ORPort

Hm, seems like it no longer segfaults since #4865.

Still, it did not consider (ServerTransportPlugin && !ORPort) an invalid configuration, and it was giving bad data to the managed proxy:

[debug] Proxy environment (cont):
extended_port: ''
or_port: '(null)'
bindaddrs: 'obfs2-0.0.0.0:33290'

Please see branch bug6274 in https://git.gitorious.org/mytor/mytor.git.

comment:2 Changed 6 years ago by nickm

Status: needs_reviewneeds_revision

Not sure I agree. It's okay to specify lots of other server-only stuff when ORPort is disabled. Rather than requiring that you turn off ServerTransportPlugin before we undefine ORPort, I think it would make more sense to just ignore ServerTransportPlugin when ORPort is disabled.

Also, this ticket is for 0.2.3.x, but the branch is on master, it appears.

comment:3 in reply to:  2 Changed 6 years ago by asn

Status: needs_revisionneeds_review

Replying to nickm:

Not sure I agree. It's okay to specify lots of other server-only stuff when ORPort is disabled. Rather than requiring that you turn off ServerTransportPlugin before we undefine ORPort, I think it would make more sense to just ignore ServerTransportPlugin when ORPort is disabled.

Makes sense.

Also, this ticket is for 0.2.3.x, but the branch is on master, it appears.

Please see branch bug6274_take2 in https://git.gitorious.org/mytor/mytor.git. I hope it's correctly rebased to maint-0.2.3.

comment:4 Changed 6 years ago by nickm

Looks mostly good. What happens in the case where the user has some ServerTransportPlugin options set, and then they turn ORPort off? We probably want to disable the ServerTransportPlugins in that case too, right?

Should we be looking at server_mode(options) rather than options->ORPort?

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

Status: needs_reviewneeds_revision

Replying to nickm:

Looks mostly good. What happens in the case where the user has some ServerTransportPlugin options set, and then they turn ORPort off? We probably want to disable the ServerTransportPlugins in that case too, right?

I suspect we are getting this for free with the pt_prepare_proxy_list_for_config_read() -> sweep_proxy_list() SIGHUP code.

Since we are now only parsing ServerTransportPlugin lines if (ServerTransportPlugin && server_mode()), without an ORPort the proxies will still be marked for removal after the parsing loop, and the proxies/transports will be terminated/disabled.

Should we be looking at server_mode(options) rather than options->ORPort?

Yes, I think so.

comment:6 Changed 6 years ago by asn

Status: needs_revisionneeds_review

Please see branch bug6274_take3 in https://git.gitorious.org/mytor/mytor.git.

I also documented the fact that we are terminating already active ServerTransportPlugins when tor stops being a relay.

Thanks for the code review.

comment:7 Changed 6 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Looks ok; merged it. Thanks!

comment:8 Changed 6 years ago by nickm

Keywords: tor-client added

comment:9 Changed 6 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.