Skip to content

Commit

Permalink
[Security Solution][Detection engine] skips geo_point non-ecs validat…
Browse files Browse the repository at this point in the history
…ion (#163487)

## Summary

While reviewing #163414, I have
noticed, that `geo_point` type still gets validated in
`computeIsEcsCompliant`, that could lead to removing some of the complex
`geo_point` type representations, notably object like ones, for example
```JSON
{
  "type": "Point",
  "coordinates": [-88.34, 20.12],
}
```

In this PR, I completely removing validation for this field type (we
even have functional test to verify it)
With changes introduced in
#163414, alert will be created
even with not valid `geo_point` fields, instead of failing (changed e2e
test name)


### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
  • Loading branch information
vitaliidm authored Aug 10, 2023
1 parent 8e63cd9 commit 48b7acf
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -551,4 +551,72 @@ describe('stripNonEcsFields', () => {
expect(removed).toEqual([]);
});
});

// geo_point is too complex so we going to skip its validation
describe('geo_point field', () => {
it('should not strip invalid geo_point field', () => {
const { result, removed } = stripNonEcsFields({
'client.location.geo': 'invalid geo_point',
});

expect(result).toEqual({
'client.location.geo': 'invalid geo_point',
});
expect(removed).toEqual([]);
});

it('should not strip valid geo_point fields', () => {
expect(
stripNonEcsFields({
'client.geo.location': [0, 90],
}).result
).toEqual({
'client.geo.location': [0, 90],
});

expect(
stripNonEcsFields({
'client.geo.location': {
type: 'Point',
coordinates: [-88.34, 20.12],
},
}).result
).toEqual({
'client.geo.location': {
type: 'Point',
coordinates: [-88.34, 20.12],
},
});

expect(
stripNonEcsFields({
'client.geo.location': 'POINT (-71.34 41.12)',
}).result
).toEqual({
'client.geo.location': 'POINT (-71.34 41.12)',
});

expect(
stripNonEcsFields({
client: {
geo: {
location: {
lat: 41.12,
lon: -71.34,
},
},
},
}).result
).toEqual({
client: {
geo: {
location: {
lat: 41.12,
lon: -71.34,
},
},
},
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,11 @@ const ecsObjectFields = getEcsObjectFields();

/**
* checks if path is a valid Ecs object type (object or flattened)
* geo_point also can be object
*/
const getIsEcsFieldObject = (path: string) => {
const ecsField = ecsFieldMap[path as keyof typeof ecsFieldMap];
return ['object', 'flattened'].includes(ecsField?.type) || ecsObjectFields[path];
return ['object', 'flattened', 'geo_point'].includes(ecsField?.type) || ecsObjectFields[path];
};

/**
Expand Down Expand Up @@ -117,6 +118,11 @@ const computeIsEcsCompliant = (value: SourceField, path: string) => {
const ecsField = ecsFieldMap[path as keyof typeof ecsFieldMap];
const isEcsFieldObject = getIsEcsFieldObject(path);

// do not validate geo_point, since it's very complex type that can be string/array/object
if (ecsField?.type === 'geo_point') {
return true;
}

// validate if value is a long type
if (ecsField?.type === 'long') {
return isValidLongType(value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,8 @@ export default ({ getService }: FtrProviderContext) => {

// we don't validate it because geo_point is very complex type with many various representations: array, different object, string with few valid patterns
// more on geo_point type https://www.elastic.co/guide/en/elasticsearch/reference/current/geo-point.html
// since .alerts-* indices allow _ignore_malformed option, alert will be indexed for this document
it('should fail creating alert when ECS field mapping is geo_point', async () => {
// since .alerts-* indices allow _ignore_malformed option, alert will be created for this document
it('should not fail creating alert when ECS field mapping is geo_point', async () => {
const document = {
client: {
geo: {
Expand Down

0 comments on commit 48b7acf

Please sign in to comment.