Opened 9 years ago

Closed 8 years ago

Last modified 7 years ago

#3325 closed defect (fixed)

Log message when a client tries to connect to an invalid hostname is incorrect

Reported by: rransom Owned by: rransom
Priority: Very Low Milestone: Tor: 0.2.3.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: easy tor-client
Cc: warms0x Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

In connection_ap_handshake_rewrite_and_attach:

  addresstype = parse_extended_hostname(socks->address,
                         remapped_to_exit || options->AllowDotExit);

  if (addresstype == BAD_HOSTNAME) {
    log_warn(LD_APP, "Invalid onion hostname %s; rejecting",
             safe_str_client(socks->address));

parse_extended_hostname also returns BAD_HOSTNAME for .exit hostnames when AllowDotExit is off.

Also, parse_extended_hostname's documentation comment does not mention BAD_HOSTNAME.

Child Tickets

Change History (9)

comment:1 Changed 8 years ago by warms0x

Cc: warms0x added

Per my discussion in #tor, it would seem the appropriate course of action is to update parse_extended_hostname's documentation. Since parse_extended_hostname actually invokes a log_warn() about the .exit notation being disable, it doesn't make much sense for connection_ap_handshake_rewrite_and_attach to do it as well.

It might make sense for parse_extended_hostname to return a new constant like DISABLED_HOSTNAME which would prevent the log_warn() in the description

comment:2 Changed 8 years ago by rransom

Milestone: Tor: 0.2.2.x-finalTor: 0.2.3.x-final

comment:3 Changed 8 years ago by nickm

Status: newneeds_review

Fix in branch bug3325 in my public repo; please review; should be straightforward.

comment:4 Changed 8 years ago by warms0x

I've grabbed the latest refs from your public repo (presuming you meant the one under git.torproject.org) and I did not see a 'bug3325' branch at all.

comment:5 Changed 8 years ago by Sebastian

Nick means https://gitweb.torproject.org/nickm/tor.git/shortlog/refs/heads/bug3325 (to get it,
git clone git://git.torproject.org/tor && cd tor && git remote add nickm git://git.torproject.org/nickm/tor && git fetch nickm && git checkout bug3325 )

comment:6 Changed 8 years ago by warms0x

My mistake, I misread the comment assuming that rransom had made it.

That said, the change looks sane to me and seems to cooperate (for whatever my opinion is worth)

comment:7 Changed 8 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Thanks for the review; merged to master for inclusion in 0.2.3.x.

comment:8 Changed 7 years ago by nickm

Keywords: tor-client added

comment:9 Changed 7 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.