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

[Serverless][Security Solution][Endpoint] Restrict endpoint exceptions on serverless via plugin sub-features #164107

Conversation

ashokaditya
Copy link
Member

@ashokaditya ashokaditya commented Aug 16, 2023

What this PR changes

branched from /pull/163759

  • Introduces new AppFeatures package @kbn/security-solution-features with the common logic and AppFeatureService to apply offering specific configurations for Security Solution features independently for Serverless and ESS. This logic is replacing the earlier AppFeatures in order to introduce new Kibana feature privileges for serverless PLIs so that new Kibana privileges introduced for serverless PLIs do not affect/show up as new Kibana feature privileges in ESS.
  • Gates endpoint exceptions on alerts/rules based on serverless PLI configurations. On serverless Endpoint exceptions should be accessible/seen only on endpoint essentials/complete.

New AppFeatures logic architecture diagram:

Security Solution Features (Current)

Note: Corresponding API changes related to endpoint exceptions will be in a new PR, along with the last set of UX changes for hiding the Endpoint exceptions tab from the Rules details page.

How to review

  • Setup for Servlerless
    • Run yarn es snapshot on a terminal window to start ES.
    • Copy config/serverless.security.yml to config/serverless.security.dev.yml
    • Run yarn serverless-security --no-base-path on another terminal window to start kibana in serverless mode
    • Run node x-pack/plugins/security_solution/scripts/endpoint/endpoint_agent_emulator.js --asSuperuser on a new window and then select 1 to Load Endoints and then 1 to Run the loader script. This will load some fake agents/alerts data to test with.

Tests (Serverless)

  • with
    { product_line: 'security', product_tier: 'essentials' } or { product_line: 'security', product_tier: 'complete' }
    and
    { product_line: 'endpoint', product_tier: 'essentials' } or { product_line: 'endpoint', product_tier: 'complete' }
  1. Navigate to Rules>Shared exception lists via http://localhost:5601/app/security/exceptions
  2. Test that you can see Endpoint Security Exception List card on the shared exception lists page.
  3. Navigate to Alerts page via app/security/alerts, you should see endpoint alerts. If not, then click on Manage Rules and then disable/enable Endpoint Security rules. That should trigger alerts to show up on the Alerts table.
  4. Click on View Details button under Actions column. Once the flyout is visible, click on Take Action and verify that Add Endpoint exception is visible/enabled/clickable on the menu.
  5. Click on More actions button under Actions column and verify that Add Endpoint exception is visible/enabled/clickable on the menu.
  6. Click on Investigate in timeline button under Actions column; when the timeline view is visible and the alert item is displayed, click on buttons mentioned in 4. and 5. above and verify the same.
  7. Navigate to Rules>DetectionRules>Endpoint Security rule under the Rules table. Select the Alerts tab.
  8. Click and verify View details,More actions and Investigate in timeline buttons same as in 4., 5., 6. above.
  9. You should be able to see the Endpoint exceptions tab as well. Click and verify that you can see the tab's content.
  • with
    { product_line: 'security', product_tier: 'essentials' } or { product_line: 'security', product_tier: 'complete' }
  1. Edit config/serverless.security.dev.yml so that endpoint product line item is commented out.
  2. Test that you can not see Endpoint Security Exception List card on the shared exception lists page.
  3. Items 4. 5. 6. as above but the menu items should be disabled. This can be verified with fake data only as with a real endpoint, endpoint alerts are actually not visible at all.

Tests (ESS)

On the ESS side, endpoint exceptions are not affected by this change and work as usual based on index privileges.

semd and others added 9 commits August 11, 2023 18:36
…features_configs_to_plugins

# Conflicts:
#	x-pack/plugins/security_solution/public/rules/links.ts
#	x-pack/plugins/security_solution/tsconfig.json
#	x-pack/plugins/security_solution_serverless/server/plugin.ts
#	x-pack/plugins/security_solution_serverless/tsconfig.json
@ashokaditya ashokaditya self-assigned this Aug 16, 2023
@ashokaditya ashokaditya added Team:Defend Workflows “EDR Workflows” sub-team of Security Solution OLM Sprint v8.11.0 labels Aug 16, 2023
@ashokaditya ashokaditya changed the title [Security Solution][Endpoint] Restrict endpoint exceptions on serverless via plugin sub-features [Serverless][Security Solution][Endpoint] Restrict endpoint exceptions on serverless via plugin sub-features Aug 16, 2023
@ashokaditya ashokaditya force-pushed the task/dw-serverless-endpoint-exceptions-pli-with-plugin-sub-features branch from fae0330 to 4119d61 Compare August 29, 2023 08:59
@kibana-ci
Copy link
Collaborator

kibana-ci commented Aug 29, 2023

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #41 / endpoint Response Actions Responder from timeline should show Responder from alert in a timeline

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 4485 4486 +1
securitySolutionServerless 413 415 +2
total +3

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/security-solution-features - 14 +14
@kbn/utility-types 15 16 +1
cases 75 84 +9
securitySolution 111 104 -7
total +17

Any counts in public APIs

Total count of every any typed public API. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats any for more detailed information.

id before after diff
securitySolution 3 0 -3

Async chunks

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

id before after diff
securitySolution 12.6MB 12.6MB +5.0KB
securitySolutionServerless 266.4KB 266.4KB +2.0B
total +5.0KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
@kbn/security-solution-features - 6 +6
securitySolution 34 32 -2
total +4

Page load bundle

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

id before after diff
cases 152.0KB 152.1KB +129.0B
securitySolution 62.6KB 61.8KB -770.0B
securitySolutionServerless 38.4KB 39.7KB +1.3KB
total +675.0B
Unknown metric groups

API count

id before after diff
@kbn/security-solution-features - 14 +14
@kbn/utility-types 36 37 +1
cases 94 104 +10
securitySolution 177 170 -7
total +18

History

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

cc @ashokaditya

Copy link
Contributor

@semd semd left a comment

Choose a reason for hiding this comment

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

@ashokaditya Great work integrating the features package.
Thank you!! 💯

Copy link
Member

@cnasikas cnasikas left a comment

Choose a reason for hiding this comment

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

ResponseOps changes LGTM

@ashokaditya ashokaditya requested a review from dasansol92 August 30, 2023 09:32
Copy link
Contributor

@MiriamAparicio MiriamAparicio left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@dasansol92 dasansol92 left a comment

Choose a reason for hiding this comment

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

A lot of work here, thanks! Left two questions but other than that, this looks good!

if (!canWriteEndpointExceptions) {
return {
exceptionActionItems: exceptionActionItems.map((item) => {
return { ...item, disabled: item.name === ACTION_ADD_ENDPOINT_EXCEPTION };
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we show any tooltip or callout when this is disabled? I wonder if having this disabled with no messaging could generate some confusion for users.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call @dasansol92. So currently without this change, we do show the Add endpoint exception menu item but it is disabled if it is a non-endpoint alert or the user doesn't have the right privilege (on index). We don't show a tooltip though.

This change is disabling the menu item based on available product-line items, so maybe we should show a tooltip to distinguish from the above case. (On the serverless side, however, the user without an endpoint PLI won't see any endpoint alerts, so this is the only time they see a disabled menu item). Thoughts @kevinlog @caitlinbetz?

createExceptionList(getExceptionList1(), getExceptionList1().list_id).as(
'exceptionListResponse'
);
createExceptionList(getExceptionList1(), getExceptionList1().list_id);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this (an the other one below) change required?

Copy link
Member Author

@ashokaditya ashokaditya Aug 31, 2023

Choose a reason for hiding this comment

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

@dasansol92 Looks like the network requests were not actually waited on, anywhere in the test suites. Might have been leftover/redundant code from /pull/152757

Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

LGTM! Great work!

Copy link
Contributor

@vitaliidm vitaliidm left a comment

Choose a reason for hiding this comment

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

Detection Engine code area LGTM

* 2.0.
*/
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: license message duplicated

Copy link
Contributor

Choose a reason for hiding this comment

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

@vitaliidm I am writing the README of the package in a separate PR, I'll clean this in there. Thanks!

@semd semd merged commit 6e367d9 into elastic:main Aug 31, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Aug 31, 2023
@ashokaditya ashokaditya deleted the task/dw-serverless-endpoint-exceptions-pli-with-plugin-sub-features branch August 31, 2023 11:36
@ashokaditya ashokaditya restored the task/dw-serverless-endpoint-exceptions-pli-with-plugin-sub-features branch August 31, 2023 12:24
@ashokaditya ashokaditya deleted the task/dw-serverless-endpoint-exceptions-pli-with-plugin-sub-features branch August 31, 2023 12:35
ashokaditya added a commit to ashokaditya/kibana that referenced this pull request Sep 14, 2023
ashokaditya added a commit that referenced this pull request Oct 2, 2023
… rule details and API changes (#165613)

## What this PR changes

Follow up of /pull/164107/

For serverless ES/Kibana, it gates exception list API for endpoint
exceptions and restricts endpoint exceptions tab on Endpoint Security
rule details based on project PLIs. If no endpoint PLIs, endpoint
exceptions should not be accessible.

- [x] Add upselling to `app/security/exceptions/details/endpoint_list`
page
- [ ] Tests (WIP) - in a follow up PR

### How to review

Best to follow along commits for a code review. Below are details to
manually test the changes.

- Setup for _Servlerless_
- Run `yarn es serverless --kill --clean --license trial -E
xpack.security.authc.api_key.enabled=true` on a terminal window to start
ES.
- Copy `config/serverless.security.yml` to
`config/serverless.security.dev.yml`
- Run `yarn serverless-security --no-base-path` on another terminal
window to start kibana in serverless mode
  - Log in using `serverless_security` user.

### Tests (Serverless)
This needs to be tested with a custom user/role and not
`elastic_serverless` which has `superuser` role.

1. ### PLI configs
`{ product_line: 'security', product_tier: 'essentials' }` or `{
product_line: 'security', product_tier: 'complete' }`
and
`{ product_line: 'endpoint', product_tier: 'essentials' }` or `{
product_line: 'endpoint', product_tier: 'complete' }`

- #### UX
1. Navigate to Rules via `http://localhost:5601/app/security/rules/`.
Click on `Add Elastic rules`.
  2. Select and add `Endpoint Security` rule.
3. Click `Endpoint Security` and navigate to the rules details page, and
you should see `Endpoint exceptions` tab. The tabs visible are `Alerts`,
`Endpoint exceptions`, `Rule exceptions`, `Execution results`.
4. Navigate to Rules>Shared Exception Lists > Endpoint Security
Exception List via `app/security/exceptions/details/endpoint_list` and
you should be able to see the page with any added endpoint exceptions.

- #### API requests (with user `serverless_security`)
  1. should get a status `200` on`POST api/exception_lists/items`
2. should get a status `200` on `POST
api/exception_lists/_export?id=endpoint_list&list_id=endpoint_list&namespace_type=agnostic&include_expired_exceptions=true`
  3. should get a status `200` on `PUT api/exception_lists/items`
  5. should get a status `200` on `DELETE api/exception_lists/items`
6. should get a status `200` on `GET
api/exception_lists/items/_find?list_id=endpoint_list&namespace_type=agnostic`

2. ### PLI configs
`{ product_line: 'security', product_tier: 'essentials' }` or `{
product_line: 'security', product_tier: 'complete' }`

- #### UX
1. Navigate to Rules via `http://localhost:5601/app/security/rules/`.
Click on `Add Elastic rules`.
  2. Select and add `Endpoint Security` rule. 
3. Click `Endpoint Security` and navigate to the rules details page, and
you should not see `Endpoint exceptions` tab. The only tabs visible are
`Alerts`, `Rule exceptions`, `Execution results`.
![Screenshot 2023-09-14 at 3 33 24
PM](https://github.com/elastic/kibana/assets/1849116/185ea210-c457-4469-a824-cdcaa2893cb6)
4. Navigate to Rules>Shared Exception Lists > Endpoint Security
Exception List via `app/security/exceptions/details/endpoint_list` and
you should see an upsell message.
![Screenshot 2023-09-14 at 3 29 14
PM](https://github.com/elastic/kibana/assets/1849116/6700fc2d-9a9d-4a57-ad7f-5505e02cec71)


- #### API requests
  1. should get a status `403` on`POST api/exception_lists/items`
2. should get a status `403` on `POST
api/exception_lists/_export?id=endpoint_list&list_id=endpoint_list&namespace_type=agnostic&include_expired_exceptions=true`
  3. should get a status `403` on `PUT api/exception_lists/items`
  6. should get a status `403` on `DELETE api/exception_lists/items`
7. should get a status `403` on `GET
api/exception_lists/items/_find?list_id=endpoint_list&namespace_type=agnostic`
---


**Flaky FTRs**
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3248
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3255


### Checklist

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [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
- [x] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [ ] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)


### Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to
identify risks that should be tested prior to the change/feature
release.

When forming the risk matrix, consider some of the following examples
and how they may potentially impact the change:

| Risk | Probability | Severity | Mitigation/Notes |

|---------------------------|-------------|----------|-------------------------|
| Multiple Spaces—unexpected behavior in non-default Kibana Space.
| Low | High | Integration tests will verify that all features are still
supported in non-default Kibana Space and when user switches between
spaces. |
| Multiple nodes—Elasticsearch polling might have race conditions
when multiple Kibana nodes are polling for the same tasks. | High | Low
| Tasks are idempotent, so executing them multiple times will not result
in logical error, but will degrade performance. To test for this case we
add plenty of unit tests around this logic and document manual testing
procedure. |
| Code should gracefully handle cases when feature X or plugin Y are
disabled. | Medium | High | Unit tests will verify that any feature flag
or plugin combination still results in our service operational. |
| [See more potential risk
examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) |


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: kibanamachine <[email protected]>
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 OLM Sprint release_note:enhancement Team:APM All issues that need APM UI Team support Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.