Globally enforce internal API restriction #193792

TinaHeiligers · 2024-09-23T20:07:20Z

This PR enforces internal API restrictions in our standard offering.
Internal APIs are subject to rapid change and are intentionally not public. By restricting their access, we protect consumers from these rapid changes.

This PR does not change any public APIs and they remain available for external consumption.

Note to reviewers:

I chose the most practical way of resolving the failures (add the header or disable the restriction).

Details

Requests to internal Kibana APIs will be restricted globally. This allows more flexibility in making breaking changes to internal APIs, without a risk to external consumers.

Why are we doing this?

The restriction is there to help mitigate the risk of breaking external integrations consuming APIs. Internal APIs are subject to frequent changes, necessary for feature development.

What this means to plugin authors :

Kibana core applies the restriction when enabled through HTTP config.

What this means to Kibana consumers:

Explicitly restricting access to internal APIs has advantages for external consumers:

Consumers can safely integrate with Kibana's stable, public APIs
Consumers are protected from Internal route development, which may involve breaking changes
Relevant information in Kibana's external documentation that is user-friendly and complete.

KB article explaining the change (tracked as part of https://github.com/elastic/kibana-team/issues/1044)

Release note

Starting with this release, requests to internal Kibana APIs are globally restricted by default. This change is designed to provide more flexibility in making breaking changes to internal APIs while protecting external consumers from unexpected disruptions.
Key Changes:
• Internal API Access: External consumers no longer have access to Kibana’s internal APIs, which are now strictly reserved for internal development and subject to frequent changes. This helps ensure that external integrations only interact with stable, public APIs.
• Error Handling: When a request is made to an internal API without the proper internal identifier (header or query parameter), Kibana will respond with a 400 Bad Request error, indicating that the route exists but is not allowed under the current Kibana configuration.

How to test this

Running kibana

Set server.restrictInternalApis: true in kibana.yml
Start es with any license
Start kibana
Make an external request to an internal API:

curl request to get the config for 9.0:

curl --location 'localhost:5601/abc/api/kibana/management/saved_objects/_bulk_get' \
--header 'Content-Type: application/json' \
--header 'kbn-xsrf: kibana' \
--header 'x-elastic-internal-origin: kibana' \
--header 'Authorization: Basic ZWxhc3RpYzpjaGFuZ2VtZQ==' \
--data '[
  {
      "type": "config",
      "id": "9.0.0"
  }
]'

The request should be successful.

Make the same curl request without the internal origin header

curl:

curl --location 'localhost:5601/abc/api/kibana/management/saved_objects/_bulk_get' \
--header 'Content-Type: application/json' \
--header 'kbn-xsrf: kibana' \
--header 'Authorization: Basic ZWxhc3RpYzpjaGFuZ2VtZQ==' \
--data '[
  {
      "type": "config",
      "id": "9.0.0"
  }
]'

The response should be an error similar to:
{"statusCode":400,"error":"Bad Request","message":"uri [/api/kibana/management/saved_objects/_bulk_get] with method [post] exists but is not available with the current configuration"}

Remove server.restrictInternalApis from kibana.yml or set it to false.
Repeat both curl requests above. Both should respond without an error.

Checklist

Delete any items that are not applicable to this PR.

[Documentation] was added for features that require explanation or tutorials (In PR [docs] Document internal API restriction in 9.0 #191943)
Unit or functional tests were updated or added to match the most common scenarios (and PR [http] api_integration tests handle internal route restriction #192407)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list (docker list updated in Enables preventing access to internal APIs #156935, cloud stack packs for 9.0 kibana to follow)

Risk Matrix

Risk	Probability	Severity	Mitigation/Notes
The restriction is knowingly bypassed by end-users adding the required header to use `internal` APIs	Low	High	Kibana's internal APIs are not documented in the OAS specifications. External consumption will be prevented unless explicitly bypassed.
Upstream services don't include the header and requests to `internal` APIs fail	Medium	Medium	The Core team needs to ensure intra-stack components are updated too

For maintainers

This was checked for breaking API changes and was labeled appropriately

TinaHeiligers · 2024-09-23T21:59:37Z

@elasticmachine merge upstream

TinaHeiligers · 2024-09-24T14:33:31Z

@elasticmachine merge upstream

TinaHeiligers · 2024-09-24T17:46:52Z

packages/core/http/core-http-server-mocks/src/test_utils.ts

@@ -68,7 +68,7 @@ export const createConfigService = ({
            shutdownTimeout: moment.duration(30, 'seconds'),
            keepaliveTimeout: 120_000,
            socketTimeout: 120_000,
-            restrictInternalApis: false,
+            restrictInternalApis: false, // disable restriction for Kibana tests


Leaves the restriction disabled for tests consuming createConfigService.

packages/core/http/core-http-server-internal/src/http_config.ts

TinaHeiligers

self review.

elasticmachine · 2024-09-24T18:56:57Z

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

lukeelmers · 2024-09-24T20:51:43Z

Before we merge - I think we should investigate two things which I just realized weren't in the rollout plan:

Is it possible to surface a warning for this in UA similar to what we are discussing for Saved Objects APIs? e.g. Show a warning if we detect non-Kibana traffic to an internal API
We should plan to draft an internal KB article due to the widespread impact of this change

Let's also add a bit more detail to our breaking change note. Even though folks shouldn't be using these APIs, we know many are, so again this would be to address the potential wide impact of this change.

TinaHeiligers · 2024-09-24T21:48:34Z

PR is back in draft to take care of https://github.com/elastic/kibana/pull/193792#issuecomment-2372364457https://github.com/elastic/kibana/pull/193792#issuecomment-2372364457

TinaHeiligers · 2024-09-24T22:41:06Z

we should investigate two things which I just realized weren't in the rollout plan:

TBH, I've also been thinking along these lines because it's the right thing to do.

The tricky part is that restricting access to internal APIs isn't a classic deprecation and we'd need something within the http service to surface external calls in the UA. I'm unsure if that's possible from within the http lifecycle or maybe easier within the router itself. @jloleysens we should discuss our options.

As for the KB, I was going to add one anyway ;-)

Let's also add a bit more detail to our breaking change note

How about rewording something similar to #156935 to highlight how this change helps consumers?

jloleysens · 2024-09-25T07:36:40Z

...within the http lifecycle or maybe easier within the router itself...

We should be able to piggy-back off work Ahmad is doing. He is creating a core usage counter for HTTP stuff that we can use to make a new route counter. The "internal route usage" counter should be registered during setup time.

As for where we detect/increment this counter, I think we should do it just before returning the 400 here:

kibana/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts

Line 57 in 244ff7b

return response.badRequest({

That handler will just need the core usage service injected. But for that we are blocked on #193668

TinaHeiligers · 2024-09-26T16:33:10Z

@elasticmachine merge upstream

TinaHeiligers · 2024-09-27T00:49:59Z

packages/core/http/core-http-server-internal/src/http_config.ts

    restrictInternalApis: offeringBasedSchema({
-      serverless: schema.boolean({ defaultValue: false }),
-      traditional: schema.boolean({ defaultValue: false }),
+      serverless: schema.boolean({ defaultValue: true }),


Changing the default also meant updating tests to either disable the restriction or add the header required for internal requests.

TinaHeiligers · 2024-09-27T00:52:01Z

src/core/server/integration_tests/config/check_dynamic_config.test.ts

@@ -82,6 +83,9 @@ describe('checking all opted-in dynamic config settings', () => {
      logging: {
        loggers: [{ name: 'root', level: 'info', appenders: ['console'] }],
      },
+      server: {
+        restrictInternalApis: false,


It's less work to disable the restriction in createTestServers than adding the header/query param to tests.
We disable the restriction by default for ftr tests too.

Should/do we have a task for updating these requests in the future or is the plan to leave them unrestricted in tests?

Should/do we have a task for updating these requests in the future or is the plan to leave them unrestricted in tests

The plan is to update requests if and when we need to, which is consistent with the base config for our FTR tests.

As for this specific test, createRootWithSettings explicitly enables the restriction. Here, we change that in setup, and is an example of how consumers can override the default should they choose to.

TinaHeiligers · 2024-09-27T02:21:47Z

@elasticmachine merge upstream

TinaHeiligers · 2024-09-27T02:22:52Z

/ci

TinaHeiligers · 2024-09-28T03:06:56Z

/ci

TinaHeiligers · 2024-09-28T18:07:20Z

Let's also add a bit more detail to our breaking change note. Even though folks shouldn't be using these APIs, we know many are, so again this would be to address the potential wide impact of this change.

@lukeelmers I've fleshed out the release note and double-checked that the UA work and KB article are in the meta issue. The plan is to merge the implementation and follow up with the UA hook once that's ready.

TinaHeiligers · 2024-09-28T18:35:47Z

@elasticmachine merge upstream

jloleysens

Great work @TinaHeiligers for chasing down all those test cases. Implementation of new default LGTM. Left some minor comments.

packages/core/http/core-http-server-internal/src/http_config.ts

jloleysens · 2024-09-30T09:57:26Z

src/core/server/integration_tests/config/check_dynamic_config.test.ts

@@ -82,6 +83,9 @@ describe('checking all opted-in dynamic config settings', () => {
      logging: {
        loggers: [{ name: 'root', level: 'info', appenders: ['console'] }],
      },
+      server: {
+        restrictInternalApis: false,


Should/do we have a task for updating these requests in the future or is the plan to leave them unrestricted in tests?

src/plugins/files/server/file_client/integration_tests/es_file_client.test.ts

TinaHeiligers · 2024-10-01T16:58:03Z

@elasticmachine merge upstream

TinaHeiligers · 2024-10-01T19:58:11Z

The FTR test failure is unrelated to this PR: #194425

rudolf · 2024-10-02T10:11:46Z

Let's also add a bit more detail to our breaking change note. Even though folks shouldn't be using these APIs, we know many are, so again this would be to address the potential wide impact of this change.

@lukeelmers I've fleshed out the release note and double-checked that the UA work and KB article are in the meta issue. The plan is to merge the implementation and follow up with the UA hook once that's ready.

I've created #194675 to track the work for surfacing internal API usage in the UA.

TinaHeiligers · 2024-10-02T14:48:31Z

@elasticmachine merge upstream

SiddharthMantri

Security changes LGTM.

kibana-ci · 2024-10-02T16:35:12Z

💚 Build Succeeded

Buildkite Build
Commit: 624a65d

Metrics [docs]

✅ unchanged

History

💛 Build #238498 was flaky d177cdb
💚 Build #238133 succeeded b42530a
💚 Build #237848 succeeded d9b3ea9
💚 Build #237829 succeeded d02ce7c
💔 Build #237819 failed 2d73f0b

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

TinaHeiligers added 2 commits September 23, 2024 11:46

Globally enforce internal API restriction

e916f4b

adds restrictInternalApis to cli args

ea02761

TinaHeiligers added v9.0.0 release_note:breaking Feature:http Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc backport:skip This commit does not require backporting labels Sep 23, 2024

Merge branch 'main' into kbn-163654-enable-internalAPIrestriction

7066fa1

elasticmachine and others added 2 commits September 24, 2024 16:33

Merge branch 'main' into kbn-163654-enable-internalAPIrestriction

e0d4ed8

Enable restriction in http config if not set through kibana.yml

b2d6a5e

TinaHeiligers commented Sep 24, 2024

View reviewed changes

packages/core/http/core-http-server-internal/src/http_config.ts Outdated Show resolved Hide resolved

TinaHeiligers commented Sep 24, 2024

View reviewed changes

TinaHeiligers marked this pull request as ready for review September 24, 2024 18:56

TinaHeiligers requested a review from a team as a code owner September 24, 2024 18:56

TinaHeiligers marked this pull request as draft September 24, 2024 21:38

elasticmachine and others added 4 commits September 26, 2024 18:33

Merge branch 'main' into kbn-163654-enable-internalAPIrestriction

81c58d7

Set default to true in config schema

b579951

Update tests to handle restriction enabled by default

ff95bde

core api integration tests

5fbfb76

TinaHeiligers commented Sep 27, 2024

View reviewed changes

Merge branch 'main' into kbn-163654-enable-internalAPIrestriction

4bc36a9

TinaHeiligers added 2 commits September 27, 2024 15:09

Update files plugin tests

2d73f0b

Disable restricting internal APIs in security functional tests

d02ce7c

Merge branch 'main' into kbn-163654-enable-internalAPIrestriction

d9b3ea9

TinaHeiligers marked this pull request as ready for review September 30, 2024 01:31

TinaHeiligers requested review from a team as code owners September 30, 2024 01:31

jloleysens approved these changes Sep 30, 2024

View reviewed changes

Dosant approved these changes Sep 30, 2024

View reviewed changes

src/plugins/files/server/file_client/integration_tests/es_file_client.test.ts Outdated Show resolved Hide resolved

Clean up

b42530a

SiddharthMantri self-requested a review October 1, 2024 13:12

Merge branch 'main' into kbn-163654-enable-internalAPIrestriction

d177cdb

rudolf mentioned this pull request Oct 2, 2024

Surface integrations with internal APIs in upgrade assistant #194675

Closed

Merge branch 'main' into kbn-163654-enable-internalAPIrestriction

624a65d

SiddharthMantri approved these changes Oct 2, 2024

View reviewed changes

TinaHeiligers merged commit fe83c0d into elastic:main Oct 2, 2024
22 checks passed

TinaHeiligers deleted the kbn-163654-enable-internalAPIrestriction branch October 2, 2024 17:08

Heenawter mentioned this pull request Oct 7, 2024

[Canvas] Timelion broken due to internal API requests being rejected #195297

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Globally enforce internal API restriction #193792

Globally enforce internal API restriction #193792

TinaHeiligers commented Sep 23, 2024 •

edited

Loading

TinaHeiligers commented Sep 23, 2024

TinaHeiligers commented Sep 24, 2024

TinaHeiligers Sep 24, 2024

TinaHeiligers left a comment

elasticmachine commented Sep 24, 2024

lukeelmers commented Sep 24, 2024

TinaHeiligers commented Sep 24, 2024

TinaHeiligers commented Sep 24, 2024 •

edited

Loading

jloleysens commented Sep 25, 2024

TinaHeiligers commented Sep 26, 2024

TinaHeiligers Sep 27, 2024

TinaHeiligers Sep 27, 2024

jloleysens Sep 30, 2024

TinaHeiligers Sep 30, 2024 •

edited

Loading

TinaHeiligers commented Sep 27, 2024

TinaHeiligers commented Sep 27, 2024

TinaHeiligers commented Sep 28, 2024

TinaHeiligers commented Sep 28, 2024 •

edited

Loading

TinaHeiligers commented Sep 28, 2024

jloleysens left a comment

jloleysens Sep 30, 2024

TinaHeiligers commented Oct 1, 2024

TinaHeiligers commented Oct 1, 2024

rudolf commented Oct 2, 2024

TinaHeiligers commented Oct 2, 2024

SiddharthMantri left a comment

kibana-ci commented Oct 2, 2024

Globally enforce internal API restriction #193792

Globally enforce internal API restriction #193792

Conversation

TinaHeiligers commented Sep 23, 2024 • edited Loading

Note to reviewers:

Details

Why are we doing this?

What this means to plugin authors :

What this means to Kibana consumers:

Release note

How to test this

Running kibana

Checklist

Risk Matrix

For maintainers

TinaHeiligers commented Sep 23, 2024

TinaHeiligers commented Sep 24, 2024

TinaHeiligers Sep 24, 2024

Choose a reason for hiding this comment

TinaHeiligers left a comment

Choose a reason for hiding this comment

elasticmachine commented Sep 24, 2024

lukeelmers commented Sep 24, 2024

TinaHeiligers commented Sep 24, 2024

TinaHeiligers commented Sep 24, 2024 • edited Loading

jloleysens commented Sep 25, 2024

TinaHeiligers commented Sep 26, 2024

TinaHeiligers Sep 27, 2024

Choose a reason for hiding this comment

TinaHeiligers Sep 27, 2024

Choose a reason for hiding this comment

jloleysens Sep 30, 2024

Choose a reason for hiding this comment

TinaHeiligers Sep 30, 2024 • edited Loading

Choose a reason for hiding this comment

TinaHeiligers commented Sep 27, 2024

TinaHeiligers commented Sep 27, 2024

TinaHeiligers commented Sep 28, 2024

TinaHeiligers commented Sep 28, 2024 • edited Loading

TinaHeiligers commented Sep 28, 2024

jloleysens left a comment

Choose a reason for hiding this comment

jloleysens Sep 30, 2024

Choose a reason for hiding this comment

TinaHeiligers commented Oct 1, 2024

TinaHeiligers commented Oct 1, 2024

rudolf commented Oct 2, 2024

TinaHeiligers commented Oct 2, 2024

SiddharthMantri left a comment

Choose a reason for hiding this comment

kibana-ci commented Oct 2, 2024

💚 Build Succeeded

Metrics [docs]

History

TinaHeiligers commented Sep 23, 2024 •

edited

Loading

TinaHeiligers commented Sep 24, 2024 •

edited

Loading

TinaHeiligers Sep 30, 2024 •

edited

Loading

TinaHeiligers commented Sep 28, 2024 •

edited

Loading