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] [Detections] EQL Rule Creation #76831

Merged
merged 17 commits into from
Sep 15, 2020
Merged

[Security Solution] [Detections] EQL Rule Creation #76831

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
cce81d4
Use existing predicate helper to avoid hardcoded strings
rylnd Sep 4, 2020
9d5ce1c
Render our field components with React.createElement
rylnd Sep 4, 2020
3e1a793
WIP: EQL Rules can be created
rylnd Sep 4, 2020
f71af23
Merge branch 'master' into eql_rules_creation
rylnd Sep 8, 2020
2314ba3
Add unit tests for changes to schema + helpers
rylnd Sep 8, 2020
e162d53
Add unit tests for new EQL query input component
rylnd Sep 9, 2020
872bbe6
Add integration test for EQL Rule creation
rylnd Sep 9, 2020
3a47b2f
Use predicate helper
rylnd Sep 9, 2020
1611b52
Throw an error if an EQL Rule is executed
rylnd Sep 9, 2020
8d88dcb
Fix failing tests
rylnd Sep 9, 2020
c85ffcf
Audit our rule statements/switches
rylnd Sep 9, 2020
5094b04
Merge branch 'master' into eql_rules_creation
rylnd Sep 9, 2020
a9a6703
Add tests around use of custom components with UseField
rylnd Sep 10, 2020
ef64b1f
Fix typo
rylnd Sep 10, 2020
3759011
Merge branch 'master' into eql_rules_creation
elasticmachine Sep 14, 2020
35b00cd
Add keys to UseField to ensure unmount
rylnd Sep 14, 2020
b9b590b
Merge branch 'master' into eql_rules_creation
elasticmachine Sep 15, 2020
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
8 changes: 4 additions & 4 deletions src/plugins/es_ui_shared/static/forms/hook_form_lib/components/use_field.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ function UseFieldComp<T = unknown>(props: Props<T>) {
} = props;

const form = useFormContext();
const componentToRender = component ?? 'input';
const ComponentToRender = component ?? 'input';
// For backward compatibility we merge the "componentProps" prop into the "rest"
const propsToForward =
componentProps !== undefined ? { ...componentProps, ...rest } : { ...rest };
Expand Down Expand Up @@ -91,9 +91,9 @@ function UseFieldComp<T = unknown>(props: Props<T>) {
return children!(field);
}

if (componentToRender === 'input') {
if (ComponentToRender === 'input') {
return (
<input
<ComponentToRender
type={field.type}
onChange={field.onChange}
value={(field.value as unknown) as string}
Expand All @@ -102,7 +102,7 @@ function UseFieldComp<T = unknown>(props: Props<T>) {
);
}

return componentToRender({ field, ...propsToForward });
return <ComponentToRender {...{ field, ...propsToForward }} />;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sebelga I haven't yet added a test to encompass this change, but you can read about the errant behavior in 9d5ce1c. My first thought was simply to assert that React.createElement is called when a custom component is used, but I'd appreciate your thoughts!

Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer to test the different use cases instead of the implementation. So I would have a dummy form with 3 types of fields (or more if there are more use cases) and make sure they render correctly (are present in the DOM)

  • no component passed --> renders an input in the dom
  • component passed --> renders it
  • memoized component passed --> renders it

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 mind explaining what you mean by "Cannot switch between UseField components"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sebelga in this PR we're swapping between two different field components based on which rule type the user has chosen. link. This causes react warnings if the components are not invoked with React.createElement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sebelga tests added here: a9a6703

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool thanks for the tests @rylnd !

For the swap of components, we took the habit of adding a key to the UseField being swapped. To make sure the component unmounts and re-mounts.

}

export const UseField = React.memo(UseFieldComp) as typeof UseFieldComp;
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/security_solution/common/detection_engine/schemas/common/schemas.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ export type Query = t.TypeOf<typeof query>;
export const queryOrUndefined = t.union([query, t.undefined]);
export type QueryOrUndefined = t.TypeOf<typeof queryOrUndefined>;

export const language = t.keyof({ kuery: null, lucene: null });
export const language = t.keyof({ eql: null, kuery: null, lucene: null });
export type Language = t.TypeOf<typeof language>;

export const languageOrUndefined = t.union([language, t.undefined]);
Expand Down Expand Up @@ -294,6 +294,7 @@ export const toOrUndefined = t.union([to, t.undefined]);
export type ToOrUndefined = t.TypeOf<typeof toOrUndefined>;

export const type = t.keyof({
eql: null,
Copy link
Contributor

Choose a reason for hiding this comment

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

does it make sense from a schema perspective to have eql be its own type? vs just another language for query

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rw-access that's an interesting point. In general, a rule's type could be derived from its other fields, and we're mainly using it as a shortcut for that determination. I don't believe there's currently any ambiguity that the type field clarifies.

Another consideration is signal.rule.type, which we currently use to e.g. determine what actions can be taken on a given alert. We could compute the type when the alert is generated so as not to have to fetch the rule and derive its type, but that would introduce some inconsistency with signal.rule being a strict subset of the rule's fields.

Since both the querying and the alert generation are unique to EQL, I'm inclined to promote that "up" to the rule type as opposed to just query/language, but I'm happy to hear your thoughts.

Copy link
Contributor

@rw-access rw-access Sep 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Since both the querying and the alert generation are unique to EQL, I'm inclined to promote that "up" to the rule type as opposed to just query/language, but I'm happy to hear your thoughts.

Yeah, I think EQL is very special snowflake compared to Lucene/KQL, because of the complexity in its response format. Like you said, I think that it makes sense that even though the schema of the rule itself is otherwise identical to KQL/Lucene, it's processed very differently. Great point.

I wonder if that adds some more precision to what type: query means. To me, I now interpret that as "find the documents that match this boolean condition". I think that helps me mentally understand the difference better.

Thanks, this gave me some mental clarity. It could be worthwhile to communicate some of that reasoning in the "Rule type" part of the UI. I think "Use KQL or Lucene to detect issues across indices." makes me think about cluster health. Maybe "Use KQL or Lucene to alert when documents match a condition" could help, but I'll defer to you and the UX team.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those are great points! I've opened a ticket to discuss our qualification of the different rule types and how we present that to the user: #77250

machine_learning: null,
query: null,
saved_query: null,
Expand Down
12 changes: 7 additions & 5 deletions ...solution/common/detection_engine/schemas/request/add_prepackaged_rules_type_dependents.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { isMlRule } from '../../../machine_learning/helpers';
import { isThresholdRule } from '../../utils';
import { AddPrepackagedRulesSchema } from './add_prepackaged_rules_schema';

export const validateAnomalyThreshold = (rule: AddPrepackagedRulesSchema): string[] => {
if (rule.type === 'machine_learning') {
if (isMlRule(rule.type)) {
if (rule.anomaly_threshold == null) {
return ['when "type" is "machine_learning" anomaly_threshold is required'];
} else {
Expand All @@ -19,7 +21,7 @@ export const validateAnomalyThreshold = (rule: AddPrepackagedRulesSchema): strin
};

export const validateQuery = (rule: AddPrepackagedRulesSchema): string[] => {
if (rule.type === 'machine_learning') {
if (isMlRule(rule.type)) {
if (rule.query != null) {
return ['when "type" is "machine_learning", "query" cannot be set'];
} else {
Expand All @@ -31,7 +33,7 @@ export const validateQuery = (rule: AddPrepackagedRulesSchema): string[] => {
};

export const validateLanguage = (rule: AddPrepackagedRulesSchema): string[] => {
if (rule.type === 'machine_learning') {
if (isMlRule(rule.type)) {
if (rule.language != null) {
return ['when "type" is "machine_learning", "language" cannot be set'];
} else {
Expand All @@ -55,7 +57,7 @@ export const validateSavedId = (rule: AddPrepackagedRulesSchema): string[] => {
};

export const validateMachineLearningJobId = (rule: AddPrepackagedRulesSchema): string[] => {
if (rule.type === 'machine_learning') {
if (isMlRule(rule.type)) {
if (rule.machine_learning_job_id == null) {
return ['when "type" is "machine_learning", "machine_learning_job_id" is required'];
} else {
Expand Down Expand Up @@ -93,7 +95,7 @@ export const validateTimelineTitle = (rule: AddPrepackagedRulesSchema): string[]
};

export const validateThreshold = (rule: AddPrepackagedRulesSchema): string[] => {
if (rule.type === 'threshold') {
if (isThresholdRule(rule.type)) {
if (!rule.threshold) {
return ['when "type" is "threshold", "threshold" is required'];
} else if (rule.threshold.value <= 0) {
Expand Down
12 changes: 7 additions & 5 deletions ...security_solution/common/detection_engine/schemas/request/create_rules_type_dependents.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { isMlRule } from '../../../machine_learning/helpers';
import { isThresholdRule } from '../../utils';
import { CreateRulesSchema } from './create_rules_schema';

export const validateAnomalyThreshold = (rule: CreateRulesSchema): string[] => {
if (rule.type === 'machine_learning') {
if (isMlRule(rule.type)) {
if (rule.anomaly_threshold == null) {
return ['when "type" is "machine_learning" anomaly_threshold is required'];
} else {
Expand All @@ -19,7 +21,7 @@ export const validateAnomalyThreshold = (rule: CreateRulesSchema): string[] => {
};

export const validateQuery = (rule: CreateRulesSchema): string[] => {
if (rule.type === 'machine_learning') {
if (isMlRule(rule.type)) {
if (rule.query != null) {
return ['when "type" is "machine_learning", "query" cannot be set'];
} else {
Expand All @@ -31,7 +33,7 @@ export const validateQuery = (rule: CreateRulesSchema): string[] => {
};

export const validateLanguage = (rule: CreateRulesSchema): string[] => {
if (rule.type === 'machine_learning') {
if (isMlRule(rule.type)) {
if (rule.language != null) {
return ['when "type" is "machine_learning", "language" cannot be set'];
} else {
Expand All @@ -55,7 +57,7 @@ export const validateSavedId = (rule: CreateRulesSchema): string[] => {
};

export const validateMachineLearningJobId = (rule: CreateRulesSchema): string[] => {
if (rule.type === 'machine_learning') {
if (isMlRule(rule.type)) {
if (rule.machine_learning_job_id == null) {
return ['when "type" is "machine_learning", "machine_learning_job_id" is required'];
} else {
Expand Down Expand Up @@ -93,7 +95,7 @@ export const validateTimelineTitle = (rule: CreateRulesSchema): string[] => {
};

export const validateThreshold = (rule: CreateRulesSchema): string[] => {
if (rule.type === 'threshold') {
if (isThresholdRule(rule.type)) {
if (!rule.threshold) {
return ['when "type" is "threshold", "threshold" is required'];
} else if (rule.threshold.value <= 0) {
Expand Down
12 changes: 7 additions & 5 deletions ...security_solution/common/detection_engine/schemas/request/import_rules_type_dependents.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { isMlRule } from '../../../machine_learning/helpers';
import { isThresholdRule } from '../../utils';
import { ImportRulesSchema } from './import_rules_schema';

export const validateAnomalyThreshold = (rule: ImportRulesSchema): string[] => {
if (rule.type === 'machine_learning') {
if (isMlRule(rule.type)) {
if (rule.anomaly_threshold == null) {
return ['when "type" is "machine_learning" anomaly_threshold is required'];
} else {
Expand All @@ -19,7 +21,7 @@ export const validateAnomalyThreshold = (rule: ImportRulesSchema): string[] => {
};

export const validateQuery = (rule: ImportRulesSchema): string[] => {
if (rule.type === 'machine_learning') {
if (isMlRule(rule.type)) {
if (rule.query != null) {
return ['when "type" is "machine_learning", "query" cannot be set'];
} else {
Expand All @@ -31,7 +33,7 @@ export const validateQuery = (rule: ImportRulesSchema): string[] => {
};

export const validateLanguage = (rule: ImportRulesSchema): string[] => {
if (rule.type === 'machine_learning') {
if (isMlRule(rule.type)) {
if (rule.language != null) {
return ['when "type" is "machine_learning", "language" cannot be set'];
} else {
Expand All @@ -55,7 +57,7 @@ export const validateSavedId = (rule: ImportRulesSchema): string[] => {
};

export const validateMachineLearningJobId = (rule: ImportRulesSchema): string[] => {
if (rule.type === 'machine_learning') {
if (isMlRule(rule.type)) {
if (rule.machine_learning_job_id == null) {
return ['when "type" is "machine_learning", "machine_learning_job_id" is required'];
} else {
Expand Down Expand Up @@ -93,7 +95,7 @@ export const validateTimelineTitle = (rule: ImportRulesSchema): string[] => {
};

export const validateThreshold = (rule: ImportRulesSchema): string[] => {
if (rule.type === 'threshold') {
if (isThresholdRule(rule.type)) {
if (!rule.threshold) {
return ['when "type" is "threshold", "threshold" is required'];
} else if (rule.threshold.value <= 0) {
Expand Down
8 changes: 5 additions & 3 deletions .../security_solution/common/detection_engine/schemas/request/patch_rules_type_dependents.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { isMlRule } from '../../../machine_learning/helpers';
import { isThresholdRule } from '../../utils';
import { PatchRulesSchema } from './patch_rules_schema';

export const validateQuery = (rule: PatchRulesSchema): string[] => {
if (rule.type === 'machine_learning') {
if (isMlRule(rule.type)) {
if (rule.query != null) {
return ['when "type" is "machine_learning", "query" cannot be set'];
} else {
Expand All @@ -19,7 +21,7 @@ export const validateQuery = (rule: PatchRulesSchema): string[] => {
};

export const validateLanguage = (rule: PatchRulesSchema): string[] => {
if (rule.type === 'machine_learning') {
if (isMlRule(rule.type)) {
if (rule.language != null) {
return ['when "type" is "machine_learning", "language" cannot be set'];
} else {
Expand Down Expand Up @@ -67,7 +69,7 @@ export const validateId = (rule: PatchRulesSchema): string[] => {
};

export const validateThreshold = (rule: PatchRulesSchema): string[] => {
if (rule.type === 'threshold') {
if (isThresholdRule(rule.type)) {
if (!rule.threshold) {
return ['when "type" is "threshold", "threshold" is required'];
} else if (rule.threshold.value <= 0) {
Expand Down
12 changes: 7 additions & 5 deletions ...security_solution/common/detection_engine/schemas/request/update_rules_type_dependents.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { isMlRule } from '../../../machine_learning/helpers';
import { isThresholdRule } from '../../utils';
import { UpdateRulesSchema } from './update_rules_schema';

export const validateAnomalyThreshold = (rule: UpdateRulesSchema): string[] => {
if (rule.type === 'machine_learning') {
if (isMlRule(rule.type)) {
if (rule.anomaly_threshold == null) {
return ['when "type" is "machine_learning" anomaly_threshold is required'];
} else {
Expand All @@ -19,7 +21,7 @@ export const validateAnomalyThreshold = (rule: UpdateRulesSchema): string[] => {
};

export const validateQuery = (rule: UpdateRulesSchema): string[] => {
if (rule.type === 'machine_learning') {
if (isMlRule(rule.type)) {
if (rule.query != null) {
return ['when "type" is "machine_learning", "query" cannot be set'];
} else {
Expand All @@ -31,7 +33,7 @@ export const validateQuery = (rule: UpdateRulesSchema): string[] => {
};

export const validateLanguage = (rule: UpdateRulesSchema): string[] => {
if (rule.type === 'machine_learning') {
if (isMlRule(rule.type)) {
if (rule.language != null) {
return ['when "type" is "machine_learning", "language" cannot be set'];
} else {
Expand All @@ -55,7 +57,7 @@ export const validateSavedId = (rule: UpdateRulesSchema): string[] => {
};

export const validateMachineLearningJobId = (rule: UpdateRulesSchema): string[] => {
if (rule.type === 'machine_learning') {
if (isMlRule(rule.type)) {
if (rule.machine_learning_job_id == null) {
return ['when "type" is "machine_learning", "machine_learning_job_id" is required'];
} else {
Expand Down Expand Up @@ -103,7 +105,7 @@ export const validateId = (rule: UpdateRulesSchema): string[] => {
};

export const validateThreshold = (rule: UpdateRulesSchema): string[] => {
if (rule.type === 'threshold') {
if (isThresholdRule(rule.type)) {
if (!rule.threshold) {
return ['when "type" is "threshold", "threshold" is required'];
} else if (rule.threshold.value <= 0) {
Expand Down
10 changes: 10 additions & 0 deletions ...k/plugins/security_solution/common/detection_engine/schemas/response/rules_schema.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,16 @@ describe('rules_schema', () => {
expect(fields.length).toEqual(2);
});

test('should return two fields for a rule of type "eql"', () => {
const fields = addQueryFields({ type: 'eql' });
expect(fields.length).toEqual(2);
});

test('should return two fields for a rule of type "threshold"', () => {
const fields = addQueryFields({ type: 'threshold' });
expect(fields.length).toEqual(2);
});

test('should return two fields for a rule of type "saved_query"', () => {
const fields = addQueryFields({ type: 'saved_query' });
expect(fields.length).toEqual(2);
Expand Down
9 changes: 5 additions & 4 deletions x-pack/plugins/security_solution/common/detection_engine/schemas/response/rules_schema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import * as t from 'io-ts';
import { isObject } from 'lodash/fp';
import { Either, left, fold } from 'fp-ts/lib/Either';
import { pipe } from 'fp-ts/lib/pipeable';
import { typeAndTimelineOnlySchema, TypeAndTimelineOnly } from './type_timeline_only_schema';
import { isMlRule } from '../../../machine_learning/helpers';

import { isMlRule } from '../../../machine_learning/helpers';
import { isThresholdRule } from '../../utils';
import {
actions,
anomaly_threshold,
Expand Down Expand Up @@ -66,6 +66,7 @@ import {
DefaultRiskScoreMappingArray,
DefaultSeverityMappingArray,
} from '../types';
import { typeAndTimelineOnlySchema, TypeAndTimelineOnly } from './type_timeline_only_schema';

/**
* This is the required fields for the rules schema response. Put all required properties on
Expand Down Expand Up @@ -205,7 +206,7 @@ export const addTimelineTitle = (typeAndTimelineOnly: TypeAndTimelineOnly): t.Mi
};

export const addQueryFields = (typeAndTimelineOnly: TypeAndTimelineOnly): t.Mixed[] => {
if (['query', 'saved_query', 'threshold'].includes(typeAndTimelineOnly.type)) {
if (['eql', 'query', 'saved_query', 'threshold'].includes(typeAndTimelineOnly.type)) {
return [
t.exact(t.type({ query: dependentRulesSchema.props.query })),
t.exact(t.type({ language: dependentRulesSchema.props.language })),
Expand All @@ -229,7 +230,7 @@ export const addMlFields = (typeAndTimelineOnly: TypeAndTimelineOnly): t.Mixed[]
};

export const addThresholdFields = (typeAndTimelineOnly: TypeAndTimelineOnly): t.Mixed[] => {
if (typeAndTimelineOnly.type === 'threshold') {
if (isThresholdRule(typeAndTimelineOnly.type)) {
return [
t.exact(t.type({ threshold: dependentRulesSchema.props.threshold })),
t.exact(t.partial({ saved_id: dependentRulesSchema.props.saved_id })),
Expand Down
4 changes: 3 additions & 1 deletion x-pack/plugins/security_solution/common/detection_engine/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,6 @@ export const hasNestedEntry = (entries: EntriesArray): boolean => {
return found.length > 0;
};

export const isThresholdRule = (ruleType: Type) => ruleType === 'threshold';
export const isEqlRule = (ruleType: Type | undefined) => ruleType === 'eql';
export const isThresholdRule = (ruleType: Type | undefined) => ruleType === 'threshold';
export const isQueryRule = (ruleType: Type | undefined) => ruleType === 'query';
2 changes: 1 addition & 1 deletion x-pack/plugins/security_solution/common/machine_learning/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,4 @@ export const isJobFailed = (jobState: string, datafeedState: string): boolean =>
return failureStates.includes(jobState) || failureStates.includes(datafeedState);
};

export const isMlRule = (ruleType: Type) => ruleType === 'machine_learning';
export const isMlRule = (ruleType: Type | undefined) => ruleType === 'machine_learning';
Loading