Skip to content

Commit

Permalink
[7.11] [Security Solution] [Detections] Add "read index" privilege ch…
Browse files Browse the repository at this point in the history
…eck on rule execution (#83134) (#86850)

* adds privilege check in rule execution function, need to abstract these lines into a util function to be used in create rules and use that check on the UI too

* fixes tests

* cleanup code, adds a unit test

* set rule to failure status if the rule does not have read privileges to ANY of the index patterns provided
  • Loading branch information
dhurley14 authored Dec 23, 2020
1 parent 7808e0c commit e9641a9
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
getListsClient,
getExceptions,
sortExceptionItems,
checkPrivileges,
} from './utils';
import { parseScheduleDates } from '../../../../common/detection_engine/parse_schedule_dates';
import { RuleExecutorOptions, SearchAfterAndBulkCreateReturnType } from './types';
Expand All @@ -42,6 +43,7 @@ jest.mock('./utils', () => {
getListsClient: jest.fn(),
getExceptions: jest.fn(),
sortExceptionItems: jest.fn(),
checkPrivileges: jest.fn(),
};
});
jest.mock('../notifications/schedule_notification_actions');
Expand Down Expand Up @@ -102,6 +104,7 @@ describe('rules_notification_alert_type', () => {
find: jest.fn(),
goingToRun: jest.fn(),
error: jest.fn(),
partialFailure: jest.fn(),
};
(ruleStatusServiceFactory as jest.Mock).mockReturnValue(ruleStatusService);
(getGapBetweenRuns as jest.Mock).mockReturnValue(moment.duration(0));
Expand All @@ -121,6 +124,21 @@ describe('rules_notification_alert_type', () => {
searchAfterTimes: [],
createdSignalsCount: 10,
});
(checkPrivileges as jest.Mock).mockImplementation((_, indices) => {
return {
index: indices.reduce(
(acc: { index: { [x: string]: { read: boolean } } }, index: string) => {
return {
[index]: {
read: true,
},
...acc,
};
},
{}
),
};
});
alertServices.callCluster.mockResolvedValue({
hits: {
total: { value: 10 },
Expand Down Expand Up @@ -167,6 +185,52 @@ describe('rules_notification_alert_type', () => {
});
});

it('should set a partial failure for when rules cannot read ALL provided indices', async () => {
(checkPrivileges as jest.Mock).mockResolvedValueOnce({
username: 'elastic',
has_all_requested: false,
cluster: {},
index: {
'myfa*': {
read: true,
},
'some*': {
read: false,
},
},
application: {},
});
payload.params.index = ['some*', 'myfa*'];
await alert.executor(payload);
expect(ruleStatusService.partialFailure).toHaveBeenCalled();
expect(ruleStatusService.partialFailure.mock.calls[0][0]).toContain(
'Missing required read permissions on indexes: ["some*"]'
);
});

it('should set a failure status for when rules cannot read ANY provided indices', async () => {
(checkPrivileges as jest.Mock).mockResolvedValueOnce({
username: 'elastic',
has_all_requested: false,
cluster: {},
index: {
'myfa*': {
read: false,
},
'some*': {
read: false,
},
},
application: {},
});
payload.params.index = ['some*', 'myfa*'];
await alert.executor(payload);
expect(ruleStatusService.error).toHaveBeenCalled();
expect(ruleStatusService.error.mock.calls[0][0]).toContain(
'The rule does not have read privileges to any of the following indices: ["myfa*","some*"]'
);
});

it('should NOT warn about the gap between runs if gap small', async () => {
(getGapBetweenRuns as jest.Mock).mockReturnValue(moment.duration(1, 'm'));
(getGapMaxCatchupRatio as jest.Mock).mockReturnValue({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
/* eslint-disable complexity */

import { Logger, KibanaRequest } from 'src/core/server';
import { partition } from 'lodash';

import {
SIGNALS_ID,
Expand Down Expand Up @@ -41,6 +42,7 @@ import {
createSearchAfterReturnType,
mergeReturns,
createSearchAfterReturnTypeFromResponse,
checkPrivileges,
} from './utils';
import { signalParamsSchema } from './signal_params_schema';
import { siemRuleActionGroups } from './siem_rule_action_groups';
Expand Down Expand Up @@ -161,8 +163,51 @@ export const signalRulesAlertType = ({

logger.debug(buildRuleMessage('[+] Starting Signal Rule execution'));
logger.debug(buildRuleMessage(`interval: ${interval}`));
let wroteStatus = false;
await ruleStatusService.goingToRun();

// check if rule has permissions to access given index pattern
// move this collection of lines into a function in utils
// so that we can use it in create rules route, bulk, etc.
try {
const inputIndex = await getInputIndex(services, version, index);
const privileges = await checkPrivileges(services, inputIndex);

const indexNames = Object.keys(privileges.index);
const [indexesWithReadPrivileges, indexesWithNoReadPrivileges] = partition(
indexNames,
(indexName) => privileges.index[indexName].read
);

if (
indexesWithReadPrivileges.length > 0 &&
indexesWithNoReadPrivileges.length >= indexesWithReadPrivileges.length
) {
// some indices have read privileges others do not.
// set a partial failure status
const errorString = `Missing required read permissions on indexes: ${JSON.stringify(
indexesWithNoReadPrivileges
)}`;
logger.debug(buildRuleMessage(errorString));
await ruleStatusService.partialFailure(errorString);
wroteStatus = true;
} else if (
indexesWithReadPrivileges.length === 0 &&
indexesWithNoReadPrivileges.length === indexNames.length
) {
// none of the indices had read privileges so set the status to failed
// since we can't search on any indices we do not have read privileges on
const errorString = `The rule does not have read privileges to any of the following indices: ${JSON.stringify(
indexesWithNoReadPrivileges
)}`;
logger.debug(buildRuleMessage(errorString));
await ruleStatusService.error(errorString);
wroteStatus = true;
}
} catch (exc) {
logger.error(buildRuleMessage(`Check privileges failed to execute ${exc}`));
}

const gap = getGapBetweenRuns({ previousStartedAt, interval, from, to });
if (gap != null && gap.asMilliseconds() > 0) {
const fromUnit = from[from.length - 1];
Expand Down Expand Up @@ -590,7 +635,7 @@ export const signalRulesAlertType = ({
`[+] Finished indexing ${result.createdSignalsCount} signals into ${outputIndex}`
)
);
if (!hasError) {
if (!hasError && !wroteStatus) {
await ruleStatusService.success('succeeded', {
bulkCreateTimeDurations: result.bulkCreateTimes,
searchAfterTimeDurations: result.searchAfterTimes,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,20 @@ export const shorthandMap = {
},
};

export const checkPrivileges = async (services: AlertServices, indices: string[]) =>
services.callCluster('transport.request', {
path: '/_security/user/_has_privileges',
method: 'POST',
body: {
index: [
{
names: indices ?? [],
privileges: ['read'],
},
],
},
});

export const getGapMaxCatchupRatio = ({
logger,
previousStartedAt,
Expand Down

0 comments on commit e9641a9

Please sign in to comment.