[tor-bugs] #18517 [Tor]: meek is broken in Tor Browser 6.0a3

[Tor Bug Tracker & Wiki]

#18517: meek is broken in Tor Browser 6.0a3
-------------------------------------------------+-------------------------
 Reporter:  gk                                   |          Owner:  teor
     Type:  defect                               |         Status:
 Priority:  Very High                            |  needs_revision
Component:  Tor                                  |      Milestone:  Tor:
 Severity:  Normal                               |  0.2.8.x-final
 Keywords:  regression must-fix-before-028-rc    |        Version:  Tor:
  meek                                           |  0.2.8.1-alpha
Parent ID:                                       |     Resolution:
 Reviewer:                                       |  Actual Points:
                                                 |         Points:
                                                 |        Sponsor:  None
-------------------------------------------------+-------------------------
Changes (by yawning):

 * status:  needs_review => needs_revision


Comment:

 Replying to [comment:12 teor]:
 > I think allowing bridges on internal addresses is a good fix for this.
 >
 > (And since they're configured in the torrc, this doesn't open any
 security holes. Bridge addresses can be updated from bridgedb, but if
 bridgedb started returning broken addresses, then that's not an issue
 clients can defend against.)

 I agree that this is probably the best way to go.

 > Please see my branch bug18517 at https://github.com/teor2345/tor.git
 > It fixes this issue and clarifies the corresponding entries in the tor
 man page.
 > It needs to go in tor-0.2.8-rc, target date end of March.

 NACK.  The check you added only checks if `UseBridges` is set in the
 torrc, and bypasses the behavior.  This isn't great, and we can/should be
 more explicit in what we allow.

 Instead, if we wish to go with the current fix, we should verify that
 `firsthop->extend_info->addr` is actually a bridge (and while we're here
 it, also validate that the port of our possibly synthetic address matches
 that on the bridge line).  We do not have an easy function to query if a
 given address/port is a configured bridge, but
 `get_transport_by_bridge_addrpor()` is 95% of what such a thing will look
 like and is trivial to add.

 Basically the check should function like:
 {{{

 int is_configured_bridge(const tor_addr_t *addr, uint16_t port) {
   // Traverse `bridge_list` (see entrynodes.c), return 1 if addr/port is
 present, 0 otherwise.
 }

 static int is_or_destination_valid(const tor_addr_t *addr, uint16_t port)
 { // Rename as appropriate...
   if (!get_options()->ExtendAllowPrivateAddresses) {
     if (get_options()->UseBridges && is_configured_bridge(addr, port)) //
 Write the 2nd function...
       return 1; // All address/port combinations listed on `Bridge`
 directives allowed.
     if (tor_addr_is_internal(addr, 0))
       return 0; // Filter out local address destinations for all non-
 Bridge ORs.
   }
   return 1; // Default allow.
 }
 }}}

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/18517#comment:15>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online