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.

Sign up for GitHub

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

Jump to bottom

[Security Solution] Fix showing integration status for single integration per package #187200

Merged
merged 2 commits into from
Jul 15, 2024
Merged

[Security Solution] Fix showing integration status for single integration per package #187200

merged 2 commits into from
Jul 15, 2024

Conversation

maximpn
Copy link
Contributor

@maximpn maximpn commented Jun 29, 2024 •
edited by kibanamachine
Loading

Resolves: #187199

Summary

This PR fixes displaying related integration status for rules referring packages with a single integration. A good example is Web Application Suspicious Activity: Unauthorized Method rule which refers APM integration. Package and integration names don't match but the prebuilt rule only refers a package name omitting the integration name.

Details

This fix changes response from GET /internal/detection_engine/fleet/integrations/all internal API endpoint by adding an additional integration for packages having a single integration which name doesn't match the package name.

For packages with a single integration and matching package and integration names there is only one integration returned with integration name and title omitted.

There are different packages with integrations

  • a package with multiple integrations
  • a package without integrations
  • a package with only one integration which name matches with the package name
  • a package with only one integration which name doesn't match with the package name

The latter case is apm package which has apmServer integration. For example Web Application Suspicious Activity: Unauthorized Method prebuilt rule specifies only apm package name which integration name is empty.

Screenshots before

Installation rule preview popover:
image

Rule details page:
image

Screenshots after

Installation rule preview popover:
image

Rule details page:
image

Rule details page (Elastic APM integration is installed and enabled):
image

@maximpn maximpn added bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team v8.15.0 labels Jun 29, 2024
@maximpn maximpn self-assigned this Jun 29, 2024
@maximpn maximpn force-pushed the fix-apm-integration-info branch from 4810dc2 to 929d799 Compare June 29, 2024 11:51
@maximpn maximpn requested a review from banderror June 29, 2024 18:35
@maximpn maximpn marked this pull request as ready for review June 29, 2024 18:35
@maximpn maximpn requested review from a team as code owners June 29, 2024 18:35
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management)

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

  • 💚 Build #218618 succeeded 5fcb419a0023fb817b4ccf4eb610290a7872cc66
  • 💔 Build #218617 failed 929d799aedab8b205645a9eaf14a8ede32208005
  • 💔 Build #218609 failed 2fe9e28d5840def0c79bbaeb6f90e57c4d28f7c0

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

cc @maximpn

@maximpn maximpn requested review from nikitaindik and removed request for banderror July 10, 2024 04:48
@elasticmachine
Copy link
Contributor

elasticmachine commented Jul 10, 2024 •
edited
Loading

⏳ Build in-progress, with failures

Failed CI Steps

Test Failures

  • [job] [logs] Defend Workflows Cypress Tests on Serverless #4 / Automated Response Actions From alerts "before all" hook for "should have generated endpoint and rule" "before all" hook for "should have generated endpoint and rule"
  • [job] [logs] Defend Workflows Cypress Tests on Serverless #7 / Endpoints page "before all" hook for "Shows endpoint on the list" "before all" hook for "Shows endpoint on the list"
  • [job] [logs] Defend Workflows Cypress Tests on Serverless #14 / Response console Execute operations: "before all" hook for ""execute --command" - should execute a command" "before all" hook for ""execute --command" - should execute a command"
  • [job] [logs] Defend Workflows Cypress Tests on Serverless #4 / Response console Host Isolation: "before all" hook for "should release an isolated host via response console" "before all" hook for "should release an isolated host via response console"
  • [job] [logs] Defend Workflows Cypress Tests on Serverless #5 / Response console Scan operation: "before all" hook for ""scan --path" - should scan a file" "before all" hook for ""scan --path" - should scan a file"

History

  • 💔 Build #220503 failed 6499c27c2b93a1e40324b351f9c6c475af70d79f
  • 💔 Build #220267 failed a4f53c5684c0258b664fd1fb4c3c6895f37f8bd3

cc @maximpn

nikitaindik
nikitaindik reviewed Jul 15, 2024
View reviewed changes
...ver/lib/detection_engine/fleet_integrations/api/get_all_integrations/extract_integrations.ts Outdated
// Unauthorized Method" refers "apm" package name while apm package has
// "apmserver" integration
//
// - (2) Some packages don't have policy templates at al,
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
// - (2) Some packages don't have policy templates at al,
// - (2) Some packages don't have policy templates at all,

nikitaindik
nikitaindik approved these changes Jul 15, 2024
View reviewed changes
Copy link
Contributor

@nikitaindik nikitaindik left a comment

Choose a reason for hiding this comment

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

Thanks @maximpn! The fix works! I have tested it locally. Also checked the code.

My only question is: Are we the only users of this API endpoint? I wonder if introduction of duplicate integrations in response can break things for any other scenarios this endpoint might have?

@maximpn maximpn force-pushed the fix-apm-integration-info branch from 41cd158 to 779caf7 Compare July 15, 2024 15:39
@maximpn
Copy link
Contributor Author

maximpn commented Jul 15, 2024

My only question is: Are we the only users of this API endpoint? I wonder if introduction of duplicate integrations in response can break things for any other scenarios this endpoint might have?

GET /internal/detection_engine/fleet/integrations/all is an internal API which was added in #178295 and only used by Rule Management UI. This bugfix backfills the gap with GET /internal/detection_engine/fleet/integrations/installed. The latter one isn't used anymore but it has that "duplicate integrations".

@maximpn maximpn enabled auto-merge (squash) July 15, 2024 15:43
@maximpn maximpn merged commit 875d6e9 into elastic:main Jul 15, 2024
40 checks passed
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jul 15, 2024
@maximpn
[Security Solution] Fix showing integration status for single integra…
b88e411 
…tion per package (elastic#187200)

**Resolves:** elastic#187199

## Summary

This PR fixes displaying related integration status for rules referring packages with a single integration. A good example is `Web Application Suspicious Activity: Unauthorized Method` rule which refers `APM` integration. Package and integration names don't match but the prebuilt rule only refers a package name omitting the integration name.

## Details

This fix changes response from `GET /internal/detection_engine/fleet/integrations/all` internal API endpoint by adding an additional integration for packages having a single integration which name doesn't match the package name.

For packages with a single integration and matching package and integration names there is only one integration returned with integration name and title omitted.

There are different packages with integrations

- a package with multiple integrations
- a package without integrations
- a package with only one integration which name matches with the package name
- a package with only one integration which name doesn't match with the package name

The latter case is `apm` package which has `apmServer` integration. For example `Web Application Suspicious Activity: Unauthorized Method` prebuilt rule specifies only `apm` package name which integration name is empty.

### Screenshots before

Installation rule preview popover:
<img width="1715" alt="image" src="https://github.com/elastic/kibana/assets/3775283/80f3d01f-5276-425b-835a-c78b69eab033">

Rule details page:
<img width="1722" alt="image" src="https://github.com/elastic/kibana/assets/3775283/85c833f9-b841-4016-8db9-43d4c68f1248">

### Screenshots after

Installation rule preview popover:
<img width="1718" alt="image" src="https://github.com/elastic/kibana/assets/3775283/a0ca1b4b-ebab-4de5-a169-1f6e55c74f35">

Rule details page:
<img width="1723" alt="image" src="https://github.com/elastic/kibana/assets/3775283/f647e536-2bc6-4ab8-8f4e-b4e923afb9ae">

Rule details page (Elastic APM integration is installed and enabled):
<img width="1718" alt="image" src="https://github.com/elastic/kibana/assets/3775283/33d12f7d-d9b9-43c3-9162-9bf7c6e015fc">

(cherry picked from commit 875d6e9)
@kibanamachine kibanamachine mentioned this pull request Jul 15, 2024
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.15

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@maximpn maximpn deleted the fix-apm-integration-info branch July 15, 2024 17:35
kibanamachine added a commit that referenced this pull request Jul 16, 2024
@kibanamachine @maximpn
[8.15] [Security Solution] Fix showing integration status for single …
fa88305 
…integration per package (#187200) (#188336)

# Backport

This will backport the following commits from `main` to `8.15`:
- [[Security Solution] Fix showing integration status for single
integration per package
(#187200)](#187200)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Maxim
Palenov","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-07-15T17:13:14Z","message":"[Security
Solution] Fix showing integration status for single integration per
package (#187200)\n\n**Resolves:**
https://github.com/elastic/kibana/issues/187199\r\n\r\n##
Summary\r\n\r\nThis PR fixes displaying related integration status for
rules referring packages with a single integration. A good example is
`Web Application Suspicious Activity: Unauthorized Method` rule which
refers `APM` integration. Package and integration names don't match but
the prebuilt rule only refers a package name omitting the integration
name.\r\n\r\n## Details\r\n\r\nThis fix changes response from `GET
/internal/detection_engine/fleet/integrations/all` internal API endpoint
by adding an additional integration for packages having a single
integration which name doesn't match the package name.\r\n\r\nFor
packages with a single integration and matching package and integration
names there is only one integration returned with integration name and
title omitted.\r\n\r\nThere are different packages with
integrations\r\n\r\n- a package with multiple integrations\r\n- a
package without integrations\r\n- a package with only one integration
which name matches with the package name\r\n- a package with only one
integration which name doesn't match with the package name\r\n\r\nThe
latter case is `apm` package which has `apmServer` integration. For
example `Web Application Suspicious Activity: Unauthorized Method`
prebuilt rule specifies only `apm` package name which integration name
is empty.\r\n\r\n### Screenshots before\r\n\r\nInstallation rule preview
popover:\r\n<img width=\"1715\" alt=\"image\"
src=\"https://github.com/elastic/kibana/assets/3775283/80f3d01f-5276-425b-835a-c78b69eab033\">\r\n\r\nRule
details page:\r\n<img width=\"1722\" alt=\"image\"
src=\"https://github.com/elastic/kibana/assets/3775283/85c833f9-b841-4016-8db9-43d4c68f1248\">\r\n\r\n###
Screenshots after\r\n\r\nInstallation rule preview popover:\r\n<img
width=\"1718\" alt=\"image\"
src=\"https://github.com/elastic/kibana/assets/3775283/a0ca1b4b-ebab-4de5-a169-1f6e55c74f35\">\r\n\r\nRule
details page:\r\n<img width=\"1723\" alt=\"image\"
src=\"https://github.com/elastic/kibana/assets/3775283/f647e536-2bc6-4ab8-8f4e-b4e923afb9ae\">\r\n\r\nRule
details page (Elastic APM integration is installed and enabled):\r\n<img
width=\"1718\" alt=\"image\"
src=\"https://github.com/elastic/kibana/assets/3775283/33d12f7d-d9b9-43c3-9162-9bf7c6e015fc\">","sha":"875d6e99f0304b3febb675faafadd60a1f9e2253","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","Team:Detections
and Resp","Team: SecuritySolution","Team:Detection Rule
Management","v8.15.0","v8.16.0"],"title":"[Security Solution] Fix
showing integration status for single integration per
package","number":187200,"url":"https://github.com/elastic/kibana/pull/187200","mergeCommit":{"message":"[Security
Solution] Fix showing integration status for single integration per
package (#187200)\n\n**Resolves:**
https://github.com/elastic/kibana/issues/187199\r\n\r\n##
Summary\r\n\r\nThis PR fixes displaying related integration status for
rules referring packages with a single integration. A good example is
`Web Application Suspicious Activity: Unauthorized Method` rule which
refers `APM` integration. Package and integration names don't match but
the prebuilt rule only refers a package name omitting the integration
name.\r\n\r\n## Details\r\n\r\nThis fix changes response from `GET
/internal/detection_engine/fleet/integrations/all` internal API endpoint
by adding an additional integration for packages having a single
integration which name doesn't match the package name.\r\n\r\nFor
packages with a single integration and matching package and integration
names there is only one integration returned with integration name and
title omitted.\r\n\r\nThere are different packages with
integrations\r\n\r\n- a package with multiple integrations\r\n- a
package without integrations\r\n- a package with only one integration
which name matches with the package name\r\n- a package with only one
integration which name doesn't match with the package name\r\n\r\nThe
latter case is `apm` package which has `apmServer` integration. For
example `Web Application Suspicious Activity: Unauthorized Method`
prebuilt rule specifies only `apm` package name which integration name
is empty.\r\n\r\n### Screenshots before\r\n\r\nInstallation rule preview
popover:\r\n<img width=\"1715\" alt=\"image\"
src=\"https://github.com/elastic/kibana/assets/3775283/80f3d01f-5276-425b-835a-c78b69eab033\">\r\n\r\nRule
details page:\r\n<img width=\"1722\" alt=\"image\"
src=\"https://github.com/elastic/kibana/assets/3775283/85c833f9-b841-4016-8db9-43d4c68f1248\">\r\n\r\n###
Screenshots after\r\n\r\nInstallation rule preview popover:\r\n<img
width=\"1718\" alt=\"image\"
src=\"https://github.com/elastic/kibana/assets/3775283/a0ca1b4b-ebab-4de5-a169-1f6e55c74f35\">\r\n\r\nRule
details page:\r\n<img width=\"1723\" alt=\"image\"
src=\"https://github.com/elastic/kibana/assets/3775283/f647e536-2bc6-4ab8-8f4e-b4e923afb9ae\">\r\n\r\nRule
details page (Elastic APM integration is installed and enabled):\r\n<img
width=\"1718\" alt=\"image\"
src=\"https://github.com/elastic/kibana/assets/3775283/33d12f7d-d9b9-43c3-9162-9bf7c6e015fc\">","sha":"875d6e99f0304b3febb675faafadd60a1f9e2253"}},"sourceBranch":"main","suggestedTargetBranches":["8.15"],"targetPullRequestStates":[{"branch":"8.15","label":"v8.15.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/187200","number":187200,"mergeCommit":{"message":"[Security
Solution] Fix showing integration status for single integration per
package (#187200)\n\n**Resolves:**
https://github.com/elastic/kibana/issues/187199\r\n\r\n##
Summary\r\n\r\nThis PR fixes displaying related integration status for
rules referring packages with a single integration. A good example is
`Web Application Suspicious Activity: Unauthorized Method` rule which
refers `APM` integration. Package and integration names don't match but
the prebuilt rule only refers a package name omitting the integration
name.\r\n\r\n## Details\r\n\r\nThis fix changes response from `GET
/internal/detection_engine/fleet/integrations/all` internal API endpoint
by adding an additional integration for packages having a single
integration which name doesn't match the package name.\r\n\r\nFor
packages with a single integration and matching package and integration
names there is only one integration returned with integration name and
title omitted.\r\n\r\nThere are different packages with
integrations\r\n\r\n- a package with multiple integrations\r\n- a
package without integrations\r\n- a package with only one integration
which name matches with the package name\r\n- a package with only one
integration which name doesn't match with the package name\r\n\r\nThe
latter case is `apm` package which has `apmServer` integration. For
example `Web Application Suspicious Activity: Unauthorized Method`
prebuilt rule specifies only `apm` package name which integration name
is empty.\r\n\r\n### Screenshots before\r\n\r\nInstallation rule preview
popover:\r\n<img width=\"1715\" alt=\"image\"
src=\"https://github.com/elastic/kibana/assets/3775283/80f3d01f-5276-425b-835a-c78b69eab033\">\r\n\r\nRule
details page:\r\n<img width=\"1722\" alt=\"image\"
src=\"https://github.com/elastic/kibana/assets/3775283/85c833f9-b841-4016-8db9-43d4c68f1248\">\r\n\r\n###
Screenshots after\r\n\r\nInstallation rule preview popover:\r\n<img
width=\"1718\" alt=\"image\"
src=\"https://github.com/elastic/kibana/assets/3775283/a0ca1b4b-ebab-4de5-a169-1f6e55c74f35\">\r\n\r\nRule
details page:\r\n<img width=\"1723\" alt=\"image\"
src=\"https://github.com/elastic/kibana/assets/3775283/f647e536-2bc6-4ab8-8f4e-b4e923afb9ae\">\r\n\r\nRule
details page (Elastic APM integration is installed and enabled):\r\n<img
width=\"1718\" alt=\"image\"
src=\"https://github.com/elastic/kibana/assets/3775283/33d12f7d-d9b9-43c3-9162-9bf7c6e015fc\">","sha":"875d6e99f0304b3febb675faafadd60a1f9e2253"}}]}]
BACKPORT-->

Co-authored-by: Maxim Palenov <[email protected]>
@maximpn maximpn mentioned this pull request Jul 16, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikitaindik nikitaindik nikitaindik approved these changes

Assignees

@maximpn maximpn

Labels
bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.15.0 v8.16.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@maximpn @elasticmachine @kibana-ci @kibanamachine @nikitaindik