Bug 1074642 - When multiple HSTS headers are received, only consider the first. draft
authorAlex Gaynor <agaynor@mozilla.com>
Thu, 20 Apr 2017 10:03:53 -0400
changeset 566515 f1c5485049a792a0771f0ba7b6b09e68228a8f74
parent 566404 c8198aa6e7677e90cc7f1e2df0a14a5cc2719055
child 625342 7e3eb0c2f255c220b2badf144beace525132db4d
push id55246
push userbmo:agaynor@mozilla.com
push dateFri, 21 Apr 2017 18:15:41 +0000
bugs1074642
milestone55.0a1
Bug 1074642 - When multiple HSTS headers are received, only consider the first. This implements a plaintext reading of RFC 6797, which says to only consider the first, however it slightly conflicts with RFC 7230, which says that sending multiple headers which can't be merged is illegal (except for a specific whitelist which HSTS isn't in). Chrome also implements HSTS using RFC 6797's description of the parsing algorithm. r=mcmanus,keeler MozReview-Commit-ID: E06uIk2IcEK
netwerk/protocol/http/nsHttpAtomList.h
netwerk/protocol/http/nsHttpHeaderArray.cpp
netwerk/protocol/http/nsHttpHeaderArray.h
netwerk/test/gtest/TestHeaders.cpp
netwerk/test/gtest/moz.build
--- a/netwerk/protocol/http/nsHttpAtomList.h
+++ b/netwerk/protocol/http/nsHttpAtomList.h
@@ -72,16 +72,17 @@ HTTP_ATOM(Proxy_Connection,          "Pr
 HTTP_ATOM(Range,                     "Range")
 HTTP_ATOM(Referer,                   "Referer")
 HTTP_ATOM(Retry_After,               "Retry-After")
 HTTP_ATOM(Server,                    "Server")
 HTTP_ATOM(Service_Worker_Allowed,    "Service-Worker-Allowed")
 HTTP_ATOM(Set_Cookie,                "Set-Cookie")
 HTTP_ATOM(Set_Cookie2,               "Set-Cookie2")
 HTTP_ATOM(Status_URI,                "Status-URI")
+HTTP_ATOM(Strict_Transport_Security, "Strict-Transport-Security")
 HTTP_ATOM(TE,                        "TE")
 HTTP_ATOM(Title,                     "Title")
 HTTP_ATOM(Timeout,                   "Timeout")
 HTTP_ATOM(Trailer,                   "Trailer")
 HTTP_ATOM(Transfer_Encoding,         "Transfer-Encoding")
 HTTP_ATOM(URI,                       "URI")
 HTTP_ATOM(Upgrade,                   "Upgrade")
 HTTP_ATOM(User_Agent,                "User-Agent")
--- a/netwerk/protocol/http/nsHttpHeaderArray.cpp
+++ b/netwerk/protocol/http/nsHttpHeaderArray.cpp
@@ -49,17 +49,17 @@ nsHttpHeaderArray::SetHeader(nsHttpAtom 
     }
 
     MOZ_ASSERT(!entry || variety != eVarietyRequestDefault,
                "Cannot set default entry which overrides existing entry!");
     if (!entry) {
         return SetHeader_internal(header, value, variety);
     } else if (merge && !IsSingletonHeader(header)) {
         return MergeHeader(header, entry, value, variety);
-    } else {
+    } else if (!IsIgnoreMultipleHeader(header)) {
         // Replace the existing string with the new value
         if (entry->variety == eVarietyResponseNetOriginalAndResponse) {
             MOZ_ASSERT(variety == eVarietyResponse);
             entry->variety = eVarietyResponseNetOriginal;
             return SetHeader_internal(header, value, variety);
         } else {
             entry->value = value;
             entry->variety = variety;
@@ -146,17 +146,17 @@ nsHttpHeaderArray::SetHeaderFromNet(nsHt
         if (NS_FAILED(rv)) {
             return rv;
         }
         if (response) {
             rv = SetHeader_internal(header, value,
                                     eVarietyResponseNetOriginal);
         }
         return rv;
-    } else {
+    } else if (!IsIgnoreMultipleHeader(header)) {
         // Multiple instances of non-mergeable header received from network
         // - ignore if same value
         if (!entry->value.Equals(value)) {
             if (IsSuspectDuplicateHeader(header)) {
                 // reply may be corrupt/hacked (ex: CLRF injection attacks)
                 return NS_ERROR_CORRUPTED_CONTENT;
             } // else silently drop value: keep value from 1st header seen
             LOG(("Header %s silently dropped as non mergeable header\n",
--- a/netwerk/protocol/http/nsHttpHeaderArray.h
+++ b/netwerk/protocol/http/nsHttpHeaderArray.h
@@ -146,16 +146,18 @@ private:
                                       const nsACString &value,
                                       HeaderVariety variety);
     MOZ_MUST_USE nsresult SetHeader_internal(nsHttpAtom header,
                                              const nsACString &value,
                                              HeaderVariety variety);
 
     // Header cannot be merged: only one value possible
     bool    IsSingletonHeader(nsHttpAtom header);
+    // Header cannot be merged, and subsequent values should be ignored
+    bool    IsIgnoreMultipleHeader(nsHttpAtom header);
     // For some headers we want to track empty values to prevent them being
     // combined with non-empty ones as a CRLF attack vector
     bool    TrackEmptyHeader(nsHttpAtom header);
 
     // Subset of singleton headers: should never see multiple, different
     // instances of these, else something fishy may be going on (like CLRF
     // injection)
     bool    IsSuspectDuplicateHeader(nsHttpAtom header);
@@ -217,17 +219,32 @@ nsHttpHeaderArray::IsSingletonHeader(nsH
            header == nsHttp::Referer                     ||
            header == nsHttp::Host                        ||
            header == nsHttp::Authorization               ||
            header == nsHttp::Proxy_Authorization         ||
            header == nsHttp::If_Modified_Since           ||
            header == nsHttp::If_Unmodified_Since         ||
            header == nsHttp::From                        ||
            header == nsHttp::Location                    ||
-           header == nsHttp::Max_Forwards;
+           header == nsHttp::Max_Forwards                ||
+           // Ignore-multiple-headers are singletons in the sense that they
+           // shouldn't be merged.
+           IsIgnoreMultipleHeader(header);
+}
+
+// These are headers for which, in the presence of multiple values, we only
+// consider the first.
+inline bool nsHttpHeaderArray::IsIgnoreMultipleHeader(nsHttpAtom header)
+{
+    // https://tools.ietf.org/html/rfc6797#section-8:
+    //
+    //     If a UA receives more than one STS header field in an HTTP
+    //     response message over secure transport, then the UA MUST process
+    //     only the first such header field.
+    return header == nsHttp::Strict_Transport_Security;
 }
 
 inline bool
 nsHttpHeaderArray::TrackEmptyHeader(nsHttpAtom header)
 {
     return header == nsHttp::Content_Length ||
            header == nsHttp::Location ||
            header == nsHttp::Access_Control_Allow_Origin;
new file mode 100644
--- /dev/null
+++ b/netwerk/test/gtest/TestHeaders.cpp
@@ -0,0 +1,29 @@
+#include "gtest/gtest.h"
+
+#include "nsHttpHeaderArray.h"
+
+
+TEST(TestHeaders, DuplicateHSTS) {
+    // When the Strict-Transport-Security header is sent multiple times, its
+    // effective value is the value of the first item. It is not merged as other
+    // headers are.
+    mozilla::net::nsHttpHeaderArray headers;
+    nsresult rv = headers.SetHeaderFromNet(
+        mozilla::net::nsHttp::Strict_Transport_Security, NS_LITERAL_CSTRING("max-age=360"), true
+    );
+    ASSERT_EQ(rv, NS_OK);
+
+    nsAutoCString h;
+    rv = headers.GetHeader(mozilla::net::nsHttp::Strict_Transport_Security, h);
+    ASSERT_EQ(rv, NS_OK);
+    ASSERT_EQ(h.get(), "max-age=360");
+
+    rv = headers.SetHeaderFromNet(
+        mozilla::net::nsHttp::Strict_Transport_Security, NS_LITERAL_CSTRING("max-age=720"), true
+    );
+    ASSERT_EQ(rv, NS_OK);
+
+    rv = headers.GetHeader(mozilla::net::nsHttp::Strict_Transport_Security, h);
+    ASSERT_EQ(rv, NS_OK);
+    ASSERT_EQ(h.get(), "max-age=360");
+}
--- a/netwerk/test/gtest/moz.build
+++ b/netwerk/test/gtest/moz.build
@@ -1,15 +1,16 @@
 # -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
 # vim: set filetype=python:
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 UNIFIED_SOURCES += [
+    'TestHeaders.cpp',
     'TestHttpAuthUtils.cpp',
     'TestProtocolProxyService.cpp',
     'TestStandardURL.cpp',
 ]
 
 include('/ipc/chromium/chromium-config.mozbuild')
 
 FINAL_LIBRARY = 'xul-gtest'