Title: [292266] trunk
Revision
292266
Author
pgrif...@igalia.com
Date
2022-04-02 09:22:20 -0700 (Sat, 02 Apr 2022)

Log Message

CSP: Improve compatibility of source matching
https://bugs.webkit.org/show_bug.cgi?id=235873

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

Update expectation as passing.

* web-platform-tests/content-security-policy/connect-src/connect-src-websocket-self.sub-expected.txt:

Source/WebCore:

- Improved handling of protocol changes:
  - For host and self sources direct upgrades are allowed (ws->wss) (http->https already worked).
  - For self sources side grades are now allowed (http->ws).
  - For self sources upgrades are always allowed (*->https, *->wss).
This is documented here: https://www.w3.org/TR/CSP3/#match-url-to-source-_expression_

I also included some minor cleanups and adding of comments.

* page/csp/ContentSecurityPolicy.cpp:
(WebCore::ContentSecurityPolicy::updateSourceSelf):
(WebCore::ContentSecurityPolicy::protocolMatchesSelf const): Deleted.
* page/csp/ContentSecurityPolicy.h:
(WebCore::ContentSecurityPolicy::selfProtocol const):
* page/csp/ContentSecurityPolicySource.cpp:
(WebCore::ContentSecurityPolicySource::ContentSecurityPolicySource):
(WebCore::ContentSecurityPolicySource::matches const):
(WebCore::ContentSecurityPolicySource::schemeMatches const):
(WebCore::ContentSecurityPolicySource::portMatches const):
* page/csp/ContentSecurityPolicySource.h:
* page/csp/ContentSecurityPolicySourceList.cpp:
(WebCore::ContentSecurityPolicySourceList::isProtocolAllowedByStar const):
(WebCore::ContentSecurityPolicySourceList::parse):

Modified Paths

Diff

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (292265 => 292266)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2022-04-02 15:59:07 UTC (rev 292265)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2022-04-02 16:22:20 UTC (rev 292266)
@@ -1,3 +1,14 @@
+2022-04-02  Patrick Griffis  <pgrif...@igalia.com>
+
+        CSP: Improve compatibility of source matching
+        https://bugs.webkit.org/show_bug.cgi?id=235873
+
+        Reviewed by Darin Adler.
+
+        Update expectation as passing.
+
+        * web-platform-tests/content-security-policy/connect-src/connect-src-websocket-self.sub-expected.txt:
+
 2022-04-01  Tim Nguyen  <n...@apple.com>
 
         [css-logical] Add support for block/inline CSS values for resize property

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/content-security-policy/connect-src/connect-src-websocket-self.sub-expected.txt (292265 => 292266)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/content-security-policy/connect-src/connect-src-websocket-self.sub-expected.txt	2022-04-02 15:59:07 UTC (rev 292265)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/content-security-policy/connect-src/connect-src-websocket-self.sub-expected.txt	2022-04-02 16:22:20 UTC (rev 292266)
@@ -1,3 +1,3 @@
 
-FAIL Expecting logs: ["allowed", "allowed"] assert_unreached: unexpected log: blocked Reached unreachable code
+PASS Expecting logs: ["allowed", "allowed"]
 

Modified: trunk/Source/WebCore/ChangeLog (292265 => 292266)


--- trunk/Source/WebCore/ChangeLog	2022-04-02 15:59:07 UTC (rev 292265)
+++ trunk/Source/WebCore/ChangeLog	2022-04-02 16:22:20 UTC (rev 292266)
@@ -1,3 +1,33 @@
+2022-04-02  Patrick Griffis  <pgrif...@igalia.com>
+
+        CSP: Improve compatibility of source matching
+        https://bugs.webkit.org/show_bug.cgi?id=235873
+
+        Reviewed by Darin Adler.
+
+        - Improved handling of protocol changes:
+          - For host and self sources direct upgrades are allowed (ws->wss) (http->https already worked).
+          - For self sources side grades are now allowed (http->ws).
+          - For self sources upgrades are always allowed (*->https, *->wss).          
+        This is documented here: https://www.w3.org/TR/CSP3/#match-url-to-source-_expression_
+
+        I also included some minor cleanups and adding of comments.
+
+        * page/csp/ContentSecurityPolicy.cpp:
+        (WebCore::ContentSecurityPolicy::updateSourceSelf):
+        (WebCore::ContentSecurityPolicy::protocolMatchesSelf const): Deleted.
+        * page/csp/ContentSecurityPolicy.h:
+        (WebCore::ContentSecurityPolicy::selfProtocol const):
+        * page/csp/ContentSecurityPolicySource.cpp:
+        (WebCore::ContentSecurityPolicySource::ContentSecurityPolicySource):
+        (WebCore::ContentSecurityPolicySource::matches const):
+        (WebCore::ContentSecurityPolicySource::schemeMatches const):
+        (WebCore::ContentSecurityPolicySource::portMatches const):
+        * page/csp/ContentSecurityPolicySource.h:
+        * page/csp/ContentSecurityPolicySourceList.cpp:
+        (WebCore::ContentSecurityPolicySourceList::isProtocolAllowedByStar const):
+        (WebCore::ContentSecurityPolicySourceList::parse):
+
 2022-04-02  Andres Gonzalez  <andresg...@apple.com>
 
         Expose AXObjectCache::treeData to the UI process.

Modified: trunk/Source/WebCore/page/csp/ContentSecurityPolicy.cpp (292265 => 292266)


--- trunk/Source/WebCore/page/csp/ContentSecurityPolicy.cpp	2022-04-02 15:59:07 UTC (rev 292265)
+++ trunk/Source/WebCore/page/csp/ContentSecurityPolicy.cpp	2022-04-02 16:22:20 UTC (rev 292266)
@@ -239,8 +239,8 @@
 
 void ContentSecurityPolicy::updateSourceSelf(const SecurityOrigin& securityOrigin)
 {
-    m_selfSourceProtocol = securityOrigin.protocol();
-    m_selfSource = makeUnique<ContentSecurityPolicySource>(*this, m_selfSourceProtocol, securityOrigin.host(), securityOrigin.port(), emptyString(), false, false);
+    m_selfSourceProtocol = securityOrigin.protocol().convertToASCIILowercase();
+    m_selfSource = makeUnique<ContentSecurityPolicySource>(*this, m_selfSourceProtocol, securityOrigin.host(), securityOrigin.port(), emptyString(), false, false, IsSelfSource::Yes);
 }
 
 void ContentSecurityPolicy::applyPolicyToScriptExecutionContext()
@@ -295,13 +295,6 @@
     return false;
 }
 
-bool ContentSecurityPolicy::protocolMatchesSelf(const URL& url) const
-{
-    if (equalLettersIgnoringASCIICase(m_selfSourceProtocol, "http"))
-        return url.protocolIsInHTTPFamily();
-    return equalIgnoringASCIICase(url.protocol(), m_selfSourceProtocol);
-}
-
 template<typename Predicate, typename... Args>
 typename std::enable_if<!std::is_convertible<Predicate, ContentSecurityPolicy::ViolatedDirectiveCallback>::value, bool>::type ContentSecurityPolicy::allPoliciesWithDispositionAllow(Disposition disposition, Predicate&& predicate, Args&&... args) const
 {

Modified: trunk/Source/WebCore/page/csp/ContentSecurityPolicy.h (292265 => 292266)


--- trunk/Source/WebCore/page/csp/ContentSecurityPolicy.h	2022-04-02 15:59:07 UTC (rev 292265)
+++ trunk/Source/WebCore/page/csp/ContentSecurityPolicy.h	2022-04-02 16:22:20 UTC (rev 292266)
@@ -176,7 +176,7 @@
     }
 
     // Used by ContentSecurityPolicySource
-    bool protocolMatchesSelf(const URL&) const;
+    const String& selfProtocol() const { return m_selfSourceProtocol; };
 
     void setUpgradeInsecureRequests(bool);
     bool upgradeInsecureRequests() const { return m_upgradeInsecureRequests; }

Modified: trunk/Source/WebCore/page/csp/ContentSecurityPolicySource.cpp (292265 => 292266)


--- trunk/Source/WebCore/page/csp/ContentSecurityPolicySource.cpp	2022-04-02 15:59:07 UTC (rev 292265)
+++ trunk/Source/WebCore/page/csp/ContentSecurityPolicySource.cpp	2022-04-02 16:22:20 UTC (rev 292266)
@@ -34,7 +34,7 @@
 
 namespace WebCore {
 
-ContentSecurityPolicySource::ContentSecurityPolicySource(const ContentSecurityPolicy& policy, const String& scheme, const String& host, std::optional<uint16_t> port, const String& path, bool hostHasWildcard, bool portHasWildcard)
+ContentSecurityPolicySource::ContentSecurityPolicySource(const ContentSecurityPolicy& policy, const String& scheme, const String& host, std::optional<uint16_t> port, const String& path, bool hostHasWildcard, bool portHasWildcard, IsSelfSource isSelfSource)
     : m_policy(policy)
     , m_scheme(scheme)
     , m_host(host)
@@ -42,11 +42,13 @@
     , m_port(port)
     , m_hostHasWildcard(hostHasWildcard)
     , m_portHasWildcard(portHasWildcard)
+    , m_isSelfSource(isSelfSource == IsSelfSource::Yes)
 {
 }
 
 bool ContentSecurityPolicySource::matches(const URL& url, bool didReceiveRedirectResponse) const
 {
+    // https://www.w3.org/TR/CSP3/#match-url-to-source-_expression_.
     if (!schemeMatches(url))
         return false;
     if (isSchemeOnly())
@@ -56,11 +58,27 @@
 
 bool ContentSecurityPolicySource::schemeMatches(const URL& url) const
 {
-    if (m_scheme.isEmpty())
-        return m_policy.protocolMatchesSelf(url);
-    if (equalLettersIgnoringASCIICase(m_scheme, "http"))
-        return url.protocolIsInHTTPFamily();
-    return equalIgnoringASCIICase(url.protocol(), m_scheme);
+    // https://www.w3.org/TR/CSP3/#match-schemes.
+    const auto& scheme = m_scheme.isEmpty() ? m_policy.selfProtocol() : m_scheme;
+    auto urlScheme = url.protocol().convertToASCIILowercase();
+
+    if (scheme == urlScheme)
+        return true;
+
+    // host-sources can do direct-upgrades.
+    if (scheme == "http" && urlScheme == "https")
+        return true;
+    if (scheme == "ws" && (urlScheme == "wss" || urlScheme == "https" || urlScheme == "http"))
+        return true;
+    if (scheme == "wss" && urlScheme == "https")
+        return true;
+
+    // self-sources can always upgrade to secure protocols and side-grade insecure protocols.
+    if ((m_isSelfSource
+        && ((urlScheme == "https" || urlScheme == "wss") || (scheme == "http" && urlScheme == "ws"))))
+        return true;
+
+    return false;
 }
 
 static bool wildcardMatches(StringView host, const String& hostWithWildcard)
@@ -103,7 +121,12 @@
     if (port == m_port)
         return true;
 
-    if ((m_port && WTF::isDefaultPortForProtocol(m_port.value(), "http")) && ((!port && url.protocolIs("https")) || (port && WTF::isDefaultPortForProtocol(port.value(), "https"))))
+    // host-source and self-source allows upgrading to a more secure scheme which allows for different ports.
+    auto defaultSecurePort = WTF::defaultPortForProtocol("https").value_or(443);
+    auto defaultInsecurePort = WTF::defaultPortForProtocol("http").value_or(80);
+    bool isUpgradeSecure = (port == defaultSecurePort) || (!port && (url.protocol() == "https" || url.protocol() == "wss"));
+    bool isCurrentUpgradable = (m_port == defaultInsecurePort) || (m_scheme == "http" && (!m_port || m_port == defaultSecurePort));
+    if (isUpgradeSecure && isCurrentUpgradable)
         return true;
 
     if (!port)

Modified: trunk/Source/WebCore/page/csp/ContentSecurityPolicySource.h (292265 => 292266)


--- trunk/Source/WebCore/page/csp/ContentSecurityPolicySource.h	2022-04-02 15:59:07 UTC (rev 292265)
+++ trunk/Source/WebCore/page/csp/ContentSecurityPolicySource.h	2022-04-02 16:22:20 UTC (rev 292266)
@@ -33,10 +33,12 @@
 class ContentSecurityPolicy;
 struct SecurityOriginData;
 
+enum class IsSelfSource : bool { No, Yes };
+
 class ContentSecurityPolicySource {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    ContentSecurityPolicySource(const ContentSecurityPolicy&, const String& scheme, const String& host, std::optional<uint16_t> port, const String& path, bool hostHasWildcard, bool portHasWildcard);
+    ContentSecurityPolicySource(const ContentSecurityPolicy&, const String& scheme, const String& host, std::optional<uint16_t> port, const String& path, bool hostHasWildcard, bool portHasWildcard, IsSelfSource);
 
     bool matches(const URL&, bool didReceiveRedirectResponse = false) const;
 
@@ -57,6 +59,7 @@
 
     bool m_hostHasWildcard;
     bool m_portHasWildcard;
+    bool m_isSelfSource;
 };
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp (292265 => 292266)


--- trunk/Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp	2022-04-02 15:59:07 UTC (rev 292265)
+++ trunk/Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp	2022-04-02 16:22:20 UTC (rev 292266)
@@ -120,9 +120,10 @@
     if (m_policy.allowContentSecurityPolicySourceStarToMatchAnyProtocol())
         return true;
 
-    // Although not allowed by the Content Security Policy Level 3 spec., we allow a data URL to match
+    // This is counter to the CSP3 spec which only allows HTTPS but Chromium also allows it.
+    bool isAllowed = url.protocolIsInHTTPFamily() || url.protocolIs("ws") || url.protocolIs("wss") || url.protocolIs(m_policy.selfProtocol());
+    // Also not allowed by the Content Security Policy Level 3 spec., we allow a data URL to match
     // "img-src *" and either a data URL or blob URL to match "media-src *" for web compatibility.
-    bool isAllowed = url.protocolIsInHTTPFamily() || url.protocolIs("ws") || url.protocolIs("wss") || m_policy.protocolMatchesSelf(url);
     if (equalIgnoringASCIICase(m_directiveName, ContentSecurityPolicyDirectiveNames::imgSrc))
         isAllowed |= url.protocolIsData();
     else if (equalIgnoringASCIICase(m_directiveName, ContentSecurityPolicyDirectiveNames::mediaSrc))
@@ -269,7 +270,7 @@
             if (isCSPDirectiveName(source->host.value))
                 m_policy.reportDirectiveAsSourceExpression(m_directiveName, source->host.value);
             if (isValidSourceForExtensionMode(source.value()))
-                m_list.append(ContentSecurityPolicySource(m_policy, source->scheme.toString(), source->host.value.toString(), source->port.value, source->path, source->host.hasWildcard, source->port.hasWildcard));
+                m_list.append(ContentSecurityPolicySource(m_policy, source->scheme.convertToASCIILowercase(), source->host.value.toString(), source->port.value, source->path, source->host.hasWildcard, source->port.hasWildcard, IsSelfSource::No));
         } else
             m_policy.reportInvalidSourceExpression(m_directiveName, String(beginSource, buffer.position() - beginSource));
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to