Commit 5cb72d5f authored by Jun Kokatsu's avatar Jun Kokatsu Committed by Commit Bot

Enable Trusted Types mitigation on WebUI by default

This change enables Trusted Types mitigation on WebUI by default.
It enforces use of safe API by default, and any use of unsafe API
(e.g. innerHTML, document.write, etc) has to overwrite Trusted Types
directives in the CSP header. Therefore this change makes security
review or audit of JavaScript code on WebUI a lot easier. Because
JavaScript could introduce XSS only by using
`trustedTypes.createPolicy` to generate untrusted html or script as
Trusted Types, or removing Trusted Types mitigation. And all of
those has to be carefully reviewed going forward.

Bug: 41905
Change-Id: Iba7b8f617bc00c27b22386e2f632402209c47549
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2353547Reviewed-by: default avatarGiovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: default avatarJohn Delaney <johnidel@chromium.org>
Reviewed-by: default avatarCarlos IL <carlosil@chromium.org>
Reviewed-by: default avatardpapad <dpapad@chromium.org>
Commit-Queue: Jun Kokatsu <Jun.Kokatsu@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#800738}
parent c394db60
...@@ -25,7 +25,11 @@ namespace { ...@@ -25,7 +25,11 @@ namespace {
bool IsContentInDocument(content::RenderFrameHost* rfh, std::string content) { bool IsContentInDocument(content::RenderFrameHost* rfh, std::string content) {
std::string script = std::string script =
"document.documentElement.innerHTML.includes('" + content + "');"; "document.documentElement.innerHTML.includes('" + content + "');";
return EvalJs(rfh, script).ExtractBool(); // Execute script in an isolated world to avoid causing a Trusted Types
// violation due to eval.
return EvalJs(rfh, script, content::EXECUTE_SCRIPT_DEFAULT_OPTIONS,
/*world_id=*/1)
.ExtractBool();
} }
} // namespace } // namespace
......
...@@ -381,7 +381,7 @@ IN_PROC_BROWSER_TEST_P(SystemWebAppLinkCaptureBrowserTest, ...@@ -381,7 +381,7 @@ IN_PROC_BROWSER_TEST_P(SystemWebAppLinkCaptureBrowserTest,
browser()->tab_strip_model()->GetActiveWebContents(), browser()->tab_strip_model()->GetActiveWebContents(),
content::JsReplace("let el = document.createElement('a');" content::JsReplace("let el = document.createElement('a');"
"el.href = $1;" "el.href = $1;"
"el.innerHTML = 'Link to SWA Page 2';" "el.textContent = 'Link to SWA Page 2';"
"document.body.appendChild(el);" "document.body.appendChild(el);"
"el.click();", "el.click();",
kPageURL))); kPageURL)));
......
...@@ -4,7 +4,6 @@ ...@@ -4,7 +4,6 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/strings/string_piece.h" #include "base/strings/string_piece.h"
#include "base/test/scoped_feature_list.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/browser/ui/tabs/tab_strip_model.h"
...@@ -17,7 +16,6 @@ ...@@ -17,7 +16,6 @@
#include "content/public/browser/notification_service.h" #include "content/public/browser/notification_service.h"
#include "content/public/browser/notification_source.h" #include "content/public/browser/notification_source.h"
#include "content/public/browser/notification_types.h" #include "content/public/browser/notification_types.h"
#include "content/public/common/content_features.h"
#include "content/public/test/browser_test.h" #include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h" #include "content/public/test/browser_test_utils.h"
...@@ -140,13 +138,11 @@ IN_PROC_BROWSER_TEST_F(ChromeURLDataManagerTest, LargeResourceScale) { ...@@ -140,13 +138,11 @@ IN_PROC_BROWSER_TEST_F(ChromeURLDataManagerTest, LargeResourceScale) {
EXPECT_NE(net::OK, observer.net_error()); EXPECT_NE(net::OK, observer.net_error());
} }
class ChromeURLDataManagerTestWithWebUIReportOnlyTrustedTypesEnabled class ChromeURLDataManagerWebUITrustedTypesTest
: public InProcessBrowserTest, : public InProcessBrowserTest,
public testing::WithParamInterface<const char*> { public testing::WithParamInterface<const char*> {
public: public:
ChromeURLDataManagerTestWithWebUIReportOnlyTrustedTypesEnabled() { ChromeURLDataManagerWebUITrustedTypesTest() = default;
feature_list_.InitAndEnableFeature(features::kWebUIReportOnlyTrustedTypes);
}
void CheckTrustedTypesViolation(base::StringPiece url) { void CheckTrustedTypesViolation(base::StringPiece url) {
std::string message_filter1 = "*This document requires*assignment*"; std::string message_filter1 = "*This document requires*assignment*";
...@@ -164,15 +160,11 @@ class ChromeURLDataManagerTestWithWebUIReportOnlyTrustedTypesEnabled ...@@ -164,15 +160,11 @@ class ChromeURLDataManagerTestWithWebUIReportOnlyTrustedTypesEnabled
content::WaitForLoadStop(content); content::WaitForLoadStop(content);
EXPECT_TRUE(console_observer.messages().empty()); EXPECT_TRUE(console_observer.messages().empty());
} }
private:
base::test::ScopedFeatureList feature_list_;
}; };
// Verify that there's no Trusted Types violation in chrome://chrome-urls // Verify that there's no Trusted Types violation in chrome://chrome-urls
IN_PROC_BROWSER_TEST_P( IN_PROC_BROWSER_TEST_P(ChromeURLDataManagerWebUITrustedTypesTest,
ChromeURLDataManagerTestWithWebUIReportOnlyTrustedTypesEnabled, NoTrustedTypesViolation) {
NoTrustedTypesViolation) {
CheckTrustedTypesViolation(GetParam()); CheckTrustedTypesViolation(GetParam());
} }
...@@ -325,7 +317,6 @@ static constexpr const char* const kChromeUrls[] = { ...@@ -325,7 +317,6 @@ static constexpr const char* const kChromeUrls[] = {
#endif #endif
}; };
INSTANTIATE_TEST_SUITE_P( INSTANTIATE_TEST_SUITE_P(,
, ChromeURLDataManagerWebUITrustedTypesTest,
ChromeURLDataManagerTestWithWebUIReportOnlyTrustedTypesEnabled, ::testing::ValuesIn(kChromeUrls));
::testing::ValuesIn(kChromeUrls));
...@@ -5,6 +5,8 @@ ...@@ -5,6 +5,8 @@
<meta name="theme-color" content="#fff"> <meta name="theme-color" content="#fff">
<meta name="viewport" <meta name="viewport"
content="initial-scale=1, minimum-scale=1, width=device-width"> content="initial-scale=1, minimum-scale=1, width=device-width">
<meta http-equiv="Content-Security-Policy"
content="require-trusted-types-for 'script'; trusted-types;">
<title>$i18n{tabTitle}</title> <title>$i18n{tabTitle}</title>
<link rel="stylesheet" href="../../common/resources/interstitial_core.css"> <link rel="stylesheet" href="../../common/resources/interstitial_core.css">
<link rel="stylesheet" href="../../common/resources/interstitial_common.css"> <link rel="stylesheet" href="../../common/resources/interstitial_common.css">
......
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
<meta charset="utf-8"> <meta charset="utf-8">
<meta name="viewport" <meta name="viewport"
content="initial-scale=1, minimum-scale=1, width=device-width"> content="initial-scale=1, minimum-scale=1, width=device-width">
<meta http-equiv="Content-Security-Policy"
content="require-trusted-types-for 'script'; trusted-types jstemplate;">
<title>$i18n{tabTitle}</title> <title>$i18n{tabTitle}</title>
<link rel="stylesheet" href="../../common/resources/interstitial_core.css"> <link rel="stylesheet" href="../../common/resources/interstitial_core.css">
<link rel="stylesheet" href="interstitial_webview_quiet.css"> <link rel="stylesheet" href="interstitial_webview_quiet.css">
......
...@@ -51,6 +51,8 @@ const char kChromeURLContentSecurityPolicyHeaderName[] = ...@@ -51,6 +51,8 @@ const char kChromeURLContentSecurityPolicyHeaderName[] =
"Content-Security-Policy"; "Content-Security-Policy";
const char kChromeURLContentSecurityPolicyReportOnlyHeaderName[] = const char kChromeURLContentSecurityPolicyReportOnlyHeaderName[] =
"Content-Security-Policy-Report-Only"; "Content-Security-Policy-Report-Only";
const char kChromeURLContentSecurityPolicyReportOnlyHeaderValue[] =
"require-trusted-types-for 'script'";
const char kChromeURLXFrameOptionsHeaderName[] = "X-Frame-Options"; const char kChromeURLXFrameOptionsHeaderName[] = "X-Frame-Options";
const char kChromeURLXFrameOptionsHeaderValue[] = "DENY"; const char kChromeURLXFrameOptionsHeaderValue[] = "DENY";
...@@ -167,8 +169,10 @@ scoped_refptr<net::HttpResponseHeaders> URLDataManagerBackend::GetHeaders( ...@@ -167,8 +169,10 @@ scoped_refptr<net::HttpResponseHeaders> URLDataManagerBackend::GetHeaders(
network::mojom::CSPDirectiveName::ImgSrc, network::mojom::CSPDirectiveName::ImgSrc,
network::mojom::CSPDirectiveName::MediaSrc, network::mojom::CSPDirectiveName::MediaSrc,
network::mojom::CSPDirectiveName::ObjectSrc, network::mojom::CSPDirectiveName::ObjectSrc,
network::mojom::CSPDirectiveName::RequireTrustedTypesFor,
network::mojom::CSPDirectiveName::ScriptSrc, network::mojom::CSPDirectiveName::ScriptSrc,
network::mojom::CSPDirectiveName::StyleSrc, network::mojom::CSPDirectiveName::StyleSrc,
network::mojom::CSPDirectiveName::TrustedTypes,
network::mojom::CSPDirectiveName::WorkerSrc}; network::mojom::CSPDirectiveName::WorkerSrc};
for (auto& directive : kAllDirectives) { for (auto& directive : kAllDirectives) {
...@@ -192,15 +196,9 @@ scoped_refptr<net::HttpResponseHeaders> URLDataManagerBackend::GetHeaders( ...@@ -192,15 +196,9 @@ scoped_refptr<net::HttpResponseHeaders> URLDataManagerBackend::GetHeaders(
kChromeURLXFrameOptionsHeaderValue); kChromeURLXFrameOptionsHeaderValue);
} }
if (base::FeatureList::IsEnabled(features::kWebUIReportOnlyTrustedTypes) && if (base::FeatureList::IsEnabled(features::kWebUIReportOnlyTrustedTypes)) {
source->ShouldAddContentSecurityPolicy()) {
std::string csp_report_only_header;
csp_report_only_header.append(source->GetContentSecurityPolicy(
network::mojom::CSPDirectiveName::RequireTrustedTypesFor));
csp_report_only_header.append(source->GetContentSecurityPolicy(
network::mojom::CSPDirectiveName::TrustedTypes));
headers->SetHeader(kChromeURLContentSecurityPolicyReportOnlyHeaderName, headers->SetHeader(kChromeURLContentSecurityPolicyReportOnlyHeaderName,
csp_report_only_header); kChromeURLContentSecurityPolicyReportOnlyHeaderValue);
} }
if (!source->AllowCaching()) if (!source->AllowCaching())
......
...@@ -9,7 +9,6 @@ ...@@ -9,7 +9,6 @@
#include "base/path_service.h" #include "base/path_service.h"
#include "base/strings/strcat.h" #include "base/strings/strcat.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/test/scoped_feature_list.h"
#include "base/threading/thread_restrictions.h" #include "base/threading/thread_restrictions.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "content/browser/child_process_security_policy_impl.h" #include "content/browser/child_process_security_policy_impl.h"
...@@ -23,7 +22,6 @@ ...@@ -23,7 +22,6 @@
#include "content/public/browser/web_ui_controller.h" #include "content/public/browser/web_ui_controller.h"
#include "content/public/browser/web_ui_data_source.h" #include "content/public/browser/web_ui_data_source.h"
#include "content/public/common/bindings_policy.h" #include "content/public/common/bindings_policy.h"
#include "content/public/common/content_features.h"
#include "content/public/common/content_paths.h" #include "content/public/common/content_paths.h"
#include "content/public/common/url_constants.h" #include "content/public/common/url_constants.h"
#include "content/public/test/browser_test.h" #include "content/public/test/browser_test.h"
...@@ -535,21 +533,9 @@ IN_PROC_BROWSER_TEST_F(WebUISecurityTest, ...@@ -535,21 +533,9 @@ IN_PROC_BROWSER_TEST_F(WebUISecurityTest,
.ExtractBool()); .ExtractBool());
} }
class WebUISecurityTestWithWebUIReportOnlyTrustedTypesEnabled // Verify that Trusted Types will block assignment to a dangerous sink
: public WebUISecurityTest { // on WebUI by default.
public: IN_PROC_BROWSER_TEST_F(WebUISecurityTest, BlockSinkAssignmentWithTrustedTypes) {
WebUISecurityTestWithWebUIReportOnlyTrustedTypesEnabled() {
feature_list_.InitAndEnableFeature(features::kWebUIReportOnlyTrustedTypes);
}
private:
base::test::ScopedFeatureList feature_list_;
};
// Verify Report-Only Trusted Types won't block assignment to a dangerous sink,
// but logs warning
IN_PROC_BROWSER_TEST_F(WebUISecurityTestWithWebUIReportOnlyTrustedTypesEnabled,
DoNotBlockSinkAssignmentOnReportOnlyTrustedTypes) {
ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(embedded_test_server()->Start());
GURL test_url(GetWebUIURL("web-ui/title1.html")); GURL test_url(GetWebUIURL("web-ui/title1.html"));
...@@ -559,17 +545,17 @@ IN_PROC_BROWSER_TEST_F(WebUISecurityTestWithWebUIReportOnlyTrustedTypesEnabled, ...@@ -559,17 +545,17 @@ IN_PROC_BROWSER_TEST_F(WebUISecurityTestWithWebUIReportOnlyTrustedTypesEnabled,
"(() => {" "(() => {"
" try {" " try {"
" document.body.innerHTML = 1;" " document.body.innerHTML = 1;"
" return 'Assignment succeeded';" " throw 'Assignment should have blocked';"
" } catch(e) {" " } catch(e) {"
" throw 'Assignment should have succeeded';" " return 'Assignment blocked';"
" }" " }"
"})();"; "})();";
{ {
WebContentsConsoleObserver console_observer(shell()->web_contents()); WebContentsConsoleObserver console_observer(shell()->web_contents());
console_observer.SetPattern( console_observer.SetPattern(
"[Report Only] This document requires 'TrustedHTML' assignment."); "This document requires 'TrustedHTML' assignment.");
EXPECT_EQ("Assignment succeeded", EXPECT_EQ("Assignment blocked",
EvalJs(shell(), kDangerousSinkUse, EXECUTE_SCRIPT_DEFAULT_OPTIONS, EvalJs(shell(), kDangerousSinkUse, EXECUTE_SCRIPT_DEFAULT_OPTIONS,
1 /* world_id */)); 1 /* world_id */));
console_observer.Wait(); console_observer.Wait();
......
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