commit c70454fd10efeb9f4cabc69f94a9a7a633c10174 Author: Georg Koppen <gk@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,