From 42a12968ac2d3a4aace4be86e9c7866dd29ea064 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Mon, 1 Nov 2021 12:26:57 -0700 Subject: [PATCH 1/8] [Reporting] Internal correction for server hostname if "server.host" is "0.0.0.0" --- .../server/config/create_config.test.ts | 69 +++++++++++-------- .../reporting/server/config/create_config.ts | 18 ++++- .../test_helpers/create_mock_levellogger.ts | 3 +- 3 files changed, 58 insertions(+), 32 deletions(-) diff --git a/x-pack/plugins/reporting/server/config/create_config.test.ts b/x-pack/plugins/reporting/server/config/create_config.test.ts index e042142a54a0f..0acca4ad44019 100644 --- a/x-pack/plugins/reporting/server/config/create_config.test.ts +++ b/x-pack/plugins/reporting/server/config/create_config.test.ts @@ -5,27 +5,29 @@ * 2.0. */ +import * as Rx from 'rxjs'; import { CoreSetup, PluginInitializerContext } from 'src/core/server'; import { coreMock } from 'src/core/server/mocks'; -import { LevelLogger } from '../lib'; -import { createMockConfigSchema } from '../test_helpers'; +import { LevelLogger } from '../lib/level_logger'; +import { createMockConfigSchema, createMockLevelLogger } from '../test_helpers'; +import { ReportingConfigType } from './'; import { createConfig$ } from './create_config'; +const createMockConfig = ( + mockInitContext: PluginInitializerContext +): Rx.Observable => mockInitContext.config.create(); + describe('Reporting server createConfig$', () => { let mockCoreSetup: CoreSetup; let mockInitContext: PluginInitializerContext; - let mockLogger: LevelLogger; + let mockLogger: jest.Mocked; beforeEach(() => { mockCoreSetup = coreMock.createSetup(); mockInitContext = coreMock.createPluginInitializerContext( createMockConfigSchema({ kibanaServer: {} }) ); - mockLogger = { - warn: jest.fn(), - debug: jest.fn(), - clone: jest.fn().mockImplementation(() => mockLogger), - } as unknown as LevelLogger; + mockLogger = createMockLevelLogger(); }); afterEach(() => { @@ -37,19 +39,12 @@ describe('Reporting server createConfig$', () => { ...createMockConfigSchema({ kibanaServer: {} }), encryptionKey: undefined, }); - const mockConfig$: any = mockInitContext.config.create(); + const mockConfig$ = createMockConfig(mockInitContext); const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise(); expect(result.encryptionKey).toMatch(/\S{32,}/); // random 32 characters - expect(result.kibanaServer).toMatchInlineSnapshot(` - Object { - "hostname": "localhost", - "port": 80, - "protocol": "http", - } - `); - expect((mockLogger.warn as any).mock.calls.length).toBe(1); - expect((mockLogger.warn as any).mock.calls[0]).toMatchObject([ + expect(mockLogger.warn.mock.calls.length).toBe(1); + expect(mockLogger.warn.mock.calls[0]).toMatchObject([ 'Generating a random key for xpack.reporting.encryptionKey. To prevent sessions from being invalidated on restart, please set xpack.reporting.encryptionKey in the kibana.yml or use the bin/kibana-encryption-keys command.', ]); }); @@ -60,10 +55,10 @@ describe('Reporting server createConfig$', () => { encryptionKey: 'iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii', }) ); - const mockConfig$: any = mockInitContext.config.create(); + const mockConfig$ = createMockConfig(mockInitContext); const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise(); expect(result.encryptionKey).toMatch('iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii'); - expect((mockLogger.warn as any).mock.calls.length).toBe(0); + expect(mockLogger.warn.mock.calls.length).toBe(0); }); it('uses the user-provided encryption key, reporting kibanaServer settings to override server info', async () => { @@ -77,7 +72,7 @@ describe('Reporting server createConfig$', () => { }, }) ); - const mockConfig$: any = mockInitContext.config.create(); + const mockConfig$ = createMockConfig(mockInitContext); const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise(); expect(result).toMatchInlineSnapshot(` @@ -108,7 +103,25 @@ describe('Reporting server createConfig$', () => { }, } `); - expect((mockLogger.warn as any).mock.calls.length).toBe(0); + expect(mockLogger.warn.mock.calls.length).toBe(0); + }); + + it('show warning when kibanaServer.hostName === "0.0.0.0"', async () => { + mockInitContext = coreMock.createPluginInitializerContext({ + encryptionKey: 'aaaaaaaaaaaaabbbbbbbbbbbbaaaaaaaaa', + kibanaServer: { hostname: '0.0.0.0', port: 5601 }, + capture: { browser: { chromium: {} } }, + }); + const mockConfig$ = createMockConfig(mockInitContext); + const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise(); + + expect(result.kibanaServer).toMatchInlineSnapshot(` + Object { + "hostname": "localhost", + "port": 5601, + "protocol": "http", + } + `); }); it('uses user-provided disableSandbox: false', async () => { @@ -118,11 +131,11 @@ describe('Reporting server createConfig$', () => { capture: { browser: { chromium: { disableSandbox: false } } }, }) ); - const mockConfig$: any = mockInitContext.config.create(); + const mockConfig$ = createMockConfig(mockInitContext); const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise(); expect(result.capture.browser.chromium).toMatchObject({ disableSandbox: false }); - expect((mockLogger.warn as any).mock.calls.length).toBe(0); + expect(mockLogger.warn.mock.calls.length).toBe(0); }); it('uses user-provided disableSandbox: true', async () => { @@ -132,11 +145,11 @@ describe('Reporting server createConfig$', () => { capture: { browser: { chromium: { disableSandbox: true } } }, }) ); - const mockConfig$: any = mockInitContext.config.create(); + const mockConfig$ = createMockConfig(mockInitContext); const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise(); expect(result.capture.browser.chromium).toMatchObject({ disableSandbox: true }); - expect((mockLogger.warn as any).mock.calls.length).toBe(0); + expect(mockLogger.warn.mock.calls.length).toBe(0); }); it('provides a default for disableSandbox', async () => { @@ -145,10 +158,10 @@ describe('Reporting server createConfig$', () => { encryptionKey: '888888888888888888888888888888888', }) ); - const mockConfig$: any = mockInitContext.config.create(); + const mockConfig$ = createMockConfig(mockInitContext); const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise(); expect(result.capture.browser.chromium).toMatchObject({ disableSandbox: expect.any(Boolean) }); - expect((mockLogger.warn as any).mock.calls.length).toBe(0); + expect(mockLogger.warn.mock.calls.length).toBe(0); }); }); diff --git a/x-pack/plugins/reporting/server/config/create_config.ts b/x-pack/plugins/reporting/server/config/create_config.ts index b1d69d17334be..42910836f8198 100644 --- a/x-pack/plugins/reporting/server/config/create_config.ts +++ b/x-pack/plugins/reporting/server/config/create_config.ts @@ -43,10 +43,22 @@ export function createConfig$( } const { kibanaServer: reportingServer } = config; const serverInfo = core.http.getServerInfo(); - // kibanaServer.hostname, default to server.host - const kibanaServerHostname = reportingServer.hostname + // set kibanaServer.hostname, default to server.host, don't allow "0.0.0.0" as it breaks in Windows + let kibanaServerHostname = reportingServer.hostname ? reportingServer.hostname : serverInfo.hostname; + if (kibanaServerHostname === '0.0.0.0') { + logger.warn( + i18n.translate('xpack.reporting.serverConfig.invalidServerHostname', { + defaultMessage: + `Found 'server.host: "0.0.0.0"' in Kibana configuration. Reporting is not able to use this as the Kibana server hostname.` + + ` To enable PNG/PDF Reporting to work, '{configKey}: localhost' is automatically set in the configuration.` + + ` You can prevent this message by adding '{configKey}: localhost' in kibana.yml.`, + values: { configKey: 'xpack.reporting.kibanaServer.hostname' }, + }) + ); + kibanaServerHostname = 'localhost'; + } // kibanaServer.port, default to server.port const kibanaServerPort = reportingServer.port ? reportingServer.port : serverInfo.port; // kibanaServer.protocol, default to server.protocol @@ -66,7 +78,7 @@ export function createConfig$( mergeMap(async (config) => { if (config.capture.browser.chromium.disableSandbox != null) { // disableSandbox was set by user - return config; + return { ...config }; } // disableSandbox was not set by user, apply default for OS diff --git a/x-pack/plugins/reporting/server/test_helpers/create_mock_levellogger.ts b/x-pack/plugins/reporting/server/test_helpers/create_mock_levellogger.ts index 1a8bfe7b70208..a6e6be47bdfcd 100644 --- a/x-pack/plugins/reporting/server/test_helpers/create_mock_levellogger.ts +++ b/x-pack/plugins/reporting/server/test_helpers/create_mock_levellogger.ts @@ -16,7 +16,6 @@ export function createMockLevelLogger() { const logger = new LevelLogger(loggingSystemMock.create()) as jest.Mocked; - logger.clone.mockImplementation(createMockLevelLogger); // logger.debug.mockImplementation(consoleLogger('debug')); // uncomment this to see debug logs in jest tests logger.info.mockImplementation(consoleLogger('info')); logger.warn.mockImplementation(consoleLogger('warn')); @@ -24,5 +23,7 @@ export function createMockLevelLogger() { logger.error.mockImplementation(consoleLogger('error')); logger.trace.mockImplementation(consoleLogger('trace')); + logger.clone.mockImplementation(() => logger); + return logger; } From 3b4cd514afbb33c8b283142e2c1dd9a2cfad8d17 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Mon, 1 Nov 2021 13:15:01 -0700 Subject: [PATCH 2/8] add schema validation --- .../plugins/reporting/server/config/schema.test.ts | 11 ++++++++++- x-pack/plugins/reporting/server/config/schema.ts | 12 +++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/reporting/server/config/schema.test.ts b/x-pack/plugins/reporting/server/config/schema.test.ts index 6ad7d03bd1a8f..e436b2b0be0dd 100644 --- a/x-pack/plugins/reporting/server/config/schema.test.ts +++ b/x-pack/plugins/reporting/server/config/schema.test.ts @@ -288,11 +288,20 @@ describe('Reporting Config Schema', () => { `); }); - it(`logs the proper validation messages`, () => { + it(`logs the proper validation messages for invalid hostname`, () => { // kibanaServer const throwValidationErr = () => ConfigSchema.validate({ kibanaServer: { hostname: '0' } }); expect(throwValidationErr).toThrowError( `[kibanaServer.hostname]: value must be a valid hostname (see RFC 1123).` ); }); + + it(`logs the proper validation messages for hostname: 0.0.0.0`, () => { + // kibanaServer + const throwValidationErr = () => + ConfigSchema.validate({ kibanaServer: { hostname: '0.0.0.0' } }); + expect(throwValidationErr).toThrowError( + `[kibanaServer.hostname]: must be a valid hostname, not "0.0.0.0"` + ); + }); }); diff --git a/x-pack/plugins/reporting/server/config/schema.ts b/x-pack/plugins/reporting/server/config/schema.ts index 5b15260be06cb..dfc1b8c8f4ed2 100644 --- a/x-pack/plugins/reporting/server/config/schema.ts +++ b/x-pack/plugins/reporting/server/config/schema.ts @@ -9,7 +9,17 @@ import { ByteSizeValue, schema, TypeOf } from '@kbn/config-schema'; import moment from 'moment'; const KibanaServerSchema = schema.object({ - hostname: schema.maybe(schema.string({ hostname: true })), + hostname: schema.maybe( + schema.string({ + hostname: true, + validate(value) { + if (/^0.0.0.0$/.test(value)) { + // prevent setting a hostname that fails in Chromium on Windows + return 'must be a valid hostname, not "0.0.0.0"'; + } + }, + }) + ), port: schema.maybe(schema.number()), protocol: schema.maybe( schema.string({ From 86fd8ce10adf718d1362d1428162ef8292e5ab74 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Tue, 2 Nov 2021 16:49:19 -0700 Subject: [PATCH 3/8] correction to the failover logic the test is validation --- .../server/config/create_config.test.ts | 33 +++++++++++++++---- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/reporting/server/config/create_config.test.ts b/x-pack/plugins/reporting/server/config/create_config.test.ts index 0acca4ad44019..d49cf82ea46ea 100644 --- a/x-pack/plugins/reporting/server/config/create_config.test.ts +++ b/x-pack/plugins/reporting/server/config/create_config.test.ts @@ -6,7 +6,7 @@ */ import * as Rx from 'rxjs'; -import { CoreSetup, PluginInitializerContext } from 'src/core/server'; +import { CoreSetup, HttpServerInfo, PluginInitializerContext } from 'src/core/server'; import { coreMock } from 'src/core/server/mocks'; import { LevelLogger } from '../lib/level_logger'; import { createMockConfigSchema, createMockLevelLogger } from '../test_helpers'; @@ -106,12 +106,26 @@ describe('Reporting server createConfig$', () => { expect(mockLogger.warn.mock.calls.length).toBe(0); }); - it('show warning when kibanaServer.hostName === "0.0.0.0"', async () => { - mockInitContext = coreMock.createPluginInitializerContext({ - encryptionKey: 'aaaaaaaaaaaaabbbbbbbbbbbbaaaaaaaaa', - kibanaServer: { hostname: '0.0.0.0', port: 5601 }, - capture: { browser: { chromium: {} } }, - }); + it(`apply failover logic to use "localhost" when "server.host" is "0.0.0.0"`, async () => { + mockInitContext = coreMock.createPluginInitializerContext( + createMockConfigSchema({ + encryptionKey: 'aaaaaaaaaaaaabbbbbbbbbbbbaaaaaaaaa', + kibanaServer: { + hostname: undefined, + port: undefined, + }, // overwrite settings added by createMockConfigSchema and apply the default settings + }) + ); + + mockCoreSetup.http.getServerInfo = jest.fn().mockImplementation( + (): HttpServerInfo => ({ + name: 'cool server', + hostname: '0.0.0.0', + port: 5601, + protocol: 'http', + }) + ); + const mockConfig$ = createMockConfig(mockInitContext); const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise(); @@ -122,6 +136,11 @@ describe('Reporting server createConfig$', () => { "protocol": "http", } `); + + expect(mockLogger.warn.mock.calls.length).toBe(1); + expect(mockLogger.warn.mock.calls[0]).toMatchObject([ + "Found 'server.host: \"0.0.0.0\"' in Kibana configuration. Reporting is not able to use this as the Kibana server hostname. To enable PNG/PDF Reporting to work, 'xpack.reporting.kibanaServer.hostname: localhost' is automatically set in the configuration. You can prevent this message by adding 'xpack.reporting.kibanaServer.hostname: localhost' in kibana.yml.", + ]); }); it('uses user-provided disableSandbox: false', async () => { From 7af5772f82c7cfe4740d2ad358c5b93d4c6962c3 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Tue, 2 Nov 2021 16:51:00 -0700 Subject: [PATCH 4/8] update per feedback --- x-pack/plugins/reporting/server/config/schema.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/reporting/server/config/schema.ts b/x-pack/plugins/reporting/server/config/schema.ts index dfc1b8c8f4ed2..9f3d0db6959e1 100644 --- a/x-pack/plugins/reporting/server/config/schema.ts +++ b/x-pack/plugins/reporting/server/config/schema.ts @@ -15,7 +15,7 @@ const KibanaServerSchema = schema.object({ validate(value) { if (/^0.0.0.0$/.test(value)) { // prevent setting a hostname that fails in Chromium on Windows - return 'must be a valid hostname, not "0.0.0.0"'; + return `cannot use '0.0.0.0' as Kibana host name, consider using the default (localhost) instead`; } }, }) From 648062024a7c74e0fb2c3eae39fa761eacaaefc3 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Tue, 2 Nov 2021 17:39:19 -0700 Subject: [PATCH 5/8] more check against invalid hostnames --- .../server/config/create_config.test.ts | 105 ++++++++++++------ .../reporting/server/config/create_config.ts | 9 +- .../reporting/server/config/schema.test.ts | 20 +++- .../plugins/reporting/server/config/schema.ts | 4 +- 4 files changed, 97 insertions(+), 41 deletions(-) diff --git a/x-pack/plugins/reporting/server/config/create_config.test.ts b/x-pack/plugins/reporting/server/config/create_config.test.ts index d49cf82ea46ea..38795a488a2e9 100644 --- a/x-pack/plugins/reporting/server/config/create_config.test.ts +++ b/x-pack/plugins/reporting/server/config/create_config.test.ts @@ -106,43 +106,6 @@ describe('Reporting server createConfig$', () => { expect(mockLogger.warn.mock.calls.length).toBe(0); }); - it(`apply failover logic to use "localhost" when "server.host" is "0.0.0.0"`, async () => { - mockInitContext = coreMock.createPluginInitializerContext( - createMockConfigSchema({ - encryptionKey: 'aaaaaaaaaaaaabbbbbbbbbbbbaaaaaaaaa', - kibanaServer: { - hostname: undefined, - port: undefined, - }, // overwrite settings added by createMockConfigSchema and apply the default settings - }) - ); - - mockCoreSetup.http.getServerInfo = jest.fn().mockImplementation( - (): HttpServerInfo => ({ - name: 'cool server', - hostname: '0.0.0.0', - port: 5601, - protocol: 'http', - }) - ); - - const mockConfig$ = createMockConfig(mockInitContext); - const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise(); - - expect(result.kibanaServer).toMatchInlineSnapshot(` - Object { - "hostname": "localhost", - "port": 5601, - "protocol": "http", - } - `); - - expect(mockLogger.warn.mock.calls.length).toBe(1); - expect(mockLogger.warn.mock.calls[0]).toMatchObject([ - "Found 'server.host: \"0.0.0.0\"' in Kibana configuration. Reporting is not able to use this as the Kibana server hostname. To enable PNG/PDF Reporting to work, 'xpack.reporting.kibanaServer.hostname: localhost' is automatically set in the configuration. You can prevent this message by adding 'xpack.reporting.kibanaServer.hostname: localhost' in kibana.yml.", - ]); - }); - it('uses user-provided disableSandbox: false', async () => { mockInitContext = coreMock.createPluginInitializerContext( createMockConfigSchema({ @@ -183,4 +146,72 @@ describe('Reporting server createConfig$', () => { expect(result.capture.browser.chromium).toMatchObject({ disableSandbox: expect.any(Boolean) }); expect(mockLogger.warn.mock.calls.length).toBe(0); }); + + describe('prevent invalid server hostnames', () => { + beforeEach(() => { + mockInitContext = coreMock.createPluginInitializerContext( + createMockConfigSchema({ + encryptionKey: 'aaaaaaaaaaaaabbbbbbbbbbbbaaaaaaaaa', + kibanaServer: { + hostname: undefined, + port: undefined, + }, // overwrite settings added by createMockConfigSchema and apply the default settings + }) + ); + }); + + it(`apply failover logic to use "localhost" when "server.host" is "0.0.0.0"`, async () => { + mockCoreSetup.http.getServerInfo = jest.fn().mockImplementation( + (): HttpServerInfo => ({ + name: 'cool server', + hostname: '0.0.0.0', + port: 5601, + protocol: 'http', + }) + ); + + const mockConfig$ = createMockConfig(mockInitContext); + const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise(); + + expect(result.kibanaServer).toMatchInlineSnapshot(` + Object { + "hostname": "localhost", + "port": 5601, + "protocol": "http", + } + `); + + expect(mockLogger.warn.mock.calls.length).toBe(1); + expect(mockLogger.warn.mock.calls[0]).toMatchObject([ + "Found 'server.host: \"0.0.0.0\"' in Kibana configuration. Reporting is not able to use this as the Kibana server hostname. To enable PNG/PDF Reporting to work, 'xpack.reporting.kibanaServer.hostname: localhost' is automatically set in the configuration. You can prevent this message by adding 'xpack.reporting.kibanaServer.hostname: localhost' in kibana.yml.", + ]); + }); + + it(`apply failover logic when using a variation of the "0" address`, async () => { + mockCoreSetup.http.getServerInfo = jest.fn().mockImplementation( + (): HttpServerInfo => ({ + name: 'cool server', + hostname: '0.0', + port: 5601, + protocol: 'http', + }) + ); + + const mockConfig$ = createMockConfig(mockInitContext); + const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise(); + + expect(result.kibanaServer).toMatchInlineSnapshot(` + Object { + "hostname": "localhost", + "port": 5601, + "protocol": "http", + } + `); + + expect(mockLogger.warn.mock.calls.length).toBe(1); + expect(mockLogger.warn.mock.calls[0]).toMatchObject([ + "Found 'server.host: \"0.0.0.0\"' in Kibana configuration. Reporting is not able to use this as the Kibana server hostname. To enable PNG/PDF Reporting to work, 'xpack.reporting.kibanaServer.hostname: localhost' is automatically set in the configuration. You can prevent this message by adding 'xpack.reporting.kibanaServer.hostname: localhost' in kibana.yml.", + ]); + }); + }); }); diff --git a/x-pack/plugins/reporting/server/config/create_config.ts b/x-pack/plugins/reporting/server/config/create_config.ts index 42910836f8198..eda1498f9f124 100644 --- a/x-pack/plugins/reporting/server/config/create_config.ts +++ b/x-pack/plugins/reporting/server/config/create_config.ts @@ -7,7 +7,8 @@ import { i18n } from '@kbn/i18n'; import crypto from 'crypto'; -import { upperFirst } from 'lodash'; +import ipaddr from 'ipaddr.js'; +import { sum, upperFirst } from 'lodash'; import { Observable } from 'rxjs'; import { map, mergeMap } from 'rxjs/operators'; import { CoreSetup } from 'src/core/server'; @@ -47,7 +48,11 @@ export function createConfig$( let kibanaServerHostname = reportingServer.hostname ? reportingServer.hostname : serverInfo.hostname; - if (kibanaServerHostname === '0.0.0.0') { + + if ( + ipaddr.isValid(kibanaServerHostname) && + !sum(ipaddr.parse(kibanaServerHostname).toByteArray()) + ) { logger.warn( i18n.translate('xpack.reporting.serverConfig.invalidServerHostname', { defaultMessage: diff --git a/x-pack/plugins/reporting/server/config/schema.test.ts b/x-pack/plugins/reporting/server/config/schema.test.ts index e436b2b0be0dd..e5035f3304dc0 100644 --- a/x-pack/plugins/reporting/server/config/schema.test.ts +++ b/x-pack/plugins/reporting/server/config/schema.test.ts @@ -301,7 +301,25 @@ describe('Reporting Config Schema', () => { const throwValidationErr = () => ConfigSchema.validate({ kibanaServer: { hostname: '0.0.0.0' } }); expect(throwValidationErr).toThrowError( - `[kibanaServer.hostname]: must be a valid hostname, not "0.0.0.0"` + `[kibanaServer.hostname]: cannot use '0.0.0.0' as Kibana host name, consider using the default (localhost) instead` + ); + }); + + it(`logs the proper validation messages for hostname set to a variation of the "0" address`, () => { + expect(() => ConfigSchema.validate({ kibanaServer: { hostname: '0.0.0' } })).toThrowError( + `[kibanaServer.hostname]: value must be a valid hostname (see RFC 1123).` + ); + + expect(() => + ConfigSchema.validate({ + kibanaServer: { hostname: '0000:0000:0000:0000:0000:0000:0000:0000' }, + }) + ).toThrowError( + `[kibanaServer.hostname]: cannot use '0.0.0.0' as Kibana host name, consider using the default (localhost) instead` + ); + + expect(() => ConfigSchema.validate({ kibanaServer: { hostname: '::' } })).toThrowError( + `[kibanaServer.hostname]: cannot use '0.0.0.0' as Kibana host name, consider using the default (localhost) instead` ); }); }); diff --git a/x-pack/plugins/reporting/server/config/schema.ts b/x-pack/plugins/reporting/server/config/schema.ts index 9f3d0db6959e1..4c56fc4c6db60 100644 --- a/x-pack/plugins/reporting/server/config/schema.ts +++ b/x-pack/plugins/reporting/server/config/schema.ts @@ -6,6 +6,8 @@ */ import { ByteSizeValue, schema, TypeOf } from '@kbn/config-schema'; +import ipaddr from 'ipaddr.js'; +import { sum } from 'lodash'; import moment from 'moment'; const KibanaServerSchema = schema.object({ @@ -13,7 +15,7 @@ const KibanaServerSchema = schema.object({ schema.string({ hostname: true, validate(value) { - if (/^0.0.0.0$/.test(value)) { + if (ipaddr.isValid(value) && !sum(ipaddr.parse(value).toByteArray())) { // prevent setting a hostname that fails in Chromium on Windows return `cannot use '0.0.0.0' as Kibana host name, consider using the default (localhost) instead`; } From 3d91c33f726cc18161ec08fde6eb1911025531f9 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Tue, 2 Nov 2021 17:43:39 -0700 Subject: [PATCH 6/8] remove silly translations for server logs --- .../reporting/server/config/create_config.ts | 40 +++++-------------- 1 file changed, 10 insertions(+), 30 deletions(-) diff --git a/x-pack/plugins/reporting/server/config/create_config.ts b/x-pack/plugins/reporting/server/config/create_config.ts index eda1498f9f124..5de54a43582ab 100644 --- a/x-pack/plugins/reporting/server/config/create_config.ts +++ b/x-pack/plugins/reporting/server/config/create_config.ts @@ -5,7 +5,6 @@ * 2.0. */ -import { i18n } from '@kbn/i18n'; import crypto from 'crypto'; import ipaddr from 'ipaddr.js'; import { sum, upperFirst } from 'lodash'; @@ -34,11 +33,8 @@ export function createConfig$( let encryptionKey = config.encryptionKey; if (encryptionKey === undefined) { logger.warn( - i18n.translate('xpack.reporting.serverConfig.randomEncryptionKey', { - defaultMessage: - 'Generating a random key for xpack.reporting.encryptionKey. To prevent sessions from being invalidated on ' + - 'restart, please set xpack.reporting.encryptionKey in the kibana.yml or use the bin/kibana-encryption-keys command.', - }) + 'Generating a random key for xpack.reporting.encryptionKey. To prevent sessions from being invalidated on ' + + 'restart, please set xpack.reporting.encryptionKey in the kibana.yml or use the bin/kibana-encryption-keys command.' ); encryptionKey = crypto.randomBytes(16).toString('hex'); } @@ -54,13 +50,9 @@ export function createConfig$( !sum(ipaddr.parse(kibanaServerHostname).toByteArray()) ) { logger.warn( - i18n.translate('xpack.reporting.serverConfig.invalidServerHostname', { - defaultMessage: - `Found 'server.host: "0.0.0.0"' in Kibana configuration. Reporting is not able to use this as the Kibana server hostname.` + - ` To enable PNG/PDF Reporting to work, '{configKey}: localhost' is automatically set in the configuration.` + - ` You can prevent this message by adding '{configKey}: localhost' in kibana.yml.`, - values: { configKey: 'xpack.reporting.kibanaServer.hostname' }, - }) + `Found 'server.host: "0.0.0.0"' in Kibana configuration. Reporting is not able to use this as the Kibana server hostname.` + + ` To enable PNG/PDF Reporting to work, 'xpack.reporting.kibanaServer.hostname: localhost' is automatically set in the configuration.` + + ` You can prevent this message by adding 'xpack.reporting.kibanaServer.hostname: localhost' in kibana.yml.` ); kibanaServerHostname = 'localhost'; } @@ -90,29 +82,17 @@ export function createConfig$( const { os, disableSandbox } = await getDefaultChromiumSandboxDisabled(); const osName = [os.os, os.dist, os.release].filter(Boolean).map(upperFirst).join(' '); - logger.debug( - i18n.translate('xpack.reporting.serverConfig.osDetected', { - defaultMessage: `Running on OS: '{osName}'`, - values: { osName }, - }) - ); + logger.debug(`Running on OS: '{osName}'`); if (disableSandbox === true) { logger.warn( - i18n.translate('xpack.reporting.serverConfig.autoSet.sandboxDisabled', { - defaultMessage: `Chromium sandbox provides an additional layer of protection, but is not supported for {osName} OS. Automatically setting '{configKey}: true'.`, - values: { - configKey: 'xpack.reporting.capture.browser.chromium.disableSandbox', - osName, - }, - }) + `Chromium sandbox provides an additional layer of protection, but is not supported for ${osName} OS.` + + ` Automatically setting 'xpack.reporting.capture.browser.chromium.disableSandbox: true'.` ); } else { logger.info( - i18n.translate('xpack.reporting.serverConfig.autoSet.sandboxEnabled', { - defaultMessage: `Chromium sandbox provides an additional layer of protection, and is supported for {osName} OS. Automatically enabling Chromium sandbox.`, - values: { osName }, - }) + `Chromium sandbox provides an additional layer of protection, and is supported for ${osName} OS.` + + ` Automatically enabling Chromium sandbox.` ); } From 5ca5a4dcdd947f7154d0cf5eb493bf91b998f022 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Tue, 2 Nov 2021 19:26:15 -0700 Subject: [PATCH 7/8] remove unused translations --- x-pack/plugins/translations/translations/ja-JP.json | 4 ---- x-pack/plugins/translations/translations/zh-CN.json | 4 ---- 2 files changed, 8 deletions(-) diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index 857e97daa3515..47876265894ec 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -19243,10 +19243,6 @@ "xpack.reporting.screenCapturePanelContent.canvasLayoutLabel": "全ページレイアウト", "xpack.reporting.screenCapturePanelContent.optimizeForPrintingHelpText": "複数のページを使用します。ページごとに最大2のビジュアライゼーションが表示されます", "xpack.reporting.screenCapturePanelContent.optimizeForPrintingLabel": "印刷用に最適化", - "xpack.reporting.serverConfig.autoSet.sandboxDisabled": "Chromiumサンドボックスは保護が強化されていますが、{osName} OSではサポートされていません。自動的に'{configKey}: true'を設定しています。", - "xpack.reporting.serverConfig.autoSet.sandboxEnabled": "Chromiumサンドボックスは保護が強化され、{osName} OSでサポートされています。自動的にChromiumサンドボックスを有効にしています。", - "xpack.reporting.serverConfig.osDetected": "OSは'{osName}'で実行しています", - "xpack.reporting.serverConfig.randomEncryptionKey": "xpack.reporting.encryptionKey のランダムキーを生成しています。再起動時にセッションが無効にならないようにするには、kibana.yml でxpack.reporting.encryptionKey を設定するか、bin/kibana-encryption-keys コマンドを使用してください。", "xpack.reporting.shareContextMenu.csvReportsButtonLabel": "CSV レポート", "xpack.reporting.shareContextMenu.pdfReportsButtonLabel": "PDF レポート", "xpack.reporting.shareContextMenu.pngReportsButtonLabel": "PNG レポート", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 6c860a5b2cc4d..0fd4fea956a11 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -19520,10 +19520,6 @@ "xpack.reporting.screenCapturePanelContent.canvasLayoutLabel": "全页面布局", "xpack.reporting.screenCapturePanelContent.optimizeForPrintingHelpText": "使用多页，每页最多显示 2 个可视化", "xpack.reporting.screenCapturePanelContent.optimizeForPrintingLabel": "打印优化", - "xpack.reporting.serverConfig.autoSet.sandboxDisabled": "Chromium 沙盒提供附加保护层，但不受 {osName} OS 支持。自动设置“{configKey}: true”。", - "xpack.reporting.serverConfig.autoSet.sandboxEnabled": "Chromium 沙盒提供附加保护层，受 {osName} OS 支持。自动启用 Chromium 沙盒。", - "xpack.reporting.serverConfig.osDetected": "正在以下 OS 上运行：“{osName}”", - "xpack.reporting.serverConfig.randomEncryptionKey": "为 xpack.reporting.encryptionKey 生成随机密钥。为防止会话在重启时失效，请在 kibana.yml 中设置 xpack.reporting.encryptionKey 或使用 bin/kibana-encryption-keys 命令。", "xpack.reporting.shareContextMenu.csvReportsButtonLabel": "CSV 报告", "xpack.reporting.shareContextMenu.pdfReportsButtonLabel": "PDF 报告", "xpack.reporting.shareContextMenu.pngReportsButtonLabel": "PNG 报告", From 82053bc37ffeeb5897ad5f451c496fe88f4ead23 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Tue, 2 Nov 2021 19:57:04 -0700 Subject: [PATCH 8/8] improve the tests --- .../server/config/create_config.test.ts | 67 ++++++------------- .../reporting/server/config/schema.test.ts | 53 ++++++--------- 2 files changed, 39 insertions(+), 81 deletions(-) diff --git a/x-pack/plugins/reporting/server/config/create_config.test.ts b/x-pack/plugins/reporting/server/config/create_config.test.ts index 38795a488a2e9..718638e1ba62b 100644 --- a/x-pack/plugins/reporting/server/config/create_config.test.ts +++ b/x-pack/plugins/reporting/server/config/create_config.test.ts @@ -147,51 +147,30 @@ describe('Reporting server createConfig$', () => { expect(mockLogger.warn.mock.calls.length).toBe(0); }); - describe('prevent invalid server hostnames', () => { - beforeEach(() => { + for (const hostname of [ + '0', + '0.0', + '0.0.0', + '0.0.0.0', + '0000:0000:0000:0000:0000:0000:0000:0000', + '::', + ]) { + it(`apply failover logic when hostname is given as ${hostname}`, async () => { mockInitContext = coreMock.createPluginInitializerContext( createMockConfigSchema({ encryptionKey: 'aaaaaaaaaaaaabbbbbbbbbbbbaaaaaaaaa', + // overwrite settings added by createMockConfigSchema and apply the default settings + // TODO make createMockConfigSchema _not_ default xpack.reporting.kibanaServer.hostname to 'localhost' kibanaServer: { hostname: undefined, port: undefined, - }, // overwrite settings added by createMockConfigSchema and apply the default settings - }) - ); - }); - - it(`apply failover logic to use "localhost" when "server.host" is "0.0.0.0"`, async () => { - mockCoreSetup.http.getServerInfo = jest.fn().mockImplementation( - (): HttpServerInfo => ({ - name: 'cool server', - hostname: '0.0.0.0', - port: 5601, - protocol: 'http', + }, }) ); - - const mockConfig$ = createMockConfig(mockInitContext); - const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise(); - - expect(result.kibanaServer).toMatchInlineSnapshot(` - Object { - "hostname": "localhost", - "port": 5601, - "protocol": "http", - } - `); - - expect(mockLogger.warn.mock.calls.length).toBe(1); - expect(mockLogger.warn.mock.calls[0]).toMatchObject([ - "Found 'server.host: \"0.0.0.0\"' in Kibana configuration. Reporting is not able to use this as the Kibana server hostname. To enable PNG/PDF Reporting to work, 'xpack.reporting.kibanaServer.hostname: localhost' is automatically set in the configuration. You can prevent this message by adding 'xpack.reporting.kibanaServer.hostname: localhost' in kibana.yml.", - ]); - }); - - it(`apply failover logic when using a variation of the "0" address`, async () => { mockCoreSetup.http.getServerInfo = jest.fn().mockImplementation( (): HttpServerInfo => ({ name: 'cool server', - hostname: '0.0', + hostname, port: 5601, protocol: 'http', }) @@ -199,19 +178,11 @@ describe('Reporting server createConfig$', () => { const mockConfig$ = createMockConfig(mockInitContext); const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise(); - - expect(result.kibanaServer).toMatchInlineSnapshot(` - Object { - "hostname": "localhost", - "port": 5601, - "protocol": "http", - } - `); - - expect(mockLogger.warn.mock.calls.length).toBe(1); - expect(mockLogger.warn.mock.calls[0]).toMatchObject([ - "Found 'server.host: \"0.0.0.0\"' in Kibana configuration. Reporting is not able to use this as the Kibana server hostname. To enable PNG/PDF Reporting to work, 'xpack.reporting.kibanaServer.hostname: localhost' is automatically set in the configuration. You can prevent this message by adding 'xpack.reporting.kibanaServer.hostname: localhost' in kibana.yml.", - ]); + expect(result.kibanaServer).toMatchObject({ + hostname: 'localhost', + port: 5601, + protocol: 'http', + }); }); - }); + } }); diff --git a/x-pack/plugins/reporting/server/config/schema.test.ts b/x-pack/plugins/reporting/server/config/schema.test.ts index e5035f3304dc0..7f1da5b55ccb6 100644 --- a/x-pack/plugins/reporting/server/config/schema.test.ts +++ b/x-pack/plugins/reporting/server/config/schema.test.ts @@ -288,38 +288,25 @@ describe('Reporting Config Schema', () => { `); }); - it(`logs the proper validation messages for invalid hostname`, () => { - // kibanaServer - const throwValidationErr = () => ConfigSchema.validate({ kibanaServer: { hostname: '0' } }); - expect(throwValidationErr).toThrowError( - `[kibanaServer.hostname]: value must be a valid hostname (see RFC 1123).` - ); - }); - - it(`logs the proper validation messages for hostname: 0.0.0.0`, () => { - // kibanaServer - const throwValidationErr = () => - ConfigSchema.validate({ kibanaServer: { hostname: '0.0.0.0' } }); - expect(throwValidationErr).toThrowError( - `[kibanaServer.hostname]: cannot use '0.0.0.0' as Kibana host name, consider using the default (localhost) instead` - ); - }); + for (const address of ['0', '0.0', '0.0.0']) { + it(`fails to validate "kibanaServer.hostname" with an invalid hostname: "${address}"`, () => { + expect(() => + ConfigSchema.validate({ + kibanaServer: { hostname: address }, + }) + ).toThrowError(`[kibanaServer.hostname]: value must be a valid hostname (see RFC 1123).`); + }); + } - it(`logs the proper validation messages for hostname set to a variation of the "0" address`, () => { - expect(() => ConfigSchema.validate({ kibanaServer: { hostname: '0.0.0' } })).toThrowError( - `[kibanaServer.hostname]: value must be a valid hostname (see RFC 1123).` - ); - - expect(() => - ConfigSchema.validate({ - kibanaServer: { hostname: '0000:0000:0000:0000:0000:0000:0000:0000' }, - }) - ).toThrowError( - `[kibanaServer.hostname]: cannot use '0.0.0.0' as Kibana host name, consider using the default (localhost) instead` - ); - - expect(() => ConfigSchema.validate({ kibanaServer: { hostname: '::' } })).toThrowError( - `[kibanaServer.hostname]: cannot use '0.0.0.0' as Kibana host name, consider using the default (localhost) instead` - ); - }); + for (const address of ['0.0.0.0', '0000:0000:0000:0000:0000:0000:0000:0000', '::']) { + it(`fails to validate "kibanaServer.hostname" hostname as zero: "${address}"`, () => { + expect(() => + ConfigSchema.validate({ + kibanaServer: { hostname: address }, + }) + ).toThrowError( + `[kibanaServer.hostname]: cannot use '0.0.0.0' as Kibana host name, consider using the default (localhost) instead` + ); + }); + } });