Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#16674 closed defect (fixed)

Allow FQDNs ending with a single '.' in our SOCKS host name checks.

Reported by: yawning Owned by:
Priority: Medium Milestone: Tor: 0.2.7.x-final
Component: Core Tor/Tor Version: Tor: 0.2.6.2-alpha
Severity: Keywords: tor-core, socks, tbb-5.0a4, TorBrowserTeam201507R
Cc: mikeperry, gk, starlight.2015q2@… Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Brought up as part of #16430. A single . is allowed by RFC 3986 ("URI Generic Syntax") in URIs to specify that a domain name is absolute rather than relative. In practice not many sites use this, but right now since we unilaterally reject the syntax, it breaks things.

Relevant portion of RFC 3986:

   Such a name consists of a sequence of domain labels separated by ".",
   each domain label starting and ending with an alphanumeric character
   and possibly also containing "-" characters.  The rightmost domain
   label of a fully qualified domain name in DNS may be followed by a
   single "." and should be if it is necessary to distinguish between
   the complete domain name and some local domain.

RFC 1928 specifies that all DOMAINNAME type addresses are fully-qualified domain name(s), so I personally think that the . is redundant, but accepting it will increase compatibility, and being somewhat tolerant here is a good thing.

Child Tickets

Change History (6)

comment:1 Changed 4 years ago by yawning

Status: newneeds_review

comment:2 Changed 4 years ago by yawning

Cc: mikeperry gk added
Keywords: tbb-5.0a4 TorBrowserTeam201507R added

Added the Tor Browser folks/Tags, since they cherrypicked #16430, and they may want this assuming I didn't mess up (similar risk, similarly tested).

comment:3 Changed 4 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merged! TBB can cherry-pick it if they like it.

comment:4 Changed 4 years ago by starlight

Cc: starlight.2015q2@… added

Active link that may be used to test both this and #16430:

http://www.nytimes.com/interactive/2015/07/22/arts/dance/20150726-vogue.html

Should last for at least a week. JavaScript must be enabled.

comment:5 Changed 4 years ago by yawning

That url seems to load fine with master, so hopefully this is fixed once and for all, or at least till someone else decides to start serving interesting A records that need workarounds.

comment:6 Changed 4 years ago by starlight

Applied

https://gitweb.torproject.org/tor.git/commit/?id=da6aa7bfa5014b980a93b38024d16b32720dc67a

to 0.2.6.10 with

https://gitweb.torproject.org/tor.git/commit/?id=3f336966a264d7cd7c6dab08fb85d85273f06d68

already in tree, rebuilt deployed and tested.

Works no problem, no log warnings with the two NYT links
from these tickets as well as another one that
popped up today.

Note: See TracTickets for help on using tickets.