From 2f8af4eec43cdde267f73ceba1e88aac0cefef45 Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Thu, 27 Apr 2023 17:06:09 -0700 Subject: [PATCH 01/24] Adds check to ensure the internal product header is provided, adds header to handler --- .../src/http_config.ts | 3 +++ .../src/http_server.ts | 16 +++++------ .../src/lifecycle_handlers.test.ts | 11 ++++++++ .../src/lifecycle_handlers.ts | 27 +++++++++++++++++-- packages/kbn-server-http-tools/src/types.ts | 2 ++ 5 files changed, 49 insertions(+), 10 deletions(-) diff --git a/packages/core/http/core-http-server-internal/src/http_config.ts b/packages/core/http/core-http-server-internal/src/http_config.ts index 1fae2568edffd..eea5ebd744bd0 100644 --- a/packages/core/http/core-http-server-internal/src/http_config.ts +++ b/packages/core/http/core-http-server-internal/src/http_config.ts @@ -150,6 +150,7 @@ const configSchema = schema.object( }, } ), + enforceInternalRequest: schema.boolean({ defaultValue: false }), // allow access to internal routes by default to prevent breaking changes in current offerings }, { validate: (rawConfig) => { @@ -223,6 +224,7 @@ export class HttpConfig implements IHttpConfig { public xsrf: { disableProtection: boolean; allowlist: string[] }; public requestId: { allowFromAnyIp: boolean; ipAllowlist: string[] }; public shutdownTimeout: Duration; + public enforceInternalRequest: boolean; /** * @internal @@ -263,6 +265,7 @@ export class HttpConfig implements IHttpConfig { this.xsrf = rawHttpConfig.xsrf; this.requestId = rawHttpConfig.requestId; this.shutdownTimeout = rawHttpConfig.shutdownTimeout; + this.enforceInternalRequest = rawHttpConfig.enforceInternalRequest; } } diff --git a/packages/core/http/core-http-server-internal/src/http_server.ts b/packages/core/http/core-http-server-internal/src/http_server.ts index 2bdf76ee23310..825341f262554 100644 --- a/packages/core/http/core-http-server-internal/src/http_server.ts +++ b/packages/core/http/core-http-server-internal/src/http_server.ts @@ -373,7 +373,7 @@ export class HttpServer { return responseToolkit.continue; }); } - +// 2 private registerOnPreAuth(fn: OnPreAuthHandler) { if (this.server === undefined) { throw new Error('Server is not created yet'); @@ -384,7 +384,7 @@ export class HttpServer { this.server.ext('onPreAuth', adoptToHapiOnPreAuth(fn, this.log)); } - +// 3: validate.headers, validate.params, jsonp, validate.query, validate.payload private registerOnPostAuth(fn: OnPostAuthHandler) { if (this.server === undefined) { throw new Error('Server is not created yet'); @@ -395,7 +395,7 @@ export class HttpServer { this.server.ext('onPostAuth', adoptToHapiOnPostAuthFormat(fn, this.log)); } - + // 1 private registerOnPreRouting(fn: OnPreRoutingHandler) { if (this.server === undefined) { throw new Error('Server is not created yet'); @@ -406,7 +406,7 @@ export class HttpServer { this.server.ext('onRequest', adoptToHapiOnRequest(fn, this.log)); } - +// 4 request-error, send response private registerOnPreResponse(fn: OnPreResponseHandler) { if (this.server === undefined) { throw new Error('Server is not created yet'); @@ -440,7 +440,7 @@ export class HttpServer { ); return sessionStorageFactory; } - +// during auth private registerAuth(fn: AuthenticationHandler) { if (this.server === undefined) { throw new Error('Server is not created yet'); @@ -514,13 +514,13 @@ export class HttpServer { }, }); } - +// in start, after server & router setup, while routes are registered. private configureRoute(route: RouterRoute) { const optionsLogger = this.log.get('options'); this.log.debug(`registering route handler for [${route.path}]`); // Hapi does not allow payload validation to be specified for 'head' or 'get' requests const validate = isSafeMethod(route.method) ? undefined : { payload: true }; - const { authRequired, tags, body = {}, timeout } = route.options; + const { authRequired, tags, body = {}, timeout } = route.options; // includes access: see kibanaRouteOptions const { accepts: allow, maxBytes, output, parse } = body; const kibanaRouteOptions: KibanaRouteOptions = { @@ -540,7 +540,7 @@ export class HttpServer { path: route.path, options: { auth: this.getAuthOption(authRequired), - app: kibanaRouteOptions, + app: kibanaRouteOptions, // includes access tags: tags ? Array.from(tags) : undefined, // TODO: This 'validate' section can be removed once the legacy platform is completely removed. // We are telling Hapi that NP routes can accept any payload, so that it can bypass the default diff --git a/packages/core/http/core-http-server-internal/src/lifecycle_handlers.test.ts b/packages/core/http/core-http-server-internal/src/lifecycle_handlers.test.ts index d13bd001bbbb9..98c6ddd4394a8 100644 --- a/packages/core/http/core-http-server-internal/src/lifecycle_handlers.test.ts +++ b/packages/core/http/core-http-server-internal/src/lifecycle_handlers.test.ts @@ -242,6 +242,17 @@ describe('versionCheck post-auth handler', () => { }); }); +describe('restrictInternal post-auth handler', () => { + let toolkit: ToolkitMock; + let responseFactory: ReturnType; + beforeEach(() => { + toolkit = createToolkit(); + responseFactory = mockRouter.createResponseFactory(); + }); + it.todo('returns a badRequest error if header is missing and required'); + it.todo('forward the request to the next interceptor if header is required and present'); +}); + describe('customHeaders pre-response handler', () => { let toolkit: ToolkitMock; diff --git a/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts b/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts index af148413265e8..df0905fd792e2 100644 --- a/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts +++ b/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts @@ -15,6 +15,7 @@ import { LifecycleRegistrar } from './http_server'; const VERSION_HEADER = 'kbn-version'; const XSRF_HEADER = 'kbn-xsrf'; const KIBANA_NAME_HEADER = 'kbn-name'; +const INTERNAL_ACCESS_ONLY_HEADER = 'x-elastic-internal-origin'; export const createXsrfPostAuthHandler = (config: HttpConfig): OnPostAuthHandler => { const { allowlist, disableProtection } = config.xsrf; @@ -38,6 +39,25 @@ export const createXsrfPostAuthHandler = (config: HttpConfig): OnPostAuthHandler return toolkit.next(); }; }; +// TODO: implement check to ensure the required header is present on requests to internal routes. See https://github.com/elastic/kibana/issues/151940 +// validates the incomming request has the required header if route options access === internal +export const createRestrictInternalRoutesPostAuthHandler = ( + config: HttpConfig +): OnPostAuthHandler => { + const isProductRequestEnforced = config.enforceProductRequest; + + return (request, response, toolkit) => { + const isInternalRoute = request.route.options.access === 'internal'; + const hasInternalKibanaRequestHeader = INTERNAL_ACCESS_ONLY_HEADER in request.headers; + if (isProductRequestEnforced && isInternalRoute && !hasInternalKibanaRequestHeader) { + // throw 400 + return response.badRequest({ + body: `uri [${request.url}] with method [${request.route.method}] exists but is not available with the current configuration`, + }); + } + return toolkit.next(); + }; +}; export const createVersionCheckPostAuthHandler = (kibanaVersion: string): OnPostAuthHandler => { return (request, response, toolkit) => { @@ -60,7 +80,6 @@ export const createVersionCheckPostAuthHandler = (kibanaVersion: string): OnPost }; }; -// TODO: implement header required for accessing internal routes. See https://github.com/elastic/kibana/issues/151940 export const createCustomHeadersPreResponseHandler = (config: HttpConfig): OnPreResponseHandler => { const { name: serverName, @@ -76,7 +95,6 @@ export const createCustomHeadersPreResponseHandler = (config: HttpConfig): OnPre 'Content-Security-Policy': cspHeader, [KIBANA_NAME_HEADER]: serverName, }; - return toolkit.next({ headers: additionalHeaders }); }; }; @@ -86,7 +104,12 @@ export const registerCoreHandlers = ( config: HttpConfig, env: Env ) => { + // add headers based on config registrar.registerOnPreResponse(createCustomHeadersPreResponseHandler(config)); + // add extra request checks stuff registrar.registerOnPostAuth(createXsrfPostAuthHandler(config)); + // add check on version registrar.registerOnPostAuth(createVersionCheckPostAuthHandler(env.packageInfo.version)); + // add check on header if the route is internal + registrar.registerOnPostAuth(createRestrictInternalRoutesPostAuthHandler(config)); // strictly speaking, we should have access to route.options.access from the request on postAuth }; diff --git a/packages/kbn-server-http-tools/src/types.ts b/packages/kbn-server-http-tools/src/types.ts index 9aec520fb3a31..f235abc9a45ea 100644 --- a/packages/kbn-server-http-tools/src/types.ts +++ b/packages/kbn-server-http-tools/src/types.ts @@ -18,6 +18,8 @@ export interface IHttpConfig { cors: ICorsConfig; ssl: ISslConfig; shutdownTimeout: Duration; + // do not allow external requests for internal APIs + enforceProduceRequest: boolean; } export interface ICorsConfig { From 34a0e5f62de7f35240abefc75c535339299da41f Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Sun, 30 Apr 2023 16:19:37 -0700 Subject: [PATCH 02/24] Adds internal origin header in fetch --- .../http/core-http-browser-internal/src/fetch.ts | 16 ++++++++++------ .../core/http/core-http-browser/src/types.ts | 1 + packages/core/http/core-http-common/index.ts | 2 +- .../core/http/core-http-common/src/constants.ts | 2 ++ .../src/lifecycle_handlers.ts | 6 +++--- packages/kbn-server-http-tools/src/types.ts | 2 +- .../bfetch/public/streaming/fetch_streaming.ts | 5 +++-- 7 files changed, 21 insertions(+), 13 deletions(-) diff --git a/packages/core/http/core-http-browser-internal/src/fetch.ts b/packages/core/http/core-http-browser-internal/src/fetch.ts index 494b7c3d2eb58..a6105997ee58b 100644 --- a/packages/core/http/core-http-browser-internal/src/fetch.ts +++ b/packages/core/http/core-http-browser-internal/src/fetch.ts @@ -19,7 +19,7 @@ import type { HttpResponse, HttpFetchOptionsWithPath, } from '@kbn/core-http-browser'; -import { ELASTIC_HTTP_VERSION_HEADER } from '@kbn/core-http-common'; +import { ELASTIC_HTTP_VERSION_HEADER, INTERNAL_ACCESS_REQUEST } from '@kbn/core-http-common'; import { HttpFetchError } from './http_fetch_error'; import { HttpInterceptController } from './http_intercept_controller'; import { interceptRequest, interceptResponse } from './intercept'; @@ -131,6 +131,7 @@ export class Fetch { ...options.headers, 'kbn-version': this.params.kibanaVersion, [ELASTIC_HTTP_VERSION_HEADER]: version, + [INTERNAL_ACCESS_REQUEST]: 'Kibana', ...(!isEmpty(context) ? new ExecutionContextContainer(context).toHeader() : {}), }), }; @@ -222,15 +223,18 @@ const validateFetchArguments = ( `Invalid fetch arguments, must either be (string, object) or (object, undefined), received (${typeof pathOrOptions}, ${typeof options})` ); } - - const invalidHeaders = Object.keys(fullOptions.headers ?? {}).filter((headerName) => - headerName.startsWith('kbn-') + // include `x-elastic-internal-origin` + const invalidHeaders = Object.keys(fullOptions.headers ?? {}).filter( + (headerName) => headerName.startsWith('kbn-') || headerName.includes(INTERNAL_ACCESS_REQUEST) ); if (invalidHeaders.length) { throw new Error( - `Invalid fetch headers, headers beginning with "kbn-" are not allowed: [${invalidHeaders.join( + // `Invalid fetch headers, headers beginning with "kbn-" are not allowed: [${invalidHeaders.join( + // ',' + // )}]` + `Invalid fetch headers: beginning with "kbn-", including "x-elastic-internal-" are not allowed: [${invalidHeaders.join( ',' - )}]` + )}]. Internal origin header` ); } diff --git a/packages/core/http/core-http-browser/src/types.ts b/packages/core/http/core-http-browser/src/types.ts index f49027cf5a0c0..46c9e590081ff 100644 --- a/packages/core/http/core-http-browser/src/types.ts +++ b/packages/core/http/core-http-browser/src/types.ts @@ -148,6 +148,7 @@ export interface IAnonymousPaths { /** * Headers to append to the request. Any headers that begin with `kbn-` are considered private to Core and will cause * {@link HttpHandler} to throw an error. + * Includes the required Header that validates internal requests to internal APIs * @public */ export interface HttpHeadersInit { diff --git a/packages/core/http/core-http-common/index.ts b/packages/core/http/core-http-common/index.ts index 1a2c19b4ea6b2..e2bb29f926b07 100644 --- a/packages/core/http/core-http-common/index.ts +++ b/packages/core/http/core-http-common/index.ts @@ -9,4 +9,4 @@ export type { IExternalUrlPolicy } from './src/external_url_policy'; export type { ApiVersion } from './src/versioning'; -export { ELASTIC_HTTP_VERSION_HEADER } from './src/constants'; +export { ELASTIC_HTTP_VERSION_HEADER, INTERNAL_ACCESS_REQUEST } from './src/constants'; diff --git a/packages/core/http/core-http-common/src/constants.ts b/packages/core/http/core-http-common/src/constants.ts index b231fdd745b36..65c5b007374d9 100644 --- a/packages/core/http/core-http-common/src/constants.ts +++ b/packages/core/http/core-http-common/src/constants.ts @@ -8,3 +8,5 @@ /** @internal */ export const ELASTIC_HTTP_VERSION_HEADER = 'elastic-api-version' as const; + +export const INTERNAL_ACCESS_REQUEST = 'x-elastic-internal-origin' as const; diff --git a/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts b/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts index df0905fd792e2..26b0eed88d80d 100644 --- a/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts +++ b/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts @@ -9,13 +9,13 @@ import { Env } from '@kbn/config'; import type { OnPostAuthHandler, OnPreResponseHandler } from '@kbn/core-http-server'; import { isSafeMethod } from '@kbn/core-http-router-server-internal'; +import { INTERNAL_ACCESS_REQUEST } from '@kbn/core-http-common/src/constants'; import { HttpConfig } from './http_config'; import { LifecycleRegistrar } from './http_server'; const VERSION_HEADER = 'kbn-version'; const XSRF_HEADER = 'kbn-xsrf'; const KIBANA_NAME_HEADER = 'kbn-name'; -const INTERNAL_ACCESS_ONLY_HEADER = 'x-elastic-internal-origin'; export const createXsrfPostAuthHandler = (config: HttpConfig): OnPostAuthHandler => { const { allowlist, disableProtection } = config.xsrf; @@ -44,11 +44,11 @@ export const createXsrfPostAuthHandler = (config: HttpConfig): OnPostAuthHandler export const createRestrictInternalRoutesPostAuthHandler = ( config: HttpConfig ): OnPostAuthHandler => { - const isProductRequestEnforced = config.enforceProductRequest; + const isProductRequestEnforced = config.enforceInternalRequest; return (request, response, toolkit) => { const isInternalRoute = request.route.options.access === 'internal'; - const hasInternalKibanaRequestHeader = INTERNAL_ACCESS_ONLY_HEADER in request.headers; + const hasInternalKibanaRequestHeader = INTERNAL_ACCESS_REQUEST in request.headers; if (isProductRequestEnforced && isInternalRoute && !hasInternalKibanaRequestHeader) { // throw 400 return response.badRequest({ diff --git a/packages/kbn-server-http-tools/src/types.ts b/packages/kbn-server-http-tools/src/types.ts index f235abc9a45ea..69049a8f45f46 100644 --- a/packages/kbn-server-http-tools/src/types.ts +++ b/packages/kbn-server-http-tools/src/types.ts @@ -19,7 +19,7 @@ export interface IHttpConfig { ssl: ISslConfig; shutdownTimeout: Duration; // do not allow external requests for internal APIs - enforceProduceRequest: boolean; + enforceInternalRequest: boolean; } export interface ICorsConfig { diff --git a/src/plugins/bfetch/public/streaming/fetch_streaming.ts b/src/plugins/bfetch/public/streaming/fetch_streaming.ts index 77e5acffc1af3..b702c6ba2c5be 100644 --- a/src/plugins/bfetch/public/streaming/fetch_streaming.ts +++ b/src/plugins/bfetch/public/streaming/fetch_streaming.ts @@ -39,13 +39,14 @@ export function fetchStreaming({ if (!isCompressionDisabled) { url = appendQueryParam(url, 'compress', 'true'); } - // Begin the request xhr.open(method, url); xhr.withCredentials = true; // Set the HTTP headers - Object.entries(headers).forEach(([k, v]) => xhr.setRequestHeader(k, v)); + Object.entries({ ...headers, 'x-elastic-internal-origin': 'Kibana' }).forEach(([k, v]) => + xhr.setRequestHeader(k, v) + ); const stream = fromStreamingXhr(xhr, signal); From 3adc1df8cc9244b8625c184c6a954e01fc4e0358 Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Sun, 30 Apr 2023 18:09:17 -0700 Subject: [PATCH 03/24] Adds header to other request constructors that do not use core fetch --- .../src/fetch.test.ts | 8 +- .../__snapshots__/http_config.test.ts.snap | 1 + .../src/http_config.ts | 6 +- .../src/http_server.ts | 16 +-- .../src/lifecycle_handlers.test.ts | 97 ++++++++++++++++++- .../src/lifecycle_handlers.ts | 10 +- .../core-http-server-mocks/src/test_utils.ts | 1 + .../src/routes/utils.test.ts | 6 +- .../src/routes/utils.ts | 4 +- .../src/core_usage_stats_client.ts | 4 +- packages/kbn-journeys/services/auth.ts | 1 + packages/kbn-server-http-tools/src/types.ts | 3 +- .../http/lifecycle_handlers.test.ts | 1 + src/plugins/bfetch/public/plugin.ts | 2 + .../public/streaming/fetch_streaming.ts | 4 +- .../dev-tools/api_debug/request_from_api.js | 2 + 16 files changed, 140 insertions(+), 26 deletions(-) diff --git a/packages/core/http/core-http-browser-internal/src/fetch.test.ts b/packages/core/http/core-http-browser-internal/src/fetch.test.ts index 28a89fb099183..e43ade8829004 100644 --- a/packages/core/http/core-http-browser-internal/src/fetch.test.ts +++ b/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', myheader: 'foo', }); }); @@ -168,7 +169,11 @@ describe('Fetch', () => { fetchMock.get('*', {}); await expect( fetchInstance.fetch('/my/path', { - headers: { myHeader: 'foo', 'kbn-version': 'CUSTOM!' }, + headers: { + myHeader: 'foo', + 'kbn-version': 'CUSTOM!', + 'x-elastic-internal-origin': 'Kibana', + }, }) ).rejects.toThrowErrorMatchingInlineSnapshot( `"Invalid fetch headers, headers beginning with \\"kbn-\\" are not allowed: [kbn-version]"` @@ -310,6 +315,7 @@ describe('Fetch', () => { headers: { 'content-type': 'application/json', 'kbn-version': 'VERSION', + 'x-elastic-internal-origin': 'Kibana', }, }); }); diff --git a/packages/core/http/core-http-server-internal/src/__snapshots__/http_config.test.ts.snap b/packages/core/http/core-http-server-internal/src/__snapshots__/http_config.test.ts.snap index 4742ac4d1fe48..87a5efcc5d279 100644 --- a/packages/core/http/core-http-server-internal/src/__snapshots__/http_config.test.ts.snap +++ b/packages/core/http/core-http-server-internal/src/__snapshots__/http_config.test.ts.snap @@ -67,6 +67,7 @@ Object { "allowFromAnyIp": false, "ipAllowlist": Array [], }, + "restrictInternalApis": false, "rewriteBasePath": false, "securityResponseHeaders": Object { "crossOriginOpenerPolicy": "same-origin", diff --git a/packages/core/http/core-http-server-internal/src/http_config.ts b/packages/core/http/core-http-server-internal/src/http_config.ts index eea5ebd744bd0..53417d9c533f8 100644 --- a/packages/core/http/core-http-server-internal/src/http_config.ts +++ b/packages/core/http/core-http-server-internal/src/http_config.ts @@ -150,7 +150,7 @@ const configSchema = schema.object( }, } ), - enforceInternalRequest: schema.boolean({ defaultValue: false }), // allow access to internal routes by default to prevent breaking changes in current offerings + restrictInternalApis: schema.boolean({ defaultValue: false }), // allow access to internal routes by default to prevent breaking changes in current offerings }, { validate: (rawConfig) => { @@ -224,7 +224,7 @@ export class HttpConfig implements IHttpConfig { public xsrf: { disableProtection: boolean; allowlist: string[] }; public requestId: { allowFromAnyIp: boolean; ipAllowlist: string[] }; public shutdownTimeout: Duration; - public enforceInternalRequest: boolean; + public restrictInternalApis: boolean; /** * @internal @@ -265,7 +265,7 @@ export class HttpConfig implements IHttpConfig { this.xsrf = rawHttpConfig.xsrf; this.requestId = rawHttpConfig.requestId; this.shutdownTimeout = rawHttpConfig.shutdownTimeout; - this.enforceInternalRequest = rawHttpConfig.enforceInternalRequest; + this.restrictInternalApis = rawHttpConfig.restrictInternalApis; } } diff --git a/packages/core/http/core-http-server-internal/src/http_server.ts b/packages/core/http/core-http-server-internal/src/http_server.ts index 825341f262554..2bdf76ee23310 100644 --- a/packages/core/http/core-http-server-internal/src/http_server.ts +++ b/packages/core/http/core-http-server-internal/src/http_server.ts @@ -373,7 +373,7 @@ export class HttpServer { return responseToolkit.continue; }); } -// 2 + private registerOnPreAuth(fn: OnPreAuthHandler) { if (this.server === undefined) { throw new Error('Server is not created yet'); @@ -384,7 +384,7 @@ export class HttpServer { this.server.ext('onPreAuth', adoptToHapiOnPreAuth(fn, this.log)); } -// 3: validate.headers, validate.params, jsonp, validate.query, validate.payload + private registerOnPostAuth(fn: OnPostAuthHandler) { if (this.server === undefined) { throw new Error('Server is not created yet'); @@ -395,7 +395,7 @@ export class HttpServer { this.server.ext('onPostAuth', adoptToHapiOnPostAuthFormat(fn, this.log)); } - // 1 + private registerOnPreRouting(fn: OnPreRoutingHandler) { if (this.server === undefined) { throw new Error('Server is not created yet'); @@ -406,7 +406,7 @@ export class HttpServer { this.server.ext('onRequest', adoptToHapiOnRequest(fn, this.log)); } -// 4 request-error, send response + private registerOnPreResponse(fn: OnPreResponseHandler) { if (this.server === undefined) { throw new Error('Server is not created yet'); @@ -440,7 +440,7 @@ export class HttpServer { ); return sessionStorageFactory; } -// during auth + private registerAuth(fn: AuthenticationHandler) { if (this.server === undefined) { throw new Error('Server is not created yet'); @@ -514,13 +514,13 @@ export class HttpServer { }, }); } -// in start, after server & router setup, while routes are registered. + private configureRoute(route: RouterRoute) { const optionsLogger = this.log.get('options'); this.log.debug(`registering route handler for [${route.path}]`); // Hapi does not allow payload validation to be specified for 'head' or 'get' requests const validate = isSafeMethod(route.method) ? undefined : { payload: true }; - const { authRequired, tags, body = {}, timeout } = route.options; // includes access: see kibanaRouteOptions + const { authRequired, tags, body = {}, timeout } = route.options; const { accepts: allow, maxBytes, output, parse } = body; const kibanaRouteOptions: KibanaRouteOptions = { @@ -540,7 +540,7 @@ export class HttpServer { path: route.path, options: { auth: this.getAuthOption(authRequired), - app: kibanaRouteOptions, // includes access + app: kibanaRouteOptions, tags: tags ? Array.from(tags) : undefined, // TODO: This 'validate' section can be removed once the legacy platform is completely removed. // We are telling Hapi that NP routes can accept any payload, so that it can bypass the default diff --git a/packages/core/http/core-http-server-internal/src/lifecycle_handlers.test.ts b/packages/core/http/core-http-server-internal/src/lifecycle_handlers.test.ts index 98c6ddd4394a8..03f1dfc4e2006 100644 --- a/packages/core/http/core-http-server-internal/src/lifecycle_handlers.test.ts +++ b/packages/core/http/core-http-server-internal/src/lifecycle_handlers.test.ts @@ -13,10 +13,12 @@ import type { OnPreResponseToolkit, OnPostAuthToolkit, OnPreRoutingToolkit, + OnPostAuthHandler, } from '@kbn/core-http-server'; import { mockRouter } from '@kbn/core-http-router-server-mocks'; import { createCustomHeadersPreResponseHandler, + createRestrictInternalRoutesPostAuthHandler, createVersionCheckPostAuthHandler, createXsrfPostAuthHandler, } from './lifecycle_handlers'; @@ -245,12 +247,103 @@ describe('versionCheck post-auth handler', () => { describe('restrictInternal post-auth handler', () => { let toolkit: ToolkitMock; let responseFactory: ReturnType; + beforeEach(() => { toolkit = createToolkit(); responseFactory = mockRouter.createResponseFactory(); }); - it.todo('returns a badRequest error if header is missing and required'); - it.todo('forward the request to the next interceptor if header is required and present'); + const createForgeRequest = ( + access: 'internal' | 'public', + headers: Record | undefined = {} + ) => { + return forgeRequest({ + method: 'get', + headers, + path: `/${access}/some-path`, + kibanaRouteOptions: { + xsrfRequired: false, + access, + }, + }); + }; + + const createForwardSuccess = (handler: OnPostAuthHandler, request: KibanaRequest) => { + toolkit.next.mockReturnValue('next' as any); + const result = handler(request, responseFactory, toolkit); + + expect(toolkit.next).toHaveBeenCalledTimes(1); + expect(responseFactory.badRequest).not.toHaveBeenCalled(); + expect(result).toBe('next'); + }; + + describe('when restriction is enabled', () => { + const config = createConfig({ + name: 'my-server-name', + restrictInternalApis: true, + }); + it('returns a bad request if called without internal origin header for internal API', () => { + const handler = createRestrictInternalRoutesPostAuthHandler(config as HttpConfig); + const request = createForgeRequest('internal'); + + responseFactory.badRequest.mockReturnValue('badRequest' as any); + + const result = handler(request, responseFactory, toolkit); + + expect(toolkit.next).not.toHaveBeenCalled(); + expect(responseFactory.badRequest.mock.calls[0][0]?.body).toMatch( + /uri \[.*\/internal\/some-path\] with method \[get\] exists but is not available with the current configuration/ + ); + expect(result).toBe('badRequest'); + }); + + it('forward the request to the next interceptor if called with internal origin header for internal API', () => { + const handler = createRestrictInternalRoutesPostAuthHandler(config as HttpConfig); + const request = createForgeRequest('internal', { 'x-elastic-internal-origin': 'Kibana' }); + createForwardSuccess(handler, request); + }); + + it('forward the request to the next interceptor if called with internal origin header for public APIs', () => { + const handler = createRestrictInternalRoutesPostAuthHandler(config as HttpConfig); + const request = createForgeRequest('public', { 'x-elastic-internal-origin': 'Kibana' }); + createForwardSuccess(handler, request); + }); + + it('forward the request to the next interceptor if called without internal origin header for public APIs', () => { + const handler = createRestrictInternalRoutesPostAuthHandler(config as HttpConfig); + const request = createForgeRequest('public'); + createForwardSuccess(handler, request); + }); + }); + + describe('when restriction is not enabled', () => { + const config = createConfig({ + name: 'my-server-name', + restrictInternalApis: false, + }); + it('forward the request to the next interceptor if called without internal origin header for internal APIs', () => { + const handler = createRestrictInternalRoutesPostAuthHandler(config as HttpConfig); + const request = createForgeRequest('internal'); + createForwardSuccess(handler, request); + }); + + it('forward the request to the next interceptor if called with internal origin header for internal API', () => { + const handler = createRestrictInternalRoutesPostAuthHandler(config as HttpConfig); + const request = createForgeRequest('internal', { 'x-elastic-internal-origin': 'Kibana' }); + createForwardSuccess(handler, request); + }); + + it('forward the request to the next interceptor if called without internal origin header for public APIs', () => { + const handler = createRestrictInternalRoutesPostAuthHandler(config as HttpConfig); + const request = createForgeRequest('public'); + createForwardSuccess(handler, request); + }); + + it('forward the request to the next interceptor if called with internal origin header for public APIs', () => { + const handler = createRestrictInternalRoutesPostAuthHandler(config as HttpConfig); + const request = createForgeRequest('public', { 'x-elastic-internal-origin': 'Kibana' }); + createForwardSuccess(handler, request); + }); + }); }); describe('customHeaders pre-response handler', () => { diff --git a/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts b/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts index 26b0eed88d80d..a97255790ed03 100644 --- a/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts +++ b/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts @@ -39,17 +39,19 @@ export const createXsrfPostAuthHandler = (config: HttpConfig): OnPostAuthHandler return toolkit.next(); }; }; -// TODO: implement check to ensure the required header is present on requests to internal routes. See https://github.com/elastic/kibana/issues/151940 -// validates the incomming request has the required header if route options access === internal + export const createRestrictInternalRoutesPostAuthHandler = ( config: HttpConfig ): OnPostAuthHandler => { - const isProductRequestEnforced = config.enforceInternalRequest; + const isRestrictionEnabled = config.restrictInternalApis; return (request, response, toolkit) => { const isInternalRoute = request.route.options.access === 'internal'; + + // only check if the header is present, not it's content. const hasInternalKibanaRequestHeader = INTERNAL_ACCESS_REQUEST in request.headers; - if (isProductRequestEnforced && isInternalRoute && !hasInternalKibanaRequestHeader) { + + if (isRestrictionEnabled && isInternalRoute && !hasInternalKibanaRequestHeader) { // throw 400 return response.badRequest({ body: `uri [${request.url}] with method [${request.route.method}] exists but is not available with the current configuration`, diff --git a/packages/core/http/core-http-server-mocks/src/test_utils.ts b/packages/core/http/core-http-server-mocks/src/test_utils.ts index 17b493e96b869..f0b50a828506c 100644 --- a/packages/core/http/core-http-server-mocks/src/test_utils.ts +++ b/packages/core/http/core-http-server-mocks/src/test_utils.ts @@ -50,6 +50,7 @@ const createConfigService = () => { shutdownTimeout: moment.duration(30, 'seconds'), keepaliveTimeout: 120_000, socketTimeout: 120_000, + restrictInternalApis: false, } as any); } if (path === 'externalUrl') { diff --git a/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/utils.test.ts b/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/utils.test.ts index 43c633290efcf..c002e12363af6 100644 --- a/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/utils.test.ts +++ b/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/utils.test.ts @@ -347,7 +347,11 @@ describe('throwIfAnyTypeNotVisibleByAPI', () => { describe('logWarnOnExternalRequest', () => { let logger: MockedLogger; - const firstPartyRequestHeaders = { 'kbn-version': 'a', referer: 'b' }; + const firstPartyRequestHeaders = { + 'kbn-version': 'a', + referer: 'b', + 'x-elastic-internal-origin': 'foo', + }; const kibRequest = httpServerMock.createKibanaRequest({ headers: firstPartyRequestHeaders }); const extRequest = httpServerMock.createKibanaRequest(); diff --git a/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/utils.ts b/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/utils.ts index 03023d167a38f..7daea1a33237e 100644 --- a/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/utils.ts +++ b/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'] + ); } export interface LogWarnOnExternalRequest { diff --git a/packages/core/usage-data/core-usage-data-server-internal/src/core_usage_stats_client.ts b/packages/core/usage-data/core-usage-data-server-internal/src/core_usage_stats_client.ts index fdb653c5c5a2a..6a4a48568d9dc 100644 --- a/packages/core/usage-data/core-usage-data-server-internal/src/core_usage_stats_client.ts +++ b/packages/core/usage-data/core-usage-data-server-internal/src/core_usage_stats_client.ts @@ -247,5 +247,7 @@ function getFieldsForCounter(prefix: string) { function getIsKibanaRequest({ 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'] + ); } diff --git a/packages/kbn-journeys/services/auth.ts b/packages/kbn-journeys/services/auth.ts index b8c68a9fbb09c..20066b0687a77 100644 --- a/packages/kbn-journeys/services/auth.ts +++ b/packages/kbn-journeys/services/auth.ts @@ -56,6 +56,7 @@ export class Auth { 'kbn-version': await this.kibanaServer.version.get(), 'sec-fetch-mode': 'cors', 'sec-fetch-site': 'same-origin', + 'x-elastic-internal-origin': 'Kibana', }, validateStatus: () => true, maxRedirects: 0, diff --git a/packages/kbn-server-http-tools/src/types.ts b/packages/kbn-server-http-tools/src/types.ts index 69049a8f45f46..57832de28e724 100644 --- a/packages/kbn-server-http-tools/src/types.ts +++ b/packages/kbn-server-http-tools/src/types.ts @@ -18,8 +18,7 @@ export interface IHttpConfig { cors: ICorsConfig; ssl: ISslConfig; shutdownTimeout: Duration; - // do not allow external requests for internal APIs - enforceInternalRequest: boolean; + restrictInternalApis: boolean; } export interface ICorsConfig { diff --git a/src/core/server/integration_tests/http/lifecycle_handlers.test.ts b/src/core/server/integration_tests/http/lifecycle_handlers.test.ts index 7033abc4174fa..e612ae054bcfa 100644 --- a/src/core/server/integration_tests/http/lifecycle_handlers.test.ts +++ b/src/core/server/integration_tests/http/lifecycle_handlers.test.ts @@ -25,6 +25,7 @@ const nameHeader = 'kbn-name'; const allowlistedTestPath = '/xsrf/test/route/whitelisted'; const xsrfDisabledTestPath = '/xsrf/test/route/disabled'; const kibanaName = 'my-kibana-name'; +const internalProductHeader = 'x-elastic-internal-origin'; const setupDeps = { context: contextServiceMock.createSetupContract(), executionContext: executionContextServiceMock.createInternalSetupContract(), diff --git a/src/plugins/bfetch/public/plugin.ts b/src/plugins/bfetch/public/plugin.ts index 5e4357139fa7b..05b4e2123eaa0 100644 --- a/src/plugins/bfetch/public/plugin.ts +++ b/src/plugins/bfetch/public/plugin.ts @@ -8,6 +8,7 @@ import { CoreSetup, CoreStart, Plugin, PluginInitializerContext } from '@kbn/core/public'; import { createStartServicesGetter } from '@kbn/kibana-utils-plugin/public'; +import { INTERNAL_ACCESS_REQUEST } from '@kbn/core-http-common'; import { fetchStreaming as fetchStreamingStatic, FetchStreamingParams } from './streaming'; import { DISABLE_BFETCH_COMPRESSION, removeLeadingSlash } from '../common'; import { createStreamingBatchedFunction, StreamingBatchedFunctionParams } from './batching'; @@ -83,6 +84,7 @@ export class BfetchPublicPlugin headers: { 'Content-Type': 'application/json', 'kbn-version': version, + [INTERNAL_ACCESS_REQUEST]: 'Kibana', ...(params.headers || {}), }, getIsCompressionDisabled, diff --git a/src/plugins/bfetch/public/streaming/fetch_streaming.ts b/src/plugins/bfetch/public/streaming/fetch_streaming.ts index b702c6ba2c5be..d5e99d7eea4c0 100644 --- a/src/plugins/bfetch/public/streaming/fetch_streaming.ts +++ b/src/plugins/bfetch/public/streaming/fetch_streaming.ts @@ -44,9 +44,7 @@ export function fetchStreaming({ xhr.withCredentials = true; // Set the HTTP headers - Object.entries({ ...headers, 'x-elastic-internal-origin': 'Kibana' }).forEach(([k, v]) => - xhr.setRequestHeader(k, v) - ); + Object.entries(headers).forEach(([k, v]) => xhr.setRequestHeader(k, v)); const stream = fromStreamingXhr(xhr, signal); diff --git a/x-pack/dev-tools/api_debug/request_from_api.js b/x-pack/dev-tools/api_debug/request_from_api.js index ca555f3b62153..e11558f973d82 100644 --- a/x-pack/dev-tools/api_debug/request_from_api.js +++ b/x-pack/dev-tools/api_debug/request_from_api.js @@ -9,6 +9,7 @@ import fetch from 'node-fetch'; import { resolve } from 'path'; import abab from 'abab'; import pkg from '../../package.json'; +import { INTERNAL_ACCESS_REQUEST } from '@kbn/core-http-common/src/constants'; function getRequestParams(argv) { // use `--host=https://somedomain.com:5601` or else http://localhost:5601 is defaulted @@ -32,6 +33,7 @@ function getRequestHeaders(auth) { 'kbn-version': pkg.version, 'Content-Type': 'application/json', Authorization: auth, + [INTERNAL_ACCESS_REQUEST]: 'kibana', }; } From a8ca69616bfb69c5321d7da5fb42fd8e20510fec Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Wed, 3 May 2023 16:41:28 -0700 Subject: [PATCH 04/24] adds validation on not injecting internal product header --- .../src/fetch.test.ts | 15 ++++++++++- .../core-http-browser-internal/src/fetch.ts | 26 ++++++++++++------- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/packages/core/http/core-http-browser-internal/src/fetch.test.ts b/packages/core/http/core-http-browser-internal/src/fetch.test.ts index e43ade8829004..92539cfe14c66 100644 --- a/packages/core/http/core-http-browser-internal/src/fetch.test.ts +++ b/packages/core/http/core-http-browser-internal/src/fetch.test.ts @@ -172,7 +172,6 @@ describe('Fetch', () => { headers: { myHeader: 'foo', 'kbn-version': 'CUSTOM!', - 'x-elastic-internal-origin': 'Kibana', }, }) ).rejects.toThrowErrorMatchingInlineSnapshot( @@ -180,6 +179,20 @@ describe('Fetch', () => { ); }); + it('should not allow overwriting of x-elastic-internal-origin header', async () => { + fetchMock.get('*', {}); + await expect( + fetchInstance.fetch('/my/path', { + headers: { + myHeader: 'foo', + 'x-elastic-internal-origin': 'anything', + }, + }) + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"Invalid fetch headers, headers beginning with \\"x-elastic-internal-\\" are not allowed: [x-elastic-internal-origin]"` + ); + }); + it('should not set kbn-system-request header by default', async () => { fetchMock.get('*', {}); await fetchInstance.fetch('/my/path', { diff --git a/packages/core/http/core-http-browser-internal/src/fetch.ts b/packages/core/http/core-http-browser-internal/src/fetch.ts index a6105997ee58b..9ca8d6c756346 100644 --- a/packages/core/http/core-http-browser-internal/src/fetch.ts +++ b/packages/core/http/core-http-browser-internal/src/fetch.ts @@ -223,18 +223,26 @@ const validateFetchArguments = ( `Invalid fetch arguments, must either be (string, object) or (object, undefined), received (${typeof pathOrOptions}, ${typeof options})` ); } - // include `x-elastic-internal-origin` - const invalidHeaders = Object.keys(fullOptions.headers ?? {}).filter( - (headerName) => headerName.startsWith('kbn-') || headerName.includes(INTERNAL_ACCESS_REQUEST) + + const invalidKbnHeaders = Object.keys(fullOptions.headers ?? {}).filter((headerName) => + headerName.startsWith('kbn-') + ); + const invalidInternalOriginProducHeader = Object.keys(fullOptions.headers ?? {}).filter( + (headerName) => headerName.includes(INTERNAL_ACCESS_REQUEST) ); - if (invalidHeaders.length) { + + if (invalidKbnHeaders.length) { + throw new Error( + `Invalid fetch headers, headers beginning with "kbn-" are not allowed: [${invalidKbnHeaders.join( + ',' + )}]` + ); + } + if (invalidInternalOriginProducHeader.length) { throw new Error( - // `Invalid fetch headers, headers beginning with "kbn-" are not allowed: [${invalidHeaders.join( - // ',' - // )}]` - `Invalid fetch headers: beginning with "kbn-", including "x-elastic-internal-" are not allowed: [${invalidHeaders.join( + `Invalid fetch headers, headers beginning with "x-elastic-internal-" are not allowed: [${invalidInternalOriginProducHeader.join( ',' - )}]. Internal origin header` + )}]` ); } From dc0f209d4d5a7954832489a9dab1c808476112c7 Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Thu, 4 May 2023 16:27:09 -0700 Subject: [PATCH 05/24] Adds api_integration tests for enforcing restrictInternalApis to http lifecycle handlers test --- .../http/lifecycle_handlers.test.ts | 135 ++++++++++++++++++ 1 file changed, 135 insertions(+) diff --git a/src/core/server/integration_tests/http/lifecycle_handlers.test.ts b/src/core/server/integration_tests/http/lifecycle_handlers.test.ts index e612ae054bcfa..926b8216d19c2 100644 --- a/src/core/server/integration_tests/http/lifecycle_handlers.test.ts +++ b/src/core/server/integration_tests/http/lifecycle_handlers.test.ts @@ -31,6 +31,43 @@ const setupDeps = { executionContext: executionContextServiceMock.createInternalSetupContract(), }; +interface HttpConfigTestOptions { + enabled?: boolean; +} +const setUpDefaultServerConfig = ({ enabled }: HttpConfigTestOptions = {}) => + ({ + hosts: ['localhost'], + maxPayload: new ByteSizeValue(1024), + shutdownTimeout: moment.duration(30, 'seconds'), + autoListen: true, + ssl: { + enabled: false, + }, + cors: { + enabled: false, + }, + compression: { enabled: true, brotli: { enabled: false } }, + name: kibanaName, + securityResponseHeaders: { + // reflects default config + strictTransportSecurity: null, + xContentTypeOptions: 'nosniff', + referrerPolicy: 'strict-origin-when-cross-origin', + permissionsPolicy: null, + crossOriginOpenerPolicy: 'same-origin', + }, + customResponseHeaders: { + 'some-header': 'some-value', + 'referrer-policy': 'strict-origin', // overrides a header that is defined by securityResponseHeaders + }, + xsrf: { disableProtection: false, allowlist: [allowlistedTestPath] }, + requestId: { + allowFromAnyIp: true, + ipAllowlist: [], + }, + restrictInternalApis: enabled ?? false, // reflects default for public routes + } as any); + describe('core lifecycle handlers', () => { let server: HttpService; let innerServer: HttpServerSetup['server']; @@ -248,4 +285,102 @@ describe('core lifecycle handlers', () => { }); }); }); + + describe('restrictInternalRoutes post-auth handler', () => { + const testInternalRoute = '/restrict_internal_routes/test/route_internal'; + const testPublicRoute = '/restrict_internal_routes/test/route_public'; + beforeEach(async () => { + router.get( + { path: testInternalRoute, validate: false, options: { access: 'internal' } }, + (context, req, res) => { + return res.ok({ body: 'ok()' }); + } + ); + router.get( + { path: testPublicRoute, validate: false, options: { access: 'public' } }, + (context, req, res) => { + return res.ok({ body: 'ok()' }); + } + ); + await server.start(); + }); + + it('accepts requests with the internal product header to internal routes', async () => { + await supertest(innerServer.listener) + .get(testInternalRoute) + .set(internalProductHeader, 'anything') + .expect(200, 'ok()'); + }); + + it('accepts requests with the internal product header to public routes', async () => { + await supertest(innerServer.listener) + .get(testPublicRoute) + .set(internalProductHeader, 'anything') + .expect(200, 'ok()'); + }); + }); +}); + +describe('core lifecyle handers with restrict internal routes enforced', () => { + let server: HttpService; + let innerServer: HttpServerSetup['server']; + let router: IRouter; + + beforeEach(async () => { + const configService = configServiceMock.create(); + configService.atPath.mockImplementation((path) => { + if (path === 'server') { + return new BehaviorSubject(setUpDefaultServerConfig({ enabled: true })); + } + if (path === 'externalUrl') { + return new BehaviorSubject({ + policy: [], + } as any); + } + if (path === 'csp') { + return new BehaviorSubject({ + strict: false, + disableEmbedding: false, + warnLegacyBrowsers: true, + }); + } + + throw new Error(`Unexpected config path: ${path}`); + }); + server = createHttpServer({ configService }); + + await server.preboot({ context: contextServiceMock.createPrebootContract() }); + const serverSetup = await server.setup(setupDeps); + router = serverSetup.createRouter('/'); + innerServer = serverSetup.server; + }); + + afterEach(async () => { + await server.stop(); + }); + + describe('restrictInternalRoutes postauth handler', () => { + const testInternalRoute = '/restrict_internal_routes/test/route_internal'; + const testPublicRoute = '/restrict_internal_routes/test/route_public'; + beforeEach(async () => { + router.get( + { path: testInternalRoute, validate: false, options: { access: 'internal' } }, + (context, req, res) => { + return res.ok({ body: 'ok()' }); + } + ); + router.get( + { path: testPublicRoute, validate: false, options: { access: 'public' } }, + (context, req, res) => { + return res.ok({ body: 'ok()' }); + } + ); + await server.start(); + }); + + it('request requests without the internal product header to internal routes', async () => { + const result = await supertest(innerServer.listener).get(testInternalRoute).expect(400); + expect(result.body.error).toBe('Bad Request'); + }); + }); }); From 5a7463f44cf06f383c8a8d45d4de80e33e981f9e Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Fri, 5 May 2023 08:57:05 -0700 Subject: [PATCH 06/24] Enables restricting access to internal APIs in serverless mode --- config/serverless.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/config/serverless.yml b/config/serverless.yml index a5177de771688..ff68b1ed5dd21 100644 --- a/config/serverless.yml +++ b/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 restriction to internal APIs see https://github.com/elastic/kibana/issues/151940 +server.http.restrictInternalApis: true From e200669ff9846ede8b6594dd7a6ca76ab4c6b761 Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Fri, 5 May 2023 11:16:23 -0700 Subject: [PATCH 07/24] adds header to isKibanaRequest in spaces plugin --- .../spaces/server/usage_stats/usage_stats_client.test.ts | 6 +++++- .../plugins/spaces/server/usage_stats/usage_stats_client.ts | 6 ++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/spaces/server/usage_stats/usage_stats_client.test.ts b/x-pack/plugins/spaces/server/usage_stats/usage_stats_client.test.ts index cab16e55118ed..32c27c74c9f79 100644 --- a/x-pack/plugins/spaces/server/usage_stats/usage_stats_client.test.ts +++ b/x-pack/plugins/spaces/server/usage_stats/usage_stats_client.test.ts @@ -27,7 +27,11 @@ describe('UsageStatsClient', () => { return { usageStatsClient, debugLoggerMock, repositoryMock }; }; - const firstPartyRequestHeaders = { 'kbn-version': 'a', referer: 'b' }; // as long as these two header fields are truthy, this will be treated like a first-party request + const firstPartyRequestHeaders = { + 'kbn-version': 'a', + referer: 'b', + 'x-elastic-internal-origin': 'c', + }; // as long as these header fields are truthy, this will be treated like a first-party request const incrementOptions = { refresh: false }; describe('#getUsageStats', () => { diff --git a/x-pack/plugins/spaces/server/usage_stats/usage_stats_client.ts b/x-pack/plugins/spaces/server/usage_stats/usage_stats_client.ts index 7f30d8981346b..0c2c43eef68a7 100644 --- a/x-pack/plugins/spaces/server/usage_stats/usage_stats_client.ts +++ b/x-pack/plugins/spaces/server/usage_stats/usage_stats_client.ts @@ -119,7 +119,9 @@ export class UsageStatsClient { } function getIsKibanaRequest(headers?: Headers) { - // The presence of these two request headers gives us a good indication that this is a first-party request from the Kibana client. + // The presence of these 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'] + ); } From 4d90466ec06fad073ef182d7f921baf5a801b04e Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Fri, 5 May 2023 11:50:07 -0700 Subject: [PATCH 08/24] Adds new header to security redirect request checked --- .../core/http/core-http-browser-internal/src/fetch.ts | 9 ++++++--- packages/core/http/core-http-common/index.ts | 2 +- packages/core/http/core-http-common/src/constants.ts | 2 +- .../src/lifecycle_handlers.ts | 4 ++-- src/plugins/bfetch/public/plugin.ts | 4 ++-- x-pack/dev-tools/api_debug/request_from_api.js | 4 ++-- .../server/authentication/can_redirect_request.test.ts | 8 ++++++++ .../server/authentication/can_redirect_request.ts | 4 +++- x-pack/test/common/services/bsearch_secure.ts | 10 ++++++++++ 9 files changed, 35 insertions(+), 12 deletions(-) diff --git a/packages/core/http/core-http-browser-internal/src/fetch.ts b/packages/core/http/core-http-browser-internal/src/fetch.ts index 9ca8d6c756346..0997329c84798 100644 --- a/packages/core/http/core-http-browser-internal/src/fetch.ts +++ b/packages/core/http/core-http-browser-internal/src/fetch.ts @@ -19,7 +19,10 @@ import type { HttpResponse, HttpFetchOptionsWithPath, } from '@kbn/core-http-browser'; -import { ELASTIC_HTTP_VERSION_HEADER, INTERNAL_ACCESS_REQUEST } from '@kbn/core-http-common'; +import { + ELASTIC_HTTP_VERSION_HEADER, + X_ELASTIC_INTERNAL_ORIGIN_REQUEST, +} from '@kbn/core-http-common'; import { HttpFetchError } from './http_fetch_error'; import { HttpInterceptController } from './http_intercept_controller'; import { interceptRequest, interceptResponse } from './intercept'; @@ -131,7 +134,7 @@ export class Fetch { ...options.headers, 'kbn-version': this.params.kibanaVersion, [ELASTIC_HTTP_VERSION_HEADER]: version, - [INTERNAL_ACCESS_REQUEST]: 'Kibana', + [X_ELASTIC_INTERNAL_ORIGIN_REQUEST]: 'Kibana', ...(!isEmpty(context) ? new ExecutionContextContainer(context).toHeader() : {}), }), }; @@ -228,7 +231,7 @@ const validateFetchArguments = ( headerName.startsWith('kbn-') ); const invalidInternalOriginProducHeader = Object.keys(fullOptions.headers ?? {}).filter( - (headerName) => headerName.includes(INTERNAL_ACCESS_REQUEST) + (headerName) => headerName.includes(X_ELASTIC_INTERNAL_ORIGIN_REQUEST) ); if (invalidKbnHeaders.length) { diff --git a/packages/core/http/core-http-common/index.ts b/packages/core/http/core-http-common/index.ts index e2bb29f926b07..f430db35b6a1d 100644 --- a/packages/core/http/core-http-common/index.ts +++ b/packages/core/http/core-http-common/index.ts @@ -9,4 +9,4 @@ export type { IExternalUrlPolicy } from './src/external_url_policy'; export type { ApiVersion } from './src/versioning'; -export { ELASTIC_HTTP_VERSION_HEADER, INTERNAL_ACCESS_REQUEST } from './src/constants'; +export { ELASTIC_HTTP_VERSION_HEADER, X_ELASTIC_INTERNAL_ORIGIN_REQUEST } from './src/constants'; diff --git a/packages/core/http/core-http-common/src/constants.ts b/packages/core/http/core-http-common/src/constants.ts index 65c5b007374d9..3227b38417523 100644 --- a/packages/core/http/core-http-common/src/constants.ts +++ b/packages/core/http/core-http-common/src/constants.ts @@ -9,4 +9,4 @@ /** @internal */ export const ELASTIC_HTTP_VERSION_HEADER = 'elastic-api-version' as const; -export const INTERNAL_ACCESS_REQUEST = 'x-elastic-internal-origin' as const; +export const X_ELASTIC_INTERNAL_ORIGIN_REQUEST = 'x-elastic-internal-origin' as const; diff --git a/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts b/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts index a97255790ed03..f22c075149058 100644 --- a/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts +++ b/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts @@ -9,7 +9,7 @@ import { Env } from '@kbn/config'; import type { OnPostAuthHandler, OnPreResponseHandler } from '@kbn/core-http-server'; import { isSafeMethod } from '@kbn/core-http-router-server-internal'; -import { INTERNAL_ACCESS_REQUEST } from '@kbn/core-http-common/src/constants'; +import { X_ELASTIC_INTERNAL_ORIGIN_REQUEST } from '@kbn/core-http-common/src/constants'; import { HttpConfig } from './http_config'; import { LifecycleRegistrar } from './http_server'; @@ -49,7 +49,7 @@ export const createRestrictInternalRoutesPostAuthHandler = ( const isInternalRoute = request.route.options.access === 'internal'; // only check if the header is present, not it's content. - const hasInternalKibanaRequestHeader = INTERNAL_ACCESS_REQUEST in request.headers; + const hasInternalKibanaRequestHeader = X_ELASTIC_INTERNAL_ORIGIN_REQUEST in request.headers; if (isRestrictionEnabled && isInternalRoute && !hasInternalKibanaRequestHeader) { // throw 400 diff --git a/src/plugins/bfetch/public/plugin.ts b/src/plugins/bfetch/public/plugin.ts index 05b4e2123eaa0..2168c023706eb 100644 --- a/src/plugins/bfetch/public/plugin.ts +++ b/src/plugins/bfetch/public/plugin.ts @@ -8,7 +8,7 @@ import { CoreSetup, CoreStart, Plugin, PluginInitializerContext } from '@kbn/core/public'; import { createStartServicesGetter } from '@kbn/kibana-utils-plugin/public'; -import { INTERNAL_ACCESS_REQUEST } from '@kbn/core-http-common'; +import { X_ELASTIC_INTERNAL_ORIGIN_REQUEST } from '@kbn/core-http-common'; import { fetchStreaming as fetchStreamingStatic, FetchStreamingParams } from './streaming'; import { DISABLE_BFETCH_COMPRESSION, removeLeadingSlash } from '../common'; import { createStreamingBatchedFunction, StreamingBatchedFunctionParams } from './batching'; @@ -84,7 +84,7 @@ export class BfetchPublicPlugin headers: { 'Content-Type': 'application/json', 'kbn-version': version, - [INTERNAL_ACCESS_REQUEST]: 'Kibana', + [X_ELASTIC_INTERNAL_ORIGIN_REQUEST]: 'Kibana', ...(params.headers || {}), }, getIsCompressionDisabled, diff --git a/x-pack/dev-tools/api_debug/request_from_api.js b/x-pack/dev-tools/api_debug/request_from_api.js index e11558f973d82..818812d5996a4 100644 --- a/x-pack/dev-tools/api_debug/request_from_api.js +++ b/x-pack/dev-tools/api_debug/request_from_api.js @@ -9,7 +9,7 @@ import fetch from 'node-fetch'; import { resolve } from 'path'; import abab from 'abab'; import pkg from '../../package.json'; -import { INTERNAL_ACCESS_REQUEST } from '@kbn/core-http-common/src/constants'; +import { X_ELASTIC_INTERNAL_ORIGIN_REQUEST } from '@kbn/core-http-common/src/constants'; function getRequestParams(argv) { // use `--host=https://somedomain.com:5601` or else http://localhost:5601 is defaulted @@ -33,7 +33,7 @@ function getRequestHeaders(auth) { 'kbn-version': pkg.version, 'Content-Type': 'application/json', Authorization: auth, - [INTERNAL_ACCESS_REQUEST]: 'kibana', + [X_ELASTIC_INTERNAL_ORIGIN_REQUEST]: 'kibana', }; } diff --git a/x-pack/plugins/security/server/authentication/can_redirect_request.test.ts b/x-pack/plugins/security/server/authentication/can_redirect_request.test.ts index f3e82d45d2e21..d2908abe188fb 100644 --- a/x-pack/plugins/security/server/authentication/can_redirect_request.test.ts +++ b/x-pack/plugins/security/server/authentication/can_redirect_request.test.ts @@ -26,6 +26,14 @@ describe('can_redirect_request', () => { expect(canRedirectRequest(request)).toBe(false); }); + it('returns false if request has a x-elastic-internal-origin header', () => { + const request = httpServerMock.createKibanaRequest({ + headers: { 'x-elastic-internal-origin': 'something' }, + }); + + expect(canRedirectRequest(request)).toBe(false); + }); + it('returns false for api routes', () => { expect( canRedirectRequest(httpServerMock.createKibanaRequest({ path: '/api/security/some' })) diff --git a/x-pack/plugins/security/server/authentication/can_redirect_request.ts b/x-pack/plugins/security/server/authentication/can_redirect_request.ts index 37e6d6b7061f7..aa9cfcd8fd0be 100644 --- a/x-pack/plugins/security/server/authentication/can_redirect_request.ts +++ b/x-pack/plugins/security/server/authentication/can_redirect_request.ts @@ -11,6 +11,7 @@ import { ROUTE_TAG_API, ROUTE_TAG_CAN_REDIRECT } from '../routes/tags'; const KIBANA_XSRF_HEADER = 'kbn-xsrf'; const KIBANA_VERSION_HEADER = 'kbn-version'; +const X_ELASTIC_INTERNAL_ORIGIN_REQUEST = 'x-elastic-internal-origin'; /** * Checks whether we can reply to the request with redirect response. We can do that @@ -22,12 +23,13 @@ export function canRedirectRequest(request: KibanaRequest) { const route = request.route; const hasVersionHeader = headers.hasOwnProperty(KIBANA_VERSION_HEADER); const hasXsrfHeader = headers.hasOwnProperty(KIBANA_XSRF_HEADER); + const hasIternalOriginHeader = headers.hasOwnProperty(X_ELASTIC_INTERNAL_ORIGIN_REQUEST); 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; return !isAjaxRequest && (!isApiRoute || route.options.tags.includes(ROUTE_TAG_CAN_REDIRECT)); } diff --git a/x-pack/test/common/services/bsearch_secure.ts b/x-pack/test/common/services/bsearch_secure.ts index 94a5abe73c901..b7bff074afc07 100644 --- a/x-pack/test/common/services/bsearch_secure.ts +++ b/x-pack/test/common/services/bsearch_secure.ts @@ -33,6 +33,7 @@ interface SendOptions { options: object; strategy: string; space?: string; + internalOrigin: string; } export class BsearchSecureService extends FtrService { @@ -43,6 +44,7 @@ export class BsearchSecureService extends FtrService { auth, referer, kibanaVersion, + internalOrigin, options, strategy, space, @@ -74,6 +76,13 @@ export class BsearchSecureService extends FtrService { .set('kbn-version', kibanaVersion) .set('kbn-xsrf', 'true') .send(options); + } else if (internalOrigin) { + result = await supertestWithoutAuth + .post(url) + .auth(auth.username, auth.password) + .set('x-elastic-internal-origin', internalOrigin) + .set('kbn-xsrf', 'true') + .send(options); } else { result = await supertestWithoutAuth .post(url) @@ -96,6 +105,7 @@ export class BsearchSecureService extends FtrService { .post(`${spaceUrl}/internal/bsearch`) .auth(auth.username, auth.password) .set('kbn-xsrf', 'true') + .set('x-elastic-internal-origin', 'Kibana') .send({ batch: [ { From 0a0cb44892b5b5fbc4d61c9deb672d3d672d3a30 Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Fri, 5 May 2023 13:50:13 -0700 Subject: [PATCH 09/24] Adds header to x-pack/dev_tools --- x-pack/dev-tools/api_debug/request_from_api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/dev-tools/api_debug/request_from_api.js b/x-pack/dev-tools/api_debug/request_from_api.js index 818812d5996a4..1a50232536377 100644 --- a/x-pack/dev-tools/api_debug/request_from_api.js +++ b/x-pack/dev-tools/api_debug/request_from_api.js @@ -33,7 +33,7 @@ function getRequestHeaders(auth) { 'kbn-version': pkg.version, 'Content-Type': 'application/json', Authorization: auth, - [X_ELASTIC_INTERNAL_ORIGIN_REQUEST]: 'kibana', + [X_ELASTIC_INTERNAL_ORIGIN_REQUEST]: 'Kibana', }; } From e823131041c6305d25947b8f253ae7a8f5c7a38c Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Fri, 5 May 2023 23:47:13 +0000 Subject: [PATCH 10/24] [CI] Auto-commit changed files from 'node scripts/lint_ts_projects --fix' --- src/plugins/bfetch/tsconfig.json | 1 + 1 file changed, 1 insertion(+) diff --git a/src/plugins/bfetch/tsconfig.json b/src/plugins/bfetch/tsconfig.json index dfc8dcab1c1ac..13136e83d88e2 100644 --- a/src/plugins/bfetch/tsconfig.json +++ b/src/plugins/bfetch/tsconfig.json @@ -11,6 +11,7 @@ "@kbn/config-schema", "@kbn/std", "@kbn/core-http-server", + "@kbn/core-http-common", ], "exclude": [ "target/**/*", From 607021322a4df2b6fccd65e54c70072d97d44361 Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Fri, 5 May 2023 17:16:58 -0700 Subject: [PATCH 11/24] Fix types --- packages/kbn-server-http-tools/src/get_server_options.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/kbn-server-http-tools/src/get_server_options.test.ts b/packages/kbn-server-http-tools/src/get_server_options.test.ts index 4af9b34dfc5f9..3fe164d0b8ada 100644 --- a/packages/kbn-server-http-tools/src/get_server_options.test.ts +++ b/packages/kbn-server-http-tools/src/get_server_options.test.ts @@ -38,6 +38,7 @@ const createConfig = (parts: Partial): IHttpConfig => ({ enabled: false, ...parts.ssl, }, + restrictInternalApis: false, }); describe('getServerOptions', () => { From 7d6038a4a5fe5f7c4e47ae91b5aaf06cbaaac621 Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Fri, 5 May 2023 18:32:36 -0700 Subject: [PATCH 12/24] Type fixes --- .../src/config/http_config.ts | 3 +++ .../base_path_proxy_server.test.ts | 1 + .../src/server/server_config.test.ts | 21 +++++++++++++++++++ .../src/server/server_config.ts | 3 +++ 4 files changed, 28 insertions(+) diff --git a/packages/kbn-cli-dev-mode/src/config/http_config.ts b/packages/kbn-cli-dev-mode/src/config/http_config.ts index f39bf673f597e..7f7a83defb8c3 100644 --- a/packages/kbn-cli-dev-mode/src/config/http_config.ts +++ b/packages/kbn-cli-dev-mode/src/config/http_config.ts @@ -38,6 +38,7 @@ export const httpConfigSchema = schema.object( }), }), ssl: sslSchema, + restrictInternalApis: schema.boolean({ defaultValue: false }), }, { unknowns: 'ignore' } ); @@ -54,6 +55,7 @@ export class HttpConfig implements IHttpConfig { socketTimeout: number; cors: ICorsConfig; ssl: ISslConfig; + restrictInternalApis: boolean; constructor(rawConfig: HttpConfigType) { this.basePath = rawConfig.basePath; @@ -65,5 +67,6 @@ export class HttpConfig implements IHttpConfig { this.socketTimeout = rawConfig.socketTimeout; this.cors = rawConfig.cors; this.ssl = new SslConfig(rawConfig.ssl); + this.restrictInternalApis = rawConfig.restrictInternalApis; } } diff --git a/packages/kbn-cli-dev-mode/src/integration_tests/base_path_proxy_server.test.ts b/packages/kbn-cli-dev-mode/src/integration_tests/base_path_proxy_server.test.ts index 1c6842f3c00a6..bb85786d97cc7 100644 --- a/packages/kbn-cli-dev-mode/src/integration_tests/base_path_proxy_server.test.ts +++ b/packages/kbn-cli-dev-mode/src/integration_tests/base_path_proxy_server.test.ts @@ -46,6 +46,7 @@ describe('BasePathProxyServer', () => { }, ssl: { enabled: false }, maxPayload: new ByteSizeValue(1024), + restrictInternalApis: false, }; const serverOptions = getServerOptions(config); diff --git a/packages/kbn-health-gateway-server/src/server/server_config.test.ts b/packages/kbn-health-gateway-server/src/server/server_config.test.ts index 66f82c0f1502b..99ba18659a725 100644 --- a/packages/kbn-health-gateway-server/src/server/server_config.test.ts +++ b/packages/kbn-health-gateway-server/src/server/server_config.test.ts @@ -58,6 +58,7 @@ describe('server config', () => { "TLSv1.3", ], "truststore": Object {}, + "restrictInternalApis": false, }, } `); @@ -188,4 +189,24 @@ describe('server config', () => { `); }); }); + + describe('restrictInternalApis', () => { + test('can specify retriction on access to internal APIs', () => { + const validOptions = [false, true]; + for (const val of validOptions) { + const { restrictInternalAPis } = config.schema.validate({ restrictInternalAPis: val }); + expect(restrictInternalAPis).toBe(val); + } + }); + + test('throws if not boolean', () => { + const configSchema = config.schema; + expect(() => configSchema.validate({ restricInternalApis: 100 })).toThrowError( + 'restrictInternalApis]: expected value of type [boolean] but got [number]' + ); + expect(() => configSchema.validate({ restricInternalApis: 'false' })).toThrowError( + 'restrictInternalApis]: expected value of type [boolean] but got [string]' + ); + }); + }); }); diff --git a/packages/kbn-health-gateway-server/src/server/server_config.ts b/packages/kbn-health-gateway-server/src/server/server_config.ts index 79c4f760c4408..57bc523a7610e 100644 --- a/packages/kbn-health-gateway-server/src/server/server_config.ts +++ b/packages/kbn-health-gateway-server/src/server/server_config.ts @@ -40,6 +40,7 @@ const configSchema = schema.object( defaultValue: 120000, }), ssl: sslSchema, + restrictInternalAPis: schema.boolean({ defaultValue: false }), }, { validate: (rawConfig) => { @@ -74,6 +75,7 @@ export class ServerConfig implements IHttpConfig { socketTimeout: number; ssl: ISslConfig; cors: ICorsConfig; + restrictInternalApis: boolean; constructor(rawConfig: ServerConfigType) { this.host = rawConfig.host; @@ -88,5 +90,6 @@ export class ServerConfig implements IHttpConfig { allowCredentials: false, allowOrigin: ['*'], }; + this.restrictInternalApis = rawConfig.restrictInternalAPis; } } From 59583ba729bb9e4413e5cc0b51f5ad21d8121e86 Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Fri, 5 May 2023 18:36:16 -0700 Subject: [PATCH 13/24] more type fixes --- .../security_and_spaces/tests/basic/search_strategy.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/x-pack/test/rule_registry/security_and_spaces/tests/basic/search_strategy.ts b/x-pack/test/rule_registry/security_and_spaces/tests/basic/search_strategy.ts index 3698839861031..8979119ac7ac7 100644 --- a/x-pack/test/rule_registry/security_and_spaces/tests/basic/search_strategy.ts +++ b/x-pack/test/rule_registry/security_and_spaces/tests/basic/search_strategy.ts @@ -66,6 +66,7 @@ export default ({ getService }: FtrProviderContext) => { }, referer: 'test', kibanaVersion, + internalOrigin: 'Kibana', options: { featureIds: [AlertConsumers.LOGS], }, @@ -87,6 +88,7 @@ export default ({ getService }: FtrProviderContext) => { }, referer: 'test', kibanaVersion, + internalOrigin: 'Kibana', options: { featureIds: [AlertConsumers.LOGS], pagination: { @@ -163,6 +165,7 @@ export default ({ getService }: FtrProviderContext) => { }, referer: 'test', kibanaVersion, + internalOrigin: 'Kibana', options: { featureIds: [AlertConsumers.SIEM, AlertConsumers.LOGS], }, @@ -192,6 +195,7 @@ export default ({ getService }: FtrProviderContext) => { }, referer: 'test', kibanaVersion, + internalOrigin: 'Kibana', options: { featureIds: [AlertConsumers.APM], }, @@ -216,6 +220,7 @@ export default ({ getService }: FtrProviderContext) => { }, referer: 'test', kibanaVersion, + internalOrigin: 'Kibana', options: { featureIds: [], }, From 78a69eaacd26e96b6533686e22f443095cccf2dd Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Sat, 6 May 2023 13:16:55 -0700 Subject: [PATCH 14/24] fix types and tests 1 --- .../src/__snapshots__/ui_settings_api.test.ts.snap | 14 ++++++++++++++ .../src/server/server_config.test.ts | 8 ++++---- .../src/server/server_config.ts | 4 ++-- .../tests/basic/search_strategy.ts | 1 + 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/packages/core/ui-settings/core-ui-settings-browser-internal/src/__snapshots__/ui_settings_api.test.ts.snap b/packages/core/ui-settings/core-ui-settings-browser-internal/src/__snapshots__/ui_settings_api.test.ts.snap index 6599746d33bf1..abee12da71e6b 100644 --- a/packages/core/ui-settings/core-ui-settings-browser-internal/src/__snapshots__/ui_settings_api.test.ts.snap +++ b/packages/core/ui-settings/core-ui-settings-browser-internal/src/__snapshots__/ui_settings_api.test.ts.snap @@ -9,6 +9,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "kbn-version": "kibanaVersion", + "x-elastic-internal-origin": "Kibana", }, "method": "POST", }, @@ -20,6 +21,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "kbn-version": "kibanaVersion", + "x-elastic-internal-origin": "Kibana", }, "method": "POST", }, @@ -36,6 +38,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "kbn-version": "kibanaVersion", + "x-elastic-internal-origin": "Kibana", }, "method": "POST", }, @@ -47,6 +50,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "kbn-version": "kibanaVersion", + "x-elastic-internal-origin": "Kibana", }, "method": "POST", }, @@ -63,6 +67,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "kbn-version": "kibanaVersion", + "x-elastic-internal-origin": "Kibana", }, "method": "POST", }, @@ -74,6 +79,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "kbn-version": "kibanaVersion", + "x-elastic-internal-origin": "Kibana", }, "method": "POST", }, @@ -113,6 +119,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "kbn-version": "kibanaVersion", + "x-elastic-internal-origin": "Kibana", }, "method": "POST", }, @@ -129,6 +136,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "kbn-version": "kibanaVersion", + "x-elastic-internal-origin": "Kibana", }, "method": "POST", }, @@ -140,6 +148,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "kbn-version": "kibanaVersion", + "x-elastic-internal-origin": "Kibana", }, "method": "POST", }, @@ -156,6 +165,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "kbn-version": "kibanaVersion", + "x-elastic-internal-origin": "Kibana", }, "method": "POST", }, @@ -167,6 +177,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "kbn-version": "kibanaVersion", + "x-elastic-internal-origin": "Kibana", }, "method": "POST", }, @@ -183,6 +194,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "kbn-version": "kibanaVersion", + "x-elastic-internal-origin": "Kibana", }, "method": "POST", }, @@ -194,6 +206,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "kbn-version": "kibanaVersion", + "x-elastic-internal-origin": "Kibana", }, "method": "POST", }, @@ -233,6 +246,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "kbn-version": "kibanaVersion", + "x-elastic-internal-origin": "Kibana", }, "method": "POST", }, diff --git a/packages/kbn-health-gateway-server/src/server/server_config.test.ts b/packages/kbn-health-gateway-server/src/server/server_config.test.ts index 99ba18659a725..0e74d41b238f8 100644 --- a/packages/kbn-health-gateway-server/src/server/server_config.test.ts +++ b/packages/kbn-health-gateway-server/src/server/server_config.test.ts @@ -194,17 +194,17 @@ describe('server config', () => { test('can specify retriction on access to internal APIs', () => { const validOptions = [false, true]; for (const val of validOptions) { - const { restrictInternalAPis } = config.schema.validate({ restrictInternalAPis: val }); - expect(restrictInternalAPis).toBe(val); + const { restrictInternalApis } = config.schema.validate({ restrictInternalAPis: val }); + expect(restrictInternalApis).toBe(val); } }); test('throws if not boolean', () => { const configSchema = config.schema; - expect(() => configSchema.validate({ restricInternalApis: 100 })).toThrowError( + expect(() => configSchema.validate({ restrictInternalApis: 100 })).toThrowError( 'restrictInternalApis]: expected value of type [boolean] but got [number]' ); - expect(() => configSchema.validate({ restricInternalApis: 'false' })).toThrowError( + expect(() => configSchema.validate({ restrictInternalApis: 'false' })).toThrowError( 'restrictInternalApis]: expected value of type [boolean] but got [string]' ); }); diff --git a/packages/kbn-health-gateway-server/src/server/server_config.ts b/packages/kbn-health-gateway-server/src/server/server_config.ts index 57bc523a7610e..3f89f422d6f6e 100644 --- a/packages/kbn-health-gateway-server/src/server/server_config.ts +++ b/packages/kbn-health-gateway-server/src/server/server_config.ts @@ -40,7 +40,7 @@ const configSchema = schema.object( defaultValue: 120000, }), ssl: sslSchema, - restrictInternalAPis: schema.boolean({ defaultValue: false }), + restrictInternalApis: schema.boolean({ defaultValue: false }), }, { validate: (rawConfig) => { @@ -90,6 +90,6 @@ export class ServerConfig implements IHttpConfig { allowCredentials: false, allowOrigin: ['*'], }; - this.restrictInternalApis = rawConfig.restrictInternalAPis; + this.restrictInternalApis = rawConfig.restrictInternalApis; } } diff --git a/x-pack/test/rule_registry/security_and_spaces/tests/basic/search_strategy.ts b/x-pack/test/rule_registry/security_and_spaces/tests/basic/search_strategy.ts index 8979119ac7ac7..0dc357419c5ba 100644 --- a/x-pack/test/rule_registry/security_and_spaces/tests/basic/search_strategy.ts +++ b/x-pack/test/rule_registry/security_and_spaces/tests/basic/search_strategy.ts @@ -144,6 +144,7 @@ export default ({ getService }: FtrProviderContext) => { }, referer: 'test', kibanaVersion, + internalOrigin: 'Kibana', options: { featureIds: [AlertConsumers.SIEM], }, From 8a5fedd10c36b94c5edc3172cd65e53103cc620e Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Sat, 6 May 2023 15:28:21 -0700 Subject: [PATCH 15/24] fix core usage stats client unit tests --- .../http/core-http-router-server-mocks/src/router.mock.ts | 2 +- .../src/core_usage_stats_client.test.ts | 6 +++++- .../src/core_usage_stats_client.ts | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/core/http/core-http-router-server-mocks/src/router.mock.ts b/packages/core/http/core-http-router-server-mocks/src/router.mock.ts index 2d39621d57c31..0d9882e8a8f36 100644 --- a/packages/core/http/core-http-router-server-mocks/src/router.mock.ts +++ b/packages/core/http/core-http-router-server-mocks/src/router.mock.ts @@ -64,7 +64,7 @@ export interface RequestFixtureOptions

{ function createKibanaRequestMock

({ path = '/path', - headers = { accept: 'something/html' }, + headers = { accept: 'something/html', 'x-elastic-internal-origin': 'someOrigin' }, params = {}, body = {}, query = {}, diff --git a/packages/core/usage-data/core-usage-data-server-internal/src/core_usage_stats_client.test.ts b/packages/core/usage-data/core-usage-data-server-internal/src/core_usage_stats_client.test.ts index a9b532148e5fe..86d1e1bbd8712 100644 --- a/packages/core/usage-data/core-usage-data-server-internal/src/core_usage_stats_client.test.ts +++ b/packages/core/usage-data/core-usage-data-server-internal/src/core_usage_stats_client.test.ts @@ -51,7 +51,11 @@ describe('CoreUsageStatsClient', () => { ); return { usageStatsClient, debugLoggerMock, basePathMock, repositoryMock }; }; - const firstPartyRequestHeaders = { 'kbn-version': 'a', referer: 'b' }; // as long as these two header fields are truthy, this will be treated like a first-party request + const firstPartyRequestHeaders = { + 'kbn-version': 'a', + referer: 'b', + 'x-elastic-internal-origin': 'c', + }; // as long as these header fields are truthy, this will be treated like a first-party request const incrementOptions = { refresh: false }; describe('#getUsageStats', () => { diff --git a/packages/core/usage-data/core-usage-data-server-internal/src/core_usage_stats_client.ts b/packages/core/usage-data/core-usage-data-server-internal/src/core_usage_stats_client.ts index 6a4a48568d9dc..f66e51ed2bf77 100644 --- a/packages/core/usage-data/core-usage-data-server-internal/src/core_usage_stats_client.ts +++ b/packages/core/usage-data/core-usage-data-server-internal/src/core_usage_stats_client.ts @@ -245,7 +245,7 @@ function getFieldsForCounter(prefix: string) { } function getIsKibanaRequest({ 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. + // The presence of these 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 && headers['x-elastic-internal-origin'] From 12a46cb63b8b49d5b35f03a07d01c9e0a0997b3c Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Sat, 6 May 2023 16:23:41 -0700 Subject: [PATCH 16/24] Fix typo, comment out for documenting intent only --- config/serverless.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/serverless.yml b/config/serverless.yml index ff68b1ed5dd21..10d63533bc759 100644 --- a/config/serverless.yml +++ b/config/serverless.yml @@ -18,5 +18,5 @@ xpack.license_management.enabled: false #xpack.canvas.enabled: false #only disabable in dev-mode xpack.reporting.enabled: false -# Enforce restriction to internal APIs see https://github.com/elastic/kibana/issues/151940 -server.http.restrictInternalApis: true +# Enforce restring access to internal APIs see https://github.com/elastic/kibana/issues/151940 +# server.restrictInternalApis: true From 30c441f776643908eebd4c8c739c04fb195899f9 Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Sat, 6 May 2023 16:59:08 -0700 Subject: [PATCH 17/24] fix test --- .../src/server/server_config.test.ts | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/packages/kbn-health-gateway-server/src/server/server_config.test.ts b/packages/kbn-health-gateway-server/src/server/server_config.test.ts index 0e74d41b238f8..8f4d2fc6c8071 100644 --- a/packages/kbn-health-gateway-server/src/server/server_config.test.ts +++ b/packages/kbn-health-gateway-server/src/server/server_config.test.ts @@ -20,6 +20,7 @@ describe('server config', () => { "valueInBytes": 1048576, }, "port": 3000, + "restrictInternalApis": false, "shutdownTimeout": "PT30S", "socketTimeout": 120000, "ssl": Object { @@ -58,7 +59,6 @@ describe('server config', () => { "TLSv1.3", ], "truststore": Object {}, - "restrictInternalApis": false, }, } `); @@ -191,12 +191,22 @@ describe('server config', () => { }); describe('restrictInternalApis', () => { + test('is false by default', () => { + const configSchema = config.schema; + const obj = {}; + expect(new ServerConfig(configSchema.validate(obj)).restrictInternalApis).toBe(false); + }); + test('can specify retriction on access to internal APIs', () => { - const validOptions = [false, true]; - for (const val of validOptions) { - const { restrictInternalApis } = config.schema.validate({ restrictInternalAPis: val }); - expect(restrictInternalApis).toBe(val); - } + const configSchema = config.schema; + expect( + new ServerConfig(configSchema.validate({ restrictInternalApis: true })).restrictInternalApis + ).toBe(true); + + expect( + new ServerConfig(configSchema.validate({ restrictInternalApis: false })) + .restrictInternalApis + ).toBe(false); }); test('throws if not boolean', () => { @@ -204,7 +214,7 @@ describe('server config', () => { expect(() => configSchema.validate({ restrictInternalApis: 100 })).toThrowError( 'restrictInternalApis]: expected value of type [boolean] but got [number]' ); - expect(() => configSchema.validate({ restrictInternalApis: 'false' })).toThrowError( + expect(() => configSchema.validate({ restrictInternalApis: 'something' })).toThrowError( 'restrictInternalApis]: expected value of type [boolean] but got [string]' ); }); From 5324c2075d0f53d06f6ba8b4c38865842ff2647f Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Sat, 6 May 2023 17:36:30 -0700 Subject: [PATCH 18/24] Adds config setting to docker list --- .../docker_generator/resources/base/bin/kibana-docker | 1 + 1 file changed, 1 insertion(+) diff --git a/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker b/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker index 34ad6e5acda32..14ebf681c0fec 100755 --- a/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker +++ b/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker @@ -161,6 +161,7 @@ kibana_vars=( server.requestId.allowFromAnyIp server.requestId.ipAllowlist server.rewriteBasePath + server.restrictInternalApis server.securityResponseHeaders.disableEmbedding server.securityResponseHeaders.permissionsPolicy server.securityResponseHeaders.referrerPolicy From 3cae610c70da1613b6a114c666a9e1c75cf248f2 Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Sat, 6 May 2023 18:48:24 -0700 Subject: [PATCH 19/24] removes header from core createKibanaRequest RouterMock --- .../core/http/core-http-router-server-mocks/src/router.mock.ts | 2 +- x-pack/plugins/security/server/authentication/authenticator.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/core/http/core-http-router-server-mocks/src/router.mock.ts b/packages/core/http/core-http-router-server-mocks/src/router.mock.ts index 0d9882e8a8f36..2d39621d57c31 100644 --- a/packages/core/http/core-http-router-server-mocks/src/router.mock.ts +++ b/packages/core/http/core-http-router-server-mocks/src/router.mock.ts @@ -64,7 +64,7 @@ export interface RequestFixtureOptions

{ function createKibanaRequestMock

({ path = '/path', - headers = { accept: 'something/html', 'x-elastic-internal-origin': 'someOrigin' }, + headers = { accept: 'something/html' }, params = {}, body = {}, query = {}, diff --git a/x-pack/plugins/security/server/authentication/authenticator.ts b/x-pack/plugins/security/server/authentication/authenticator.ts index 8599eb287989e..67482aff48551 100644 --- a/x-pack/plugins/security/server/authentication/authenticator.ts +++ b/x-pack/plugins/security/server/authentication/authenticator.ts @@ -138,6 +138,7 @@ const ACCESS_AGREEMENT_ROUTE = '/security/access_agreement'; const OVERWRITTEN_SESSION_ROUTE = '/security/overwritten_session'; function assertRequest(request: KibanaRequest) { + // console.log('request:', request); if (!(request instanceof CoreKibanaRequest)) { throw new Error(`Request should be a valid "KibanaRequest" instance, was [${typeof request}].`); } From 78c0fc05648ea7b7da2d4b6b457c196b12818796 Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Sat, 6 May 2023 18:49:38 -0700 Subject: [PATCH 20/24] deletes debug console --- x-pack/plugins/security/server/authentication/authenticator.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugins/security/server/authentication/authenticator.ts b/x-pack/plugins/security/server/authentication/authenticator.ts index 67482aff48551..8599eb287989e 100644 --- a/x-pack/plugins/security/server/authentication/authenticator.ts +++ b/x-pack/plugins/security/server/authentication/authenticator.ts @@ -138,7 +138,6 @@ const ACCESS_AGREEMENT_ROUTE = '/security/access_agreement'; const OVERWRITTEN_SESSION_ROUTE = '/security/overwritten_session'; function assertRequest(request: KibanaRequest) { - // console.log('request:', request); if (!(request instanceof CoreKibanaRequest)) { throw new Error(`Request should be a valid "KibanaRequest" instance, was [${typeof request}].`); } From 01e4f1e9075b73f8d010d2ac4012f06ec2da4d19 Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Mon, 8 May 2023 16:11:55 -0700 Subject: [PATCH 21/24] updates codeowners file --- .github/CODEOWNERS | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index fa2a580b4ae83..3388c8f87b798 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -922,6 +922,7 @@ x-pack/test/observability_functional @elastic/actionable-observability /WORKSPACE.bazel @elastic/kibana-operations /.buildkite/ @elastic/kibana-operations /kbn_pm/ @elastic/kibana-operations +/x-pack/dev-tools @elastic/kibana-operations # Appex QA /src/dev/code_coverage @elastic/appex-qa From cb868135407d93e360a5045a4fbedec13fe12eec Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Mon, 8 May 2023 17:32:17 -0700 Subject: [PATCH 22/24] update types --- .../security_and_spaces/tests/basic/search_strategy.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/test/rule_registry/security_and_spaces/tests/basic/search_strategy.ts b/x-pack/test/rule_registry/security_and_spaces/tests/basic/search_strategy.ts index 874664bed2cb8..c663d028cd2ac 100644 --- a/x-pack/test/rule_registry/security_and_spaces/tests/basic/search_strategy.ts +++ b/x-pack/test/rule_registry/security_and_spaces/tests/basic/search_strategy.ts @@ -189,6 +189,7 @@ export default ({ getService }: FtrProviderContext) => { }, referer: 'test', kibanaVersion, + internalOrigin: 'Kibana', options: { featureIds: [AlertConsumers.SIEM], runtimeMappings: { From e321d1b52731363d09f1804ca8888b33a0db9621 Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Tue, 9 May 2023 09:17:19 -0700 Subject: [PATCH 23/24] tests happy path for retriction enforced on internal routes --- .../integration_tests/http/lifecycle_handlers.test.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/core/server/integration_tests/http/lifecycle_handlers.test.ts b/src/core/server/integration_tests/http/lifecycle_handlers.test.ts index 926b8216d19c2..2ba0aa0e6f8e4 100644 --- a/src/core/server/integration_tests/http/lifecycle_handlers.test.ts +++ b/src/core/server/integration_tests/http/lifecycle_handlers.test.ts @@ -382,5 +382,12 @@ describe('core lifecyle handers with restrict internal routes enforced', () => { const result = await supertest(innerServer.listener).get(testInternalRoute).expect(400); expect(result.body.error).toBe('Bad Request'); }); + + it('accepts requests with the internal product header to internal routes', async () => { + await supertest(innerServer.listener) + .get(testInternalRoute) + .set(internalProductHeader, 'anything') + .expect(200, 'ok()'); + }); }); }); From f38d8bd1bdc61de52b427d4731c9d5e4115b5b4d Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Tue, 9 May 2023 09:45:29 -0700 Subject: [PATCH 24/24] Removes internal product origin header from canRedirectRequest --- .../server/authentication/can_redirect_request.test.ts | 6 +++--- .../security/server/authentication/can_redirect_request.ts | 4 +--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/security/server/authentication/can_redirect_request.test.ts b/x-pack/plugins/security/server/authentication/can_redirect_request.test.ts index d2908abe188fb..47a09297ff866 100644 --- a/x-pack/plugins/security/server/authentication/can_redirect_request.test.ts +++ b/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', () => { expect(canRedirectRequest(httpServerMock.createKibanaRequest())).toBe(true); }); @@ -26,12 +26,12 @@ describe('can_redirect_request', () => { expect(canRedirectRequest(request)).toBe(false); }); - it('returns false if request has a x-elastic-internal-origin header', () => { + it('returns true if request has a x-elastic-internal-origin header', () => { const request = httpServerMock.createKibanaRequest({ headers: { 'x-elastic-internal-origin': 'something' }, }); - expect(canRedirectRequest(request)).toBe(false); + expect(canRedirectRequest(request)).toBe(true); }); it('returns false for api routes', () => { diff --git a/x-pack/plugins/security/server/authentication/can_redirect_request.ts b/x-pack/plugins/security/server/authentication/can_redirect_request.ts index aa9cfcd8fd0be..37e6d6b7061f7 100644 --- a/x-pack/plugins/security/server/authentication/can_redirect_request.ts +++ b/x-pack/plugins/security/server/authentication/can_redirect_request.ts @@ -11,7 +11,6 @@ import { ROUTE_TAG_API, ROUTE_TAG_CAN_REDIRECT } from '../routes/tags'; const KIBANA_XSRF_HEADER = 'kbn-xsrf'; const KIBANA_VERSION_HEADER = 'kbn-version'; -const X_ELASTIC_INTERNAL_ORIGIN_REQUEST = 'x-elastic-internal-origin'; /** * Checks whether we can reply to the request with redirect response. We can do that @@ -23,13 +22,12 @@ export function canRedirectRequest(request: KibanaRequest) { const route = request.route; const hasVersionHeader = headers.hasOwnProperty(KIBANA_VERSION_HEADER); const hasXsrfHeader = headers.hasOwnProperty(KIBANA_XSRF_HEADER); - const hasIternalOriginHeader = headers.hasOwnProperty(X_ELASTIC_INTERNAL_ORIGIN_REQUEST); const isApiRoute = route.options.tags.includes(ROUTE_TAG_API) || route.path.startsWith('/api/') || route.path.startsWith('/internal/'); - const isAjaxRequest = hasVersionHeader || hasXsrfHeader || hasIternalOriginHeader; + const isAjaxRequest = hasVersionHeader || hasXsrfHeader; return !isAjaxRequest && (!isApiRoute || route.options.tags.includes(ROUTE_TAG_CAN_REDIRECT)); }