Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CPDNPQ-2485] make content security policy more secure #2170

Closed
wants to merge 4 commits into from
Closed

[CPDNPQ-2485] make content security policy more secure #2170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3b9c7df
make content security policy more secure
alkesh Feb 3, 2025
5fb6ca6
remove csp_reports route and controller
alkesh Feb 4, 2025
c4fb6e6
use nonced_javascript_tag instead of script tags
alkesh Feb 4, 2025
1a168bb
allow unsafe-inline for flipper-ui
alkesh Feb 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 0 additions & 35 deletions app/controllers/csp_reports_controller.rb
View file
Open in desktop

This file was deleted.

6 changes: 3 additions & 3 deletions app/views/layouts/admin.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@

<!DOCTYPE html>
<html lang="en" class="govuk-template govuk-template--wide">
<%= render partial: "layouts/shared/head" %>

<body class="govuk-template__body ">
<%= render partial: "shared/analytics/google_noscript" %>
<%= render partial: "layouts/shared/govuk_javascript" %>

<script>
<%= nonced_javascript_tag do %>
document.body.className = ((document.body.className) ? document.body.className + ' js-enabled' : 'js-enabled');
</script>
<% end %>

<a href="#main-content" class="govuk-skip-link">Skip to main content</a>

Expand Down
4 changes: 2 additions & 2 deletions app/views/layouts/api_docs.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
<%= render partial: "layouts/shared/govuk_javascript" %>
<%= render partial: "shared/analytics/google_noscript" %>

<script>
<%= nonced_javascript_tag do %>
document.body.className = ((document.body.className) ? document.body.className + ' js-enabled' : 'js-enabled');
</script>
<% end %>

<a href="#main-content" class="govuk-skip-link">Skip to main content</a>

Expand Down
4 changes: 2 additions & 2 deletions app/views/layouts/shared/_govuk_javascript.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<script nonce="<%= request.content_security_policy_nonce %>">
<%= nonced_javascript_tag do %>
document.body.className += ' js-enabled' + ('noModule' in HTMLScriptElement.prototype ? ' govuk-frontend-supported' : '');
</script>
<% end %>
4 changes: 2 additions & 2 deletions app/views/layouts/shared/_head.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@
<%= yield :head %>

<% if Rails.env.development? %>
<script>
<%= nonced_javascript_tag do %>
console.log(<%= session.to_json.html_safe %>)
</script>
<% end %>
<% end %>
</head>
8 changes: 4 additions & 4 deletions app/views/shared/analytics/_google.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
<% if cookies["consented-to-cookies"] == "accept" %>
<script>(function(w,d,s,l,i){w[l]=w[l]||[];w[l].push({'gtm.start':
<%= nonced_javascript_tag do %>(function(w,d,s,l,i){w[l]=w[l]||[];w[l].push({'gtm.start':
new Date().getTime(),event:'gtm.js'});var f=d.getElementsByTagName(s)[0],
j=d.createElement(s),dl=l!='dataLayer'?'&l='+l:'';j.async=true;j.src=
'https://www.googletagmanager.com/gtm.js?id='+i+dl;f.parentNode.insertBefore(j,f);
})(window,document,'script','dataLayer','<%= Analytics::GOOGLE_TAG_MANAGER_ID %>');</script>
})(window,document,'script','dataLayer','<%= Analytics::GOOGLE_TAG_MANAGER_ID %>');<% end %>

<script async src="https://www.googletagmanager.com/gtag/js?id=<%= Analytics::GOOGLE_ANALYTICS_ID %>"></script>

<script>
<%= nonced_javascript_tag do %>
window.dataLayer = window.dataLayer || [];
function gtag(){dataLayer.push(arguments);}
gtag('js', new Date());
gtag('config', '<%= Analytics::GOOGLE_ANALYTICS_ID %>');
</script>
<% end %>
<% end %>
26 changes: 1 addition & 25 deletions config/initializers/content_security_policy.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1 @@
# Be sure to restart your server when you modify this file.

# Define an application-wide content security policy.
# See the Securing Rails Applications Guide for more information:
# https://guides.rubyonrails.org/security.html#content-security-policy-header

# Rails.application.configure do
# config.content_security_policy do |policy|
# policy.default_src :self, :https
# policy.font_src :self, :https, :data
# policy.img_src :self, :https, :data
# policy.object_src :none
# policy.script_src :self, :https
# policy.style_src :self, :https
# # Specify URI for violation reports
# # policy.report_uri "/csp-violation-report-endpoint"
# end
#
# # Generate session nonces for permitted importmap and inline scripts
# config.content_security_policy_nonce_generator = ->(request) { request.session.id.to_s }
# config.content_security_policy_nonce_directives = %w(script-src)
#
# # Report violations without enforcing the policy.
# # config.content_security_policy_report_only = true
# end
# CSP config is in config/initializers/secure_headers.rb
9 changes: 0 additions & 9 deletions config/initializers/rack_attack.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ def self.protected_path?(request)
PROTECTED_ROUTES.any? { |route| request.path.starts_with?(route) }
end

def self.csp_report_path?(request)
request.path == "/csp_reports"
end

def self.get_an_identity_webhook_path?(request)
request.path.starts_with?("/api/v1/get_an_identity/webhook_messages")
end
Expand All @@ -48,11 +44,6 @@ def self.auth_token(request)
request.ip if protected_path?(request)
end

# Throttle /csp_reports requests by IP (5rpm)
throttle("csp_reports req/ip", limit: 5, period: 1.minute) do |request|
request.ip if csp_report_path?(request)
end

# Throttle /api/v1/get_an_identity/webhook_messages requests by IP (1000 requests per 5 minutes)
throttle("API get an identity webhook message requests by ip", limit: 1000, period: 5.minutes) do |request|
request.ip if get_an_identity_webhook_path?(request)
Expand Down
20 changes: 13 additions & 7 deletions config/initializers/secure_headers.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# frozen_string_literal: true

# need to disable Lint/PercentStringArray here, because CSP keywords need to be single-quoted
# rubocop:disable Lint/PercentStringArray
SecureHeaders::Configuration.default do |config|
config.x_frame_options = "DENY"
Expand All @@ -9,8 +10,14 @@
config.x_permitted_cross_domain_policies = "none"
config.referrer_policy = %w[origin-when-cross-origin strict-origin-when-cross-origin]

google_analytics = %w[www.google-analytics.com ssl.google-analytics.com *.googletagmanager.com tagmanager.google.com *.googleusercontent.com *.gstatic.com]
google_analytics = %w[*.google-analytics.com *.analytics.google.com *.googletagmanager.com tagmanager.google.com *.googleusercontent.com *.gstatic.com]
tracking_pixels = %w[www.facebook.com px.ads.linkedin.com]
sentry = []

if ENV["SENTRY_REPORT_URI"]
sentry_report_uri = [ENV["SENTRY_REPORT_URI"]]
sentry = [sentry_report_uri.host]
end

config.csp = SecureHeaders::OPT_OUT

Expand All @@ -19,19 +26,18 @@
base_uri: %w['self'],
upgrade_insecure_requests: true,
child_src: %w['self'],
connect_src: %W['self'] + google_analytics,
connect_src: %w['self'] + google_analytics + sentry,
font_src: %w['self' *.gov.uk fonts.gstatic.com],
form_action: %w['self'],
frame_ancestors: %w['self'],
frame_src: %w['self'] + google_analytics,
img_src: %W['self' data: *.gov.uk] + google_analytics + tracking_pixels,
img_src: %w['self' data: *.gov.uk] + google_analytics + tracking_pixels,
manifest_src: %w['self'],
media_src: %w['self'],
script_src: %W['self' 'unsafe-inline' 'unsafe-eval' *.gov.uk https://cdn.jsdelivr.net/npm/chart.js] + google_analytics,
style_src: %w['self' 'unsafe-inline' *.gov.uk fonts.googleapis.com] + google_analytics,
script_src: %w['self' *.gov.uk https://cdn.jsdelivr.net/npm/chart.js] + google_analytics,
style_src: %w['self' 'unsafe-inline' *.gov.uk fonts.googleapis.com] + google_analytics, # unsafe-inline is needed for Flipper:UI
worker_src: %w['self'],
# upgrade_insecure_requests: !Rails.env.development?, # see https://www.w3.org/TR/upgrade-insecure-requests/
report_uri: %w[/csp_reports],
report_uri: sentry_report_uri,
}
end
# rubocop:enable Lint/PercentStringArray
2 changes: 0 additions & 2 deletions config/routes.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,6 @@
end
end

resource :csp_reports, only: %i[create]

get "maintenance_banners/dismiss", to: "maintenance_banners#dismiss", as: :maintenance_banner_dismiss

get "/404", to: "errors#not_found", via: :all
Expand Down
40 changes: 0 additions & 40 deletions spec/requests/csp_reports_controller_spec.rb
View file
Open in desktop

This file was deleted.

10 changes: 0 additions & 10 deletions spec/requests/rate_limiting_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,6 @@ def change_condition
end
end

it_behaves_like "a rate limited endpoint", "csp_reports req/ip" do
def perform_request
post csp_reports_path, params: {}.to_json
end

def change_condition
set_request_ip(other_ip)
end
end

it_behaves_like "a rate limited endpoint", "API get an identity webhook message requests by ip" do
before do
default_headers["X-Hub-Signature-256"] = "signature"
Expand Down
Loading