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

Jump to bottom

[Security Solution][Detections] Optimize rule fetching endpoints [WIP] #90639

Closed
wants to merge 1 commit into from
Closed

[Security Solution][Detections] Optimize rule fetching endpoints [WIP] #90639

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
213 changes: 178 additions & 35 deletions ...ck/plugins/security_solution/server/lib/detection_engine/routes/rules/find_rules_route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@ import { DETECTION_ENGINE_RULES_URL } from '../../../../../common/constants';
import { findRules } from '../../rules/find_rules';
import { transformValidateFindAlerts } from './validate';
import { transformError, buildSiemResponse } from '../utils';
import { getRuleActionsSavedObject } from '../../rule_actions/get_rule_actions_saved_object';
import { ruleStatusSavedObjectsClientFactory } from '../../signals/rule_status_saved_objects_client';
import { bulkFetchRuleActionsOfManyRules, getRuleActionsSavedObject, RulesActionsSavedObject } from '../../rule_actions/get_rule_actions_saved_object';
import { RuleStatusSavedObjectsClient, ruleStatusSavedObjectsClientFactory } from '../../signals/rule_status_saved_objects_client';
import { buildRouteValidation } from '../../../../utils/build_validation/route_validation';
import { IScopedClusterClient, ElasticsearchClient, SavedObjectsFindResult } from '../../../../../../../../src/core/server';
import { IRuleSavedAttributesSavedObjectAttributes, IRuleStatusSOAttributes } from '../../rules/types';

export const findRulesRoute = (router: SecuritySolutionPluginRouter) => {
router.get(
Expand All @@ -43,12 +45,13 @@ export const findRulesRoute = (router: SecuritySolutionPluginRouter) => {
const { query } = request;
const alertsClient = context.alerting?.getAlertsClient();
const savedObjectsClient = context.core.savedObjects.client;
const elasticsearchClient = context.core.elasticsearch.client;

if (!alertsClient) {
return siemResponse.error({ statusCode: 404 });
}

const ruleStatusClient = ruleStatusSavedObjectsClientFactory(savedObjectsClient);
// const ruleStatusClient = ruleStatusSavedObjectsClientFactory(savedObjectsClient);
const rules = await findRules({
alertsClient,
perPage: query.per_page,
Expand All @@ -59,45 +62,94 @@ export const findRulesRoute = (router: SecuritySolutionPluginRouter) => {
fields: query.fields,
});

if (rules.data.length === 0) {
const [validated, errors] = transformValidateFindAlerts(rules, [], []);
if (errors != null) {
return siemResponse.error({ statusCode: 500, body: errors });
} else {
return response.ok({ body: validated ?? {} });
}
}

const ruleIds = rules.data.map((rule) => rule.id);

// // if any rules attempted to execute but failed before the rule executor is called,
// // an execution status will be written directly onto the rule via the kibana alerting framework,
// // which we are filtering on and will write a failure status
// // for any rules found to be in a failing state into our rule status saved objects
// const failingRules = rules.data.filter(
// (rule) => rule.executionStatus != null && rule.executionStatus.status === 'error'
// );

// const ruleStatuses = await Promise.all(
// rules.data.map(async (rule) => {
// const results = await ruleStatusClient.find({
// perPage: 1,
// sortField: 'statusDate',
// sortOrder: 'desc',
// search: rule.id,
// searchFields: ['alertId'],
// });
// const failingRule = failingRules.find((badRule) => badRule.id === rule.id);
// if (failingRule != null) {
// if (results.saved_objects.length > 0) {
// results.saved_objects[0].attributes.status = 'failed';
// results.saved_objects[0].attributes.lastFailureAt = failingRule.executionStatus.lastExecutionDate.toISOString();
// }
// }
// return results;
// })
// );

// const ruleActions = await Promise.all(
// rules.data.map(async (rule) => {
// const results = await getRuleActionsSavedObject({
// savedObjectsClient,
// ruleAlertId: rule.id,
// });

// return results;
// })
// );

const [ruleActions, ruleStatuses] = await Promise.all([
bulkFetchRuleActionsOfManyRules({ savedObjectsClient, ruleIds }),
bulkFetchLatestStatusesOfManyRules(elasticsearchClient.asCurrentUser, ruleIds),
]);
Comment on lines +115 to +118
Copy link
Contributor Author

@banderror banderror Feb 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Please review this from the security standpoint (access privileges, isolation (spaces), error handling, potential issues, etc.).

  • Is it ok to query .kibana index via elasticsearchClient.asCurrentUser, or privileges to .kibana are not guaranteed to all users by default?
  • I guess it would be better to query asInternalUser. Would it be safe in this particular case?


// if any rules attempted to execute but failed before the rule executor is called,
// an execution status will be written directly onto the rule via the kibana alerting framework,
// which we are filtering on and will write a failure status
// for any rules found to be in a failing state into our rule status saved objects
const failingRules = rules.data.filter(
(rule) => rule.executionStatus != null && rule.executionStatus.status === 'error'
);
rules.data.forEach((rule) => {
const isRuleFailing =
rule.executionStatus != null && rule.executionStatus.status === 'error';

const ruleStatuses = await Promise.all(
rules.data.map(async (rule) => {
const results = await ruleStatusClient.find({
perPage: 1,
sortField: 'statusDate',
sortOrder: 'desc',
search: rule.id,
searchFields: ['alertId'],
});
const failingRule = failingRules.find((badRule) => badRule.id === rule.id);
if (failingRule != null) {
if (results.saved_objects.length > 0) {
results.saved_objects[0].attributes.status = 'failed';
results.saved_objects[0].attributes.lastFailureAt = failingRule.executionStatus.lastExecutionDate.toISOString();
}
if (isRuleFailing) {
const ruleStatus = ruleStatuses.find((status) => status.alertId === rule.id);
if (ruleStatus != null) {
ruleStatus.status = 'failed';
ruleStatus.lastFailureAt = rule.executionStatus.lastExecutionDate.toISOString();
}
return results;
})
);
const ruleActions = await Promise.all(
rules.data.map(async (rule) => {
const results = await getRuleActionsSavedObject({
savedObjectsClient,
ruleAlertId: rule.id,
});

return results;
})
);
}
});

const ruleStatusesFindResultMock = ruleStatuses.map((status) => {
return {
saved_objects: [{ attributes: status }] as Array<
SavedObjectsFindResult<IRuleSavedAttributesSavedObjectAttributes>
>,
total: 0,
per_page: 0,
page: 0,
};
});

const [validated, errors] = transformValidateFindAlerts(rules, ruleActions, ruleStatuses);
const [validated, errors] = transformValidateFindAlerts(
rules,
ruleActions,
ruleStatusesFindResultMock
);
if (errors != null) {
return siemResponse.error({ statusCode: 500, body: errors });
} else {
Expand All @@ -113,3 +165,94 @@ export const findRulesRoute = (router: SecuritySolutionPluginRouter) => {
}
);
};

// const bulkFetchRuleActionsOfManyRules = async (
// client: ElasticsearchClient,
// ruleIds: string[]
// ): Promise<Array<RulesActionsSavedObject | null>> => {
// if (ruleIds.length === 0) {
// return [];
// }

// const response = await client.search({
// index: '.kibana',
// ignore_unavailable: true,
// size: ruleIds.length,
// body: {
// size: ruleIds.length,
// query: {
// bool: {
// filter: [
// { term: { type: 'siem-detection-engine-rule-actions' } },
// { terms: { 'siem-detection-engine-rule-actions.ruleAlertId': ruleIds } },
// ],
// },
// },
// },
// });

// const actionsHits = response.body.hits?.hits ?? [];
// const actions = actionsHits.filter(Boolean).map((hit: any) => {
// const id = hit._id;
// const attributes = hit._source?.['siem-detection-engine-rule-actions'] ?? {};
// return { id, ...attributes };
// });

// return actions.filter(Boolean) as RulesActionsSavedObject[];
// };

const bulkFetchLatestStatusesOfManyRules = async (
client: ElasticsearchClient,
ruleIds: string[]
): Promise<IRuleSavedAttributesSavedObjectAttributes[]> => {
// This is not only a performance optimization. If ids is empty, the search
// request to Elasticsearch using terms aggregation will throw a 400 error,
// which is not what we want here.
if (ruleIds.length === 0) {
return [];
}

const response = await client.search({
index: '.kibana',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: Kibana index name should be retrieved from the config.

ignore_unavailable: true,
size: 0,
body: {
size: 0,
query: {
bool: {
filter: [
{ term: { type: 'siem-detection-engine-rule-status' } },
{ terms: { 'siem-detection-engine-rule-status.alertId': ruleIds } },
],
},
},
aggs: {
rules: {
terms: {
field: 'siem-detection-engine-rule-status.alertId',
size: ruleIds.length,
},
aggs: {
latest_status: {
top_hits: {
size: 1,
sort: [{ 'siem-detection-engine-rule-status.statusDate': { order: 'desc' } }],
},
},
},
},
},
},
});

const ruleBuckets = response.body.aggregations?.rules?.buckets ?? [];
const statusHits = ruleBuckets.map((bucket: any) => {
const [firstHit] = bucket?.latest_status?.hits?.hits ?? [];
return firstHit ?? null;
});
const statuses = statusHits.map((hit: any) => {
return hit?._source?.['siem-detection-engine-rule-status'] ?? null;
});

return statuses.filter(Boolean) as IRuleSavedAttributesSavedObjectAttributes[];
};
Comment on lines +204 to +258
Copy link
Contributor Author

@banderror banderror Feb 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Please review this from the security standpoint (access privileges, isolation (spaces), error handling, potential issues, etc.).

Note that:

  • Detection rules are saved objects with "type": "alert" and "alert.alertTypeId": "siem.signals".
  • Rule statuses are saved objects with "type": "siem-detection-engine-rule-status".
  • Rule statuses have "foreign keys" pointing to detections rules via their ids. If a rule has an "_id": "alert:b921e020-6a13-11eb-bf61-5f56ac457c9b", its status object will contain a foreign key "alertId": "b921e020-6a13-11eb-bf61-5f56ac457c9b" in its saved object attributes.
  • In this endpoint handler ruleIds are not coming from request parameters. We retrieve them via alerting client which uses saved objects client. Which should handle spaces and RBAC for us correctly (I guess?).
  • See more notes on our data model in the PR description.

18 changes: 18 additions & 0 deletions .../security_solution/server/lib/detection_engine/routes/rules/find_rules_route_actions.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
[
null,
{
"id": "fd61c550-66e1-11eb-89d7-93d66aca027c",
"actions": [
{
"group": "default",
"id": "7a8a4990-66e1-11eb-89d7-93d66aca027c",
"params": {
"body": "Rule:\n{{context.rule.name}}\n{{context.rule.id}}\n\nAlert:\n{{alertName}}\n{{alertId}}\n{{alertActionGroupName}}\n\nTags:\n{{tags}}\n\n{{date}}"
},
"action_type_id": ".webhook"
}
],
"alertThrottle": null,
"ruleThrottle": "rule"
}
]
Loading