[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #2841 [Tor Bridge]: Implementing the pluggable-transport spec
#2841: Implementing the pluggable-transport spec
------------------------+---------------------------------------------------
Reporter: asn | Owner: asn
Type: task | Status: needs_review
Priority: normal | Milestone: Tor: 0.2.3.x-final
Component: Tor Bridge | Version:
Keywords: | Parent:
Points: | Actualpoints:
------------------------+---------------------------------------------------
Comment(by nickm):
Replying to [comment:10 asn]:
> Replying to [comment:9 nickm]:
> > * I don't think it should be an error to have a bridge listed with a
corresponding transport configured: it makes the bridge unusable, sure,
but we can still use the other bridges.
> >
> > * Why give a warning and reject the configuration if we have
transports configured without corresponding bridges? That seems
acceptable to me: just because I know about a transport doesn't mean I
should have to use it.
> >
> > * Having bridge_info_t keep a pointer to the transport_(info_)t seems
potentially risky if there's a chance we'll free the transport before we
free the bridge. Even if that can't happen now, it seems needlessly
fragile.
> >
>
> Alright I'll handle these three comments together.
>
> I removed the transport_t pointer from the bridge_info_t. I'm now using
transport_get_by_name() to get the bridge's transport instead of using
bridge->transport.
Sounds good.
> This change though, essentially makes match_bridges_with_transports()
semi-useless. Do you think we should still keep
match_bridges_with_transports() around so that it warns the user of
configuration anomalies (like the ones you mentioned in those two comments
above)?
>
> I'm not sure what the correct course of action is, when the user gives
Bridge lines with methods without corresponding ClientTransportPlugin
lines (or the opposite.).
> I was thinking that it would probably be user misconfiguration (typo or
forgetting to add a line) and that's why I was erroring out.
I agree with rransom that it's reasonable to have lots of
ClientTransportPlugin entries with no corresponding bridge using them. On
the other hand, if you have a bridge that you can't use because of a
missing ClientTransportPlugin, we should probably warn that we won't be
using that bridge.
My rationale for not warning in the first case is that it's a fine idea to
configure Tor (or have Tor come preconfigured) with a list of all the
transports you know about. You shouldn't have to disable the transports
just because you aren't using them.
For a bridge, on the other hand, you probably wouldn't configure it if you
didn't want to be able to use it. Having a bridge that you can't use is
normal, but it *is* something you'd want to know about.
> I don't have too strong feelings about this, but there are some small
things we should take care off if we don't error out.
> For example with the current code, if the user gives a Bridge line with
a method without a ClientTransportPlugin line, tor attempts to connect to
the Bridge addrport directly thinking that there is no pluggable transport
for this bridge.
> This happens because find_transport_by_bridge_addrport() returns either
the transport or NULL, and would return NULL on the above case. Do you
think this is a case we should be handling with something more than a
warning?
Absolutely. We should never connect directly to a bridge that was
configured with a transport.
> > * clear_transport_list should probably set transport_list to NULL.
> >
>
> What do you mean? clear_transport_list() just initiates transport_list,
like clear_bridge_list() does for bridge_list. It doesn't free it,
transport_list is a static smartlist just like bridge_list.
Well, what frees the the transport list on exit? Something must: we try
to free all our ram on exit so as to make it easier to find and debug
leaks.
> > In config.c:
> >
> > * In options_act(), it looks like we only call clear_transport_list()
if ClientTransportPlugin is set. Consider what this will do if we have
some transports configured, and then we remove all the configured
transports. It seems that all the old transports will remain configured.
> >
>
> Oh you are right! Shouldn't we do the same thing for Bridges?
(mark_bridge_list() and sweep_bridge_list() are only called if Bridges is
set)
I think we should, if that's so. Open a bug?
> > * Do we want to actually have ClientTransportPlugin exclusive with all
other proxy settings?
> >
>
> I'm not sure. The current code won't work if ClientTransportPlugin is
not exclusive with all other proxy settings, and I was considering that a
feature.
> If you think that it shouldn't be exclusive, we should change that.
If it's harder to implement chained proxies, we can call this an
unimplemented feature, and wait to see whether anyone needs it.
It might be that we need to teach the plugins to use proxies, rather than
having Tor manage the chaining; I can't figure out how else to implement
it in the likeliest case where we have a local transport plugin that needs
to go to the internet via a proxy.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/2841#comment:12>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs