Commit 1dd689c0 authored by abarth@webkit.org's avatar abarth@webkit.org

2011-03-27 Adam Barth <abarth@webkit.org>

        Reviewed by Eric Seidel.

        Fix script-src redirect handling
        https://bugs.webkit.org/show_bug.cgi?id=57196

        Test both allow => disallow and disallow => allow redirect cases.
        Previously, we had incorrect expectations for one of the redirect
        cases.  Also, I've updated the policy syntax to match the default-src
        syntax.

        * http/tests/security/contentSecurityPolicy/script-src-redirect-expected.txt:
        * http/tests/security/contentSecurityPolicy/script-src-redirect.html:
2011-03-27  Adam Barth  <abarth@webkit.org>

        Reviewed by Eric Seidel.

        Fix script-src redirect handling
        https://bugs.webkit.org/show_bug.cgi?id=57196

        Resource-loading requirements in CSP apply to each hop in the redirect
        chain.  To make that work properly, we need to move enforcement into
        the loader.  Fortunately, we already have a choke-point in the loader
        for enforcing this kind of policy.

        * dom/ScriptElement.cpp:
        (WebCore::ScriptElement::requestScript):
        * html/parser/HTMLDocumentParser.cpp:
        * html/parser/HTMLDocumentParser.h:
        * html/parser/HTMLScriptRunnerHost.h:
        * loader/cache/CachedResourceLoader.cpp:
        (WebCore::CachedResourceLoader::canRequest):
        * page/ContentSecurityPolicy.cpp:
        (WebCore::ContentSecurityPolicy::allowScriptFromSource):
        * page/ContentSecurityPolicy.h:


git-svn-id: svn://svn.chromium.org/blink/trunk@82085 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent b13ab418
2011-03-27 Adam Barth <abarth@webkit.org>
Reviewed by Eric Seidel.
Fix script-src redirect handling
https://bugs.webkit.org/show_bug.cgi?id=57196
Test both allow => disallow and disallow => allow redirect cases.
Previously, we had incorrect expectations for one of the redirect
cases. Also, I've updated the policy syntax to match the default-src
syntax.
* http/tests/security/contentSecurityPolicy/script-src-redirect-expected.txt:
* http/tests/security/contentSecurityPolicy/script-src-redirect.html:
2011-03-27 Yuta Kitamura <yutak@chromium.org> 2011-03-27 Yuta Kitamura <yutak@chromium.org>
Unreviewed, add new Chromium test result and expectations. Unreviewed, add new Chromium test result and expectations.
......
Loads an iframe which in turns tries to load an external script. The request for the script is redirected to 'localhost'. The iframe has a content security policy disabling external scripts from hosts other than 'localhost'. So the script should be allowed to run. Loads an iframe which in turns tries to load an external script. The request for the script is redirected to 'localhost'. The iframe has a content security policy disabling external scripts from hosts other than 'localhost'. So the script should be allowed to run.
-------- --------
Frame: '<!--framePath //<!--frame0-->-->' Frame: '<!--framePath //<!--frame0-->-->'
-------- --------
FAIL PASS
--------
Frame: '<!--framePath //<!--frame1-->-->'
--------
PASS
...@@ -12,6 +12,7 @@ if (window.layoutTestController) { ...@@ -12,6 +12,7 @@ if (window.layoutTestController) {
<p> <p>
Loads an iframe which in turns tries to load an external script. The request for the script is redirected to 'localhost'. The iframe has a content security policy disabling external scripts from hosts other than 'localhost'. So the script should be allowed to run. Loads an iframe which in turns tries to load an external script. The request for the script is redirected to 'localhost'. The iframe has a content security policy disabling external scripts from hosts other than 'localhost'. So the script should be allowed to run.
</p> </p>
<iframe src="http://127.0.0.1:8000/security/contentSecurityPolicy/resources/echo-script-src.pl?should_run=yes&csp=allow%20*%3B%20script-src%20'localhost'&q=http://127.0.0.1:8000/security/contentSecurityPolicy/resources/redir.php%3furl=http://localhost:8000/security/contentSecurityPolicy/resources/script.js"></iframe> <iframe src="http://127.0.0.1:8000/security/contentSecurityPolicy/resources/echo-script-src.pl?should_run=no&csp=%20script-src%20localhost&q=http://127.0.0.1:8000/security/contentSecurityPolicy/resources/redir.php%3furl=http://localhost:8000/security/contentSecurityPolicy/resources/script.js"></iframe>
<iframe src="http://127.0.0.1:8000/security/contentSecurityPolicy/resources/echo-script-src.pl?should_run=no&csp=%20script-src%20127.0.0.1&q=http://127.0.0.1:8000/security/contentSecurityPolicy/resources/redir.php%3furl=http://localhost:8000/security/contentSecurityPolicy/resources/script.js"></iframe>
</body> </body>
</html> </html>
2011-03-27 Adam Barth <abarth@webkit.org>
Reviewed by Eric Seidel.
Fix script-src redirect handling
https://bugs.webkit.org/show_bug.cgi?id=57196
Resource-loading requirements in CSP apply to each hop in the redirect
chain. To make that work properly, we need to move enforcement into
the loader. Fortunately, we already have a choke-point in the loader
for enforcing this kind of policy.
* dom/ScriptElement.cpp:
(WebCore::ScriptElement::requestScript):
* html/parser/HTMLDocumentParser.cpp:
* html/parser/HTMLDocumentParser.h:
* html/parser/HTMLScriptRunnerHost.h:
* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::canRequest):
* page/ContentSecurityPolicy.cpp:
(WebCore::ContentSecurityPolicy::allowScriptFromSource):
* page/ContentSecurityPolicy.h:
2011-03-27 Jer Noble <jer.noble@apple.com> 2011-03-27 Jer Noble <jer.noble@apple.com>
Reviewed by Maciej Stachowiak. Reviewed by Maciej Stachowiak.
...@@ -232,9 +232,6 @@ bool ScriptElement::prepareScript(const TextPosition1& scriptStartPosition, Lega ...@@ -232,9 +232,6 @@ bool ScriptElement::prepareScript(const TextPosition1& scriptStartPosition, Lega
bool ScriptElement::requestScript(const String& sourceUrl) bool ScriptElement::requestScript(const String& sourceUrl)
{ {
if (!m_element->document()->contentSecurityPolicy()->canLoadExternalScriptFromSrc(sourceUrl))
return false;
RefPtr<Document> originalDocument = m_element->document(); RefPtr<Document> originalDocument = m_element->document();
if (!m_element->dispatchBeforeLoadEvent(sourceUrl)) if (!m_element->dispatchBeforeLoadEvent(sourceUrl))
return false; return false;
......
...@@ -478,11 +478,6 @@ void HTMLDocumentParser::stopWatchingForLoad(CachedResource* cachedScript) ...@@ -478,11 +478,6 @@ void HTMLDocumentParser::stopWatchingForLoad(CachedResource* cachedScript)
cachedScript->removeClient(this); cachedScript->removeClient(this);
} }
bool HTMLDocumentParser::shouldLoadExternalScriptFromSrc(const AtomicString& srcValue)
{
return document()->contentSecurityPolicy()->canLoadExternalScriptFromSrc(srcValue);
}
void HTMLDocumentParser::notifyFinished(CachedResource* cachedResource) void HTMLDocumentParser::notifyFinished(CachedResource* cachedResource)
{ {
// pumpTokenizer can cause this parser to be detached from the Document, // pumpTokenizer can cause this parser to be detached from the Document,
......
...@@ -108,7 +108,6 @@ private: ...@@ -108,7 +108,6 @@ private:
// HTMLScriptRunnerHost // HTMLScriptRunnerHost
virtual void watchForLoad(CachedResource*); virtual void watchForLoad(CachedResource*);
virtual void stopWatchingForLoad(CachedResource*); virtual void stopWatchingForLoad(CachedResource*);
virtual bool shouldLoadExternalScriptFromSrc(const AtomicString&);
virtual HTMLInputStream& inputStream() { return m_input; } virtual HTMLInputStream& inputStream() { return m_input; }
// CachedResourceClient // CachedResourceClient
......
...@@ -44,8 +44,6 @@ public: ...@@ -44,8 +44,6 @@ public:
// Implementors must call cachedResource->removeClient() immediately. // Implementors must call cachedResource->removeClient() immediately.
virtual void stopWatchingForLoad(CachedResource*) = 0; virtual void stopWatchingForLoad(CachedResource*) = 0;
// Implementors can block certain script loads (for XSSAuditor, etc.)
virtual bool shouldLoadExternalScriptFromSrc(const AtomicString&) = 0;
virtual HTMLInputStream& inputStream() = 0; virtual HTMLInputStream& inputStream() = 0;
}; };
......
...@@ -34,6 +34,7 @@ ...@@ -34,6 +34,7 @@
#include "CachedScript.h" #include "CachedScript.h"
#include "CachedXSLStyleSheet.h" #include "CachedXSLStyleSheet.h"
#include "Console.h" #include "Console.h"
#include "ContentSecurityPolicy.h"
#include "DOMWindow.h" #include "DOMWindow.h"
#include "Document.h" #include "Document.h"
#include "Frame.h" #include "Frame.h"
...@@ -217,9 +218,6 @@ bool CachedResourceLoader::canRequest(CachedResource::Type type, const KURL& url ...@@ -217,9 +218,6 @@ bool CachedResourceLoader::canRequest(CachedResource::Type type, const KURL& url
} }
break; break;
#endif #endif
default:
ASSERT_NOT_REACHED();
break;
} }
// Given that the load is allowed by the same-origin policy, we should // Given that the load is allowed by the same-origin policy, we should
...@@ -253,11 +251,12 @@ bool CachedResourceLoader::canRequest(CachedResource::Type type, const KURL& url ...@@ -253,11 +251,12 @@ bool CachedResourceLoader::canRequest(CachedResource::Type type, const KURL& url
// Prefetch cannot affect the current document. // Prefetch cannot affect the current document.
break; break;
#endif #endif
default:
ASSERT_NOT_REACHED();
break;
} }
// FIXME: Consider letting the embedder block mixed content loads. // FIXME: Consider letting the embedder block mixed content loads.
if (type == CachedResource::Script && !m_document->contentSecurityPolicy()->allowScriptFromSource(url))
return false;
return true; return true;
} }
......
...@@ -430,9 +430,9 @@ bool ContentSecurityPolicy::allowJavaScriptURLs() const ...@@ -430,9 +430,9 @@ bool ContentSecurityPolicy::allowJavaScriptURLs() const
return !m_scriptSrc; return !m_scriptSrc;
} }
bool ContentSecurityPolicy::canLoadExternalScriptFromSrc(const String& url) const bool ContentSecurityPolicy::allowScriptFromSource(const KURL& url) const
{ {
return !m_scriptSrc || m_scriptSrc->allows(KURL(ParsedURLString, url)); return !m_scriptSrc || m_scriptSrc->allows(url);
} }
// policy = directive-list // policy = directive-list
......
...@@ -32,6 +32,7 @@ ...@@ -32,6 +32,7 @@
namespace WebCore { namespace WebCore {
class CSPDirective; class CSPDirective;
class KURL;
class SecurityOrigin; class SecurityOrigin;
class ContentSecurityPolicy : public RefCounted<ContentSecurityPolicy> { class ContentSecurityPolicy : public RefCounted<ContentSecurityPolicy> {
...@@ -45,8 +46,7 @@ public: ...@@ -45,8 +46,7 @@ public:
void didReceiveHeader(const String&); void didReceiveHeader(const String&);
bool allowJavaScriptURLs() const; bool allowJavaScriptURLs() const;
// FIXME: Rename canLoadExternalScriptFromSrc to allowScriptFromURL. bool allowScriptFromSource(const KURL&) const;
bool canLoadExternalScriptFromSrc(const String& url) const;
private: private:
explicit ContentSecurityPolicy(SecurityOrigin*); explicit ContentSecurityPolicy(SecurityOrigin*);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment