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

Jump to bottom

Add CSP header to all requests, including api requests #144902

Merged
merged 1 commit into from
Nov 16, 2022
Merged

Add CSP header to all requests, including api requests #144902

merged 1 commit into from
Nov 16, 2022

Conversation

watson
Copy link
Contributor

@watson watson commented Nov 9, 2022 •
edited by kibanamachine
Loading

Previously /api/* requests didn't include a Content-Security-Policy header, now they do.

Closes #143871

Note to reviewers

I've tried to add tests that validate that 1) kbn-name and Content-Security-Policy cannot be overwitten by plugins/route handlers and 2) all routes are now covered, both /api/* and otherwise. It was difficult to find a good place to test this, but I think what I have now in the PR coveres it. Please verify

Release note

Adds a Content-Security-Policy header to all /api/* responses. Any Kibana HTTP endpoint starting with /api/* previously didn't include a Content-Security-Policy header, but will now share the same header as the regular Kibana HTTP routes.

Todo

  • Figure out what type of release note, if any, should be accompanied by this PR and author it
  • Figure out if this PR should be backported to 7.17
  • Update tests to validate that /api/* requests now have a Content-Security-Policy header (I think this is now indirectly checked by the new lifecycle test added in this PR since that's validating any route)
  • Update tests to validate that the Content-Security-Policy header cannot be overwritten, similar to the old tests
  • Investigate: When configuring Kibana with an enrolment token it seems that the page is rendered differently and not caught by the expected preResponse handler

@watson watson added the backport:skip This commit does not require backporting label Nov 9, 2022
@watson watson self-assigned this Nov 9, 2022
Add CSP header to all requests, including api requests
0db0c29 
Previously api requests didn't include a Content-Security-Policy header
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 59 65 +6
osquery 108 113 +5
securitySolution 441 447 +6
total +19

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 67 73 +6
osquery 109 115 +6
securitySolution 518 524 +6
total +20

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @watson

@watson watson marked this pull request as ready for review November 15, 2022 13:56
@watson watson requested a review from a team as a code owner November 15, 2022 13:56
@watson watson requested a review from a team November 15, 2022 16:13
@watson watson added release_note:enhancement backport:all-open Backport to all branches that could still receive a release and removed backport:skip This commit does not require backporting labels Nov 15, 2022
pgayvallet
pgayvallet approved these changes Nov 16, 2022
View reviewed changes
Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

. It's not any more work to do it now than to do it later, if anything, it's easier to implement once we have a concrete use-case.

Fair enough

!remindme 6m

thomheymann
thomheymann approved these changes Nov 16, 2022
View reviewed changes
Copy link
Contributor

@thomheymann thomheymann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! All working as expected.

@watson watson merged commit 5550ab6 into elastic:main Nov 16, 2022
@watson watson deleted the csp-everywhere branch November 16, 2022 20:45
@kibanamachine kibanamachine mentioned this pull request Nov 16, 2022
Merged
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 16, 2022
Add CSP header to all requests, including api requests (elastic#144902)
296444e 
Previously `/api/*` requests didn't include a `Content-Security-Policy`
header, now they do.

Closes elastic#143871

(cherry picked from commit 5550ab6)
@kibanamachine
Copy link
Contributor

💔 Some backports could not be created

Status Branch Result
❌ 7.17 Backport failed because of merge conflicts
❌ 8.5 Backport failed because of merge conflicts
✅ 8.6

Note: Successful backport PRs will be merged automatically after passing CI.

Manual backport

To create the backport manually run:

node scripts/backport --pr 144902

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Nov 16, 2022
@kibanamachine
[8.6] Add CSP header to all requests, including api requests (#144902) (
a9f7ba6 
#145449)

# Backport

This will backport the following commits from `main` to `8.6`:
- [Add CSP header to all requests, including api requests
(#144902)](#144902)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Thomas
Watson","email":"watson@elastic.co"},"sourceCommit":{"committedDate":"2022-11-16T20:45:10Z","message":"Add
CSP header to all requests, including api requests
(#144902)\n\nPreviously `/api/*` requests didn't include a
`Content-Security-Policy`\r\nheader, now they do.\r\n\r\nCloses
#143871","sha":"5550ab6cb10fbfddf437a74900103bb33dd1afa4","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","backport:all-open","v8.7.0"],"number":144902,"url":"https://github.com/elastic/kibana/pull/144902","mergeCommit":{"message":"Add
CSP header to all requests, including api requests
(#144902)\n\nPreviously `/api/*` requests didn't include a
`Content-Security-Policy`\r\nheader, now they do.\r\n\r\nCloses
#143871","sha":"5550ab6cb10fbfddf437a74900103bb33dd1afa4"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/144902","number":144902,"mergeCommit":{"message":"Add
CSP header to all requests, including api requests
(#144902)\n\nPreviously `/api/*` requests didn't include a
`Content-Security-Policy`\r\nheader, now they do.\r\n\r\nCloses
#143871","sha":"5550ab6cb10fbfddf437a74900103bb33dd1afa4"}}]}]
BACKPORT-->

Co-authored-by: Thomas Watson <watson@elastic.co>
@watson watson mentioned this pull request Nov 17, 2022
Merged
watson pushed a commit to watson/kibana that referenced this pull request Nov 17, 2022
Add CSP header to all requests, including api requests (elastic#144902)
ffe9a55 
Previously `/api/*` requests didn't include a `Content-Security-Policy`
header, now they do.

Closes elastic#143871

(cherry picked from commit 5550ab6)

# Conflicts:
#	src/core/server/http_resources/http_resources_service.test.ts
#	src/core/server/http_resources/http_resources_service.ts
watson pushed a commit to watson/kibana that referenced this pull request Nov 17, 2022
Add CSP header to all requests, including api requests (elastic#144902)
553bad0 
Previously `/api/*` requests didn't include a `Content-Security-Policy`
header, now they do.

Closes elastic#143871

(cherry picked from commit 5550ab6)

# Conflicts:
#	packages/core/http/core-http-server-mocks/src/test_utils.ts
#	src/core/server/http/lifecycle_handlers.test.ts
#	src/core/server/http_resources/http_resources_service.test.ts
#	src/core/server/http_resources/http_resources_service.ts
@watson watson mentioned this pull request Nov 17, 2022
Merged
@watson
Copy link
Contributor Author

watson commented Nov 17, 2022

💚 All backports created successfully

Status Branch Result
✅ 8.5
✅ 7.17

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

watson pushed a commit that referenced this pull request Nov 17, 2022
[8.5] Add CSP header to all requests, including api requests (#144902) (
723a55e 
#145552)

# Backport

This will backport the following commits from `main` to `8.5`:
- [Add CSP header to all requests, including api requests
(#144902)](#144902)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Thomas
Watson","email":"watson@elastic.co"},"sourceCommit":{"committedDate":"2022-11-16T20:45:10Z","message":"Add
CSP header to all requests, including api requests
(#144902)\n\nPreviously `/api/*` requests didn't include a
`Content-Security-Policy`\r\nheader, now they do.\r\n\r\nCloses
#143871","sha":"5550ab6cb10fbfddf437a74900103bb33dd1afa4","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","backport:all-open","v8.6.0","v8.7.0"],"number":144902,"url":"https://github.com/elastic/kibana/pull/144902","mergeCommit":{"message":"Add
CSP header to all requests, including api requests
(#144902)\n\nPreviously `/api/*` requests didn't include a
`Content-Security-Policy`\r\nheader, now they do.\r\n\r\nCloses
#143871","sha":"5550ab6cb10fbfddf437a74900103bb33dd1afa4"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"8.6","label":"v8.6.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/145449","number":145449,"state":"MERGED","mergeCommit":{"sha":"a9f7ba61f128e61beb936d2caff143e93e3321ea","message":"[8.6]
Add CSP header to all requests, including api requests (#144902)
(#145449)\n\n# Backport\n\nThis will backport the following commits from
`main` to `8.6`:\n- [Add CSP header to all requests, including api
requests\n(#144902)](https://github.com/elastic/kibana/pull/144902)\n\n<!---
Backport version: 8.9.7 -->\n\n### Questions ?\nPlease refer to the
[Backport
tool\ndocumentation](https://github.com/sqren/backport)\n\n<!--BACKPORT
[{\"author\":{\"name\":\"Thomas\nWatson\",\"email\":\"watson@elastic.co\"},\"sourceCommit\":{\"committedDate\":\"2022-11-16T20:45:10Z\",\"message\":\"Add\nCSP
header to all requests, including api
requests\n(#144902)\\n\\nPreviously `/api/*` requests didn't include
a\n`Content-Security-Policy`\\r\\nheader, now they
do.\\r\\n\\r\\nCloses\n#143871\",\"sha\":\"5550ab6cb10fbfddf437a74900103bb33dd1afa4\",\"branchLabelMapping\":{\"^v8.7.0$\":\"main\",\"^v(\\\\d+).(\\\\d+).\\\\d+$\":\"$1.$2\"}},\"sourcePullRequest\":{\"labels\":[\"release_note:enhancement\",\"backport:all-open\",\"v8.7.0\"],\"number\":144902,\"url\":\"https://github.com/elastic/kibana/pull/144902\",\"mergeCommit\":{\"message\":\"Add\nCSP
header to all requests, including api
requests\n(#144902)\\n\\nPreviously `/api/*` requests didn't include
a\n`Content-Security-Policy`\\r\\nheader, now they
do.\\r\\n\\r\\nCloses\n#143871\",\"sha\":\"5550ab6cb10fbfddf437a74900103bb33dd1afa4\"}},\"sourceBranch\":\"main\",\"suggestedTargetBranches\":[],\"targetPullRequestStates\":[{\"branch\":\"main\",\"label\":\"v8.7.0\",\"labelRegex\":\"^v8.7.0$\",\"isSourceBranch\":true,\"state\":\"MERGED\",\"url\":\"https://github.com/elastic/kibana/pull/144902\",\"number\":144902,\"mergeCommit\":{\"message\":\"Add\nCSP
header to all requests, including api
requests\n(#144902)\\n\\nPreviously `/api/*` requests didn't include
a\n`Content-Security-Policy`\\r\\nheader, now they
do.\\r\\n\\r\\nCloses\n#143871\",\"sha\":\"5550ab6cb10fbfddf437a74900103bb33dd1afa4\"}}]}]\nBACKPORT-->\n\nCo-authored-by:
Thomas Watson
<watson@elastic.co>"}},{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/144902","number":144902,"mergeCommit":{"message":"Add
CSP header to all requests, including api requests
(#144902)\n\nPreviously `/api/*` requests didn't include a
`Content-Security-Policy`\r\nheader, now they do.\r\n\r\nCloses
#143871","sha":"5550ab6cb10fbfddf437a74900103bb33dd1afa4"}}]}]
BACKPORT-->
@KOTungseth KOTungseth added the Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! label Nov 17, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

benakansara pushed a commit to benakansara/kibana that referenced this pull request Nov 17, 2022
@benakansara
Add CSP header to all requests, including api requests (elastic#144902)
59cfa35 
Previously `/api/*` requests didn't include a `Content-Security-Policy`
header, now they do.

Closes elastic#143871
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Nov 18, 2022
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

watson pushed a commit that referenced this pull request Nov 21, 2022
@kibanamachine
[7.17] Add CSP header to all requests, including api requests (#144902)…
18c40db 
… (#145553)

# Backport

This will backport the following commits from `main` to `7.17`:
- [Add CSP header to all requests, including api requests
(#144902)](#144902)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Thomas
Watson","email":"watson@elastic.co"},"sourceCommit":{"committedDate":"2022-11-16T20:45:10Z","message":"Add
CSP header to all requests, including api requests
(#144902)\n\nPreviously `/api/*` requests didn't include a
`Content-Security-Policy`\r\nheader, now they do.\r\n\r\nCloses
#143871","sha":"5550ab6cb10fbfddf437a74900103bb33dd1afa4","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","backport:all-open","v8.6.0","v8.7.0"],"number":144902,"url":"https://github.com/elastic/kibana/pull/144902","mergeCommit":{"message":"Add
CSP header to all requests, including api requests
(#144902)\n\nPreviously `/api/*` requests didn't include a
`Content-Security-Policy`\r\nheader, now they do.\r\n\r\nCloses
#143871","sha":"5550ab6cb10fbfddf437a74900103bb33dd1afa4"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"8.6","label":"v8.6.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/145449","number":145449,"state":"MERGED","mergeCommit":{"sha":"a9f7ba61f128e61beb936d2caff143e93e3321ea","message":"[8.6]
Add CSP header to all requests, including api requests (#144902)
(#145449)\n\n# Backport\n\nThis will backport the following commits from
`main` to `8.6`:\n- [Add CSP header to all requests, including api
requests\n(#144902)](https://github.com/elastic/kibana/pull/144902)\n\n<!---
Backport version: 8.9.7 -->\n\n### Questions ?\nPlease refer to the
[Backport
tool\ndocumentation](https://github.com/sqren/backport)\n\n<!--BACKPORT
[{\"author\":{\"name\":\"Thomas\nWatson\",\"email\":\"watson@elastic.co\"},\"sourceCommit\":{\"committedDate\":\"2022-11-16T20:45:10Z\",\"message\":\"Add\nCSP
header to all requests, including api
requests\n(#144902)\\n\\nPreviously `/api/*` requests didn't include
a\n`Content-Security-Policy`\\r\\nheader, now they
do.\\r\\n\\r\\nCloses\n#143871\",\"sha\":\"5550ab6cb10fbfddf437a74900103bb33dd1afa4\",\"branchLabelMapping\":{\"^v8.7.0$\":\"main\",\"^v(\\\\d+).(\\\\d+).\\\\d+$\":\"$1.$2\"}},\"sourcePullRequest\":{\"labels\":[\"release_note:enhancement\",\"backport:all-open\",\"v8.7.0\"],\"number\":144902,\"url\":\"https://github.com/elastic/kibana/pull/144902\",\"mergeCommit\":{\"message\":\"Add\nCSP
header to all requests, including api
requests\n(#144902)\\n\\nPreviously `/api/*` requests didn't include
a\n`Content-Security-Policy`\\r\\nheader, now they
do.\\r\\n\\r\\nCloses\n#143871\",\"sha\":\"5550ab6cb10fbfddf437a74900103bb33dd1afa4\"}},\"sourceBranch\":\"main\",\"suggestedTargetBranches\":[],\"targetPullRequestStates\":[{\"branch\":\"main\",\"label\":\"v8.7.0\",\"labelRegex\":\"^v8.7.0$\",\"isSourceBranch\":true,\"state\":\"MERGED\",\"url\":\"https://github.com/elastic/kibana/pull/144902\",\"number\":144902,\"mergeCommit\":{\"message\":\"Add\nCSP
header to all requests, including api
requests\n(#144902)\\n\\nPreviously `/api/*` requests didn't include
a\n`Content-Security-Policy`\\r\\nheader, now they
do.\\r\\n\\r\\nCloses\n#143871\",\"sha\":\"5550ab6cb10fbfddf437a74900103bb33dd1afa4\"}}]}]\nBACKPORT-->\n\nCo-authored-by:
Thomas Watson
<watson@elastic.co>"}},{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/144902","number":144902,"mergeCommit":{"message":"Add
CSP header to all requests, including api requests
(#144902)\n\nPreviously `/api/*` requests didn't include a
`Content-Security-Policy`\r\nheader, now they do.\r\n\r\nCloses
#143871","sha":"5550ab6cb10fbfddf437a74900103bb33dd1afa4"}}]}]
BACKPORT-->

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine kibanamachine added v7.17.8 and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomheymann thomheymann thomheymann approved these changes

@pgayvallet pgayvallet pgayvallet approved these changes

Assignees

@watson watson

Labels
backport:all-open Backport to all branches that could still receive a release release_note:enhancement Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! v7.17.8 v8.5.2 v8.6.0 v8.7.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Apply Content-Security-Policy to all server responses
7 participants
@watson @kibana-ci @kibanamachine @elasticmachine @thomheymann @pgayvallet @KOTungseth