Opened 4 years ago

Last modified 23 months ago

#14579 new defect

Clients cannot use multiple transports with a single bridge

Reported by: sysrqb Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-client tor-bridge tor-pt
Cc: bastik.public@… Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

(note, this bug is unverified)

It seems that when a client adds more than one of a bridge's pluggable transport as its bridges, it only ever uses the address and port of the first transport it adds. Therefore, in the current situation, it appears if the first PT is disabled on the server-side (or the connection is blocked on the wire, or connections timeout for whatever reason) and the client attempts to use the second transport, it will try to establish the connection using the ip address and port number of the first transport. It may try connecting using the correct transport, but it'll probably connect to the wrong place.

Specifically, all the transports are added to the bridge list, but we only use the information from the first bridge in the list with a given ID and ignore the rest.

From learned_bridge_descriptor()

    /* Choose the first bridge which either has an ID which matches
     * this routerinfo or the same address and orport if we don't
     * know the ID */
    bridge_info_t *bridge = get_configured_bridge_by_routerinfo(ri);
    time_t now = time(NULL);
    router_set_status(ri->cache_info.identity_digest, 1);

    if (bridge) { /* if we actually want to use this one */
      node_t *node;
      /* it's here; schedule its re-fetch for a long time from now. */
      if (!from_cache)
        download_status_reset(&bridge->fetch_status);                                   

      /* get a *node_t for this bridge, based on its ID */
      node = node_get_mutable_by_id(ri->cache_info.identity_digest);
      tor_assert(node);
      /* Fill in the node_t using the details of the bridge we
       * retrieved above - the first bridge in the list
       */
      rewrite_node_address_for_bridge(bridge, node);

Then, we choose which entry node we want to use, in onion_extend_cpath(), where choose_good_entry_server() returns the *node_t representing the entry guard/bridge we should use, which it gets by ID:

    const node_t *r = choose_good_entry_server(purpose, state);
    if (r) {
      /* If we're a client, use the preferred address rather than the
         primary address, for potentially connecting to an IPv6 OR
         port. */
      info = extend_info_from_node(r, server_mode(get_options()) == 0);

from populate_live_entry_guards()

  SMARTLIST_FOREACH_BEGIN(all_entry_guards, const entry_guard_t *, entry) {
      const char *msg;
      node = entry_is_live(entry, entry_flags, &msg);

and finally, in extend_info_from_node() where we use the address and port defined in the *node_t:

  if (for_direct_connect)
    node_get_pref_orport(node, &ap);
  else
    node_get_prim_orport(node, &ap);

  log_debug(LD_CIRC, "using %s for %s",
            fmt_addrport(&ap.addr, ap.port),
            node->ri ? node->ri->nickname : node->rs->nickname);

  if (node->ri)
    return extend_info_new(node->ri->nickname,
                             node->identity,
                             node->ri->onion_pkey,
                             node->ri->onion_curve25519_pkey,
                             &ap.addr,
                             ap.port);

Child Tickets

TicketStatusOwnerSummaryComponent
#14581newLooking up bridge by ID may choose the wrong bridgeCore Tor/Tor

Change History (13)

comment:1 Changed 4 years ago by sysrqb

the first fix which comes to mind is to give each transport a unique ID, possibly salting the bridge's ID using the (transport name, addr, port) tuple and creating a node for each transport.

We might need to require that users define the bridge's ID when they configure their bridges, so we can have a mapping between transports and descriptor. Better idea welcome.

comment:2 Changed 4 years ago by bastik

Cc: bastik.public@… added

comment:3 Changed 4 years ago by nickm

Status: newassigned

comment:4 Changed 4 years ago by nickm

Keywords: 027-triaged-1-out added

Marking triaged-out items from first round of 0.2.7 triage.

comment:5 Changed 4 years ago by nickm

Milestone: Tor: 0.2.7.x-finalTor: 0.2.???

Make all non-needs_review, non-needs_revision, 027-triaged-1-out items belong to 0.2.???

comment:6 Changed 2 years ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:7 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:8 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:9 Changed 2 years ago by nickm

Keywords: 027-triaged-in added

comment:10 Changed 2 years ago by nickm

Keywords: 027-triaged-in removed

comment:11 Changed 2 years ago by nickm

Keywords: 027-triaged-1-out removed

comment:12 Changed 2 years ago by nickm

Status: assignednew

Change the status of all assigned/accepted Tor tickets with owner="" to "new".

comment:13 Changed 23 months ago by nickm

Keywords: tor-client tor-bridge tor-pt added
Severity: Normal
Note: See TracTickets for help on using tickets.