Enables preventing access to internal APIs #156935

TinaHeiligers · 2023-05-05T23:41:52Z

Part of #151940

fix #152282
fix #152283
fix #152293
partially address #152287

Summary

This PR follows on from the initial work done to more formally declare APIs as internal or public:

Allows internal API access to be configurable
Adapts Core's internal router to handle the access parameter
Adapts Core's browser-side http service to send an additional x-elastic-internal-origin: Kibana header to in all requests
Adapts some browser-side code not using Core's http service.

Details

Requests to internal Kibana APIs will be restricted for serverless. This allows us more flexibility in making breaking changes to internal APIs, without having to go through a lengthy deprecation period, as is required for changes to public APIs.

Why are we doing this?

The restriction is there to help mitigate external adoption of (what is intended to be) experimental/internal APIs. These APIs are subject to frequent changes and extensive adoption externally has historically forced us into corners of having to maintain some APIs as is to prevent breaking end-user integrations.

Explicitly enforcing a difference between internal and public APIs has a few other advantages:

We're only promising to maintain a small subset of Kibana's ~ 2500 APIs,
We can make breaking changes to internal routes
External documentation overhead reduced (please don't forget about your internal customers!)

There's always the catch that our code is public and end-users can still get around the restriction by adding the x-elastic-internal-origin header. However, if and when they do so and something 'breaks', we're not held liable.

What this means to plugin authors :

Core applies the restriction when enabled through HTTP config as server.restrictInternalApis: <boolean>.

What this means is that once we "flip the switch", any requests to internal APIs will throw if they don't send the x-elastic-internal-product header.

I've handled the known cases in this issue and opened #152287 asking teams to help out.

Checklist

Documentation was added for features that require explanation or tutorials (code comments and jsdocs. We explicitly don't intend to document the config option publically)
Unit or functional tests were updated or added to match the most common scenarios
If a plugin configuration key changed, check if it needs to be added to the docker list
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud (not needed for cloud-current offerings)

Risk Matrix

Risk	Probability	Severity	Mitigation/Notes
end-users add the `x-elastic-internal-origin` header to access `internal` APIs	Low	High	All public documentation on `internal` APIs must have the `experimental` warning and clearly state that `internal` APIs are subject to frequent changes
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

…ader to handler

… lifecycle handlers test

…fix'

TinaHeiligers · 2023-05-07T23:49:04Z

@elasticmachine merge upstream

…internal

TinaHeiligers · 2023-05-08T00:13:56Z

config/serverless.yml

@@ -17,3 +17,6 @@ xpack.license_management.enabled: false
 # Other disabled plugins
 #xpack.canvas.enabled: false #only disabable in dev-mode
 xpack.reporting.enabled: false
+
+# Enforce restring access to internal APIs see https://github.com/elastic/kibana/issues/151940
+# server.restrictInternalApis: true


Added here for intended purpose. Once all intra-stack components and Kibana browser-side code not using Core's http service are updated, we can enable the restriction.

TinaHeiligers

Self review

TinaHeiligers · 2023-05-08T00:15:01Z

packages/core/http/core-http-browser-internal/src/fetch.test.ts

@@ -160,6 +160,7 @@ describe('Fetch', () => {
      expect(fetchMock.lastOptions()!.headers).toMatchObject({
        'content-type': 'application/json',
        'kbn-version': 'VERSION',
+        'x-elastic-internal-origin': 'Kibana',


We're not concerned about the content here, only that the header is present.

TinaHeiligers · 2023-05-08T00:16:41Z

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

@@ -150,6 +150,7 @@ const configSchema = schema.object(
        },
      }
    ),
+    restrictInternalApis: schema.boolean({ defaultValue: false }), // allow access to internal routes by default to prevent breaking changes in current offerings


Code comment to explain the intent.

TinaHeiligers · 2023-05-08T00:20:36Z

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

@@ -39,6 +40,27 @@ export const createXsrfPostAuthHandler = (config: HttpConfig): OnPostAuthHandler
  };
 };

+export const createRestrictInternalRoutesPostAuthHandler = (


This takes a similar approach to createVersionCheckPostAuthHandler.

TinaHeiligers · 2023-05-08T00:25:36Z

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

@@ -86,7 +106,12 @@ export const registerCoreHandlers = (
  config: HttpConfig,
  env: Env
 ) => {
+  // add headers based on config


We don't often update these and I added comments to more easily follow where they're applied.

TinaHeiligers · 2023-05-08T00:26:15Z

packages/core/saved-objects/core-saved-objects-server-internal/src/routes/utils.ts

@@ -157,7 +157,9 @@ export interface BulkGetItem {
 export function isKibanaRequest({ headers }: KibanaRequest) {
  // The presence of these two request headers gives us a good indication that this is a first-party request from the Kibana client.
  // We can't be 100% certain, but this is a reasonable attempt.
-  return headers && headers['kbn-version'] && headers.referer;
+  return (
+    headers && headers['kbn-version'] && headers.referer && headers['x-elastic-internal-origin']


ATM, there are 4: core usage stats client, cases plugin, rule registry, and spaces usage stats client. I've added the header to core's implementation.

Are the concerns Joe raised still val a valid reason to have 4 implementations of isKibanaRequest or can we add the extra header to that check?

A while back, Joe raised concerns about adding a header to all Kibana requests:

other integrations might attempt to use this header

adding a new header might present problems for users with reverse proxies that may strip headers not in an allow-list; this would cause false negative for first-party request detection

If these are still valid, then having different implementations is warrented. @response-ops @kibana-security would a single method fullfil your needs or are concerns around false negatives be an issue for you?

IMO (1) and (2) should be mitigated because we will not be restricting access to internal endpoints by default so everything should continue working as-is for on-prem.

TinaHeiligers · 2023-05-08T00:27:08Z

packages/core/usage-data/core-usage-data-server-internal/src/core_usage_stats_client.ts

  // We can't be 100% certain, but this is a reasonable attempt.
-  return headers && headers['kbn-version'] && headers.referer;
+  return (
+    headers && headers['kbn-version'] && headers.referer && headers['x-elastic-internal-origin']


similar comment to above

TinaHeiligers

More self review comments

src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker

src/plugins/bfetch/public/plugin.ts

x-pack/dev-tools/api_debug/request_from_api.js

TinaHeiligers · 2023-05-08T00:51:10Z

x-pack/plugins/security/server/authentication/can_redirect_request.ts


  const isApiRoute =
    route.options.tags.includes(ROUTE_TAG_API) ||
    route.path.startsWith('/api/') ||
    route.path.startsWith('/internal/');
-  const isAjaxRequest = hasVersionHeader || hasXsrfHeader;
+  const isAjaxRequest = hasVersionHeader || hasXsrfHeader || hasIternalOriginHeader;


@elastic/kibana-security I took the initiative and applied the implementation. Feel free to suggest changes, I'm not too familiar with your domains.

Thanks, @TinaHeiligers. Question for you: Under what conditions will requests be expected to have an internal origin header, but not one of the version or xsrf headers?

@legrego good question! I can't think of one TBH. Can you?
Maybe it's better not to add the internal header, it could become confusing down the line. I'll make the change.

++ yeah I don't think this change is necessary at this point, but thanks for taking this function into consideration!

elasticmachine · 2023-05-09T01:45:07Z

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

jloleysens

Good job @TinaHeiligers ! Thanks for responding to my questions.

x-pack/plugins/security/server/authentication/can_redirect_request.test.ts

legrego · 2023-05-09T18:05:55Z

x-pack/plugins/security/server/authentication/can_redirect_request.test.ts

@@ -11,7 +11,7 @@ import { ROUTE_TAG_API, ROUTE_TAG_CAN_REDIRECT } from '../routes/tags';
 import { canRedirectRequest } from './can_redirect_request';

 describe('can_redirect_request', () => {
-  it('returns true if request does not have either a kbn-version or kbn-xsrf header', () => {
+  it('returns true if request does not have either a kbn-version or kbn-xsrf header or x-elastic-internal-origin', () => {


Suggested change

it('returns true if request does not have either a kbn-version or kbn-xsrf header or x-elastic-internal-origin', () => {

it('returns true if request does not have either a kbn-version or kbn-xsrf header', () => {

lukasolson

Bfetch changes LGTM

tsullivan

bfetch changes LGTM

tsullivan

bfetch changes LGTM

TinaHeiligers · 2023-05-10T00:34:02Z

@elasticmachine merge upstream

…internal

TinaHeiligers · 2023-05-10T02:39:08Z

@elasticmachine merge upstream

…internal

kibana-ci · 2023-05-10T11:25:16Z

💚 Build Succeeded

Buildkite Build
Commit: 3bd27de

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`bfetch`	22	24	+2

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/core-http-common`	0	1	+1
`@kbn/core-http-server-internal`	49	50	+1
`@kbn/server-http-tools`	50	51	+1
total			+3

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`bfetch`	6.9KB	6.9KB	+37.0B
`core`	363.9KB	364.1KB	+267.0B
total			+304.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/core-http-common`	5	6	+1
`@kbn/core-http-server-internal`	55	56	+1
`@kbn/server-http-tools`	53	54	+1
total			+3

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	19	21	+2
`securitySolution`	400	404	+4
total			+6

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	20	22	+2
`securitySolution`	480	484	+4
total			+6

History

💔 Build #126758 failed 0093f59
💔 Build #126746 failed 24ff4c3
💚 Build #126658 succeeded f38d8bd
💚 Build #126420 succeeded cb86813
💔 Build #126417 failed f5b2a27

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

fix #163654 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 elastic/kibana-team#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 1. Set `server.restrictInternalApis: true` in `kibana.yml` 2. Start es with any license 3. Start kibana 4. Make an external request to an internal API: <details> <summary>curl request to get the config for 9.0:</summary> ```unset 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" } ]' ``` </details> The request should be successful. 5. Make the same curl request without the internal origin header <details> <summary>curl:</summary> ```unset 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" } ]' ``` </details> 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"}` 6. Remove `server.restrictInternalApis` from `kibana.yml` or set it to `false`. 7. Repeat both curl requests above. Both should respond without an error. ### Checklist Delete any items that are not applicable to this PR. - [X] [Documentation] was added for features that require explanation or tutorials (In PR #191943) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios (and PR #192407) - [x] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) (docker list updated in #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 - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: Elastic Machine <[email protected]>

TinaHeiligers and others added 10 commits May 5, 2023 16:39

Adds check to ensure the internal product header is provided, adds he…

2f8af4e

…ader to handler

Adds internal origin header in fetch

34a0e5f

Adds header to other request constructors that do not use core fetch

3adc1df

adds validation on not injecting internal product header

a8ca696

Adds api_integration tests for enforcing restrictInternalApis to http…

dc0f209

… lifecycle handlers test

Enables restricting access to internal APIs in serverless mode

5a7463f

adds header to isKibanaRequest in spaces plugin

e200669

Adds new header to security redirect request checked

4d90466

Adds header to x-pack/dev_tools

0a0cb44

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

e823131

…fix'

TinaHeiligers added Feature:http release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.9.0 labels May 5, 2023

TinaHeiligers added 10 commits May 5, 2023 17:16

Fix types

6070213

Type fixes

7d6038a

more type fixes

59583ba

fix types and tests 1

78a69ea

fix core usage stats client unit tests

8a5fedd

Fix typo, comment out for documenting intent only

12a46cb

fix test

30c441f

Adds config setting to docker list

5324c20

removes header from core createKibanaRequest RouterMock

3cae610

deletes debug console

78c0fc0

Merge branch 'main' into kbn-152282-http-handle-access-param-in-core-…

1a9247b

…internal

TinaHeiligers commented May 8, 2023

View reviewed changes

TinaHeiligers marked this pull request as ready for review May 9, 2023 01:45

TinaHeiligers requested review from a team as code owners May 9, 2023 01:45

jloleysens approved these changes May 9, 2023

View reviewed changes

botelastic bot added the Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) label May 9, 2023

jbudz approved these changes May 9, 2023

View reviewed changes

TinaHeiligers added 2 commits May 9, 2023 09:17

tests happy path for retriction enforced on internal routes

e321d1b

Removes internal product origin header from canRedirectRequest

f38d8bd

legrego reviewed May 9, 2023

View reviewed changes

x-pack/plugins/security/server/authentication/can_redirect_request.test.ts Show resolved Hide resolved

legrego approved these changes May 9, 2023

View reviewed changes

lukasolson approved these changes May 9, 2023

View reviewed changes

tsullivan approved these changes May 9, 2023

View reviewed changes

Merge branch 'main' into kbn-152282-http-handle-access-param-in-core-…

24ff4c3

…internal

TinaHeiligers enabled auto-merge (squash) May 10, 2023 00:34

kibanamachine and others added 2 commits May 9, 2023 22:39

Merge branch 'main' into kbn-152282-http-handle-access-param-in-core-…

0093f59

…internal

Merge branch 'main' into kbn-152282-http-handle-access-param-in-core-…

3bd27de

…internal

TinaHeiligers merged commit 7bbe92f into elastic:main May 10, 2023

TinaHeiligers mentioned this pull request May 16, 2023

[http] Prevent access to internal-only APIs when running in serverless #151940

Closed

jeramysoucy mentioned this pull request Jun 12, 2023

[Kibana Core] Allow configuration to specify an array of "disabled routes" for server AND UI #157266

Closed

pheyos mentioned this pull request Jul 12, 2023

Enable restriction of internal APIs for serverless #161733

Closed

juliaElastic mentioned this pull request Sep 6, 2023

Add beat serverless tests & related support elastic/elastic-agent#3258

Closed

4 tasks

TinaHeiligers mentioned this pull request Sep 23, 2024

Globally enforce internal API restriction #193792

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enables preventing access to internal APIs #156935

Enables preventing access to internal APIs #156935

TinaHeiligers commented May 5, 2023 •

edited

Loading

TinaHeiligers commented May 7, 2023

TinaHeiligers May 8, 2023

TinaHeiligers left a comment

TinaHeiligers May 8, 2023

TinaHeiligers May 8, 2023 •

edited

Loading

TinaHeiligers May 8, 2023

TinaHeiligers May 8, 2023

TinaHeiligers May 8, 2023

jloleysens May 8, 2023

TinaHeiligers May 8, 2023

TinaHeiligers left a comment

TinaHeiligers May 8, 2023

legrego May 9, 2023

TinaHeiligers May 9, 2023 •

edited

Loading

legrego May 9, 2023

elasticmachine commented May 9, 2023

jloleysens left a comment

legrego May 9, 2023

lukasolson left a comment

tsullivan left a comment

tsullivan left a comment

TinaHeiligers commented May 10, 2023

TinaHeiligers commented May 10, 2023

kibana-ci commented May 10, 2023

API count

ESLint disabled line counts

Total ESLint disabled count

	it('returns true if request does not have either a kbn-version or kbn-xsrf header or x-elastic-internal-origin', () => {
	it('returns true if request does not have either a kbn-version or kbn-xsrf header', () => {

Enables preventing access to internal APIs #156935

Enables preventing access to internal APIs #156935

Conversation

TinaHeiligers commented May 5, 2023 • edited Loading

Summary

Details

Why are we doing this?

What this means to plugin authors :

Checklist

Risk Matrix

For maintainers

TinaHeiligers commented May 7, 2023

Choose a reason for hiding this comment

TinaHeiligers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TinaHeiligers May 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TinaHeiligers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TinaHeiligers May 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented May 9, 2023

jloleysens left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukasolson left a comment

Choose a reason for hiding this comment

tsullivan left a comment

Choose a reason for hiding this comment

tsullivan left a comment

Choose a reason for hiding this comment

TinaHeiligers commented May 10, 2023

TinaHeiligers commented May 10, 2023

kibana-ci commented May 10, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Page load bundle

API count

ESLint disabled line counts

Total ESLint disabled count

History

TinaHeiligers commented May 5, 2023 •

edited

Loading

TinaHeiligers May 8, 2023 •

edited

Loading

TinaHeiligers May 9, 2023 •

edited

Loading