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

Add criticality fields and risk score fields to alert schema #174626

Merged
merged 17 commits into from Jan 31, 2024

Conversation

nkhristinin
Copy link
Contributor

@nkhristinin nkhristinin commented Jan 10, 2024

Update alerts fields names for asset criticality, and add risk score field

We want to update
kibana.alert.user.criticality_level to host.asset.criticality
kibana.alert.host.criticality_level to host.asset.criticality

kibana.alert.user.criticality_level and kibana.alert.host.criticality_level will be still present in the schema/mappings, for backward compatibility as it was released to serverless/

Also, we added host.risk.calculated_score_norm, host.risk.calculated_level, user.risk.calculated_score_norm, user.risk.calculated_level.
Those fields enriched alerts from8.5.0, but weren't added to the alert schema

@SourinPaul approved usage of new fields

@nkhristinin
Copy link
Contributor Author

/ci

@nkhristinin
Copy link
Contributor Author

/ci

@SourinPaul
Copy link

@nkhristinin Since asset criticality has not yet been released (as of 8.12), I recommend we remove the older Kibana fieldmapsets so it does not clutter the alert fields.

With the above internal fields cleaned up, I approve merging this PR so we use the proposed ECS fields instead.

cc: @jaredburgettelastic @paulewing

@nkhristinin
Copy link
Contributor Author

@elasticmachine merge upstream

@nkhristinin nkhristinin marked this pull request as ready for review January 25, 2024 12:15
@nkhristinin nkhristinin requested review from a team as code owners January 25, 2024 12:15
@nkhristinin
Copy link
Contributor Author

With the above internal fields cleaned up, I approve merging this PR so we use the proposed ECS fields instead.

As there change that it was released in serverless, those fields can be in alert_mappings, but not in the alert document.

I will not remove them from mappings for backwards compatibility

@nkhristinin nkhristinin added the release_note:feature Makes this part of the condensed release notes label Jan 25, 2024
@nkhristinin
Copy link
Contributor Author

@elasticmachine merge upstream

@nkhristinin nkhristinin requested a review from a team as a code owner January 25, 2024 13:46

export const alertsFieldMap8130 = {
...alertsFieldMap840,
/**
* @deprecated Use ALERT_HOST_CRITICALITY instead.
Copy link
Contributor

Choose a reason for hiding this comment

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

I wasn't able to see these JSDoc being used anywhere, either on the broader alertsFieldMap8130 type, nor on pulling the value out. Since the key itself is deprecated, perhaps that's good enough?

export const ALERT_HOST_CRITICALITY = `${ALERT_NAMESPACE}.host.criticality_level` as const;
export const ALERT_USER_CRITICALITY = `${ALERT_NAMESPACE}.user.criticality_level` as const;
/**
* @deprecated Use ALERT_HOST_CRITICALITY
Copy link
Contributor

Choose a reason for hiding this comment

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

This format will actually link to the other type:

Suggested change
* @deprecated Use ALERT_HOST_CRITICALITY
* @deprecated Use {@link ALERT_HOST_CRITICALITY}


export const ALERT_HOST_CRITICALITY = `host.asset.criticality` as const;
export const ALERT_USER_CRITICALITY = `user.asset.criticality` as const;
export const ALERT_HOST_RISK_SCORE_CALCULATED_LEVEL = `host.risk.calculated_level` as const;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do these need these ALERT_ prefixes if the fields aren't specific to alerts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, for those fields I want to show that they are related to alerts.

We do have host.risk.calculated_level in other places in our app, but host.asset.criticality unique only for alerts, so I wanted to specify that

Copy link
Contributor

@kqualters-elastic kqualters-elastic left a comment

Choose a reason for hiding this comment

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

lgtm, but traditionally it's been @elastic/response-ops that want to keep the alerts as data schemas as light as possible

@nkhristinin
Copy link
Contributor Author

@elasticmachine merge upstream

@@ -122,6 +122,9 @@ const SecurityAlertOptional = rt.partial({
'ecs.version': schemaString,
'event.action': schemaString,
'event.kind': schemaString,
'host.asset.criticality': schemaString,
'host.risk.calculated_level': schemaString,
Copy link
Contributor

Choose a reason for hiding this comment

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

host.risk.calculated_level and host.risk.calculated_score_norm are already in the ECS component template, which is referenced by by the security alerts index. Do we need to redefine them in the security alerts component template?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch!

I will check if I can have in alert types, but remove from field_maps, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested locally, you rights, the field already in alert mappings, we probably don't need them here. Thanks!

@@ -204,6 +207,9 @@ const SecurityAlertOptional = rt.partial({
'kibana.alert.workflow_user': schemaString,
'kibana.version': schemaString,
tags: schemaStringArray,
'user.asset.criticality': schemaString,
'user.risk.calculated_level': schemaString,
Copy link
Contributor

@ymao1 ymao1 Jan 30, 2024

Choose a reason for hiding this comment

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

same question about user.risk.calculated_score_norm and user.risk.calculated_level. If they are already in the ECS component template, do we need to redefine them in the security alerts component template?

@nkhristinin
Copy link
Contributor Author

@elasticmachine merge upstream

@nkhristinin
Copy link
Contributor Author

@elasticmachine merge upstream

@nkhristinin
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@ymao1 ymao1 left a comment

Choose a reason for hiding this comment

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

Response ops changes LGTM

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 11.4MB 11.4MB +76.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@rylnd rylnd left a comment

Choose a reason for hiding this comment

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

I was briefly hung up on this exchange, but it sounds like you and Sourin have come to an agreement there 👍 .

@ymao1 thanks for catching those redundancies!

EA/SecSol changes LGTM.

@nkhristinin nkhristinin merged commit f3a7fd6 into elastic:main Jan 31, 2024
39 checks passed
@kibanamachine kibanamachine added v8.13.0 backport:skip This commit does not require backporting labels Jan 31, 2024
WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Feb 5, 2024
…#174626)

## Update alerts fields names for asset criticality, and add risk score
field

We want to update 
`kibana.alert.user.criticality_level` to `host.asset.criticality`
`kibana.alert.host.criticality_level` to `host.asset.criticality`

`kibana.alert.user.criticality_level` and
`kibana.alert.host.criticality_level` will be still present in the
schema/mappings, for backward compatibility as it was released to
serverless/

Also, we added `host.risk.calculated_score_norm`,
`host.risk.calculated_level`, `user.risk.calculated_score_norm`,
`user.risk.calculated_level`.
Those fields enriched alerts
from[8.5.0](elastic#139478), but weren't
added to the alert schema

@SourinPaul
[approved](elastic#174626 (comment))
usage of new fields

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Feb 6, 2024
…#174626)

## Update alerts fields names for asset criticality, and add risk score
field

We want to update 
`kibana.alert.user.criticality_level` to `host.asset.criticality`
`kibana.alert.host.criticality_level` to `host.asset.criticality`

`kibana.alert.user.criticality_level` and
`kibana.alert.host.criticality_level` will be still present in the
schema/mappings, for backward compatibility as it was released to
serverless/

Also, we added `host.risk.calculated_score_norm`,
`host.risk.calculated_level`, `user.risk.calculated_score_norm`,
`user.risk.calculated_level`.
Those fields enriched alerts
from[8.5.0](elastic#139478), but weren't
added to the alert schema

@SourinPaul
[approved](elastic#174626 (comment))
usage of new fields

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
…#174626)

## Update alerts fields names for asset criticality, and add risk score
field

We want to update 
`kibana.alert.user.criticality_level` to `host.asset.criticality`
`kibana.alert.host.criticality_level` to `host.asset.criticality`

`kibana.alert.user.criticality_level` and
`kibana.alert.host.criticality_level` will be still present in the
schema/mappings, for backward compatibility as it was released to
serverless/

Also, we added `host.risk.calculated_score_norm`,
`host.risk.calculated_level`, `user.risk.calculated_score_norm`,
`user.risk.calculated_level`.
Those fields enriched alerts
from[8.5.0](elastic#139478), but weren't
added to the alert schema

@SourinPaul
[approved](elastic#174626 (comment))
usage of new fields

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
…#174626)

## Update alerts fields names for asset criticality, and add risk score
field

We want to update 
`kibana.alert.user.criticality_level` to `host.asset.criticality`
`kibana.alert.host.criticality_level` to `host.asset.criticality`

`kibana.alert.user.criticality_level` and
`kibana.alert.host.criticality_level` will be still present in the
schema/mappings, for backward compatibility as it was released to
serverless/

Also, we added `host.risk.calculated_score_norm`,
`host.risk.calculated_level`, `user.risk.calculated_score_norm`,
`user.risk.calculated_level`.
Those fields enriched alerts
from[8.5.0](elastic#139478), but weren't
added to the alert schema

@SourinPaul
[approved](elastic#174626 (comment))
usage of new fields

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
…#174626)

## Update alerts fields names for asset criticality, and add risk score
field

We want to update 
`kibana.alert.user.criticality_level` to `host.asset.criticality`
`kibana.alert.host.criticality_level` to `host.asset.criticality`

`kibana.alert.user.criticality_level` and
`kibana.alert.host.criticality_level` will be still present in the
schema/mappings, for backward compatibility as it was released to
serverless/

Also, we added `host.risk.calculated_score_norm`,
`host.risk.calculated_level`, `user.risk.calculated_score_norm`,
`user.risk.calculated_level`.
Those fields enriched alerts
from[8.5.0](elastic#139478), but weren't
added to the alert schema

@SourinPaul
[approved](elastic#174626 (comment))
usage of new fields

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:feature Makes this part of the condensed release notes v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants