[tor-commits] [tor-browser-bundle/master] Bug 16674: Allow trailing '.' in FQDNs

[mikeperry at torproject.org]

commit 3ad5030cfbdf3686defc63f4896fdd2e325ed742
Author: Mike Perry <mikeperry-git at torproject.org>
Date:   Mon Jul 27 08:07:57 2015 -0700

    Bug 16674: Allow trailing '.' in FQDNs
    
    Backport of Tor patch.
---
 gitian/descriptors/linux/gitian-tor.yml   |    2 +
 gitian/descriptors/mac/gitian-tor.yml     |    2 +
 gitian/descriptors/windows/gitian-tor.yml |    2 +
 gitian/patches/bug16674.patch             |   74 +++++++++++++++++++++++++++++
 4 files changed, 80 insertions(+)

diff --git a/gitian/descriptors/linux/gitian-tor.yml b/gitian/descriptors/linux/gitian-tor.yml
index e62a1f0..bfd0e18 100644
--- a/gitian/descriptors/linux/gitian-tor.yml
+++ b/gitian/descriptors/linux/gitian-tor.yml
@@ -23,6 +23,7 @@ files:
 - "bug8405.patch"
 - "bug15482.patch"
 - "bug16430.patch"
+- "bug16674.patch"
 - "dzip.sh"
 - "openssl-linux32-utils.zip"
 - "openssl-linux64-utils.zip"
@@ -85,6 +86,7 @@ script: |
   else
       git am ~/build/bug15482.patch
       git am ~/build/bug16430.patch
+      git am ~/build/bug16674.patch
   fi
   mkdir -p $OUTDIR/src
   #git archive HEAD | tar -x -C $OUTDIR/src
diff --git a/gitian/descriptors/mac/gitian-tor.yml b/gitian/descriptors/mac/gitian-tor.yml
index d305ff7..c755851 100644
--- a/gitian/descriptors/mac/gitian-tor.yml
+++ b/gitian/descriptors/mac/gitian-tor.yml
@@ -19,6 +19,7 @@ files:
 - "bug8405.patch"
 - "bug15482.patch"
 - "bug16430.patch"
+- "bug16674.patch"
 - "apple-uni-sdk-10.6_20110407-0.flosoft1_i386.deb"
 - "multiarch-darwin11-cctools127.2-gcc42-5666.3-llvmgcc42-2336.1-Linux-120724.tar.xz"
 - "dzip.sh"
@@ -63,6 +64,7 @@ script: |
   else
       git am ~/build/bug15482.patch
       git am ~/build/bug16430.patch
+      git am ~/build/bug16674.patch
   fi
   mkdir -p $OUTDIR/src
   #git archive HEAD | tar -x -C $OUTDIR/src
diff --git a/gitian/descriptors/windows/gitian-tor.yml b/gitian/descriptors/windows/gitian-tor.yml
index 22fda90..a6be6b4 100644
--- a/gitian/descriptors/windows/gitian-tor.yml
+++ b/gitian/descriptors/windows/gitian-tor.yml
@@ -19,6 +19,7 @@ files:
 - "bug8405.patch"
 - "bug15482.patch"
 - "bug16430.patch"
+- "bug16674.patch"
 - "binutils.tar.bz2"
 - "dzip.sh"
 - "mingw-w64-win32-utils.zip"
@@ -63,6 +64,7 @@ script: |
   else
       git am ~/build/bug15482.patch
       git am ~/build/bug16430.patch
+      git am ~/build/bug16674.patch
   fi
   mkdir -p $OUTDIR/src
   #git archive HEAD | tar -x -C $OUTDIR/src
diff --git a/gitian/patches/bug16674.patch b/gitian/patches/bug16674.patch
new file mode 100644
index 0000000..9497684
--- /dev/null
+++ b/gitian/patches/bug16674.patch
@@ -0,0 +1,74 @@
+From da6aa7bfa5014b980a93b38024d16b32720dc67a Mon Sep 17 00:00:00 2001
+From: Yawning Angel <yawning at schwanenlied.me>
+Date: Mon, 27 Jul 2015 12:58:40 +0000
+Subject: [PATCH] Allow a single trailing `.` when validating FQDNs from SOCKS.
+
+URI syntax (and DNS syntax) allows for a single trailing `.` to
+explicitly distinguish between a relative and absolute
+(fully-qualified) domain name. While this is redundant in that RFC 1928
+DOMAINNAME addresses are *always* fully-qualified, certain clients
+blindly pass the trailing `.` along in the request.
+
+Fixes bug 16674; bugfix on 0.2.6.2-alpha.
+---
+ changes/bug16674     |  5 +++++
+ src/common/util.c    |  6 ++++++
+ src/test/test_util.c | 12 ++++++++++++
+ 3 files changed, 23 insertions(+)
+ create mode 100644 changes/bug16674
+
+diff --git a/changes/bug16674 b/changes/bug16674
+new file mode 100644
+index 0000000..de55523
+--- /dev/null
++++ b/changes/bug16674
+@@ -0,0 +1,5 @@
++  o Minor features (client):
++    - Relax the validation done to hostnames in SOCKS5 requests, and allow
++      a single trailing '.' to cope with clients that pass FQDNs using that
++      syntax to explicitly indicate that the domain name is
++      fully-qualified. Fixes bug 16674; bugfix on 0.2.6.2-alpha.
+diff --git a/src/common/util.c b/src/common/util.c
+index 618e6a1..1aac4fc 100644
+--- a/src/common/util.c
++++ b/src/common/util.c
+@@ -1056,6 +1056,12 @@ string_is_valid_hostname(const char *string)
+       break;
+     }
+ 
++    /* Allow a single terminating '.' used rarely to indicate domains
++     * are FQDNs rather than relative. */
++    if ((c_sl_idx > 0) && (c_sl_idx + 1 == c_sl_len) && !*c) {
++      continue;
++    }
++
+     do {
+       if ((*c >= 'a' && *c <= 'z') ||
+           (*c >= 'A' && *c <= 'Z') ||
+diff --git a/src/test/test_util.c b/src/test/test_util.c
+index 0f64c26..2bffb17 100644
+--- a/src/test/test_util.c
++++ b/src/test/test_util.c
+@@ -4285,7 +4285,19 @@ test_util_hostname_validation(void *arg)
+   // comply with a ~30 year old standard.
+   tt_assert(string_is_valid_hostname("core3_euw1.fabrik.nytimes.com"));
+ 
++  // Firefox passes FQDNs with trailing '.'s  directly to the SOCKS proxy,
++  // which is redundant since the spec states DOMAINNAME addresses are fully
++  // qualified.  While unusual, this should be tollerated.
++  tt_assert(string_is_valid_hostname("core9_euw1.fabrik.nytimes.com."));
++  tt_assert(!string_is_valid_hostname("..washingtonpost.is.better.com"));
++  tt_assert(!string_is_valid_hostname("so.is..ft.com"));
++  tt_assert(!string_is_valid_hostname("..."));
++
+   // XXX: do we allow single-label DNS names?
++  // We shouldn't for SOCKS (spec says "contains a fully-qualified domain name"
++  // but only test pathologically malformed traling '.' cases for now.
++  tt_assert(!string_is_valid_hostname("."));
++  tt_assert(!string_is_valid_hostname(".."));
+ 
+   done:
+   return;
+-- 
+1.9.1
+