diff --git a/x-pack/plugins/cloud_defend/public/common/constants.ts b/x-pack/plugins/cloud_defend/public/common/constants.ts index 48b3ccde479d7..dc71f11ea4577 100644 --- a/x-pack/plugins/cloud_defend/public/common/constants.ts +++ b/x-pack/plugins/cloud_defend/public/common/constants.ts @@ -13,3 +13,5 @@ export const MAX_CONDITION_VALUE_LENGTH_BYTES = 511; // max length for all condi // TODO: temporary until I change condition value length checks in the yaml editor view to be byte based. export const MAX_CONDITION_VALUE_LENGTH = 64; + +export const FIM_OPERATIONS = ['createFile', 'modifyFile', 'deleteFile']; diff --git a/x-pack/plugins/cloud_defend/public/common/utils.test.ts b/x-pack/plugins/cloud_defend/public/common/utils.test.ts index 86f5d06b02365..bb687c480da90 100644 --- a/x-pack/plugins/cloud_defend/public/common/utils.test.ts +++ b/x-pack/plugins/cloud_defend/public/common/utils.test.ts @@ -11,9 +11,13 @@ import { getSelectorConditions, conditionCombinationInvalid, getRestrictedValuesForCondition, + validateBlockRestrictions, + selectorsIncludeConditionsForFIMOperationsUsingSlashStarStar, } from './utils'; import { MOCK_YAML_CONFIGURATION, MOCK_YAML_INVALID_CONFIGURATION } from '../test/mocks'; +import { Selector, Response } from '../types'; + describe('getSelectorsAndResponsesFromYaml', () => { it('converts yaml into arrays of selectors and responses', () => { const { selectors, responses } = getSelectorsAndResponsesFromYaml(MOCK_YAML_CONFIGURATION); @@ -103,3 +107,218 @@ describe('getRestrictedValuesForCondition', () => { expect(values).toEqual(['fork', 'exec']); }); }); + +describe('validateBlockRestrictions', () => { + it('reports an error when some of the FIM selectors arent using targetFilePath', () => { + const selectors: Selector[] = [ + { + type: 'file', + name: 'sel1', + operation: ['createFile'], + }, + { + type: 'file', + name: 'sel2', + operation: ['modifyFile'], + targetFilePath: ['/**'], + }, + ]; + + const responses: Response[] = [ + { + type: 'file', + match: ['sel1', 'sel2'], + actions: ['block', 'alert'], + }, + ]; + + const errors = validateBlockRestrictions(selectors, responses); + + expect(errors).toHaveLength(1); + }); + + it('reports an error when none of the FIM selectors use targetFilePath', () => { + const selectors: Selector[] = [ + { + type: 'file', + name: 'sel1', + operation: ['createFile'], + }, + { + type: 'file', + name: 'sel2', + operation: ['modifyFile'], + }, + ]; + + const responses: Response[] = [ + { + type: 'file', + match: ['sel1', 'sel2'], + actions: ['block', 'alert'], + }, + ]; + + const errors = validateBlockRestrictions(selectors, responses); + + expect(errors).toHaveLength(1); + }); + + it('passes validation when all FIM selectors (response.match) use targetFilePath', () => { + const selectors: Selector[] = [ + { + type: 'file', + name: 'sel1', + operation: ['createFile'], + targetFilePath: ['/usr/bin/**', '/etc/**'], + }, + { + type: 'file', + name: 'sel2', + operation: ['modifyFile'], + targetFilePath: ['/usr/bin/**', '/etc/**'], + }, + ]; + + const responses: Response[] = [ + { + type: 'file', + match: ['sel1', 'sel2'], + actions: ['block', 'alert'], + }, + ]; + + const errors = validateBlockRestrictions(selectors, responses); + + expect(errors).toHaveLength(0); + }); + + it('passes validation with non fim selectors mixed in', () => { + const selectors: Selector[] = [ + { + type: 'file', + name: 'sel1', + operation: ['createFile'], + targetFilePath: ['/usr/bin/**', '/etc/**'], + }, + { + type: 'file', + name: 'sel2', + operation: ['createExecutable', 'modifyExecutable'], // this should be allowed. FIM = createFile, modifyFile, deleteFile + }, + ]; + + const responses: Response[] = [ + { + type: 'file', + match: ['sel1', 'sel2'], + actions: ['block', 'alert'], + }, + ]; + + const errors = validateBlockRestrictions(selectors, responses); + + expect(errors).toHaveLength(0); + }); + + it('passes validation if at least 1 exclude uses targetFilePath', () => { + const selectors: Selector[] = [ + { + type: 'file', + name: 'sel1', + operation: ['createFile'], + }, + { + type: 'file', + name: 'excludePaths', + targetFilePath: ['/etc/**'], + }, + ]; + + const responses: Response[] = [ + { + type: 'file', + match: ['sel1'], + exclude: ['excludePaths'], + actions: ['block', 'alert'], + }, + ]; + + const errors = validateBlockRestrictions(selectors, responses); + + expect(errors).toHaveLength(0); + }); + + it('passes validation if block isnt used', () => { + const selectors: Selector[] = [ + { + type: 'file', + name: 'sel1', + operation: ['createFile'], + }, + ]; + + const responses: Response[] = [ + { + type: 'file', + match: ['sel1'], + exclude: ['excludePaths'], + actions: ['alert'], + }, + ]; + + const errors = validateBlockRestrictions(selectors, responses); + + expect(errors).toHaveLength(0); + }); +}); + +describe('selectorsIncludeConditionsForFIMOperationsUsingSlashStarStar', () => { + it('returns true', () => { + const selectors: Selector[] = [ + { + type: 'file', + name: 'sel1', + operation: ['createFile'], + targetFilePath: ['/**'], + }, + ]; + + const response: Response = { + type: 'file', + match: ['sel1'], + actions: ['block', 'alert'], + }; + + const result = selectorsIncludeConditionsForFIMOperationsUsingSlashStarStar( + selectors, + response.match + ); + + expect(result).toBeTruthy(); + }); + + it('returns false', () => { + const selectors: Selector[] = [ + { + type: 'file', + name: 'sel1', + operation: ['createFile'], + targetFilePath: ['/usr/bin/**'], + }, + ]; + + const response: Response = { + type: 'file', + match: ['sel1'], + actions: ['block', 'alert'], + }; + + const result = selectorsIncludeConditionsForFIMOperationsUsingSlashStarStar( + selectors, + response.match + ); + + expect(result).toBeFalsy(); + }); +}); diff --git a/x-pack/plugins/cloud_defend/public/common/utils.ts b/x-pack/plugins/cloud_defend/public/common/utils.ts index f85f67125b7a9..80a4d7dd6e110 100644 --- a/x-pack/plugins/cloud_defend/public/common/utils.ts +++ b/x-pack/plugins/cloud_defend/public/common/utils.ts @@ -7,6 +7,7 @@ import yaml from 'js-yaml'; import { NewPackagePolicy } from '@kbn/fleet-plugin/public'; import { i18n } from '@kbn/i18n'; +import { errorBlockActionRequiresTargetFilePath } from '../components/control_general_view/translations'; import { Selector, Response, @@ -21,6 +22,7 @@ import { import { MAX_CONDITION_VALUE_LENGTH_BYTES, MAX_SELECTORS_AND_RESPONSES_PER_TYPE, + FIM_OPERATIONS, } from './constants'; export function getInputFromPolicy(policy: NewPackagePolicy, inputId: string) { @@ -72,6 +74,84 @@ export function getTotalsByType(selectors: Selector[], responses: Response[]) { return totalsByType; } +function selectorsIncludeConditionsForFIMOperations( + selectors: Selector[], + conditions: SelectorCondition[], + selectorNames?: string[], + requireForAll?: boolean +) { + const result = + selectorNames && + selectorNames.reduce((prev, cur, index) => { + const selector = selectors.find((s) => s.name === cur); + const usesFIM = selector?.operation?.some((r) => FIM_OPERATIONS.indexOf(r) >= 0); + const hasAllConditions = + !usesFIM || + !!( + selector && + conditions.reduce((p, c) => { + return p && selector.hasOwnProperty(c); + }, true) + ); + + if (requireForAll) { + if (index === 0) { + return hasAllConditions; + } + + return prev && hasAllConditions; + } else { + return prev || hasAllConditions; + } + }, false); + + return !!result; +} + +export function selectorsIncludeConditionsForFIMOperationsUsingSlashStarStar( + selectors: Selector[], + selectorNames?: string[] +) { + const result = + selectorNames && + selectorNames.reduce((prev, cur) => { + const selector = selectors.find((s) => s.name === cur); + const usesFIM = selector?.operation?.some((r) => FIM_OPERATIONS.indexOf(r) >= 0); + return prev || !!(usesFIM && selector?.targetFilePath?.includes('/**')); + }, false); + + return !!result; +} + +export function validateBlockRestrictions(selectors: Selector[], responses: Response[]) { + const errors: string[] = []; + + responses.forEach((response) => { + if (response.actions.includes('block')) { + // check if any selectors are using FIM operations + // and verify that targetFilePath is specfied in all 'match' selectors + // or at least one 'exclude' selector + const excludeUsesTargetFilePath = selectorsIncludeConditionsForFIMOperations( + selectors, + ['targetFilePath'], + response.exclude + ); + const matchSelectorsAllUsingTargetFilePath = selectorsIncludeConditionsForFIMOperations( + selectors, + ['targetFilePath'], + response.match, + true + ); + + if (!(matchSelectorsAllUsingTargetFilePath || excludeUsesTargetFilePath)) { + errors.push(errorBlockActionRequiresTargetFilePath); + } + } + }); + + return errors; +} + export function validateMaxSelectorsAndResponses(selectors: Selector[], responses: Response[]) { const errors: string[] = []; const totalsByType = getTotalsByType(selectors, responses); @@ -223,12 +303,15 @@ export function getYamlFromSelectorsAndResponses(selectors: Selector[], response }, schema); responses.reduce((current, response: any) => { - if (current && response && response.type) { + if (current && response) { if (current[response.type]) { - current[response.type]?.responses.push(response); + current[response.type].responses.push(response); + } else { + current[response.type] = { selectors: [], responses: [response] }; } } + // the 'any' cast is used so we can keep 'response.type' type safe delete response.type; return current; diff --git a/x-pack/plugins/cloud_defend/public/components/control_general_view/index.test.tsx b/x-pack/plugins/cloud_defend/public/components/control_general_view/index.test.tsx index 7606f1e2d0c1d..9244286de0e67 100644 --- a/x-pack/plugins/cloud_defend/public/components/control_general_view/index.test.tsx +++ b/x-pack/plugins/cloud_defend/public/components/control_general_view/index.test.tsx @@ -99,28 +99,6 @@ describe('', () => { } }); - it('should prevent user from adding a process response if no there are no process selectors', async () => { - const testPolicy = ` - file: - selectors: - - name: test - operation: ['createFile'] - responses: - - match: [test] - actions: [alert, block] - `; - - const { getByTestId } = render( - - ); - - userEvent.click(getByTestId('cloud-defend-btnAddResponse')); - await waitFor(() => userEvent.click(getByTestId('cloud-defend-btnAddProcessResponse'))); - - expect(onChange.mock.calls.length).toBe(0); - expect(getByTestId('cloud-defend-btnAddProcessResponse')).toBeDisabled(); - }); - it('updates selector name used in response.match, if its name is changed', async () => { const { getByTitle, getAllByTestId, rerender } = render(); diff --git a/x-pack/plugins/cloud_defend/public/components/control_general_view/index.tsx b/x-pack/plugins/cloud_defend/public/components/control_general_view/index.tsx index 0c126de3b39ec..4aaf7fe3d6f2e 100644 --- a/x-pack/plugins/cloud_defend/public/components/control_general_view/index.tsx +++ b/x-pack/plugins/cloud_defend/public/components/control_general_view/index.tsx @@ -63,24 +63,6 @@ const AddButton = ({ type, onSelectType, selectors, responses }: AddSelectorButt onSelectType('process'); }, [onSelectType]); - const selectorCounts = useMemo(() => { - return selectors.reduce( - (cur, next) => { - if (next.type === 'file') { - cur.file++; - } else { - cur.process++; - } - - return cur; - }, - { - file: 0, - process: 0, - } - ); - }, [selectors]); - const isSelector = type === 'Selector'; const items = [ @@ -88,10 +70,7 @@ const AddButton = ({ type, onSelectType, selectors, responses }: AddSelectorButt key={`addFile${type}`} icon="document" onClick={addFile} - disabled={ - (type === 'Response' && selectorCounts.file === 0) || - totalsByType.file >= MAX_SELECTORS_AND_RESPONSES_PER_TYPE - } + disabled={totalsByType.file >= MAX_SELECTORS_AND_RESPONSES_PER_TYPE} data-test-subj={`cloud-defend-btnAddFile${type}`} > {isSelector ? i18n.fileSelector : i18n.fileResponse} @@ -100,10 +79,7 @@ const AddButton = ({ type, onSelectType, selectors, responses }: AddSelectorButt key={`addProcess${type}`} icon="gear" onClick={addProcess} - disabled={ - (type === 'Response' && selectorCounts.process === 0) || - totalsByType.process >= MAX_SELECTORS_AND_RESPONSES_PER_TYPE - } + disabled={totalsByType.process >= MAX_SELECTORS_AND_RESPONSES_PER_TYPE} data-test-subj={`cloud-defend-btnAddProcess${type}`} > {isSelector ? i18n.processSelector : i18n.processResponse} @@ -282,8 +258,8 @@ export const ControlGeneralView = ({ policy, onChange, show }: ViewDeps) => { delete updatedSelector.hasErrors; } - const updatedSelectors: Selector[] = [...selectors]; - let updatedResponses: Response[] = [...responses]; + const updatedSelectors: Selector[] = JSON.parse(JSON.stringify(selectors)); + let updatedResponses: Response[] = JSON.parse(JSON.stringify(responses)); if (old.name !== updatedSelector.name) { // update all references to this selector in responses @@ -306,7 +282,7 @@ export const ControlGeneralView = ({ policy, onChange, show }: ViewDeps) => { }); } - updatedSelectors[index] = updatedSelector; + updatedSelectors[index] = JSON.parse(JSON.stringify(updatedSelector)); onUpdateYaml(updatedSelectors, updatedResponses); }, [onUpdateYaml, responses, selectors] @@ -314,8 +290,12 @@ export const ControlGeneralView = ({ policy, onChange, show }: ViewDeps) => { const onResponseChange = useCallback( (updatedResponse: Response, index: number) => { - const updatedResponses: Response[] = [...responses]; - updatedResponses[index] = updatedResponse; + if (updatedResponse.hasErrors === false) { + delete updatedResponse.hasErrors; + } + + const updatedResponses: Response[] = JSON.parse(JSON.stringify(responses)); + updatedResponses[index] = { ...updatedResponse }; onUpdateYaml(selectors, updatedResponses); }, [onUpdateYaml, responses, selectors] diff --git a/x-pack/plugins/cloud_defend/public/components/control_general_view/translations.ts b/x-pack/plugins/cloud_defend/public/components/control_general_view/translations.ts index 1cb3d92150b6b..cae9f65e6c565 100644 --- a/x-pack/plugins/cloud_defend/public/components/control_general_view/translations.ts +++ b/x-pack/plugins/cloud_defend/public/components/control_general_view/translations.ts @@ -160,6 +160,29 @@ export const errorActionRequired = i18n.translate('xpack.cloudDefend.errorAction defaultMessage: 'At least one action is required.', }); +export const errorBlockActionRequiresTargetFilePath = i18n.translate( + 'xpack.cloudDefend.errorBlockActionRequiresTargetFilePath', + { + defaultMessage: + 'The "block" action requires targetFilePath be included in all "match" selectors using FIM operations (createFile, modifyFile or deleteFile) or in at least one "exclude" selector.', + } +); + +export const warningFIMUsingSlashStarStarTitle = i18n.translate( + 'xpack.cloudDefend.warningFIMUsingSlashStarStarTitle', + { + defaultMessage: 'Warning: Blocking FIM operations', + } +); + +export const warningFIMUsingSlashStarStarText = i18n.translate( + 'xpack.cloudDefend.warningFIMUsingSlashStarStarText', + { + defaultMessage: + 'It is dangerous to block FIM operations (createFile, modifyFile, deleteFile) using a targetFilePath of /**. This can lead to system instability.', + } +); + export const getSelectorIconTooltip = (type: SelectorType) => { switch (type) { case 'process': diff --git a/x-pack/plugins/cloud_defend/public/components/control_general_view_response/index.tsx b/x-pack/plugins/cloud_defend/public/components/control_general_view_response/index.tsx index af037c70bd71a..d01b8efa219b6 100644 --- a/x-pack/plugins/cloud_defend/public/components/control_general_view_response/index.tsx +++ b/x-pack/plugins/cloud_defend/public/components/control_general_view_response/index.tsx @@ -4,8 +4,9 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ -import React, { useMemo, useState, useCallback, ChangeEvent } from 'react'; +import React, { useMemo, useState, useCallback, ChangeEvent, useEffect } from 'react'; import { + EuiCallOut, EuiIcon, EuiToolTip, EuiText, @@ -36,7 +37,11 @@ import { ControlFormErrorMap, } from '../../types'; import * as i18n from '../control_general_view/translations'; -import { getSelectorTypeIcon } from '../../common/utils'; +import { + getSelectorTypeIcon, + validateBlockRestrictions, + selectorsIncludeConditionsForFIMOperationsUsingSlashStarStar, +} from '../../common/utils'; // max number of names to show in title (in collapsed state) // selectorA, selectorB, selectorC, selectorD [+5] @@ -64,22 +69,61 @@ export const ControlGeneralViewResponse = ({ const [accordionState, setAccordionState] = useState<'open' | 'closed'>( responses.length - 1 === index ? 'open' : 'closed' ); + + const logSelected = response.actions.includes('log'); + const alertSelected = response.actions.includes('alert'); + const blockSelected = response.actions.includes('block'); + + const warnFIMUsingSlashStarStar = useMemo( + () => + blockSelected && + selectorsIncludeConditionsForFIMOperationsUsingSlashStarStar(selectors, response.match), + [blockSelected, response.match, selectors] + ); + + const errors = useMemo(() => { + const errs: ControlFormErrorMap = {}; + + if (response.match.length === 0) { + errs.match = [i18n.errorValueRequired]; + } + + if (response.actions.length === 0) { + errs.actions = [i18n.errorActionRequired]; + } + + if (blockSelected) { + const blockErrors = validateBlockRestrictions(selectors, [response]); + if (blockErrors.length > 0) { + errs.response = blockErrors; + } + } + + return errs; + }, [response, selectors, blockSelected]); + + const errorList = useMemo(() => Object.values(errors), [errors]); + const onResponseChange = useCallback( (resp: Response, i: number) => { - const hasMatch = resp.match.length > 0; - const hasActions = resp.actions.length > 0; - - if (!hasMatch || !hasActions) { + if (errorList.length) { resp.hasErrors = true; - } else { - delete resp.hasErrors; } onChange(resp, i); }, - [onChange] + [errorList.length, onChange] ); + useEffect(() => { + const hasErrors = errorList.length > 0; + const changed = (hasErrors && !response.hasErrors) || (!hasErrors && response.hasErrors); + if (changed) { + response.hasErrors = hasErrors; + onChange(response, index); + } + }, [errorList.length, index, onChange, response]); + const onTogglePopover = useCallback(() => { setPopoverOpen(!isPopoverOpen); }, [isPopoverOpen]); @@ -160,10 +204,6 @@ export const ControlGeneralViewResponse = ({ onResponseChange(updatedResponse, index); }, [index, onResponseChange, response]); - const logSelected = response.actions.includes('log'); - const alertSelected = response.actions.includes('alert'); - const blockSelected = response.actions.includes('block'); - const onToggleAction = useCallback( (e: ChangeEvent) => { const action = e.currentTarget?.id?.match(ACTION_ID_REGEX)?.[1] as ResponseAction; @@ -190,20 +230,6 @@ export const ControlGeneralViewResponse = ({ [index, onResponseChange, response] ); - const errors = useMemo(() => { - const errs: ControlFormErrorMap = {}; - - if (response.match.length === 0) { - errs.match = [i18n.errorValueRequired]; - } - - if (response.actions.length === 0) { - errs.actions = [i18n.errorActionRequired]; - } - - return errs; - }, [response.actions.length, response.match.length]); - const onToggleAccordion = useCallback((isOpen: boolean) => { setAccordionState(isOpen ? 'open' : 'closed'); }, []); @@ -224,8 +250,6 @@ export const ControlGeneralViewResponse = ({ }; }, [accordionState, response.match]); - const errorList = useMemo(() => Object.values(errors), [errors]); - return ( 0}> + {warnFIMUsingSlashStarStar && ( + + +

{i18n.warningFIMUsingSlashStarStarText}

+ + + )} { const onYamlChanges = useCallback( (opts: OnChangeDeps) => { - if (isYamlViewSelected) { - opts.updatedPolicy = policy; - onChange(opts); - setIsValid(opts.isValid); - } + opts.updatedPolicy = policy; + onChange(opts); + setIsValid(opts.isValid); }, - [isYamlViewSelected, onChange, policy] + [onChange, policy] ); return ( @@ -81,9 +79,7 @@ export const ControlSettings = ({ policy, onChange }: SettingsDeps) => { onChange={onGeneralChanges} /> )} - {isYamlViewSelected && ( - - )} + ); diff --git a/x-pack/plugins/cloud_defend/public/components/control_yaml_view/index.tsx b/x-pack/plugins/cloud_defend/public/components/control_yaml_view/index.tsx index 6f48203474f49..b93a281f232f1 100644 --- a/x-pack/plugins/cloud_defend/public/components/control_yaml_view/index.tsx +++ b/x-pack/plugins/cloud_defend/public/components/control_yaml_view/index.tsx @@ -17,6 +17,7 @@ import { validateStringValuesForCondition, getSelectorsAndResponsesFromYaml, validateMaxSelectorsAndResponses, + validateBlockRestrictions, } from '../../common/utils'; import * as i18n from './translations'; import { ViewDeps, SelectorConditionsMap, SelectorCondition } from '../../types'; @@ -45,6 +46,7 @@ export const ControlYamlView = ({ policy, onChange, show }: ViewDeps) => { const { selectors, responses } = getSelectorsAndResponsesFromYaml(value); errors.push(...validateMaxSelectorsAndResponses(selectors, responses)); + errors.push(...validateBlockRestrictions(selectors, responses)); // validate selectors selectors.forEach((selector) => { @@ -74,6 +76,8 @@ export const ControlYamlView = ({ policy, onChange, show }: ViewDeps) => { }, []); useEffect(() => { + if (!show) return; + // for on mount const otherErrors = validateAdditional(configuration); if (otherErrors.length !== additionalErrors.length) { @@ -107,11 +111,19 @@ export const ControlYamlView = ({ policy, onChange, show }: ViewDeps) => { return () => { listener.dispose(); }; - }, [editorErrors, onChange, policy, additionalErrors.length, validateAdditional, configuration]); + }, [ + editorErrors, + onChange, + policy, + additionalErrors.length, + validateAdditional, + configuration, + show, + ]); const onYamlChange = useCallback( (value) => { - if (input?.vars) { + if (show && input?.vars) { input.vars.configuration.value = value; const errs = validateAdditional(value); @@ -123,7 +135,7 @@ export const ControlYamlView = ({ policy, onChange, show }: ViewDeps) => { }); } }, - [editorErrors.length, input?.vars, onChange, policy, validateAdditional] + [editorErrors.length, input?.vars, onChange, policy, show, validateAdditional] ); return (