From cc0d15a8869e25eb02b5e829e24ae3933419760f Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 17 Jun 2016 15:03:30 -0600 Subject: [PATCH 01/11] Initial secure_headers config after some testing. --- Gemfile | 3 +++ Gemfile.lock | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/Gemfile b/Gemfile index 81e8ff60ad52..fbd899ccf09d 100644 --- a/Gemfile +++ b/Gemfile @@ -349,3 +349,6 @@ gem 'health_check', '~> 2.1.0' # System information gem 'vmstat', '~> 2.1.0' gem 'sys-filesystem', '~> 1.1.6' + +# Secure headers for Content Security Policy +gem 'secure_headers', '~> 3.3' diff --git a/Gemfile.lock b/Gemfile.lock index 0987fd5665a0..ace9f103b58c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -645,6 +645,8 @@ GEM sdoc (0.3.20) json (>= 1.1.3) rdoc (~> 3.10) + secure_headers (3.3.2) + useragent seed-fu (2.3.6) activerecord (>= 3.1) activesupport (>= 3.1) @@ -767,6 +769,7 @@ GEM get_process_mem (~> 0) unicorn (>= 4, < 6) uniform_notifier (1.9.0) + useragent (0.16.7) uuid (2.3.8) macaddr (~> 1.0) version_sorter (2.0.0) @@ -944,6 +947,7 @@ DEPENDENCIES sass-rails (~> 5.0.0) scss_lint (~> 0.47.0) sdoc (~> 0.3.20) + secure_headers (~> 3.3) seed-fu (~> 2.3.5) select2-rails (~> 3.5.9) sentry-raven (~> 1.1.0) -- GitLab From e8e608765e875814b89847d59b4699175746596a Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 17 Jun 2016 15:26:21 -0600 Subject: [PATCH 02/11] Fix that which hath been broken. Except the sidekiq admin iframe. --- .../admin/background_jobs_controller.rb | 4 ++ config/initializers/secure_headers.rb | 38 +++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 config/initializers/secure_headers.rb diff --git a/app/controllers/admin/background_jobs_controller.rb b/app/controllers/admin/background_jobs_controller.rb index 338496013a02..7ccbe7c4232d 100644 --- a/app/controllers/admin/background_jobs_controller.rb +++ b/app/controllers/admin/background_jobs_controller.rb @@ -2,5 +2,9 @@ class Admin::BackgroundJobsController < Admin::ApplicationController def show ps_output, _ = Gitlab::Popen.popen(%W(ps -U #{Gitlab.config.gitlab.user} -o pid,pcpu,pmem,stat,start,command)) @sidekiq_processes = ps_output.split("\n").grep(/sidekiq/) + + override_x_frame_options("SAMEORIGIN") + + override_content_security_policy_directives(frame_ancestors: %w('self')) end end diff --git a/config/initializers/secure_headers.rb b/config/initializers/secure_headers.rb new file mode 100644 index 000000000000..7ac4c7ace8e8 --- /dev/null +++ b/config/initializers/secure_headers.rb @@ -0,0 +1,38 @@ +SecureHeaders::Configuration.default do |config| + config.cookies = { + secure: true, # mark all cookies as "Secure" + httponly: true, # mark all cookies as "HttpOnly" + samesite: { + strict: true # mark all cookies as SameSite=Strict + } + } + config.x_frame_options = "DENY" + config.x_content_type_options = "nosniff" + config.x_xss_protection = "1; mode=block" + config.x_download_options = "noopen" + config.x_permitted_cross_domain_policies = "none" + config.referrer_policy = "origin-when-cross-origin" + config.csp = { + # "meta" values. these will shaped the header, but the values are not included in the header. + report_only: true, # default: false + preserve_schemes: true, # default: false. Schemes are removed from host sources to save bytes and discourage mixed content. + + # directive values: these values will directly translate into source directives + default_src: %w('none'), + frame_src: %w('self'), + connect_src: %w('self'), + font_src: %w('self'), + img_src: %w('self' www.gravatar.com secure.gravatar.com), + media_src: %w('none'), + object_src: %w('none'), + script_src: %w('unsafe-inline' 'unsafe-eval' 'self' maxcdn.bootstrapcdn.com), + style_src: %w('unsafe-inline' 'self'), + base_uri: %w('self'), + child_src: %w('self'), + form_action: %w('self'), + frame_ancestors: %w('none'), + block_all_mixed_content: true, # see http://www.w3.org/TR/mixed-content/ + upgrade_insecure_requests: true, # see https://www.w3.org/TR/upgrade-insecure-requests/ + report_uri: %w('') + } +end -- GitLab From 4984d1a6484017ea33778c8f743e47b9162aee21 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 17 Jun 2016 15:47:26 -0600 Subject: [PATCH 03/11] Remove unsafe eval directive from scripts. --- config/initializers/secure_headers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/secure_headers.rb b/config/initializers/secure_headers.rb index 7ac4c7ace8e8..075a5fc1876d 100644 --- a/config/initializers/secure_headers.rb +++ b/config/initializers/secure_headers.rb @@ -25,7 +25,7 @@ img_src: %w('self' www.gravatar.com secure.gravatar.com), media_src: %w('none'), object_src: %w('none'), - script_src: %w('unsafe-inline' 'unsafe-eval' 'self' maxcdn.bootstrapcdn.com), + script_src: %w('unsafe-inline' 'self' maxcdn.bootstrapcdn.com), style_src: %w('unsafe-inline' 'self'), base_uri: %w('self'), child_src: %w('self'), -- GitLab From e5d6f33378c302bc65b5637dfeff9d5a852647d5 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Mon, 20 Jun 2016 15:53:17 -0600 Subject: [PATCH 04/11] Update image policy to allow external images over HTTPS. --- config/initializers/secure_headers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/secure_headers.rb b/config/initializers/secure_headers.rb index 075a5fc1876d..3788dbf9473d 100644 --- a/config/initializers/secure_headers.rb +++ b/config/initializers/secure_headers.rb @@ -22,7 +22,7 @@ frame_src: %w('self'), connect_src: %w('self'), font_src: %w('self'), - img_src: %w('self' www.gravatar.com secure.gravatar.com), + img_src: %w('self' www.gravatar.com secure.gravatar.com https:), media_src: %w('none'), object_src: %w('none'), script_src: %w('unsafe-inline' 'self' maxcdn.bootstrapcdn.com), -- GitLab From e0ffbf0edb7bdda290225259945e0fb6e7b270bb Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Tue, 5 Jul 2016 14:20:50 -0600 Subject: [PATCH 05/11] Add the CSP reporting URI of Sentry. --- config/initializers/secure_headers.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/config/initializers/secure_headers.rb b/config/initializers/secure_headers.rb index 3788dbf9473d..66aca5fb46b3 100644 --- a/config/initializers/secure_headers.rb +++ b/config/initializers/secure_headers.rb @@ -1,3 +1,10 @@ +require 'gitlab/current_settings' +include Gitlab::CurrentSettings + +uri = URI.parse(current_application_settings.sentry_dsn) + +CSP_REPORT_URI = "#{uri.scheme}://#{uri.host}/api#{uri.path}/csp-report/?sentry_key=#{uri.user}" + SecureHeaders::Configuration.default do |config| config.cookies = { secure: true, # mark all cookies as "Secure" @@ -33,6 +40,6 @@ frame_ancestors: %w('none'), block_all_mixed_content: true, # see http://www.w3.org/TR/mixed-content/ upgrade_insecure_requests: true, # see https://www.w3.org/TR/upgrade-insecure-requests/ - report_uri: %w('') + report_uri: %W(#{CSP_REPORT_URI}) } end -- GitLab From 2e9bf6a750e92a729266ac6ed2f8e32385aa4ec4 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Tue, 5 Jul 2016 15:06:43 -0600 Subject: [PATCH 06/11] Add Sidekiq-specific headers. --- app/controllers/admin/background_jobs_controller.rb | 6 ++---- config/initializers/secure_headers.rb | 12 ++++++++++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/app/controllers/admin/background_jobs_controller.rb b/app/controllers/admin/background_jobs_controller.rb index 7ccbe7c4232d..133c6bc012bc 100644 --- a/app/controllers/admin/background_jobs_controller.rb +++ b/app/controllers/admin/background_jobs_controller.rb @@ -1,10 +1,8 @@ class Admin::BackgroundJobsController < Admin::ApplicationController def show + use_secure_headers_override(:background_jobs) + ps_output, _ = Gitlab::Popen.popen(%W(ps -U #{Gitlab.config.gitlab.user} -o pid,pcpu,pmem,stat,start,command)) @sidekiq_processes = ps_output.split("\n").grep(/sidekiq/) - - override_x_frame_options("SAMEORIGIN") - - override_content_security_policy_directives(frame_ancestors: %w('self')) end end diff --git a/config/initializers/secure_headers.rb b/config/initializers/secure_headers.rb index 66aca5fb46b3..e69117e05213 100644 --- a/config/initializers/secure_headers.rb +++ b/config/initializers/secure_headers.rb @@ -2,7 +2,6 @@ include Gitlab::CurrentSettings uri = URI.parse(current_application_settings.sentry_dsn) - CSP_REPORT_URI = "#{uri.scheme}://#{uri.host}/api#{uri.path}/csp-report/?sentry_key=#{uri.user}" SecureHeaders::Configuration.default do |config| @@ -32,7 +31,7 @@ img_src: %w('self' www.gravatar.com secure.gravatar.com https:), media_src: %w('none'), object_src: %w('none'), - script_src: %w('unsafe-inline' 'self' maxcdn.bootstrapcdn.com), + script_src: %w('unsafe-inline' 'self'), style_src: %w('unsafe-inline' 'self'), base_uri: %w('self'), child_src: %w('self'), @@ -42,4 +41,13 @@ upgrade_insecure_requests: true, # see https://www.w3.org/TR/upgrade-insecure-requests/ report_uri: %W(#{CSP_REPORT_URI}) } + + if Rails.env.development? + config.csp[:script_src] << "maxcdn.bootstrapcdn.com" + end +end + +SecureHeaders::Configuration.override(:background_jobs) do |config| + config.csp[:frame_ancestors] = %w('self') + config.x_frame_options = 'SAMEORIGIN' end -- GitLab From 3ee8eb113d019df8e5312af42fb2f369577c324c Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Tue, 5 Jul 2016 15:15:29 -0600 Subject: [PATCH 07/11] Only report to Sentry when it's enabled. --- config/initializers/secure_headers.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/config/initializers/secure_headers.rb b/config/initializers/secure_headers.rb index e69117e05213..6cc7484d7487 100644 --- a/config/initializers/secure_headers.rb +++ b/config/initializers/secure_headers.rb @@ -1,8 +1,12 @@ require 'gitlab/current_settings' include Gitlab::CurrentSettings -uri = URI.parse(current_application_settings.sentry_dsn) -CSP_REPORT_URI = "#{uri.scheme}://#{uri.host}/api#{uri.path}/csp-report/?sentry_key=#{uri.user}" +if Rails.env.production? && current_application_settings.sentry_enabled + uri = URI.parse(current_application_settings.sentry_dsn) + CSP_REPORT_URI = "#{uri.scheme}://#{uri.host}/api#{uri.path}/csp-report/?sentry_key=#{uri.user}" +else + CSP_REPORT_URI = '' +end SecureHeaders::Configuration.default do |config| config.cookies = { -- GitLab From fa56c34b478c39639abfc51fbde6f55b5641ab1e Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Tue, 5 Jul 2016 15:24:10 -0600 Subject: [PATCH 08/11] Remove background_jobs-specific headers. --- app/controllers/admin/background_jobs_controller.rb | 2 -- config/initializers/secure_headers.rb | 5 ----- 2 files changed, 7 deletions(-) diff --git a/app/controllers/admin/background_jobs_controller.rb b/app/controllers/admin/background_jobs_controller.rb index 133c6bc012bc..338496013a02 100644 --- a/app/controllers/admin/background_jobs_controller.rb +++ b/app/controllers/admin/background_jobs_controller.rb @@ -1,7 +1,5 @@ class Admin::BackgroundJobsController < Admin::ApplicationController def show - use_secure_headers_override(:background_jobs) - ps_output, _ = Gitlab::Popen.popen(%W(ps -U #{Gitlab.config.gitlab.user} -o pid,pcpu,pmem,stat,start,command)) @sidekiq_processes = ps_output.split("\n").grep(/sidekiq/) end diff --git a/config/initializers/secure_headers.rb b/config/initializers/secure_headers.rb index 6cc7484d7487..a704dd2ee7e6 100644 --- a/config/initializers/secure_headers.rb +++ b/config/initializers/secure_headers.rb @@ -50,8 +50,3 @@ config.csp[:script_src] << "maxcdn.bootstrapcdn.com" end end - -SecureHeaders::Configuration.override(:background_jobs) do |config| - config.csp[:frame_ancestors] = %w('self') - config.x_frame_options = 'SAMEORIGIN' -end -- GitLab From b2752c46f4884681b09f6562920d177918e66278 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Tue, 5 Jul 2016 17:52:44 -0600 Subject: [PATCH 09/11] Only enable CSP policies when relevant features are enabled. Gravatar, Google Analytics, Piwik, Recaptcha, etc. --- config/initializers/secure_headers.rb | 28 ++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/config/initializers/secure_headers.rb b/config/initializers/secure_headers.rb index a704dd2ee7e6..44425b74d437 100644 --- a/config/initializers/secure_headers.rb +++ b/config/initializers/secure_headers.rb @@ -32,7 +32,7 @@ frame_src: %w('self'), connect_src: %w('self'), font_src: %w('self'), - img_src: %w('self' www.gravatar.com secure.gravatar.com https:), + img_src: %w('self' https:), media_src: %w('none'), object_src: %w('none'), script_src: %w('unsafe-inline' 'self'), @@ -46,7 +46,33 @@ report_uri: %W(#{CSP_REPORT_URI}) } + # Allow Bootstrap Linter in development mode. if Rails.env.development? config.csp[:script_src] << "maxcdn.bootstrapcdn.com" end + + # Recaptcha + if current_application_settings.recaptcha_enabled + config.csp[:script_src] << "https://www.google.com/recaptcha/" + config.csp[:script_src] << "https://www.gstatic.com/recaptcha/" + config.csp[:frame_src] << "https://www.google.com/recaptcha/" + end + + # Gravatar + if current_application_settings.gravatar_enabled? + config.csp[:img_src] << "www.gravatar.com" + config.csp[:img_src] << "secure.gravatar.com" + config.csp[:img_src] << Gitlab.config.gravatar.host + end + + # Piwik + if Gitlab.config.extra.has_key?('piwik_url') && Gitlab.config.extra.has_key?('piwik_site_id') + config.csp[:script_src] << Gitlab.config.extra.piwik_url + config.csp[:img_src] << Gitlab.config.extra.piwik_url + end + + # Google Analytics + if Gitlab.config.extra.has_key?('google_analytics_id') + config.csp[:script_src] << "https://www.google-analytics.com" + end end -- GitLab From 460fc6c4f38120a4aa58f243bd3f0c8244902837 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Wed, 6 Jul 2016 16:00:55 -0600 Subject: [PATCH 10/11] Document the CSP file. --- config/initializers/secure_headers.rb | 49 +++++++++++++++++++++------ 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/config/initializers/secure_headers.rb b/config/initializers/secure_headers.rb index 44425b74d437..7a2f0eab3c0d 100644 --- a/config/initializers/secure_headers.rb +++ b/config/initializers/secure_headers.rb @@ -1,6 +1,8 @@ require 'gitlab/current_settings' include Gitlab::CurrentSettings +# If Sentry is enabled and the Rails app is running in production mode, +# this will construct the Report URI for Sentry. if Rails.env.production? && current_application_settings.sentry_enabled uri = URI.parse(current_application_settings.sentry_dsn) CSP_REPORT_URI = "#{uri.scheme}://#{uri.host}/api#{uri.path}/csp-report/?sentry_key=#{uri.user}" @@ -8,14 +10,20 @@ CSP_REPORT_URI = '' end +# Content Security Policy Headers +# For more information on CSP see: +# - https://gitlab.com/gitlab-org/gitlab-ce/issues/18231 +# - https://developer.mozilla.org/en-US/docs/Web/Security/CSP/CSP_policy_directives SecureHeaders::Configuration.default do |config| + # Mark all cookies as "Secure", "HttpOnly", and "SameSite=Strict". config.cookies = { - secure: true, # mark all cookies as "Secure" - httponly: true, # mark all cookies as "HttpOnly" + secure: true, + httponly: true, samesite: { - strict: true # mark all cookies as SameSite=Strict + strict: true } } + # Disallow iframes. config.x_frame_options = "DENY" config.x_content_type_options = "nosniff" config.x_xss_protection = "1; mode=block" @@ -23,26 +31,44 @@ config.x_permitted_cross_domain_policies = "none" config.referrer_policy = "origin-when-cross-origin" config.csp = { - # "meta" values. these will shaped the header, but the values are not included in the header. - report_only: true, # default: false - preserve_schemes: true, # default: false. Schemes are removed from host sources to save bytes and discourage mixed content. + # "Meta" values. + report_only: true, + preserve_schemes: true, - # directive values: these values will directly translate into source directives + # "Directive" values. + # Default source allows nothing, more permissive values are set per-policy. default_src: %w('none'), - frame_src: %w('self'), + # (Deprecated) Don't allow iframes. + frame_src: %w('none'), + # Only allow XMLHTTPRequests from the GitLab instance itself. connect_src: %w('self'), + # Only load local fonts. font_src: %w('self'), + # Load local images, any external image available over HTTPS. img_src: %w('self' https:), + # Audio and video can't be played on GitLab currently, so it's disabled. media_src: %w('none'), + # Don't allow , , or elements. object_src: %w('none'), + # Allow local scripts and inline scripts. script_src: %w('unsafe-inline' 'self'), + # Allow local stylesheets and inline styles. style_src: %w('unsafe-inline' 'self'), + # The URIs that a user agent may use as the document base URL. base_uri: %w('self'), + # Only allow local iframes and service workers child_src: %w('self'), + # Only submit form information to the GitLab instance. form_action: %w('self'), + # Disallow any parents from embedding a page in an iframe. frame_ancestors: %w('none'), - block_all_mixed_content: true, # see http://www.w3.org/TR/mixed-content/ - upgrade_insecure_requests: true, # see https://www.w3.org/TR/upgrade-insecure-requests/ + # Don't allow any plugins (Flash, Shockwave, etc.) + plugin_types: %w('none'), + # Blocks all mixed (HTTP) content. + block_all_mixed_content: true, + # Upgrades insecure requests to HTTPS when possible. + upgrade_insecure_requests: true, + # Reports are sent to Sentry if it's enabled, nowhere otherwise. report_uri: %W(#{CSP_REPORT_URI}) } @@ -51,11 +77,12 @@ config.csp[:script_src] << "maxcdn.bootstrapcdn.com" end - # Recaptcha + # reCAPTCHA if current_application_settings.recaptcha_enabled config.csp[:script_src] << "https://www.google.com/recaptcha/" config.csp[:script_src] << "https://www.gstatic.com/recaptcha/" config.csp[:frame_src] << "https://www.google.com/recaptcha/" + config.x_frame_options = "SAMEORIGIN" end # Gravatar -- GitLab From c2fe22f8f419a8e562f0f34e8c0f478aefc34ec0 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Mon, 18 Jul 2016 12:30:23 -0600 Subject: [PATCH 11/11] Minor policy refinements. --- config/initializers/secure_headers.rb | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/config/initializers/secure_headers.rb b/config/initializers/secure_headers.rb index 7a2f0eab3c0d..9fd24a667cc2 100644 --- a/config/initializers/secure_headers.rb +++ b/config/initializers/secure_headers.rb @@ -1,3 +1,6 @@ +# CSP headers have to have single quotes, so failures relating to quotes +# inside Ruby string arrays are irrelevant. +# rubocop:disable Lint/PercentStringArray require 'gitlab/current_settings' include Gitlab::CurrentSettings @@ -23,8 +26,6 @@ strict: true } } - # Disallow iframes. - config.x_frame_options = "DENY" config.x_content_type_options = "nosniff" config.x_xss_protection = "1; mode=block" config.x_download_options = "noopen" @@ -45,13 +46,13 @@ # Only load local fonts. font_src: %w('self'), # Load local images, any external image available over HTTPS. - img_src: %w('self' https:), + img_src: %w(* 'self' data:), # Audio and video can't be played on GitLab currently, so it's disabled. media_src: %w('none'), # Don't allow , , or elements. object_src: %w('none'), # Allow local scripts and inline scripts. - script_src: %w('unsafe-inline' 'self'), + script_src: %w('unsafe-inline' 'unsafe-eval' 'self'), # Allow local stylesheets and inline styles. style_src: %w('unsafe-inline' 'self'), # The URIs that a user agent may use as the document base URL. @@ -63,15 +64,18 @@ # Disallow any parents from embedding a page in an iframe. frame_ancestors: %w('none'), # Don't allow any plugins (Flash, Shockwave, etc.) - plugin_types: %w('none'), + plugin_types: %w(), # Blocks all mixed (HTTP) content. block_all_mixed_content: true, # Upgrades insecure requests to HTTPS when possible. - upgrade_insecure_requests: true, - # Reports are sent to Sentry if it's enabled, nowhere otherwise. - report_uri: %W(#{CSP_REPORT_URI}) + upgrade_insecure_requests: true } + # Reports are sent to Sentry if it's enabled. + if current_application_settings.sentry_enabled + config.csp[:report_uri] = %W(#{CSP_REPORT_URI}) + end + # Allow Bootstrap Linter in development mode. if Rails.env.development? config.csp[:script_src] << "maxcdn.bootstrapcdn.com" -- GitLab