Issue #2070 - When multiple HSTS headers are received, only consider the first.

This implements a plain interpretations of RFC 6797, which says to only consider
the first HSTS header.
This 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),
so this situation should never occur in the first place (and would therefore not need
the explicit entry in RFC 6797).

It improves HSTS robustness dealing with non-compliant servers.

Resolves #2070

netwerk/protocol/http/nsHttpAtomList.h

@@ -79,6 +79,7 @@ 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")

netwerk/protocol/http/nsHttpHeaderArray.cpp

@@ -79,7 +79,7 @@ nsHttpHeaderArray::SetHeader(nsHttpAtom header,
         return SetHeader_internal(header, headerName, 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);
@@ -190,7 +190,7 @@ nsHttpHeaderArray::SetHeaderFromNet(nsHttpAtom header,
                                     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)) {

netwerk/protocol/http/nsHttpHeaderArray.h

@@ -160,6 +160,8 @@ private:
 
     // 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);
@@ -231,7 +233,22 @@ nsHttpHeaderArray::IsSingletonHeader(nsHttpAtom header)
            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

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 coalesced like
+    // 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");
+}

netwerk/test/gtest/moz.build

@@ -5,6 +5,7 @@
 
 UNIFIED_SOURCES += [
     'TestBase64Stream.cpp',
+    'TestHeaders.cpp',
     'TestProtocolProxyService.cpp',
     'TestStandardURL.cpp',
 ]