Commit 6e4e4585 authored by abarth@chromium.org's avatar abarth@chromium.org

Revert of Implementing mixed content for forms posting to insecure location...

Revert of Implementing mixed content for forms posting to insecure location from secure ones (https://codereview.chromium.org/311033003/)

Reason for revert:
This CL incorrectly integrates with the inspector.

Notice that in Element::didModifyAttribute, InspectorInstrumentation::didModifyDOMAttr will be called with the original value, not the mutated value.

Original issue's description:
> Implementing mixed content for forms posting to insecure location from secure ones.
> 
> When a form "action" attribute is pointing to insecure location from a secure one, this is flagged as mixed content. This is even checked if the pages dynamically changes the "action" attribute after the initial loading. The mixed content is non-blocking by default, however, this can be overridden in the preferences, and this will cause a submit to silently fail, other than a console message.
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175714

TBR=jww@chromium.org,mkwst@chromium.org,mohammed@chromium.org
NOTREECHECKS=true
NOTRY=true

Review URL: https://codereview.chromium.org/319183008

git-svn-id: svn://svn.chromium.org/blink/trunk@175715 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent cd10a0f6
CONSOLE WARNING: line 2: The page at 'https://127.0.0.1:8443/security/mixedContent/resources/frame-with-insecure-formSubmission.html' was loaded over HTTPS, but is submitting data to an insecure location at 'http://127.0.0.1:8080/security/resources/boring.html': this content should also be submitted over HTTPS.
This test opens a window that shows a form with "action" pointing to insecure location. We should trigger a mixed content callback even though we've set the preference to block this, because we've overriden the preferences via a web permission client callback.
<html>
<body>
<script>
if (window.testRunner) {
testRunner.waitUntilDone();
testRunner.dumpAsText();
testRunner.setCanOpenWindows();
testRunner.setCloseRemainingWindowsWhenComplete(true);
testRunner.overridePreference("WebKitAllowDisplayingInsecureContent", false);
testRunner.setAllowDisplayOfInsecureContent(true);
}
window.addEventListener("message", function (e) {
if (window.testRunner)
testRunner.notifyDone();
}, false);
</script>
<p>This test opens a window that shows a form with "action" pointing to insecure
location. We should trigger a mixed content callback even though we've set the preference to block this, because we've overriden the preferences via a web permission client callback.</p>
<script>
window.open("https://127.0.0.1:8443/security/mixedContent/resources/frame-with-insecure-formSubmission.html");
</script>
</body>
</html>
CONSOLE ERROR: line 2: [blocked] The page at 'https://127.0.0.1:8443/security/mixedContent/resources/frame-with-insecure-formSubmission.html' was loaded over HTTPS, but is submitting data to an insecure location at 'http://127.0.0.1:8080/security/resources/boring.html': this content should also be submitted over HTTPS.
This test opens a window that shows a form with "action" pointing to an insecure location. We should not trigger a mixed content callback even though the main frame in the window is HTTPS and the form is pointing to insecure content, because we've set the preference to block this.
<html>
<body>
<script>
if (window.testRunner) {
testRunner.waitUntilDone();
testRunner.dumpAsText();
testRunner.setCanOpenWindows();
testRunner.setCloseRemainingWindowsWhenComplete(true);
testRunner.overridePreference("WebKitAllowDisplayingInsecureContent", false);
}
window.addEventListener("message", function (e) {
if (window.testRunner)
testRunner.notifyDone();
}, false);
</script>
<p>This test opens a window that shows a form with "action" pointing to an insecure
location. We should not trigger a mixed content callback even though the main frame in the window is HTTPS and the form is pointing to insecure content, because we've set the preference to block this.</p>
<script>
window.open("https://127.0.0.1:8443/security/mixedContent/resources/frame-with-insecure-formSubmission.html");
</script>
</body>
</html>
CONSOLE WARNING: line 2: The page at 'https://127.0.0.1:8443/security/mixedContent/resources/frame-with-insecure-formSubmission.html' was loaded over HTTPS, but is submitting data to an insecure location at 'http://127.0.0.1:8080/security/resources/boring.html': this content should also be submitted over HTTPS.
This test opens a window that shows a form with "action" pointing to an insecure location. We should trigger a mixed content callback because the main frame in the window is HTTPS but is posting to insecure location.
<html>
<body>
<script>
if (window.testRunner) {
testRunner.waitUntilDone();
testRunner.dumpAsText();
testRunner.setCanOpenWindows();
testRunner.setCloseRemainingWindowsWhenComplete(true);
}
window.addEventListener("message", function (e) {
if (window.testRunner)
testRunner.notifyDone();
}, false);
</script>
<p>This test opens a window that shows a form with "action" pointing to an insecure
location. We should trigger a mixed content callback because the main frame
in the window is HTTPS but is posting to insecure location.</p>
<script>
window.open("https://127.0.0.1:8443/security/mixedContent/resources/frame-with-insecure-formSubmission.html");
</script>
</body>
</html>
<form action="http://127.0.0.1:8080/security/resources/boring.html"
method="post">
</form>
<script>
window.onload = function() {
if (window.opener)
window.opener.postMessage('done', '*');
};
</script>
<form action="https://127.0.0.1:8443/security/originHeader/resources/print-origin.cgi"
<form action="http://127.0.0.1:8000/security/originHeader/resources/print-origin.cgi"
method="POST">
</form>
<script>document.forms[0].submit();</script>
......@@ -438,8 +438,6 @@ public:
MixedContentImage = 438,
MixedContentMedia = 439,
DocumentFonts = 440,
MixedContentForm = 441,
MixedContentSubmittedForm = 442,
// Add new features immediately above this line. Don't change assigned
// numbers of any item, and don't reuse removed slots.
// Also, run update_use_counter_feature_enum.py in chromium/src/tools/metrics/histograms/
......
......@@ -37,10 +37,6 @@
#include "core/events/Event.h"
#include "core/events/GenericEventQueue.h"
#include "core/events/ScopedEventQueue.h"
#include "core/frame/DOMWindow.h"
#include "core/frame/LocalFrame.h"
#include "core/frame/UseCounter.h"
#include "core/frame/csp/ContentSecurityPolicy.h"
#include "core/html/HTMLCollection.h"
#include "core/html/HTMLDialogElement.h"
#include "core/html/HTMLImageElement.h"
......@@ -50,10 +46,12 @@
#include "core/html/forms/FormController.h"
#include "core/loader/FrameLoader.h"
#include "core/loader/FrameLoaderClient.h"
#include "core/loader/MixedContentChecker.h"
#include "core/frame/DOMWindow.h"
#include "core/frame/LocalFrame.h"
#include "core/frame/UseCounter.h"
#include "core/frame/csp/ContentSecurityPolicy.h"
#include "core/rendering/RenderTextControl.h"
#include "platform/UserGestureIndicator.h"
#include "wtf/text/AtomicString.h"
using namespace std;
......@@ -61,15 +59,6 @@ namespace WebCore {
using namespace HTMLNames;
namespace {
KURL getActionURL(const Document& document, const String& action)
{
return (action.isEmpty() ? document.url() : document.completeURL(action));
}
} // namespace
HTMLFormElement::HTMLFormElement(Document& document)
: HTMLElement(formTag, document)
#if !ENABLE(OILPAN)
......@@ -358,10 +347,6 @@ void HTMLFormElement::submit(Event* event, bool activateSubmitButton, bool proce
m_wasUserSubmitted = processingUserGesture;
KURL actionURL = getActionURL(document(), m_attributes.action());
if (MixedContentChecker::isMixedContent(document().securityOrigin(), actionURL))
UseCounter::count(document(), UseCounter::MixedContentSubmittedForm);
RefPtrWillBeRawPtr<HTMLFormControlElement> firstSuccessfulSubmitButton = nullptr;
bool needButtonActivation = activateSubmitButton; // do we need to activate a submit button?
......@@ -805,20 +790,4 @@ void HTMLFormElement::setDemoted(bool demoted)
m_wasDemoted = demoted;
}
void HTMLFormElement::attributeChanged(const QualifiedName& name, const AtomicString& newValue, AttributeModificationReason)
{
Element::attributeChanged(name, newValue);
if (name == actionAttr) {
// If the new action attribute is pointing to insecure "action" location from a secure page
// it is marked as "passive" mixed content. In other words, it will just
// show a console warning unless the user override the preferences to
// block all mixed content.
KURL actionURL = getActionURL(document(), m_attributes.action());
if (!document().frame()->loader().mixedContentChecker()->canSubmitToInsecureForm(document().securityOrigin(), actionURL))
Element::attributeChanged(name, AtomicString(""));
if (MixedContentChecker::isMixedContent(document().securityOrigin(), actionURL))
UseCounter::count(document(), UseCounter::MixedContentForm);
}
}
} // namespace
......@@ -118,9 +118,6 @@ public:
void anonymousNamedGetter(const AtomicString& name, bool&, RefPtrWillBeRawPtr<RadioNodeList>&, bool&, RefPtrWillBeRawPtr<Element>&);
protected:
virtual void attributeChanged(const QualifiedName&, const AtomicString&, AttributeModificationReason = ModifiedDirectly) OVERRIDE;
private:
explicit HTMLFormElement(Document&);
......
......@@ -58,14 +58,14 @@ bool MixedContentChecker::isMixedContent(SecurityOrigin* securityOrigin, const K
return !SecurityOrigin::isSecure(url);
}
bool MixedContentChecker::canDisplayInsecureContentInternal(SecurityOrigin* securityOrigin, const KURL& url, const MixedContentType type) const
bool MixedContentChecker::canDisplayInsecureContent(SecurityOrigin* securityOrigin, const KURL& url) const
{
if (!isMixedContent(securityOrigin, url))
return true;
Settings* settings = m_frame->settings();
bool allowed = client()->allowDisplayingInsecureContent(settings && settings->allowDisplayOfInsecureContent(), securityOrigin, url);
logWarning(allowed, url, type);
logWarning(allowed, "displayed", url);
if (allowed)
client()->didDisplayInsecureContent();
......@@ -73,15 +73,15 @@ bool MixedContentChecker::canDisplayInsecureContentInternal(SecurityOrigin* secu
return allowed;
}
bool MixedContentChecker::canRunInsecureContentInternal(SecurityOrigin* securityOrigin, const KURL& url, const MixedContentType type) const
bool MixedContentChecker::canRunInsecureContentInternal(SecurityOrigin* securityOrigin, const KURL& url, bool isWebSocket) const
{
if (!isMixedContent(securityOrigin, url))
return true;
Settings* settings = m_frame->settings();
bool allowedPerSettings = settings && (settings->allowRunningOfInsecureContent() || ((type == WebSocket) && settings->allowConnectingInsecureWebSocket()));
bool allowedPerSettings = settings && (settings->allowRunningOfInsecureContent() || (isWebSocket && settings->allowConnectingInsecureWebSocket()));
bool allowed = client()->allowRunningInsecureContent(allowedPerSettings, securityOrigin, url);
logWarning(allowed, url, type);
logWarning(allowed, "ran", url);
if (allowed)
client()->didRunInsecureContent(securityOrigin, url);
......@@ -89,21 +89,9 @@ bool MixedContentChecker::canRunInsecureContentInternal(SecurityOrigin* security
return allowed;
}
void MixedContentChecker::logWarning(bool allowed, const KURL& target, const MixedContentType type) const
void MixedContentChecker::logWarning(bool allowed, const String& action, const KURL& target) const
{
String message = String(allowed ? "" : "[blocked] ") + "The page at '" + m_frame->document()->url().elidedString() + "' was loaded over HTTPS, but ";
switch (type) {
case Display:
message.append("displayed insecure content from '" + target.elidedString() + "': this content should also be loaded over HTTPS.\n");
break;
case Execution:
case WebSocket:
message.append("ran insecure content from '" + target.elidedString() + "': this content should also be loaded over HTTPS.\n");
break;
case Submission:
message.append("is submitting data to an insecure location at '" + target.elidedString() + "': this content should also be submitted over HTTPS.\n");
break;
}
String message = String(allowed ? "" : "[blocked] ") + "The page at '" + m_frame->document()->url().elidedString() + "' was loaded over HTTPS, but " + action + " insecure content from '" + target.elidedString() + "': this content should also be loaded over HTTPS.\n";
MessageLevel messageLevel = allowed ? WarningMessageLevel : ErrorMessageLevel;
m_frame->document()->addConsoleMessage(SecurityMessageSource, messageLevel, message);
}
......
......@@ -45,41 +45,24 @@ class MixedContentChecker {
public:
MixedContentChecker(LocalFrame*);
bool canDisplayInsecureContent(SecurityOrigin* securityOrigin, const KURL& url) const
{
return canDisplayInsecureContentInternal(securityOrigin, url, MixedContentChecker::Display);
}
bool canSubmitToInsecureForm(SecurityOrigin* securityOrigin, const KURL& url) const
{
return canDisplayInsecureContentInternal(securityOrigin, url, MixedContentChecker::Submission);
}
bool canDisplayInsecureContent(SecurityOrigin*, const KURL&) const;
bool canRunInsecureContent(SecurityOrigin* securityOrigin, const KURL& url) const
{
return canRunInsecureContentInternal(securityOrigin, url, MixedContentChecker::Execution);
return canRunInsecureContentInternal(securityOrigin, url, false);
}
bool canConnectInsecureWebSocket(SecurityOrigin* securityOrigin, const KURL& url) const
{
return canRunInsecureContentInternal(securityOrigin, url, MixedContentChecker::WebSocket);
return canRunInsecureContentInternal(securityOrigin, url, true);
}
static bool isMixedContent(SecurityOrigin*, const KURL&);
private:
enum MixedContentType {
Display,
Execution,
WebSocket,
Submission
};
// FIXME: This should probably have a separate client from FrameLoader.
FrameLoaderClient* client() const;
bool canDisplayInsecureContentInternal(SecurityOrigin*, const KURL&, const MixedContentType) const;
bool canRunInsecureContentInternal(SecurityOrigin*, const KURL&, const MixedContentType) const;
bool canRunInsecureContentInternal(SecurityOrigin*, const KURL&, bool isWebSocket) const;
void logWarning(bool allowed, const KURL& i, const MixedContentType) const;
void logWarning(bool allowed, const String& action, const KURL&) const;
LocalFrame* m_frame;
};
......
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