[tor-commits] [tor-browser/tor-browser-52.8.0esr-7.5-1] Bug 21537: Mark .onion cookies as secure

[gk at torproject.org]

commit 1f33ee1778b0ad0f696977fbcbae67f72d34b99f
Author: Georg Koppen <gk at torproject.org>
Date:   Fri Apr 13 16:59:24 2018 +0000

    Bug 21537: Mark .onion cookies as secure
---
 netwerk/cookie/nsCookieService.cpp | 60 ++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 28 deletions(-)

diff --git a/netwerk/cookie/nsCookieService.cpp b/netwerk/cookie/nsCookieService.cpp
index cf1d91e2df36..f9390d7ef093 100644
--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@@ -3112,6 +3112,26 @@ PathMatches(nsCookie* aCookie, const nsACString& aPath) {
   return true;
 }
 
+static bool
+IsSecureHost(nsIURI *aHostURI) {
+  bool isSecure = true;
+  // If SchemeIs fails, assume an insecure connection, to be on the safe side.
+  if (aHostURI && NS_FAILED(aHostURI->SchemeIs("https", &isSecure))) {
+    isSecure = false;
+  }
+  // In case HTTPS is not used check whether we have a .onion host in which
+  // case the cookie is secure, too.
+  if (aHostURI && !isSecure) {
+    nsresult rv;
+    nsAutoCString hostFromURI;
+    rv = aHostURI->GetAsciiHost(hostFromURI);
+    if (NS_SUCCEEDED(rv)) {
+      isSecure = StringEndsWith(hostFromURI, NS_LITERAL_CSTRING(".onion"));
+    }
+  }
+  return isSecure;
+}
+
 void
 nsCookieService::GetCookieStringInternal(nsIURI *aHostURI,
                                          bool aIsForeign,
@@ -3164,13 +3184,10 @@ nsCookieService::GetCookieStringInternal(nsIURI *aHostURI,
   // If it changes, please update that function, or file a bug for someone
   // else to do so.
 
-  // check if aHostURI is using an https secure protocol.
-  // if it isn't, then we can't send a secure cookie over the connection.
-  // if SchemeIs fails, assume an insecure connection, to be on the safe side
-  bool isSecure;
-  if (NS_FAILED(aHostURI->SchemeIs("https", &isSecure))) {
-    isSecure = false;
-  }
+  // Check if aHostURI is using the HTTPS protocol or if the domain is a
+  // .onion. If it isn't, then we can't send a secure cookie over the
+  // connection.
+  bool isSecure = IsSecureHost(aHostURI);
 
   nsCookie *cookie;
   AutoTArray<nsCookie*, 8> foundCookieList;
@@ -3316,20 +3333,10 @@ nsCookieService::SetCookieInternal(nsIURI                        *aHostURI,
   // so we can handle them separately.
   bool newCookie = ParseAttributes(aCookieHeader, cookieAttributes);
 
-  // Collect telemetry on how often secure cookies are set from non-secure
-  // origins, and vice-versa.
-  //
-  // 0 = nonsecure and "http:"
-  // 1 = nonsecure and "https:"
-  // 2 = secure and "http:"
-  // 3 = secure and "https:"
-  bool isHTTPS;
-  nsresult rv = aHostURI->SchemeIs("https", &isHTTPS);
-  if (NS_SUCCEEDED(rv)) {
-    Telemetry::Accumulate(Telemetry::COOKIE_SCHEME_SECURITY,
-                          ((cookieAttributes.isSecure)? 0x02 : 0x00) |
-                          ((isHTTPS)? 0x01 : 0x00));
-  }
+  // Check if aHostURI is using the HTTPS protocol or if the domain is a
+  // .onion. If it isn't, then we can't send a secure cookie over the
+  // connection.
+  bool isSecure = IsSecureHost(aHostURI);
 
   int64_t currentTimeInUsec = PR_Now();
 
@@ -3369,7 +3376,7 @@ nsCookieService::SetCookieInternal(nsIURI                        *aHostURI,
     return newCookie;
   }
   // magic prefix checks. MUST be run after CheckDomain() and CheckPath()
-  if (!CheckPrefixes(cookieAttributes, isHTTPS)) {
+  if (!CheckPrefixes(cookieAttributes, isSecure)) {
     COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, savedCookieHeader, "failed the prefix tests");
     return newCookie;
   }
@@ -3457,13 +3464,10 @@ nsCookieService::AddInternal(const nsCookieKey             &aKey,
     return;
   }
 
-  bool isSecure = true;
-  if (aHostURI && NS_FAILED(aHostURI->SchemeIs("https", &isSecure))) {
-    isSecure = false;
-  }
+  bool isSecure = IsSecureHost(aHostURI);
 
-  // If the new cookie is non-https and wants to set secure flag,
-  // browser have to ignore this new cookie.
+  // If the new cookie is non-https or not set by a .onion domain, and wants to
+  // set secure flag, browser have to ignore this new cookie.
   // (draft-ietf-httpbis-cookie-alone section 3.1)
   if (mLeaveSecureAlone && aCookie->IsSecure() && !isSecure) {
     COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, aCookieHeader,