BridgeDB should pass pluggable transport shared-secrets to clients

added component::circumvention/bridgedb owner::isis priority::high pt resolution::fixed status::closed type::defect labels

Oh, in case anyone cares, the relevant "Tor writes shared-secret to extra-info descriptor" ticket is #8978 (moved).

Trac:
Cc: sysrqb, isis to sysrqb, isis, phw

Okay, I read over 642ea61bda70f2dafbd89e27aa91a5d452888358 in asn's bug8978 branch. It's just an unlimited number of unsanitised key=value pairs.

From #9445 (moved):

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.

Another minor source of confusion, in little-t tor, the [arglist] is carried around as a comma-separated string, whereas in the actual bridge extra-info descriptors it seems to be space-separated like this:

transport obfs3 11.22.33.44:443 [exec /usr/local/bin/obfsproxy managed]

and so it's not actually following the [arglist] specification. If I understood that spec correctly, it should be like this:

transport obfs3 11.22.33.44:443 [ARGS:exec=/usr/local/bin/obfsproxy,runtime=managed]

or something.

Trac:
Owner: N/A to isis
Status: new to assigned

Changing priority to 'major' because #9445 (moved) is blocking for TBB-3.0.

Also, changing to 'needs more info' because I've no idea what the arglist in a transport line should look like in general, and less idea what it should look like for ScrambleSuit.

Trac:
Priority: normal to major
Status: assigned to needs_information

The idea was that the elements of the arglist are space-separated as in:

transport obfs3 11.22.33.44:443 shared-secret=asedqwe secondarg=lol

The spec is indeed unclear and should be fixed.

Do you think that:

transport obfs3 11.22.33.44:443 shared-secret=asedqwe,secondarg=lol

would have been better?

Replying to asn:

The spec is indeed unclear and should be fixed.

Do you think that: {{{ transport obfs3 11.22.33.44:443 shared-secret=asedqwe,secondarg=lol }}} would have been better?

Yes, I think the latter one, with the commas in-between, is likely easier to parse since the arglist is of unspecified length.

Is there a different purpose for the [exec /usr/local/bin/obfsproxy managed] portion of the current descriptors?

Replying to isis:

Is there a different purpose for the [exec /usr/local/bin/obfsproxy managed] portion of the current descriptors?

That's defined in the server-side's torrc. It isn't written to the extra-info doc.

An example is [https://gitweb.torproject.org/torspec.git/blob/HEAD:/proposals/180-pluggable-transport.txt#l171 [0]]

bridge trebuchet www.example.com:3333 keyid=09F911029D74E35BD84156C5635688C009F909F9 rocks=20 height=5.6m

And more generally [https://gitweb.torproject.org/torspec.git/blob/HEAD:/proposals/180-pluggable-transport.txt#l145 [1]]:

   Bridge method address:port [[keyid=]id-fingerprint] [k=v] [k=v] [k=v]

Replying to asn:

The idea was that the elements of the arglist are space-separated as in:
transport obfs3 11.22.33.44:443 shared-secret=asedqwe secondarg=lol
}}}

The spec is indeed unclear and should be fixed.

Yup, I think the spec if contradictory. It presents the examples (as in my last comment) with a space-separated arglist but then it defines the options, afaict, as comma-separated.

The spec defines [https://gitweb.torproject.org/torspec.git/blob/HEAD:/proposals/180-pluggable-transport.txt#l457 [2]] the arglist as: {{{ Bridges put the 'method' lines in their extra-info documents. transport SP SP address:port [SP arglist] NL

The address:port are as returned from an SMETHOD line (unless they are replaced by the FORWARD: directive). The arglist is a K=V,... list as returned in the ARGS: part of the SMETHOD line's Options component.


Where ARGS: is

  - ARGS:K=V,K=V,K=V

    If this option is set, the K=V arguments are added to Tor's
    extrainfo document.


and that leads me to think extra-info documents present args as comma-separated. dir-spec does not specify the format of arglist, either. Did I misinterpret something? If not, then you prefer to change the spec? (I'm not opposed to this, just looking for clarity).

Replying to sysrqb:

Replying to isis:

Is there a different purpose for the [exec /usr/local/bin/obfsproxy managed] portion of the current descriptors?

That's defined in the server-side's torrc. It isn't written to the extra-info doc.

An example is [https://gitweb.torproject.org/torspec.git/blob/HEAD:/proposals/180-pluggable-transport.txt#l171 [0]]
bridge trebuchet www.example.com:3333 keyid=09F911029D74E35BD84156C5635688C009F909F9 rocks=20 height=5.6m
}}}

The above line is what BridgeDB is supposed to create for the the lines it hands out to clients (i.e. over the web interface and through email). Though actually, I was also misremembering what the transport lines of the extrainfo bridge descriptors look like. Currently, in the cached-extrainfo bridge descriptors, there are lines which look like this: {{{ transport obfs3 6.6.6.6:6666

and I _thought_ that the above line ended in:

[exec /usr/local/bin/obfsproxy managed]

though I must have been mixing up the spec and what I remembered of the descriptors.

> And more generally [https://gitweb.torproject.org/torspec.git/blob/HEAD:/proposals/180-pluggable-transport.txt#l145 [1]]:
> ```
>    Bridge method address:port [[keyid=]id-fingerprint] [k=v] [k=v] [k=v]
> }}}

Again, for clarity, the above line is the spec for what BridgeDB needs to give to clients, not what BridgeDB should expect to find in a descriptor.

For the record, I like the idea of the bridge extrainfo descriptors looking like this:
{{{
transport obfs666 6.6.6.6:6666 argone=/usr/local/bin/obfsproxy,shared_secret=xyzzy

and for BridgeDB to take that information from the descriptor and distribute the following line to clients:

bridge obfs666 6.6.6.6:6666 keyid=0123456789abcdef0123456789abcdef01234567 argone=/usr/local/bin/obfsproxy shared_secret=xyzzy

where most of that info is taken from the transport line of the bridge extrainfo descriptor, all except for the fingerprint, which is taken from the bridge-server-descriptor.

Hi,

looking at the code again, the extrainfo line looks indeed like this:

transport obfs666 6.6.6.6:6666 argone=/usr/local/bin/obfsproxy,shared_secret=xyzzy

I also pushed branch bug9013 in https://git.torproject.org/user/asn/torspec.git. Maybe the added example will make things more comprehensible. I will ask Nick to merge it if you like it.

Replying to asn:

looking at the code again, the extrainfo line looks indeed like this: {{{ transport obfs666 6.6.6.6:6666 argone=/usr/local/bin/obfsproxy,shared_secret=xyzzy }}}

I think that using the path to the binary in an example will confuse some people. Is there a reason a PT should ever send a file path as an argument?

I also pushed branch bug9013 in https://git.torproject.org/user/asn/torspec.git. Maybe the added example will make things more comprehensible. I will ask Nick to merge it if you like it. I think that looks fine, unless isis has (or others have) objections I think it should be merged. It's simple but it reenforces the correct format.

Replying to sysrqb:

Replying to asn:
looking at the code again, the extrainfo line looks indeed like this:
transport obfs666 6.6.6.6:6666 argone=/usr/local/bin/obfsproxy,shared_secret=xyzzy
}}}
I think that using the path to the binary in an example will confuse some people. Is there a reason a PT should ever send a file path as an argument?

Ah, ok, here is another example: {{{ transport obfs666 6.6.6.6:6666 argone=valueone,shared_secret=xyzzy


atm I can't think of a reason that a PT would ever send a file path, but it could as well happen in the future.

For the record: all that ScrambleSuit adds is a password argument which can look like this: password=LLDNOWV7I4P6RKFJMDEMIY2GNU2IQISA. The argument consists of 20 random bytes encoded with Base32.

In my client's torrc, I would put:

Bridge scramblesuit a.b.c.d:1234 password=LLDNOWV7I4P6RKFJMDEMIY2GNU2IQISA

In my server's torrc, I would put:

ServerTransportListenAddr scramblesuit a.b.c.d:1234
ServerTransportOptions scramblesuit password=LLDNOWV7I4P6RKFJMDEMIY2GNU2IQISA

Let me know if I can help with anything.

Please review branch bug9013 in my public repo (sorry, it's been sitting locally for a while).

phw, thanks! I think we should be good to go as soon as this is merged.

Trac:
Status: needs_information to needs_review

Replying to sysrqb:

Please review branch bug9013 in my public repo (sorry, it's been sitting locally for a while).

Looks good!

I rebased it on top of develop in my branch fix/9013_r1. CI results are here and this is passing. I'm merging this in and closing as fixed.

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

closed

mentioned in issue #9156 (moved)

mentioned in issue #9406 (closed)

mentioned in issue #9445 (moved)

moved to tpo/anti-censorship/bridgedb#9013 (closed)

BridgeDB should pass pluggable transport shared-secrets to clients

Child items ...

Activity