[D4C] additional error handling for 'block' action added + policy editor UI fixes

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'];

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();
+  });
+});

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;

x-pack/plugins/cloud_defend/public/components/control_general_view/index.test.tsx

@@ -99,28 +99,6 @@ describe('<ControlGeneralView />', () => {
     }
   });
 
-  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(
-      <WrappedComponent policy={getCloudDefendNewPolicyMock(testPolicy)} />
-    );
-
-    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(<WrappedComponent />);