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

Set HTTP2 as default if SSL is enabled and add deprecation log if SSL is not enabled or protocol is set to HTTP1 #204384

Merged
merged 16 commits into from
Jan 3, 2025
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
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
6 changes: 3 additions & 3 deletions docs/setup/settings.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -472,9 +472,9 @@ 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`*
[[server-protocol]] `server.protocol`::
experimental[] The http protocol to use, either `http1` or `http2`. Set to `http1` to opt out of `HTTP/2` support when TLS is enabled. Use of `http1` may impact browser loading performance especially for dashboards with many panels.
*Default*: `http2` if TLS is enabled, otherwise `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
Expand Down
24 changes: 24 additions & 0 deletions docs/upgrade-notes.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -179,4 +179,28 @@ It will no longer be possible to create new scripted fields directly from the *D

*Action* +
Migrate to runtime fields or ES|QL instead of creating new scripted fields. Existing scripted fields can still be edited or deleted.
====


[float]
=== Known issue
jesuswr marked this conversation as resolved.
Show resolved Hide resolved

[discrete]
[[known-issue-204384]]
jesuswr marked this conversation as resolved.
Show resolved Hide resolved
.Now HTTP/2 is the default protocol when TLS is enabled and a deprecation warning appears if HTTP/2 is not enabled or TLS is not configured (9.0.0)
[%collapsible]
====
*Details* +
Starting from version 9.0.0, HTTP/2 is the default protocol when TLS is enabled. This ensures improved performance and security. However, if HTTP/2 is not enabled or TLS is not configured, a deprecation warning will be added.

For more information, refer to {kibana-pull}204384[#204384].

*Impact* +
Systems that have TLS enabled but don't specify a protocol will start using HTTP/2 in 9.0.0.
Systems that use HTTP/1 or don't have TLS configured will get a deprecation warning.

*Action* +
Verify that TLS is properly configured by enabling it and providing valid certificates in the settings. Test your system to ensure that connections are established securely over HTTP/2.

If your Kibana server is hosted behind a load balancer or reverse proxy we recommend testing your deployment configuration before upgrading to 9.0.
====
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,22 @@ describe('cdn', () => {
});
});

describe('http1 protocol', () => {
it('uses http1 as default if protocol is empty and ssl is not enabled', () => {
expect(
config.schema.validate({
ssl: {
enabled: false,
},
})
).toEqual(
expect.objectContaining({
protocol: 'http1',
})
);
});
});

describe('http2 protocol', () => {
it('throws if http2 is enabled but TLS is not', () => {
expect(() =>
Expand Down Expand Up @@ -642,6 +658,22 @@ describe('http2 protocol', () => {
})
);
});
it('uses http2 as default if protocol is empty and ssl is enabled', () => {
expect(
config.schema.validate({
ssl: {
enabled: true,
supportedProtocols: ['TLSv1.2'],
certificate: '/path/to/certificate',
key: '/path/to/key',
},
})
).toEqual(
expect.objectContaining({
protocol: 'http2',
})
);
});
});

describe('HttpConfig', () => {
Expand Down
36 changes: 32 additions & 4 deletions packages/core/http/core-http-server-internal/src/http_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { uuidRegexp } from '@kbn/core-base-server-internal';
import type { HttpProtocol, ICspConfig, IExternalUrlConfig } from '@kbn/core-http-server';
import type { IHttpEluMonitorConfig } from '@kbn/core-http-server/src/elu_monitor';
import type { HandlerResolutionStrategy } from '@kbn/core-http-router-server-internal';
import { get } from 'lodash';
import { CspConfig, CspConfigType } from './csp';
import { ExternalUrlConfig } from './external_url';
import {
Expand Down Expand Up @@ -123,9 +124,16 @@ const configSchema = schema.object(
}
},
}),
protocol: schema.oneOf([schema.literal('http1'), schema.literal('http2')], {
defaultValue: 'http1',
}),
protocol: schema.conditional(
schema.siblingRef('ssl.enabled'),
schema.literal(true),
schema.oneOf([schema.literal('http1'), schema.literal('http2')], {
defaultValue: 'http2',
}),
schema.oneOf([schema.literal('http1'), schema.literal('http2')], {
defaultValue: 'http1',
})
),
host: schema.string({
defaultValue: 'localhost',
hostname: true,
Expand Down Expand Up @@ -290,7 +298,27 @@ export type HttpConfigType = TypeOf<typeof configSchema>;
export const config: ServiceConfigDescriptor<HttpConfigType> = {
path: 'server' as const,
schema: configSchema,
deprecations: ({ rename }) => [rename('maxPayloadBytes', 'maxPayload', { level: 'warning' })],
deprecations: ({ rename }) => [
rename('maxPayloadBytes', 'maxPayload', { level: 'warning' }),
(settings, fromPath, addDeprecation, { docLinks }) => {
const cfg = get(settings, fromPath);
if (!cfg?.ssl?.enabled || cfg?.protocol === 'http1') {
addDeprecation({
level: 'warning',
title: `Consider enabling TLS and using HTTP/2 to improve security and performance.`,
configPath: `${fromPath}.protocol,${fromPath}.ssl.enabled`,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added one deprecation for both config values since they are related

message: `TLS is not enabled, or the HTTP protocol is set to HTTP/1. Enabling TLS and using HTTP/2 improves security and performance.`,
correctiveActions: {
manualSteps: [
`Set up TLS by configuring ${fromPath}.ssl.`,
`Set the protocol to 'http2' by updating ${fromPath}.protocol to 'http2' in your configuration.`,
],
},
documentationUrl: docLinks.server.protocol,
});
}
},
],
};

export class HttpConfig implements IHttpConfig {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,17 @@ describe('configuration deprecations', () => {
});

if (getFips() === 0) {
it('should not log deprecation warnings for default configuration', async () => {
it('should log one warning for default configuration, the http/tls deprecation warning', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it ok to have deprecation warnings on default config? Seems ok since the issue is to log when http2 or tls are not enabled but tls can't be enabled by default

Copy link
Contributor

Choose a reason for hiding this comment

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

Generally we wouldn't want to log a deprecation right "out of the box" after installing Kibana. But given that TLS is such an important security configuration I think this is fine

root = createRoot();

await root.preboot();
await root.setup();

const logs = loggingSystemMock.collect(mockLoggingSystem);
expect(logs.warn.flat()).toHaveLength(0);
expect(logs.warn.flat()).toHaveLength(1);
expect(logs.warn.flat()[0]).toEqual(
'TLS is not enabled, or the HTTP protocol is set to HTTP/1. Enabling TLS and using HTTP/2 improves security and performance.'
);
});
} else {
it('fips is enabled and the default configuration has been overridden', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,9 @@ export const getDocLinks = ({ kibanaBranch, buildFlavor }: GetDocLinkOptions): D
ruleApiOverview: `${SECURITY_SOLUTION_DOCS}rule-api-overview.html`,
configureAlertSuppression: `${SECURITY_SOLUTION_DOCS}alert-suppression.html#_configure_alert_suppression`,
},
server: {
protocol: `${KIBANA_DOCS}settings.html#server-protocol`,
},
securitySolution: {
artifactControl: `${SECURITY_SOLUTION_DOCS}artifact-control.html`,
avcResults: `${ELASTIC_WEBSITE_URL}blog/elastic-av-comparatives-business-security-test`,
Expand Down
3 changes: 3 additions & 0 deletions src/platform/packages/shared/kbn-doc-links/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,9 @@ export interface DocLinks {
readonly ruleApiOverview: string;
readonly configureAlertSuppression: string;
};
readonly server: {
readonly protocol: string;
};
readonly securitySolution: {
readonly aiAssistant: string;
readonly artifactControl: string;
Expand Down
3 changes: 3 additions & 0 deletions test/server_integration/http/ssl_redirect/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) {
`--server.ssl.key=${KBN_KEY_PATH}`,
`--server.ssl.certificate=${KBN_CERT_PATH}`,
`--server.ssl.redirectHttpFromPort=${redirectPort}`,
// supertest is configured with http1 so it fails when redirecting
// to an http2 server
`--server.protocol=http1`,
],
},
};
Expand Down