-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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.
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
Enable API protection in serverless #162149
Conversation
`restrictInternalApis` set to `true` enforces the restriction to `internal` API's. The restriction is only enabled in serverless.
Pinging @elastic/kibana-core (Team:Core) |
Pinging @elastic/kibana-security (Team:Security) |
Tests against Update: Clarifying above: When the
#161672 changes the default to
|
@@ -48,7 +48,7 @@ data.search.sessions.enabled: false | |||
advanced_settings.enabled: false | |||
|
|||
# Enforce restring access to internal APIs see https://github.com/elastic/kibana/issues/151940 | |||
# server.restrictInternalApis: true | |||
server.restrictInternalApis: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jloleysens @pgayvallet suggests a CLI implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggested to do something else in https://github.com/elastic/kibana/pull/161733/files#r1262308667 because the purpose of the linked PR was to enabled restricted API mode for tests, and enabling it globally wasn't the best way of doing so.
If your PR intends to enabled the restriction in production, then uncommenting the value in the serverless config file makes sense.
What does it mean in practice? Does it affect internal routes defined with
If API is public, does it require |
Hey @azasypkin ! In practice it means that all routes that match We are going to be doing an analysis of affected systems first before merging this PR (like resource bundles, login etc). Will likely take a while longer before this is ready.
No it will not require the header to use the endpoint. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocking pending further analysis of affected systems
👍
Great, thanks for confirming! |
…forced (#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: #162149 --------- Co-authored-by: kibanamachine <[email protected]>
Subset of #161337 Unblocks #162149 ## Summary This PR uses the access 'public' option when registering the `GET /api/security/logout` and `POST /api/security/saml/callback` APIs. This will ensure they have public access in serverless, while all other APIs will default to internal. PR #161672 changes default access of registered endpoints to 'internal', meaning that API owners have to explicitly set access: public to pass the API protection restriction. This PR also adds internal headers to the existing serverless Spaces API tests. This unblocks the PR to enable API protection in serverless (#162149). --------- Co-authored-by: kibanamachine <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's wait 1 week from today to merge this -- we should address failing tests though.
FWIW #162258 has been merged.
@elasticmachine merge upstream |
@jloleysens The smoke test failed miserably 😞 . There's an issue security solution cypress tests. |
@elastic/security-solution your serverless cyprus test fail with API protection enabled. |
@elasticmachine merge upstream |
@paul-tavares oh dear, it looks like we need to do more work around cypress. kibana/x-pack/test_serverless/functional/test_suites/security/cypress/tasks/login.ts Lines 19 to 47 in c0030da
There might be a few others too |
added label to make sure we run all Kibana's Cypress tests |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]
History
To update your PR or re-run it, just comment with: |
@paul-tavares one of the cypress test failed but didn't block the build. |
I just took a look and its not one of the tests we worked on. It came from |
This reverts commit f2e23d7.
Closes #161337 ## Summary Uses build flavor(see #161930) to disable specific Kibana security, spaces, and encrypted saved objects HTTP API routes in serverless (see details in #161337). HTTP APIs that will be public in serverless have been handled in #162523. **IMPORTANT: This PR leaves login, user, and role routes enabled. The primary reason for this is due to several testing mechanisms that rely on basic authentication and custom roles (UI, Cypress). These tests will be modified to use SAML authentication and serverless roles in the immediate future. Once this occurs, we will disable these routes.** ### Testing This PR also implements testing API access in serverless. - The testing strategy for disabled routes in serverless is to verify a `404 not found `response. - The testing strategy for internal access routes in serverless is to verify that without the internal request header (`x-elastic-internal-origin`), a `400 bad request response` is received, then verify that with the internal request header, a `200 ok response` is received. - The strategy for public routes in serverless is to verify a `200 ok` or `203 redirect` is received. ~~blocked by #161930~~ ~~blocked by #162149 for test implementation~~ --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Aleh Zasypkin <[email protected]> Co-authored-by: Aleh Zasypkin <[email protected]>
fix #162117
restrictInternalApis
set totrue
enforces the restriction tointernal
API's. The restriction is only enabled in serverless.What this means to plugin authors and stack components :
Access to internal Kibana HTTP API endpoints is restricted in serverless and calls to these endpoints will respond with
400
when the request doesn't include anx-elastic-internal-origin header
.The header is automatically included when using core's browser-side HTTP service (e.g. core.http.fetch('....'))
where the restriction is not applied:
access: public
remains open.For more details, see #152404
Risk
internal
APIsinternal
public documentation oninternal
APIs must have theexperimental
warning and clearly state thatinternal
APIs are subject to frequent changesinternal
APIs fail