Skip to content

Commit

Permalink
[SIEM] Fixes critical and blocker bugs with querying Anomalies from ML (
Browse files Browse the repository at this point in the history
#40885) (#40944)

## Summary

* Fixes a critical and blocking bug where we were querying against _ALL_ ML Jobs. Now we _only_ query for jobs that have the SIEM Group tag.

* Fixes a critical and blocking bug where `ui/chrome` getBasePath() was not being added in the fetch API areas

* Fixes a critical and blocking bug where we were querying `influencers` rather than `criteriaFields` when showing scores within the details pages for the "Max Anomaly Jobs". This caused missing jobs from the details and incorrect results.

* Fixes a critical and blocking bug where we were not using `isInitialized` from the URL loading in which case we could be loading a slightly different time range or give incorrect results.

* Fixes a critical and blocking bug where we were not filtering on `source` vs `destination` on the IP details page. Instead we were querying for everything and then filtering to either of the two as in a "source OR destination". Now instead, we only show what the user selects. 

* Fixes an embarrassing and potentially critical bug where React Router was warning about the usage of React.Memo. Instead I swapped back to using recompose pure so that we do not see warnings about the `ml-hosts` and `ml-network` routes.

### Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

- [ ] This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)

~- [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)~

~- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials~

- [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios

~- [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)~

### For maintainers

- [x] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)
- [x] This includes a feature addition or change that requires a release note and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)
  • Loading branch information
FrankHassanabad committed Jul 12, 2019
1 parent d5fb645 commit 63a82bb
Show file tree
Hide file tree
Showing 26 changed files with 524 additions and 86 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import * as React from 'react';
import { InfluencerInput, Anomalies } from '../types';
import { InfluencerInput, Anomalies, CriteriaFields } from '../types';
import { useAnomaliesTableData } from './use_anomalies_table_data';

interface ChildrenArgs {
Expand All @@ -14,18 +14,22 @@ interface ChildrenArgs {
}

interface Props {
influencers: InfluencerInput[] | null;
influencers?: InfluencerInput[];
startDate: number;
endDate: number;
criteriaFields?: CriteriaFields[];
children: (args: ChildrenArgs) => React.ReactNode;
skip: boolean;
}

export const AnomalyTableProvider = React.memo<Props>(
({ influencers, startDate, endDate, children }) => {
({ influencers, startDate, endDate, children, criteriaFields, skip }) => {
const [isLoadingAnomaliesData, anomaliesData] = useAnomaliesTableData({
criteriaFields,
influencers,
startDate,
endDate,
skip,
});
return <>{children({ isLoadingAnomaliesData, anomaliesData })}</>;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { InfluencerInput } from '../types';
import { influencersToString, getThreshold } from './use_anomalies_table_data';
import { influencersOrCriteriaToString, getThreshold } from './use_anomalies_table_data';
import { AppKibanaFrameworkAdapter } from '../../../lib/adapters/framework/kibana_framework_adapter';

describe('use_anomalies_table_data', () => {
Expand All @@ -16,7 +16,7 @@ describe('use_anomalies_table_data', () => {
fieldValue: 'value-1',
},
];
const influencerString = influencersToString(influencers);
const influencerString = influencersOrCriteriaToString(influencers);
expect(influencerString).toEqual('field-1:value-1');
});

Expand All @@ -31,18 +31,13 @@ describe('use_anomalies_table_data', () => {
fieldValue: 'value-2',
},
];
const influencerString = influencersToString(influencers);
const influencerString = influencersOrCriteriaToString(influencers);
expect(influencerString).toEqual('field-1:value-1field-2:value-2');
});

test('should return an empty string when the array is empty', () => {
const influencers: InfluencerInput[] = [];
const influencerString = influencersToString(influencers);
expect(influencerString).toEqual('');
});

test('should return an empty string when passed null', () => {
const influencerString = influencersToString(null);
const influencerString = influencersOrCriteriaToString(influencers);
expect(influencerString).toEqual('');
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,29 @@
import { useState, useEffect, useContext } from 'react';
import moment from 'moment-timezone';
import { anomaliesTableData } from '../api/anomalies_table_data';
import { InfluencerInput, Anomalies } from '../types';
import { InfluencerInput, Anomalies, CriteriaFields } from '../types';
import {
KibanaConfigContext,
AppKibanaFrameworkAdapter,
} from '../../../lib/adapters/framework/kibana_framework_adapter';
import { hasMlUserPermissions } from '../permissions/has_ml_user_permissions';
import { MlCapabilitiesContext } from '../permissions/ml_capabilities_provider';
import { useSiemJobs } from '../../ml_popover/hooks/use_siem_jobs';

interface Args {
influencers: InfluencerInput[] | null;
influencers?: InfluencerInput[];
endDate: number;
startDate: number;
threshold?: number;
skip?: boolean;
criteriaFields?: CriteriaFields[];
}

type Return = [boolean, Anomalies | null];

export const influencersToString = (influencers: InfluencerInput[] | null): string =>
export const influencersOrCriteriaToString = (
influencers: InfluencerInput[] | CriteriaFields[]
): string =>
influencers == null
? ''
: influencers.reduce((accum, item) => `${accum}${item.fieldName}:${item.fieldValue}`, '');
Expand Down Expand Up @@ -58,28 +62,31 @@ export const getThreshold = (
};

export const useAnomaliesTableData = ({
influencers,
criteriaFields = [],
influencers = [],
startDate,
endDate,
threshold = -1,
skip = false,
}: Args): Return => {
const [tableData, setTableData] = useState<Anomalies | null>(null);
const [, siemJobs] = useSiemJobs(true);
const [loading, setLoading] = useState(true);
const config = useContext(KibanaConfigContext);
const capabilities = useContext(MlCapabilitiesContext);
const userPermissions = hasMlUserPermissions(capabilities);

const fetchFunc = async (
influencersInput: InfluencerInput[] | null,
influencersInput: InfluencerInput[],
criteriaFieldsInput: CriteriaFields[],
earliestMs: number,
latestMs: number
) => {
if (userPermissions && influencersInput != null && !skip) {
if (userPermissions && !skip && siemJobs.length > 0) {
const data = await anomaliesTableData(
{
jobIds: [],
criteriaFields: [],
jobIds: siemJobs,
criteriaFields: criteriaFieldsInput,
aggregationInterval: 'auto',
threshold: getThreshold(config, threshold),
earliestMs,
Expand All @@ -105,8 +112,16 @@ export const useAnomaliesTableData = ({

useEffect(() => {
setLoading(true);
fetchFunc(influencers, startDate, endDate);
}, [influencersToString(influencers), startDate, endDate, skip, userPermissions]);
fetchFunc(influencers, criteriaFields, startDate, endDate);
}, [
influencersOrCriteriaToString(influencers),
influencersOrCriteriaToString(criteriaFields),
startDate,
endDate,
skip,
userPermissions,
siemJobs.join(),
]);

return [loading, tableData];
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
*/

import chrome from 'ui/chrome';
import { Anomalies, InfluencerInput } from '../types';
import { Anomalies, InfluencerInput, CriteriaFields } from '../types';
import { throwIfNotOk } from './throw_if_not_ok';

export interface Body {
jobIds: string[];
criteriaFields: string[];
criteriaFields: CriteriaFields[];
influencers: InfluencerInput[];
aggregationInterval: string;
threshold: number;
Expand All @@ -31,7 +31,7 @@ export const anomaliesTableData = async (
headers: Record<string, string | undefined>
): Promise<Anomalies> => {
try {
const response = await fetch('/api/ml/results/anomalies_table_data', {
const response = await fetch(`${chrome.getBasePath()}/api/ml/results/anomalies_table_data`, {
method: 'POST',
credentials: 'same-origin',
body: JSON.stringify(body),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ export const getMlCapabilities = async (
headers: Record<string, string | undefined>
): Promise<MlCapabilities> => {
try {
const response = await fetch('/api/ml/ml_capabilities', {
const response = await fetch(`${chrome.getBasePath()}/api/ml/ml_capabilities`, {
method: 'GET',
credentials: 'same-origin',
headers: {
'kbn-system-api': 'true',
'content-Type': 'application/json',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import React from 'react';

import { match as RouteMatch, Redirect, Route, Switch } from 'react-router-dom';
import { QueryString } from 'ui/utils/query_string';
import { pure } from 'recompose';
import { addEntitiesToKql } from './add_entities_to_kql';
import { replaceKQLParts } from './replace_kql_parts';
import { emptyEntity, multipleEntities, getMultipleEntities } from './entity_helpers';
Expand All @@ -24,7 +25,7 @@ interface QueryStringType {
timerange: string | null;
}

export const MlHostConditionalContainer = React.memo<MlHostConditionalProps>(({ match }) => (
export const MlHostConditionalContainer = pure<MlHostConditionalProps>(({ match }) => (
<Switch>
<Route
strict
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import React from 'react';

import { match as RouteMatch, Redirect, Route, Switch } from 'react-router-dom';
import { QueryString } from 'ui/utils/query_string';
import { pure } from 'recompose';
import { addEntitiesToKql } from './add_entities_to_kql';
import { replaceKQLParts } from './replace_kql_parts';
import { emptyEntity, getMultipleEntities, multipleEntities } from './entity_helpers';
Expand All @@ -24,7 +25,7 @@ interface QueryStringType {
timerange: string | null;
}

export const MlNetworkConditionalContainer = React.memo<MlNetworkConditionalProps>(({ match }) => (
export const MlNetworkConditionalContainer = pure<MlNetworkConditionalProps>(({ match }) => (
<Switch>
<Route
strict
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ export const operators = ['and', 'or', 'not'];
export const removeKqlVariablesUsingRegex = (expression: string) => {
const myRegexp = /(\s+)*(and|or|not){0,1}(\s+)*([\w\.\-\[\]]+)\s*:\s*"(\$[\w\.\-\(\)\[\]]+\$)"(\s+)*(and|or|not){0,1}(\s+)*/g;
return expression.replace(myRegexp, replacement);
// return expression.replace(myRegexp, '');
};

export const replacement = (match: string, ...parts: string[]): string => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { getCriteriaFromHostType } from './get_criteria_from_host_type';
import { HostsType } from '../../../store/hosts/model';

describe('get_criteria_from_host_type', () => {
test('returns host names from criteria if the host type is details', () => {
const criteria = getCriteriaFromHostType(HostsType.details, 'zeek-iowa');
expect(criteria).toEqual([{ fieldName: 'host.name', fieldValue: 'zeek-iowa' }]);
});

test('returns empty array from criteria if the host type is page but rather an empty array', () => {
const criteria = getCriteriaFromHostType(HostsType.page, 'zeek-iowa');
expect(criteria).toEqual([]);
});

test('returns empty array from criteria if the host name is undefined and host type is details', () => {
const criteria = getCriteriaFromHostType(HostsType.details, undefined);
expect(criteria).toEqual([]);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { HostsType } from '../../../store/hosts/model';
import { CriteriaFields } from '../types';

export const getCriteriaFromHostType = (
type: HostsType,
hostName: string | undefined
): CriteriaFields[] => {
if (type === HostsType.details && hostName != null) {
return [{ fieldName: 'host.name', fieldValue: hostName }];
} else {
return [];
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { getCriteriaFromNetworkType } from './get_criteria_from_network_type';
import { NetworkType } from '../../../store/network/model';
import { FlowTarget } from '../../../graphql/types';

describe('get_criteria_from_network_type', () => {
test('returns network names from criteria if the network type is details and it is source', () => {
const criteria = getCriteriaFromNetworkType(
NetworkType.details,
'127.0.0.1',
FlowTarget.source
);
expect(criteria).toEqual([{ fieldName: 'source.ip', fieldValue: '127.0.0.1' }]);
});

test('returns network names from criteria if the network type is details and it is destination', () => {
const criteria = getCriteriaFromNetworkType(
NetworkType.details,
'127.0.0.1',
FlowTarget.destination
);
expect(criteria).toEqual([{ fieldName: 'destination.ip', fieldValue: '127.0.0.1' }]);
});

test('returns empty array if the network type is page', () => {
const criteria = getCriteriaFromNetworkType(
NetworkType.page,
'127.0.0.1',
FlowTarget.destination
);
expect(criteria).toEqual([]);
});

test('returns empty array if flowTarget is missing', () => {
const criteria = getCriteriaFromNetworkType(NetworkType.page, '127.0.0.1');
expect(criteria).toEqual([]);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { CriteriaFields } from '../types';
import { NetworkType } from '../../../store/network/model';
import { FlowTarget } from '../../../graphql/types';

export const getCriteriaFromNetworkType = (
type: NetworkType,
ip: string | undefined,
flowTarget?: FlowTarget
): CriteriaFields[] => {
if (type === NetworkType.details && ip != null) {
if (flowTarget === FlowTarget.source) {
return [{ fieldName: 'source.ip', fieldValue: ip }];
} else if (flowTarget === FlowTarget.destination) {
return [{ fieldName: 'destination.ip', fieldValue: ip }];
} else {
return [];
}
} else {
return [];
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { HostItem } from '../../../graphql/types';
import { CriteriaFields } from '../types';
import { hostToCriteria } from './host_to_criteria';

describe('host_to_criteria', () => {
test('converts a host to a criteria', () => {
const hostItem: HostItem = {
host: {
name: ['host-name'],
},
};
const expectedCriteria: CriteriaFields[] = [
{
fieldName: 'host.name',
fieldValue: 'host-name',
},
];
expect(hostToCriteria(hostItem)).toEqual(expectedCriteria);
});

test('returns an empty array if the host.name is null', () => {
const hostItem: HostItem = {
host: {
name: null,
},
};
expect(hostToCriteria(hostItem)).toEqual([]);
});

test('returns an empty array if the host is null', () => {
const hostItem: HostItem = {
host: null,
};
expect(hostToCriteria(hostItem)).toEqual([]);
});
});
Loading

0 comments on commit 63a82bb

Please sign in to comment.