github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-24 09:48:58 -04:00
Code Issues Projects Releases Packages Wiki Activity

[SIEM][Detection Engine] Critical blocker, fixes pre-packaged rule miscounts (#56708)

Browse source 
## Summary

* Found multiple issues with how unstable finds can occur where iterating over multiple pages of find API with saved objects might return the same results per page and omit things as you try to figure out which pre-packaged rules are installed and which ones are not.
* This makes a distinct trade off of doing more JSON.parse() on the event loop by querying all the pre-packaged rules at one time. This however gives a stable and accurate count
* Fixed the tags aggregation to do the same thing.
* Fixes https://github.com/elastic/siem-team/issues/506

### Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~- [ ] This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)~~

~~- [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)~~

~~- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials~~

- [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios

~~- [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)~~

### For maintainers

~~- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~~

- [x] This includes a feature addition or change that requires a release note and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
This commit is contained in:
Frank Hassanabad 2020-02-03 19:49:42 -07:00 • committed by GitHub
parent f82d44e754
commit 585fce8d61
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 73 additions and 219 deletions
Download patch file Download diff file Expand all files Collapse all files

191
x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/get_existing_prepackaged_rules.test.ts
View file

@ -35,32 +35,7 @@ describe('get_existing_prepackaged_rules', () => {
expect(rules).toEqual([getResult()]);
});
test('should return 2 items over two pages, one per page', async () => {
const alertsClient = alertsClientMock.create();
const result1 = getResult();
result1.params.immutable = true;
result1.id = '4baa53f8-96da-44ee-ad58-41bccb7f9f3d';
const result2 = getResult();
result2.params.immutable = true;
result2.id = '5baa53f8-96da-44ee-ad58-41bccb7f9f3d';
alertsClient.find.mockResolvedValueOnce(
getFindResultWithMultiHits({ data: [result1], perPage: 1, page: 1, total: 2 })
);
alertsClient.find.mockResolvedValueOnce(
getFindResultWithMultiHits({ data: [result2], perPage: 1, page: 2, total: 2 })
);
const unsafeCast: AlertsClient = (alertsClient as unknown) as AlertsClient;
const rules = await getExistingPrepackagedRules({
alertsClient: unsafeCast,
});
expect(rules).toEqual([result1, result2]);
});
test('should return 3 items with over 3 pages one per page', async () => {
test('should return 3 items over 1 page with all on one page', async () => {
const alertsClient = alertsClientMock.create();
const result1 = getResult();
@ -75,40 +50,17 @@ describe('get_existing_prepackaged_rules', () => {
result3.params.immutable = true;
result3.id = 'f3e1bf0b-b95f-43da-b1de-5d2f0af2287a';
// first result mock which is for returning the total
alertsClient.find.mockResolvedValueOnce(
getFindResultWithMultiHits({ data: [result1], perPage: 1, page: 1, total: 3 })
getFindResultWithMultiHits({
data: [result1],
perPage: 1,
page: 1,
total: 3,
})
);
alertsClient.find.mockResolvedValueOnce(
getFindResultWithMultiHits({ data: [result2], perPage: 1, page: 2, total: 3 })
);
alertsClient.find.mockResolvedValueOnce(
getFindResultWithMultiHits({ data: [result3], perPage: 1, page: 2, total: 3 })
);
const unsafeCast: AlertsClient = (alertsClient as unknown) as AlertsClient;
const rules = await getExistingPrepackagedRules({
alertsClient: unsafeCast,
});
expect(rules).toEqual([result1, result2, result3]);
});
test('should return 3 items over 1 pages with all on one page', async () => {
const alertsClient = alertsClientMock.create();
const result1 = getResult();
result1.params.immutable = true;
result1.id = '4baa53f8-96da-44ee-ad58-41bccb7f9f3d';
const result2 = getResult();
result2.params.immutable = true;
result2.id = '5baa53f8-96da-44ee-ad58-41bccb7f9f3d';
const result3 = getResult();
result3.params.immutable = true;
result3.id = 'f3e1bf0b-b95f-43da-b1de-5d2f0af2287a';
// second mock which will return all the data on a single page
alertsClient.find.mockResolvedValueOnce(
getFindResultWithMultiHits({
data: [result1, result2, result3],
@ -137,7 +89,7 @@ describe('get_existing_prepackaged_rules', () => {
expect(rules).toEqual([getResult()]);
});
test('should return 2 items over two pages, one per page', async () => {
test('should return 2 items over 1 page', async () => {
const alertsClient = alertsClientMock.create();
const result1 = getResult();
@ -146,11 +98,19 @@ describe('get_existing_prepackaged_rules', () => {
const result2 = getResult();
result2.id = '5baa53f8-96da-44ee-ad58-41bccb7f9f3d';
// first result mock which is for returning the total
alertsClient.find.mockResolvedValueOnce(
getFindResultWithMultiHits({ data: [result1], perPage: 1, page: 1, total: 2 })
getFindResultWithMultiHits({
data: [result1],
perPage: 1,
page: 1,
total: 2,
})
);
// second mock which will return all the data on a single page
alertsClient.find.mockResolvedValueOnce(
getFindResultWithMultiHits({ data: [result2], perPage: 1, page: 2, total: 2 })
getFindResultWithMultiHits({ data: [result1, result2], perPage: 2, page: 1, total: 2 })
);
const unsafeCast: AlertsClient = (alertsClient as unknown) as AlertsClient;
@ -160,7 +120,7 @@ describe('get_existing_prepackaged_rules', () => {
expect(rules).toEqual([result1, result2]);
});
test('should return 3 items with over 3 pages one per page', async () => {
test('should return 3 items over 1 page with all on one page', async () => {
const alertsClient = alertsClientMock.create();
const result1 = getResult();
@ -172,37 +132,17 @@ describe('get_existing_prepackaged_rules', () => {
const result3 = getResult();
result3.id = 'f3e1bf0b-b95f-43da-b1de-5d2f0af2287a';
// first result mock which is for returning the total
alertsClient.find.mockResolvedValueOnce(
getFindResultWithMultiHits({ data: [result1], perPage: 1, page: 1, total: 3 })
getFindResultWithMultiHits({
data: [result1],
perPage: 3,
page: 1,
total: 3,
})
);
alertsClient.find.mockResolvedValueOnce(
getFindResultWithMultiHits({ data: [result2], perPage: 1, page: 2, total: 3 })
);
alertsClient.find.mockResolvedValueOnce(
getFindResultWithMultiHits({ data: [result3], perPage: 1, page: 2, total: 3 })
);
const unsafeCast: AlertsClient = (alertsClient as unknown) as AlertsClient;
const rules = await getNonPackagedRules({
alertsClient: unsafeCast,
});
expect(rules).toEqual([result1, result2, result3]);
});
test('should return 3 items over 1 pages with all on one page', async () => {
const alertsClient = alertsClientMock.create();
const result1 = getResult();
result1.id = '4baa53f8-96da-44ee-ad58-41bccb7f9f3d';
const result2 = getResult();
result2.id = '5baa53f8-96da-44ee-ad58-41bccb7f9f3d';
const result3 = getResult();
result3.id = 'f3e1bf0b-b95f-43da-b1de-5d2f0af2287a';
// second mock which will return all the data on a single page
alertsClient.find.mockResolvedValueOnce(
getFindResultWithMultiHits({
data: [result1, result2, result3],
@ -241,11 +181,19 @@ describe('get_existing_prepackaged_rules', () => {
const result2 = getResult();
result2.id = '5baa53f8-96da-44ee-ad58-41bccb7f9f3d';
// first result mock which is for returning the total
alertsClient.find.mockResolvedValueOnce(
getFindResultWithMultiHits({ data: [result1], perPage: 1, page: 1, total: 2 })
getFindResultWithMultiHits({
data: [result1],
perPage: 1,
page: 1,
total: 2,
})
);
// second mock which will return all the data on a single page
alertsClient.find.mockResolvedValueOnce(
getFindResultWithMultiHits({ data: [result2], perPage: 1, page: 2, total: 2 })
getFindResultWithMultiHits({ data: [result1, result2], perPage: 2, page: 1, total: 2 })
);
const unsafeCast: AlertsClient = (alertsClient as unknown) as AlertsClient;
@ -255,67 +203,6 @@ describe('get_existing_prepackaged_rules', () => {
});
expect(rules).toEqual([result1, result2]);
});
test('should return 3 items with over 3 pages one per page', async () => {
const alertsClient = alertsClientMock.create();
const result1 = getResult();
result1.id = '4baa53f8-96da-44ee-ad58-41bccb7f9f3d';
const result2 = getResult();
result2.id = '5baa53f8-96da-44ee-ad58-41bccb7f9f3d';
const result3 = getResult();
result3.id = 'f3e1bf0b-b95f-43da-b1de-5d2f0af2287a';
alertsClient.find.mockResolvedValueOnce(
getFindResultWithMultiHits({ data: [result1], perPage: 1, page: 1, total: 3 })
);
alertsClient.find.mockResolvedValueOnce(
getFindResultWithMultiHits({ data: [result2], perPage: 1, page: 2, total: 3 })
);
alertsClient.find.mockResolvedValueOnce(
getFindResultWithMultiHits({ data: [result3], perPage: 1, page: 2, total: 3 })
);
const unsafeCast: AlertsClient = (alertsClient as unknown) as AlertsClient;
const rules = await getRules({
alertsClient: unsafeCast,
filter: '',
});
expect(rules).toEqual([result1, result2, result3]);
});
test('should return 3 items over 1 pages with all on one page', async () => {
const alertsClient = alertsClientMock.create();
const result1 = getResult();
result1.id = '4baa53f8-96da-44ee-ad58-41bccb7f9f3d';
const result2 = getResult();
result2.id = '5baa53f8-96da-44ee-ad58-41bccb7f9f3d';
const result3 = getResult();
result3.id = 'f3e1bf0b-b95f-43da-b1de-5d2f0af2287a';
alertsClient.find.mockResolvedValueOnce(
getFindResultWithMultiHits({
data: [result1, result2, result3],
perPage: 3,
page: 1,
total: 3,
})
);
const unsafeCast: AlertsClient = (alertsClient as unknown) as AlertsClient;
const rules = await getRules({
alertsClient: unsafeCast,
filter: '',
});
expect(rules).toEqual([result1, result2, result3]);
});
});
describe('getRulesCount', () => {

55
x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/get_existing_prepackaged_rules.ts
View file

@ -9,7 +9,6 @@ import { AlertsClient } from '../../../../../alerting';
import { RuleAlertType, isAlertTypes } from './types';
import { findRules } from './find_rules';
export const DEFAULT_PER_PAGE = 100;
export const FILTER_NON_PREPACKED_RULES = `alert.attributes.tags: "${INTERNAL_IMMUTABLE_KEY}:false"`;
export const FILTER_PREPACKED_RULES = `alert.attributes.tags: "${INTERNAL_IMMUTABLE_KEY}:true"`;
@ -33,84 +32,56 @@ export const getRulesCount = async ({
filter,
perPage: 1,
page: 1,
sortField: 'createdAt',
sortOrder: 'desc',
});
return firstRule.total;
};
export const getRules = async ({
alertsClient,
perPage = DEFAULT_PER_PAGE,
filter,
}: {
alertsClient: AlertsClient;
perPage?: number;
filter: string;
}): Promise<RuleAlertType[]> => {
const firstPrepackedRules = await findRules({
const count = await getRulesCount({ alertsClient, filter });
const rules = await findRules({
alertsClient,
filter,
perPage,
perPage: count,
page: 1,
sortField: 'createdAt',
sortOrder: 'desc',
});
const totalPages = Math.ceil(firstPrepackedRules.total / firstPrepackedRules.perPage);
if (totalPages <= 1) {
if (isAlertTypes(firstPrepackedRules.data)) {
return firstPrepackedRules.data;
} else {
// If this was ever true, you have a really messed up system.
// This is keep typescript happy since we have an unknown with data
return [];
}
} else {
const returnPrepackagedRules = await Array(totalPages - 1)
.fill({})
.map((_, page) => {
// page index starts at 2 as we already got the first page and we have more pages to go
return findRules({
alertsClient,
filter,
perPage,
page: page + 2,
});
})
.reduce<Promise<object[]>>(async (accum, nextPage) => {
return [...(await accum), ...(await nextPage).data];
}, Promise.resolve(firstPrepackedRules.data));
if (isAlertTypes(returnPrepackagedRules)) {
return returnPrepackagedRules;
} else {
// If this was ever true, you have a really messed up system.
// This is keep typescript happy since we have an unknown with data
return [];
}
if (isAlertTypes(rules.data)) {
return rules.data;
} else {
// If this was ever true, you have a really messed up system.
// This is keep typescript happy since we have an unknown with data
return [];
}
};
export const getNonPackagedRules = async ({
alertsClient,
perPage = DEFAULT_PER_PAGE,
}: {
alertsClient: AlertsClient;
perPage?: number;
}): Promise<RuleAlertType[]> => {
return getRules({
alertsClient,
perPage,
filter: FILTER_NON_PREPACKED_RULES,
});
};
export const getExistingPrepackagedRules = async ({
alertsClient,
perPage = DEFAULT_PER_PAGE,
}: {
alertsClient: AlertsClient;
perPage?: number;
}): Promise<RuleAlertType[]> => {
return getRules({
alertsClient,
perPage,
filter: FILTER_PREPACKED_RULES,
});
};

46
x-pack/legacy/plugins/siem/server/lib/detection_engine/tags/read_tags.ts
View file

@ -9,8 +9,6 @@ import { INTERNAL_IDENTIFIER } from '../../../../common/constants';
import { AlertsClient } from '../../../../../alerting';
import { findRules } from '../rules/find_rules';
const DEFAULT_PER_PAGE: number = 1000;
export interface TagType {
id: string;
tags: string[];
@ -42,39 +40,37 @@ export const convertTagsToSet = (tagObjects: object[]): Set<string> => {
// Ref: https://www.elastic.co/guide/en/kibana/master/saved-objects-api.html
export const readTags = async ({
alertsClient,
perPage = DEFAULT_PER_PAGE,
}: {
alertsClient: AlertsClient;
perPage?: number;
}): Promise<string[]> => {
const tags = await readRawTags({ alertsClient, perPage });
const tags = await readRawTags({ alertsClient });
return tags.filter(tag => !tag.startsWith(INTERNAL_IDENTIFIER));
};
export const readRawTags = async ({
alertsClient,
perPage = DEFAULT_PER_PAGE,
}: {
alertsClient: AlertsClient;
perPage?: number;
}): Promise<string[]> => {
const firstTags = await findRules({ alertsClient, fields: ['tags'], perPage, page: 1 });
const firstSet = convertTagsToSet(firstTags.data);
const totalPages = Math.ceil(firstTags.total / firstTags.perPage);
if (totalPages <= 1) {
return Array.from(firstSet);
} else {
const returnTags = await Array(totalPages - 1)
.fill({})
.map((_, page) => {
// page index starts at 2 as we already got the first page and we have more pages to go
return findRules({ alertsClient, fields: ['tags'], perPage, page: page + 2 });
})
.reduce<Promise<Set<string>>>(async (accum, nextTagPage) => {
const tagArray = convertToTags((await nextTagPage).data);
return new Set([...(await accum), ...tagArray]);
}, Promise.resolve(firstSet));
return Array.from(returnTags);
}
// Get just one record so we can get the total count
const firstTags = await findRules({
alertsClient,
fields: ['tags'],
perPage: 1,
page: 1,
sortField: 'createdAt',
sortOrder: 'desc',
});
// Get all the rules to aggregate over all the tags of the rules
const rules = await findRules({
alertsClient,
fields: ['tags'],
perPage: firstTags.total,
sortField: 'createdAt',
sortOrder: 'desc',
page: 1,
});
const tagSet = convertTagsToSet(rules.data);
return Array.from(tagSet);
};