From 790736e00b4296290bd55f8732a1a2ad7c8e7291 Mon Sep 17 00:00:00 2001 From: Patrick Mueller Date: Wed, 24 Mar 2021 17:02:06 -0400 Subject: [PATCH] [actions] adds proxyBypassHosts and proxyOnlyHosts Kibana config keys resolves https://github.com/elastic/kibana/issues/92949 This PR adds two new Kibana config keys to further customize when the proxy is used when making HTTP requests. Prior to this PR, if a proxy was set via the `xpack.actions.proxyUrl` config key, all requests would be proxied. Now, there's a further refinement in that hostnames can be added to the `xpack.actions.proxyBypassHosts` and `xpack.actions.proxyBypassHosts` config keys. Only one of these config keys can be used at a time. If the target URL hostname of the HTTP request is listed in the `proxyBypassHosts` list, the proxy won't be used. If the target URL hostname of the HTTP request is **NOT** listed in the `proxyOnlyHosts` list, the proxy won't be used. Depending on the customer's environment, it may be easier to list the hosts to bypass, or easier to list the hosts that should only be proxied, so they can choose either method. --- .../actions/server/actions_client.test.ts | 2 + .../actions/server/actions_config.test.ts | 2 + .../plugins/actions/server/actions_config.ts | 7 +++ .../lib/axios_utils.test.ts | 12 +++- .../builtin_action_types/lib/axios_utils.ts | 2 +- .../lib/get_custom_agents.test.ts | 12 +++- .../lib/get_custom_agents.ts | 25 +++++++- .../lib/send_email.test.ts | 2 + .../server/builtin_action_types/slack.test.ts | 2 + .../server/builtin_action_types/slack.ts | 2 +- x-pack/plugins/actions/server/config.ts | 59 +++++++++++-------- x-pack/plugins/actions/server/types.ts | 2 + 12 files changed, 97 insertions(+), 32 deletions(-) diff --git a/x-pack/plugins/actions/server/actions_client.test.ts b/x-pack/plugins/actions/server/actions_client.test.ts index a333d86b27129ff..92d3b4f29d96758 100644 --- a/x-pack/plugins/actions/server/actions_client.test.ts +++ b/x-pack/plugins/actions/server/actions_client.test.ts @@ -406,6 +406,8 @@ describe('create()', () => { preconfigured: {}, proxyRejectUnauthorizedCertificates: true, rejectUnauthorized: true, + proxyBypassHosts: undefined, + proxyOnlyHosts: undefined, }); const localActionTypeRegistryParams = { diff --git a/x-pack/plugins/actions/server/actions_config.test.ts b/x-pack/plugins/actions/server/actions_config.test.ts index cae6777a82441fe..79d346a1c39cf82 100644 --- a/x-pack/plugins/actions/server/actions_config.test.ts +++ b/x-pack/plugins/actions/server/actions_config.test.ts @@ -19,6 +19,8 @@ const defaultActionsConfig: ActionsConfig = { preconfigured: {}, proxyRejectUnauthorizedCertificates: true, rejectUnauthorized: true, + proxyBypassHosts: undefined, + proxyOnlyHosts: undefined, }; describe('ensureUriAllowed', () => { diff --git a/x-pack/plugins/actions/server/actions_config.ts b/x-pack/plugins/actions/server/actions_config.ts index 2787f8f97110155..b724dcc55a9d510 100644 --- a/x-pack/plugins/actions/server/actions_config.ts +++ b/x-pack/plugins/actions/server/actions_config.ts @@ -93,11 +93,18 @@ function getProxySettingsFromConfig(config: ActionsConfig): undefined | ProxySet return { proxyUrl: config.proxyUrl, + proxyBypassHosts: arrayAsSet(config.proxyBypassHosts), + proxyOnlyHosts: arrayAsSet(config.proxyOnlyHosts), proxyHeaders: config.proxyHeaders, proxyRejectUnauthorizedCertificates: config.proxyRejectUnauthorizedCertificates, }; } +function arrayAsSet(arr: T[] | undefined): Set | undefined { + if (!arr) return; + return new Set(arr); +} + export function getActionsConfigurationUtilities( config: ActionsConfig ): ActionsConfigurationUtilities { diff --git a/x-pack/plugins/actions/server/builtin_action_types/lib/axios_utils.test.ts b/x-pack/plugins/actions/server/builtin_action_types/lib/axios_utils.test.ts index 6a67f4f6752c253..1c6451013ddbfe3 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/lib/axios_utils.test.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/lib/axios_utils.test.ts @@ -13,6 +13,8 @@ import { loggingSystemMock } from '../../../../../../src/core/server/mocks'; import { actionsConfigMock } from '../../actions_config.mock'; import { getCustomAgents } from './get_custom_agents'; +const TestUrl = 'https://elastic.co/foo/bar/baz'; + const logger = loggingSystemMock.create().get() as jest.Mocked; const configurationUtilities = actionsConfigMock.create(); jest.mock('axios'); @@ -66,17 +68,19 @@ describe('request', () => { configurationUtilities.getProxySettings.mockReturnValue({ proxyRejectUnauthorizedCertificates: true, proxyUrl: 'https://localhost:1212', + proxyBypassHosts: undefined, + proxyOnlyHosts: undefined, }); - const { httpAgent, httpsAgent } = getCustomAgents(configurationUtilities, logger); + const { httpAgent, httpsAgent } = getCustomAgents(configurationUtilities, logger, TestUrl); const res = await request({ axios, - url: 'http://testProxy', + url: TestUrl, logger, configurationUtilities, }); - expect(axiosMock).toHaveBeenCalledWith('http://testProxy', { + expect(axiosMock).toHaveBeenCalledWith(TestUrl, { method: 'get', data: {}, httpAgent, @@ -94,6 +98,8 @@ describe('request', () => { configurationUtilities.getProxySettings.mockReturnValue({ proxyUrl: ':nope:', proxyRejectUnauthorizedCertificates: false, + proxyBypassHosts: undefined, + proxyOnlyHosts: undefined, }); const res = await request({ axios, diff --git a/x-pack/plugins/actions/server/builtin_action_types/lib/axios_utils.ts b/x-pack/plugins/actions/server/builtin_action_types/lib/axios_utils.ts index f86f3b86c506af1..edce36909614260 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/lib/axios_utils.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/lib/axios_utils.ts @@ -30,7 +30,7 @@ export const request = async ({ validateStatus?: (status: number) => boolean; auth?: AxiosBasicCredentials; }): Promise => { - const { httpAgent, httpsAgent } = getCustomAgents(configurationUtilities, logger); + const { httpAgent, httpsAgent } = getCustomAgents(configurationUtilities, logger, url); return await axios(url, { ...rest, diff --git a/x-pack/plugins/actions/server/builtin_action_types/lib/get_custom_agents.test.ts b/x-pack/plugins/actions/server/builtin_action_types/lib/get_custom_agents.test.ts index 340ac0f6dda3a20..5b7c6ee06188d9c 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/lib/get_custom_agents.test.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/lib/get_custom_agents.test.ts @@ -14,6 +14,8 @@ import { loggingSystemMock } from '../../../../../../src/core/server/mocks'; import { actionsConfigMock } from '../../actions_config.mock'; const logger = loggingSystemMock.create().get() as jest.Mocked; +const targetUrl = 'https://elastic.co/foo/bar/baz'; + describe('getCustomAgents', () => { const configurationUtilities = actionsConfigMock.create(); @@ -21,8 +23,10 @@ describe('getCustomAgents', () => { configurationUtilities.getProxySettings.mockReturnValue({ proxyUrl: 'https://someproxyhost', proxyRejectUnauthorizedCertificates: false, + proxyBypassHosts: undefined, + proxyOnlyHosts: undefined, }); - const { httpAgent, httpsAgent } = getCustomAgents(configurationUtilities, logger); + const { httpAgent, httpsAgent } = getCustomAgents(configurationUtilities, logger, targetUrl); expect(httpAgent instanceof HttpProxyAgent).toBeTruthy(); expect(httpsAgent instanceof HttpsProxyAgent).toBeTruthy(); }); @@ -31,14 +35,16 @@ describe('getCustomAgents', () => { configurationUtilities.getProxySettings.mockReturnValue({ proxyUrl: ':nope: not a valid URL', proxyRejectUnauthorizedCertificates: false, + proxyBypassHosts: undefined, + proxyOnlyHosts: undefined, }); - const { httpAgent, httpsAgent } = getCustomAgents(configurationUtilities, logger); + const { httpAgent, httpsAgent } = getCustomAgents(configurationUtilities, logger, targetUrl); expect(httpAgent).toBe(undefined); expect(httpsAgent instanceof HttpsAgent).toBeTruthy(); }); test('return default agents for undefined proxy options', () => { - const { httpAgent, httpsAgent } = getCustomAgents(configurationUtilities, logger); + const { httpAgent, httpsAgent } = getCustomAgents(configurationUtilities, logger, targetUrl); expect(httpAgent).toBe(undefined); expect(httpsAgent instanceof HttpsAgent).toBeTruthy(); }); diff --git a/x-pack/plugins/actions/server/builtin_action_types/lib/get_custom_agents.ts b/x-pack/plugins/actions/server/builtin_action_types/lib/get_custom_agents.ts index 92ababf830aa733..ff2d005f4d84151 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/lib/get_custom_agents.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/lib/get_custom_agents.ts @@ -19,7 +19,8 @@ interface GetCustomAgentsResponse { export function getCustomAgents( configurationUtilities: ActionsConfigurationUtilities, - logger: Logger + logger: Logger, + url: string ): GetCustomAgentsResponse { const proxySettings = configurationUtilities.getProxySettings(); const defaultAgents = { @@ -33,6 +34,28 @@ export function getCustomAgents( return defaultAgents; } + let targetUrl: URL; + try { + targetUrl = new URL(url); + } catch (err) { + logger.warn(`error determining proxy state for invalid url "${url}", using default agents`); + return defaultAgents; + } + + // filter out hostnames in the proxy bypass or only lists + const { hostname } = targetUrl; + + if (proxySettings.proxyBypassHosts) { + if (proxySettings.proxyBypassHosts.has(hostname)) { + return defaultAgents; + } + } + + if (proxySettings.proxyOnlyHosts) { + if (!proxySettings.proxyOnlyHosts.has(hostname)) { + return defaultAgents; + } + } logger.debug(`Creating proxy agents for proxy: ${proxySettings.proxyUrl}`); let proxyUrl: URL; try { diff --git a/x-pack/plugins/actions/server/builtin_action_types/lib/send_email.test.ts b/x-pack/plugins/actions/server/builtin_action_types/lib/send_email.test.ts index cc3f03f50c36fec..3675ad5d2165fff 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/lib/send_email.test.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/lib/send_email.test.ts @@ -76,6 +76,8 @@ describe('send_email module', () => { { proxyUrl: 'https://example.com', proxyRejectUnauthorizedCertificates: false, + proxyBypassHosts: undefined, + proxyOnlyHosts: undefined, } ); diff --git a/x-pack/plugins/actions/server/builtin_action_types/slack.test.ts b/x-pack/plugins/actions/server/builtin_action_types/slack.test.ts index 6479e29b5a76ff6..a821c50d658dc50 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/slack.test.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/slack.test.ts @@ -195,6 +195,8 @@ describe('execute()', () => { configurationUtilities.getProxySettings.mockReturnValue({ proxyUrl: 'https://someproxyhost', proxyRejectUnauthorizedCertificates: false, + proxyBypassHosts: undefined, + proxyOnlyHosts: undefined, }); const actionTypeProxy = getActionType({ logger: mockedLogger, diff --git a/x-pack/plugins/actions/server/builtin_action_types/slack.ts b/x-pack/plugins/actions/server/builtin_action_types/slack.ts index a6173229e3267ac..7e55aafa87261e0 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/slack.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/slack.ts @@ -131,7 +131,7 @@ async function slackExecutor( const { message } = params; const proxySettings = configurationUtilities.getProxySettings(); - const customAgents = getCustomAgents(configurationUtilities, logger); + const customAgents = getCustomAgents(configurationUtilities, logger, webhookUrl); const agent = webhookUrl.toLowerCase().startsWith('https') ? customAgents.httpsAgent : customAgents.httpAgent; diff --git a/x-pack/plugins/actions/server/config.ts b/x-pack/plugins/actions/server/config.ts index b4f29b752957fba..1ff2d5f1ccc9562 100644 --- a/x-pack/plugins/actions/server/config.ts +++ b/x-pack/plugins/actions/server/config.ts @@ -15,32 +15,45 @@ const preconfiguredActionSchema = schema.object({ secrets: schema.recordOf(schema.string(), schema.any(), { defaultValue: {} }), }); -export const configSchema = schema.object({ - enabled: schema.boolean({ defaultValue: true }), - allowedHosts: schema.arrayOf( - schema.oneOf([schema.string({ hostname: true }), schema.literal(AllowedHosts.Any)]), - { - defaultValue: [AllowedHosts.Any], - } - ), - enabledActionTypes: schema.arrayOf( - schema.oneOf([schema.string(), schema.literal(EnabledActionTypes.Any)]), - { - defaultValue: [AllowedHosts.Any], - } - ), - preconfigured: schema.recordOf(schema.string(), preconfiguredActionSchema, { - defaultValue: {}, - validate: validatePreconfigured, - }), - proxyUrl: schema.maybe(schema.string()), - proxyHeaders: schema.maybe(schema.recordOf(schema.string(), schema.string())), - proxyRejectUnauthorizedCertificates: schema.boolean({ defaultValue: true }), - rejectUnauthorized: schema.boolean({ defaultValue: true }), -}); +export const configSchema = schema.object( + { + enabled: schema.boolean({ defaultValue: true }), + allowedHosts: schema.arrayOf( + schema.oneOf([schema.string({ hostname: true }), schema.literal(AllowedHosts.Any)]), + { + defaultValue: [AllowedHosts.Any], + } + ), + enabledActionTypes: schema.arrayOf( + schema.oneOf([schema.string(), schema.literal(EnabledActionTypes.Any)]), + { + defaultValue: [AllowedHosts.Any], + } + ), + preconfigured: schema.recordOf(schema.string(), preconfiguredActionSchema, { + defaultValue: {}, + validate: validatePreconfigured, + }), + proxyUrl: schema.maybe(schema.string()), + proxyHeaders: schema.maybe(schema.recordOf(schema.string(), schema.string())), + proxyRejectUnauthorizedCertificates: schema.boolean({ defaultValue: true }), + proxyBypassHosts: schema.maybe(schema.arrayOf(schema.string({ hostname: true }))), + proxyOnlyHosts: schema.maybe(schema.arrayOf(schema.string({ hostname: true }))), + rejectUnauthorized: schema.boolean({ defaultValue: true }), + }, + { validate } +); export type ActionsConfig = TypeOf; +function validate(actionsConfig_: unknown): string | undefined { + const actionsConfig = actionsConfig_ as ActionsConfig; + + if (actionsConfig.proxyBypassHosts && actionsConfig.proxyOnlyHosts) { + return `properties proxyBypassHosts and proxyOnlyHosts cannot be used together`; + } +} + const invalidActionIds = new Set(['', '__proto__', 'constructor']); function validatePreconfigured(preconfigured: Record): string | undefined { diff --git a/x-pack/plugins/actions/server/types.ts b/x-pack/plugins/actions/server/types.ts index 4e3916f5d6e231d..6830f013ade5f67 100644 --- a/x-pack/plugins/actions/server/types.ts +++ b/x-pack/plugins/actions/server/types.ts @@ -133,6 +133,8 @@ export interface ActionTaskExecutorParams { export interface ProxySettings { proxyUrl: string; + proxyBypassHosts: Set | undefined; + proxyOnlyHosts: Set | undefined; proxyHeaders?: Record; proxyRejectUnauthorizedCertificates: boolean; }