Opened 7 years ago

Closed 7 years ago

#7013 closed task (implemented)

Allow specifying the bind address of managed proxies

Reported by: asn Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Keywords: tor-bridge, pt
Cc: dcf@… Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Tor instructs managed proxies to bind to a random port the first time they operate. It then stores that port in its state file, and the next time it spawns the same proxy it instructs it to bind to that port.

Currently, bridge operators don't have a way to specify the TCP port that their managed proxies will use to bind, and people rely on hacks, like editing the state file, to achieve this.
People need specific bind addresses because of port forwarding and because they might want to have their obfsproxy bind to a different IP address than their INADDR_ANY.

We should think about adding an optional configuration parameter to allow bind address customization. This can be:
a) An optional parameter in the ServerTransportPlugin line.
b) A new torrc line called ServerTransportBindaddr or ServerTransportPluginBindaddr.

I think I'm drifting towards 'b'. It's easier to implement and in the future we might also want ServerTransportPluginStateDirectory (etc.) and I'm not sure if we want to polute the ServerTransportPlugin line that much.

Child Tickets

TicketStatusOwnerSummaryComponent
#7257closedUpdate pt-spec.txt to allow specifying the bind address of managed proxiesCore Tor/Tor

Change History (16)

comment:1 Changed 7 years ago by arma

I'd go with ListenAddress rather than BindAddr, to go with the pattern we picked for Socks, Control, etc.

As for 'a' or 'b', the question is: if you have multiple server transport plugins, will they all want to use the same values for ListenAddress, StateDirectory, etc? My guess would be no. That means we either stick it all in the servertransportplugin line, or we do another LINELIST_V hack (with associated config.c headaches). Or we do some other smart trick.

comment:2 Changed 7 years ago by dcf

Cc: dcf@… added

comment:3 in reply to:  1 Changed 7 years ago by asn

Replying to arma:

I'd go with ListenAddress rather than BindAddr, to go with the pattern we picked for Socks, Control, etc.

As for 'a' or 'b', the question is: if you have multiple server transport plugins, will they all want to use the same values for ListenAddress, StateDirectory, etc? My guess would be no. That means we either stick it all in the servertransportplugin line, or we do another LINELIST_V hack (with associated config.c headaches). Or we do some other smart trick.

Indeed. Sticking them all in the ServerTransportPlugin will be ugly.

I was thinking of something like this:

ServerTransportPlugin obfs2,obfs3,stegotorus exec /usr/bin/obfsproxy --managed
ServerTransportListenAddr obfs2 0.0.0.0:4200
ServerTransportListenAddr stegotorus 98.23.4.45:6559

I'm also into smart tricks if someone has one.

comment:4 Changed 7 years ago by arma

Ah. And then Tor sends the listenaddr for that transport to whichever plugin claims to offer the transport. Sounds like a fine plan.

comment:5 Changed 7 years ago by nickm

Milestone: Tor: unspecified

comment:6 Changed 7 years ago by asn

Status: newneeds_review

Please see branch bug7013 in https://git.torproject.org/user/asn/tor.git.

comment:7 Changed 7 years ago by asn

Oh. Also see #7257 for the torspec side of this ticket.

comment:8 Changed 7 years ago by dcf

The code looks good to me. I tested the patch and it works the way I expect.

comment:9 Changed 7 years ago by nickm

Do you really want to be using "tor_addr_port_lookup" ? the 'lookup' means that it can potentially do hostname resolution.

Looks fine otherwise.

comment:10 in reply to:  9 Changed 7 years ago by asn

Replying to nickm:

Do you really want to be using "tor_addr_port_lookup" ? the 'lookup' means that it can potentially do hostname resolution.

Good call. When writing that code, I noticed that all addrport parsing code in config.c uses tor_addr_port_lookup, and I considered it an idiom of some kind.
I added a fixup commit for this issue.

Should we also change the rest of the places in config.c where tor_addr_port_lookup is used to use tor_addr_port_split?

comment:11 Changed 7 years ago by nickm

Hm. tor_addr_split isn't quite right; it allows anything in the address portion when instead we almost certainly want to make sure it's an IPv4 or an IPv6 address, right? I suggested that we not use tor_addr_split because we don't want these things to be hostnames. Do we?

There really ought to be a tor_addr_port_parse. I think we could build one out of tor_addr_parse and tor_addr_split, or out of tor_addr_parse_mask_ports with appropriate arguments set to NULL.

Should we also change the rest of the places in config.c where tor_addr_port_lookup is used to use tor_addr_port_split?

Not as part of this ticket. Only when it seems likely that DNS resolution would be a poor idea.

comment:12 Changed 7 years ago by asn

Fetch again! How does the tor_addr_port_parse() look like to you?

comment:13 Changed 7 years ago by nickm

With a name like that, I'd expect it to yield the address as a tor_addr_t, not a char*. (Compare tor_addr_parse).

comment:14 Changed 7 years ago by nickm

Status: needs_reviewneeds_revision

comment:15 in reply to:  13 Changed 7 years ago by asn

Status: needs_revisionneeds_review

Replying to nickm:

With a name like that, I'd expect it to yield the address as a tor_addr_t, not a char*. (Compare tor_addr_parse).

OK. Please see branch bug7013_take2 for an alternative version of the latest commit that uses tor_addr_t.

comment:16 Changed 7 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed

Looks okay; squashed it and merged it. Thanks!

Note: See TracTickets for help on using tickets.