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.

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

[SIEM][Detection Engine] Add validation for Rule Actions #63332

Conversation

patrykkopycinski
Copy link
Contributor

@patrykkopycinski patrykkopycinski commented Apr 13, 2020

Summary

Before:
Because we are treating ActionForm as a black box and we don't have a way to be notified about the errors we are not able to prevent saving rule with incorrect actions value what can cause not sending the actions and because we don't provide to the user any information about the status of the actions it can cause that the user will not know that there is something wrong with the actions until he takes a look in the logs.

After:
This PR adds validation on the SIEM side to check and display validation to the user. We have leveraged builtin actions validation and extended them with mustache template validation and connector config validation.

screencapture-localhost-5601-app-siem-2020-04-13-14_41_58

Checklist

Delete any items that are not applicable to this PR.

@patrykkopycinski patrykkopycinski added bug Fixes for quality problems that affect the customer experience v8.0.0 v7.7.0 labels Apr 13, 2020
@patrykkopycinski patrykkopycinski requested a review from a team as a code owner April 13, 2020 09:45
@patrykkopycinski patrykkopycinski self-assigned this Apr 13, 2020
@patrykkopycinski patrykkopycinski added the release_note:skip Skip the PR/issue when compiling release notes label Apr 13, 2020
@patrykkopycinski patrykkopycinski changed the title Fix/siem rule actions form validation [SIEM][Detection Engine] Add validation for Rule Actions Apr 13, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/siem (Team:SIEM)

@spong
Copy link
Member

spong commented Apr 14, 2020

In testing the app would crash when selecting an action type, so I wasn't able to make it too far. First time testing I had a connector configured already and was able to add/remove actions (still saw a crash when adding an action I didn't have a connector for), and once I cleared out my connectors selecting any action would crash the app.

Crash after selecting action type:
image

Crash when I had a connector configured and was adding/removing actions:
remove_and_add_action

Crash when no connectors were configured and trying to add a single action:
adding_actions_crash

Stack trace

[Violation] 'change' handler took 234ms
type_registry.ts:47 Uncaught Error: Object type "" is not registered.
    at TypeRegistry.get (type_registry.ts:47)
    at action_form.tsx:531
    at Array.map (<anonymous>)
    at ActionForm (action_form.tsx:520)
    at renderWithHooks (react-dom.development.js:16260)
    at updateFunctionComponent (react-dom.development.js:18347)
    at beginWork$1 (react-dom.development.js:20176)
    at HTMLUnknownElement.callCallback (react-dom.development.js:336)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:385)
    at invokeGuardedCallback (react-dom.development.js:440)
get @ type_registry.ts:47
(anonymous) @ action_form.tsx:531
ActionForm @ action_form.tsx:520
renderWithHooks @ react-dom.development.js:16260
updateFunctionComponent @ react-dom.development.js:18347
beginWork$1 @ react-dom.development.js:20176
callCallback @ react-dom.development.js:336
invokeGuardedCallbackDev @ react-dom.development.js:385
invokeGuardedCallback @ react-dom.development.js:440
beginWork$$1 @ react-dom.development.js:25780
performUnitOfWork @ react-dom.development.js:24695
workLoopSync @ react-dom.development.js:24671
performSyncWorkOnRoot @ react-dom.development.js:24270
(anonymous) @ react-dom.development.js:12199
unstable_runWithPriority @ scheduler.development.js:697
runWithPriority$2 @ react-dom.development.js:12149
flushSyncCallbackQueueImpl @ react-dom.development.js:12194
flushSyncCallbackQueue @ react-dom.development.js:12182
discreteUpdates$1 @ react-dom.development.js:24423
discreteUpdates @ react-dom.development.js:1438
dispatchDiscreteEvent @ react-dom.development.js:5881
react_devtools_backend.js:6 The above error occurred in the <ActionForm> component:
    in ActionForm
    in Unknown (created by StepRuleActionsComponent)
    in div (created by EuiForm)
    in EuiForm (created by StepRuleActionsComponent)
    in div (created by EuiForm)
    in EuiForm (created by Form)
    in FormProvider (created by Form)
    in Form (created by StepRuleActionsComponent)
    in div (created by StyledDiv)
    in StyledDiv
    in StyledDiv (created by StepRuleActionsComponent)
    in StepRuleActionsComponent (created by EditRulePageComponent)
    in div (created by EuiPanel)
    in EuiPanel (created by MyPanel)
    in MyPanel (created by StepPanelComponent)
    in StepPanelComponent (created by EditRulePageComponent)
    in div (created by EuiTabbedContent)
    in div (created by EuiTabbedContent)
    in EuiTabbedContent (created by EditRulePageComponent)
    in div (created by Wrapper)
    in Wrapper (created by WrapperPageComponent)
    in WrapperPageComponent (created by EditRulePageComponent)
    in EditRulePageComponent (created by DetectionEngineContainerComponent)
    in Route (created by DetectionEngineContainerComponent)
    in Switch (created by DetectionEngineContainerComponent)
    in ManageUserInfo (created by DetectionEngineContainerComponent)
    in DetectionEngineContainerComponent (created by Context.Consumer)
    in Route (created by Query)
    in Switch (created by Query)
    in Provider (created by App)
    in App (created by ErrorBoundary)
    in ErrorBoundary (created by DragDropContext)
    in DragDropContext
    in Unknown
    in Unknown (created by ConnectFunction)
    in ConnectFunction (created by Query)
    in Query
    in Unknown (created by HomePage)
    in main (created by Main)
    in Main (created by HomePage)
    in div (created by WrappedByAutoSizer)
    in WrappedByAutoSizer (created by HomePage)
    in HomePage (created by PageRouterComponent)
    in Route (created by PageRouterComponent)
    in Switch (created by PageRouterComponent)
    in Router (created by PageRouterComponent)
    in ManageRoutesSpyComponent (created by PageRouterComponent)
    in PageRouterComponent (created by AppPluginRootComponent)
    in Unknown (created by AppPluginRootComponent)
    in ThemeProvider (created by AppPluginRootComponent)
    in ApolloProvider (created by AppPluginRootComponent)
    in Provider (created by AppPluginRootComponent)
    in ManageGlobalToaster (created by AppPluginRootComponent)
    in AppPluginRootComponent (created by StartAppComponent)
    in EuiContext (created by I18nContext)
    in PseudoLocaleWrapper (created by I18nProvider)
    in IntlProvider (created by I18nProvider)
    in I18nProvider (created by I18nContext)
    in I18nContext (created by StartAppComponent)
    in EuiErrorBoundary (created by StartAppComponent)
    in StartAppComponent (created by SiemAppComponent)
    in Provider (created by SiemAppComponent)
    in SiemAppComponent

React will try to recreate this component tree from scratch using the error boundary you provided, ErrorBoundary.
r @ react_devtools_backend.js:6
logCapturedError @ react-dom.development.js:21843
logError @ react-dom.development.js:21880
callback @ react-dom.development.js:23268
callCallback @ react-dom.development.js:13829
commitUpdateEffects @ react-dom.development.js:13867
commitUpdateQueue @ react-dom.development.js:13857
commitLifeCycles @ react-dom.development.js:22135
commitLayoutEffects @ react-dom.development.js:25344
callCallback @ react-dom.development.js:336
invokeGuardedCallbackDev @ react-dom.development.js:385
invokeGuardedCallback @ react-dom.development.js:440
commitRootImpl @ react-dom.development.js:25082
unstable_runWithPriority @ scheduler.development.js:697
runWithPriority$2 @ react-dom.development.js:12149
commitRoot @ react-dom.development.js:24922
finishSyncRender @ react-dom.development.js:24329
performSyncWorkOnRoot @ react-dom.development.js:24307
(anonymous) @ react-dom.development.js:12199
unstable_runWithPriority @ scheduler.development.js:697
runWithPriority$2 @ react-dom.development.js:12149
flushSyncCallbackQueueImpl @ react-dom.development.js:12194
flushSyncCallbackQueue @ react-dom.development.js:12182
discreteUpdates$1 @ react-dom.development.js:24423
discreteUpdates @ react-dom.development.js:1438
dispatchDiscreteEvent @ react-dom.development.js:5881
react-beautiful-dnd.cjs.js:170 Uncaught Error: Object type "" is not registered.
    at TypeRegistry.get (type_registry.ts:47)
    at action_form.tsx:531
    at Array.map (<anonymous>)
    at ActionForm (action_form.tsx:520)
    at renderWithHooks (react-dom.development.js:16260)
    at updateFunctionComponent (react-dom.development.js:18347)
    at beginWork$1 (react-dom.development.js:20176)
    at HTMLUnknownElement.callCallback (react-dom.development.js:336)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:385)
    at invokeGuardedCallback (react-dom.development.js:440)
get @ type_registry.ts:47
(anonymous) @ action_form.tsx:531
ActionForm @ action_form.tsx:520
renderWithHooks @ react-dom.development.js:16260
updateFunctionComponent @ react-dom.development.js:18347
beginWork$1 @ react-dom.development.js:20176
callCallback @ react-dom.development.js:336
invokeGuardedCallbackDev @ react-dom.development.js:385
invokeGuardedCallback @ react-dom.development.js:440
beginWork$$1 @ react-dom.development.js:25780
performUnitOfWork @ react-dom.development.js:24695
workLoopSync @ react-dom.development.js:24671
performSyncWorkOnRoot @ react-dom.development.js:24270
(anonymous) @ react-dom.development.js:12199
unstable_runWithPriority @ scheduler.development.js:697
runWithPriority$2 @ react-dom.development.js:12149
flushSyncCallbackQueueImpl @ react-dom.development.js:12194
flushSyncCallbackQueue @ react-dom.development.js:12182
discreteUpdates$1 @ react-dom.development.js:24423
discreteUpdates @ react-dom.development.js:1438
dispatchDiscreteEvent @ react-dom.development.js:5881
react_devtools_backend.js:6 The above error occurred in the <ErrorBoundary> component:
    in ErrorBoundary (created by DragDropContext)
    in DragDropContext
    in Unknown
    in Unknown (created by ConnectFunction)
    in ConnectFunction (created by Query)
    in Query
    in Unknown (created by HomePage)
    in main (created by Main)
    in Main (created by HomePage)
    in div (created by WrappedByAutoSizer)
    in WrappedByAutoSizer (created by HomePage)
    in HomePage (created by PageRouterComponent)
    in Route (created by PageRouterComponent)
    in Switch (created by PageRouterComponent)
    in Router (created by PageRouterComponent)
    in ManageRoutesSpyComponent (created by PageRouterComponent)
    in PageRouterComponent (created by AppPluginRootComponent)
    in Unknown (created by AppPluginRootComponent)
    in ThemeProvider (created by AppPluginRootComponent)
    in ApolloProvider (created by AppPluginRootComponent)
    in Provider (created by AppPluginRootComponent)
    in ManageGlobalToaster (created by AppPluginRootComponent)
    in AppPluginRootComponent (created by StartAppComponent)
    in EuiContext (created by I18nContext)
    in PseudoLocaleWrapper (created by I18nProvider)
    in IntlProvider (created by I18nProvider)
    in I18nProvider (created by I18nContext)
    in I18nContext (created by StartAppComponent)
    in EuiErrorBoundary (created by StartAppComponent)
    in StartAppComponent (created by SiemAppComponent)
    in Provider (created by SiemAppComponent)
    in SiemAppComponent

React will try to recreate this component tree from scratch using the error boundary you provided, EuiErrorBoundary.

Debugging this to ActionForm:

const actionTypeName = actionTypesIndex
? actionTypesIndex[actionItem.actionTypeId].name
: actionItem.actionTypeId;

we see actionTypeId is undefined, and thus it can't find the corresponding actionTypeName.

Tracing this further up the stack to where actions is set on ActionForm, we see this memo which is keeping actions in sync with field.value:

const actions: AlertAction[] = useMemo(
() => (!isEmpty(field.value) ? (field.value as AlertAction[]) : []),
[field.value]

and then following where field.value gets set leads us to:

const updatedActions = [...(field.value as Array<Partial<AlertAction>>)];
updatedActions[index] = deepMerge(updatedActions[index], { id });
field.setValue(updatedActions);

at which point we see field.value is always being set to [{id: "1"}], which does not have the actionTypeId.

Note: In debugging setActionIdByIndex, I was always seeing the args index=0, id="1" regardless of the action selected. So either this useCallback is stale, or perhaps something is going on in the downstream ActionForm?

In effort to rule out any special state/config I did a fresh checkout of this PR, reset my kibana.index, xpack.task_manager.index and xpack.siem.signalsIndex to a clean state, and I was still seeing these crashes on action type selection.

Going to switch gears to code reviewing, but let's touch base if you're not able to reproduce this in your environment @patrykkopycinski.

elasticmachine and others added 2 commits May 11, 2020 13:09
…-actions-form-validation

# Conflicts:
#	x-pack/plugins/siem/public/alerts/components/rules/rule_actions_field/translations.tsx
#	x-pack/plugins/siem/public/alerts/components/rules/step_rule_actions/schema.test.tsx
#	x-pack/plugins/siem/public/alerts/components/rules/step_rule_actions/utils.test.ts
#	x-pack/plugins/siem/public/alerts/components/rules/step_rule_actions/utils.ts
#	x-pack/plugins/siem/public/pages/detection_engine/rules/components/rule_actions_field/index.test.tsx
#	x-pack/plugins/siem/public/pages/detection_engine/rules/components/rule_actions_field/index.tsx
#	x-pack/plugins/siem/public/pages/detection_engine/rules/components/step_rule_actions/index.test.tsx
#	x-pack/plugins/siem/public/pages/detection_engine/rules/components/step_rule_actions/index.tsx
#	x-pack/plugins/siem/public/pages/detection_engine/rules/components/step_rule_actions/schema.tsx
…-actions-form-validation

# Conflicts:
#	x-pack/plugins/siem/public/alerts/components/rules/step_rule_actions/schema.tsx
#	x-pack/plugins/triggers_actions_ui/public/index.ts
@patrykkopycinski
Copy link
Contributor Author

@elasticmachine merge upstream

@patrykkopycinski
Copy link
Contributor Author

@elasticmachine merge upstream

1 similar comment
@patrykkopycinski
Copy link
Contributor Author

@elasticmachine merge upstream

@patrykkopycinski
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@patrykkopycinski patrykkopycinski merged commit 9df3d9f into elastic:master May 15, 2020
@patrykkopycinski patrykkopycinski deleted the fix/siem-rule-actions-form-validation branch May 15, 2020 15:15
patrykkopycinski added a commit to patrykkopycinski/kibana that referenced this pull request May 15, 2020
patrykkopycinski added a commit to patrykkopycinski/kibana that referenced this pull request May 15, 2020
# Conflicts:
#	x-pack/plugins/siem/public/alerts/components/rules/rule_actions_field/index.test.tsx
#	x-pack/plugins/siem/public/alerts/components/rules/rule_actions_field/index.tsx
#	x-pack/plugins/siem/public/alerts/components/rules/step_rule_actions/index.test.tsx
#	x-pack/plugins/siem/public/alerts/components/rules/step_rule_actions/index.tsx
#	x-pack/plugins/siem/public/alerts/components/rules/step_rule_actions/schema.tsx
#	x-pack/plugins/siem/public/pages/detection_engine/rules/components/rule_actions_field/translations.tsx
#	x-pack/plugins/siem/public/pages/detection_engine/rules/components/step_rule_actions/schema.test.tsx
#	x-pack/plugins/siem/public/pages/detection_engine/rules/components/step_rule_actions/utils.test.ts
#	x-pack/plugins/siem/public/pages/detection_engine/rules/components/step_rule_actions/utils.ts
#	x-pack/plugins/triggers_actions_ui/public/index.ts
patrykkopycinski added a commit to patrykkopycinski/kibana that referenced this pull request May 15, 2020
# Conflicts:
#	x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/components/rule_actions_field/translations.tsx
#	x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/components/step_rule_actions/schema.test.tsx
#	x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/components/step_rule_actions/utils.test.ts
#	x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/components/step_rule_actions/utils.ts
#	x-pack/plugins/siem/public/alerts/components/rules/rule_actions_field/index.test.tsx
#	x-pack/plugins/siem/public/alerts/components/rules/rule_actions_field/index.tsx
#	x-pack/plugins/siem/public/alerts/components/rules/step_rule_actions/index.test.tsx
#	x-pack/plugins/siem/public/alerts/components/rules/step_rule_actions/index.tsx
#	x-pack/plugins/siem/public/alerts/components/rules/step_rule_actions/schema.tsx
#	x-pack/plugins/triggers_actions_ui/public/index.ts
gmmorris added a commit to gmmorris/kibana that referenced this pull request May 15, 2020
* master: (191 commits)
  [Maps] Get number of categories from palette (elastic#66454)
  move oss features registration to KP (elastic#66524)
  [kbn/plugin-helpers] typescript-ify (elastic#66513)
  Add kibana-operations as codeowners for .ci/es-snapshots and vars/ (elastic#66746)
  FTR: move basic services under common folder (elastic#66563)
  Migrate Beats Management UI to KP (elastic#65791)
  [CI] Add 20 minutes to overall build timeout
  lint import from restricted zones for export exressions (elastic#66588)
  [SIEM][Detection Engine] Add validation for Rule Actions (elastic#63332)
  KP plugins shouldn't need package.json (elastic#66654)
  Replace agent metrics link with the new one (elastic#66632)
  [CI] Add one retry to setup step (elastic#66638)
  [CI] Add slack alerts to tracked branch jobs, change default channel, change formatting (elastic#66580)
  [docLinks] Add docLinks to CoreSetup. (elastic#66631)
  [DOCS] Rename monitoring collection from internal to legacy (elastic#65781)
  unskip newsfeed tests (elastic#66562)
  [NP] Migrate uiSettings owned by Kibana app (elastic#64321)
  [ML] Functional tests - stabilize typing in DFA mml input (elastic#66706)
  [Map] return bounding box for static feature collection without joins (elastic#66607)
  remove trailing slash in graph sample data links (elastic#66358)
  ...
@MindyRS MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Sep 23, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:SIEM v7.7.1 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants