Figure out a way to deal with bridges missing PT arguments.
From IRC, apparently approximately one in seven ScrambleSuit bridges do not have a password on the bridge line.
I am 99% certain this is due to people running ScrambleSuit on tor-0.2.4.x, as passing pluggable transport arguments via the extrainfo document is not supported in that version, and transports.c:parse_smethod_line() does not sufficiently validate the SMETHOD line, so the arguments are silently dropped.
This leads to Bridges that publish extrainfo documents containing Bridge lines that are totally useless (No ScrambleSuit password etc), and will never get used.
This may be something that has to be worked around on the BridgeDB side (teach it about transports that require arguments, ignore bridges with malformed bridge lines), but that seems suboptimal (people thinking they are contributing bridges, when they realistically are not).
Activity
Perhaps obfsproxy/scramblesuit should check the Tor version when being called, and if there were an easy way for PTs to specify a set of Tor version which are compatible, then it could alert the bridge operator if the version is incompatible.
In the meantime, BridgeDB could have some temporary logic to not use scramblesuit transports which do not have passwords, since these are already deployed and there's not much we could do to fix them.
Trac:
Cc: N/A to isis
Keywords: N/A deleted, scramblesuit, bridgedb-dist addedReplying to isis:
Perhaps obfsproxy/scramblesuit should check the Tor version when being called, and if there were an easy way for PTs to specify a set of Tor version which are compatible, then it could alert the bridge operator if the version is incompatible.
In the hindsight is 20/20 department, we don't have anything like
TOR_VERSIONin the pt environment space, although that would be a great thing to add. I'm not fundamentally opposed to this, but this approach still leaves the problem of people running old tor/obfsproxy/obfs4proxy still publishing busted extrainfo documents.Beyond the little-t tor changes required here (which would be quite trivial), this approach would also require changes to our pt code (and breaking working configs because they happened to upgrade the pt and not tor may be kind of rude).
In the meantime, BridgeDB could have some temporary logic to not use scramblesuit transports which do not have passwords, since these are already deployed and there's not much we could do to fix them.
That would be excellent (obfs4 will more than likely have the same problem).
For the record (yes I know it's unlikely to happen, just documenting it), a hypothetical maint-0.2.4 patch would be along the lines of adding:
if (smartlist_len(items) > 3) { log_warn(LD_CONFIG, "Server managed proxy sent us a SMETHOD line " "with too many arguments."); goto err; }This approach still would require filtering changes to BridgeDB for people that do not upgrade, but requires no pt side changes and won't break current 0.2.5.x configs that upgrade the pt but not tor (like changing ScrambleSuit/obfs4 to require a
TOR_VERSIONbeing set would).Replying to yawning:
we don't have anything like
TOR_VERSIONin the pt environment space,Aside, if we want other projects to use pluggable transports, we shouldn't add a TOR_VERSION variable. Otherwise those other projects will need to lie and say they're a specific version of Tor, need to be aware of Tor's changelog, and need to match features with a particular Tor version. It's better to signal support for specific features than have them implied by a version number.
Replying to dcf:
Replying to yawning:
we don't have anything like
TOR_VERSIONin the pt environment space,Aside, if we want other projects to use pluggable transports, we shouldn't add a TOR_VERSION variable. Otherwise those other projects will need to lie and say they're a specific version of Tor, need to be aware of Tor's changelog, and need to match features with a particular Tor version. It's better to signal support for specific features than have them implied by a version number.
This is probably true. AFAIK we currently do not have 3rd parties using the pt config protocol, but it would be good to keep this in mind. Offtopic, I'm kicking around the idea of doing a pt protocol version 2 with feedback from the rest of the obfuscation people (quite a few of the warts in the existing pt protocol could be fixed in a cleaner manner if we did something like this, otherwise we would need a lot of feature signals).
So to sum up the current situation:
- At a minimum, regardless of how we address this on the tor side, BridgeDB will need to filter out people running bridges that publish busted extrainfo descriptors (ScrambleSuit/obfs4 on tor 0.2.4.x), till affected tor versions are gone.
- I'm not overly hopeful about contact info for the broken bridges being filled in (probably missing or invalid for the majority), but we should at least send a post to tor-relays@ highlighting this issue.
- The ship on fixing this the right way (patching maint-0.2.4.x, yes, it is a bug) has sailed, so if we are going to patch tor, we need other options.
- The current leading candidate for a tor side patch would be to add something like
TOR_PT_EXTENSIONS=smethod-args(more to be added to the list as time goes on) to 0.2.5.x/0.2.6.x, and change obfsproxy/obfs4proxy toSMETHOD-ERRORif the env var is not present. This will fix new deployments, but will not address people that do not upgrade tor or obfsproxy/obfs4proxy. This will additionally break working configurations that upgrade to new obfsproxy/obfs4proxy without upgrading tor.
Does this seem accurate to people? I'm not particularly thrilled at the idea of patching 0.2.5.x/0.2.6.x for this because of the gotchas, but if other people think strongly that it would be a good idea, then I can start working on the relevant patches.
I'm also not very enthusiastic about adding workarounds in Tor for this. The good thing is that
0.2.5will soon get released and eventually become the stable release.Adding transport-specific workarounds in BridgeDB is not great either, but maybe we can get away with it because BridgeDB is in Python (fix will be 3 lines) and has a more fluid development cycle.
Another good thing is that all those scramblesuit bridges are probably providing obfs3 too, so they are not entirely wasted.
Finally, I'm not entirely against
TOR_VERSIONbecause it seems like potentially useful information and can help us hotfix such issues in the future (it's also easy to implement). Maybe instead ofTOR_VERSION, we name it in a more general manner (likeAPP_VERSION) or something, and let other projects fill in their own things.Replying to asn:
Finally, I'm not entirely against
TOR_VERSIONbecause it seems like potentially useful information and can help us hotfix such issues in the future (it's also easy to implement). Maybe instead ofTOR_VERSION, we name it in a more general manner (likeAPP_VERSION) or something, and let other projects fill in their own things.I don't know, I still think you might come to regret such a design... Even if the variable is called APP_VERSION, you're going to add code to ScrambleSuit that says
app, version = parse_app_version(APP_VERSION) if app == "tor" and version >= 0.2.5: do something else: do something elseIf any other project wants to use ScrambleSuit, and wants ScrambleSuit to take the "do something" branch, it will have no choice but to say "I am tor 0.2.5." Or else ScrambleSuit needs to have a catalog of applications and version numbers within it. It all starts to take some of the "pluggable" out of pluggable transports.
Think about the case of pluggable transports using a proxy. Not all versions of tor support telling the pluggable transport to use a proxy. But those that do, advertise their support by setting the TOR_PT_PROXY variable. Pluggable transports that support the feature show that they support it by writing a "PROXY DONE" line. The design instead could have had the pluggable transport sniffing the version number to check for proxy support, or by declaring a new managed proxy protocol version and simply requiring that all pluggable transports need to support proxies. But the way it actually works is much nicer. It's backwards compatible and does what you want in every combination (including failing with an error when tor wants a proxy but the PT doesn't support it). Could we find a design like that?
To put in in perspective, remember we once had a situation where there were a bunch of useless obfs3 bridges in BridgeDB because people installed obfsproxy but didn't open a port on their firewall. That's basically the same situation we're in now. With 0.2.5 only going to become more common, maybe this is a problem that will resolve itself.
Replying to nickm:
Not much progress since the 0.2.5 release, and it appears that 0.2.5 solved this. Calling don't-need-to-do-for-0.2.7 for now.... but Isis, if you think this is still worth doing, then my change here could be totally wrong.
I'm not thrilled to have PT-specific code in BridgeDB, because it'll be a bit of a rabbithole and maintenance nightmare, but I still see at least a few hundred scramblesuit bridges without the
passwordPT arg. The other transport deployments are doing much better than scramblesuit, e.g. there are currently 0 broken obfs4 lines in BridgeDB.Replying to nickm:
Okay. So, Tor 0.2.7 comes out in September-ish (we hope) and will start reaching TB alphas sometime around then. Would fixing this on such a timeframe be worthwhile, or might we hope that the scramblesuit situation will be better by then?
I don't really see the situation changing from the last time I commented on this. The problem is people still running bridges with tor 0.2.4.x and pts that require 0.2.5.x, and the only sensible tor side fix for this is still "patch maint-0.2.4 to break all the broken bridges".
Exposing the tor version in the 0.2.7.x timeframe does nothing to fix this (and complicates all of the pt code). It is something I will seriously consider if/when a pt protocol 2.0 happens.
At this point I favor either "Kludge something in bridgedb till 0.2.4.x dies" or "do nothing, since this will only get better over time".
I just grepped for the contact lines for the broken bridges, and not a single broken PT bridge has contact info. (I was going to send emails to the operators before trashing their bridges.)
As a slight tangent, I think some of these bridges might have something extra wonky going on; I saw bridge extrainfo descriptors like:
[…] transport scramblesuit 1.1.1.1:1111 […] bridge-ip-transports scramblesuit=8which shouldn't be possible… unless these
passwordless scramblesuit bridges are being used as private bridges and the operators wanted to send us statistics? Only some minority of thepasswordless scramblesuit bridges had wonkybridge-ip-transportsreporting scramblesuit usage (~1:20 broken ones). At least one broken scramblesuit bridge hadbridge-ip-transports scramblesuit=16.It's nice of them to send us statistics, I guess?
Until Tor-0.2.4.x is deprecated and no longer in use, I've placed a temporary fix in BridgeDB. It isn't elegant or anything, but it works, and there's a note to this ticket saying that at some point in the future we can remove it.
The patch is in my
fix/13202-missing-pt-argsbranch, and basically consists of adding the following method to thebridgedb.bridges.PluggableTransportclass, which is called whenever thePluggableTransport's attributes are checked, which happens whenever it is updated:+ def _checkArguments(self): + """This method is a temporary fix for PTs with missing arguments + (see `#13202 <https://bugs.torproject.org/13202`_). This method can + be removed after Tor-0.2.4.x is deprecated. + """ + # obfs4 requires (iat-mode && (cert || (node-id && public-key))): + if self.methodname == 'obfs4': + if self.arguments.get('iat-mode'): + if (self.arguments.get('cert') or \ + (self.arguments.get('node-id') and self.arguments.get('public-key'))): + return True + # scramblesuit requires (password): + elif self.methodname == 'scramblesuit': + if self.arguments.get('password'): + return True + else: + return True + + return FalseThere are also unittests to ensure this behaviour works as expected.
Trac:
Keywords: N/A deleted, bridgedb-0.3.2 added
Status: assigned to needs_reviewmentioned in issue #15397 (moved)
moved to tpo/core/tor#13202 (closed)
