Implementing mixed content for forms posting to insecure location from secure ones

Source/core/html/HTMLFormElement.cpp

Index: Source/core/html/HTMLFormElement.cpp
diff --git a/Source/core/html/HTMLFormElement.cpp b/Source/core/html/HTMLFormElement.cpp
index edf2c15f82829b4e631f0f0fe688ad1274974835..3bfa95accaef87d31cbc9f84cf1f57e0dab9c4ab 100644
--- a/Source/core/html/HTMLFormElement.cpp
+++ b/Source/core/html/HTMLFormElement.cpp
@@ -38,6 +38,10 @@
 #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"
@@ -47,10 +51,7 @@
 #include "core/html/forms/FormController.h"
 #include "core/loader/FrameLoader.h"
 #include "core/loader/FrameLoaderClient.h"
-#include "core/frame/DOMWindow.h"
-#include "core/frame/LocalFrame.h"
-#include "core/frame/UseCounter.h"
-#include "core/frame/csp/ContentSecurityPolicy.h"
+#include "core/loader/MixedContentChecker.h"
 #include "core/rendering/RenderTextControl.h"
 #include "platform/UserGestureIndicator.h"
 
@@ -346,6 +347,11 @@ void HTMLFormElement::submit(Event* event, bool activateSubmitButton, bool proce
     if (!view || !frame || !frame->page())
         return;
 
+    // Mixed content with form submission to insecure "action"

[jww, 2014/06/05 17:35:05]

This comment is actually a bit unclear. Can you be more explicit with something
like:

"On submission of a form a secure origin with an action pointing to an insecure
action, if passive mixed content blocking is enabled, block the submission."

[mhm, 2014/06/05 17:47:43]

Done.

+    KURL actionURL = getActionURL();
+    if (!frame->loader().mixedContentChecker()->canDisplayInsecureContent(document().securityOrigin(), actionURL))

[Mike West, 2014/06/05 17:51:26]

Please add a UseCounter here so we know what percentage of form submissions are
being treated as passive mixed content. See
https://codereview.chromium.org/312253003/ for some counters I just added to
ResourceLoader.

I'd also suggest adding one UseCounter::count call to the top of this method to
measure the total number of submissions as a baseline.

+        return;
+
     m_wasUserSubmitted = processingUserGesture;
 
     RefPtrWillBeRawPtr<HTMLFormControlElement> firstSuccessfulSubmitButton = nullptr;
@@ -791,4 +797,17 @@ 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 mixed content.

[jww, 2014/06/05 17:35:05]

You should probably clarify "passive mixed content."

[mhm, 2014/06/05 17:47:43]

Done.

+        KURL actionURL = getActionURL();
+        if (MixedContentChecker::isMixedContent(document().securityOrigin(), actionURL))

[Mike West, 2014/06/05 17:51:26]

This is strange, but I understand why you did it. :)

How about:

  if (!document().....->canDisplayInsecureContent())
    // Set the HTMLFormElement action to the empty string.
  }
  if (...isMixedContent(...)) {
    UseCounter::count(...);
  }

+            document().frame()->loader().mixedContentChecker()->canDisplayInsecureContent(document().securityOrigin(), actionURL);
+    }
+}
+
 } // namespace