[tor-bugs] #5603 [Tor Client]: get_configured_bridge_by_addr_port_digest is not robust (was: tor doesn't notice some Bridge line edits after SIGHUP)

[Tor Bug Tracker & Wiki]

#5603: get_configured_bridge_by_addr_port_digest is not robust
------------------------+---------------------------------------------------
 Reporter:  asn         |          Owner:                    
     Type:  defect      |         Status:  new               
 Priority:  major       |      Milestone:  Tor: 0.2.3.x-final
Component:  Tor Client  |        Version:                    
 Keywords:              |         Parent:                    
   Points:              |   Actualpoints:                    
------------------------+---------------------------------------------------

Comment(by asn):

 I've been kinda hesitant in fixing this, because
 `get_configured_bridge_by_addr_port_digest()` is a popular function, and
 I'm afraid that I might break stuff. For example, it's called when adding
 bridges from the config, when we learn a new identity digest for a bridge,
 when we add a bridge to the routerlist, etc..

 `get_configured_bridge_by_addr_port_digest()` doesn't have a well-defined
 interface or behavior either. For example, some interesting things about
 the function:

 a) It seems that all its arguments are optional. If an addrport is
 '''not''' given, but a digest is given and it matches the digest of a
 configured bridge, the configured bridge is returned. Similarly, if a
 digest is '''not''' given, but the addrport can match a configured bridge,
 the configured bridge is returned.

 b) If '''all''' arguments are provided (addr, port '''and''' digest), if
 the given digest matches a configured bridge's digest, the configured
 bridge is returned, even if the addrport doesn't match. The same is not
 true, if the digests don't match but the addrports match.

 c) If an addrport and digest are given, and a configured bridge
 '''doesn't''' have a stored digest but its addrport matches the provided
 addrport, the configured bridge is returned.

 I'm not sure which of the above properties are bugs and which are design
 choices (for example, `learned_router_identity()` seems to use c)), but
 it's hard to fix this ticket without a well defined interface/behavior.

 To minimize the breakage, maybe we can teach
 `get_configured_bridge_by_addr_port_digest()` about pluggable transports
 in 0.2.3, and then refactor it in 0.2.4. Note that this would not solve
 bugs like ''b) If a bridge has a fingerprint and it remains the same, but
 its address/port changes, tor won't notice it.'' in
 `bridge_add_from_config()`.

 Teaching the function about PTs would involve adding a `const char
 *transport` argument to the function. `transport` should be optional: if
 it's not given, we still return bridges that match the other given
 arguments. If `transport` is given, we '''don't''' return bridges that
 match the other arguments but have a different `transport`.

 Refactoring the function would involve specifying exactly what the
 function expects as arguments, and how it treats its arguments. We should
 also see if a single function can handle all our use cases, and whether we
 need to split it into multiple functions with different logic.

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