-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Security Solution] Data quality dashboard persistence #173185
Conversation
Pinging @elastic/security-threat-hunting (Team:Threat Hunting) |
Pinging @elastic/security-threat-hunting-explore (Team:Threat Hunting:Explore) |
@elasticmachine merge upstream |
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.
Spaces should be listed under optionalPlugins
and you'll have to implement the if spaces do ...
pattern.
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.
"spaces" can be disabled and should be listed as an optional plugin. See https://github.com/elastic/kibana/blob/main/src/plugins/saved_search/kibana.jsonc
This is the expected behavior. This data stream is "space-aware", it can not be created on server start, it is created during runtime when we know the space the user is using. If there's an error in the creation it will pop up to the UI. |
We discussed that with @dhru42, we don't want to restrict the /results POST or GET from the UI, all users that can use the data quality dashboard should be able to persist the check results and read the persisted checks. We are already preventing data leaks by checking the user's index privileges against the results being retrieved. This criteria only affects this specific data quality dashboard API route, which is only used to retrieve the latest check results of authorized index patterns. Arbitrary queries to |
@semd and I paired to debug this, and observed that an index (instead of a data stream) with empty mappings was created around the time the error above started appearing. These observations were made via:
It appears that Elasticsearch created the index as a side effect, but we weren't able to reproduce this behavior. ResolutionIf the Data Quality page displays an
|
Yes, both users consistently observed a |
const userEsClient = services.core.elasticsearch.client.asCurrentUser; | ||
const privileges = await userEsClient.security.hasPrivileges({ | ||
index: [ | ||
{ names: patterns.split(','), privileges: ['all', 'read', 'view_index_metadata'] }, |
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.
Consider revising the implementation of this privileges
check, because per the following screenshot:
the following privileges are required to check an index:
monitor
ormanage
view_index_metadata
read
Consider:
- The
all
privilege is not required - either
monitor
ormanage
is required to check an index- In a future release,
read
may be expanded to beread
orread_cross_cluster
, if the Elasticsearchstats
and_ilm/explain
APIs are enhanced to support CCS.
- In a future release,
{ names: patterns.split(','), privileges: ['all', 'read', 'view_index_metadata'] }, | ||
], | ||
}); | ||
const authorizedPatterns = Object.keys(privileges.index).filter((pattern) => |
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.
Consider an alternative authorization check, because per the following scenario, this check appears to return unexpected results when two users visit the Data Quality page, but have different index privileges for the same pattern.
Example: given a pattern from the Security Solution default, e.g. packetbeat-*
, one user may have privileges to view all packetbeat-*
data streams and indicies (i.e. .ds-packetbeat-8.9.2-2023.12.06-000001
and packetbeat-7.17.9-2024.01.08-000002
), while another user may have privileges for ONLY data streams created by the 8.x
version of Packetbeat (i.e. .ds-packetbeat-8.9.2-2023.12.06-000001
), but not the 7.x
version of Packetbeat.
To demonstrate the unexpected results, first add the following console.log
statements to the authorization step, such that the Kibana server logs include the privileges
and derived authorizedPatterns
:
// Confirm user has authorization for the requested patterns
const { patterns } = request.query;
const userEsClient = services.core.elasticsearch.client.asCurrentUser;
const privileges = await userEsClient.security.hasPrivileges({
index: [
{ names: patterns.split(','), privileges: ['all', 'read', 'view_index_metadata'] },
],
});
console.log('--> privileges', JSON.stringify(privileges, null, 2));
const authorizedPatterns = Object.keys(privileges.index).filter((pattern) =>
Object.values(privileges.index[pattern]).some((v) => v === true)
);
console.log('--> authorizedPatterns', JSON.stringify(authorizedPatterns, null, 2));
if (authorizedPatterns.length === 0) {
return response.ok({ body: [] });
}
Next, create a new role called ONLY_SOME_PACKETBEAT_ROLE
, such that
- The user has access to all
auditbeat-*
indices - The user has access to only some
packetbeat-*
indices
per the following screenshot of the ONLY_SOME_PACKETBEAT_ROLE
:
In the screenshot above, the following pattern ONLY matches data streams created by the 8.x
version of Packetbeat:
packetbeat-8.*
As a result, users with the ONLY_SOME_PACKETBEAT_ROLE
may view (and check), for example, a data stream created by the 8.x
version of Packetbeat, with a backing index named:
.ds-packetbeat-8.9.2-2023.12.06-000001
but that same user may NOT check an index created by the 7.x
version of Packetbeat, e.g. an index named:
packetbeat-7.17.9-2024.01.08-000002
Next create a user named somepacketbeat
, and assign that user the ONLY_SOME_PACKETBEAT_ROLE
, per the following screenshot:
Finally, create one more user named alsosuperuser
, and assign that user the superuser
role, per the screenshot below:
Navigate to Stack Management > Users. Per the screenshot below:
✅ The alsosuperuser
user has the superuser
role (just like the elastic
user)
✅ The somepacketbeat
user has the ONLY_SOME_PACKETBEAT_ROLE
Establish a baseline with the elastic
user
-
Login to Kibana as the
elastic
user, who has thesuperuser
role -
Navigate to Security > Dashboards > Data Qualty
-
Click the
Check all
button
Note that:
✅ The .alerts-security.alerts-default
pattern contains results, per the screenshot below
✅ The auditbeat-*
pattern contains results, per previous screenshot above
✅ The packetbeat-*
pattern contains 6
pages of results, which include indices from both the 8.x
and 7.x
versions of Packetbeat, per the screeenshot below:
Also observe the Kibana server logs console.log
output for the elastic
user:
--> privileges {
"username": "elastic",
"has_all_requested": true,
"cluster": {},
"index": {
".alerts-security.alerts-default": {
"all": true,
"read": true,
"view_index_metadata": true
},
"auditbeat-*": {
"all": true,
"read": true,
"view_index_metadata": true
},
"logs-*": {
"all": true,
"read": true,
"view_index_metadata": true
},
"packetbeat-*": {
"all": true,
"read": true,
"view_index_metadata": true
}
},
"application": {}
}
--> authorizedPatterns [
".alerts-security.alerts-default",
"auditbeat-*",
"logs-*",
"packetbeat-*"
]
Compare the alsosuperuser
user with the elastic
user
-
Login to Kibana as the
alsosuperuser
user, who also has thesuperuser
role, just like theelastic
user -
Navigate to Security > Dashboards > Data Quality
Note that:
✅ As expected, Last checked: --
is displayed for the alsosuperuser
user, because this user has never performed a data quality check
✅ As expected, the .alerts-security.alerts-default
pattern contains results, because the alsosuperuser
is viewing results created by the elastic
user, per the screenshot below:
✅ As expected, the auditbeat-*
pattern contains results (from the elastic
user), per previous screenshot above
✅ As expected, the packetbeat-*
pattern contains 6
pages of results (created by the elastic
user), which include indices from both the 8.x
and 7.x
versions of Packetbeat, per the screeenshot below:
Also observe the Kibana server logs console.log
output for the alsosuperuser
user:
--> privileges {
"username": "alsosuperuser",
"has_all_requested": true,
"cluster": {},
"index": {
".alerts-security.alerts-default": {
"all": true,
"read": true,
"view_index_metadata": true
},
"auditbeat-*": {
"all": true,
"read": true,
"view_index_metadata": true
},
"logs-*": {
"all": true,
"read": true,
"view_index_metadata": true
},
"packetbeat-*": {
"all": true,
"read": true,
"view_index_metadata": true
}
},
"application": {}
}
--> authorizedPatterns [
".alerts-security.alerts-default",
"auditbeat-*",
"logs-*",
"packetbeat-*"
]
So far, the output demonstrates that:
✅ As expected, the alsosuperuser
user may view all the persisted results from the elastic
user
✅ As expected, the privileges
and derived authorizedPatterns
are the same for both the alsosuperuser
and elastic
user
Compare the somepacketbeat
user with the elastic
user (has unexpected results)
-
Login to Kibana as the
somepacketbeat
user, who has DIFFERENT permissions compared with theelastic
andalsosuperuser
users -
Navigate to Security > Dashboards > Data Quality
Note that:
✅ As expected, Last checked: --
is displayed for the somepacketbeat
user, because this user has never performed a data quality check, per the screeenshot below
✅ As expected, the .alerts-security.alerts-default
pattern is not displayed, because the somepacketbeat
user (unlike the elastic
user) does not have access to the .alerts-security.alerts-default
pattern
✅ As expected, the auditbeat-*
pattern contains results (from the elastic
user)
✅ As expected, the packetbeat-*
pattern contains 3
pages of indices, (as opposed to 6
for the elastic
and alsosuperuser
users), because the ONLY_SOME_PACKETBEAT_ROLE
role only allows access to data streams created by the 8.x
version of Packetbeat
❌ NOT EXPECTED: The packetbeat-*
results (created by the elastic
user) are NOT shown for the somepacketbeat
user, even though the somepacketbeat
has access to the 8.x
Packetbeat data streams
This unexpected behavior appears to be related to the following unexpected authorizedPatterns
derived state in the Kibana server log for the somepacketbeat
user:
--> privileges {
"username": "somepacketbeat",
"has_all_requested": false,
"cluster": {},
"index": {
"auditbeat-*": {
"all": false,
"read": true,
"view_index_metadata": true
},
"packetbeat-*": {
"all": false,
"read": false,
"view_index_metadata": false
}
},
"application": {}
}
--> authorizedPatterns [
"auditbeat-*"
]
❌ NOT EXPECTED: The authorizedPatterns
in the Kibana server log output for the somepacketbeat
user does NOT include Packeatbeat, even though the stats
API (correctly) returns results for data streams created by the 8.x
version of packetbeat.
It appears that Packetbeat is not included in authorizedPatterns
because the privileges
entry for Packetbeat in the console.log
output:
"packetbeat-*": {
"all": false,
"read": false,
"view_index_metadata": false
}
},
has "read": false
and "view_index_metadata": false
.
It appears both read
and view_index_metadata
are false
for the somepacketbeat
user because the packetbeat-*
pattern matches data streams created by the 8.x version of Packetbeat (e.g. .ds-packetbeat-8.9.2-2023.12.06-000001
), which the user has access to, AND indices created by the 7.x version (e.g. packetbeat-7.17.9-2024.01.08-000002
), which the somepacketbeat
user does NOT have access to.
Conclusion: it appears that the current implementation of authorization returns incorrect results when the specified pattern is not applicable to ALL data streams and / or indices that match the pattern.
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 @semd for creating the new reusable Data Stream Adapter package, @kbn/data-stream-adapter
, and consuming it to persist results in the Data Quality Dashboard! 🙏
The new package is useful for this use case (Data Quality dashboard persistence), and for creating data streams in future use cases.
Desk testing
🟢 Plugins that create new data streams via the @kbn/data-stream-adapter
package at startup (when Kibana server starts) are resilient to errors that may occur.
For example, at least 4 API calls are made (some in parallel) at startup to create the data stream (metadata, without a backing index), index template, and component templates required to persist Data Quality dashboard results.
✅ Any one of the 4 API calls in this example may fail. While desk testing, none of those (simulated) API failures will prevent Kibana from starting. Details of the failure (at startup) are logged to the Kibana log.
✅ For some use cases, like the Data Quality dashboard, the backing indices for a data stream may only be created on demand (when users access a page), because the newly-created indices are space-specific. Errors that occur when attempting to create space-specific backing indices (on demand) are also logged to the Kibana log.
✅ The Data Quality dashboard displays error toasts when errors occur while reading or writing persisted results
🟡 Consider merging this PR to make the @kbn/data-stream-adapter
package available to PRs that may consume it, but for now, consider disabling the creation of the data streams required for persisting Data Quality results at startup. This approach would enable the content of the Data Quality checks to evolve in a follow-up PR without the need to migrate mappings and results from their current state (in this PR) to a future state. Similarly, this would allow for revisions to the authorization check that's specific to data quality results.
LGTM 🚀
@elasticmachine merge upstream |
Sure, I am going to disable the persistence layer of the data quality dashboard so we can merge and make the data stream adapter library available in main.
|
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 adding this feature, Sergi, decent work! Very clean code and well structured. 👍
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Public APIs missing comments
Async chunks
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @semd |
* main: (520 commits) Update Kibana code editor dependencies (#171720) [SLOs] Hide view in app in slo alerts table in slo details page (#175441) [api-docs] 2024-01-25 Daily api_docs build (#175502) [DOCS] Add buildkite links to doc preview comments (#175463) skip flaky suite (#175443) [Security Solution][Timeline] refactor timeline modal save timeline button (#175343) [RAM] Stack Management::Rules loses user selections when navigating back (#174954) [Security Solution][Timeline] refactor timeline modal attach to case button (#175163) Upgrade EUI to v92.1.1 (#174955) [Fleet]: Beta label is shown inconsistently while selecting proxy under Fleet settings. (#170634) [Cloud Security] Rules Combo Box filters Custom component (#175175) skip flaky suite (#175407) [Security Solution][Timeline] refactor timeline modal open timeline button (#175335) [Embedded Console] Introduce kbnSolutionNavOffset CSS variable (#175348) [Console] disable access to embedded console without dev tools capability (#175321) fix(x-pack/reporting): use FIPS-compliant ID generator `uuidv4` in Reporting plugin (#174809) [Security Solution] Data quality dashboard persistence (#173185) [RAM][Observability] Add alert fields table to Observability flyout (#174685) test: add missing await for connector table disappearance (#175430) [RAM][Maintenance Window] Fix maintenance window FE types and transforms (#173888) ...
## Summary follow-up of #173185 This PR enables the persistence layer implemented in the previous PR, applying the following changes: - Update the mapping to store unitary index results instead of storing the whole pattern with the results in each document. - Change the query to get the stored results by aggregating documents by indexName. The authorized indexNames derived from the `pattern` parameter are retrieved using the `indices.get` request. - A bug involving a race condition with the initialization and the retrieval of stored results, resulting in an unintended reset of the results in the UI, has been fixed. https://github.com/elastic/kibana/assets/17747913/0598606b-c5f4-42b3-901c-f86a3cac65e4 --------- Co-authored-by: Kibana Machine <[email protected]>
## Summary follow-up of elastic#173185 This PR enables the persistence layer implemented in the previous PR, applying the following changes: - Update the mapping to store unitary index results instead of storing the whole pattern with the results in each document. - Change the query to get the stored results by aggregating documents by indexName. The authorized indexNames derived from the `pattern` parameter are retrieved using the `indices.get` request. - A bug involving a race condition with the initialization and the retrieval of stored results, resulting in an unintended reset of the results in the UI, has been fixed. https://github.com/elastic/kibana/assets/17747913/0598606b-c5f4-42b3-901c-f86a3cac65e4 --------- Co-authored-by: Kibana Machine <[email protected]>
## Summary issue elastic/security-team#7382 ### Data Stream Adapter This PR introduces the `@kbn/data-stream-adapter` package, which is a utility library to facilitate Data Stream creation and maintenance in Kibana, it was inspired by the data stream implementation in the Alerts plugin. The library has two exports: - `DataStreamSpacesAdapter`: to manage space data streams. It uses the `name-of-the-data-stream-<spaceId>` naming pattern. - `DataStreamAdapter`: to manage single (not space-aware) data streams. Usage examples in the package [README](https://github.com/elastic/kibana/blob/450be0369decdef156902d90a5f7292250ebd8cb/packages/kbn-data-stream-adapter/README.md) ### Data Quality Dashboard The `DataStreamSpacesAdapter` has been integrated into the data quality dashboard to store all the quality checks users perform. The information stored is the metadata (also used for telemetry) and the actual data rendered in the tables. FieldMap definition [here](https://github.com/elastic/kibana/blob/450be0369decdef156902d90a5f7292250ebd8cb/x-pack/plugins/ecs_data_quality_dashboard/server/lib/data_stream/results_field_map.ts) ### Demo https://github.com/elastic/kibana/assets/17747913/311a0bf5-004b-46d7-8140-52a233361c91 --------- Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Philippe Oberti <[email protected]> Co-authored-by: Garrett Spong <[email protected]> Co-authored-by: Efe Gürkan YALAMAN <[email protected]> Co-authored-by: Tiago Costa <[email protected]> Co-authored-by: Sander Philipse <[email protected]> Co-authored-by: JD Kurma <[email protected]> Co-authored-by: Jan Monschke <[email protected]> Co-authored-by: Patryk Kopyciński <[email protected]> Co-authored-by: Khristinin Nikita <[email protected]> Co-authored-by: Marco Liberati <[email protected]> Co-authored-by: Julia Rechkunova <[email protected]> Co-authored-by: Stratoula Kalafateli <[email protected]> Co-authored-by: Davis McPhee <[email protected]> Co-authored-by: Eyo O. Eyo <[email protected]> Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Marta Bondyra <[email protected]> Co-authored-by: Søren Louv-Jansen <[email protected]> Co-authored-by: Dzmitry Lemechko <[email protected]> Co-authored-by: Candace Park <[email protected]>
## Summary follow-up of elastic#173185 This PR enables the persistence layer implemented in the previous PR, applying the following changes: - Update the mapping to store unitary index results instead of storing the whole pattern with the results in each document. - Change the query to get the stored results by aggregating documents by indexName. The authorized indexNames derived from the `pattern` parameter are retrieved using the `indices.get` request. - A bug involving a race condition with the initialization and the retrieval of stored results, resulting in an unintended reset of the results in the UI, has been fixed. https://github.com/elastic/kibana/assets/17747913/0598606b-c5f4-42b3-901c-f86a3cac65e4 --------- Co-authored-by: Kibana Machine <[email protected]>
## Summary follow-up of elastic#173185 This PR enables the persistence layer implemented in the previous PR, applying the following changes: - Update the mapping to store unitary index results instead of storing the whole pattern with the results in each document. - Change the query to get the stored results by aggregating documents by indexName. The authorized indexNames derived from the `pattern` parameter are retrieved using the `indices.get` request. - A bug involving a race condition with the initialization and the retrieval of stored results, resulting in an unintended reset of the results in the UI, has been fixed. https://github.com/elastic/kibana/assets/17747913/0598606b-c5f4-42b3-901c-f86a3cac65e4 --------- Co-authored-by: Kibana Machine <[email protected]>
Summary
issue https://github.com/elastic/security-team/issues/7382
Data Stream Adapter
This PR introduces the
@kbn/data-stream-adapter
package, which is a utility library to facilitate Data Stream creation and maintenance in Kibana, it was inspired by the data stream implementation in the Alerts plugin.The library has two exports:
DataStreamSpacesAdapter
: to manage space data streams. It uses thename-of-the-data-stream-<spaceId>
naming pattern.DataStreamAdapter
: to manage single (not space-aware) data streams.Usage examples in the package README
Data Quality Dashboard
The
DataStreamSpacesAdapter
has been integrated into the data quality dashboard to store all the quality checks users perform. The information stored is the metadata (also used for telemetry) and the actual data rendered in the tables.FieldMap definition here
Demo
DQDashboard_persistance_demo.mov