-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[RAC] Remove rbac on security solution side #110472
[RAC] Remove rbac on security solution side #110472
Conversation
…ses (elastic#108588)" This reverts commit 1fd7038. This leaves the rule registry mock changes
…ierM/kibana into disable-rbac-alert-security-solution
…ierM/kibana into disable-rbac-alert-security-solution
Pinging @elastic/security-threat-hunting (Team:Threat Hunting) |
…ierM/kibana into disable-rbac-alert-security-solution
…ivileges page before load
…ierm/kibana into disable-rbac-alert-security-solution
…ierm/kibana into disable-rbac-alert-security-solution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes to x-pack/test/api_integration/apis/security/privileges.ts
LGTM - we are removing feature privileges that were introduced in v7.15.0
(via #108450), which hasn't been released yet. As such, there is no risk of breaking change or surprise-on-upgrade.
I am approving on code-review of this file only, to unblock this PR. I expect the security solutions team to have reviewed & tested the rest of the changes accordingly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mapping and template changes LGTM
source: `if (ctx._source['${ALERT_WORKFLOW_STATUS}'] != null) { | ||
ctx._source['${ALERT_WORKFLOW_STATUS}'] = '${status}' | ||
} | ||
if (ctx._source.signal != null && ctx._source.signal.status != null) { | ||
ctx._source.signal.status = '${status}' | ||
}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this additional logic to check for workflow_status in this route since it's security solution specific, but the route will still function correctly with it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this update @XavierM, @yctercero, and @jonathan-buttner 🙏
- Desk tested functionality locally over a zoom with several other 👀 in the Security Solution and o11y apps with a
superuser
role in a newly-created Kibana space - This review does not cover other authz scenarios
LGTM 🚀
…ierM/kibana into disable-rbac-alert-security-solution
@elasticmachine merge upstream |
@@ -532,7 +532,6 @@ export const waitForAlertsToPopulate = async (alertCountThreshold = 1) => { | |||
cy.waitUntil( | |||
() => { | |||
refreshPage(); | |||
cy.get(LOADING_INDICATOR).should('exist'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yctercero is there any reason why this line has been deleted? This was added to fix some tests and remove flakiness
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / general / Chrome X-Pack UI Plugin Functional Tests.x-pack/test/plugin_functional/test_suites/global_search/global_search_providers·ts.GlobalSearch API GlobalSearch providers SavedObject provider can search for index patternsStandard Out
Stack Trace
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
💔 Backport failed
To backport manually run: |
* wip to remove rbac * Revert "[Cases] Include rule registry client for updating alert statuses (elastic#108588)" This reverts commit 1fd7038. This leaves the rule registry mock changes * remove rbac on Trend/Count alert * update detection api for status * remove @kbn-alerts packages * fix leftover * Switching cases to leverage update by query for alert status * Adding missed files * fix bad logic * updating tests for use_alerts_privileges * remove index alias/fields * fix types * fix plugin to get the right index names * left over of alis on template * forget to use current user for create/read route index * updated alerts page to not show table when no privileges and updates to tests * fix bug when switching between o11y and security solution * updates tests and move to use privileges page when user tries to access alerts without proper access * updating jest tests * pairing with yara * bring back kbn-alerts after discussion with the team * fix types * fix index field for o11y * fix bug with updating index priv state * fix i18n issue and update api docs * fix refresh on alerts * fix render view on alerts * updating tests and checking for null in alerts page to not show no privileges page before load * fix details rules Co-authored-by: Jonathan Buttner <jonathan.buttner@elastic.co> Co-authored-by: Yara Tercero <yara.tercero@elastic.co> # Conflicts: # x-pack/plugins/security_solution/cypress/integration/detection_alerts/alerts_details.spec.ts
* wip to remove rbac * Revert "[Cases] Include rule registry client for updating alert statuses (elastic#108588)" This reverts commit 1fd7038. This leaves the rule registry mock changes * remove rbac on Trend/Count alert * update detection api for status * remove @kbn-alerts packages * fix leftover * Switching cases to leverage update by query for alert status * Adding missed files * fix bad logic * updating tests for use_alerts_privileges * remove index alias/fields * fix types * fix plugin to get the right index names * left over of alis on template * forget to use current user for create/read route index * updated alerts page to not show table when no privileges and updates to tests * fix bug when switching between o11y and security solution * updates tests and move to use privileges page when user tries to access alerts without proper access * updating jest tests * pairing with yara * bring back kbn-alerts after discussion with the team * fix types * fix index field for o11y * fix bug with updating index priv state * fix i18n issue and update api docs * fix refresh on alerts * fix render view on alerts * updating tests and checking for null in alerts page to not show no privileges page before load * fix details rules Co-authored-by: Jonathan Buttner <jonathan.buttner@elastic.co> Co-authored-by: Yara Tercero <yara.tercero@elastic.co> # Conflicts: # x-pack/plugins/security_solution/cypress/integration/detection_alerts/alerts_details.spec.ts
* [RAC] Remove rbac on security solution side (#110472) * wip to remove rbac * Revert "[Cases] Include rule registry client for updating alert statuses (#108588)" This reverts commit 1fd7038. This leaves the rule registry mock changes * remove rbac on Trend/Count alert * update detection api for status * remove @kbn-alerts packages * fix leftover * Switching cases to leverage update by query for alert status * Adding missed files * fix bad logic * updating tests for use_alerts_privileges * remove index alias/fields * fix types * fix plugin to get the right index names * left over of alis on template * forget to use current user for create/read route index * updated alerts page to not show table when no privileges and updates to tests * fix bug when switching between o11y and security solution * updates tests and move to use privileges page when user tries to access alerts without proper access * updating jest tests * pairing with yara * bring back kbn-alerts after discussion with the team * fix types * fix index field for o11y * fix bug with updating index priv state * fix i18n issue and update api docs * fix refresh on alerts * fix render view on alerts * updating tests and checking for null in alerts page to not show no privileges page before load * fix details rules Co-authored-by: Jonathan Buttner <jonathan.buttner@elastic.co> Co-authored-by: Yara Tercero <yara.tercero@elastic.co> # Conflicts: # x-pack/plugins/security_solution/cypress/integration/detection_alerts/alerts_details.spec.ts * skip test Co-authored-by: Xavier Mouligneau <189600+XavierM@users.noreply.github.com>
* [Security Solution] Updates loock-back time on Cypress tests (#110609) * updates loock-back time * updates loock-back value for 'expectedExportedRule' * skips tests to unblock 7.15 branch * [RAC] Remove rbac on security solution side (#110472) * wip to remove rbac * Revert "[Cases] Include rule registry client for updating alert statuses (#108588)" This reverts commit 1fd7038. This leaves the rule registry mock changes * remove rbac on Trend/Count alert * update detection api for status * remove @kbn-alerts packages * fix leftover * Switching cases to leverage update by query for alert status * Adding missed files * fix bad logic * updating tests for use_alerts_privileges * remove index alias/fields * fix types * fix plugin to get the right index names * left over of alis on template * forget to use current user for create/read route index * updated alerts page to not show table when no privileges and updates to tests * fix bug when switching between o11y and security solution * updates tests and move to use privileges page when user tries to access alerts without proper access * updating jest tests * pairing with yara * bring back kbn-alerts after discussion with the team * fix types * fix index field for o11y * fix bug with updating index priv state * fix i18n issue and update api docs * fix refresh on alerts * fix render view on alerts * updating tests and checking for null in alerts page to not show no privileges page before load * fix details rules Co-authored-by: Jonathan Buttner <jonathan.buttner@elastic.co> Co-authored-by: Yara Tercero <yara.tercero@elastic.co> # Conflicts: # x-pack/plugins/security_solution/cypress/integration/detection_alerts/alerts_details.spec.ts * skip tests Co-authored-by: Gloria Hornero <snootchie.boochies@gmail.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Xavier Mouligneau <189600+XavierM@users.noreply.github.com>
Summary
From: https://github.com/elastic/security-team/issues/1607#issuecomment-907142876
Users of Elastic Security solutions commonly rely on the application of Elasticsearch index privileges to users' roles to control access to the .siem-signals* indices within their deployments. Any change that causes these privileges to be ignored or bypassed would be considered a security violation or data leakage by these users.
This practice is especially common amongst:
Organizations that have large SOC teams with multiple tiers of analysts
Organizations that operate under strict interpretations of GDPR and other data protection standards and require them to limit the exposure of personal data even to security analysts.
Organizations that provide a managed SIEM service or other managed security service to multiple customers (MSSP)
Organizations that maintain a Center of Excellence (COE) for Elastic Deployments used for Security Solution.
Common use cases include:
Using Elasticsearch index privileges with document level security to prohibit lower tier analysts from accessing alerts created from sensitive source data. One Kibana space is used for T1/T2/T3 analysts, who collaborate to run the SOC. Some detection rules operate on data that contains personal information of organization employees. T1 analysts do not have sufficient security clearance to access that source data. Since alerts contain information copied from the source events, T1 analysts must be blocked from accessing those alerts that contain personal data. Users set up document level security on .siem-signals- indices that prohibit T1 analyst roles from accessing alert documents that have a certain field:value such event.dataset:sensitive_data
Using Elasticsearch index privileges with field level security to prohibit lower tier analysts from accessing fields in alerts that may have been enriched with personal data of organization employees. For example, source data, even from data sources that do not contain personal data, may be enriched with an employee ID, name, or email address at various points in the data ingestion process. Lower tier analysts do not have security clearance to view those fields in alerts, so their roles are configured to deny access to those fields such as ECS field user.full_name. T1 analysts may escalate these alerts to higher tier analysts or a SOC manager, who need to be able to see these fields in order to initiate incident response.
Using Elasticsearch index privileges with document level security as a secondary control (i.e. belt and suspenders) to block access to one tenant's data by other tenants in "single cluster multi-tenant" environment. The user has configured one tenant per Kibana space, and is relying on Elasticsearch index privileges applied to each tenant's roles such that they can access only alerts in their siem-signals- indices. In addition, they configure document-level security so that only alerts that contain a field:value like ECS field organization.name:tenant1 can be accessed. This way, if there is ever an error with regards to space assignments or usage, the DLS privileges will provide a secondary control to protect the privacy of tenant data.
To preserve proper expected operation of security solution environments, the general requirements for adding Kibana feature privileges and sub-feature privileges to Alerts should include all of the following:
All Elasticsearch security settings, including index-level, document-level, field-level, and attribute-based access controls must continue to operate properly on alerts indices.
Alerts indices must continue to be private to the Kibana Space they are created in by default
Alert indices must continue to be accessible via Cross Cluster Search
Alert indices must continue to be available as input data for rules (aka "rules on alerts")
Alert data must continue to be available for all non-solution Kibana apps (discover, lens, graph), and all Elasticsearch analysis.
Checklist