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

[Security Solution][Endpoint] Allow wildcard in trusted app paths #97623

Merged
merged 39 commits into from
Apr 29, 2021
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
57bf657
show operator dropdown for path field
ashokaditya Apr 20, 2021
dd84507
update translation to use consistent values
ashokaditya Apr 20, 2021
9f72250
update schema to validate path values
ashokaditya Apr 20, 2021
fa9e3ce
add tests for field and operator values
ashokaditya Apr 20, 2021
8c19290
review changes
ashokaditya Apr 21, 2021
1deab39
update schema to enforce dropdown validation for PATH field
ashokaditya Apr 21, 2021
baac452
add tests for schema updates
ashokaditya Apr 21, 2021
5d9e848
optimise dropdown list for re-renders
ashokaditya Apr 21, 2021
9badfcc
Merge branch 'master' into sec-team-543/allow-wildcard-for-paths
kibanamachine Apr 21, 2021
f657b80
align input fields and keep alignments when resized
ashokaditya Apr 22, 2021
2ac56ee
correctly enter operator data on trusted app CRUD
ashokaditya Apr 22, 2021
c047025
update tests
ashokaditya Apr 22, 2021
74d5193
remove redundant code
ashokaditya Apr 22, 2021
dbd0933
better type assertion
ashokaditya Apr 23, 2021
f6cd3a8
move operator options out of component
ashokaditya Apr 23, 2021
a6725b9
derive keys from operator entry field
ashokaditya Apr 23, 2021
4def289
update type
ashokaditya Apr 23, 2021
0a60598
use custom styles for aligning input fields
ashokaditya Apr 23, 2021
c3643f0
add a custom type for trusted_apps operator
ashokaditya Apr 23, 2021
f9cb7ed
add wildcard entry type
ashokaditya Apr 26, 2021
b3f5dc4
use the new entry type
ashokaditya Apr 26, 2021
617aa7e
update tests
ashokaditya Apr 26, 2021
cc001e7
update name for wildcard type so that it can be used also for cased i…
ashokaditya Apr 26, 2021
f744808
Merge branch 'master' into sec-team-543/allow-wildcard-for-paths
kibanamachine Apr 27, 2021
6a874c0
update artifacts to support wildcard entries
ashokaditya Apr 27, 2021
b2cf223
add tests for list schemas
ashokaditya Apr 27, 2021
6f2d0d7
add placeholders for path values
ashokaditya Apr 27, 2021
284352e
ignore type check for now
ashokaditya Apr 27, 2021
bcf615a
add type assertion
ashokaditya Apr 27, 2021
835812f
remove unnecessary test
ashokaditya Apr 27, 2021
dbd3532
fix types
ashokaditya Apr 27, 2021
ddc3d72
add a note to entries
ashokaditya Apr 28, 2021
0bf8199
remove redundant type assertions
ashokaditya Apr 28, 2021
2dc4fd3
move placeholder text logic to utils
ashokaditya Apr 28, 2021
eb8c572
pass the style as prop
ashokaditya Apr 28, 2021
d7cb47b
update api doc
ashokaditya Apr 28, 2021
e8e9ce5
Merge branch 'master' into sec-team-543/allow-wildcard-for-paths
kibanamachine Apr 29, 2021
330731e
make placeholderText a function expression
ashokaditya Apr 29, 2021
4e7dce9
use semantic names for functions
ashokaditya Apr 29, 2021
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
9 changes: 9 additions & 0 deletions x-pack/plugins/lists/common/schemas/common/schemas.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,15 @@ describe('Common schemas', () => {
expect(message.schema).toEqual(payload);
});

test('it should validate for "wildcard_caseless"', () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

I've not added tests like this one in some of the files (handler and mapping) in this commit cause I think it makes more sense to add tests for validation of path values when we get to it. Else it'll be just copying a lot of tests with a changed operator value.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why did we need to change the List plugin's tests and schema? We don't use their APIs to validate trusted apps. (maybe this schema is used by the ExceptionsListClient to validate payloads?)

Copy link
Member Author

@ashokaditya ashokaditya Apr 23, 2021

Choose a reason for hiding this comment

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

So this was needed cause the conditions that depend on the operator values use an EntriesArray type from the /lists/../../../schemas.ts. So without this change routes/trusted_apps/mapping.ts#L76 TS complains that the new operator value wildcard_caseless never overlaps with 'excluded' | 'included'.

I could trace this back to pull/83661

But yes the tests here are not entirely necessary.

const payload = 'wildcard_caseless';
const decoded = operator.decode(payload);
const message = pipe(decoded, foldLeftRight);

expect(getPaths(left(message.errors))).toEqual([]);
expect(message.schema).toEqual(payload);
});

test('it should contain same amount of keys as enum', () => {
// Might seem like a weird test, but its meant to
// ensure that if operator is updated, you
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/lists/common/schemas/common/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ export enum OperatorTypeEnum {
NESTED = 'nested',
MATCH = 'match',
MATCH_ANY = 'match_any',
WILDCARD = 'wildcard',
EXISTS = 'exists',
LIST = 'list',
}
Expand Down
19 changes: 17 additions & 2 deletions x-pack/plugins/lists/common/schemas/types/entries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,27 @@ import { entriesMatch } from './entry_match';
import { entriesExists } from './entry_exists';
import { entriesList } from './entry_list';
import { entriesNested } from './entry_nested';
import { entriesMatchWildcard } from './entry_match_wildcard';

export const entry = t.union([entriesMatch, entriesMatchAny, entriesList, entriesExists]);
export const entry = t.union([
ashokaditya marked this conversation as resolved.
Show resolved Hide resolved
entriesMatch,
entriesMatchAny,
entriesList,
entriesExists,
entriesNested,
entriesMatchWildcard,
]);
export type Entry = t.TypeOf<typeof entry>;

export const entriesArray = t.array(
t.union([entriesMatch, entriesMatchAny, entriesList, entriesExists, entriesNested])
t.union([
entriesMatch,
entriesMatchAny,
entriesList,
entriesExists,
entriesNested,
entriesMatchWildcard,
])
);
export type EntriesArray = t.TypeOf<typeof entriesArray>;

Expand Down
21 changes: 21 additions & 0 deletions x-pack/plugins/lists/common/schemas/types/entry_match_wildcard.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import * as t from 'io-ts';

import { NonEmptyString } from '../../shared_imports';
import { operator } from '../common/schemas';

export const entriesMatchWildcard = t.exact(
t.type({
field: NonEmptyString,
operator,
type: t.keyof({ wildcard: null }),
value: NonEmptyString,
})
);
export type EntryMatchWildcard = t.TypeOf<typeof entriesMatchWildcard>;
1 change: 1 addition & 0 deletions x-pack/plugins/lists/common/schemas/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export * from './default_namespace';
export * from './entries';
export * from './entry_match';
export * from './entry_match_any';
export * from './entry_match_wildcard';
export * from './entry_list';
export * from './entry_exists';
export * from './entry_nested';
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/lists/common/shared_exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export {
EntryExists,
EntryMatch,
EntryMatchAny,
EntryMatchWildcard,
EntryNested,
EntryList,
EntriesArray,
Expand All @@ -39,6 +40,7 @@ export {
nestedEntryItem,
entriesMatch,
entriesMatchAny,
entriesMatchWildcard,
entriesExists,
entriesList,
namespaceType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
EntryExists,
EntryMatch,
EntryMatchAny,
EntryMatchWildcard,
EntryNested,
ExceptionListItemSchema,
OperatorEnum,
Expand All @@ -34,7 +35,7 @@ export interface EmptyEntry {
id: string;
field: string | undefined;
operator: OperatorEnum;
type: OperatorTypeEnum.MATCH | OperatorTypeEnum.MATCH_ANY;
type: OperatorTypeEnum.MATCH | OperatorTypeEnum.MATCH_ANY | OperatorTypeEnum.WILDCARD;
value: string | string[] | undefined;
}

Expand All @@ -53,6 +54,7 @@ export interface EmptyNestedEntry {
entries: Array<
| (EntryMatch & { id?: string })
| (EntryMatchAny & { id?: string })
| (EntryMatchWildcard & { id?: string })
| (EntryExists & { id?: string })
>;
}
Expand All @@ -69,6 +71,7 @@ export type BuilderEntryNested = Omit<EntryNested, 'entries'> & {
entries: Array<
| (EntryMatch & { id?: string })
| (EntryMatchAny & { id?: string })
| (EntryMatchWildcard & { id?: string })
| (EntryExists & { id?: string })
>;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,30 @@ describe('When invoking Trusted Apps Schema', () => {
expect(() => body.validate(bodyMsg)).not.toThrow();
});

it('should validate `entry.type` does not accept `wildcard` when field is NOT PATH', () => {
const bodyMsg = createNewTrustedApp({
entries: [
createConditionEntry({
field: ConditionEntryField.HASH,
type: 'wildcard',
}),
],
});
expect(() => body.validate(bodyMsg)).toThrow();
});

it('should validate `entry.type` accepts `wildcard` when field is PATH', () => {
const bodyMsg = createNewTrustedApp({
entries: [
createConditionEntry({
field: ConditionEntryField.PATH,
type: 'wildcard',
}),
],
});
expect(() => body.validate(bodyMsg)).not.toThrow();
});

it('should validate `entry.value` required', () => {
const { value, ...entry } = createConditionEntry();
expect(() => body.validate(createNewTrustedApp({ entries: [entry] }))).toThrow();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,13 @@ export const GetTrustedAppsRequestSchema = {
}),
};

const ConditionEntryTypeSchema = schema.literal('match');
const ConditionEntryOperatorSchema = schema.literal('included');
const ConditionEntryTypeSchema = schema.conditional(
schema.siblingRef('field'),
ConditionEntryField.PATH,
schema.oneOf([schema.literal('match'), schema.literal('wildcard')]),
schema.literal('match')
);
const ConditionEntryOperatorSchema = schema.literal('included' as ConditionEntry['operator']);
ashokaditya marked this conversation as resolved.
Show resolved Hide resolved

/*
* A generic Entry schema to be used for a specific entry schema depending on the OS
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

import { TypeOf } from '@kbn/config-schema';
import { ApplicationStart } from 'kibana/public';

import { Entry } from '../../../../lists/common/schemas/types/entries';
import {
DeleteTrustedAppsRequestSchema,
GetOneTrustedAppRequestSchema,
Expand Down Expand Up @@ -69,9 +71,14 @@ export enum ConditionEntryField {
SIGNER = 'process.Ext.code_signature',
}

export enum OperatorFieldIds {
is = 'is',
matches = 'matches',
}

export interface ConditionEntry<T extends ConditionEntryField = ConditionEntryField> {
field: T;
type: 'match';
type: Entry['type'];
operator: 'included';
value: string;
}
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/security_solution/common/shared_imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export {
EntryExists,
EntryMatch,
EntryMatchAny,
EntryMatchWildcard,
EntryNested,
EntryList,
EntriesArray,
Expand All @@ -38,6 +39,7 @@ export {
nestedEntryItem,
entriesMatch,
entriesMatchAny,
entriesMatchWildcard,
entriesExists,
entriesList,
namespaceType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
Entry,
EntryMatch,
EntryMatchAny,
EntryMatchWildcard,
EntryExists,
ExceptionListItemSchema,
CreateExceptionListItemSchema,
Expand Down Expand Up @@ -92,6 +93,7 @@ export interface EmptyNestedEntry {
type: OperatorTypeEnum.NESTED;
entries: Array<
| (EntryMatch & { id?: string })
| (EntryMatchWildcard & { id?: string })
| (EntryMatchAny & { id?: string })
| (EntryExists & { id?: string })
>;
Expand All @@ -108,6 +110,7 @@ export type BuilderEntryNested = Omit<EntryNested, 'entries'> & {
id?: string;
entries: Array<
| (EntryMatch & { id?: string })
| (EntryMatchWildcard & { id?: string })
| (EntryMatchAny & { id?: string })
| (EntryExists & { id?: string })
>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ let onRemoveMock: jest.Mock;
let onChangeMock: jest.Mock;
let onVisitedMock: jest.Mock;

const entry: Readonly<ConditionEntry> = {
const baseEntry: Readonly<ConditionEntry> = {
field: ConditionEntryField.HASH,
type: 'match',
operator: 'included',
Expand All @@ -38,7 +38,8 @@ describe('Condition entry input', () => {
const getElement = (
subject: string,
os: OperatingSystem = OperatingSystem.WINDOWS,
isRemoveDisabled: boolean = false
isRemoveDisabled: boolean = false,
entry: ConditionEntry = baseEntry
) => (
<ConditionEntryInput
os={os}
Expand All @@ -64,10 +65,10 @@ describe('Condition entry input', () => {
expect(onChangeMock).toHaveBeenCalledTimes(1);
expect(onChangeMock).toHaveBeenCalledWith(
{
...entry,
...baseEntry,
field: { target: { value: field } },
},
entry
baseEntry
);
}
);
Expand All @@ -77,7 +78,7 @@ describe('Condition entry input', () => {
expect(onRemoveMock).toHaveBeenCalledTimes(0);
element.find('[data-test-subj="testOnRemove-remove"]').first().simulate('click');
expect(onRemoveMock).toHaveBeenCalledTimes(1);
expect(onRemoveMock).toHaveBeenCalledWith(entry);
expect(onRemoveMock).toHaveBeenCalledWith(baseEntry);
});

it('should not be able to call on remove for field input because disabled', () => {
Expand All @@ -92,7 +93,7 @@ describe('Condition entry input', () => {
expect(onVisitedMock).toHaveBeenCalledTimes(0);
element.find('[data-test-subj="testOnVisited-value"]').first().simulate('blur');
expect(onVisitedMock).toHaveBeenCalledTimes(1);
expect(onVisitedMock).toHaveBeenCalledWith(entry);
expect(onVisitedMock).toHaveBeenCalledWith(baseEntry);
});

it('should change value for field input', () => {
Expand All @@ -105,10 +106,10 @@ describe('Condition entry input', () => {
expect(onChangeMock).toHaveBeenCalledTimes(1);
expect(onChangeMock).toHaveBeenCalledWith(
{
...entry,
...baseEntry,
value: 'new value',
},
entry
baseEntry
);
});

Expand Down Expand Up @@ -138,4 +139,24 @@ describe('Condition entry input', () => {
.props() as EuiSuperSelectProps<string>;
expect(superSelectProps.options.length).toBe(2);
});

it('should have operator value selected when field is HASH', () => {
const element = shallow(getElement('testOperatorOptions'));
const inputField = element.find('[data-test-subj="testOperatorOptions-operator"]');
expect(inputField.contains('is'));
});

it('should show operator dorpdown with two values when field is PATH', () => {
const element = shallow(
getElement('testOperatorOptions', undefined, undefined, {
...baseEntry,
field: ConditionEntryField.PATH,
})
);
const superSelectProps = element
.find('[data-test-subj="testOperatorOptions-operator"]')
.first()
.props() as EuiSuperSelectProps<string>;
expect(superSelectProps.options.length).toBe(2);
});
});
Loading