[Security Solution] Fixes: Queries with nested field types fail open with failed to create query: [nested] failed to find nested object under path [threat.enrichments] errors for indexes where the nested fields are unmapped (#134866)

## [Security Solution] Fixes: Queries with `nested` field types fail open with `failed to create query: [nested] failed to find nested object under path [threat.enrichments]` errors for indexes where the nested fields are unmapped

This PR implements a fix for <https://github.com/elastic/kibana/issues/130340>, where queries with [nested](https://www.elastic.co/guide/en/elasticsearch/reference/current/nested.html) field types failed open with `failed to create query: [nested] failed to find nested object under path [threat.enrichments]` errors for indexes where the nested fields are unmapped.

The fix uses the new `nestedIgnoreUnmapped` option to the `buildEsQuery` API introduced in <https://github.com/elastic/kibana/pull/134580> as a fix for issue <https://github.com/elastic/kibana/issues/130348>.

Please see <https://github.com/elastic/kibana/issues/130340> for a deep dive on the issue being fixed.

### Before

 Before this fix, Timeline queries that used the `nested` query syntax in requests did NOT contain the `ignore_unmapped` option, per the example request below:

```json
                      "nested": {
                        "path": "threat.enrichments",
                        "query": {
                          "bool": {
                            "should": [
                              {
                                "match": {
                                  "threat.enrichments.matched.atomic": "a4f87cbcd2a4241da77b6bf0c5d9e8553fec991f"
                                }
                              }
                            ],
                            "minimum_should_match": 1
                          }
                        },
                        "score_mode": "none"
                      }
```

_Above: Timeline requests for fields with the `nested` query syntax did NOT contain the `ignore_unmapped` option (when inspected)_

When indexes where the nested fields were unmapped were searched:

- Elasticsearch returned a `200` status code
- The response from Elasticsearch included shard failures, per the example response below:

```json
  "_shards": {
    "total": 5,
    "successful": 3,
    "skipped": 0,
    "failed": 2,
    "failures": [
      {
        "shard": 0,
        "index": ".ds-logs-endpoint.events.process-default-2022.06.13-000001",
        "node": "3nAChOVOQKy92bhuDztcgA",
        "reason": {
          "type": "query_shard_exception",
          "reason": "failed to create query: [nested] failed to find nested object under path [threat.enrichments]",
```

_Above: Timeline responses contained shard failures (when inspected)_

### After

 After this fix, Timeline queries that use the `nested` syntax in requests contain the `"ignore_unmapped": true` option, per the example request below:

```json
                      "nested": {
                        "path": "threat.enrichments",
                        "query": {
                          "bool": {
                            "should": [
                              {
                                "match": {
                                  "threat.enrichments.matched.atomic": "a4f87cbcd2a4241da77b6bf0c5d9e8553fec991f"
                                }
                              }
                            ],
                            "minimum_should_match": 1
                          }
                        },
                        "score_mode": "none",
                        "ignore_unmapped": true
                      }
```

_Above: Timeline requests with the `nested` query syntax `"ignore_unmapped": true` option (when inspected)_

When indexes where the nested fields were unmapped are searched:

- Elasticsearch (still) returs a `200` status code
- The response from Elasticsearch does NOT include shard failures, per the example response below:

```json
  "_shards": {
    "total": 5,
    "successful": 5,
    "skipped": 0,
    "failed": 0
  },
```

### A tail of two `convertToBuildEsQuery` functions

While fixing this PR, it was noted that there are two different implementations of the `convertToBuildEsQuery` function in:

- `x-pack/plugins/security_solution/public/common/lib/keury/index.ts`
- `x-pack/plugins/timelines/public/components/utils/keury/index.ts`

The implementations of these functions are not the same. Specifically, the return type of the former implementation is:

```ts
[string, undefined] | [undefined, Error]
```

and the latter is just:

```ts
string
```

- This PR reduces the implementations of `convertToBuildEsQuery` down to a single function exported by the `timelines` plugin in `x-pack/plugins/timelines/public/components/utils/keury/index.ts`

- To minimize the scope of the changes in this PR, the previous Security Solution implementation in `x-pack/plugins/security_solution/public/common/lib/keury/index.ts` re-exports the new `timelines` implementation.

### Desk testing

See the _Reproduction steps_ section of <https://github.com/elastic/kibana/issues/130340> for details
This commit is contained in:
Andrew Goldstein 2022-06-23 09:44:06 -06:00 committed by GitHub
parent 74df8c1736
commit 2e844f0c24
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 352 additions and 176 deletions

View file

@ -1,65 +0,0 @@
/*
* 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 { escapeKuery } from '.';
describe('Kuery escape', () => {
it('should not remove white spaces quotes', () => {
const value = ' netcat';
const expected = ' netcat';
expect(escapeKuery(value)).to.be(expected);
});
it('should escape quotes', () => {
const value = 'I said, "Hello."';
const expected = 'I said, \\"Hello.\\"';
expect(escapeKuery(value)).to.be(expected);
});
it('should escape special characters', () => {
const value = `This \\ has (a lot of) <special> characters, don't you *think*? "Yes."`;
const expected = `This \\ has (a lot of) <special> characters, don't you *think*? \\"Yes.\\"`;
expect(escapeKuery(value)).to.be(expected);
});
it('should NOT escape keywords', () => {
const value = 'foo and bar or baz not qux';
const expected = 'foo and bar or baz not qux';
expect(escapeKuery(value)).to.be(expected);
});
it('should NOT escape keywords next to each other', () => {
const value = 'foo and bar or not baz';
const expected = 'foo and bar or not baz';
expect(escapeKuery(value)).to.be(expected);
});
it('should not escape keywords without surrounding spaces', () => {
const value = 'And this has keywords, or does it not?';
const expected = 'And this has keywords, or does it not?';
expect(escapeKuery(value)).to.be(expected);
});
it('should NOT escape uppercase keywords', () => {
const value = 'foo AND bar';
const expected = 'foo AND bar';
expect(escapeKuery(value)).to.be(expected);
});
it('should escape special characters and NOT keywords', () => {
const value = 'Hello, "world", and <nice> to meet you!';
const expected = 'Hello, \\"world\\", and <nice> to meet you!';
expect(escapeKuery(value)).to.be(expected);
});
it('should escape newlines and tabs', () => {
const value = 'This\nhas\tnewlines\r\nwith\ttabs';
const expected = 'This\\nhas\\tnewlines\\r\\nwith\\ttabs';
expect(escapeKuery(value)).to.be(expected);
});
});

View file

@ -5,93 +5,10 @@
* 2.0.
*/
import { isEmpty, isString, flow } from 'lodash/fp';
import {
EsQueryConfig,
Query,
Filter,
buildEsQuery,
toElasticsearchQuery,
fromKueryExpression,
DataViewBase,
} from '@kbn/es-query';
export const convertKueryToElasticSearchQuery = (
kueryExpression: string,
indexPattern?: DataViewBase
) => {
try {
return kueryExpression
? JSON.stringify(toElasticsearchQuery(fromKueryExpression(kueryExpression), indexPattern))
: '';
} catch (err) {
return '';
}
};
export const convertKueryToDslFilter = (kueryExpression: string, indexPattern: DataViewBase) => {
try {
return kueryExpression
? toElasticsearchQuery(fromKueryExpression(kueryExpression), indexPattern)
: {};
} catch (err) {
return {};
}
};
export const escapeQueryValue = (val: number | string = ''): string | number => {
if (isString(val)) {
if (isEmpty(val)) {
return '""';
}
return `"${escapeKuery(val)}"`;
}
return val;
};
const escapeWhitespace = (val: string) =>
val.replace(/\t/g, '\\t').replace(/\r/g, '\\r').replace(/\n/g, '\\n');
// See the SpecialCharacter rule in kuery.peg
const escapeSpecialCharacters = (val: string) => val.replace(/["]/g, '\\$&'); // $& means the whole matched string
// See the Keyword rule in kuery.peg
// I do not think that we need that anymore since we are doing a full match_phrase all the time now => return `"${escapeKuery(val)}"`;
// const escapeAndOr = (val: string) => val.replace(/(\s+)(and|or)(\s+)/gi, '$1\\$2$3');
// const escapeNot = (val: string) => val.replace(/not(\s+)/gi, '\\$&');
export const escapeKuery = flow(escapeSpecialCharacters, escapeWhitespace);
export const convertToBuildEsQuery = ({
config,
indexPattern,
queries,
filters,
}: {
config: EsQueryConfig;
indexPattern: DataViewBase;
queries: Query[];
filters: Filter[];
}): [string, undefined] | [undefined, Error] => {
try {
return [
JSON.stringify(
buildEsQuery(
indexPattern,
queries,
filters.filter((f) => f.meta.disabled === false),
{
...config,
dateFormatTZ: undefined,
}
)
),
undefined,
];
} catch (error) {
return [undefined, error];
}
};
export {
convertKueryToDslFilter,
convertKueryToElasticSearchQuery,
convertToBuildEsQuery,
escapeKuery,
escapeQueryValue,
} from '@kbn/timelines-plugin/public';

View file

@ -153,27 +153,61 @@ export const combineQueries = ({
kqlQuery,
kqlMode,
isEventViewer,
}: CombineQueries): { filterQuery: string } | null => {
}: CombineQueries): { filterQuery: string | undefined; kqlError: Error | undefined } | null => {
const kuery: Query = { query: '', language: kqlQuery.language };
if (isEmpty(dataProviders) && isEmpty(kqlQuery.query) && isEmpty(filters) && !isEventViewer) {
return null;
} else if (isEmpty(dataProviders) && isEmpty(kqlQuery.query) && isEventViewer) {
const [filterQuery, kqlError] = convertToBuildEsQuery({
config,
queries: [kuery],
indexPattern,
filters,
});
return {
filterQuery: convertToBuildEsQuery({ config, queries: [kuery], indexPattern, filters }),
filterQuery,
kqlError,
};
} else if (isEmpty(dataProviders) && isEmpty(kqlQuery.query) && !isEmpty(filters)) {
const [filterQuery, kqlError] = convertToBuildEsQuery({
config,
queries: [kuery],
indexPattern,
filters,
});
return {
filterQuery: convertToBuildEsQuery({ config, queries: [kuery], indexPattern, filters }),
filterQuery,
kqlError,
};
} else if (isEmpty(dataProviders) && !isEmpty(kqlQuery.query)) {
kuery.query = `(${kqlQuery.query})`;
const [filterQuery, kqlError] = convertToBuildEsQuery({
config,
queries: [kuery],
indexPattern,
filters,
});
return {
filterQuery: convertToBuildEsQuery({ config, queries: [kuery], indexPattern, filters }),
filterQuery,
kqlError,
};
} else if (!isEmpty(dataProviders) && isEmpty(kqlQuery)) {
kuery.query = `(${buildGlobalQuery(dataProviders, browserFields)})`;
const [filterQuery, kqlError] = convertToBuildEsQuery({
config,
queries: [kuery],
indexPattern,
filters,
});
return {
filterQuery: convertToBuildEsQuery({ config, queries: [kuery], indexPattern, filters }),
filterQuery,
kqlError,
};
}
const operatorKqlQuery = kqlMode === 'filter' ? 'and' : 'or';
@ -181,8 +215,17 @@ export const combineQueries = ({
kuery.query = `((${buildGlobalQuery(dataProviders, browserFields)})${postpend(
kqlQuery.query as string
)})`;
const [filterQuery, kqlError] = convertToBuildEsQuery({
config,
queries: [kuery],
indexPattern,
filters,
});
return {
filterQuery: convertToBuildEsQuery({ config, queries: [kuery], indexPattern, filters }),
filterQuery,
kqlError,
};
};

View file

@ -6,7 +6,8 @@
*/
import expect from '@kbn/expect';
import { escapeKuery } from '.';
import { convertToBuildEsQuery, escapeKuery } from '.';
import { mockIndexPattern } from '../../../mock/index_pattern';
describe('Kuery escape', () => {
it('should not remove white spaces quotes', () => {
@ -63,3 +64,272 @@ describe('Kuery escape', () => {
expect(escapeKuery(value)).to.be(expected);
});
});
describe('convertToBuildEsQuery', () => {
/**
* All the fields in this query, except for `@timestamp`,
* are nested fields https://www.elastic.co/guide/en/elasticsearch/reference/current/nested.html
*
* This mix of nested and non-nested fields will be used to verify that:
* Nested fields are converted to use the `nested` query syntax
* The `nested` query syntax includes the `ignore_unmapped` option
* Non-nested fields are NOT converted to the `nested` query syntax
* Non-nested fields do NOT include the `ignore_unmapped` option
*/
const queryWithNestedFields = [
{
query:
'((threat.enrichments: { matched.atomic: a4f87cbcd2a4241da77b6bf0c5d9e8553fec991f } and threat.enrichments: { matched.type: indicator_match_rule } and threat.enrichments: { matched.field: file.hash.md5 }) and (@timestamp : *))',
language: 'kuery',
},
];
/** A search bar filter (displayed below the KQL / Lucene search bar ) */
const filters = [
{
meta: {
alias: null,
negate: false,
disabled: false,
type: 'exists',
key: '_id',
value: 'exists',
},
query: {
exists: {
field: '_id',
},
},
},
];
const config = {
allowLeadingWildcards: true,
queryStringOptions: {
analyze_wildcard: true,
},
ignoreFilterIfFieldNotInIndex: false,
dateFormatTZ: 'Browser',
};
it('should, by default, build a query where the `nested` fields syntax includes the `"ignore_unmapped":true` option', () => {
const [converted, _] = convertToBuildEsQuery({
config,
queries: queryWithNestedFields,
indexPattern: mockIndexPattern,
filters,
});
expect(JSON.parse(converted ?? '')).to.eql({
bool: {
must: [],
filter: [
{
bool: {
filter: [
{
bool: {
filter: [
{
// ✅ Nested fields are converted to use the `nested` query syntax
nested: {
path: 'threat.enrichments',
query: {
bool: {
should: [
{
match: {
'threat.enrichments.matched.atomic':
'a4f87cbcd2a4241da77b6bf0c5d9e8553fec991f',
},
},
],
minimum_should_match: 1,
},
},
score_mode: 'none',
// ✅ The `nested` query syntax includes the `ignore_unmapped` option
ignore_unmapped: true,
},
},
{
nested: {
path: 'threat.enrichments',
query: {
bool: {
should: [
{
match: {
'threat.enrichments.matched.type': 'indicator_match_rule',
},
},
],
minimum_should_match: 1,
},
},
score_mode: 'none',
ignore_unmapped: true,
},
},
{
nested: {
path: 'threat.enrichments',
query: {
bool: {
should: [
{
match: {
'threat.enrichments.matched.field': 'file.hash.md5',
},
},
],
minimum_should_match: 1,
},
},
score_mode: 'none',
ignore_unmapped: true,
},
},
],
},
},
{
bool: {
should: [
{
exists: {
// ✅ Non-nested fields are NOT converted to the `nested` query syntax
// ✅ Non-nested fields do NOT include the `ignore_unmapped` option
field: '@timestamp',
},
},
],
minimum_should_match: 1,
},
},
],
},
},
{
exists: {
field: '_id',
},
},
],
should: [],
must_not: [],
},
});
});
it('should, when the default is overridden, build a query where `nested` fields include the `"ignore_unmapped":false` option', () => {
const configWithOverride = {
...config,
nestedIgnoreUnmapped: false, // <-- override the default
};
const [converted, _] = convertToBuildEsQuery({
config: configWithOverride,
queries: queryWithNestedFields,
indexPattern: mockIndexPattern,
filters,
});
expect(JSON.parse(converted ?? '')).to.eql({
bool: {
must: [],
filter: [
{
bool: {
filter: [
{
bool: {
filter: [
{
nested: {
path: 'threat.enrichments',
query: {
bool: {
should: [
{
match: {
'threat.enrichments.matched.atomic':
'a4f87cbcd2a4241da77b6bf0c5d9e8553fec991f',
},
},
],
minimum_should_match: 1,
},
},
score_mode: 'none',
ignore_unmapped: false, // <-- overridden by the config to be false
},
},
{
nested: {
path: 'threat.enrichments',
query: {
bool: {
should: [
{
match: {
'threat.enrichments.matched.type': 'indicator_match_rule',
},
},
],
minimum_should_match: 1,
},
},
score_mode: 'none',
ignore_unmapped: false,
},
},
{
nested: {
path: 'threat.enrichments',
query: {
bool: {
should: [
{
match: {
'threat.enrichments.matched.field': 'file.hash.md5',
},
},
],
minimum_should_match: 1,
},
},
score_mode: 'none',
ignore_unmapped: false,
},
},
],
},
},
{
bool: {
should: [
{
exists: {
field: '@timestamp',
},
},
],
minimum_should_match: 1,
},
},
],
},
},
{
exists: {
field: '_id',
},
},
],
should: [],
must_not: [],
},
});
});
});

View file

@ -74,20 +74,24 @@ export const convertToBuildEsQuery = ({
indexPattern: DataViewBase;
queries: Query[];
filters: Filter[];
}) => {
}): [string, undefined] | [undefined, Error] => {
try {
return JSON.stringify(
buildEsQuery(
indexPattern,
queries,
filters.filter((f) => f.meta.disabled === false),
{
...config,
dateFormatTZ: undefined,
}
)
);
} catch (exp) {
return '';
return [
JSON.stringify(
buildEsQuery(
indexPattern,
queries,
filters.filter((f) => f.meta.disabled === false),
{
nestedIgnoreUnmapped: true, // by default, prevent shard failures when unmapped `nested` fields are queried: https://github.com/elastic/kibana/issues/130340
...config,
dateFormatTZ: undefined,
}
)
),
undefined,
];
} catch (error) {
return [undefined, error];
}
};

View file

@ -77,6 +77,13 @@ export { getActionsColumnWidth } from './components/t_grid/body/column_headers/h
export { DEFAULT_ACTION_BUTTON_WIDTH } from './components/t_grid/body/constants';
export { useBulkActionItems } from './hooks/use_bulk_action_items';
export { getPageRowIndex } from '../common/utils/pagination';
export {
convertKueryToDslFilter,
convertKueryToElasticSearchQuery,
convertToBuildEsQuery,
escapeKuery,
escapeQueryValue,
} from './components/utils/keury';
// This exports static code and TypeScript types,
// as well as, Kibana Platform `plugin()` initializer.