Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#4725 closed defect (fixed)

Managed proxies should not bind locally by default

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

Description

The default addrport in get_bindaddr_for_transport() is 127.0.0.1:0 which will instruct the transport to bind locally and not be accessible by the rest of the world.

Changing the default to 0.0.0.0:0 should do the trick.

Child Tickets

Change History (14)

comment:1 Changed 8 years ago by asn

Status: newneeds_review

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

comment:2 Changed 8 years ago by nickm

That seems wrong to me. We would want client proxies to listen on 127.0.0.1, and server proxies to listen on INADDR_ANY, right?

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

Replying to nickm:

That seems wrong to me. We would want client proxies to listen on 127.0.0.1, and server proxies to listen on INADDR_ANY, right?

We don't set TOR_PT_SERVER_BINDADDR for client proxies, and they should, by default, listen on 127.0.0.1. We only set TOR_PT_SERVER_BINDADDR for server proxies [0].

Please have a look at branch bug4725_take2 where I use the INADDR_ANY definition, which I did not know of, instead of hardcoding "0.0.0.0".

[0]:
set_managed_proxy_environment():

  if (mp->is_server) {
    bindaddr = get_bindaddr_for_proxy(mp);
...
    tor_asprintf(tmp++, "TOR_PT_SERVER_BINDADDR=%s", bindaddr);
...
  }
...

comment:4 Changed 8 years ago by asn

Priority: normalmajor

I suspect that this bug blocks managed proxy servers from being accessed from the rest of the Internet. Raising 'Priority' to 'major'.

comment:5 Changed 8 years ago by nickm

Ah, my bad on the confusion. It was the function name that threw me: "get_bindaddr_for_proxy" says "for_proxy", which to me meant "for a proxy", not "for a server-side proxy". We should probably have a ticket for editing the documentation and function names so that server-only and client-only functions are clearly distinguished.

comment:6 Changed 8 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

ok, merged the take2 version. 0.0.0.0 would have been fine, though: INADDR_ANY is just another name for it.

comment:7 Changed 8 years ago by arma

Resolution: fixed
Status: closedreopened

comment:8 Changed 8 years ago by arma

Can somebody summarize what would have gone into the 'changes' file for this, if we had written one? It's not too late to go back in time and say what it was in the changelog.

comment:9 Changed 8 years ago by arma

George says 539cb627f71 is the commit in question

comment:10 in reply to:  8 Changed 8 years ago by asn

Replying to arma:

Can somebody summarize what would have gone into the 'changes' file for this, if we had written one? It's not too late to go back in time and say what it was in the changelog.

How about:

  o Major bugfixes:
    - Fix a bug where server managed proxies were unreachable from the
      Internet, because tor asked them to bind on localhost. Fixes bug
      4725; bugfix on 0.2.3.9-alpha.

(Feel free to tone down the 'Major' severity.)

I also pushed it in branch bug4725_take2.

comment:11 Changed 8 years ago by asn

Status: reopenedneeds_review

comment:12 Changed 8 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

merged bug4725_take2

comment:13 Changed 8 years ago by nickm

Keywords: tor-bridge added

comment:14 Changed 8 years ago by nickm

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