[HTTP] First pass of making Kibana work with internal restrictions enforced #162258

jloleysens · 2023-07-19T15:50:10Z

Summary

When turning on server.restrictInternalApis a number of issues surfaced due to defaulting to internal resulting in 400s for:

HTTP resources
Static assets via registerStaticDir
Use of res.render(Html|Js|Css) outside of HTTP resources

This PR:

defaults our HTTP resources service to register routes by default public, same for static dirs.
Did an audit of all renderX usages, if outside of HTTP resources I added an explicit access: public
...what else?

Set `access: 'public'` for known set of "system" routes

Method	Path	Comment
GET	/api/status
GET	/api/stats
GET	/translations/{locale}.json
GET	/api/fleet/agent_policies
GET	/api/task_manager/_background_task_utilization
GET	/internal/task_manager/_background_task_utilization
GET	/internal/detection_engine/health/_cluster
POST	/internal/detection_engine/health/_cluster
GET	/internal/detection_engine/health/_space
POST	/internal/detection_engine/health/_space
POST	/internal/detection_engine/health/_rule
POST	/internal/detection_engine/health/_setup
GET	/bootstrap.js
GET	/bootstrap-anonymous.js
GET	*/bundles/	Core's routes for serving JS & CSS bundles

How to test

Run this PR with kibana.dev.yml containing server.restrictInternalApis: true and navigate around Kibana UI checking that there are no 400s in the network resources tab due to access restrictions.

Notes

Either left a comment about why access was set public or a simple unit test to check that we are setting access for a given route

To do

Manually test Kibana
Manually test with interactiveSetup plugin
Add integration and e2e test (will do in a follow up PR)

Related: #162149

…cted

…fix'

TinaHeiligers

We'll need to extend our testing around protecting internal APIs. An overall test asserting against the ones that are public would be a start. I'm thinking along the lines of the test we have for keeping track of which saved objects have migrations, but not necessarily a hash map.
We can build on it as we make more changes.

packages/core/status/core-status-server-internal/src/routes/status_preboot.ts

packages/core/status/core-status-server-internal/src/status_service.test.ts

src/plugins/usage_collection/server/routes/stats/stats.ts

elasticmachine · 2023-07-24T10:16:14Z

Pinging @elastic/kibana-core (Team:Core)

…fix'

elasticmachine · 2023-07-24T10:27:35Z

Pinging @elastic/fleet (Team:Fleet)

x-pack/plugins/fleet/server/routes/agent_policy/index.ts

juliaElastic

LGTM

kibana-ci · 2023-07-24T17:36:10Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: c27062b

Failed CI Steps

Test Failures

[job] [logs] Security Solution Tests #1 / Detections : Page Filters Number fields are not visible in field edit panel Number fields are not visible in field edit panel

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`home`	211	213	+2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`home`	163.9KB	163.9KB	+48.0B

History

💛 Build #144238 was flaky 6ecb2fc
💚 Build #143933 succeeded 2b6a23f
💔 Build #143873 failed a342b0f
💔 Build #143830 failed 31885b4
💔 Build #143615 failed e0141c9

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

TinaHeiligers

I'm worried about the flaky cypress test. The tests are super hard to debug for non-domain owners. @elastic/appex-sharedux is there something we can do to make cypress testing easier and easier to debug?

PR code changes to fine and running it locally works!
LGTM

pgayvallet · 2023-07-25T06:47:02Z

packages/core/i18n/core-i18n-server-internal/src/routes/translations.ts

@@ -28,6 +28,7 @@ export const registerTranslationsRoute = (router: IRouter, locale: string) => {
        }),
      },
      options: {
+        access: 'public',


A lot of Core routes that were (rightfully) switched to public aren't listed in the issue's description (e.g this one). Future us would probably be very grateful if we had an exhaustive list somewhere for when we may want to switch some of them back to internal (e.g when updating the internal consumers to provide the header)

pgayvallet · 2023-07-25T08:06:49Z

src/plugins/home/public/application/load_tutorials.js

 headers.append('kbn-xsrf', 'kibana');
+headers.append(X_ELASTIC_INTERNAL_ORIGIN_REQUEST, 'kibana');


🙈 I don't even want to know why the home plugin isn't using core's fetch service here...

xcrzx

Hey @jloleysens, I've checked some other internal endpoints that are being used externally. Here's a list of endpoints called by the diagnostic tool (used by our support team): https://github.com/elastic/support-diagnostics/blob/34a6670959e03b81301abf941f5fe6e4ff11c736/src/main/resources/kibana-rest.yml.

This list includes:

/internal/detection_engine/prebuilt_rules/status
/internal/security/me

I believe we should add access: 'public' to these endpoints as well. Also, other Security Solution teams might be using different internal endpoints for external tools so broader communication about the new restriction would be much appreciated.

ymao1

response ops changes LGTM

…forced (elastic#162258) ## Summary When turning on `server.restrictInternalApis` a number of issues surfaced due to defaulting to internal resulting in `400`s for: * HTTP resources * Static assets via `registerStaticDir` * Use of `res.render(Html|Js|Css)` outside of HTTP resources This PR: * defaults our HTTP resources service to register routes by default `public`, same for static dirs. * Did an audit of all renderX usages, if outside of HTTP resources I added an explicit `access: public` * ...what else? ### Set `access: 'public'` for known set of "system" routes Method | Path | Comment -- | -- | -- GET | /api/status GET | /api/stats GET | /translations/{locale}.json GET | /api/fleet/agent_policies GET | /api/task_manager/_background_task_utilization GET | /internal/task_manager/_background_task_utilization GET | /internal/detection_engine/health/_cluster POST | /internal/detection_engine/health/_cluster GET | /internal/detection_engine/health/_space POST | /internal/detection_engine/health/_space POST | /internal/detection_engine/health/_rule POST | /internal/detection_engine/health/_setup GET | /bootstrap.js GET | /bootstrap-anonymous.js GET | \*\*/bundles/\* | Core's routes for serving JS & CSS bundles ## How to test Run this PR with `kibana.dev.yml` containing `server.restrictInternalApis: true` and navigate around Kibana UI checking that there are no `400`s in the network resources tab due to access restrictions. ## Notes * Either left a comment about why `access` was set public or a simple unit test to check that we are setting access for a given route ## To do - [x] Manually test Kibana - [x] Manually test with `interactiveSetup` plugin - [ ] Add integration and e2e test (will do in a follow up PR) Related: elastic#162149 --------- Co-authored-by: kibanamachine <[email protected]>

jloleysens and others added 8 commits July 19, 2023 17:48

first pass of getting Kibana up and running with internal APIs restri…

1edc472

…cted

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

e0141c9

…fix'

fix jest test

31885b4

Merge branch 'main' into default-http-resources-to-public

12d557a

set /api/status to public

e97d93a

set /api/stats to public

261ae49

set /api/stats to public

a342b0f

fix test

2b6a23f

TinaHeiligers approved these changes Jul 22, 2023

View reviewed changes

jloleysens and others added 6 commits July 24, 2023 11:57

added test for setting "public" on registerStaticDirs

2face7f

rename test

2c560d2

added test against translations route options

a6e78b8

added test for options against bootstrap.js routes

db08571

added comments to stats and status routes

06861d5

Merge branch 'main' into default-http-resources-to-public

7e3d74b

jloleysens marked this pull request as ready for review July 24, 2023 10:12

jloleysens requested review from a team as code owners July 24, 2023 10:12

jloleysens added Feature:http Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc release_note:skip Skip the PR/issue when compiling release notes v8.10.0 labels Jul 24, 2023

jloleysens requested a review from TinaHeiligers July 24, 2023 10:16

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

f81fe1e

…fix'

Dosant approved these changes Jul 24, 2023

View reviewed changes

set access for bevy of known system paths

6ecb2fc

jloleysens requested review from a team as code owners July 24, 2023 10:27

jloleysens requested a review from xcrzx July 24, 2023 10:27

botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Jul 24, 2023

juliaElastic reviewed Jul 24, 2023

View reviewed changes

x-pack/plugins/fleet/server/routes/agent_policy/index.ts Show resolved Hide resolved

juliaElastic approved these changes Jul 24, 2023

View reviewed changes

Merge branch 'main' into default-http-resources-to-public

c27062b

TinaHeiligers approved these changes Jul 24, 2023

View reviewed changes

pgayvallet approved these changes Jul 25, 2023

View reviewed changes

xcrzx approved these changes Jul 26, 2023

View reviewed changes

jloleysens mentioned this pull request Jul 26, 2023

Set new header for diag requests to Kibana on serverless elastic/support-diagnostics#657

Open

ymao1 approved these changes Jul 26, 2023

View reviewed changes

jloleysens merged commit 32b5903 into elastic:main Jul 26, 2023

kibanamachine added the backport:skip This commit does not require backporting label Jul 26, 2023

jloleysens deleted the default-http-resources-to-public branch July 26, 2023 12:48

jloleysens mentioned this pull request Jul 26, 2023

Enable API protection in serverless #162149

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HTTP] First pass of making Kibana work with internal restrictions enforced #162258

[HTTP] First pass of making Kibana work with internal restrictions enforced #162258

jloleysens commented Jul 19, 2023 •

edited

Loading

TinaHeiligers left a comment

elasticmachine commented Jul 24, 2023

elasticmachine commented Jul 24, 2023

juliaElastic left a comment

kibana-ci commented Jul 24, 2023

TinaHeiligers left a comment

pgayvallet Jul 25, 2023

pgayvallet Jul 25, 2023

xcrzx left a comment

ymao1 left a comment

		headers.append('kbn-xsrf', 'kibana');
		headers.append(X_ELASTIC_INTERNAL_ORIGIN_REQUEST, 'kibana');

[HTTP] First pass of making Kibana work with internal restrictions enforced #162258

[HTTP] First pass of making Kibana work with internal restrictions enforced #162258

Conversation

jloleysens commented Jul 19, 2023 • edited Loading

Summary

Set access: 'public' for known set of "system" routes

How to test

Notes

To do

TinaHeiligers left a comment

Choose a reason for hiding this comment

elasticmachine commented Jul 24, 2023

elasticmachine commented Jul 24, 2023

juliaElastic left a comment

Choose a reason for hiding this comment

kibana-ci commented Jul 24, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Async chunks

History

TinaHeiligers left a comment

Choose a reason for hiding this comment

pgayvallet Jul 25, 2023

Choose a reason for hiding this comment

pgayvallet Jul 25, 2023

Choose a reason for hiding this comment

xcrzx left a comment

Choose a reason for hiding this comment

ymao1 left a comment

Choose a reason for hiding this comment

jloleysens commented Jul 19, 2023 •

edited

Loading

Set `access: 'public'` for known set of "system" routes