Skip to content

Commit 4599293

Browse files
committed
[Security Solution] [Detections] Add "read index" privilege check on rule execution (#83134)
* 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
1 parent 893feee commit 4599293

File tree

3 files changed

+124
-1
lines changed

3 files changed

+124
-1
lines changed

x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
getListsClient,
1717
getExceptions,
1818
sortExceptionItems,
19+
checkPrivileges,
1920
} from './utils';
2021
import { parseScheduleDates } from '../../../../common/detection_engine/parse_schedule_dates';
2122
import { RuleExecutorOptions, SearchAfterAndBulkCreateReturnType } from './types';
@@ -42,6 +43,7 @@ jest.mock('./utils', () => {
4243
getListsClient: jest.fn(),
4344
getExceptions: jest.fn(),
4445
sortExceptionItems: jest.fn(),
46+
checkPrivileges: jest.fn(),
4547
};
4648
});
4749
jest.mock('../notifications/schedule_notification_actions');
@@ -102,6 +104,7 @@ describe('rules_notification_alert_type', () => {
102104
find: jest.fn(),
103105
goingToRun: jest.fn(),
104106
error: jest.fn(),
107+
partialFailure: jest.fn(),
105108
};
106109
(ruleStatusServiceFactory as jest.Mock).mockReturnValue(ruleStatusService);
107110
(getGapBetweenRuns as jest.Mock).mockReturnValue(moment.duration(0));
@@ -121,6 +124,21 @@ describe('rules_notification_alert_type', () => {
121124
searchAfterTimes: [],
122125
createdSignalsCount: 10,
123126
});
127+
(checkPrivileges as jest.Mock).mockImplementation((_, indices) => {
128+
return {
129+
index: indices.reduce(
130+
(acc: { index: { [x: string]: { read: boolean } } }, index: string) => {
131+
return {
132+
[index]: {
133+
read: true,
134+
},
135+
...acc,
136+
};
137+
},
138+
{}
139+
),
140+
};
141+
});
124142
alertServices.callCluster.mockResolvedValue({
125143
hits: {
126144
total: { value: 10 },
@@ -167,6 +185,52 @@ describe('rules_notification_alert_type', () => {
167185
});
168186
});
169187

188+
it('should set a partial failure for when rules cannot read ALL provided indices', async () => {
189+
(checkPrivileges as jest.Mock).mockResolvedValueOnce({
190+
username: 'elastic',
191+
has_all_requested: false,
192+
cluster: {},
193+
index: {
194+
'myfa*': {
195+
read: true,
196+
},
197+
'some*': {
198+
read: false,
199+
},
200+
},
201+
application: {},
202+
});
203+
payload.params.index = ['some*', 'myfa*'];
204+
await alert.executor(payload);
205+
expect(ruleStatusService.partialFailure).toHaveBeenCalled();
206+
expect(ruleStatusService.partialFailure.mock.calls[0][0]).toContain(
207+
'Missing required read permissions on indexes: ["some*"]'
208+
);
209+
});
210+
211+
it('should set a failure status for when rules cannot read ANY provided indices', async () => {
212+
(checkPrivileges as jest.Mock).mockResolvedValueOnce({
213+
username: 'elastic',
214+
has_all_requested: false,
215+
cluster: {},
216+
index: {
217+
'myfa*': {
218+
read: false,
219+
},
220+
'some*': {
221+
read: false,
222+
},
223+
},
224+
application: {},
225+
});
226+
payload.params.index = ['some*', 'myfa*'];
227+
await alert.executor(payload);
228+
expect(ruleStatusService.error).toHaveBeenCalled();
229+
expect(ruleStatusService.error.mock.calls[0][0]).toContain(
230+
'The rule does not have read privileges to any of the following indices: ["myfa*","some*"]'
231+
);
232+
});
233+
170234
it('should NOT warn about the gap between runs if gap small', async () => {
171235
(getGapBetweenRuns as jest.Mock).mockReturnValue(moment.duration(1, 'm'));
172236
(getGapMaxCatchupRatio as jest.Mock).mockReturnValue({

x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
/* eslint-disable complexity */
88

99
import { Logger, KibanaRequest } from 'src/core/server';
10+
import { partition } from 'lodash';
1011

1112
import {
1213
SIGNALS_ID,
@@ -41,6 +42,7 @@ import {
4142
createSearchAfterReturnType,
4243
mergeReturns,
4344
createSearchAfterReturnTypeFromResponse,
45+
checkPrivileges,
4446
} from './utils';
4547
import { signalParamsSchema } from './signal_params_schema';
4648
import { siemRuleActionGroups } from './siem_rule_action_groups';
@@ -161,8 +163,51 @@ export const signalRulesAlertType = ({
161163

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

169+
// check if rule has permissions to access given index pattern
170+
// move this collection of lines into a function in utils
171+
// so that we can use it in create rules route, bulk, etc.
172+
try {
173+
const inputIndex = await getInputIndex(services, version, index);
174+
const privileges = await checkPrivileges(services, inputIndex);
175+
176+
const indexNames = Object.keys(privileges.index);
177+
const [indexesWithReadPrivileges, indexesWithNoReadPrivileges] = partition(
178+
indexNames,
179+
(indexName) => privileges.index[indexName].read
180+
);
181+
182+
if (
183+
indexesWithReadPrivileges.length > 0 &&
184+
indexesWithNoReadPrivileges.length >= indexesWithReadPrivileges.length
185+
) {
186+
// some indices have read privileges others do not.
187+
// set a partial failure status
188+
const errorString = `Missing required read permissions on indexes: ${JSON.stringify(
189+
indexesWithNoReadPrivileges
190+
)}`;
191+
logger.debug(buildRuleMessage(errorString));
192+
await ruleStatusService.partialFailure(errorString);
193+
wroteStatus = true;
194+
} else if (
195+
indexesWithReadPrivileges.length === 0 &&
196+
indexesWithNoReadPrivileges.length === indexNames.length
197+
) {
198+
// none of the indices had read privileges so set the status to failed
199+
// since we can't search on any indices we do not have read privileges on
200+
const errorString = `The rule does not have read privileges to any of the following indices: ${JSON.stringify(
201+
indexesWithNoReadPrivileges
202+
)}`;
203+
logger.debug(buildRuleMessage(errorString));
204+
await ruleStatusService.error(errorString);
205+
wroteStatus = true;
206+
}
207+
} catch (exc) {
208+
logger.error(buildRuleMessage(`Check privileges failed to execute ${exc}`));
209+
}
210+
166211
const gap = getGapBetweenRuns({ previousStartedAt, interval, from, to });
167212
if (gap != null && gap.asMilliseconds() > 0) {
168213
const fromUnit = from[from.length - 1];
@@ -590,7 +635,7 @@ export const signalRulesAlertType = ({
590635
`[+] Finished indexing ${result.createdSignalsCount} signals into ${outputIndex}`
591636
)
592637
);
593-
if (!hasError) {
638+
if (!hasError && !wroteStatus) {
594639
await ruleStatusService.success('succeeded', {
595640
bulkCreateTimeDurations: result.bulkCreateTimes,
596641
searchAfterTimeDurations: result.searchAfterTimes,

x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,20 @@ export const shorthandMap = {
5252
},
5353
};
5454

55+
export const checkPrivileges = async (services: AlertServices, indices: string[]) =>
56+
services.callCluster('transport.request', {
57+
path: '/_security/user/_has_privileges',
58+
method: 'POST',
59+
body: {
60+
index: [
61+
{
62+
names: indices ?? [],
63+
privileges: ['read'],
64+
},
65+
],
66+
},
67+
});
68+
5569
export const getGapMaxCatchupRatio = ({
5670
logger,
5771
previousStartedAt,

0 commit comments

Comments
 (0)