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] Reduce permissions. #90302

Merged
merged 4 commits into from
Feb 11, 2021
Merged

[Fleet] Reduce permissions. #90302

merged 4 commits into from
Feb 11, 2021

Conversation

skh
Copy link
Contributor

@skh skh commented Feb 4, 2021

Summary

Partly fixes #89713

This change removes the backing indices from the privileges of the fleet_enroll rule.

It changes the privileges of the role from ['write', 'create_index', 'indices:admin/auto_create'] to ['auto_configure', 'create_doc'].

How to test this

Try to break it with your typical use of fleet. If you have use cases that are not covered by CI tests, please do test those.

Alternatively, look at the changes and comment when you think they are problematic.

@skh skh added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Fleet Fleet team's agent central management project Team:Fleet Team label for Observability Data Collection Fleet team v7.12.0 labels Feb 4, 2021
@skh skh self-assigned this Feb 4, 2021
@scunningham
Copy link

I am concerned these privileges will not work in the case where the index template exists for the data stream, but the data stream has not been created yet. When I attempted it here it failed. Have these permissions been validated in Kibana for this edge case?

@ruflin ruflin requested a review from scunningham February 5, 2021 14:01
@skh skh force-pushed the 89713-fewer-permissions branch from 06d8020 to 66688db Compare February 8, 2021 11:46
@skh
Copy link
Contributor Author

skh commented Feb 8, 2021

@scunningham I think I'm seeing the same as you.

How I'm testing:

Result:

  • Agent is enrolled and displayed as healthy (I'm not sure this is correct either)
  • No data streams are added
  • No data visible from Discover either
  • elasticsearch logs show lots of these:
info [o.e.x.i.IndexLifecycleTransition] [kovalevskaya] moving index [.ds-metrics-elastic_agent.metricbeat-default-2021.02.08-000001] from [{"phase":"hot","action":"unfollow","name":"wait-for-index-color"}] to [{"phase":"hot","action":"rollover","name":"check-rollover-ready"}] in policy [metrics]
  • authorization errors in beats logs like this:
{"log.level":"warn","@timestamp":"2021-02-08T15:57:17.705+0100","log.logger":"elasticsearch","log.origin":{"file.name":"elasticsearch/client.go","file.line":408},"message":"Cannot index event publisher.Event{Content:beat.Event{Timestamp:time.Time{wall:0xc0007233302c1a3a, ext:1150841823848, loc:(*time.Location)(0xeb12640)}, Meta:{\"raw_index\":\"metrics-system.socket_summary-default\"}, Fields:{\"agent\":{\"ephemeral_id\":\"8020b590-bd1b-4b01-a0db-8b1c8fe6f5cd\",\"id\":\"a158eb9a-2c6b-4f80-ad8e-1b8d559c895a\",\"name\":\"kovalevskaya\",\"type\":\"metricbeat\",\"version\":\"8.0.0\"},\"data_stream\":{\"dataset\":\"system.socket_summary\",\"namespace\":\"default\",\"type\":\"metrics\"},\"ecs\":{\"version\":\"1.7.0\"},\"elastic_agent\":{\"id\":\"c19d5660-6a1a-11eb-8ae7-35d9a2b7fe6b\",\"snapshot\":true,\"version\":\"8.0.0\"},\"event\":{\"dataset\":\"system.socket_summary\",\"duration\":78485356,\"module\":\"system\"},\"host\":{\"architecture\":\"x86_64\",\"containerized\":false,\"hostname\":\"kovalevskaya\",\"id\":\"a0772cf87cbc4e1190503c6c715d4134\",\"ip\":[\"192.168.178.42\",\"fd00::e3f7:f580:388d:4986\",\"fd00::382e:652e:1d3c:fc98\",\"2001:a61:2526:ae01:b73d:5178:cfb0:5bd\",\"2001:a61:2526:ae01:f6c3:f70f:f235:8228\",\"fe80::86cb:bb2d:756b:cfb9\",\"172.17.0.1\"],\"mac\":[\"68:54:5a:6d:70:a4\",\"02:42:5e:bd:8c:f0\"],\"name\":\"kovalevskaya\",\"os\":{\"codename\":\"groovy\",\"family\":\"debian\",\"kernel\":\"5.8.0-38-generic\",\"name\":\"Ubuntu\",\"platform\":\"ubuntu\",\"version\":\"20.10 (Groovy Gorilla)\"}},\"metricset\":{\"name\":\"socket_summary\",\"period\":10000},\"service\":{\"type\":\"system\"},\"system\":{\"socket\":{\"summary\":{\"all\":{\"count\":894,\"listening\":12},\"tcp\":{\"all\":{\"close_wait\":3,\"closing\":0,\"count\":876,\"established\":851,\"fin_wait1\":10,\"fin_wait2\":0,\"last_ack\":0,\"listening\":12,\"orphan\":10,\"syn_recv\":0,\"syn_sent\":0,\"time_wait\":0},\"memory\":77824},\"udp\":{\"all\":{\"count\":18},\"memory\":61440}}}}}, Private:interface {}(nil), TimeSeries:true}, Flags:0x0, Cache:publisher.EventCache{m:common.MapStr(nil)}} (status=403): {\"type\":\"security_exception\",\"reason\":\"action [indices:admin/mapping/auto_put] is unauthorized for API key id [EpoQgncBctTlKKOgypt9] of user [fleet_enroll] on indices [.ds-metrics-system.socket_summary-default-2021.02.08-000001], this action is granted by the index privileges [auto_configure,manage,write,all]\"}","ecs.version":"1.6.0"}

@ph @ruflin How should we proceed here? Wait for a conclusion in elastic/elasticsearch#68414 ?

@ph
Copy link
Contributor

ph commented Feb 9, 2021

@scunningham as you have noted these privileges aren't working for us?

@skh Let's do this, keep the permission we had before but use the pattern that you have in this PR, we have to see where the permission discussion from @scunningham is going, we can't do more than that.

@skh skh force-pushed the 89713-fewer-permissions branch from 66688db to ac190bf Compare February 9, 2021 15:20
@skh
Copy link
Contributor Author

skh commented Feb 9, 2021

keep the permission we had before but use the pattern that you have in this PR

ac190bf changes the privileges to what they were before. The backing indices are still removed.

@skh skh marked this pull request as ready for review February 9, 2021 15:23
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Feature:Fleet)

@skh skh requested review from nchaulet, simitt and kevinlog February 9, 2021 15:24
@skh
Copy link
Contributor Author

skh commented Feb 10, 2021

@elasticmachine merge upstream

1 similar comment
@skh
Copy link
Contributor Author

skh commented Feb 10, 2021

@elasticmachine merge upstream

Copy link
Contributor

@ph ph left a comment

Choose a reason for hiding this comment

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

The changes look good to me, @skh have you tested it with the Elastic Agent?

@skh
Copy link
Contributor Author

skh commented Feb 10, 2021

have you tested it with the Elastic Agent?

Yes, local installation on linux, from tar.gz, with default policy & system integration. I see data streams coming in.

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.

🚀 Tested locally and it works as expected

@skh skh force-pushed the 89713-fewer-permissions branch from a521d70 to e4f5feb Compare February 10, 2021 14:29
@skh
Copy link
Contributor Author

skh commented Feb 10, 2021

Privileges were changed to ['auto_configure', 'create_doc', 'indices:admin/auto_create'] after discussion with @scunningham

@ph
Copy link
Contributor

ph commented Feb 10, 2021

@EricDavisX FYI when @skh merges this we should keep an eye on the QA side of it for any errors concerning permission on ingestion.

@skh
Copy link
Contributor Author

skh commented Feb 10, 2021

Do we need both 'indices:admin/auto_create' and 'auto_configure'? I believe the latter implies the former.

Indeed it seems to, thanks for pointing that out.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@skh skh merged commit 9870ade into elastic:master Feb 11, 2021
skh added a commit to skh/kibana that referenced this pull request Feb 11, 2021
* Reduce permissions.

* Change permissions back.

* Reducing permissions on fleet_enroll role

- 'write', 'create_index' -> 'auto_configure', 'create_doc'

* Remove indices:admin/auto_create from privileges.
gmmorris added a commit to gmmorris/kibana that referenced this pull request Feb 11, 2021
* master: (44 commits)
  [APM] Add experimental support for Data Streams (elastic#89650)
  [Search Session] Control "Kibana / Search Sessions" management section by privileges (elastic#90818)
  [Lens] Median as default function (elastic#90952)
  Implement custom global header banner (elastic#87438)
  [Fleet] Reduce permissions. (elastic#90302)
  Update dependency @elastic/charts to v24.5.1 (elastic#89822)
  [Create index pattern] Can't create single character index without wildcard (elastic#90919)
  [ts/build_ts_refs] add support for --clean flag (elastic#91060)
  Don't clean when running e2e tests (elastic#91057)
  Fixes track_total_hits in the body not having an effect when using search strategy (elastic#91068)
  [Security Solution][Detections] Adds list plugin Saved Objects to Security feature privilege (elastic#90895)
  Removing the code plugin entirely for 8.0 (elastic#77940)
  chore(NA): move the instruction to remove yarn global bazelisk package into the first place on install bazel tools (elastic#91026)
  [jest/ci] remove max-old-space-size override to use 4gb default (elastic#91020)
  [Fleet] Restrict integration changes for managed policies (elastic#90675)
  [CI] Fix auto-backport condditions so that it doesn't trigger for other labels (elastic#91042)
  [DOCS] Uses variable to refer to query profiler (elastic#90976)
  [App Search] Relevance Tuning logic listeners (elastic#89461)
  [Metrics UI] Fix saving/loading saved views from URL (elastic#90216)
  Limit cardinality of transaction.name (elastic#90955)
  ...
skh added a commit that referenced this pull request Feb 11, 2021
* Reduce permissions.

* Change permissions back.

* Reducing permissions on fleet_enroll role

- 'write', 'create_index' -> 'auto_configure', 'create_doc'

* Remove indices:admin/auto_create from privileges.
Copy link

@scunningham scunningham left a comment

Choose a reason for hiding this comment

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

Looks good to me. I think this is the best we can do right now.

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Fleet Fleet team's agent central management project release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fleet should restrict the permissions on the keys.
7 participants