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

[Fleet] Allow packages to specify index privileges #112397

Merged

Conversation

hop-dev
Copy link
Contributor

@hop-dev hop-dev commented Sep 16, 2021

Summary

Closes #109047

This PR will not be ready to merge until we have tested it end to end.

Allow packages to specify a restricted set of index privileges as part of their manifest. There was previously some stub code which handled permissions however this was never fully implemented, @axw proposed moving these to elasticsearch.privileges.indices.

if a package has elasticsearch.privileges.indices in their data stream definition then:

  • remove any invalid or forbidden privileges e.g delete is not allowed
  • add these privileges to the package policy saved object
  • read these fields from the package policy saved object when generating the agent policy

Testing Steps

First I had to modify a package to specify index privileges in its manifest, for testing i used the custom logs package, these steps won't be necessary once elastic/apm-server#6139 is merged.

  • in the integrations repo edit packages/log/data_stream/log/manifest.yml to include the following at the top level:
elasticsearch:
  privileges:
    indices: [auto_configure, create_doc, maintenance, monitor, read]
  • in the package directory then run elastic-package build
  • run elastic-package stack up -v -d --services package-registry to start the package registry in docker locally
  • in kibana.dev.yml set xpack.fleet.registryUrl: http://localhost:8080

We can now test if these privileges are added to the API key when I install the integration (on mac):

  • add the custom logs package to a policy
  • in dev tools run GET /.fleet-agents/_search
  • find an agent that is part of your test policy and copy the default_api_key
  • in a terminal run echo -n "<your token>" | base64
  • copy that output into the following curl command, note you may need to change the index pattern depending on the test data stream you have used:
curl "http://localhost:9200/_security/user/_has_privileges" \
-H "Connection: keep-alive" \
-H "Content-Type: application/json" \
-H "Authorization: ApiKey <your base64 encoded token>" \
-d '{ "index" : [ { "names": [ "logs-generic-default" ] , "privileges": ["auto_configure", "create_doc", "maintenance", "monitor", "read"] } ] }'
  • the response should contain "has_all_requested":true

Checklist

Delete any items that are not applicable to this PR.

@hop-dev hop-dev added v8.0.0 Team:Fleet Team label for Observability Data Collection Fleet team auto-backport Deprecated - use backport:version if exact versions are needed v7.16.0 labels Sep 16, 2021
@hop-dev hop-dev self-assigned this Sep 16, 2021
@joshdover
Copy link
Contributor

Testing

I haven't found a good way of testing this in fleet yet, I can see I am adding the correct privileges to the agent policy but I am struggling to verify that the key is then created with the correct privileges:

  • I dont believe I can use the has_privileges API for an API key
  • I cant see how to retrieve the API key to use it manually in a curl request to check its privileges (the keys are created by fleet-server)

I believe you should be able to use the _has_privileges API, at least I was able to with a key I created myself. That said, I don't think you'll be able to retrieve the API key since it is created by Fleet Server as you mentioned.

I think the best course of action is to explore adding a test to the e2e-testing repo once the changes are in for the APM package as well. However, I'm not sure we have examples that use apm-server and test ingesting APM traces, so we would need to add that.

@hop-dev hop-dev marked this pull request as ready for review September 16, 2021 14:28
@hop-dev hop-dev requested a review from a team as a code owner September 16, 2021 14:28
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@hop-dev
Copy link
Contributor Author

hop-dev commented Sep 16, 2021

Moving out of draft as I have been able to test e2e, I've updated the test steps in the description.

@nchaulet nchaulet self-requested a review September 16, 2021 15:58
@@ -25,6 +25,11 @@ export interface NewPackagePolicyInputStream {
data_stream: {
dataset: string;
type: string;
elasticsearch?: {
Copy link
Member

Choose a reason for hiding this comment

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

I am wondering if we should update the saved object mapping here https://github.com/elastic/kibana/blob/master/x-pack/plugins/fleet/server/saved_objects/index.ts/#L251-L253 (as input is not enabled it probably change nothing)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @nchaulet I've added it now. Privileges may include cluster privileges in the future as well, I've set it as flattened as I dont think we would ever need to search on individual privileges?

@axw
Copy link
Member

axw commented Sep 20, 2021

I think the best course of action is to explore adding a test to the e2e-testing repo once the changes are in for the APM package as well. However, I'm not sure we have examples that use apm-server and test ingesting APM traces, so we would need to add that.

FWIW, I intend to add a functional test for this in the apm-server repo. We have tests which run apm-server along with Elasticsearch, Kibana, and fleet-server already (e.g. see https://github.com/elastic/apm-server/blob/master/systemtest/fleet_test.go).

Of course it would be great to have functional testing for Fleet specifically too, but it could perhaps be deferred.

@hop-dev
Copy link
Contributor Author

hop-dev commented Sep 20, 2021

@elasticmachine merge upstream

Copy link
Member

@nchaulet nchaulet left a comment

Choose a reason for hiding this comment

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

🚀

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

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
fleet 1075 1076 +1

Page load bundle

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

id before after diff
fleet 130.1KB 130.2KB +163.0B

Saved Objects .kibana field count

Every field in each saved object type adds overhead to Elasticsearch. Kibana needs to keep the total field count below Elasticsearch's default limit of 1000 fields. Only specify field mappings for the fields you wish to search on or query. See https://www.elastic.co/guide/en/kibana/master/development-plugin-saved-objects.html#_mappings

id before after diff
ingest-package-policies 33 35 +2
Unknown metric groups

API count

id before after diff
fleet 1176 1177 +1

History

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

cc @hop-dev

@hop-dev hop-dev merged commit 7bcef12 into elastic:master Sep 20, 2021
@hop-dev hop-dev deleted the feature-109047-override-index-privileges branch September 20, 2021 12:40
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Sep 20, 2021
* tidy: move default index privs to const

* add index privileges to package policy SO schema

* add default index privileges const

* add privileges to epm schema

* add privileges to input stream types

* use new const for default index privileges

* permissions being added to policy

* fix unit tests

* add note about export

* tidy: move default index privs to const

* add index privileges to package policy SO schema

* add default index privileges const

* add privileges to epm schema

* add privileges to input stream types

* use new const for default index privileges

* permissions being added to policy

* fix unit tests

* add note about export

* remove outdated tests

* return enabled check to start of function

* add privileges to SO mapping

Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Sep 20, 2021
* tidy: move default index privs to const

* add index privileges to package policy SO schema

* add default index privileges const

* add privileges to epm schema

* add privileges to input stream types

* use new const for default index privileges

* permissions being added to policy

* fix unit tests

* add note about export

* tidy: move default index privs to const

* add index privileges to package policy SO schema

* add default index privileges const

* add privileges to epm schema

* add privileges to input stream types

* use new const for default index privileges

* permissions being added to policy

* fix unit tests

* add note about export

* remove outdated tests

* return enabled check to start of function

* add privileges to SO mapping

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Mark Hopkin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:enhancement Team:Fleet Team label for Observability Data Collection Fleet team v7.16.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fleet] Add support for Elasticsearch privileges in data stream manifest
6 participants