Skip to content

Add a keyword for default values in the CSP

Dominic Couture requested to merge dcouture-csp-merge-logic into master

What does this MR do and why?

When configuring the CSP for the gprd-cny environment I hit a little problem where the values I didn't specify in the YAML were overwritten by the values from the grpd environment. To avoid this I would need to explicitly set each directive to empty but that would basically disable the CSP. More details in gitlab-com/gl-infra/k8s-workloads/gitlab-com!1837 (comment 980188906)

To circumvent this problem, I'm introducing a <default_value> special value that isn't falsey but has the same effect and falls back to default values.

Screenshots or screen recordings

These are strongly recommended to assist reviewers and reduce the time to merge your change.

How to set up and validate locally

In your gitlab.yml compare the behavior between

  gitlab:
    content_security_policy:
      enabled: true
      report_only: false
      directives:
        script_src: false
$ curl http://127.0.0.1:3000/ -is | grep -i 'content-security-policy.*script-src'
$ # no result

and

  gitlab:
    content_security_policy:
      enabled: true
      report_only: false
      directives:
        script_src: <default_value>
$ curl http://127.0.0.1:3000/ -is | grep -i 'content-security-policy.*script-src'
Content-Security-Policy: base-uri 'self'; child-src https://www.google.com/recaptcha/ https://www.recaptcha.net/ https://content.googleapis.com https://content-compute.googleapis.com https://content-cloudbilling.googleapis.com https://content-cloudresourcemanager.googleapis.com https://www.googletagmanager.com/ns.html http://127.0.0.1:3000/rails/letter_opener/ http://127.0.0.1:3000/admin/ http://127.0.0.1:3000/assets/ http://127.0.0.1:3000/-/speedscope/index.html http://127.0.0.1:3000/-/sandbox/mermaid http://127.0.0.1:3000/assets/ blob: data:; connect-src 'self' http://127.0.0.1:3808 ws://127.0.0.1:3808 ws://127.0.0.1:3000; default-src 'self'; font-src 'self'; form-action 'self' https: http:; frame-ancestors 'self'; frame-src https://www.google.com/recaptcha/ https://www.recaptcha.net/ https://content.googleapis.com https://content-compute.googleapis.com https://content-cloudbilling.googleapis.com https://content-cloudresourcemanager.googleapis.com https://www.googletagmanager.com/ns.html http://127.0.0.1:3000/rails/letter_opener/ http://127.0.0.1:3000/admin/ http://127.0.0.1:3000/assets/ http://127.0.0.1:3000/-/speedscope/index.html http://127.0.0.1:3000/-/sandbox/mermaid; img-src 'self' data: blob: http: https:; manifest-src 'self'; media-src 'self' data:; object-src 'none'; script-src 'strict-dynamic' 'self' 'unsafe-inline' 'unsafe-eval' https://www.google.com/recaptcha/ https://www.recaptcha.net https://apis.google.com 'nonce-6xvSxtdRATS+lqEBvA4P+w=='; style-src 'self' 'unsafe-inline'; worker-src http://127.0.0.1:3000/assets/ blob: data:

Before this MR both would have removed the script-src directive because <default_value> is an invalid directive

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Dominic Couture

Merge request reports