From 8fe204fcab5926ecd28b1ebfad1e62dd035ba822 Mon Sep 17 00:00:00 2001 From: Patrick Mueller Date: Tue, 12 Jan 2021 11:54:40 -0500 Subject: [PATCH] [actions] fixes action proxies to set the right agent based on target url (#86415) Previously, the http and https proxy agents used by actions were created based on the protocol of the proxy URL itself - if the proxy was an http URL, both the generated http and https agents supplied to axios were actually both http proxy agents; if the proxy was an https URL, both the generated http and https agents supplied to axios were both https proxy agents. This PR changes so that both an http and https proxy agent are created and assigned as the appropriate agents for axios. Similar changes were made to the slack action, which does not directly use axios. --- .../lib/axios_utils.test.ts | 50 ++++++-- .../builtin_action_types/lib/axios_utils.ts | 12 +- .../lib/get_proxy_agent.test.ts | 30 ----- .../lib/get_proxy_agent.ts | 31 ----- .../lib/get_proxy_agents.test.ts | 44 +++++++ .../lib/get_proxy_agents.ts | 52 ++++++++ .../server/builtin_action_types/slack.ts | 16 +-- .../server/manual_tests/forward_proxy.js | 112 ++++++++++++++++++ 8 files changed, 267 insertions(+), 80 deletions(-) delete mode 100644 x-pack/plugins/actions/server/builtin_action_types/lib/get_proxy_agent.test.ts delete mode 100644 x-pack/plugins/actions/server/builtin_action_types/lib/get_proxy_agent.ts create mode 100644 x-pack/plugins/actions/server/builtin_action_types/lib/get_proxy_agents.test.ts create mode 100644 x-pack/plugins/actions/server/builtin_action_types/lib/get_proxy_agents.ts create mode 100644 x-pack/plugins/actions/server/manual_tests/forward_proxy.js 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 7e938e766657..32e1b233274c 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 @@ -5,10 +5,11 @@ */ import axios from 'axios'; -import HttpProxyAgent from 'http-proxy-agent'; import { Logger } from '../../../../../../src/core/server'; import { addTimeZoneToDate, request, patch, getErrorMessage } from './axios_utils'; import { loggingSystemMock } from '../../../../../../src/core/server/mocks'; +import { getProxyAgents } from './get_proxy_agents'; + const logger = loggingSystemMock.create().get() as jest.Mocked; jest.mock('axios'); const axiosMock = (axios as unknown) as jest.Mock; @@ -27,6 +28,7 @@ describe('addTimeZoneToDate', () => { describe('request', () => { beforeEach(() => { + jest.resetAllMocks(); axiosMock.mockImplementation(() => ({ status: 200, headers: { 'content-type': 'application/json' }, @@ -58,23 +60,57 @@ describe('request', () => { }); }); - test('it have been called with proper proxy agent', async () => { + test('it have been called with proper proxy agent for a valid url', async () => { + const proxySettings = { + proxyRejectUnauthorizedCertificates: true, + proxyUrl: 'https://localhost:1212', + }; + const { httpAgent, httpsAgent } = getProxyAgents(proxySettings, logger); + const res = await request({ axios, - url: '/testProxy', + url: 'http://testProxy', logger, proxySettings: { - proxyUrl: 'http://localhost:1212', + proxyUrl: 'https://localhost:1212', + proxyRejectUnauthorizedCertificates: true, + }, + }); + + expect(axiosMock).toHaveBeenCalledWith('http://testProxy', { + method: 'get', + data: {}, + headers: undefined, + httpAgent, + httpsAgent, + params: undefined, + proxy: false, + validateStatus: undefined, + }); + expect(res).toEqual({ + status: 200, + headers: { 'content-type': 'application/json' }, + data: { incidentId: '123' }, + }); + }); + + test('it have been called with proper proxy agent for an invalid url', async () => { + const res = await request({ + axios, + url: 'https://testProxy', + logger, + proxySettings: { + proxyUrl: ':nope:', proxyRejectUnauthorizedCertificates: false, }, }); - expect(axiosMock).toHaveBeenCalledWith('/testProxy', { + expect(axiosMock).toHaveBeenCalledWith('https://testProxy', { method: 'get', data: {}, headers: undefined, - httpAgent: new HttpProxyAgent('http://localhost:1212'), - httpsAgent: new HttpProxyAgent('http://localhost:1212'), + httpAgent: undefined, + httpsAgent: undefined, params: undefined, proxy: false, validateStatus: undefined, 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 e26a3b686179..322da1077af1 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 @@ -7,7 +7,7 @@ import { AxiosInstance, Method, AxiosResponse, AxiosBasicCredentials } from 'axios'; import { Logger } from '../../../../../../src/core/server'; import { ProxySettings } from '../../types'; -import { getProxyAgent } from './get_proxy_agent'; +import { getProxyAgents } from './get_proxy_agents'; export const request = async ({ axios, @@ -32,15 +32,17 @@ export const request = async ({ validateStatus?: (status: number) => boolean; auth?: AxiosBasicCredentials; }): Promise => { + const { httpAgent, httpsAgent } = getProxyAgents(proxySettings, logger); + return await axios(url, { method, data: data ?? {}, params, auth, - // use httpsAgent and embedded proxy: false, to be able to handle fail on invalid certs - httpsAgent: proxySettings ? getProxyAgent(proxySettings, logger) : undefined, - httpAgent: proxySettings ? getProxyAgent(proxySettings, logger) : undefined, - proxy: false, // the same way as it done for IncomingWebhook in + // use httpAgent and httpsAgent and set axios proxy: false, to be able to handle fail on invalid certs + httpAgent, + httpsAgent, + proxy: false, headers, validateStatus, }); diff --git a/x-pack/plugins/actions/server/builtin_action_types/lib/get_proxy_agent.test.ts b/x-pack/plugins/actions/server/builtin_action_types/lib/get_proxy_agent.test.ts deleted file mode 100644 index 8623a67e8a68..000000000000 --- a/x-pack/plugins/actions/server/builtin_action_types/lib/get_proxy_agent.test.ts +++ /dev/null @@ -1,30 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import HttpProxyAgent from 'http-proxy-agent'; -import { HttpsProxyAgent } from 'https-proxy-agent'; -import { Logger } from '../../../../../../src/core/server'; -import { getProxyAgent } from './get_proxy_agent'; -import { loggingSystemMock } from '../../../../../../src/core/server/mocks'; -const logger = loggingSystemMock.create().get() as jest.Mocked; - -describe('getProxyAgent', () => { - test('return HttpsProxyAgent for https proxy url', () => { - const agent = getProxyAgent( - { proxyUrl: 'https://someproxyhost', proxyRejectUnauthorizedCertificates: false }, - logger - ); - expect(agent instanceof HttpsProxyAgent).toBeTruthy(); - }); - - test('return HttpProxyAgent for http proxy url', () => { - const agent = getProxyAgent( - { proxyUrl: 'http://someproxyhost', proxyRejectUnauthorizedCertificates: false }, - logger - ); - expect(agent instanceof HttpProxyAgent).toBeTruthy(); - }); -}); diff --git a/x-pack/plugins/actions/server/builtin_action_types/lib/get_proxy_agent.ts b/x-pack/plugins/actions/server/builtin_action_types/lib/get_proxy_agent.ts deleted file mode 100644 index 957d31546b01..000000000000 --- a/x-pack/plugins/actions/server/builtin_action_types/lib/get_proxy_agent.ts +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import HttpProxyAgent from 'http-proxy-agent'; -import { HttpsProxyAgent } from 'https-proxy-agent'; -import { Logger } from '../../../../../../src/core/server'; -import { ProxySettings } from '../../types'; - -export function getProxyAgent( - proxySettings: ProxySettings, - logger: Logger -): HttpsProxyAgent | HttpProxyAgent { - logger.debug(`Create proxy agent for ${proxySettings.proxyUrl}.`); - - if (/^https/i.test(proxySettings.proxyUrl)) { - const proxyUrl = new URL(proxySettings.proxyUrl); - return new HttpsProxyAgent({ - host: proxyUrl.hostname, - port: Number(proxyUrl.port), - protocol: proxyUrl.protocol, - headers: proxySettings.proxyHeaders, - // do not fail on invalid certs if value is false - rejectUnauthorized: proxySettings.proxyRejectUnauthorizedCertificates, - }); - } else { - return new HttpProxyAgent(proxySettings.proxyUrl); - } -} diff --git a/x-pack/plugins/actions/server/builtin_action_types/lib/get_proxy_agents.test.ts b/x-pack/plugins/actions/server/builtin_action_types/lib/get_proxy_agents.test.ts new file mode 100644 index 000000000000..759ca9296826 --- /dev/null +++ b/x-pack/plugins/actions/server/builtin_action_types/lib/get_proxy_agents.test.ts @@ -0,0 +1,44 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import HttpProxyAgent from 'http-proxy-agent'; +import { HttpsProxyAgent } from 'https-proxy-agent'; +import { Logger } from '../../../../../../src/core/server'; +import { getProxyAgents } from './get_proxy_agents'; +import { loggingSystemMock } from '../../../../../../src/core/server/mocks'; +const logger = loggingSystemMock.create().get() as jest.Mocked; + +describe('getProxyAgents', () => { + test('get agents for valid proxy URL', () => { + const { httpAgent, httpsAgent } = getProxyAgents( + { proxyUrl: 'https://someproxyhost', proxyRejectUnauthorizedCertificates: false }, + logger + ); + expect(httpAgent instanceof HttpProxyAgent).toBeTruthy(); + expect(httpsAgent instanceof HttpsProxyAgent).toBeTruthy(); + }); + + test('return undefined agents for invalid proxy URL', () => { + const { httpAgent, httpsAgent } = getProxyAgents( + { proxyUrl: ':nope: not a valid URL', proxyRejectUnauthorizedCertificates: false }, + logger + ); + expect(httpAgent).toBe(undefined); + expect(httpsAgent).toBe(undefined); + }); + + test('return undefined agents for null proxy options', () => { + const { httpAgent, httpsAgent } = getProxyAgents(null, logger); + expect(httpAgent).toBe(undefined); + expect(httpsAgent).toBe(undefined); + }); + + test('return undefined agents for undefined proxy options', () => { + const { httpAgent, httpsAgent } = getProxyAgents(undefined, logger); + expect(httpAgent).toBe(undefined); + expect(httpsAgent).toBe(undefined); + }); +}); diff --git a/x-pack/plugins/actions/server/builtin_action_types/lib/get_proxy_agents.ts b/x-pack/plugins/actions/server/builtin_action_types/lib/get_proxy_agents.ts new file mode 100644 index 000000000000..45f962429ad2 --- /dev/null +++ b/x-pack/plugins/actions/server/builtin_action_types/lib/get_proxy_agents.ts @@ -0,0 +1,52 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { Agent } from 'http'; +import HttpProxyAgent from 'http-proxy-agent'; +import { HttpsProxyAgent } from 'https-proxy-agent'; +import { Logger } from '../../../../../../src/core/server'; +import { ProxySettings } from '../../types'; + +interface GetProxyAgentsResponse { + httpAgent: Agent | undefined; + httpsAgent: Agent | undefined; +} + +export function getProxyAgents( + proxySettings: ProxySettings | undefined | null, + logger: Logger +): GetProxyAgentsResponse { + const undefinedResponse = { + httpAgent: undefined, + httpsAgent: undefined, + }; + + if (!proxySettings) { + return undefinedResponse; + } + + logger.debug(`Creating proxy agents for proxy: ${proxySettings.proxyUrl}`); + let proxyUrl: URL; + try { + proxyUrl = new URL(proxySettings.proxyUrl); + } catch (err) { + logger.warn(`invalid proxy URL "${proxySettings.proxyUrl}" ignored`); + return undefinedResponse; + } + + const httpAgent = new HttpProxyAgent(proxySettings.proxyUrl); + const httpsAgent = (new HttpsProxyAgent({ + host: proxyUrl.hostname, + port: Number(proxyUrl.port), + protocol: proxyUrl.protocol, + headers: proxySettings.proxyHeaders, + // do not fail on invalid certs if value is false + rejectUnauthorized: proxySettings.proxyRejectUnauthorizedCertificates, + }) as unknown) as Agent; + // vsCode wasn't convinced HttpsProxyAgent is an http.Agent, so we convinced it + + return { httpAgent, httpsAgent }; +} 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 c9a3c39afd04..07ea7b62f360 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/slack.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/slack.ts @@ -6,8 +6,7 @@ import { URL } from 'url'; import { curry } from 'lodash'; -import { HttpsProxyAgent } from 'https-proxy-agent'; -import HttpProxyAgent from 'http-proxy-agent'; +import { Agent } from 'http'; import { i18n } from '@kbn/i18n'; import { schema, TypeOf } from '@kbn/config-schema'; import { IncomingWebhook, IncomingWebhookResult } from '@slack/webhook'; @@ -24,7 +23,7 @@ import { ExecutorType, } from '../types'; import { ActionsConfigurationUtilities } from '../actions_config'; -import { getProxyAgent } from './lib/get_proxy_agent'; +import { getProxyAgents } from './lib/get_proxy_agents'; export type SlackActionType = ActionType<{}, ActionTypeSecretsType, ActionParamsType, unknown>; export type SlackActionTypeExecutorOptions = ActionTypeExecutorOptions< @@ -128,9 +127,13 @@ async function slackExecutor( const { webhookUrl } = secrets; const { message } = params; - let proxyAgent: HttpsProxyAgent | HttpProxyAgent | undefined; + let httpProxyAgent: Agent | undefined; if (execOptions.proxySettings) { - proxyAgent = getProxyAgent(execOptions.proxySettings, logger); + const httpProxyAgents = getProxyAgents(execOptions.proxySettings, logger); + httpProxyAgent = webhookUrl.toLowerCase().startsWith('https') + ? httpProxyAgents.httpsAgent + : httpProxyAgents.httpAgent; + logger.debug(`IncomingWebhook was called with proxyUrl ${execOptions.proxySettings.proxyUrl}`); } @@ -138,8 +141,7 @@ async function slackExecutor( // https://slack.dev/node-slack-sdk/webhook // node-slack-sdk use Axios inside :) const webhook = new IncomingWebhook(webhookUrl, { - // @ts-expect-error The types exposed by 'HttpsProxyAgent' isn't up to date with 'Agent' - agent: proxyAgent, + agent: httpProxyAgent, }); result = await webhook.send(message); } catch (err) { diff --git a/x-pack/plugins/actions/server/manual_tests/forward_proxy.js b/x-pack/plugins/actions/server/manual_tests/forward_proxy.js new file mode 100644 index 000000000000..5c48b9d2393a --- /dev/null +++ b/x-pack/plugins/actions/server/manual_tests/forward_proxy.js @@ -0,0 +1,112 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +/* +This module implements two forward http proxies, http on 8080 and https on 8443, +which can be used with the config xpack.actions.proxyUrl to emulate customers +using forward proxies with Kibana actions. You can use either the http or https +versions, both can forward proxy http and https traffic: + + xpack.actions.proxyUrl: http://localhost:8080 + OR + xpack.actions.proxyUrl: https://localhost:8443 + +When using the https-based version, you may need to set the following option +as well: + + xpack.actions.rejectUnauthorized: false + +If the server you are connecting to via the proxy is https and has self-signed +certificates, you'll also need to set + + xpack.actions.proxyRejectUnauthorizedCertificates: false +*/ + +const HTTP_PORT = 8080; +const HTTPS_PORT = 8443; + +// starts http and https proxies to use to test actions within Kibana + +const fs = require('fs'); +const net = require('net'); +const url = require('url'); +const http = require('http'); +const https = require('https'); +const httpProxy = require('http-proxy'); + +const httpsOptions = { + key: fs.readFileSync('packages/kbn-dev-utils/certs/kibana.key', 'utf8'), + cert: fs.readFileSync('packages/kbn-dev-utils/certs/kibana.crt', 'utf8'), +}; + +const proxy = httpProxy.createServer(); + +createServer('http', HTTP_PORT); +createServer('https', HTTPS_PORT); + +function createServer(protocol, port) { + let httpServer; + + if (protocol === 'http') { + httpServer = http.createServer(); + } else { + httpServer = https.createServer(httpsOptions); + } + + httpServer.on('request', httpRequest); + httpServer.on('connect', httpsRequest); + httpServer.listen(port); + log(`proxy server started: ${protocol}:/localhost:${port}`); + + // handle http requests + function httpRequest(req, res) { + log(`${protocol} server: request for: ${req.url}`); + const parsedUrl = url.parse(req.url); + if (parsedUrl.hostname == null) { + res.writeHead(200, { 'Content-Type': 'text/plain' }); + res.end('this is a proxy server'); + return; + } + const target = parsedUrl.protocol + '//' + parsedUrl.hostname; + proxy.web(req, res, { target: target, secure: false }); + } + + // handle https requests + // see: https://nodejs.org/dist/latest-v14.x/docs/api/http.html#http_event_connect + function httpsRequest(req, socket, head) { + log(`${protocol} proxy server: request for target: https://${req.url}`); + const serverUrl = url.parse('https://' + req.url); + const serverSocket = net.connect(serverUrl.port, serverUrl.hostname, () => { + socket.write('HTTP/1.1 200 Connection Established\r\nProxy-agent: Node-Proxy\r\n\r\n'); + serverSocket.write(head); + serverSocket.pipe(socket); + socket.pipe(serverSocket); + }); + socket.on('error', (err) => { + log(`error on socket to proxy: ${err}`); + socket.destroy(); + serverSocket.destroy(); + }); + serverSocket.on('error', (err) => { + log(`error on socket to target: ${err}`); + socket.destroy(); + serverSocket.destroy(); + }); + } +} + +function log(message) { + console.log(`${new Date().toISOString()} - ${message}`); +} + +/* +Test with: + +curl -v -k --proxy-insecure -x http://127.0.0.1:8080 http://www.google.com +curl -v -k --proxy-insecure -x http://127.0.0.1:8080 https://www.google.com +curl -v -k --proxy-insecure -x https://127.0.0.1:8443 http://www.google.com +curl -v -k --proxy-insecure -x https://127.0.0.1:8443 https://www.google.com +*/