managed obfsproxy cannot talk to tor on other than 127.0.0.1

changed milestone to %Tor: 0.2.3.x-final

added component::core tor/tor milestone::Tor: 0.2.3.x-final owner::asn priority::medium reporter::wwaites resolution::fixed status::closed tor-bridge type::defect labels

Trac:
Username: wwaites

obfsproxy-orlisten.diff

Trac:
Username: wwaites

The attached diff appears to fix the problem, though there may be a better way to find out the apparent IP address and port than just pulling it from the (first such line in the) config.

Trac:
Username: wwaites

Yeah; we want to have it bind to the actual address we're listening on, not to the content of a string from the configuration file. With proposal 186, these strings can get pretty complex. Also, we can wind up autodetecting an address based on an instruction to use a given port, or picking a port based on getting "orport auto" or such.

router_get_advertised_or_port is a step in the right direction, but this needs to actually hook into our logic for figuring out what our address is.

Trac:
Component: Pluggable transport to Tor Bridge
Milestone: N/A to Tor: 0.2.3.x-final

Trac:
Status: new to needs_revision

Trac:
Username: skep
Cc: N/A to skep@riseup.net

Should we make a router_get_first_advertised_or_addr(), which does a connection_get_by_type(CONN_TYPE_OR_LISTENER); and returns its addr? This way we should get the address of a lucky OR listener.

Or should we make a get_primary_or_addr() which acts like get_primary_or_port(), looping through all the configured port_cfg_ts, but returns the address instead of the port? I'm not sure how port_cfg_t is created, and I'm not sure if that would simply return the configuration file string.

Maybe we should also spec. this change, and mention that if a bridge has multiple OR listeners, there is no rule to select a specific one to put in TOR_PT_ORPORT. Or maybe we should make a rule?

Trac:
Cc: skep@riseup.net to skep@riseup.net, twilde@cymru.com

Please see bug4865 in https://git.gitorious.org/mytor/mytor.git.

Consider the following:

a) I'm not super-sure about how config.c:configured_ports works. In get_first_listener_addrport_string(), I basically followed the logic of router_get_advertised_or_port().

b) Since 93414707aec510c253cf150b654b04dab45cf9e9, set_managed_proxy_environment() can't fail anymore. I'm not sure how to handle get_first_listener_addrport_string() fails. I'm currently asserting its return value. I don't think this is the correct solution. I didn't have time to think of something else.

c) I did the following tests:

orport auto
--
orport 5555
--
ORPort auto
ORListenAddress 192.168.2.1:10001
--
ORPort auto
ORListenAddress 192.168.2.1:10001
ORListenAddress 192.168.2.1:10000
--
ORPort 5555
ORListenAddress 192.168.2.1:10001
ORListenAddress 192.168.2.1:10000

the results seem correct. I tested it by running a managed proxy with a logfile on debug severity, and checking the environment variable logs.

My suggestion is that if you like the code, and you have some time to test it, you can merge it. If you don't have the time to test it, I would like to do some more testing before merging.

Trac:
Status: needs_revision to needs_review

Similarly, obfsproxy server should listen for incoming on the same interface instead of 0.0.0.0. It would be nice to be able to configure all this in some way. I've been using the "unmanaged" transport to work around it.

But that trick is probably difficult on the client side of obfsproxy, if i wanted to make the connections go out on a specific interface like OutboundBindAddress.

Trac:
Username: fermenthor
Cc: skep@riseup.net, twilde@cymru.com to skep@riseup.net, twilde@cymru.com, fermenthor@gmail.com

In #5143 (moved), murble reports that IPv6 ORPort addresses are not passed correctly to obfsproxy. He says that tor configured with ORPort "[::]:9001", passes TOR_PT_ORPORT="127.0.0.1:0" to obfsproxy.

The current bug4865 branch passes TOR_PT_ORPORT=":::9001" to obfsproxy, when tor is configured with ORPort "[::]:9001". Maybe it should be passing TOR_PT_ORPORT="[::]:9001"?

This doesn't look too hard to pull off. We will probably have to use a variant of fmt_addr() that uses tor_addr_to_str() with decorate set to 1.

Cleaned up the branch, added some more documentation and implemented the IPv6 case. You can find the new code in bug4865_take2.

The fmt_addr and fmt_addr_decorate macros need to be documented. Also, standard C best practices suggest enclosing macro arguments in parenthesis, as in

#define fmt_addr(a) fmt_addr_iml((a), 0)

I think router_get_active_listener_port_by_type is probably a questionable interface. It can't work predictably with multiple listeners, and it can't help you if the address being listened on isn't the one you expected. It looks like we're already making that mistake with CFG_AUTO_PORT handling elsewhere, though, so maybe only an XXXX comment is in order.

When iterating over configured_ports, you've got to skip the nolisten ones, I think. Also, we could run into trouble if we call "get_first_listener_addrport_for_pt" before the listeners are started. Is this possible? Also, that function should check for failure from router_get_active_listener_port_by_type.

"get_first_listener_addrport_for_pt" is not a great name; we generally name functions based on what they do, not who uses them.

Other than that, it seems plausible.

Trac:
Status: needs_review to needs_revision

Replying to nickm:

The fmt_addr and fmt_addr_decorate macros need to be documented. Also, standard C best practices suggest enclosing macro arguments in parenthesis, as in {{{ #define fmt_addr(a) fmt_addr_iml((a), 0) }}}

I once read https://www.securecoding.cert.org/confluence/display/seccode/PRE01-C.+Use+parentheses+within+macros+around+parameter+names (see PRE01-EX1) and that's why I did not put any parenthesis there. In any case, fixed.

I think router_get_active_listener_port_by_type is probably a questionable interface. It can't work predictably with multiple listeners, and it can't help you if the address being listened on isn't the one you expected. It looks like we're already making that mistake with CFG_AUTO_PORT handling elsewhere, though, so maybe only an XXXX comment is in order.

I agree. I added an XXX.

When iterating over configured_ports, you've got to skip the nolisten ones, I think. Also, we could run into trouble if we call "get_first_listener_addrport_for_pt" before the listeners are started. Is this possible? Also, that function should check for failure from router_get_active_listener_port_by_type.

Fixed. I don't think it's possible to get there before the listeners are started. Listeners are started in options_act_reversible() (even before privsep). OTOH, create_managed_proxy_environment is called after parsing all transport lines and after a couple of run_scheduled_events() ticks. My tests agree to this assumption.

"get_first_listener_addrport_for_pt" is not a great name; we generally name functions based on what they do, not who uses them.

Fixed.

Other than that, it seems plausible.

Pushed new stuff to bug4865_take2.

Trac:
Status: needs_revision to needs_review

Hm. I think it looks ok now. Merging.

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

Trac:
Keywords: N/A deleted, tor-bridge added

Trac:
Component: Tor Bridge to Tor

closed

mentioned in issue #4927 (closed)

mentioned in issue #5104 (moved)

mentioned in issue #5143 (moved)

mentioned in issue #6274 (moved)

moved to tpo/core/tor#4865 (closed)

managed obfsproxy cannot talk to tor on other than 127.0.0.1

Child items 0

Activity