github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-24 09:48:58 -04:00
Code Issues Projects Releases Packages Wiki Activity

[Security Solution] Improve rule statuses if user has no permissions to source index (#115114)

Browse source 
* Prevent error in field_caps from silencing privilege errors

* Fix threshold bug and fix privileges in new executor

* Fix unit tests

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Marshall Main 2021-10-19 22:40:26 -04:00 • committed by GitHub
parent 5d1b5104fc
commit ba20ea1630
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 212 additions and 136 deletions
Download patch file Download diff file Expand all files Collapse all files

91
x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_wrapper.ts
View file

@ -6,13 +6,8 @@
*/
import { isEmpty } from 'lodash';
import { flow } from 'fp-ts/lib/function';
import { Either, chain, fold, tryCatch } from 'fp-ts/lib/Either';
import { TIMESTAMP } from '@kbn/rule-data-utils';
import { parseScheduleDates } from '@kbn/securitysolution-io-ts-utils';
import { ListArray } from '@kbn/securitysolution-io-ts-list-types';
import { toError } from '@kbn/securitysolution-list-api';
import { createPersistenceRuleTypeWrapper } from '../../../../../rule_registry/server';
import { buildRuleMessageFactory } from './factories/build_rule_message_factory';
@ -136,58 +131,46 @@ export const createSecurityRuleTypeWrapper: CreateSecurityRuleTypeWrapper =
const inputIndices = params.index ?? [];
const [privileges, timestampFieldCaps] = await Promise.all([
checkPrivilegesFromEsClient(esClient, inputIndices),
esClient.fieldCaps({
index: index ?? ['*'],
fields: hasTimestampOverride
? [TIMESTAMP, timestampOverride as string]
: [TIMESTAMP],
include_unmapped: true,
}),
]);
const privileges = await checkPrivilegesFromEsClient(esClient, inputIndices);
fold<Error, Promise<boolean>, void>(
async (error: Error) => logger.error(buildRuleMessage(error.message)),
async (status: Promise<boolean>) => (wroteWarningStatus = await status)
)(
flow(
() =>
tryCatch(
() =>
hasReadIndexPrivileges({
...basicLogArguments,
privileges,
logger,
buildRuleMessage,
ruleStatusClient,
}),
toError
),
chain((wroteStatus: unknown) =>
tryCatch(
() =>
hasTimestampFields({
...basicLogArguments,
wroteStatus: wroteStatus as boolean,
timestampField: hasTimestampOverride
? (timestampOverride as string)
: '@timestamp',
ruleName: name,
timestampFieldCapsResponse: timestampFieldCaps,
inputIndices,
ruleStatusClient,
logger,
buildRuleMessage,
}),
toError
)
)
)() as Either<Error, Promise<boolean>>
);
wroteWarningStatus = await hasReadIndexPrivileges({
...basicLogArguments,
privileges,
logger,
buildRuleMessage,
ruleStatusClient,
});
if (!wroteWarningStatus) {
const timestampFieldCaps = await services.scopedClusterClient.asCurrentUser.fieldCaps(
{
index,
fields: hasTimestampOverride
? ['@timestamp', timestampOverride as string]
: ['@timestamp'],
include_unmapped: true,
}
);
wroteWarningStatus = await hasTimestampFields({
...basicLogArguments,
timestampField: hasTimestampOverride ? (timestampOverride as string) : '@timestamp',
timestampFieldCapsResponse: timestampFieldCaps,
inputIndices,
ruleStatusClient,
logger,
buildRuleMessage,
});
}
}
} catch (exc) {
logger.error(buildRuleMessage(`Check privileges failed to execute ${exc}`));
const errorMessage = buildRuleMessage(`Check privileges failed to execute ${exc}`);
logger.error(errorMessage);
await ruleStatusClient.logStatusChange({
...basicLogArguments,
message: errorMessage,
newStatus: RuleExecutionStatus['partial failure'],
});
wroteWarningStatus = true;
}
let hasError = false;
const { tuples, remainingGap } = getRuleRangeTuples({

5
x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts
View file

@ -263,7 +263,8 @@ describe('signal_rule_alert_type', () => {
2,
expect.objectContaining({
newStatus: RuleExecutionStatus['partial failure'],
message: 'Missing required read privileges on the following indices: ["some*"]',
message:
'This rule may not have the required read privileges to the following indices/index patterns: ["some*"]',
})
);
});
@ -293,7 +294,7 @@ describe('signal_rule_alert_type', () => {
expect.objectContaining({
newStatus: RuleExecutionStatus['partial failure'],
message:
'This rule may not have the required read privileges to the following indices: ["myfa*","some*"]',
'This rule may not have the required read privileges to the following indices/index patterns: ["myfa*","some*"]',
})
);
});

74
x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts
View file

@ -8,12 +8,9 @@
import { Logger, SavedObject } from 'src/core/server';
import isEmpty from 'lodash/isEmpty';
import { chain, tryCatch } from 'fp-ts/lib/TaskEither';
import { flow } from 'fp-ts/lib/function';
import * as t from 'io-ts';
import { validateNonExact, parseScheduleDates } from '@kbn/securitysolution-io-ts-utils';
import { toError, toPromise } from '@kbn/securitysolution-list-api';
import {
SIGNALS_ID,
@ -191,53 +188,44 @@ export const signalRulesAlertType = ({
index,
experimentalFeatures,
});
const [privileges, timestampFieldCaps] = await Promise.all([
checkPrivileges(services, inputIndices),
services.scopedClusterClient.asCurrentUser.fieldCaps({
const privileges = await checkPrivileges(services, inputIndices);
wroteWarningStatus = await hasReadIndexPrivileges({
...basicLogArguments,
privileges,
logger,
buildRuleMessage,
ruleStatusClient,
});
if (!wroteWarningStatus) {
const timestampFieldCaps = await services.scopedClusterClient.asCurrentUser.fieldCaps({
index,
fields: hasTimestampOverride
? ['@timestamp', timestampOverride as string]
: ['@timestamp'],
include_unmapped: true,
}),
]);
wroteWarningStatus = await flow(
() =>
tryCatch(
() =>
hasReadIndexPrivileges({
...basicLogArguments,
privileges,
logger,
buildRuleMessage,
ruleStatusClient,
}),
toError
),
chain((wroteStatus) =>
tryCatch(
() =>
hasTimestampFields({
...basicLogArguments,
wroteStatus: wroteStatus as boolean,
timestampField: hasTimestampOverride
? (timestampOverride as string)
: '@timestamp',
timestampFieldCapsResponse: timestampFieldCaps,
inputIndices,
ruleStatusClient,
logger,
buildRuleMessage,
}),
toError
)
),
toPromise
)();
});
wroteWarningStatus = await hasTimestampFields({
...basicLogArguments,
timestampField: hasTimestampOverride ? (timestampOverride as string) : '@timestamp',
timestampFieldCapsResponse: timestampFieldCaps,
inputIndices,
ruleStatusClient,
logger,
buildRuleMessage,
});
}
}
} catch (exc) {
logger.error(buildRuleMessage(`Check privileges failed to execute ${exc}`));
const errorMessage = buildRuleMessage(`Check privileges failed to execute ${exc}`);
logger.error(errorMessage);
await ruleStatusClient.logStatusChange({
...basicLogArguments,
message: errorMessage,
newStatus: RuleExecutionStatus['partial failure'],
});
wroteWarningStatus = true;
}
const { tuples, remainingGap } = getRuleRangeTuples({
logger,

8
x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/bulk_create_threshold_signals.ts
View file

@ -66,8 +66,11 @@ const getTransformedHits = (
timestampOverride: TimestampOverrideOrUndefined,
signalHistory: ThresholdSignalHistory
) => {
if (results.aggregations == null) {
return [];
}
const aggParts = threshold.field.length
? results.aggregations && getThresholdAggregationParts(results.aggregations)
? getThresholdAggregationParts(results.aggregations)
: {
field: null,
index: 0,
@ -132,8 +135,7 @@ const getTransformedHits = (
};
return getCombinations(
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
(results.aggregations![aggParts.name] as { buckets: TermAggregationBucket[] }).buckets,
(results.aggregations[aggParts.name] as { buckets: TermAggregationBucket[] }).buckets,
0,
aggParts.field
).reduce((acc: Array<BaseHit<SignalSource>>, bucket) => {

4
x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.test.ts
View file

@ -781,7 +781,6 @@ describe('utils', () => {
};
mockLogger.error.mockClear();
const res = await hasTimestampFields({
wroteStatus: false,
timestampField,
ruleName: 'myfakerulename',
// eslint-disable-next-line @typescript-eslint/no-explicit-any
@ -825,7 +824,6 @@ describe('utils', () => {
};
mockLogger.error.mockClear();
const res = await hasTimestampFields({
wroteStatus: false,
timestampField,
ruleName: 'myfakerulename',
// eslint-disable-next-line @typescript-eslint/no-explicit-any
@ -855,7 +853,6 @@ describe('utils', () => {
};
mockLogger.error.mockClear();
const res = await hasTimestampFields({
wroteStatus: false,
timestampField,
ruleName: 'Endpoint Security',
// eslint-disable-next-line @typescript-eslint/no-explicit-any
@ -885,7 +882,6 @@ describe('utils', () => {
};
mockLogger.error.mockClear();
const res = await hasTimestampFields({
wroteStatus: false,
timestampField,
ruleName: 'NOT Endpoint Security',
// eslint-disable-next-line @typescript-eslint/no-explicit-any

39
x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts
View file

@ -115,34 +115,15 @@ export const hasReadIndexPrivileges = async (args: {
} = args;
const indexNames = Object.keys(privileges.index);
const [indexesWithReadPrivileges, indexesWithNoReadPrivileges] = partition(
const [, indexesWithNoReadPrivileges] = partition(
indexNames,
(indexName) => privileges.index[indexName].read
);
if (indexesWithReadPrivileges.length > 0 && indexesWithNoReadPrivileges.length > 0) {
if (indexesWithNoReadPrivileges.length > 0) {
// some indices have read privileges others do not.
// set a warning status
const errorString = `Missing required read privileges on the following indices: ${JSON.stringify(
indexesWithNoReadPrivileges
)}`;
logger.error(buildRuleMessage(errorString));
await ruleStatusClient.logStatusChange({
message: errorString,
ruleId,
ruleName,
ruleType,
spaceId,
newStatus: RuleExecutionStatus['partial failure'],
});
return true;
} else if (
indexesWithReadPrivileges.length === 0 &&
indexesWithNoReadPrivileges.length === indexNames.length
) {
// none of the indices had read privileges so set the status to failed
// since we can't search on any indices we do not have read privileges on
const errorString = `This rule may not have the required read privileges to the following indices: ${JSON.stringify(
const errorString = `This rule may not have the required read privileges to the following indices/index patterns: ${JSON.stringify(
indexesWithNoReadPrivileges
)}`;
logger.error(buildRuleMessage(errorString));
@ -160,7 +141,6 @@ export const hasReadIndexPrivileges = async (args: {
};
export const hasTimestampFields = async (args: {
wroteStatus: boolean;
timestampField: string;
ruleName: string;
// any is derived from here
@ -176,7 +156,6 @@ export const hasTimestampFields = async (args: {
buildRuleMessage: BuildRuleMessage;
}): Promise<boolean> => {
const {
wroteStatus,
timestampField,
ruleName,
timestampFieldCapsResponse,
@ -189,7 +168,7 @@ export const hasTimestampFields = async (args: {
buildRuleMessage,
} = args;
if (!wroteStatus && isEmpty(timestampFieldCapsResponse.body.indices)) {
if (isEmpty(timestampFieldCapsResponse.body.indices)) {
const errorString = `This rule is attempting to query data from Elasticsearch indices listed in the "Index pattern" section of the rule definition, however no index matching: ${JSON.stringify(
inputIndices
)} was found. This warning will continue to appear until a matching index is created or this rule is de-activated. ${
@ -208,10 +187,9 @@ export const hasTimestampFields = async (args: {
});
return true;
} else if (
!wroteStatus &&
(isEmpty(timestampFieldCapsResponse.body.fields) ||
timestampFieldCapsResponse.body.fields[timestampField] == null ||
timestampFieldCapsResponse.body.fields[timestampField]?.unmapped?.indices != null)
isEmpty(timestampFieldCapsResponse.body.fields) ||
timestampFieldCapsResponse.body.fields[timestampField] == null ||
timestampFieldCapsResponse.body.fields[timestampField]?.unmapped?.indices != null
) {
// if there is a timestamp override and the unmapped array for the timestamp override key is not empty,
// warning
@ -236,7 +214,7 @@ export const hasTimestampFields = async (args: {
});
return true;
}
return wroteStatus;
return false;
};
export const checkPrivileges = async (
@ -257,6 +235,7 @@ export const checkPrivilegesFromEsClient = async (
index: [
{
names: indices ?? [],
allow_restricted_indices: true,
privileges: ['read'],
},
],

107
x-pack/test/detection_engine_api_integration/security_and_spaces/tests/check_privileges.ts Normal file
View file

@ -0,0 +1,107 @@
/*
* 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 expect from '@kbn/expect';
import { DETECTION_ENGINE_RULES_URL } from '../../../../plugins/security_solution/common/constants';
import { FtrProviderContext } from '../../common/ftr_provider_context';
import {
createSignalsIndex,
deleteSignalsIndex,
deleteAllAlerts,
waitForRuleSuccessOrStatus,
getRuleForSignalTesting,
createRuleWithAuth,
getThresholdRuleForSignalTesting,
} from '../../utils';
import { createUserAndRole, deleteUserAndRole } from '../../../common/services/security_solution';
import { ROLES } from '../../../../plugins/security_solution/common/test';
import { ThresholdCreateSchema } from '../../../../plugins/security_solution/common/detection_engine/schemas/request';
// eslint-disable-next-line import/no-default-export
export default ({ getService }: FtrProviderContext) => {
const supertest = getService('supertest');
const esArchiver = getService('esArchiver');
const supertestWithoutAuth = getService('supertestWithoutAuth');
describe('check_privileges', () => {
before(async () => {
await esArchiver.load('x-pack/test/functional/es_archives/auditbeat/hosts');
await esArchiver.load('x-pack/test/functional/es_archives/security_solution/alias');
await createSignalsIndex(supertest);
});
after(async () => {
await esArchiver.unload('x-pack/test/functional/es_archives/auditbeat/hosts');
await esArchiver.unload('x-pack/test/functional/es_archives/security_solution/alias');
await deleteSignalsIndex(supertest);
});
beforeEach(async () => {
await deleteAllAlerts(supertest);
});
afterEach(async () => {
await deleteAllAlerts(supertest);
});
describe('should set status to partial failure when user has no access', () => {
const indexTestCases = [
['host_alias'],
['host_alias', 'auditbeat-8.0.0'],
['host_alias*'],
['host_alias*', 'auditbeat-*'],
];
indexTestCases.forEach((index) => {
it(`for KQL rule with index param: ${index}`, async () => {
const rule = getRuleForSignalTesting(index);
await createUserAndRole(getService, ROLES.detections_admin);
const { id } = await createRuleWithAuth(supertestWithoutAuth, rule, {
user: ROLES.detections_admin,
pass: 'changeme',
});
await waitForRuleSuccessOrStatus(supertest, id, 'partial failure');
const { body } = await supertest
.post(`${DETECTION_ENGINE_RULES_URL}/_find_statuses`)
.set('kbn-xsrf', 'true')
.send({ ids: [id] })
.expect(200);
expect(body[id]?.current_status?.last_success_message).to.eql(
`This rule may not have the required read privileges to the following indices/index patterns: ["${index[0]}"]`
);
await deleteUserAndRole(getService, ROLES.detections_admin);
});
it(`for threshold rule with index param: ${index}`, async () => {
const rule: ThresholdCreateSchema = {
...getThresholdRuleForSignalTesting(index),
threshold: {
field: [],
value: 700,
},
};
await createUserAndRole(getService, ROLES.detections_admin);
const { id } = await createRuleWithAuth(supertestWithoutAuth, rule, {
user: ROLES.detections_admin,
pass: 'changeme',
});
await waitForRuleSuccessOrStatus(supertest, id, 'partial failure');
const { body } = await supertest
.post(`${DETECTION_ENGINE_RULES_URL}/_find_statuses`)
.set('kbn-xsrf', 'true')
.send({ ids: [id] })
.expect(200);
expect(body[id]?.current_status?.last_success_message).to.eql(
`This rule may not have the required read privileges to the following indices/index patterns: ["${index[0]}"]`
);
await deleteUserAndRole(getService, ROLES.detections_admin);
});
});
});
});
};

1
x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts
View file

@ -18,6 +18,7 @@ export default ({ loadTestFile }: FtrProviderContext): void => {
loadTestFile(require.resolve('./add_actions'));
loadTestFile(require.resolve('./update_actions'));
loadTestFile(require.resolve('./add_prepackaged_rules'));
loadTestFile(require.resolve('./check_privileges'));
loadTestFile(require.resolve('./create_rules'));
loadTestFile(require.resolve('./create_rules_bulk'));
loadTestFile(require.resolve('./create_index'));

19
x-pack/test/detection_engine_api_integration/utils.ts
View file

@ -916,6 +916,25 @@ export const createRule = async (
return body;
};
/**
* Helper to cut down on the noise in some of the tests. This checks for
* an expected 200 still and does not try to any retries.
* @param supertest The supertest deps
* @param rule The rule to create
*/
export const createRuleWithAuth = async (
supertest: SuperTest.SuperTest<SuperTest.Test>,
rule: CreateRulesSchema,
auth: { user: string; pass: string }
): Promise<FullResponseSchema> => {
const { body } = await supertest
.post(DETECTION_ENGINE_RULES_URL)
.set('kbn-xsrf', 'true')
.auth(auth.user, auth.pass)
.send(rule);
return body;
};
/**
* Helper to cut down on the noise in some of the tests. This checks for
* an expected 200 still and does not do any retries.