Add http2 support for Kibana server (#183465)

## Summary Part of #7104 Add support for `http2` to the Kibana server. `http2` can be enabled by setting `server.protocol: http2` in the Kibana config file. *Note: by default, enabling `http2` requires a valid `h2c` configuration, meaning that it can only run over HTTPS with TLS1.2+* ```yaml ## kibana.yaml server.protocol: http2 server.ssl.enabled: true server.ssl.key: path/to/key server.ssl.certificate: path/my/cerf ``` ## What is this PR doing ### Add HTTP2 support for the Kibana server #### - Plug http2 to the Kibana server Even if HAPI was never officially updated to really support HTTP2, node's `http`/`https`/`http2` modules are compatible enough to be able to just instantiate an http2 server/listener and provide it to HAPI "as a plain https listener". There were some tweaks to do (mostly silencing a few warnings that HAPI was causing by sending http2-illegal headers such as `Connection`), but overall, it went smoothly. #### - Add config validation By default, Kibana will require a valid `h2c` configuration to accept enabling `http2`. It means that TLS must be enabled and that TLS1.2+ should at least be in the list of supported SSL protocols (`server.ssl.supportedProtocols`). Note that default value of this setting includes TLS1.2 and 1.3. #### - Add escape hatch to run `h2` without `h2c` In some situations, it may be required to enable http2 without a valid `h2c` configuration. Kibana supports it, by setting `server.http2.allowUnsecure` to `true`. (*Note, however, that if http2 is enabled without TLS, ALPN protocol negotiation won't work, meaning that most http2 agents/clients will fail connecting unless they're explictly configured to use http2.*) ### Add documentation about this new feature #### - Update the user-facing doc about this new `server.protocol` setting Update the user-facing Kibana settings documentation to include this `http.protocol` setting (and refer to `server.http2.allowUnsecure`) **Note: this setting, and this feature, are considered as experimental** ### Adapt our dev tooling to support running Kibana with http2 enabled #### - Add a `--http2` flag to the dev CLI Enabling this flag will add the proper configuration settings to run Kibana with `http2` enabled in an (almost) valid `h2c` configutation. *Note: when using this flag, even if listening on the same port, the Kibana server will be accessible over https, meaning that you need to use https in your browser to access it. Aka `http://localhost:5601` won't work, you need to use `https://localhost:5601`. Also, we're using the self-signed dev certificates, meaning that you must go though the scary warning of your browser* #### - Implement an http2-compatible base-path proxy The current base path proxy is based on `hapi` and `hapi/h2o2`. I tried for a bunch hours trying to hack around to make it work with http2 proxying, but ultimately gave up and implemented a new version from scratch. Note that with some additional efforts, this new http2 basepath proxy could probably fully replace the existing one and be used for both http1 and http2 traffic, but it's an optimization / refactoring that did not feel required for this PR. ### Adapt the FTR to run suites against http2 #### - Add support to run FTR test suite against an h2c-enabled Kibana Note that with ALPN, clients using http1 should be (and are) able to communicate with http2 Kibana, given h2c/alpn allows protocol negitiation. So adapting our FTR tooling was not really about making it work with http2 (which worked out of the box), but making it work with **the self signed certifcates we use for https on dev mode** Note that I'm not a big fan of what I had to do, however, realistically this was the only possible approach if we want to run arbitrary test suites with TLS/HTTP2 enabled without massively changing our FTR setup. Operations and QA, feel free to chime in there, as this is your territory. #### - Change some FTR test suites to run against an HTTP2-enabled server I added a quick `configureHTTP2` helper function to take any "final" FTR suite config and mutate it to enable `http2`. I then enabled it on a few suites locally, to make sure the suites were passing correctly. I kept two suites running with http2 enabled: - the `console` oss functional tests - the `home` oss functional tests We could possibly enable it for more, but we need to figure out what kind of strategy we want on that matter (see below) ## What is this pull request NOT doing #### - Making sure everything works when HTTP2 is enabled I navigated the applications quite a bit, and did not see anything broken, however I obviously wasn't able to do a full coverage. Also, the self-signed certificate was a huge pain to detect issues really caused by http2 compared to issues because the local setup isn't valid `h2c`. In theory though (famous last words) anything not doing http/1.1 specific hacks such as bfetch should work fine with http2, given that even if using non-http2 clients, ALPN should just allow to fallback to http/1.x (this part was tested) #### - Enabling HTTP2 by default PR isn't doing it for obvious reasons. #### - Enabling HTTP2 for all FTR suites First of all, it's not that easy, because it requires adapting various parts of the config (and even some var env...), and we don't have any proper way to override config "at the end". For instance, if you add the http2 config on a top level config (e.g. the oss functional one that is reuse by the whole world - learned the hard way), it won't work because higher-level configs redefined (and override) the `browser` part of the config, loosing the settings added to run the browser in insecure mode. Secondly, I'm not sure we really need to run that many suites with http2 enabled. I learned working on that PR that we only have like one suite where https is enabled for the Kibana server, and I feel like it could be fine to have the same for http2. In theory it's just a protocol change, unless parts of our apps (e.g. bfetch) are doing things that are specific to http/1.1, switching to http2 should be an implementation detail. But I'd love to get @elastic/kibana-operations and @elastic/appex-qa opinion on that one, given they have more expertise than I do on that area. - Running performances tests We should absolutely run perf testing between http/1.1 over https and http/2, to make sure that it goes into the right directly (at least in term of user perceived speed), but I did not do it in the scope of this PR (and @dmlemeshko is on PTO so... 😅) ## Release Note Add support for `http2` to the Kibana server. `http2` can be enabled by setting `server.protocol: http2` in the Kibana config file. Note: by default, enabling `http2` requires a valid `h2c` configuration, meaning that it can only run over HTTPS with TLS1.2+ Please refer to the Kibana config documentation for more details. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
elastic · Jun 3, 2024 · dea26c6 · dea26c6
1 parent dd35670
commit dea26c6
Show file tree

Hide file tree

Showing 52 changed files with 1,613 additions and 168 deletions.
diff --git a/docs/setup/settings.asciidoc b/docs/setup/settings.asciidoc
@@ -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`::
+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.
 

diff --git a/package.json b/package.json
@@ -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",
     "ignore": "^5.3.0",
     "jest": "^29.6.1",
     "jest-canvas-mock": "^2.5.2",

diff --git a/packages/core/http/core-http-router-server-internal/src/request.test.ts b/packages/core/http/core-http-router-server-internal/src/request.test.ts
@@ -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', () => {

diff --git a/packages/core/http/core-http-router-server-internal/src/request.ts b/packages/core/http/core-http-router-server-internal/src/request.ts
@@ -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
@@ -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';
+}
diff --git a/packages/core/http/core-http-router-server-internal/src/router.ts b/packages/core/http/core-http-router-server-internal/src/router.ts
@@ -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,
@@ -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}`,
+        });
+      }
       return hapiResponseAdapter.handle(kibanaResponse);
     } catch (error) {
       // capture error

diff --git a/packages/core/http/core-http-router-server-internal/src/strip_illegal_http2_headers.test.ts b/packages/core/http/core-http-router-server-internal/src/strip_illegal_http2_headers.test.ts
@@ -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',
+    });
+  });
+});
diff --git a/packages/core/http/core-http-router-server-internal/src/strip_illegal_http2_headers.ts b/packages/core/http/core-http-router-server-internal/src/strip_illegal_http2_headers.ts
@@ -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);
+};
diff --git a/packages/core/http/core-http-router-server-internal/tsconfig.json b/packages/core/http/core-http-router-server-internal/tsconfig.json
@@ -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/**/*",

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
diff --git a/packages/core/http/core-http-server-internal/src/http_config.test.ts b/packages/core/http/core-http-server-internal/src/http_config.test.ts
@@ -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;