Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution][Investigations][Timeline] - Update getExceptions to use parameters #145889

Merged
merged 4 commits into from
Nov 28, 2022
Merged

[Security Solution][Investigations][Timeline] - Update getExceptions to use parameters #145889

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e77189e
update threshold filter
michaelolo24 Nov 21, 2022
d56a35d
reorder and add parameters to test
michaelolo24 Nov 21, 2022
cc75ce9
Merge branch 'main' into fix-threshold-exceptions-filter
michaelolo24 Nov 22, 2022
41fd2b7
Merge branch 'main' into fix-threshold-exceptions-filter
kibanamachine Nov 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 27 additions & 8 deletions ...ublic/detections/components/alerts_table/timeline_actions/use_investigate_in_timeline.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { useDispatch } from 'react-redux';
import { EuiContextMenuItem } from '@elastic/eui';

import { i18n } from '@kbn/i18n';
import { ALERT_RULE_EXCEPTIONS_LIST } from '@kbn/rule-data-utils';
import { ALERT_RULE_EXCEPTIONS_LIST, ALERT_RULE_PARAMETERS } from '@kbn/rule-data-utils';
import type { ExceptionListId } from '@kbn/securitysolution-io-ts-list-types';
import { useApi } from '@kbn/securitysolution-list-hooks';

Expand Down Expand Up @@ -52,9 +52,27 @@ export const useInvestigateInTimeline = ({

const getExceptionFilter = useCallback(
async (ecsData: Ecs): Promise<Filter | undefined> => {
const exceptionsLists = (getField(ecsData, ALERT_RULE_EXCEPTIONS_LIST) ?? []).reduce(
(acc: ExceptionListId[], next: string) => {
const parsedList = JSON.parse(next);
// This pulls exceptions list information from `_source`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for context here.

// This primarily matters for the old `signal` alerts a user may be viewing
// as new exception lists are pulled from kibana.alert.rule.parameters[0].exception_lists;
// Source was removed in favour of the fields api which passes the exceptions_list via `kibana.alert.rule.parameters`
let exceptionsList = getField(ecsData, ALERT_RULE_EXCEPTIONS_LIST) ?? [];

if (exceptionsList.length === 0) {
try {
const ruleParameters = getField(ecsData, ALERT_RULE_PARAMETERS) ?? {};
if (ruleParameters.length > 0) {
const parametersObject = JSON.parse(ruleParameters[0]);
exceptionsList = parametersObject?.exceptions_list ?? [];
}
} catch (error) {
// do nothing, just fail silently as parametersObject is initialized
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parametersObject

Did you mean exceptionList?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, I updated the code, but not the comment, will do a follow up cleanup PR for this. Thanks!

}
}
const detectionExceptionsLists = exceptionsList.reduce(
(acc: ExceptionListId[], next: string | object) => {
// parsed rule.parameters returns an object else use the default string representation
const parsedList = typeof next === 'string' ? JSON.parse(next) : next;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think that this line(JSON.parse) might result in exception when ......next === ''

Copy link
Contributor Author

@michaelolo24 michaelolo24 Nov 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure exceptions list will ever actually be ''. The existing code didn't have checks for this either, but it may be worth guarding against, will add a change for this as well, thanks!

if (parsedList.type === 'detection') {
const formattedList = {
exception_list_id: parsedList.list_id,
Expand All @@ -67,14 +85,15 @@ export const useInvestigateInTimeline = ({
[]
);

if (exceptionsLists.length > 0) {
let exceptionFilter;
if (detectionExceptionsLists.length > 0) {
await getExceptionFilterFromIds({
exceptionListIds: exceptionsLists,
exceptionListIds: detectionExceptionsLists,
excludeExceptions: true,
chunkSize: 20,
alias: 'Exceptions',
onSuccess: (filter) => {
return filter;
exceptionFilter = filter;
},
onError: (err: string[]) => {
addError(err, {
Expand All @@ -86,7 +105,7 @@ export const useInvestigateInTimeline = ({
},
});
}
return undefined;
return exceptionFilter;
},
[addError, getExceptionFilterFromIds]
);
Expand Down
9 changes: 8 additions & 1 deletion x-pack/plugins/timelines/server/search_strategy/timeline/factory/helpers/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@
* 2.0.
*/

import { ALERT_RULE_CONSUMER, ALERT_RISK_SCORE, ALERT_SEVERITY } from '@kbn/rule-data-utils';
import {
ALERT_RULE_CONSUMER,
ALERT_RISK_SCORE,
ALERT_SEVERITY,
ALERT_RULE_PARAMETERS,
} from '@kbn/rule-data-utils';
import { ENRICHMENT_DESTINATION_PATH } from '../../../../../common/constants';

export const MATCHED_ATOMIC = 'matched.atomic';
Expand Down Expand Up @@ -40,6 +45,7 @@ export const CTI_ROW_RENDERER_FIELDS = [
FEED_NAME_REFERENCE,
];

// TODO: update all of these fields to use the constants from technical field names
export const TIMELINE_EVENTS_FIELDS = [
ALERT_RULE_CONSUMER,
'@timestamp',
Expand All @@ -58,6 +64,7 @@ export const TIMELINE_EVENTS_FIELDS = [
'kibana.alert.rule.version',
ALERT_SEVERITY,
ALERT_RISK_SCORE,
ALERT_RULE_PARAMETERS,
'kibana.alert.threshold_result',
'kibana.alert.building_block_type',
'kibana.alert.suppression.docs_count',
Expand Down
91 changes: 47 additions & 44 deletions ...ns/timelines/server/search_strategy/timeline/factory/helpers/format_timeline_data.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,50 @@ describe('formatTimelineData', () => {
field: 'kibana.alert.rule.uuid',
value: ['15d82f10-0926-11ed-bece-6b0c033d0075'],
},
{
field: 'kibana.alert.rule.parameters.sourceId',
value: ['default'],
},
{
field: 'kibana.alert.rule.parameters.nodeType',
value: ['host'],
},
{
field: 'kibana.alert.rule.parameters.criteria.comparator',
value: ['>'],
},
{
field: 'kibana.alert.rule.parameters.criteria.timeSize',
value: ['1'],
},
{
field: 'kibana.alert.rule.parameters.criteria.metric',
value: ['cpu'],
},
{
field: 'kibana.alert.rule.parameters.criteria.threshold',
value: ['10'],
},
{
field: 'kibana.alert.rule.parameters.criteria.customMetric.aggregation',
value: ['avg'],
},
{
field: 'kibana.alert.rule.parameters.criteria.customMetric.id',
value: ['alert-custom-metric'],
},
{
field: 'kibana.alert.rule.parameters.criteria.customMetric.field',
value: [''],
},
{
field: 'kibana.alert.rule.parameters.criteria.customMetric.type',
value: ['custom'],
},
{
field: 'kibana.alert.rule.parameters.criteria.timeUnit',
value: ['d'],
},
{
field: 'event.action',
value: ['active'],
Expand Down Expand Up @@ -466,50 +510,6 @@ describe('formatTimelineData', () => {
field: 'kibana.version',
value: ['8.4.0'],
},
{
field: 'kibana.alert.rule.parameters.sourceId',
value: ['default'],
},
{
field: 'kibana.alert.rule.parameters.nodeType',
value: ['host'],
},
{
field: 'kibana.alert.rule.parameters.criteria.comparator',
value: ['>'],
},
{
field: 'kibana.alert.rule.parameters.criteria.timeSize',
value: ['1'],
},
{
field: 'kibana.alert.rule.parameters.criteria.metric',
value: ['cpu'],
},
{
field: 'kibana.alert.rule.parameters.criteria.threshold',
value: ['10'],
},
{
field: 'kibana.alert.rule.parameters.criteria.customMetric.aggregation',
value: ['avg'],
},
{
field: 'kibana.alert.rule.parameters.criteria.customMetric.id',
value: ['alert-custom-metric'],
},
{
field: 'kibana.alert.rule.parameters.criteria.customMetric.field',
value: [''],
},
{
field: 'kibana.alert.rule.parameters.criteria.customMetric.type',
value: ['custom'],
},
{
field: 'kibana.alert.rule.parameters.criteria.timeUnit',
value: ['d'],
},
],
ecs: {
'@timestamp': ['2022-07-21T22:38:57.888Z'],
Expand All @@ -528,6 +528,9 @@ describe('formatTimelineData', () => {
consumer: ['infrastructure'],
name: ['test 1212'],
uuid: ['15d82f10-0926-11ed-bece-6b0c033d0075'],
parameters: [
'{"sourceId":"default","nodeType":"host","criteria":[{"comparator":">","timeSize":1,"metric":"cpu","threshold":[10],"customMetric":{"aggregation":"avg","id":"alert-custom-metric","field":"","type":"custom"},"timeUnit":"d"}]}',
],
},
workflow_status: ['open'],
},
Expand Down