get_configured_bridge_by_addr_port_digest() does not work well if digest == NULL

get_configured_bridge_by_addr_port_digest() returns NULL if digest is NULL (the caller doesn't care about the digest) and bridge->identity set.

Normally, the function should return any bridge with addr/port that matches even if it has an identity digest set.

This bug becomes visible when Tor does a second SOCKS request to the pluggable transport proxy after it has learned the identity digest of a bridge. In that second request, the SOCKS arguments are not passed, since digest == NULL but bridge->identity is set.

changed milestone to %Tor: 0.2.5.x-final

added component::core tor/tor milestone::Tor: 0.2.5.x-final priority::medium pt resolution::fixed status::closed tor-client type::defect labels

I see that you've opened this against 0.2.5.x. Is that because 0.2.4.x doesn't send SOCKS arguments or something, or is this a bug in 0.2.4.x too?

Replying to nickm:

I see that you've opened this against 0.2.5.x. Is that because 0.2.4.x doesn't send SOCKS arguments or something, or is this a bug in 0.2.4.x too?

Indeed. Only 0.2.5.x sends SOCKS arguments. See comment:33:ticket:3594 for a message from your past self.

(Reminder to self: This bug was found and reported by Yawning Angel, btw.)

Trac:
0001-Fix-a-get_configured_bridge_by_addr_port_digest-func.patch

Trac:
Status: new to needs_review

That patch would disable digest checking entirely, and allow any digest to match so long as the addr/port was correct.

I did a fixed-up version as e6590efaa77c8cf186ce92e6ebad175e9c6450d1. Thanks!

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

Nick's patch looks good to me.

Replying to asn:

This bug becomes visible when Tor does a second SOCKS request to the pluggable transport proxy after it has learned the identity digest of a bridge. In that second request, the SOCKS arguments are not passed, since digest == NULL but bridge->identity is set.

Can you describe this bug from the perspective of the user? (The changelog should be what changed from the perspective of the user, not what function got changed to pass its arguments differently.) Thanks!

I'd expect that the undesired behavior would have been "some pluggable transports top working after the first connection." George, am I right?

nickm is correct. Maybe a more verbose description would be "Pluggable transports that use client parameters stop working on subsequent connections to a bridge, if the first connection was terminated".

(Also, we don't have any PTs with client-side parameters atm. scramblesuit will soon become the first one.)

Thanks!

  o Minor bugfixes:
    - Fix a bug where the first connection works to a bridge that uses a
      pluggable transport with client-side parameters, but we don't send
      the client-side parameters on subsequent connections. (We don't
      use any pluggable transports with client-side parameters yet,
      but ScrambleSuit will soon become the first one.) Fixes bug 9162;
      bugfix on 0.2.0.3-alpha. Based on a patch from "rl1987".

Replying to asn:

This bug becomes visible when Tor does a second SOCKS request to the pluggable transport proxy after it has learned the identity digest of a bridge. In that second request, the SOCKS arguments are not passed, since digest == NULL but bridge->identity is set.

In answer to why there's a second SOCKS connection, it sounds like #7733 (moved).

closed

mentioned in issue #9228 (moved)

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

mentioned in issue tpo/anti-censorship/pluggable-transports/trac#9228 (closed)