[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_review
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 teor):

 * status:  needs_revision => needs_review


Comment:

 Replying to [comment:15 yawning]:
 > Replying to [comment:12 teor]:
 > > I think allowing bridges on internal addresses is a good fix for this.
 > > Please see my branch bug18517 at https://github.com/teor2345/tor.git
 > 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.
 > }
 > }}}

 Calling this Y1.

 Y1 is addressed by the fixup commit 4b271ad in my branch bug18517.

 This commit:
 * adds extend_info_is_a_configured_bridge, which calls
 addr_is_a_configured_bridge, based on the existing function
 get_configured_bridge_by_addr_port_digest
 * replaces the original patch's options->UseBridges with
 extend_info_is_a_configured_bridge

 I think addresses Yawning's concerns, and fixes dcf's issue with pluggable
 transports with dummy private addresses (and restores bridges on private
 addresses).

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