Commit bec9ad2e authored by Carlos IL's avatar Carlos IL Committed by Chromium LUCI CQ

Add variations param and metrics for mixed form redirects

Bug: 1158636

Change-Id: I8d654ac690fbdc2da3212cacdc3e36018e5dede5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2590665
Commit-Queue: Carlos IL <carlosil@chromium.org>
Commit-Queue: Chris Thompson <cthomp@chromium.org>
Auto-Submit: Carlos IL <carlosil@chromium.org>
Reviewed-by: default avatarChris Thompson <cthomp@chromium.org>
Reviewed-by: default avatarEmily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#838964}
parent 2e9e011e
This diff is collapsed.
<html>
<head>
<title>Page that displays an form whose action URL redirects to http</title>
<script>
function submitForm() {
form = document.getElementById("insecureForm");
form.submit();
}
</script>
</head>
<body>
This page contains an form which targets URL that redirects to http,
causing insecure content (when this page is loaded over https).<br>
<form id="insecureForm" action="/server-redirect-301?http://does-not-exist.test" method="post">
<input type="submit" />
</form>
</body>
</html>
<html>
<head>
<title>Page that displays an form whose action URL redirects to http</title>
<script>
function submitForm() {
form = document.getElementById("insecureForm");
form.submit();
}
</script>
</head>
<body>
This page contains an form which targets URL that redirects to http,
causing insecure content (when this page is loaded over https).<br>
<form id="insecureForm" action="/server-redirect-302?http://does-not-exist.test" method="post">
<input type="submit" />
</form>
</body>
</html>
<html>
<head>
<title>Page that displays an form whose action URL redirects to http</title>
<script>
function submitForm() {
form = document.getElementById("insecureForm");
form.submit();
}
</script>
</head>
<body>
This page contains an form which targets URL that redirects to http,
causing insecure content (when this page is loaded over https).<br>
<form id="insecureForm" action="/server-redirect-308?http://does-not-exist.test" method="post">
<input type="submit" />
</form>
</body>
</html>
<html>
<head>
<title>Page that displays an form whose action URL redirects to http</title>
<script>
function submitForm() {
form = document.getElementById("insecureForm");
form.submit();
}
</script>
</head>
<body>
This page contains an form which targets URL that redirects to http,
causing insecure content (when this page is loaded over https).<br>
<form id="insecureForm" action="/server-redirect-307?http://does-not-exist.test" method="post">
<input type="submit" />
</form>
</body>
</html>
<html>
<head>
<title>Page that displays an form whose action URL redirects to http</title>
<script>
function submitForm() {
form = document.getElementById("insecureForm");
form.submit();
}
</script>
</head>
<body>
This page contains an form which targets URL that redirects to http,
causing insecure content (when this page is loaded over https).<br>
<form id="insecureForm" action="/redirect_to_http" method="get">
<input type="submit" />
</form>
</body>
</html>
......@@ -5,6 +5,8 @@
#include "components/security_interstitials/content/insecure_form_navigation_throttle.h"
#include "base/feature_list.h"
#include "base/metrics/field_trial_params.h"
#include "base/metrics/histogram_functions.h"
#include "components/prefs/pref_service.h"
#include "components/security_interstitials/content/insecure_form_blocking_page.h"
#include "components/security_interstitials/content/insecure_form_tab_storage.h"
......@@ -13,11 +15,20 @@
#include "components/security_interstitials/core/pref_names.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/web_contents.h"
#include "net/http/http_status_code.h"
#include "services/network/public/cpp/is_potentially_trustworthy.h"
#include "url/origin.h"
#include "url/url_constants.h"
namespace {
void LogMixedFormInterstitialMetrics(
security_interstitials::InsecureFormNavigationThrottle::
InterstitialTriggeredState state) {
base::UmaHistogramEnumeration("Security.MixedForm.InterstitialTriggerState",
state);
}
bool IsInsecureFormAction(const GURL& action_url) {
if (action_url.SchemeIs(url::kBlobScheme) ||
action_url.SchemeIs(url::kFileSystemScheme))
......@@ -25,6 +36,7 @@ bool IsInsecureFormAction(const GURL& action_url) {
return !network::IsOriginPotentiallyTrustworthy(
url::Origin::Create(action_url));
}
} // namespace
namespace security_interstitials {
......@@ -39,6 +51,45 @@ InsecureFormNavigationThrottle::~InsecureFormNavigationThrottle() = default;
content::NavigationThrottle::ThrottleCheckResult
InsecureFormNavigationThrottle::WillStartRequest() {
return GetThrottleResultForMixedForm(false /* is_redirect */);
}
content::NavigationThrottle::ThrottleCheckResult
InsecureFormNavigationThrottle::WillRedirectRequest() {
return GetThrottleResultForMixedForm(true /* is_redirect */);
}
content::NavigationThrottle::ThrottleCheckResult
InsecureFormNavigationThrottle::WillProcessResponse() {
// If there is an InsecureFormTabStorage associated to |web_contents_|, clear
// the IsProceeding flag.
InsecureFormTabStorage* tab_storage = InsecureFormTabStorage::FromWebContents(
navigation_handle()->GetWebContents());
if (tab_storage)
tab_storage->SetIsProceeding(false);
return content::NavigationThrottle::PROCEED;
}
const char* InsecureFormNavigationThrottle::GetNameForLogging() {
return "InsecureFormNavigationThrottle";
}
// static
std::unique_ptr<InsecureFormNavigationThrottle>
InsecureFormNavigationThrottle::MaybeCreateNavigationThrottle(
content::NavigationHandle* navigation_handle,
std::unique_ptr<SecurityBlockingPageFactory> blocking_page_factory,
PrefService* prefs) {
if (!base::FeatureList::IsEnabled(kInsecureFormSubmissionInterstitial) ||
(prefs && !prefs->GetBoolean(prefs::kMixedFormsWarningsEnabled)))
return nullptr;
return std::make_unique<InsecureFormNavigationThrottle>(
navigation_handle, std::move(blocking_page_factory));
}
content::NavigationThrottle::ThrottleCheckResult
InsecureFormNavigationThrottle::GetThrottleResultForMixedForm(
bool is_redirect) {
content::NavigationHandle* handle = navigation_handle();
if (!handle->IsFormSubmission())
return content::NavigationThrottle::PROCEED;
......@@ -65,6 +116,38 @@ InsecureFormNavigationThrottle::WillStartRequest() {
if (tab_storage->IsProceeding())
return content::NavigationThrottle::PROCEED;
if (is_redirect) {
std::string feature_mode = base::GetFieldTrialParamValueByFeature(
kInsecureFormSubmissionInterstitial,
kInsecureFormSubmissionInterstitialMode);
// 307 and 308 redirects for POST forms are special because they can leak
// form data if done over HTTP.
if ((handle->GetResponseHeaders()->response_code() ==
net::HTTP_TEMPORARY_REDIRECT ||
handle->GetResponseHeaders()->response_code() ==
net::HTTP_PERMANENT_REDIRECT) &&
handle->IsPost()) {
LogMixedFormInterstitialMetrics(
InterstitialTriggeredState::kMixedFormRedirectWithFormData);
if (feature_mode !=
kInsecureFormSubmissionInterstitialModeIncludeRedirects &&
feature_mode !=
kInsecureFormSubmissionInterstitialModeIncludeRedirectsWithFormData) {
return content::NavigationThrottle::PROCEED;
}
} else {
LogMixedFormInterstitialMetrics(
InterstitialTriggeredState::kMixedFormRedirectNoFormData);
if (feature_mode !=
kInsecureFormSubmissionInterstitialModeIncludeRedirects) {
return content::NavigationThrottle::PROCEED;
}
}
} else {
LogMixedFormInterstitialMetrics(
InterstitialTriggeredState::kMixedFormDirect);
}
std::unique_ptr<InsecureFormBlockingPage> blocking_page =
blocking_page_factory_->CreateInsecureFormBlockingPage(contents,
handle->GetURL());
......@@ -75,37 +158,4 @@ InsecureFormNavigationThrottle::WillStartRequest() {
CANCEL, net::ERR_BLOCKED_BY_CLIENT, interstitial_html);
}
content::NavigationThrottle::ThrottleCheckResult
InsecureFormNavigationThrottle::WillRedirectRequest() {
return WillStartRequest();
}
content::NavigationThrottle::ThrottleCheckResult
InsecureFormNavigationThrottle::WillProcessResponse() {
// If there is an InsecureFormTabStorage associated to |web_contents_|, clear
// the IsProceeding flag.
InsecureFormTabStorage* tab_storage = InsecureFormTabStorage::FromWebContents(
navigation_handle()->GetWebContents());
if (tab_storage)
tab_storage->SetIsProceeding(false);
return content::NavigationThrottle::PROCEED;
}
const char* InsecureFormNavigationThrottle::GetNameForLogging() {
return "InsecureFormNavigationThrottle";
}
// static
std::unique_ptr<InsecureFormNavigationThrottle>
InsecureFormNavigationThrottle::MaybeCreateNavigationThrottle(
content::NavigationHandle* navigation_handle,
std::unique_ptr<SecurityBlockingPageFactory> blocking_page_factory,
PrefService* prefs) {
if (!base::FeatureList::IsEnabled(kInsecureFormSubmissionInterstitial) ||
(prefs && !prefs->GetBoolean(prefs::kMixedFormsWarningsEnabled)))
return nullptr;
return std::make_unique<InsecureFormNavigationThrottle>(
navigation_handle, std::move(blocking_page_factory));
}
} // namespace security_interstitials
......@@ -18,6 +18,16 @@ namespace security_interstitials {
class InsecureFormNavigationThrottle : public content::NavigationThrottle {
public:
// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
// Exposed for testing.
enum class InterstitialTriggeredState {
kMixedFormDirect = 0,
kMixedFormRedirectWithFormData = 1,
kMixedFormRedirectNoFormData = 2,
kMaxValue = kMixedFormRedirectNoFormData,
};
InsecureFormNavigationThrottle(
content::NavigationHandle* navigation_handle,
std::unique_ptr<SecurityBlockingPageFactory> blocking_page_factory);
......@@ -36,6 +46,9 @@ class InsecureFormNavigationThrottle : public content::NavigationThrottle {
PrefService* prefs);
private:
content::NavigationThrottle::ThrottleCheckResult
GetThrottleResultForMixedForm(bool is_redirect);
std::unique_ptr<SecurityBlockingPageFactory> blocking_page_factory_;
};
......
......@@ -9,4 +9,11 @@ namespace security_interstitials {
const base::Feature kInsecureFormSubmissionInterstitial{
"InsecureFormSubmissionInterstitial", base::FEATURE_ENABLED_BY_DEFAULT};
const char kInsecureFormSubmissionInterstitialMode[] = "mode";
const char kInsecureFormSubmissionInterstitialModeIncludeRedirects[] =
"include-redirects";
const char
kInsecureFormSubmissionInterstitialModeIncludeRedirectsWithFormData[] =
"include-redirects-with-form-data";
} // namespace security_interstitials
......@@ -12,6 +12,18 @@ namespace security_interstitials {
// Controls whether an interstitial is shown when submitting a mixed form.
extern const base::Feature kInsecureFormSubmissionInterstitial;
// Controls if the insecure form interstitial is enabled for forms that intially
// submit to https, but redirect to http. If not set the interstitial will only
// be shown for forms that submit directly to http.
extern const char kInsecureFormSubmissionInterstitialMode[];
// If set to this mode, the interstitial will be shown for any redirect over
// http.
extern const char kInsecureFormSubmissionInterstitialModeIncludeRedirects[];
// If set to this mode, the interstitial will only be shown for redirects over
// http that expose form data (i.e. 307 or 308 redirects for POST method forms).
extern const char
kInsecureFormSubmissionInterstitialModeIncludeRedirectsWithFormData[];
} // namespace security_interstitials
#endif // COMPONENTS_SECURITY_INTERSTITIALS_CORE_FEATURES_H_
......@@ -49616,6 +49616,16 @@ Called by update_use_counter_css.py.-->
<int value="4" label="Scripting Content with Certificate Errors"/>
</enum>
<enum name="MixedFormInterstitialTriggeredState">
<int value="0" label="Mixed Form that submits directly to HTTP"/>
<int value="1"
label="Mixed Form that includes HTTP in redirect chain, with exposed
data"/>
<int value="2"
label="Mixed Form that includes HTTP in redirect chain, without exposed
data"/>
</enum>
<enum name="MjpegDecodeAcceleratorErrorCode">
<int value="0" label="NO_ERRORS"/>
<int value="1" label="INVALID_ARGUMENT"/>
......@@ -98,6 +98,17 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
</summary>
</histogram>
<histogram name="Security.MixedForm.InterstitialTriggerState"
enum="MixedFormInterstitialTriggeredState" expires_after="M92">
<owner>carlosil@chromium.org</owner>
<owner>security-enamel@chromium.org</owner>
<summary>
Logs when an interstitial was triggered (or would have been triggered but
was not due to the currently enabled experiment mode) and whether it was a
direct submission to an insecure URL, or a redirect through one.
</summary>
</histogram>
<histogram base="true" name="Security.PageEndReason" enum="PageEndReason"
expires_after="2021-06-06">
<owner>cthomp@chromium.org</owner>
......
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