From 96d1e2766f852cc538900676a3ff47796db9f423 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Wed, 5 Jul 2023 19:27:03 +0200 Subject: [PATCH] [network] Prepare for requiring Authorization in CORS ACAH preflight The Authorization header with a JSON Web Token (JWT) can be sent via XMLHttpRequest without explicit authorization via Access-Control headers. According to the spec, this must always explicitly be mentioned in ACAH request headers and isn't allowed to be wildcarded. However, nobody currently obeys this rule and many websites are misconfigured because Chromium and Firefox always allowed it. This patch adds the more stricter code but keeps it behind an #ifdef 0 to be released later on if and when there is enough consensus on the web to obey this spec. This patch explicitly avoids the added complexity Mozilla added to educate web devs since our role in that respect is not significant. it's not preffed and it won't throw an explicit deprecation warning. See Mozilla bugs 1687364 and 1841019. --- netwerk/protocol/http/nsCORSListenerProxy.cpp | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/netwerk/protocol/http/nsCORSListenerProxy.cpp b/netwerk/protocol/http/nsCORSListenerProxy.cpp index 499c03094b..d2c37415e5 100644 --- a/netwerk/protocol/http/nsCORSListenerProxy.cpp +++ b/netwerk/protocol/http/nsCORSListenerProxy.cpp @@ -1344,7 +1344,7 @@ nsCORSPreflightListener::CheckPreflightRequestApproved(nsIRequest* aRequest) headerVal); nsTArray headers; nsCCharSeparatedTokenizer headerTokens(headerVal, ','); - bool allowAllHeaders = false; + bool wildcard = false; while(headerTokens.hasMoreTokens()) { const nsDependentCSubstring& header = headerTokens.nextToken(); if (header.IsEmpty()) { @@ -1356,19 +1356,31 @@ nsCORSPreflightListener::CheckPreflightRequestApproved(nsIRequest* aRequest) return NS_ERROR_DOM_BAD_URI; } if (header.EqualsLiteral("*") && !mWithCredentials) { - allowAllHeaders = true; + wildcard = true; } else { headers.AppendElement(header); } } - if (!allowAllHeaders) { - for (uint32_t i = 0; i < mPreflightHeaders.Length(); ++i) { - if (!headers.Contains(mPreflightHeaders[i], - nsCaseInsensitiveCStringArrayComparator())) { - LogBlockedRequest(aRequest, "CORSMissingAllowHeaderFromPreflight", - NS_ConvertUTF8toUTF16(mPreflightHeaders[i]).get()); - return NS_ERROR_DOM_BAD_URI; - } + for (uint32_t i = 0; i < mPreflightHeaders.Length(); ++i) { + if (wildcard + // Access-Control-Allow-Headers is '*', so we should skip these checks. +#if 0 + && !mPreflightHeaders[i].LowerCaseEqualsASCII("authorization") + // However, according to the spec, 'Authorization' isn't allowed to be + // wildcarded here and must always be explicitly mentioned. + // Fixme: Mainstream keeps this disabled because nobody obeys this rule. + // This should be flipped on when either mainstream does or when there's enough + // effort to make websites adhere to the spec, to keep our implementation + // in line with the consensus on the web. +#endif + ) { + continue; + } + if (!headers.Contains(mPreflightHeaders[i], + nsCaseInsensitiveCStringArrayComparator())) { + LogBlockedRequest(aRequest, "CORSMissingAllowHeaderFromPreflight", + NS_ConvertUTF8toUTF16(mPreflightHeaders[i]).get()); + return NS_ERROR_DOM_BAD_URI; } }