Skip to content

Commit

Permalink
adds whitelisting to slack and pagerduty action types (#52989)
Browse files Browse the repository at this point in the history
The Slack and Pagerduty actions currently do not do whitelist validation, this PR adds the requirement for the PD and Slack action's respective target URLs to also be whitelisted.
  • Loading branch information
gmmorris authored Dec 16, 2019
1 parent 866a387 commit a057456
Show file tree
Hide file tree
Showing 12 changed files with 251 additions and 79 deletions.
10 changes: 8 additions & 2 deletions x-pack/legacy/plugins/actions/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,14 @@ Built-In-Actions are configured using the _xpack.actions_ namespoace under _kiba

| Namespaced Key | Description | Type |
| ------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------- |
| _xpack.actions._**enabled** | Feature toggle which enabled Actions in Kibana. | boolean |
| _xpack.actions._**WhitelistedHosts** | Which _hostnames_ are whitelisted for the Built-In-Action? This list should contain hostnames of every external service you wish to interact with using Webhooks, Email or any other built in Action. Note that you may use the string "\*" in place of a specific hostname to enable Kibana to target any URL, but keep in mind the potential use of such a feature to execute [SSRF](https://www.owasp.org/index.php/Server_Side_Request_Forgery) attacks from your server. | Array<String> |
| _xpack.actions._**enabled** | Feature toggle which enabled Actions in Kibana. | boolean |
| _xpack.actions._**whitelistedHosts** | Which _hostnames_ are whitelisted for the Built-In-Action? This list should contain hostnames of every external service you wish to interact with using Webhooks, Email or any other built in Action. Note that you may use the string "\*" in place of a specific hostname to enable Kibana to target any URL, but keep in mind the potential use of such a feature to execute [SSRF](https://www.owasp.org/index.php/Server_Side_Request_Forgery) attacks from your server. | Array<String> |

#### Whitelisting Built-in Action Types
It is worth noting that the **whitelistedHosts** configuation applies to built-in action types (such as Slack, or PagerDuty) as well.

Uniquely, the _PagerDuty Action Type_ has been configured to support the service's Events API (at _https://events.pagerduty.com/v2/enqueue_, which you can read about [here](https://v2.developer.pagerduty.com/docs/events-api-v2)) as a default, but this too, must be included in the whitelist before the PagerDuty action can be used.


### Configuration Utilities

Expand Down
14 changes: 14 additions & 0 deletions x-pack/legacy/plugins/actions/server/actions_config.mock.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/*
* 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 { ActionsConfigurationUtilities } from './actions_config';

export const configUtilsMock: ActionsConfigurationUtilities = {
isWhitelistedHostname: _ => true,
isWhitelistedUri: _ => true,
ensureWhitelistedHostname: _ => {},
ensureWhitelistedUri: _ => {},
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,14 @@
*/

import { ActionExecutor, TaskRunnerFactory } from '../lib';
import { ActionsConfigurationUtilities } from '../actions_config';
import { ActionTypeRegistry } from '../action_type_registry';
import { taskManagerMock } from '../../../task_manager/task_manager.mock';
import { registerBuiltInActionTypes } from './index';
import { Logger } from '../../../../../../src/core/server';
import { loggingServiceMock } from '../../../../../../src/core/server/mocks';
import { configUtilsMock } from '../actions_config.mock';

const ACTION_TYPE_IDS = ['.index', '.email', '.pagerduty', '.server-log', '.slack', '.webhook'];
const MOCK_KIBANA_CONFIG_UTILS: ActionsConfigurationUtilities = {
isWhitelistedHostname: _ => true,
isWhitelistedUri: _ => true,
ensureWhitelistedHostname: _ => {},
ensureWhitelistedUri: _ => {},
};

export function createActionTypeRegistry(): {
logger: jest.Mocked<Logger>;
Expand All @@ -32,7 +26,7 @@ export function createActionTypeRegistry(): {
registerBuiltInActionTypes({
logger,
actionTypeRegistry,
actionsConfigUtils: MOCK_KIBANA_CONFIG_UTILS,
actionsConfigUtils: configUtilsMock,
});
return { logger, actionTypeRegistry };
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,16 @@ import { getActionType as getWebhookActionType } from './webhook';
export function registerBuiltInActionTypes({
logger,
actionTypeRegistry,
actionsConfigUtils,
actionsConfigUtils: configurationUtilities,
}: {
logger: Logger;
actionTypeRegistry: ActionTypeRegistry;
actionsConfigUtils: ActionsConfigurationUtilities;
}) {
actionTypeRegistry.register(getServerLogActionType({ logger }));
actionTypeRegistry.register(getSlackActionType());
actionTypeRegistry.register(
getEmailActionType({ logger, configurationUtilities: actionsConfigUtils })
);
actionTypeRegistry.register(getSlackActionType({ configurationUtilities }));
actionTypeRegistry.register(getEmailActionType({ logger, configurationUtilities }));
actionTypeRegistry.register(getIndexActionType({ logger }));
actionTypeRegistry.register(getPagerDutyActionType({ logger }));
actionTypeRegistry.register(
getWebhookActionType({ logger, configurationUtilities: actionsConfigUtils })
);
actionTypeRegistry.register(getPagerDutyActionType({ logger, configurationUtilities }));
actionTypeRegistry.register(getWebhookActionType({ logger, configurationUtilities }));
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,14 @@ jest.mock('./lib/post_pagerduty', () => ({
postPagerduty: jest.fn(),
}));

import { getActionType } from './pagerduty';
import { ActionType, Services, ActionTypeExecutorOptions } from '../types';
import { validateConfig, validateSecrets, validateParams } from '../lib';
import { savedObjectsClientMock } from '../../../../../../src/core/server/mocks';
import { postPagerduty } from './lib/post_pagerduty';
import { createActionTypeRegistry } from './index.test';
import { Logger } from '../../../../../../src/core/server';
import { configUtilsMock } from '../actions_config.mock';

const postPagerdutyMock = postPagerduty as jest.Mock;

Expand All @@ -24,10 +27,12 @@ const services: Services = {
};

let actionType: ActionType;
let mockedLogger: jest.Mocked<Logger>;

beforeAll(() => {
const { actionTypeRegistry } = createActionTypeRegistry();
const { logger, actionTypeRegistry } = createActionTypeRegistry();
actionType = actionTypeRegistry.get(ACTION_TYPE_ID);
mockedLogger = logger;
});

describe('get()', () => {
Expand All @@ -50,6 +55,40 @@ describe('validateConfig()', () => {
`"error validating action type config: [shouldNotBeHere]: definition for this key is missing"`
);
});

test('should validate and pass when the pagerduty url is whitelisted', () => {
actionType = getActionType({
logger: mockedLogger,
configurationUtilities: {
...configUtilsMock,
ensureWhitelistedUri: url => {
expect(url).toEqual('https://events.pagerduty.com/v2/enqueue');
},
},
});

expect(
validateConfig(actionType, { apiUrl: 'https://events.pagerduty.com/v2/enqueue' })
).toEqual({ apiUrl: 'https://events.pagerduty.com/v2/enqueue' });
});

test('config validation returns an error if the specified URL isnt whitelisted', () => {
actionType = getActionType({
logger: mockedLogger,
configurationUtilities: {
...configUtilsMock,
ensureWhitelistedUri: _ => {
throw new Error(`target url is not whitelisted`);
},
},
});

expect(() => {
validateConfig(actionType, { apiUrl: 'https://events.pagerduty.com/v2/enqueue' });
}).toThrowErrorMatchingInlineSnapshot(
`"error validating action type config: error configuring pagerduty action: target url is not whitelisted"`
);
});
});

describe('validateSecrets()', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { schema, TypeOf } from '@kbn/config-schema';
import { postPagerduty } from './lib/post_pagerduty';
import { Logger } from '../../../../../../src/core/server';
import { ActionType, ActionTypeExecutorOptions, ActionTypeExecutorResult } from '../types';
import { ActionsConfigurationUtilities } from '../actions_config';

// uses the PagerDuty Events API v2
// https://v2.developer.pagerduty.com/docs/events-api-v2
Expand All @@ -19,10 +20,10 @@ const PAGER_DUTY_API_URL = 'https://events.pagerduty.com/v2/enqueue';

export type ActionTypeConfigType = TypeOf<typeof ConfigSchema>;

const ConfigSchema = schema.object({
const configSchemaProps = {
apiUrl: schema.nullable(schema.string()),
});

};
const ConfigSchema = schema.object(configSchemaProps);
// secrets definition

export type ActionTypeSecretsType = TypeOf<typeof SecretsSchema>;
Expand Down Expand Up @@ -86,19 +87,47 @@ function validateParams(paramsObject: any): string | void {
}

// action type definition
export function getActionType({ logger }: { logger: Logger }): ActionType {
export function getActionType({
logger,
configurationUtilities,
}: {
logger: Logger;
configurationUtilities: ActionsConfigurationUtilities;
}): ActionType {
return {
id: '.pagerduty',
name: 'pagerduty',
validate: {
config: ConfigSchema,
config: schema.object(configSchemaProps, {
validate: curry(valdiateActionTypeConfig)(configurationUtilities),
}),
secrets: SecretsSchema,
params: ParamsSchema,
},
executor: curry(executor)({ logger }),
};
}

function valdiateActionTypeConfig(
configurationUtilities: ActionsConfigurationUtilities,
configObject: ActionTypeConfigType
) {
try {
configurationUtilities.ensureWhitelistedUri(getPagerDutyApiUrl(configObject));
} catch (whitelistError) {
return i18n.translate('xpack.actions.builtin.pagerduty.pagerdutyConfigurationError', {
defaultMessage: 'error configuring pagerduty action: {message}',
values: {
message: whitelistError.message,
},
});
}
}

function getPagerDutyApiUrl(config: ActionTypeConfigType): string {
return config.apiUrl || PAGER_DUTY_API_URL;
}

// action executor

async function executor(
Expand All @@ -111,7 +140,7 @@ async function executor(
const params = execOptions.params as ActionParamsType;
const services = execOptions.services;

const apiUrl = config.apiUrl || PAGER_DUTY_API_URL;
const apiUrl = getPagerDutyApiUrl(config);
const headers = {
'Content-Type': 'application/json',
'X-Routing-Key': secrets.routingKey,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@
*/

import { ActionType, Services, ActionTypeExecutorOptions } from '../types';
import { ActionTypeRegistry } from '../action_type_registry';
import { savedObjectsClientMock } from '../../../../../../src/core/server/mocks';
import { ActionExecutor, validateParams, validateSecrets, TaskRunnerFactory } from '../lib';
import { validateParams, validateSecrets } from '../lib';
import { getActionType } from './slack';
import { taskManagerMock } from '../../../task_manager/task_manager.mock';
import { configUtilsMock } from '../actions_config.mock';

const ACTION_TYPE_ID = '.slack';

Expand All @@ -18,47 +17,19 @@ const services: Services = {
savedObjectsClient: savedObjectsClientMock.create(),
};

let actionTypeRegistry: ActionTypeRegistry;
let actionType: ActionType;

async function mockSlackExecutor(options: ActionTypeExecutorOptions): Promise<any> {
const { params } = options;
const { message } = params;
if (message == null) throw new Error('message property required in parameter');

const failureMatch = message.match(/^failure: (.*)$/);
if (failureMatch != null) {
const failMessage = failureMatch[1];
throw new Error(`slack mockExecutor failure: ${failMessage}`);
}

return {
text: `slack mockExecutor success: ${message}`,
};
}

beforeAll(() => {
actionTypeRegistry = new ActionTypeRegistry({
taskManager: taskManagerMock.create(),
taskRunnerFactory: new TaskRunnerFactory(new ActionExecutor()),
});
actionTypeRegistry.register(getActionType({ executor: mockSlackExecutor }));
actionType = actionTypeRegistry.get(ACTION_TYPE_ID);

test('ensure action type is valid', () => {
expect(actionType).toBeTruthy();
actionType = getActionType({
async executor(options: ActionTypeExecutorOptions): Promise<any> {},
configurationUtilities: configUtilsMock,
});
});

describe('action is registered', () => {
test('gets registered with builtin actions', () => {
expect(actionTypeRegistry.has(ACTION_TYPE_ID)).toEqual(true);
});

describe('action registeration', () => {
test('returns action type', () => {
const returnedActionType = actionTypeRegistry.get(ACTION_TYPE_ID);
expect(returnedActionType.id).toEqual(ACTION_TYPE_ID);
expect(returnedActionType.name).toEqual('slack');
expect(actionType.id).toEqual(ACTION_TYPE_ID);
expect(actionType.name).toEqual('slack');
});
});

Expand Down Expand Up @@ -104,9 +75,64 @@ describe('validateActionTypeSecrets()', () => {
`"error validating action type secrets: [webhookUrl]: expected value of type [string] but got [number]"`
);
});

test('should validate and pass when the slack webhookUrl is whitelisted', () => {
actionType = getActionType({
configurationUtilities: {
...configUtilsMock,
ensureWhitelistedUri: url => {
expect(url).toEqual('https://api.slack.com/');
},
},
});

expect(validateSecrets(actionType, { webhookUrl: 'https://api.slack.com/' })).toEqual({
webhookUrl: 'https://api.slack.com/',
});
});

test('config validation returns an error if the specified URL isnt whitelisted', () => {
actionType = getActionType({
configurationUtilities: {
...configUtilsMock,
ensureWhitelistedUri: url => {
throw new Error(`target url is not whitelisted`);
},
},
});

expect(() => {
validateSecrets(actionType, { webhookUrl: 'https://api.slack.com/' });
}).toThrowErrorMatchingInlineSnapshot(
`"error validating action type secrets: error configuring slack action: target url is not whitelisted"`
);
});
});

describe('execute()', () => {
beforeAll(() => {
async function mockSlackExecutor(options: ActionTypeExecutorOptions): Promise<any> {
const { params } = options;
const { message } = params;
if (message == null) throw new Error('message property required in parameter');

const failureMatch = message.match(/^failure: (.*)$/);
if (failureMatch != null) {
const failMessage = failureMatch[1];
throw new Error(`slack mockExecutor failure: ${failMessage}`);
}

return {
text: `slack mockExecutor success: ${message}`,
};
}

actionType = getActionType({
executor: mockSlackExecutor,
configurationUtilities: configUtilsMock,
});
});

test('calls the mock executor with success', async () => {
const response = await actionType.executor({
actionId: 'some-id',
Expand Down
Loading

0 comments on commit a057456

Please sign in to comment.