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

Migrate Host risk and User risk UI to ECS schema #140080

Merged
merged 10 commits into from
Sep 9, 2022

Conversation

machadoum
Copy link
Member

@machadoum machadoum commented Sep 6, 2022

Links:
issue
PR for transforms and dashboards
proposed ECS risk fields.

Summary

Migrate Host risk and User risk indices to the ECS schema

Affected components:

  • User risk score tab
  • User detail overview
  • User detail risk score tab
  • Host detail overview
  • Host risk score tab
  • Host detail risk score tab
  • Host flyout
  • User flyout
  • Entity analytics Host table
  • Entity analytics User table
  • Host overview
  • All hosts tab (risk column)

Add mocked data

node ./scripts/es_archiver load ./x-pack/test/security_solution_cypress/es_archives/risky_users --config ./x-pack/test/security_solution_cypress/config.ts --es-url https://admin:<PASSWORD>@<ENVIRONMENT> --kibana-url http://admin:<PASSWORD>@localhost:5601/

node ./scripts/es_archiver load ./x-pack/test/security_solution_cypress/es_archives/risky_hosts --config ./x-pack/test/security_solution_cypress/config.ts --es-url https://admin:<PASSWORD>@<ENVIRONMENT> --kibana-url http://admin:<PASSWORD>@localhost:5601/

TODO

@machadoum machadoum self-assigned this Sep 6, 2022
@machadoum machadoum added release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting Security Solution Threat Hunting Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Explore v8.5.0 labels Sep 6, 2022
@machadoum machadoum force-pushed the siem-explore-issue-4858 branch from 7eb5992 to a85509b Compare September 6, 2022 13:34
@machadoum machadoum marked this pull request as ready for review September 6, 2022 14:01
@machadoum machadoum requested review from a team as code owners September 6, 2022 14:01
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@machadoum machadoum changed the title Migrate Host risk and User risk indices to ECS schema Migrate Host risk and User risk UI to ECS schema Sep 6, 2022
@machadoum machadoum force-pushed the siem-explore-issue-4858 branch from fad3f1d to f8bce6c Compare September 7, 2022 08:48
@machadoum machadoum force-pushed the siem-explore-issue-4858 branch from 2c54e53 to 47a0b7d Compare September 8, 2022 08:24
const getListItemsFromHits = (items: HostsRiskScore[]): LinkPanelListItem[] => {
return items.map(({ host, risk_stats: riskStats, risk: copy }) => ({
const getListItemsFromHits = (items: HostRiskScore[]): LinkPanelListItem[] => {
return items.map(({ host }) => ({
title: host.name,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
title: host.name,
title: get(item, RiskScoreFields.hostName),

Copy link
Member Author

Choose a reason for hiding this comment

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

get isn't type-safe.

title: host.name,
count: riskStats.risk_score,
copy,
count: host.risk.calculated_score_norm,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
count: host.risk.calculated_score_norm,
count: get(item, RiskScoreFields.hostRiskScore),

count: riskStats.risk_score,
copy,
count: host.risk.calculated_score_norm,
copy: host.risk.calculated_level,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
copy: host.risk.calculated_level,
copy: get(item, RiskScoreFields.hostRisk),

@@ -106,7 +106,9 @@ export const UserOverview = React.memo<UserSummaryProps>(
title: i18n.USER_RISK_SCORE,
description: (
<>
{userRiskData ? Math.round(userRiskData.risk_stats.risk_score) : getEmptyTagValue()}
{userRiskData
? Math.round(userRiskData.user.risk.calculated_score_norm)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
? Math.round(userRiskData.user.risk.calculated_score_norm)
? Math.round(get(userRiskData, RiskScoreFields.userRiskScore))

@@ -115,7 +117,10 @@ export const UserOverview = React.memo<UserSummaryProps>(
description: (
<>
{userRiskData ? (
<RiskScore severity={userRiskData.risk as RiskSeverity} hideBackgroundColor />
<RiskScore
severity={userRiskData.user.risk.calculated_level as RiskSeverity}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
severity={userRiskData.user.risk.calculated_level as RiskSeverity}
severity={get(userRiskData, RiskScoreFields.userRisk)}

@@ -92,7 +92,7 @@ async function enhanceEdges(
const hostsRiskByHostName: Record<string, string> | undefined = hostRiskData?.hits.hits.reduce(
(acc, hit) => ({
...acc,
[hit._source?.host.name ?? '']: hit._source?.risk,
[hit._source?.host.name ?? '']: hit._source?.host.risk.calculated_level,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
[hit._source?.host.name ?? '']: hit._source?.host.risk.calculated_level,
[hit._source?.host.name ?? '']: get(hit, `_source.${RiskScoreFields.hostRisk}`),

@machadoum machadoum force-pushed the siem-explore-issue-4858 branch from 9b774a8 to 48842e7 Compare September 8, 2022 13:12
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

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 6.4MB 6.4MB +355.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
securitySolution 259.9KB 260.1KB +222.0B

History

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

cc @machadoum

Copy link
Member

@MadameSheema MadameSheema left a comment

Choose a reason for hiding this comment

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

security-engineering-productivity changes LGTM

@machadoum machadoum enabled auto-merge (squash) September 8, 2022 14:27
@@ -24,12 +25,16 @@ export const buildKpiRiskScoreQuery = ({
aggs: {
risk: {
terms: {
field: 'risk.keyword',
field:
entity === RiskScoreEntity.user ? RiskScoreFields.userRisk : RiskScoreFields.hostRisk,
},
aggs: {
unique_entries: {
cardinality: {
Copy link
Contributor

Choose a reason for hiding this comment

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

@machadoum , Since we are already aggregating by a field such as RiskScoreFields.userRisk or RiskScoreFields.hostRisk, wouldn't cardinality be always 1 (i.e. unique_entries) for that particular field? Is it intensional? See screenshot below.

image

Do you think below was the intention? Apologies if I have missed something.

 {
  "aggs": {
    "risk": {
      "terms": {
        "field": "user.risk.calculated_score_norm"
      }
    },
    "unique_entries": {
      "cardinality": {
        "field": "user.risk.calculated_score_norm"
      }
    }
  }
}

Copy link
Member Author

@machadoum machadoum Sep 9, 2022

Choose a reason for hiding this comment

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

There is a mistake in the code above, unique_entries.cardinality.field should be user.name or host.name. If the fields of the term aggregation and cardinality are the same, the result would always be 1, but that is not the case. Here is a sample of the query that runs:

    "aggs": {
      "risk": {
        "terms": {
          "field": "host.risk.calculated_level"
        },
        "aggs": {
          "unique_entries": {
            "cardinality": {
              "field": "host.name"
            }
          }
        }
    }

The terms aggregation groups all entries by risk severity, and cardinality counts how many users or hosts exist for each severity.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay... this makes sense. My bad 😰

Copy link
Contributor

@logeekal logeekal left a comment

Choose a reason for hiding this comment

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

PR checked-out and desk tested. User risk fields are populating without any issue. LGTM

@machadoum machadoum merged commit 8ce4748 into elastic:main Sep 9, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Sep 9, 2022
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:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Explore Team:Threat Hunting Security Solution Threat Hunting Team v8.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants