Opened 6 years ago

Closed 6 years ago

#9445 closed task (fixed)

Tor Launcher should be more relaxed about bridge line input

Reported by: bastik Owned by: brade
Priority: High Milestone:
Component: Applications/Tor Launcher Version:
Severity: Keywords: tbb-3.0-stable-blocker, tbb-usability
Cc: mcs@…, isis@…, g.koppen@… Actual Points:
Parent ID: #9444 Points:
Reviewer: Sponsor:

Description

Upon launch a user has to be able to selct if he/she wants to make use of pluggable transports, if the version of TBB includes them.

Ideally with the option to override normal bridges, so that "normal" bridges don't get used.
(They may know that those don't work.)

Furthermore it would be nice to let them choose which framework (WebSocket, Obfsproxy) to use or which not; ideally down to the actual transport (Flashproxy, obfs2, obfs3).
(The reason behind this is that educated users may know that they look suspsious to the filtering device when they use obfs2 or Websockets.)

Child Tickets

Attachments (1)

designdraft suggestion.png (49.0 KB) - added by bastik 6 years ago.
how it could look like

Download all attachments as: .zip

Change History (17)

Changed 6 years ago by bastik

Attachment: designdraft suggestion.png added

how it could look like

comment:1 Changed 6 years ago by isis

Cc: isis@… added

This was discussed briefly in #tor-dev while I was testing bridge support in the alpha 3.x builds (specifically 3.0.3a):

13:45          isis ) if you tell TBB-3.0.3a on startup to "use a proxy" and set it to socks5://127.0.0.1:9050, and then tell it that you need to use bridges, it 
                      doesn't configure the bridge lines in your torrc, instead it decides to try to connect to <bridge_ipaddr>:<orport> via the socks5 proxy, which 
              makes tor complain if WarnSocks 1
13:46          isis ) hmm. and then it just hangs out there, chilling out with the progress bar at like 10%, doing nothing.
13:47          isis ) well, at least it's confirmed that TOR_CONTROL_PASSWD='"passwd"' with the weird quoting works, and having two controllers works okay
13:51          isis ) ah, also, when i tried entering obfs2 and obfs3 bridges into torbutton on startup, it barfed appropriately, though it also did (i think) some 
                      amount of processing to the text inside the text box in the "Tor Status" window, because it was originally like "1.1.1.1:1111\nobfs2 
              2.2.2.2:2222\nobfs3 3.3.3.3:3333\n" and then i ended up with "1.1.1.1:1111\nobfs2\n2.2.2.2:2222\nobfs3\n3.3.3.3:3333\n" ...
13:51          isis ) ... with extra newlines
13:52        sysrqb ) ah, new lines around the transport
14:00          isis ) sysrqb: yeah, which could be pretty bad, because if a client is trying to connect and they see the barfing logs with some mumbling about "not 
                      formatted correctly" plus the thing in the "Network Settings" window which says "in the form <IP>:<port>" they may just remove the transport 
                      names, and not the corresponding IPs
14:01          isis ) this would be particularly bad because it would 1) still not work, and 2) cause a normal outgoing tor conn to a PT bridge, giving away to 
                      censors that there is a bridge on that IP:port pair
14:04        sysrqb ) isis: oh, huh, that's strange
14:06        sysrqb ) isis: i guess you can add your lack-of-transport-support to #9445
14:08     mikeperry ) isis: yeah, that is a tor launcher bug I guess, not tobtton
15:52          isis ) mikeperry: also, the problem with transport lines and newlines in torlauncher/torbutton that i mentioned earlier, see http://sprunge.us/CQfL . 
                      it did manage to set the Bridge lines in Data/Tor/torrc eventually, but only after restarting TBB.

comment:2 Changed 6 years ago by gk

Cc: g.koppen@… added

comment:4 Changed 6 years ago by brade

Cc: mcs@… added

comment:5 Changed 6 years ago by mikeperry

Keywords: tbb-3.0 tbb-usability added
Priority: normalmajor

I think the answer here is to make the bridge input to Tor Launcher input format checking more relaxed, and take bridge lines directly from bridgedb and set them in the torrc/via SETCONF, perhaps verifying only that they start with 'bridge'.

I would also like to have the ability to support identity fingerprints and any other pluggable transport-related key material for bridge users (#9499).

I don't think this should be too hard, and if we ship a set of pluggable transports, it will make #9156 and all of the pluggable transport integration stuff easier to develop and test. I think this should be a high priority as a result.

comment:6 Changed 6 years ago by mikeperry

Keywords: tbb-3.0-stable-blocker added; tbb-3.0 removed

We should also do this before we release as beta/stable, to minimize user confusion.

comment:7 in reply to:  5 Changed 6 years ago by isis

Replying to mikeperry:

I think the answer here is to make the bridge input to Tor Launcher input format checking more relaxed, and take bridge lines directly from bridgedb and set them in the torrc/via SETCONF, perhaps verifying only that they start with 'bridge'.

BridgeDB recently removed the "bridge " prefix from both regular bridge lines (commit) and pluggable transport lines (commit), because PT-bundle users tried to put the lines directly into Vidalia, which glitched out on it (#9156).

BridgeDB can return whatever format you like, as long as it is specified.

I would also like to have the ability to support identity fingerprints and any other pluggable transport-related key material for bridge users (#9499).

It can also return ID fingerprints, there is code for this already though I haven't tested it.

The [arglist] portion of an extra-info descriptor transport string is somewhat problematic, with the current way that it is specified -- though it does make sense to be spec'd this way. Basically, tor does no sanitisation of the transport line [arglist] for a pluggable transport sending args, because it is within the treat model to assume that the transport is a trusted application.

However, this puts all the responsibility of parsing on BridgeDB. Which is also fine, and much more doable in Python than in C...it's just that writers of pluggable transports which they would like to see deployed need to create a spec, and need to create a ticket for BridgeDB that points to the spec and says exactly what BridgeDB should parse for. An example of a ticket which does not help me understand what to parse for is a vague "BridgeDB should pass pluggable transport shared-secrets to clients" (#9013) ticket.

I don't think this should be too hard, and if we ship a set of pluggable transports, it will make #9156 and all of the pluggable transport integration stuff easier to develop and test. I think this should be a high priority as a result.

Agreed.

@mikeperry: Are you thinking of making two separate sets of bundles again, like in the TBB-2.x series? Because now that we've lost all the QT junk, we could probably fit a Python interpreter and the deployed PTs in there, and then we wouldn't have to deal with users not knowing the difference between the two bundles.

comment:8 Changed 6 years ago by mikeperry

@isis: For usability reasons my long term goal is to try to cram all of the pluggable transports into The One True Bundle. Hopefully they still fit in the gmail attachment limit for gettor, but even if they don't, we'll probably have to find some other solution anyway for gettor, because the intersection of gettor users and PT users is probably high.

As for argument parsing/validation, if these PT lines call for execution of arbitrary programs, I am now once again scared. If they are just passing args, I am less scared.

comment:9 Changed 6 years ago by mikeperry

Summary: Include support for Pluggable TransportsTor Launcher should be more relaxed about bridge line input

Retitling this ticket, because I want it to be only about making Tor Launcher allow full PT lines to get pasted into its bridge input fields and get passed to the underlying Tor, which is the simple change.

Full support for all PTs is a function of including them, getting the paths right, etc, and should all be under separate tickets, perhaps siblings under #9444.

comment:10 Changed 6 years ago by mikeperry

As I said in #9156, I think the most sane way forward here is to check to see if 'bridge ' is present at the beginning of the line, and if yes, accept it, otherwise prepend 'bridge ' to it and shove it into SETCONF+SAVECONF/torrc, and let Tor decide if it is actually valid with respect to the PT schemes installed or not. That way we can support both formats in Tor Launcher.

Are there any code exec-level dangers to pasting arbitrary bridge lines with the current PT scheme?

comment:11 in reply to:  10 Changed 6 years ago by sysrqb

Replying to mikeperry:

As I said in #9156, I think the most sane way forward here is to check to see if 'bridge ' is present at the beginning of the line, and if yes, accept it, otherwise prepend 'bridge ' to it and shove it into SETCONF+SAVECONF/torrc, and let Tor decide if it is actually valid with respect to the PT schemes installed or not. That way we can support both formats in Tor Launcher.

This will be a good way to do it because users may try to reuse the bridges they added in Vidalia, and the settings menu in Vidalia does not display the 'Bridge' keyword, so they will likely not include it.

Are there any code exec-level dangers to pasting arbitrary bridge lines with the current PT scheme?

At present, all argument validation is (supposed to be) handled by the PT. Tor Launcher can probably perform some validation of the basic syntax, but it's not expected to do so for the optional arglist (which becomes a deep, dark hole). None of the fully-implemented PTs use the optional arglist, so there isn't a way to pass any kind of executable string to them. However, this is possible, but any implemented PT that relies on external args for execing will need extensive validation checks - something I don't think Tor Launcher should be expected to implement.

comment:12 Changed 6 years ago by isis

Priority: majorblocker
Status: newneeds_information

Changing status to 'blocking' because #9156 is blocking on this.

From #9156: comment 36

Should BridgeDB keep the "bridge " prefix or get rid of it? We can't stay in limbo with it half removed, and I really don't want to switch back and forth.

This basically boils down to:

PlanA

We get rid of the "bridge " prefix on the lines that BridgeDB returns to users. Doing so means:

  • Continuing to cause problems for Vidalia users.
  • Continuing to cause problems for anyone still using TBB-2.x (though this problem will disappear with time).

PlanB

We keep the prefix. Doing so means:

  • We won't have to explain to torrc-using people that they need to add it to the beginning of every line.
  • We remain backward-compatible with anything which parses this output and was expecting there to be lines starting with "bridge ".
  • TorLauncher can continue to parse for it, instead of having to parse for the IP address right away (which is likely more difficult, though in JS I don't know).

I say fuck Vidalia, and vote for PlanB.

comment:13 Changed 6 years ago by mcs

We made a few small changes to Tor Launcher. Summary:

  • Less parsing of input is done. We still trim leading and trailing whitespace, but spaces are no longer turned into linebreaks (each bridge is assumed to be on one line of input).
  • We always prepend "bridge" when displaying bridges. This is consistent with PlanB.

https://gitweb.torproject.org/tor-launcher.git/commit/6c064c5c652ec104ca600e9609ec53e6400ef9db

comment:14 Changed 6 years ago by mcs

Priority: blockermajor

We made a few more improvements to Tor Launcher:

https://gitweb.torproject.org/tor-launcher.git/commit/b709f9ffbbad9867fb1041824b9a1df4d43fed73

Ideally, Tor Launcher would detect situations that will never result in a successful bootstrap (e.g., if the user has a non-PT bundle and only PT bridges are configured). Unfortunately the tor process logs a warning but does not produce a bootstrap error so Tor Launcher just displays a frozen progress bar. Should we file a bug against tor?

comment:15 Changed 6 years ago by mikeperry

Yes, the bootstrap problem sounds like it should be fixed on the Tor end.

However, it looks like you also hacked around this by having Tor Launcher watch for warn+error logs during bootstrap. We should now be able to close this bug and open a new one for the adoption of any future bootstrap notification fixes for Tor, right?

Or is there another reason you left this at 'needs_information'?

comment:16 in reply to:  15 Changed 6 years ago by mcs

Resolution: fixed
Status: needs_informationclosed

Replying to mikeperry:

Yes, the bootstrap problem sounds like it should be fixed on the Tor end.

However, it looks like you also hacked around this by having Tor Launcher watch for warn+error logs during bootstrap. We should now be able to close this bug and open a new one for the adoption of any future bootstrap notification fixes for Tor, right?

Or is there another reason you left this at 'needs_information'?

Tor Launcher will make the "Copy Tor Log To Clipboard" button available if it detects warnings or errors via the tor log. But it will still hang with the "Connecting..." progress bar showing because Tor Launcher does not treat a WARN or ERR from the log as a bootstrap failure. And I don't think it can because other WARNs are about temporary or unimportant conditions.

I will close this bug and open a new one against tor.

Note: See TracTickets for help on using tickets.