Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add http2 support for Kibana server #183465

Merged
merged 41 commits into from
Jun 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
364ee52
add `protocol` to the http config
pgayvallet May 15, 2024
2205584
extract getServerTLSOptions
pgayvallet May 15, 2024
d893a06
refactor listener creation logic + add http2 listener support
pgayvallet May 15, 2024
a9f73a1
fix types
pgayvallet May 15, 2024
7ba90c9
add http2 related warnings to the list of ignored ones
pgayvallet May 15, 2024
8b7f775
add smoke tests to check supertest behavior
pgayvallet May 16, 2024
c250d41
Enabled ALPN negotiation on the http2 TLS listener
pgayvallet May 17, 2024
9c2f9aa
update test
pgayvallet May 17, 2024
3bc8265
Merge remote-tracking branch 'upstream/main' into kbn-7104-http2-reborn
pgayvallet May 23, 2024
1eb5d7f
post-merge fixes
pgayvallet May 23, 2024
0a06093
set right value for protocol on KibanaRequest
pgayvallet May 23, 2024
447548d
add get_listener tests for http2 listeners
pgayvallet May 23, 2024
f43ee8f
adapt bfetch to not use banned headers
pgayvallet May 23, 2024
ffbab26
fix test types
pgayvallet May 23, 2024
537a365
update snapshot
pgayvallet May 23, 2024
e7819fb
add http.protocol config validation
pgayvallet May 23, 2024
12b4a1c
strip http2 illegal headers when serving http2 requests
pgayvallet May 23, 2024
55c0e10
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine May 23, 2024
ec6e95c
Add documentation for `server.protocol`
pgayvallet May 23, 2024
84009a5
add `--http2` dev cli flag
pgayvallet May 24, 2024
1ae9eb2
Merge remote-tracking branch 'upstream/main' into kbn-7104-http2-reborn
pgayvallet May 24, 2024
41b7951
add http2 implementation for basepath proxy server
pgayvallet May 27, 2024
0799cba
right.
pgayvallet May 27, 2024
fd5b5a8
add test for http2 proxy
pgayvallet May 27, 2024
5be430e
Merge remote-tracking branch 'upstream/main' into kbn-7104-http2-reborn
pgayvallet May 27, 2024
943acfa
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine May 27, 2024
8bd0bc7
ignore cert errors for playwright
pgayvallet May 27, 2024
1d579cd
enable HTTP2 for functional oss FTR suites
pgayvallet May 27, 2024
2dcdc59
fix newsfeed config def
pgayvallet May 27, 2024
ca2fb97
fix xpack func config
pgayvallet May 27, 2024
da6d7d9
making progress with FTR tests
pgayvallet May 28, 2024
69f81f2
enable http2 for the home function test suite
pgayvallet May 28, 2024
9fa7428
enable http2 for the console function test suite
pgayvallet May 28, 2024
e61e45f
self-review
pgayvallet May 28, 2024
279b49c
remove some comments
pgayvallet May 28, 2024
c257870
remove added whiteline
pgayvallet May 28, 2024
ecebcbd
Add new settings to kibana-docker
pgayvallet May 30, 2024
d8ed82f
Merge remote-tracking branch 'upstream/main' into kbn-7104-http2-reborn
pgayvallet May 30, 2024
d374a63
include protocol in BPP ready message
pgayvallet May 30, 2024
37e54cf
review NIT
pgayvallet May 30, 2024
7092c3b
Merge remote-tracking branch 'upstream/main' into kbn-7104-http2-reborn
pgayvallet Jun 3, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/setup/settings.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,14 @@ identifies this {kib} instance. *Default: `"your-hostname"`*
{kib} is served by a back end server. This
setting specifies the port to use. *Default: `5601`*

`server.protocol`::
jbudz marked this conversation as resolved.
Show resolved Hide resolved
experimental[] The http protocol to use, either `http1` or `http2`. Set to `http2` to enable `HTTP/2` support for the {kib} server.
*Default: `http1`*
+
NOTE: By default, enabling `http2` requires a valid `h2c` configuration, meaning that TLS must be enabled via <<server-ssl-enabled, `server.ssl.enabled`>>
and <<server-ssl-supportedProtocols, `server.ssl.supportedProtocols`>>, if specified, must contain at least `TLSv1.2` or `TLSv1.3`. Strict validation of
the `h2c` setup can be disabled by adding `server.http2.allowUnsecure: true` to the configuration.

[[server-requestId-allowFromAnyIp]] `server.requestId.allowFromAnyIp`::
Sets whether or not the `X-Opaque-Id` header should be trusted from any IP address for identifying requests in logs and forwarded to Elasticsearch.

Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1627,6 +1627,8 @@
"html": "1.0.0",
"html-loader": "^1.3.2",
"http-proxy": "^1.18.1",
"http2-proxy": "^5.0.53",
"http2-wrapper": "^2.2.1",
Comment on lines +1630 to +1631
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dev deps - only used by the new http2 basepathproxy server in dev setup

"ignore": "^5.3.0",
"jest": "^29.6.1",
"jest-canvas-mock": "^2.5.2",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,18 +217,42 @@ describe('CoreKibanaRequest', () => {
});

describe('route.protocol property', () => {
it('return a static value for now as only http1 is supported', () => {
it('return the correct value for http/1.0 requests', () => {
const request = hapiMocks.createRequest({
raw: {
req: {
httpVersion: '2.0',
httpVersion: '1.0',
},
},
});
const kibanaRequest = CoreKibanaRequest.from(request);

expect(kibanaRequest.protocol).toEqual('http1');
});
it('return the correct value for http/1.1 requests', () => {
const request = hapiMocks.createRequest({
raw: {
req: {
httpVersion: '1.1',
},
},
});
const kibanaRequest = CoreKibanaRequest.from(request);

expect(kibanaRequest.protocol).toEqual('http1');
});
it('return the correct value for http/2 requests', () => {
const request = hapiMocks.createRequest({
raw: {
req: {
httpVersion: '2.0',
},
},
});
const kibanaRequest = CoreKibanaRequest.from(request);

expect(kibanaRequest.protocol).toEqual('http2');
});
});

describe('route.options.authRequired property', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,7 @@ export class CoreKibanaRequest<
});

this.httpVersion = isRealReq ? request.raw.req.httpVersion : '1.0';
// hardcoded for now as only supporting http1
this.protocol = 'http1';
this.protocol = getProtocolFromHttpVersion(this.httpVersion);

this.route = deepFreeze(this.getRouteInfo(request));
this.socket = isRealReq
Expand Down Expand Up @@ -374,3 +373,7 @@ function sanitizeRequest(req: Request): { query: unknown; params: unknown; body:
body: req.payload,
};
}

function getProtocolFromHttpVersion(httpVersion: string): HttpProtocol {
return httpVersion.split('.')[0] === '2' ? 'http2' : 'http1';
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import { HapiResponseAdapter } from './response_adapter';
import { wrapErrors } from './error_wrapper';
import { Method } from './versioned_router/types';
import { prepareRouteConfigValidation } from './util';
import { stripIllegalHttp2Headers } from './strip_illegal_http2_headers';

export type ContextEnhancer<
P,
Expand Down Expand Up @@ -265,6 +266,14 @@ export class Router<Context extends RequestHandlerContextBase = RequestHandlerCo

try {
const kibanaResponse = await handler(kibanaRequest, kibanaResponseFactory);
if (kibanaRequest.protocol === 'http2' && kibanaResponse.options.headers) {
kibanaResponse.options.headers = stripIllegalHttp2Headers({
headers: kibanaResponse.options.headers,
isDev: this.options.isDev ?? false,
logger: this.log,
requestContext: `${request.route.method} ${request.route.path}`,
});
}
Comment on lines +269 to +276
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Illegal http2 headers (see stripIllegalHttp2Headers for the list) are not only causing warnings, they can also just terminate the Kibana process.

That's why we're removing it in the handler wrapper, and we log a warning in development mode so that developers can be informed and remove it from their handler.

return hapiResponseAdapter.handle(kibanaResponse);
} catch (error) {
// capture error
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { stripIllegalHttp2Headers } from './strip_illegal_http2_headers';
import { loggerMock, MockedLogger } from '@kbn/logging-mocks';

describe('stripIllegalHttp2Headers', () => {
let logger: MockedLogger;

beforeEach(() => {
logger = loggerMock.create();
});

it('removes illegal http2 headers', () => {
const headers = {
'x-foo': 'bar',
'x-hello': 'dolly',
connection: 'keep-alive',
'proxy-connection': 'keep-alive',
'keep-alive': 'true',
upgrade: 'probably',
'transfer-encoding': 'chunked',
'http2-settings': 'yeah',
};
const output = stripIllegalHttp2Headers({
headers,
isDev: false,
logger,
requestContext: 'requestContext',
});

expect(output).toEqual({
'x-foo': 'bar',
'x-hello': 'dolly',
});
});

it('ignores case when detecting headers', () => {
const headers = {
'x-foo': 'bar',
'x-hello': 'dolly',
Connection: 'keep-alive',
'Proxy-Connection': 'keep-alive',
'kEeP-AlIvE': 'true',
};
const output = stripIllegalHttp2Headers({
headers,
isDev: false,
logger,
requestContext: 'requestContext',
});

expect(output).toEqual({
'x-foo': 'bar',
'x-hello': 'dolly',
});
});

it('logs a warning about the illegal header when in dev mode', () => {
const headers = {
'x-foo': 'bar',
Connection: 'keep-alive',
};
stripIllegalHttp2Headers({
headers,
isDev: true,
logger,
requestContext: 'requestContext',
});

expect(logger.warn).toHaveBeenCalledTimes(1);
expect(logger.warn).toHaveBeenCalledWith(
`Handler for "requestContext" returned an illegal http2 header: Connection. Please check "request.protocol" in handlers before assigning connection headers`
);
});

it('does not log a warning about the illegal header when not in dev mode', () => {
const headers = {
'x-foo': 'bar',
Connection: 'keep-alive',
};
stripIllegalHttp2Headers({
headers,
isDev: false,
logger,
requestContext: 'requestContext',
});

expect(logger.warn).not.toHaveBeenCalled();
});

it('does not mutate the original headers', () => {
const headers = {
'x-foo': 'bar',
Connection: 'keep-alive',
};
stripIllegalHttp2Headers({
headers,
isDev: true,
logger,
requestContext: 'requestContext',
});

expect(headers).toEqual({
'x-foo': 'bar',
Connection: 'keep-alive',
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import type { Logger } from '@kbn/logging';
import type { ResponseHeaders } from '@kbn/core-http-server';

// from https://github.com/nodejs/node/blob/v22.2.0/lib/internal/http2/util.js#L557
const ILLEGAL_HTTP2_CONNECTION_HEADERS = new Set([
'connection',
'proxy-connection',
'keep-alive',
'upgrade',
'transfer-encoding',
'http2-settings',
]);

/**
* Return a new version of the provided headers, with all illegal http2 headers removed.
* If `isDev` is `true`, will also log a warning if such header is encountered.
*/
export const stripIllegalHttp2Headers = ({
headers,
isDev,
logger,
requestContext,
}: {
headers: ResponseHeaders;
isDev: boolean;
logger: Logger;
requestContext: string;
}): ResponseHeaders => {
return Object.entries(headers).reduce((output, [headerName, headerValue]) => {
if (ILLEGAL_HTTP2_CONNECTION_HEADERS.has(headerName.toLowerCase())) {
if (isDev) {
logger.warn(
`Handler for "${requestContext}" returned an illegal http2 header: ${headerName}. Please check "request.protocol" in handlers before assigning connection headers`
);
}
} else {
output[headerName as keyof ResponseHeaders] = headerValue;
}
return output;
}, {} as ResponseHeaders);
};
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
"@kbn/hapi-mocks",
"@kbn/core-logging-server-mocks",
"@kbn/logging",
"@kbn/core-http-common"
"@kbn/core-http-common",
"@kbn/logging-mocks"
],
"exclude": [
"target/**/*",
Expand Down

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 @@ -571,6 +571,73 @@ describe('cdn', () => {
});
});

describe('http2 protocol', () => {
it('throws if http2 is enabled but TLS is not', () => {
expect(() =>
config.schema.validate({
protocol: 'http2',
ssl: {
enabled: false,
},
})
).toThrowErrorMatchingInlineSnapshot(
`"http2 requires TLS to be enabled. Use 'http2.allowUnsecure: true' to allow running http2 without a valid h2c setup"`
);
});
it('throws if http2 is enabled but TLS has no suitable versions', () => {
expect(() =>
config.schema.validate({
protocol: 'http2',
ssl: {
enabled: true,
supportedProtocols: ['TLSv1.1'],
certificate: '/path/to/certificate',
key: '/path/to/key',
},
})
).toThrowErrorMatchingInlineSnapshot(
`"http2 requires 'ssl.supportedProtocols' to include TLSv1.2 or TLSv1.3. Use 'http2.allowUnsecure: true' to allow running http2 without a valid h2c setup"`
);
});
it('does not throws if http2 is enabled and TLS is not if http2.allowUnsecure is true', () => {
expect(
config.schema.validate({
protocol: 'http2',
http2: {
allowUnsecure: true,
},
ssl: {
enabled: false,
},
})
).toEqual(
expect.objectContaining({
protocol: 'http2',
})
);
});
it('does not throws if supportedProtocols are not valid for h2c if http2.allowUnsecure is true', () => {
expect(
config.schema.validate({
protocol: 'http2',
http2: {
allowUnsecure: true,
},
ssl: {
enabled: true,
supportedProtocols: ['TLSv1.1'],
certificate: '/path/to/certificate',
key: '/path/to/key',
},
})
).toEqual(
expect.objectContaining({
protocol: 'http2',
})
);
});
});

describe('HttpConfig', () => {
it('converts customResponseHeaders to strings or arrays of strings', () => {
const httpSchema = config.schema;
Expand Down
Loading