FEATURE: Enable strict-dynamic Content-Security-Policy by default (#26051)

Ref https://meta.discourse.org/t/298172 and https://meta.discourse.org/t/295603
This commit is contained in:
David Taylor 2024-03-07 15:20:31 +00:00 committed by GitHub
parent ac0808a320
commit 92d357f91a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 43 additions and 20 deletions

View File

@ -1746,8 +1746,8 @@ en:
content_security_policy_report_only: "Enable Content-Security-Policy-Report-Only (CSP)" content_security_policy_report_only: "Enable Content-Security-Policy-Report-Only (CSP)"
content_security_policy_collect_reports: "Enable CSP violation report collection at /csp_reports" content_security_policy_collect_reports: "Enable CSP violation report collection at /csp_reports"
content_security_policy_frame_ancestors: "Restrict who can embed this site in iframes via CSP. Control allowed hosts on <a href='%{base_path}/admin/customize/embedding'>Embedding</a>" content_security_policy_frame_ancestors: "Restrict who can embed this site in iframes via CSP. Control allowed hosts on <a href='%{base_path}/admin/customize/embedding'>Embedding</a>"
content_security_policy_script_src: "Additional allowlisted script sources. The current host and CDN are included by default. See <a href='https://meta.discourse.org/t/mitigate-xss-attacks-with-content-security-policy/104243' target='_blank'>Mitigate XSS Attacks with Content Security Policy.</a> (CSP)" content_security_policy_script_src: "Additional allowlisted script sources. The current host and CDN are included by default. See <a href='https://meta.discourse.org/t/mitigate-xss-attacks-with-content-security-policy/104243' target='_blank'>Mitigate XSS Attacks with Content Security Policy.</a> (CSP). Host sources will be ignored when content_security_policy_strict_dynamic is enabled."
content_security_policy_strict_dynamic: "EXPERIMENTAL: Use a strict-dynamic content security policy. This is more modern and more flexible than our default CSP configuration. This is fully functional, but not yet tested with all themes and plugins." content_security_policy_strict_dynamic: "Use a strict-dynamic content security policy to reduce the need for manual configuration. Check <a href='https://meta.discourse.org/t/298172' target='_blank'>the announcement</a> for more information. The ability to disable this setting will be removed soon."
invalidate_inactive_admin_email_after_days: "Admin accounts that have not visited the site in this number of days will need to re-validate their email address before logging in. Set to 0 to disable." invalidate_inactive_admin_email_after_days: "Admin accounts that have not visited the site in this number of days will need to re-validate their email address before logging in. Set to 0 to disable."
include_secure_categories_in_tag_counts: "When enabled, count of topics for a tag will include topics that are in read restricted categories for all users. When disabled, normal users are only shown a count of topics for a tag where all the topics are in public categories." include_secure_categories_in_tag_counts: "When enabled, count of topics for a tag will include topics that are in read restricted categories for all users. When disabled, normal users are only shown a count of topics for a tag where all the topics are in public categories."
display_personal_messages_tag_counts: "When enabled, count of personal messages tagged with a given tag will be displayed." display_personal_messages_tag_counts: "When enabled, count of personal messages tagged with a given tag will be displayed."

View File

@ -1992,7 +1992,7 @@ security:
type: simple_list type: simple_list
default: "" default: ""
content_security_policy_strict_dynamic: content_security_policy_strict_dynamic:
default: false default: true
invalidate_inactive_admin_email_after_days: invalidate_inactive_admin_email_after_days:
default: 365 default: 365
min: 0 min: 0

View File

@ -75,7 +75,8 @@ class ContentSecurityPolicy
sources = Array(sources).map { |s| normalize_source(s) } sources = Array(sources).map { |s| normalize_source(s) }
if SiteSetting.content_security_policy_strict_dynamic if SiteSetting.content_security_policy_strict_dynamic &&
%w[script-src worker-src].include?(directive.to_s)
# Strip any sources which are ignored under strict-dynamic # Strip any sources which are ignored under strict-dynamic
# If/when we make strict-dynamic the only option, we could print deprecation warnings # If/when we make strict-dynamic the only option, we could print deprecation warnings
# asking plugin/theme authors to remove the unnecessary config # asking plugin/theme authors to remove the unnecessary config

View File

@ -2,6 +2,8 @@
RSpec.describe "content security policy integration" do RSpec.describe "content security policy integration" do
it "adds the csp headers correctly" do it "adds the csp headers correctly" do
Fabricate(:admin) # to avoid 'new installation' screen
SiteSetting.content_security_policy = false SiteSetting.content_security_policy = false
get "/" get "/"
expect(response.headers["Content-Security-Policy"]).to eq(nil) expect(response.headers["Content-Security-Policy"]).to eq(nil)
@ -9,9 +11,15 @@ RSpec.describe "content security policy integration" do
SiteSetting.content_security_policy = true SiteSetting.content_security_policy = true
get "/" get "/"
expect(response.headers["Content-Security-Policy"]).to be_present expect(response.headers["Content-Security-Policy"]).to be_present
expect(response.headers["Content-Security-Policy"]).to match(
/script-src 'nonce-[^']+' 'strict-dynamic';/,
)
end end
context "with different hostnames" do context "with different hostnames - legacy" do
before { SiteSetting.content_security_policy_strict_dynamic = false }
before do before do
SiteSetting.content_security_policy = true SiteSetting.content_security_policy = true
RailsMultisite::ConnectionManagement.stubs(:current_db_hostnames).returns( RailsMultisite::ConnectionManagement.stubs(:current_db_hostnames).returns(
@ -39,7 +47,9 @@ RSpec.describe "content security policy integration" do
end end
end end
context "with different protocols" do context "with different protocols - legacy" do
before { SiteSetting.content_security_policy_strict_dynamic = false }
it "forces https when the site setting is enabled" do it "forces https when the site setting is enabled" do
SiteSetting.force_https = true SiteSetting.force_https = true
get "/" get "/"

View File

@ -40,7 +40,8 @@ RSpec.describe ContentSecurityPolicy do
end end
end end
describe "worker-src" do describe "legacy worker-src" do
before { SiteSetting.content_security_policy_strict_dynamic = false }
it "has expected values" do it "has expected values" do
worker_srcs = parse(policy)["worker-src"] worker_srcs = parse(policy)["worker-src"]
expect(worker_srcs).to eq( expect(worker_srcs).to eq(
@ -54,7 +55,8 @@ RSpec.describe ContentSecurityPolicy do
end end
end end
describe "script-src" do describe "legacy script-src" do
before { SiteSetting.content_security_policy_strict_dynamic = false }
it "always has self, logster, sidekiq, and assets" do it "always has self, logster, sidekiq, and assets" do
script_srcs = parse(policy)["script-src"] script_srcs = parse(policy)["script-src"]
expect(script_srcs).to include( expect(script_srcs).to include(
@ -185,6 +187,18 @@ RSpec.describe ContentSecurityPolicy do
end end
end end
describe "strict-dynamic script-src and worker-src" do
it "includes strict-dynamic keyword" do
script_srcs = parse(policy)["script-src"]
expect(script_srcs).to include("'strict-dynamic'")
end
it "does not set worker-src" do
worker_src = parse(policy)["worker-src"]
expect(worker_src).to eq(nil)
end
end
describe "manifest-src" do describe "manifest-src" do
it "is set to self" do it "is set to self" do
expect(parse(policy)["manifest-src"]).to eq(["'self'"]) expect(parse(policy)["manifest-src"]).to eq(["'self'"])
@ -231,7 +245,9 @@ RSpec.describe ContentSecurityPolicy do
end end
end end
it "can extend script-src, object-src, manifest-src" do it "can extend script-src, object-src, manifest-src - legacy" do
SiteSetting.content_security_policy_strict_dynamic = false
plugin = plugin_class.new(nil, "#{Rails.root}/spec/fixtures/plugins/csp_extension/plugin.rb") plugin = plugin_class.new(nil, "#{Rails.root}/spec/fixtures/plugins/csp_extension/plugin.rb")
plugin.activate! plugin.activate!
@ -272,13 +288,9 @@ RSpec.describe ContentSecurityPolicy do
end end
end end
it "only includes unsafe-inline for qunit paths" do context "with a theme - legacy" do
expect(parse(policy(path_info: "/qunit"))["script-src"]).to include("'unsafe-eval'") before { SiteSetting.content_security_policy_strict_dynamic = false }
expect(parse(policy(path_info: "/wizard/qunit"))["script-src"]).to include("'unsafe-eval'")
expect(parse(policy(path_info: "/"))["script-src"]).to_not include("'unsafe-eval'")
end
context "with a theme" do
let!(:theme) do let!(:theme) do
Fabricate(:theme).tap do |t| Fabricate(:theme).tap do |t|
settings = <<~YML settings = <<~YML
@ -379,9 +391,9 @@ RSpec.describe ContentSecurityPolicy do
end end
it "can be extended by site setting" do it "can be extended by site setting" do
SiteSetting.content_security_policy_script_src = "from-site-setting.com|from-site-setting.net" SiteSetting.content_security_policy_script_src = "'unsafe-eval'"
expect(parse(policy)["script-src"]).to include("from-site-setting.com", "from-site-setting.net") expect(parse(policy)["script-src"]).to include("'unsafe-eval'")
end end
def parse(csp_string) def parse(csp_string)

View File

@ -647,14 +647,14 @@ RSpec.describe ApplicationController do
get "/" get "/"
script_src = parse(response.headers["Content-Security-Policy"])["script-src"] script_src = parse(response.headers["Content-Security-Policy"])["script-src"]
expect(script_src).to_not include("example.com") expect(script_src).to_not include("'unsafe-inline'")
SiteSetting.content_security_policy_script_src = "example.com" SiteSetting.content_security_policy_script_src = "'unsafe-inline'"
get "/" get "/"
script_src = parse(response.headers["Content-Security-Policy"])["script-src"] script_src = parse(response.headers["Content-Security-Policy"])["script-src"]
expect(script_src).to include("example.com") expect(script_src).to include("'unsafe-inline'")
end end
it "does not set CSP when responding to non-HTML" do it "does not set CSP when responding to non-HTML" do