[tor-bugs] #18833 [Tor]: "TestSocks 1" is silent for socks5-not-hostname connections

[Tor Bug Tracker & Wiki]

#18833: "TestSocks 1" is silent for socks5-not-hostname connections
------------------------+--------------------------
     Reporter:  arma    |      Owner:
         Type:  defect  |     Status:  new
     Priority:  Medium  |  Milestone:  Tor: 0.2.???
    Component:  Tor     |    Version:
     Severity:  Normal  |   Keywords:  029-proposed
Actual Points:          |  Parent ID:
       Points:          |   Reviewer:
      Sponsor:          |
------------------------+--------------------------
 In parse_socks(), we do
 {{{
           if (log_sockstype)
             log_notice(LD_APP,
                   "Your application (using socks5 to port %d) instructed "
                   "Tor to take care of the DNS resolution itself if "
                   "necessary. This is good.", req->port);
 }}}
 and
 {{{
         if (log_sockstype)
           log_notice(LD_APP,
                      "Your application (using socks4a to port %d)
 instructed "
                      "Tor to take care of the DNS resolution itself if "
                      "necessary. This is good.", req->port);
 }}}
 But for the "you gave us socks5, the wrong kind" case, we call
 log_unsafe_socks_warning(), and it ends up not logging anything.

 On first look, the issue is in log_unsafe_socks_warning(), where it says
 {{{
   if (safe_socks) {
 }}}

 In commit a7334f5, we see
 {{{
 -  if (safe_socks || (m = rate_limit_log(&socks_ratelim, approx_time())))
 {
 -    log_warn(LD_APP,
 +  if (safe_socks) {
 +    log_fn_ratelim(&socks_ratelim, LOG_WARN, LD_APP,
 }}}

 That doesn't look right, does it?

 (That commit got into 0.2.4.11-alpha it seems.)

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