From 7e606471236e4a7eda1727334ddd44fe802b816a Mon Sep 17 00:00:00 2001 From: Carlos IL Date: Fri, 16 Oct 2020 22:58:12 +0000 Subject: [PATCH] Enable mixed image autoupgrades by default Bug: 1042877 Change-Id: Ib4ba9d336bde09e82c17594dcd105448b2ff8916 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2454853 Commit-Queue: Carlos IL Commit-Queue: Bo Reviewed-by: Nate Fischer Reviewed-by: Bo Reviewed-by: Ilya Sherman Reviewed-by: Charlie Harrison Reviewed-by: Avi Drissman Reviewed-by: Mike West Auto-Submit: Carlos IL Cr-Commit-Position: refs/heads/master@{#818153} --- .../chromium/android_webview/AwSettings.java | 13 ++++----- .../conversion_registration_browsertest.cc | 5 ++-- .../browser/site_per_process_browsertest.cc | 25 ++++++++++++++-- .../variations/fieldtrial_testing_config.json | 21 -------------- .../core/loader/mixed_content_checker.cc | 12 +------- third_party/blink/web_tests/NeverFixTests | 4 --- third_party/blink/web_tests/TestExpectations | 29 ++++++++++++++++--- third_party/blink/web_tests/VirtualTestSuites | 5 ---- ...ed-content-issue-creation-img-expected.txt | 2 +- ...t-status-optionally-blockable-expected.txt | 2 +- ...mg-no-referrer-when-downgrade-expected.txt | 1 + ...r-policy-conflicting-policies-expected.txt | 1 + ...onflicting-policies-image-set-expected.txt | 1 + .../mixed-autoupgrade/optionally/README.txt | 1 - 14 files changed, 62 insertions(+), 60 deletions(-) create mode 100644 third_party/blink/web_tests/http/tests/security/referrer-policy-attribute-img-no-referrer-when-downgrade-expected.txt create mode 100644 third_party/blink/web_tests/http/tests/security/referrer-policy-conflicting-policies-expected.txt create mode 100644 third_party/blink/web_tests/http/tests/security/referrer-policy-conflicting-policies-image-set-expected.txt delete mode 100644 third_party/blink/web_tests/virtual/autoupgrade-optionally-blockable-mixed-content/http/tests/mixed-autoupgrade/optionally/README.txt diff --git a/android_webview/java/src/org/chromium/android_webview/AwSettings.java b/android_webview/java/src/org/chromium/android_webview/AwSettings.java index 107ee181cb48de..dfe64bfef81925 100644 --- a/android_webview/java/src/org/chromium/android_webview/AwSettings.java +++ b/android_webview/java/src/org/chromium/android_webview/AwSettings.java @@ -1838,14 +1838,11 @@ private boolean getUseStricMixedContentCheckingLocked() { @CalledByNative private boolean getAllowMixedContentAutoupgradesLocked() { - assert Thread.holdsLock(mAwSettingsLock); - // We only allow mixed content autoupgrades (upgrading HTTP subresources to HTTPS in HTTPS - // sites) when the mixed content mode is set to MIXED_CONTENT_COMPATIBILITY, which keeps it - // in line with the behavior in Chrome. With MIXED_CONTENT_ALWAYS_ALLOW, we disable - // autoupgrades since the developer is explicitly allowing mixed content, whereas with - // MIXED_CONTENT_NEVER_ALLOW, there is no need to autoupgrade since the content will be - // blocked. - return mMixedContentMode == WebSettings.MIXED_CONTENT_COMPATIBILITY_MODE; + // TODO(crbug.com/1139424): Mixed content autoupgrades are temporarily disabled completely + // on WebView. This should remain as is for MIXED_CONTENT_ALWAYS_ALLOW and + // MIXED_CONTENT_NEVER_ALLOW, but should be controlled via a variations flag for + // MIXED_CONTENT_COMPATIBILITY_MODE. + return false; } public boolean getOffscreenPreRaster() { diff --git a/content/browser/conversions/conversion_registration_browsertest.cc b/content/browser/conversions/conversion_registration_browsertest.cc index b0251de180235d..5d24f45e009c0f 100644 --- a/content/browser/conversions/conversion_registration_browsertest.cc +++ b/content/browser/conversions/conversion_registration_browsertest.cc @@ -313,6 +313,9 @@ IN_PROC_BROWSER_TEST_F( ConversionRegistrationBrowserTest, RegisterWithDifferentUrlTypes_ConversionReceivedOrIgnored) { const char kSecureHost[] = "a.test"; + // TODO(crbug.com/1137113): Should include a test where an insecure request is + // blocked from conversion registration if it is made on a secure page. Note + // that this can't work for image requests due to image auto-upgrade. struct { std::string page_host; std::string redirect_host; @@ -327,8 +330,6 @@ IN_PROC_BROWSER_TEST_F( {kSecureHost /* page_host */, kSecureHost /* redirect_host */, true /* conversion_expected */}, {"insecure.com" /* page_host */, kSecureHost /* redirect_host */, - false /* conversion_expected */}, - {kSecureHost /* page_host */, "insecure.com" /* redirect_host */, false /* conversion_expected */}}; for (const auto& test_case : kTestCases) { diff --git a/content/browser/site_per_process_browsertest.cc b/content/browser/site_per_process_browsertest.cc index d40e817995c84c..ba7589dece3cf9 100644 --- a/content/browser/site_per_process_browsertest.cc +++ b/content/browser/site_per_process_browsertest.cc @@ -6978,6 +6978,22 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessBrowserTest, EvalJs(foo_root, "self.origin;")); } +// A subclass of SitePerProcessIgnoreCertErrorsBrowsertest that disables mixed +// content autoupgrades. +// TODO(carlosil): Since the flag will be cleaned up eventually, this should be +// changed to proper plumbing that adds the relevant urls to the allowlist. +class SitePerProcessIgnoreCertErrorsAllowMixedContentBrowserTest + : public SitePerProcessIgnoreCertErrorsBrowserTest { + public: + SitePerProcessIgnoreCertErrorsAllowMixedContentBrowserTest() { + feature_list.InitAndDisableFeature( + blink::features::kMixedContentAutoupgrade); + } + + private: + base::test::ScopedFeatureList feature_list; +}; + // Tests that the WebContents is notified when passive mixed content is // displayed in an OOPIF. The test ignores cert errors so that an HTTPS // iframe can be loaded from a site other than localhost (the @@ -6988,8 +7004,9 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessBrowserTest, #else #define MAYBE_PassiveMixedContentInIframe PassiveMixedContentInIframe #endif -IN_PROC_BROWSER_TEST_P(SitePerProcessIgnoreCertErrorsBrowserTest, - MAYBE_PassiveMixedContentInIframe) { +IN_PROC_BROWSER_TEST_P( + SitePerProcessIgnoreCertErrorsAllowMixedContentBrowserTest, + MAYBE_PassiveMixedContentInIframe) { net::EmbeddedTestServer https_server(net::EmbeddedTestServer::TYPE_HTTPS); https_server.ServeFilesFromSourceDirectory(GetTestDataFilePath()); SetupCrossSiteRedirector(&https_server); @@ -15965,6 +15982,10 @@ INSTANTIATE_TEST_SUITE_P(All, INSTANTIATE_TEST_SUITE_P(All, SitePerProcessIgnoreCertErrorsBrowserTest, testing::ValuesIn(RenderDocumentFeatureLevelValues())); +INSTANTIATE_TEST_SUITE_P( + All, + SitePerProcessIgnoreCertErrorsAllowMixedContentBrowserTest, + testing::ValuesIn(RenderDocumentFeatureLevelValues())); INSTANTIATE_TEST_SUITE_P(All, SitePerProcessProgrammaticScrollTest, testing::ValuesIn(RenderDocumentFeatureLevelValues())); diff --git a/testing/variations/fieldtrial_testing_config.json b/testing/variations/fieldtrial_testing_config.json index a915c469e4fe12..1736532bae57a4 100644 --- a/testing/variations/fieldtrial_testing_config.json +++ b/testing/variations/fieldtrial_testing_config.json @@ -41,27 +41,6 @@ ] } ], - "AllPassiveMixedContentAutoupgrade": [ - { - "platforms": [ - "android", - "chromeos", - "linux", - "mac" - ], - "experiments": [ - { - "name": "Enabled_10", - "params": { - "mode": "all-passive" - }, - "enable_features": [ - "AutoupgradeMixedContent" - ] - } - ] - } - ], "AmpBackgroundTab": [ { "platforms": [ diff --git a/third_party/blink/renderer/core/loader/mixed_content_checker.cc b/third_party/blink/renderer/core/loader/mixed_content_checker.cc index 25500205c82ba2..863bf2183fe391 100644 --- a/third_party/blink/renderer/core/loader/mixed_content_checker.cc +++ b/third_party/blink/renderer/core/loader/mixed_content_checker.cc @@ -771,17 +771,7 @@ bool MixedContentChecker::ShouldAutoupgrade( return false; } - auto autoupgrade_mode = base::GetFieldTrialParamValueByFeature( - blink::features::kMixedContentAutoupgrade, - blink::features::kMixedContentAutoupgradeModeParamName); - - if (autoupgrade_mode == - blink::features::kMixedContentAutoupgradeModeAllPassive) { - return true; - } - - // Otherwise we default to excluding images. - return type != mojom::RequestContextType::IMAGE; + return true; } void MixedContentChecker::CheckMixedPrivatePublic( diff --git a/third_party/blink/web_tests/NeverFixTests b/third_party/blink/web_tests/NeverFixTests index bc0c442cd83765..e5361ac30e2bcd 100644 --- a/third_party/blink/web_tests/NeverFixTests +++ b/third_party/blink/web_tests/NeverFixTests @@ -1415,10 +1415,6 @@ crbug.com/869492 virtual/threaded/external/wpt/feature-policy/experimental-featu crbug.com/869492 virtual/threaded/external/wpt/feature-policy/experimental-features/lazyload/lazyload-enabled-image-tentative.sub.html [ Skip ] crbug.com/869492 virtual/threaded/external/wpt/feature-policy/experimental-features/lazyload/lazyload-image-loading-lazy-tentative.sub.html [ Skip ] -# Tests that only work when the mixed content autoupgrade feature is enabled. -# Covered in virtual test suite. -http/tests/mixed-autoupgrade/* [ Skip ] - # Not expected to pass in default configuration, only virtual test suite. crbug.com/830901 fast/webgl/video-metadata/texImage-video-last-uploaded-metadata.html [ Skip ] virtual/webgl-extra-video-texture-metadata/fast/webgl/video-metadata/texImage-video-last-uploaded-metadata.html [ Pass ] diff --git a/third_party/blink/web_tests/TestExpectations b/third_party/blink/web_tests/TestExpectations index d5770b2704fc48..7af9af2566cacf 100644 --- a/third_party/blink/web_tests/TestExpectations +++ b/third_party/blink/web_tests/TestExpectations @@ -5990,10 +5990,6 @@ crbug.com/1025274 external/wpt/mixed-content/gen/top.meta/unset/audio-tag.https. crbug.com/1025274 external/wpt/mixed-content/gen/top.meta/unset/video-tag.https.html [ Failure ] crbug.com/1025274 http/tests/security/mixedContent/insecure-audio-video-in-main-frame.html [ Failure ] -#Mixed content autoupgrade tests that can be re-enabled once image autoupgrades launch -crbug.com/1042877 virtual/autoupgrade-optionally-blockable-mixed-content/http/tests/mixed-autoupgrade/optionally/image-upgrade-console-message.https.html [ Failure ] -crbug.com/1042877 virtual/autoupgrade-optionally-blockable-mixed-content/http/tests/mixed-autoupgrade/optionally/image-upgrade.https.html [ Failure ] - # DevTools roll crbug.com/1052111 http/tests/devtools/a11y-axe-core/audits-start-view-a11y-test.js [ Skip ] @@ -6433,6 +6429,31 @@ crbug.com/1131252 virtual/threaded/external/wpt/css/css-transforms/animation/tra crbug.com/1136163 [ Linux ] external/wpt/pointerevents/pointerlock/pointerevent_movementxy_with_pointerlock.html [ Failure ] +# Mixed content autoupgrades make these tests not applicable, since they check for mixed content images, the tests can be removed when cleaning up pre autoupgrades mixed content code. +crbug.com/1042877 external/wpt/fetch/cross-origin-resource-policy/scheme-restriction.https.window.html [ Failure ] +crbug.com/1042877 external/wpt/mixed-content/gen/top.meta/unset/img-tag.https.html [ Failure ] +crbug.com/1042877 external/wpt/mixed-content/imageset.https.sub.html [ Failure ] +crbug.com/1042877 external/wpt/upgrade-insecure-requests/gen/iframe-blank-inherit.meta/unset/img-tag.https.html [ Failure ] +crbug.com/1042877 external/wpt/upgrade-insecure-requests/gen/srcdoc-inherit.meta/unset/img-tag.https.html [ Failure ] +crbug.com/1042877 external/wpt/upgrade-insecure-requests/gen/top.meta/unset/img-tag.https.html [ Failure ] +crbug.com/1042877 virtual/omt-worker-fetch/external/wpt/fetch/cross-origin-resource-policy/scheme-restriction.https.window.html [ Failure ] +crbug.com/1042877 virtual/omt-worker-fetch/external/wpt/upgrade-insecure-requests/gen/iframe-blank-inherit.meta/unset/img-tag.https.html [ Failure ] +crbug.com/1042877 virtual/omt-worker-fetch/external/wpt/upgrade-insecure-requests/gen/srcdoc-inherit.meta/unset/img-tag.https.html [ Failure ] +crbug.com/1042877 virtual/omt-worker-fetch/external/wpt/upgrade-insecure-requests/gen/top.meta/unset/img-tag.https.html [ Failure ] +crbug.com/1042877 http/tests/security/mixedContent/insecure-css-image-with-reload.html [ Failure ] +crbug.com/1042877 http/tests/security/mixedContent/insecure-image-in-iframe.html [ Failure ] +crbug.com/1042877 http/tests/security/mixedContent/insecure-image-in-main-frame-allowed.html [ Failure ] +crbug.com/1042877 http/tests/security/mixedContent/insecure-image-in-main-frame-blocked.html [ Failure ] +crbug.com/1042877 http/tests/security/mixedContent/insecure-image-in-main-frame.html [ Failure ] +crbug.com/1042877 http/tests/security/mixedContent/preload-insecure-image-in-main-frame-blocked.html [ Failure ] +crbug.com/1042877 http/tests/security/mixedContent/strict-mode-image-blocked.https.html [ Timeout ] +crbug.com/1042877 http/tests/security/mixedContent/strict-mode-image-in-frame-blocked.https.html [ Failure ] +crbug.com/1042877 http/tests/security/mixedContent/strict-mode-image-reportonly.https.php [ Failure ] +crbug.com/1042877 http/tests/security/mixedContent/strict-mode-via-pref-image-blocked.https.html [ Failure ] + +# Mixed content autoupgrades cause test to fail because test relied on http subresources to test a different origin, needs to be changed to not rely on HTTP URLs. +crbug.com/1042877 http/tests/security/img-crossorigin-redirect-credentials.https.html [ Failure ] + # Sheriff 2020-09-30 crbug.com/1133821 virtual/off-main-thread-css-paint/external/wpt/css/css-paint-api/one-custom-property-animation.https.html [ Pass Failure ] crbug.com/1133821 virtual/off-main-thread-css-paint/external/wpt/css/css-paint-api/two-custom-property-animation.https.html [ Pass Failure ] diff --git a/third_party/blink/web_tests/VirtualTestSuites b/third_party/blink/web_tests/VirtualTestSuites index bb479bb6a1200f..cdaf1167356361 100644 --- a/third_party/blink/web_tests/VirtualTestSuites +++ b/third_party/blink/web_tests/VirtualTestSuites @@ -427,11 +427,6 @@ "args": ["--enable-blink-features=AutomaticLazyImageLoading", "--disable-blink-features=RestrictAutomaticLazyImageLoadingToDataSaver"] }, - { - "prefix" : "autoupgrade-optionally-blockable-mixed-content", - "bases": ["http/tests/mixed-autoupgrade/optionally"], - "args": ["--enable-features=AutoupgradeMixedContent"] - }, { "prefix": "webgl-extra-video-texture-metadata", "bases": ["fast/webgl/video-metadata"], diff --git a/third_party/blink/web_tests/http/tests/inspector-protocol/issues/mixed-content-issue-creation-img-expected.txt b/third_party/blink/web_tests/http/tests/inspector-protocol/issues/mixed-content-issue-creation-img-expected.txt index 783ed55376813a..52bc3433b198fa 100644 --- a/third_party/blink/web_tests/http/tests/inspector-protocol/issues/mixed-content-issue-creation-img-expected.txt +++ b/third_party/blink/web_tests/http/tests/inspector-protocol/issues/mixed-content-issue-creation-img-expected.txt @@ -14,7 +14,7 @@ Inspector issue: { requestId : url : http://devtools.test:8000/inspector-protocol/resources/img.png } - resolutionStatus : MixedContentWarning + resolutionStatus : MixedContentAutomaticallyUpgraded resourceType : Image } } diff --git a/third_party/blink/web_tests/http/tests/inspector-protocol/request-mixed-content-status-optionally-blockable-expected.txt b/third_party/blink/web_tests/http/tests/inspector-protocol/request-mixed-content-status-optionally-blockable-expected.txt index 6e2e317e8b0cb3..fef0aecbfaafc1 100644 --- a/third_party/blink/web_tests/http/tests/inspector-protocol/request-mixed-content-status-optionally-blockable-expected.txt +++ b/third_party/blink/web_tests/http/tests/inspector-protocol/request-mixed-content-status-optionally-blockable-expected.txt @@ -1,5 +1,5 @@ Tests that willSendRequest contains the correct mixed content status for passive mixed content. Network agent enabled Mixed content type of https://example.test:8443/inspector-protocol/resources/passive-mixed-content-iframe.html: none -Mixed content type of http://example.test:8000/resources/square.png: optionally-blockable +Mixed content type of https://example.test:8000/resources/square.png: none diff --git a/third_party/blink/web_tests/http/tests/security/referrer-policy-attribute-img-no-referrer-when-downgrade-expected.txt b/third_party/blink/web_tests/http/tests/security/referrer-policy-attribute-img-no-referrer-when-downgrade-expected.txt new file mode 100644 index 00000000000000..1eca604ebd15bb --- /dev/null +++ b/third_party/blink/web_tests/http/tests/security/referrer-policy-attribute-img-no-referrer-when-downgrade-expected.txt @@ -0,0 +1 @@ +CONSOLE WARNING: Mixed Content: The page at 'https://127.0.0.1:8443/security/referrer-policy-attribute-img-no-referrer-when-downgrade.html' was loaded over HTTPS, but requested an insecure element 'http://127.0.0.1:8000/security/resources/green-if-no-referrer.php'. This request was automatically upgraded to HTTPS, For more information see https://blog.chromium.org/2019/10/no-more-mixed-messages-about-https.html diff --git a/third_party/blink/web_tests/http/tests/security/referrer-policy-conflicting-policies-expected.txt b/third_party/blink/web_tests/http/tests/security/referrer-policy-conflicting-policies-expected.txt new file mode 100644 index 00000000000000..a5ae63b3e6cfa1 --- /dev/null +++ b/third_party/blink/web_tests/http/tests/security/referrer-policy-conflicting-policies-expected.txt @@ -0,0 +1 @@ +CONSOLE WARNING: Mixed Content: The page at 'https://127.0.0.1:8443/security/resources/referrer-policy-conflicting-policies.html' was loaded over HTTPS, but requested an insecure element 'http://127.0.0.1:8080/security/resources/green-if-no-referrer.php'. This request was automatically upgraded to HTTPS, For more information see https://blog.chromium.org/2019/10/no-more-mixed-messages-about-https.html diff --git a/third_party/blink/web_tests/http/tests/security/referrer-policy-conflicting-policies-image-set-expected.txt b/third_party/blink/web_tests/http/tests/security/referrer-policy-conflicting-policies-image-set-expected.txt new file mode 100644 index 00000000000000..7d1b9c0f5561bb --- /dev/null +++ b/third_party/blink/web_tests/http/tests/security/referrer-policy-conflicting-policies-image-set-expected.txt @@ -0,0 +1 @@ +CONSOLE WARNING: Mixed Content: The page at 'https://127.0.0.1:8443/security/resources/referrer-policy-conflicting-policies-image-set.html' was loaded over HTTPS, but requested an insecure element 'http://127.0.0.1:8080/security/resources/green-if-no-referrer.php'. This request was automatically upgraded to HTTPS, For more information see https://blog.chromium.org/2019/10/no-more-mixed-messages-about-https.html diff --git a/third_party/blink/web_tests/virtual/autoupgrade-optionally-blockable-mixed-content/http/tests/mixed-autoupgrade/optionally/README.txt b/third_party/blink/web_tests/virtual/autoupgrade-optionally-blockable-mixed-content/http/tests/mixed-autoupgrade/optionally/README.txt deleted file mode 100644 index 1d85d52c2a7fed..00000000000000 --- a/third_party/blink/web_tests/virtual/autoupgrade-optionally-blockable-mixed-content/http/tests/mixed-autoupgrade/optionally/README.txt +++ /dev/null @@ -1 +0,0 @@ -Tests that depend on the AutoupgradeMixedContent feature.