Skip to content

Commit

Permalink
[Security Solution][Endpoint] Improve endpoint performance with TA wi…
Browse files Browse the repository at this point in the history
…ldcard paths (#120349)

* Show full executable names in placeholder for wildcard paths

fixes elastic/security-team/issues/2293

* Show soft warning when wildcard also in executable names

fixes elastic/security-team/issues/2293

* add wildcard path entries to fake TA list

refs elastic/security-team/issues/2293

* Append a process.name entry when executable name in wildcard path

fixes elastic/security-team/issues/2293

* commit using [email protected]

* linux should always use  `_cased` types

review changes

* use better TS

* use matcher functions to compute operator value for linux

review suggestions

* use path to extract filenames on server side

review suggestions

* improve regex for windows and unix filepaths

review suggestions

* update test mocks

review changes

* update regex to match multi spaces and single chars with spaces in filenames

* add comment to explain

review suggestions

* update copy

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
ashokaditya and kibanamachine authored Dec 16, 2021
1 parent b311c40 commit 1dd669c
Show file tree
Hide file tree
Showing 14 changed files with 296 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ export class ExceptionsListItemGenerator extends BaseDataGenerator<ExceptionList
field: ConditionEntryField.PATH,
operator: 'included',
type: 'match',
value: '/one/two/three',
value:
overrides.os_types && overrides.os_types[0] === 'windows'
? 'c:\\fol\\bin.exe'
: '/one/two/three',
},
],
id: this.seededUUIDv4(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,29 @@ export class TrustedAppGenerator extends BaseDataGenerator<TrustedApp> {
...(scopeType === 'policy' ? { policies: this.randomArray(5, () => this.randomUUID()) } : {}),
}) as EffectScope;

const os = overrides.os ?? 'windows';
const pathEntry = this.randomChoice([
{
field: ConditionEntryField.PATH,
operator: 'included',
type: 'match',
value: os !== 'windows' ? '/one/two/three' : 'c:\\fol\\bin.exe',
},
{
field: ConditionEntryField.PATH,
operator: 'included',
type: 'wildcard',
value: os !== 'windows' ? '/one/t*/*re/three.app' : 'c:\\fol*\\*ub*\\bin.exe',
},
]);

// TS types are conditional when it comes to the combination of OS and ENTRIES
// @ts-expect-error TS2322
return merge(
{
description: `Generator says we trust ${name}`,
name,
os: this.randomOSFamily(),
os,
effectScope,
entries: [
{
Expand All @@ -82,12 +98,7 @@ export class TrustedAppGenerator extends BaseDataGenerator<TrustedApp> {
type: 'match',
value: '1234234659af249ddf3e40864e9fb241',
},
{
field: ConditionEntryField.PATH,
operator: 'included',
type: 'match',
value: '/one/two/three',
},
pathEntry,
],
},
overrides
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { isPathValid } from './validations';
import { isPathValid, hasSimpleExecutableName } from './validations';
import { OperatingSystem, ConditionEntryField } from '../../types';

describe('Unacceptable Windows wildcard paths', () => {
Expand Down Expand Up @@ -504,3 +504,58 @@ describe('Unacceptable Mac/Linux exact paths', () => {
).toEqual(false);
});
});

describe('Executable filenames with wildcard PATHS', () => {
it('should return TRUE when MAC/LINUX wildcard paths have an executable name', () => {
expect(
hasSimpleExecutableName({
os: OperatingSystem.LINUX,
type: 'wildcard',
value: '/opt/*/app',
})
).toEqual(true);
expect(
hasSimpleExecutableName({
os: OperatingSystem.MAC,
type: 'wildcard',
value: '/op*/**/app.dmg',
})
).toEqual(true);
});

it('should return TRUE when WINDOWS wildcards paths have a executable name', () => {
expect(
hasSimpleExecutableName({
os: OperatingSystem.WINDOWS,
type: 'wildcard',
value: 'c:\\**\\path.exe',
})
).toEqual(true);
});

it('should return FALSE when MAC/LINUX wildcard paths have a wildcard in executable name', () => {
expect(
hasSimpleExecutableName({
os: OperatingSystem.LINUX,
type: 'wildcard',
value: '/op/*/*pp',
})
).toEqual(false);
expect(
hasSimpleExecutableName({
os: OperatingSystem.MAC,
type: 'wildcard',
value: '/op*/b**/ap.m**',
})
).toEqual(false);
});
it('should return FALSE when WINDOWS wildcards paths have a wildcard in executable name', () => {
expect(
hasSimpleExecutableName({
os: OperatingSystem.WINDOWS,
type: 'wildcard',
value: 'c:\\**\\pa*h.exe',
})
).toEqual(false);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,35 @@ export const getDuplicateFields = (entries: ConditionEntry[]) => {
.map((entry) => entry[0]);
};

/*
* regex to match executable names
* starts matching from the eol of the path
* file names with a single or multiple spaces (for spaced names)
* and hyphens and combinations of these that produce complex names
* such as:
* c:\home\lib\dmp.dmp
* c:\home\lib\my-binary-app-+/ some/ x/ dmp.dmp
* /home/lib/dmp.dmp
* /home/lib/my-binary-app+-\ some\ x\ dmp.dmp
*/
const WIN_EXEC_PATH = /\\(\w+|\w*[\w+|-]+\/ +)+\w+[\w+|-]+\.*\w+$/i;
const UNIX_EXEC_PATH = /(\/|\w*[\w+|-]+\\ +)+\w+[\w+|-]+\.*\w*$/i;

export const hasSimpleExecutableName = ({
os,
type,
value,
}: {
os: OperatingSystem;
type: TrustedAppEntryTypes;
value: string;
}): boolean => {
if (type === 'wildcard') {
return os === OperatingSystem.WINDOWS ? WIN_EXEC_PATH.test(value) : UNIX_EXEC_PATH.test(value);
}
return true;
};

export const isPathValid = ({
os,
field,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ import { ConditionEntryField, OperatingSystem, TrustedAppEntryTypes } from '../e

export const getPlaceholderText = () => ({
windows: {
wildcard: 'C:\\sample\\**\\*',
wildcard: 'C:\\sample\\**\\path.exe',
exact: 'C:\\sample\\path.exe',
},
others: {
wildcard: '/opt/**/*',
wildcard: '/opt/**/app',
exact: '/opt/bin',
},
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ describe.each([
render();

expect(renderResult.getByTestId('testCard-criteriaConditions').textContent).toEqual(
' OSIS WindowsAND process.hash.*IS 1234234659af249ddf3e40864e9fb241AND process.executable.caselessIS /one/two/three'
' OSIS WindowsAND process.hash.*IS 1234234659af249ddf3e40864e9fb241AND process.executable.caselessIS c:\\fol\\bin.exe'
);
});

Expand All @@ -124,7 +124,7 @@ describe.each([
render();

expect(renderResult.getByTestId('testCard-criteriaConditions').textContent).toEqual(
` OSIS ${OS_LINUX}, ${OS_MAC}, ${OS_WINDOWS}AND process.hash.*IS 1234234659af249ddf3e40864e9fb241AND process.executable.caselessIS /one/two/three`
` OSIS ${OS_LINUX}, ${OS_MAC}, ${OS_WINDOWS}AND process.hash.*IS 1234234659af249ddf3e40864e9fb241AND process.executable.caselessIS c:\\fol\\bin.exe`
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ describe.each([
await fireEvent.click(renderResult.getByTestId('testCard-collapse'));
});
expect(renderResult.getByTestId('testCard-criteriaConditions').textContent).toEqual(
' OSIS WindowsAND process.hash.*IS 1234234659af249ddf3e40864e9fb241AND process.executable.caselessIS /one/two/three'
' OSIS WindowsAND process.hash.*IS 1234234659af249ddf3e40864e9fb241AND process.executable.caselessIS c:\\fol\\bin.exe'
);
expect(renderResult.getByTestId('testCard-collapse').textContent).toEqual('Hide details');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export const getExceptionProviderMock = (): ExceptionListItemSchema => {
field: 'process.executable.caseless',
operator: 'included',
type: 'match',
value: '/one/two/three',
value: 'c:\\fol\\bin.exe',
},
],
tags: ['policy:all'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export const trustedAppsGetListHttpMocks =
const generator = new ExceptionsListItemGenerator('seed');
const perPage = apiQueryParams.per_page ?? 10;
const data = Array.from({ length: Math.min(perPage, 50) }, () =>
generator.generate({ list_id: ENDPOINT_TRUSTED_APPS_LIST_ID })
generator.generate({ list_id: ENDPOINT_TRUSTED_APPS_LIST_ID, os_types: ['windows'] })
);

// FIXME: remove hard-coded IDs below adn get them from the new FleetPackagePolicyGenerator (#2262)
Expand Down Expand Up @@ -128,7 +128,9 @@ export const trustedAppsGetOneHttpMocks =
method: 'get',
handler: ({ query }): ExceptionListItemSchema => {
const apiQueryParams = query as ReadExceptionListItemSchema;
const exceptionItem = new ExceptionsListItemGenerator('seed').generate();
const exceptionItem = new ExceptionsListItemGenerator('seed').generate({
os_types: ['windows'],
});

exceptionItem.item_id = apiQueryParams.item_id ?? exceptionItem.item_id;
exceptionItem.namespace_type =
Expand All @@ -155,7 +157,7 @@ export const trustedAppPostHttpMocks = httpHandlerMockFactory<TrustedAppPostHttp
body as string
) as CreateExceptionListItemSchema;
const response: ExceptionListItemSchema = {
...new ExceptionsListItemGenerator('seed').generate(),
...new ExceptionsListItemGenerator('seed').generate({ os_types: ['windows'] }),
...updatedExceptionItem,
};
response.id = path.split('/').pop() ?? response.id;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,9 @@ describe('Policy trusted apps layout', () => {
if (options.query?.filter === hasAnyQuery) {
const exceptionsGenerator = new ExceptionsListItemGenerator('seed');
return {
data: Array.from({ length: 10 }, () => exceptionsGenerator.generate()),
data: Array.from({ length: 10 }, () =>
exceptionsGenerator.generate({ os_types: ['windows'] })
),
total: 10,
page: 0,
per_page: 10,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
import {
isValidHash,
isPathValid,
hasSimpleExecutableName,
} from '../../../../../../common/endpoint/service/trusted_apps/validations';

import { useIsExperimentalFeatureEnabled } from '../../../../../common/hooks/use_experimental_features';
Expand Down Expand Up @@ -136,6 +137,13 @@ const validateFormValues = (values: MaybeImmutable<NewTrustedApp>): ValidationRe
);
} else {
values.entries.forEach((entry, index) => {
const isValidPathEntry = isPathValid({
os: values.os,
field: entry.field,
type: entry.type,
value: entry.value,
});

if (!entry.field || !entry.value.trim()) {
isValid = false;
addResultToValidation(
Expand All @@ -161,9 +169,7 @@ const validateFormValues = (values: MaybeImmutable<NewTrustedApp>): ValidationRe
values: { row: index + 1 },
})
);
} else if (
!isPathValid({ os: values.os, field: entry.field, type: entry.type, value: entry.value })
) {
} else if (!isValidPathEntry) {
addResultToValidation(
validation,
'entries',
Expand All @@ -173,6 +179,22 @@ const validateFormValues = (values: MaybeImmutable<NewTrustedApp>): ValidationRe
values: { row: index + 1 },
})
);
} else if (
isValidPathEntry &&
!hasSimpleExecutableName({ os: values.os, value: entry.value, type: entry.type })
) {
addResultToValidation(
validation,
'entries',
'warnings',
i18n.translate(
'xpack.securitySolution.trustedapps.create.conditionFieldDegradedPerformanceMsg',
{
defaultMessage: `[{row}] A wildcard in the filename will affect the endpoint's performance`,
values: { row: index + 1 },
}
)
);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ describe('When on the Trusted Apps Page', () => {
field: 'process.executable.caseless',
operator: 'included',
type: 'match',
value: '/one/two/three',
value: 'c:\\fol\\bin.exe',
},
],
os_types: ['windows'],
Expand Down
Loading

0 comments on commit 1dd669c

Please sign in to comment.