[tor-bugs] #24734 [Core Tor/Tor]: Remove the return value of fascist_firewall_choose_address_node()

Tor Bug Tracker & Wiki blackhole at torproject.org
Sat Apr 28 01:47:16 UTC 2018


#24734: Remove the return value of fascist_firewall_choose_address_node()
-------------------------------------------------+-------------------------
 Reporter:  teor                                 |          Owner:  neel
     Type:  defect                               |         Status:
                                                 |  merge_ready
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.3.4.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  ipv6, tor-relay,                     |  Actual Points:
  034-triage-20180328, 034-removed-20180328      |
Parent ID:  #24403                               |         Points:  0.5
 Reviewer:  mikeperry                            |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by mikeperry):

 Replying to [comment:19 teor]:
 > Replying to [comment:18 mikeperry]:
 > > Ok I took Neel's patches and put them in a branch. I also noticed a
 couple logic issues:
 > > 1. Since we're getting rid of the return value, and there was some
 code relying on it that was changed to use tor_add_port_is_valid_ap(), we
 need to zero the ap pointer before any early return from these functions
 for those correctness checks to still be valid. I moved the zeroing code
 up in a fixup commit for this.
 >
 > Thanks, that deals with my second review point in:
 >
 https://trac.torproject.org/projects/tor/ticket/24734?replyto=18#comment:9
 >
 > > I double-checked that tor_addr_port_is_valid_ap() rejects these zeroed
 addrs. It does, if they are also not AF_INET.
 >
 > That's not quite accurate. tor_addr*_is_valid*() rejects addresses/ports
 when:
 > * the address is all zeroes, and for_listening is 0
 > * the port is zero, and for_listenining is 0
 > * the address not an IPv4 address, and for_listening is 1
 >
 > Since for_listening is 0 in all the checks added in the patch, the first
 two cases apply.

 What I meant was: because this calls tor_addr_make_null with AF_UNSPEC,
 then the family will not be AF_INET. So tor_addr_is_valid() cannot return
 false for this null addr, even if for_listening is true.

 > > 2. There were a couple of places that lacked a tor_assert(ap). So I
 added them.
 >
 > Thanks, that deals with my first review point in:
 >
 https://trac.torproject.org/projects/tor/ticket/24734?replyto=18#comment:9
 >
 > > I added fixup commits for these.
 > >
 > > Neel - be sure to consider calling, surrounding, and future code when
 you make changes like this.
 > >
 > > I think this is ready to go:
 https://oniongit.eu/mikeperry/tor/commits/bug24734
 >
 > I'm happy with it. Does it pass all the unit tests?

 They do on my system. (Do you know what I have to do to connect travis to
 oniongit? Does it only check github? And does that require config?)

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


More information about the tor-bugs mailing list