Skip to content

Commit

Permalink
Globally enforce internal API restriction (#193792)
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
TinaHeiligers and elasticmachine authored Oct 2, 2024
1 parent 1b05007 commit fe83c0d
Show file tree
Hide file tree
Showing 52 changed files with 281 additions and 71 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -539,13 +539,13 @@ describe('restrictInternalApis', () => {
restrictInternalApis: true,
});
});
it('defaults to false', () => {
it('defaults to true', () => {
expect(
config.schema.validate({ restrictInternalApis: undefined }, { serverless: true })
).toMatchObject({ restrictInternalApis: false });
).toMatchObject({ restrictInternalApis: true });
expect(
config.schema.validate({ restrictInternalApis: undefined }, { traditional: true })
).toMatchObject({ restrictInternalApis: false });
).toMatchObject({ restrictInternalApis: true });
});
});

Expand Down Expand Up @@ -677,7 +677,7 @@ describe('HttpConfig', () => {
});
});

it('defaults restrictInternalApis to false', () => {
it('defaults restrictInternalApis to true', () => {
const rawConfig = config.schema.validate({}, {});
const rawCspConfig = cspConfig.schema.validate({});
const rawPermissionsPolicyConfig = permissionsPolicyConfig.schema.validate({});
Expand All @@ -687,6 +687,6 @@ describe('HttpConfig', () => {
ExternalUrlConfig.DEFAULT,
rawPermissionsPolicyConfig
);
expect(httpConfig.restrictInternalApis).toBe(false);
expect(httpConfig.restrictInternalApis).toBe(true);
});
});
11 changes: 4 additions & 7 deletions packages/core/http/core-http-server-internal/src/http_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,11 +205,8 @@ const configSchema = schema.object(
},
}
),
// allow access to internal routes by default to prevent breaking changes in current offerings
restrictInternalApis: offeringBasedSchema({
serverless: schema.boolean({ defaultValue: false }),
traditional: schema.boolean({ defaultValue: false }),
}),
// disable access to internal routes by default
restrictInternalApis: schema.boolean({ defaultValue: true }),

versioned: schema.object({
/**
Expand Down Expand Up @@ -385,8 +382,8 @@ export class HttpConfig implements IHttpConfig {
this.requestId = rawHttpConfig.requestId;
this.shutdownTimeout = rawHttpConfig.shutdownTimeout;

// default to `false` to prevent breaking changes in current offerings
this.restrictInternalApis = rawHttpConfig.restrictInternalApis ?? false;
// defaults to `true` if not set through config.
this.restrictInternalApis = rawHttpConfig.restrictInternalApis;
this.eluMonitor = rawHttpConfig.eluMonitor;
this.versioned = rawHttpConfig.versioned;
this.oas = rawHttpConfig.oas;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export const createConfigService = ({
shutdownTimeout: moment.duration(30, 'seconds'),
keepaliveTimeout: 120_000,
socketTimeout: 120_000,
restrictInternalApis: false,
restrictInternalApis: false, // disable restriction for Kibana tests
versioned: {
versionResolution: 'oldest',
strictClientVersionCheck: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const DEFAULTS_SETTINGS = {
// port and aren't affected by the timing issues in test environment.
port: 0,
xsrf: { disableProtection: true },
restrictInternalApis: true,
},
logging: {
root: {
Expand Down Expand Up @@ -149,6 +150,7 @@ export function createRootWithCorePlugins(
console: { type: 'console', layout: { type: 'pattern' } },
},
},
server: { restrictInternalApis: true },
// createRootWithSettings sets default value to "true", so undefined should be threatened as "true".
...(cliArgs.oss === false
? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ const getServerlessESClient = ({ port }: { port: number }) => {
const getServerlessDefault = () => {
return {
server: {
restrictInternalApis: true,
restrictInternalApis: true, // has no effect, defaults to true
versioned: {
versionResolution: 'newest',
strictClientVersionCheck: false,
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-config/src/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export interface CliArgs {
cache: boolean;
dist: boolean;
serverless?: boolean;
retrictInternalApis?: boolean;
}

/** @internal */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe('server config', () => {
},
"payloadTimeout": 20000,
"port": 3000,
"restrictInternalApis": false,
"restrictInternalApis": true,
"shutdownTimeout": "PT30S",
"socketTimeout": 120000,
"ssl": Object {
Expand Down Expand Up @@ -193,10 +193,10 @@ describe('server config', () => {
});

describe('restrictInternalApis', () => {
test('is false by default', () => {
test('is true by default', () => {
const configSchema = config.schema;
const obj = {};
expect(new ServerConfig(configSchema.validate(obj)).restrictInternalApis).toBe(false);
expect(new ServerConfig(configSchema.validate(obj)).restrictInternalApis).toBe(true);
});

test('can specify retriction on access to internal APIs', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const configSchema = schema.object(
defaultValue: 120000,
}),
ssl: sslSchema,
restrictInternalApis: schema.boolean({ defaultValue: false }),
restrictInternalApis: schema.boolean({ defaultValue: true }),
},
{
validate: (rawConfig) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-server-http-tools/src/get_listener.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const createConfig = (parts: Partial<IHttpConfig>): IHttpConfig => ({
enabled: false,
...parts.ssl,
},
restrictInternalApis: false,
restrictInternalApis: true,
});

describe('getServerListener', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const createConfig = (parts: Partial<IHttpConfig>): IHttpConfig => ({
enabled: false,
...parts.ssl,
},
restrictInternalApis: false,
restrictInternalApis: true,
});

describe('getServerOptions', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-server-http-tools/src/get_tls_options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const createConfig = (parts: Partial<IHttpConfig>): IHttpConfig => ({
enabled: false,
...parts.ssl,
},
restrictInternalApis: false,
restrictInternalApis: true,
});

describe('getServerTLSOptions', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ describe('PUT /internal/core/_settings', () => {
logging: {
loggers: [{ name: loggerName, level: 'error', appenders: ['console'] }],
},
server: { restrictInternalApis: false },
};
const { startES, startKibana } = createTestServers({
adjustTimeout: (t: number) => jest.setTimeout(t),
Expand Down Expand Up @@ -82,6 +83,9 @@ describe('checking all opted-in dynamic config settings', () => {
logging: {
loggers: [{ name: 'root', level: 'info', appenders: ['console'] }],
},
server: {
restrictInternalApis: false,
},
};

set(settings, PLUGIN_SYSTEM_ENABLE_ALL_PLUGINS_CONFIG_PATH, true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ describe('default route provider', () => {
root = createRootWithCorePlugins({
server: {
basePath: '/hello',
restrictInternalApis: false,
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ describe('trace', () => {
requestId: {
allowFromAnyIp: true,
},
restrictInternalApis: false,
},
execution_context: {
enabled: true,
Expand Down Expand Up @@ -191,6 +192,7 @@ describe('trace', () => {
requestId: {
allowFromAnyIp: true,
},
restrictInternalApis: false,
},
});
await rootExecutionContextDisabled.preboot();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ const configService = createConfigService({
allowFromAnyIp: true,
ipAllowlist: [],
},
restrictInternalApis: false,
} as any,
});
const contextSetup = contextServiceMock.createSetupContract();
Expand Down
2 changes: 2 additions & 0 deletions src/core/server/integration_tests/http/core_services.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ describe('http service', () => {
root = createRoot({
plugins: { initialize: false },
elasticsearch: { skipStartupConnectionCheck: true },
server: { restrictInternalApis: false },
});
await root.preboot();
});
Expand Down Expand Up @@ -185,6 +186,7 @@ describe('http service', () => {
root = createRoot({
plugins: { initialize: false },
elasticsearch: { skipStartupConnectionCheck: true },
server: { restrictInternalApis: false },
});
await root.preboot();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ describe('Http2 - Smoke tests', () => {
redirectHttpFromPort: 10003,
},
shutdownTimeout: '5s',
restrictInternalApis: false,
});
config = new HttpConfig(rawConfig, CSP_CONFIG, EXTERNAL_URL_CONFIG, PERMISSIONS_POLICY_CONFIG);
server = new HttpServer(coreContext, 'tests', of(config.shutdownTimeout));
Expand Down
1 change: 1 addition & 0 deletions src/core/server/integration_tests/http/http_auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ describe('http auth', () => {
root = createRoot({
plugins: { initialize: false },
elasticsearch: { skipStartupConnectionCheck: true },
server: { restrictInternalApis: false },
});
await root.preboot();
});
Expand Down
1 change: 1 addition & 0 deletions src/core/server/integration_tests/http/http_server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ describe('Http server', () => {
enabled: false,
},
shutdownTimeout: moment.duration(5, 's'),
restrictInternalApis: false,
} as any;

server = new HttpServer(coreContext, 'tests', of(config.shutdownTimeout));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ const testConfig: Parameters<typeof createConfigService>[0] = {
'referrer-policy': 'strict-origin', // overrides a header that is defined by securityResponseHeaders
},
xsrf: { disableProtection: false, allowlist: [allowlistedTestPath] },
restrictInternalApis: false,
},
};

Expand Down
5 changes: 5 additions & 0 deletions src/core/server/integration_tests/http/logging.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ describe('request logging', () => {
const root = createRoot({
plugins: { initialize: false },
elasticsearch: { skipStartupConnectionCheck: true },
server: { restrictInternalApis: false },
});
await root.preboot();
const { http } = await root.setup();
Expand Down Expand Up @@ -74,6 +75,7 @@ describe('request logging', () => {
initialize: false,
},
elasticsearch: { skipStartupConnectionCheck: true },
server: { restrictInternalApis: false },
});
await root.preboot();
const { http } = await root.setup();
Expand Down Expand Up @@ -121,6 +123,7 @@ describe('request logging', () => {
initialize: false,
},
elasticsearch: { skipStartupConnectionCheck: true },
server: { restrictInternalApis: false },
};

beforeEach(() => {
Expand Down Expand Up @@ -332,6 +335,7 @@ describe('request logging', () => {
initialize: false,
},
elasticsearch: { skipStartupConnectionCheck: true },
server: { restrictInternalApis: false },
});
await root.preboot();
const { http } = await root.setup();
Expand Down Expand Up @@ -431,6 +435,7 @@ describe('request logging', () => {
initialize: false,
},
elasticsearch: { skipStartupConnectionCheck: true },
server: { restrictInternalApis: false },
});
await root.preboot();
const { http } = await root.setup();
Expand Down
6 changes: 4 additions & 2 deletions src/core/server/integration_tests/http/oas.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ it('is disabled by default', async () => {
});

it('handles requests when enabled', async () => {
const server = await startService({ config: { server: { oas: { enabled: true } } } });
const server = await startService({
config: { server: { oas: { enabled: true }, restrictInternalApis: false } },
});
const result = await supertest(server.listener).get('/api/oas');
expect(result.status).toBe(200);
});
Expand Down Expand Up @@ -157,7 +159,7 @@ it.each([
'can filter paths based on query params $queryParam',
async ({ queryParam, includes, excludes }) => {
const server = await startService({
config: { server: { oas: { enabled: true } } },
config: { server: { oas: { enabled: true }, restrictInternalApis: false } },
createRoutes: (getRouter) => {
const router1 = getRouter(Symbol('myPlugin'));
router1.get(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ function applyTestsWithDisableUnsafeEvalSetTo(disableUnsafeEval: boolean) {
csp: { disableUnsafeEval },
plugins: { initialize: false },
elasticsearch: { skipStartupConnectionCheck: true },
server: { restrictInternalApis: false },
});
await root.preboot();
});
Expand Down
1 change: 1 addition & 0 deletions src/core/server/integration_tests/logging/logging.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ function createRoot() {
},
],
},
server: { restrictInternalApis: false },
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ function createRoot(appenderConfig: any) {
},
],
},
server: { restrictInternalApis: false },
});
}

Expand Down
1 change: 1 addition & 0 deletions src/core/server/integration_tests/node/logging.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ function createRootWithRoles(roles: string[]) {
level: 'info',
},
},
server: { restrictInternalApis: false },
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ describe('POST /api/saved_objects/_bulk_create', () => {

const result = await supertest(httpSetup.server.listener)
.post('/api/saved_objects/_bulk_create')
.set('x-elastic-internal-origin', 'kibana')
.send([
{
id: 'abc123',
Expand Down Expand Up @@ -127,6 +128,7 @@ describe('POST /api/saved_objects/_bulk_create', () => {

await supertest(httpSetup.server.listener)
.post('/api/saved_objects/_bulk_create')
.set('x-elastic-internal-origin', 'kibana')
.send(docs)
.expect(200);

Expand Down Expand Up @@ -158,6 +160,7 @@ describe('POST /api/saved_objects/_bulk_create', () => {
it('returns with status 400 when a type is hidden from the HTTP APIs', async () => {
const result = await supertest(httpSetup.server.listener)
.post('/api/saved_objects/_bulk_create')
.set('x-elastic-internal-origin', 'kibana')
.send([
{
id: 'hiddenID',
Expand All @@ -177,6 +180,7 @@ describe('POST /api/saved_objects/_bulk_create', () => {
it('logs a warning message when called', async () => {
await supertest(httpSetup.server.listener)
.post('/api/saved_objects/_bulk_create')
.set('x-elastic-internal-origin', 'kibana')
.send([
{
id: 'abc1234',
Expand Down
Loading

0 comments on commit fe83c0d

Please sign in to comment.