Bug 1228457 create pref to allow blocking .onion at dns level rfc 7687 r=valentin.gosu

modules/libpref/init/all.js

@@ -1695,6 +1695,13 @@ pref("network.dnsCacheExpirationGracePeriod", 60);
 // This preference can be used to turn off DNS prefetch.
 pref("network.dns.disablePrefetch", false);
 
+// This preference controls whether .onion hostnames are
+// rejected before being given to DNS. RFC 7686
+pref("network.dns.blockDotOnion", true);
+
+// These domains are treated as localhost equivalent
+pref("network.dns.localDomains", "");
+
 // Contols whether or not "localhost" should resolve when offline
 pref("network.dns.offline-localhost", true);
 

netwerk/dns/nsDNSService2.cpp

@@ -50,6 +50,7 @@ static const char kPrefDnsCacheGrace[]       = "network.dnsCacheExpirationGraceP
 static const char kPrefIPv4OnlyDomains[]     = "network.dns.ipv4OnlyDomains";
 static const char kPrefDisableIPv6[]         = "network.dns.disableIPv6";
 static const char kPrefDisablePrefetch[]     = "network.dns.disablePrefetch";
+static const char kPrefBlockDotOnion[]       = "network.dns.blockDotOnion";
 static const char kPrefDnsLocalDomains[]     = "network.dns.localDomains";
 static const char kPrefDnsOfflineLocalhost[] = "network.dns.offline-localhost";
 static const char kPrefDnsNotifyResolution[] = "network.dns.notifyResolution";
@@ -543,6 +544,7 @@ nsDNSService::Init()
     bool     disableIPv6      = false;
     bool     offlineLocalhost = true;
     bool     disablePrefetch  = false;
+    bool     blockDotOnion    = true;
     int      proxyType        = nsIProtocolProxyService::PROXYCONFIG_DIRECT;
     bool     notifyResolution = false;
 
@@ -566,6 +568,7 @@ nsDNSService::Init()
         prefs->GetCharPref(kPrefDnsLocalDomains, getter_Copies(localDomains));
         prefs->GetBoolPref(kPrefDnsOfflineLocalhost, &offlineLocalhost);
         prefs->GetBoolPref(kPrefDisablePrefetch, &disablePrefetch);
+        prefs->GetBoolPref(kPrefBlockDotOnion, &blockDotOnion);
 
         // If a manual proxy is in use, disable prefetch implicitly
         prefs->GetIntPref("network.proxy.type", &proxyType);
@@ -585,6 +588,7 @@ nsDNSService::Init()
             prefs->AddObserver(kPrefDisableIPv6, this, false);
             prefs->AddObserver(kPrefDnsOfflineLocalhost, this, false);
             prefs->AddObserver(kPrefDisablePrefetch, this, false);
+            prefs->AddObserver(kPrefBlockDotOnion, this, false);
             prefs->AddObserver(kPrefDnsNotifyResolution, this, false);
 
             // Monitor these to see if there is a change in proxy configuration
@@ -618,6 +622,7 @@ nsDNSService::Init()
         mIPv4OnlyDomains = ipv4OnlyDomains; // exchanges buffer ownership
         mOfflineLocalhost = offlineLocalhost;
         mDisableIPv6 = disableIPv6;
+        mBlockDotOnion = blockDotOnion;
 
         // Disable prefetching either by explicit preference or if a manual proxy is configured 
         mDisablePrefetch = disablePrefetch || (proxyType == nsIProtocolProxyService::PROXYCONFIG_MANUAL);
@@ -698,22 +703,32 @@ nsDNSService::SetPrefetchEnabled(bool inVal)
     return NS_OK;
 }
 
-static inline bool PreprocessHostname(bool              aLocalDomain,
+nsresult
+nsDNSService::PreprocessHostname(bool              aLocalDomain,
                                  const nsACString &aInput,
                                  nsIIDNService    *aIDN,
                                  nsACString       &aACE)
 {
+    // Enforce RFC 7686
+    if (mBlockDotOnion &&
+        StringEndsWith(aInput, NS_LITERAL_CSTRING(".onion"))) {
+        return NS_ERROR_UNKNOWN_HOST;
+    }
+
     if (aLocalDomain) {
         aACE.AssignLiteral("localhost");
-        return true;
+        return NS_OK;
     }
 
     if (!aIDN || IsASCII(aInput)) {
         aACE = aInput;
-        return true;
+        return NS_OK;
     }
 
-    return IsUTF8(aInput) && NS_SUCCEEDED(aIDN->ConvertUTF8toACE(aInput, aACE));
+    if (!(IsUTF8(aInput) && NS_SUCCEEDED(aIDN->ConvertUTF8toACE(aInput, aACE)))) {
+        return NS_ERROR_FAILURE;
+    }
+    return NS_OK;
 }
 
 NS_IMETHODIMP
@@ -760,8 +775,10 @@ nsDNSService::AsyncResolveExtended(const nsACString  &aHostname,
         return NS_ERROR_OFFLINE;
 
     nsCString hostname;
-    if (!PreprocessHostname(localDomain, aHostname, idn, hostname))
+    nsresult rv = PreprocessHostname(localDomain, aHostname, idn, hostname);
-        return NS_ERROR_FAILURE;
+    if (NS_FAILED(rv)) {
+        return rv;
+    }
 
     if (mOffline &&
         (!mOfflineLocalhost || !hostname.LowerCaseEqualsASCII("localhost"))) {
@@ -791,9 +808,8 @@ nsDNSService::AsyncResolveExtended(const nsACString  &aHostname,
 
     // addref for resolver; will be released when OnLookupComplete is called.
     NS_ADDREF(req);
-    nsresult rv = res->ResolveHost(req->mHost.get(), flags, af,
+    rv = res->ResolveHost(req->mHost.get(), flags, af,
-                                   req->mNetworkInterface.get(),
+                          req->mNetworkInterface.get(), req);
-                                   req);
     if (NS_FAILED(rv)) {
         NS_RELEASE(req);
         NS_RELEASE(*result);
@@ -837,8 +853,10 @@ nsDNSService::CancelAsyncResolveExtended(const nsACString  &aHostname,
         return NS_ERROR_OFFLINE;
 
     nsCString hostname;
-    if (!PreprocessHostname(localDomain, aHostname, idn, hostname))
+    nsresult rv = PreprocessHostname(localDomain, aHostname, idn, hostname);
-        return NS_ERROR_FAILURE;
+    if (NS_FAILED(rv)) {
+        return rv;
+    }
 
     uint16_t af = GetAFForLookup(hostname, aFlags);
 
@@ -872,8 +890,10 @@ nsDNSService::Resolve(const nsACString &aHostname,
     NS_ENSURE_TRUE(res, NS_ERROR_OFFLINE);
 
     nsCString hostname;
-    if (!PreprocessHostname(localDomain, aHostname, idn, hostname))
+    nsresult rv = PreprocessHostname(localDomain, aHostname, idn, hostname);
-        return NS_ERROR_FAILURE;
+    if (NS_FAILED(rv)) {
+        return rv;
+    }
 
     if (mOffline &&
         (!mOfflineLocalhost || !hostname.LowerCaseEqualsASCII("localhost"))) {
@@ -897,7 +917,7 @@ nsDNSService::Resolve(const nsACString &aHostname,
 
     uint16_t af = GetAFForLookup(hostname, flags);
 
-    nsresult rv = res->ResolveHost(hostname.get(), flags, af, "", &syncReq);
+    rv = res->ResolveHost(hostname.get(), flags, af, "", &syncReq);
     if (NS_SUCCEEDED(rv)) {
         // wait for result
         while (!syncReq.mDone)

netwerk/dns/nsDNSService2.h

@@ -43,6 +43,11 @@ private:
 
     uint16_t GetAFForLookup(const nsACString &host, uint32_t flags);
 
+    nsresult PreprocessHostname(bool              aLocalDomain,
+                                const nsACString &aInput,
+                                nsIIDNService    *aIDN,
+                                nsACString       &aACE);
+
     RefPtr<nsHostResolver>  mResolver;
     nsCOMPtr<nsIIDNService>   mIDN;
 
@@ -55,6 +60,7 @@ private:
     nsAdoptingCString                         mIPv4OnlyDomains;
     bool                                      mDisableIPv6;
     bool                                      mDisablePrefetch;
+    bool                                      mBlockDotOnion;
     bool                                      mFirstTime;
     bool                                      mOffline;
     bool                                      mNotifyResolution;

netwerk/test/unit/test_dns_onion.js

@@ -0,0 +1,70 @@
+var dns = Cc["@mozilla.org/network/dns-service;1"].getService(Ci.nsIDNSService);
+var threadManager = Cc["@mozilla.org/thread-manager;1"].getService(Ci.nsIThreadManager);
+var mainThread = threadManager.currentThread;
+
+var onionPref;
+var localdomainPref;
+var prefs = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch);
+
+// check that we don't lookup .onion
+var listenerBlock = {
+  onLookupComplete: function(inRequest, inRecord, inStatus) {
+    do_check_false(Components.isSuccessCode(inStatus));
+    do_test_dontBlock();
+  },
+  QueryInterface: function(aIID) {
+    if (aIID.equals(Ci.nsIDNSListener) ||
+        aIID.equals(Ci.nsISupports)) {
+      return this;
+    }
+    throw Cr.NS_ERROR_NO_INTERFACE;
+  }
+};
+
+// check that we do lookup .onion (via pref)
+var listenerDontBlock = {
+  onLookupComplete: function(inRequest, inRecord, inStatus) {
+    var answer = inRecord.getNextAddrAsString();
+    do_check_true(answer == "127.0.0.1" || answer == "::1");
+    all_done();
+  },
+  QueryInterface: function(aIID) {
+    if (aIID.equals(Ci.nsIDNSListener) ||
+        aIID.equals(Ci.nsISupports)) {
+      return this;
+    }
+    throw Cr.NS_ERROR_NO_INTERFACE;
+  }
+};
+
+function do_test_dontBlock() {
+  prefs.setBoolPref("network.dns.blockDotOnion", false);
+  dns.asyncResolve("private.onion", 0, listenerDontBlock, mainThread);
+}
+
+function do_test_block() {
+  prefs.setBoolPref("network.dns.blockDotOnion", true);
+  try {
+    dns.asyncResolve("private.onion", 0, listenerBlock, mainThread);
+  } catch (e) {
+    // it is ok for this negative test to fail fast
+    do_check_true(true);
+    do_test_dontBlock();
+  }
+}
+
+function all_done() {
+  // reset locally modified prefs
+  prefs.setCharPref("network.dns.localDomains", localdomainPref);
+  prefs.setBoolPref("network.dns.blockDotOnion", onionPref);
+  do_test_finished();
+}
+
+function run_test() {
+  onionPref = prefs.getBoolPref("network.dns.blockDotOnion");
+  localdomainPref = prefs.getCharPref("network.dns.localDomains");
+  prefs.setCharPref("network.dns.localDomains", "private.onion");
+  do_test_block();
+  do_test_pending();
+}
+

netwerk/test/unit/xpcshell.ini

@@ -187,6 +187,7 @@ skip-if = bits != 32
 [test_data_protocol.js]
 [test_dns_service.js]
 [test_dns_offline.js]
+[test_dns_onion.js]
 [test_dns_localredirect.js]
 [test_dns_proxy_bypass.js]
 [test_duplicate_headers.js]