Skip to content

Extend fleet-server service account privileges#82600

Merged
axw merged 4 commits intoelastic:masterfrom
axw:fleetserver-apmsampledtraces
Jan 24, 2022
Merged

Extend fleet-server service account privileges#82600
axw merged 4 commits intoelastic:masterfrom
axw:fleetserver-apmsampledtraces

Conversation

@axw
Copy link
Copy Markdown
Member

@axw axw commented Jan 14, 2022

Allow elastic/fleet-server service account to additionally read, monitor, and refresh traces-apm.sampled-* data streams.

These data streams do not contain any sensitive information. Fleet-server itself does not need to perform these actions, but it creates API Keys for APM Server, which does need to.

Fixes elastic/fleet-server#1048

@elasticsearchmachine elasticsearchmachine added the external-contributor Pull request authored by a developer outside the Elasticsearch team label Jan 14, 2022
@axw
Extend fleet-server service account privileges
21b0203 
Allow elastic/fleet-server service account to
additionally read, monitor, and refresh
traces-apm.sampled-* data streams.

These data streams do not contain any sensitive
information. Fleet-server itself does not need
to perform these actions, but it creates API
Keys for APM Server, which does need to.
@axw axw force-pushed the fleetserver-apmsampledtraces branch from df6d012 to 21b0203 Compare January 14, 2022 08:06
@axw axw added the :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC label Jan 14, 2022
@axw axw marked this pull request as ready for review January 14, 2022 10:35
@elasticmachine elasticmachine added the Team:Security Meta label for security team label Jan 14, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Jan 14, 2022

Pinging @elastic/es-security (Team:Security)

@axw axw added >bug and removed >enhancement labels Jan 14, 2022
@axw
Copy link
Copy Markdown
Member Author

axw commented Jan 14, 2022

Not really sure if this should be classified as a bug or not. It's expected that fleet-server should be able to issue API Keys to APM Server that allow it to read/monitor/manage traces-apm.sampled-*. It can't though, because the service account doesn't include the privileges.

@axw axw mentioned this pull request Jan 14, 2022
Closed
@ruflin
Copy link
Copy Markdown
Contributor

ruflin commented Jan 14, 2022

@axw I remember someone mentioned that this feature would land in 8.1? Is this already shipped?

ruflin
ruflin approved these changes Jan 19, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

This change LGTM. But I would like to also get @ph and @joshdover review on this before merging.

@mtojek @jsoriano FYI: The spec around what permissions would be allowed for package-spec get extended here.

@ruflin ruflin requested review from joshdover and ph January 19, 2022 09:02
@jsoriano
Copy link
Copy Markdown
Member

jsoriano commented Jan 19, 2022 •
edited
Loading

@mtojek @jsoriano FYI: The spec around what permissions would be allowed for package-spec get extended here.

I think that there is no limitation in the package-spec of the permissions that can be used 🤔

UPDATE: This is something requested, not done yet: elastic/package-spec#255

ywangd
ywangd approved these changes Jan 20, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

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

LGTM

@axw @ywangd
Update x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/s…
e4fe170 
…ecurity/authc/service/ElasticServiceAccounts.java

Co-authored-by: Yang Wang <ywangd@gmail.com>
axw
axw commented Jan 20, 2022
View reviewed changes
Copy link
Copy Markdown
Member Author

@axw axw left a comment

Choose a reason for hiding this comment

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

Update docs and tests to match privileges change

@axw
Copy link
Copy Markdown
Member Author

axw commented Jan 20, 2022

@elasticmachine update branch

@ywangd
Copy link
Copy Markdown
Member

ywangd commented Jan 20, 2022

Failure is genuine but not related to this PR. I raised #82840

@ywangd
Copy link
Copy Markdown
Member

ywangd commented Jan 20, 2022

@elasticmachine run elasticsearch-ci/part-2

@jsoriano jsoriano mentioned this pull request Jan 20, 2022
Open
@axw
Copy link
Copy Markdown
Member Author

axw commented Jan 24, 2022

In the interest of getting the issue in apm-server fixed, I'm going to merge this.

@ph @joshdover if you have concerns, please do still leave a review and I'll follow up.

@axw axw removed the v8.0.0 label Jan 24, 2022
@axw axw merged commit ef1f7a5 into elastic:master Jan 24, 2022
@axw axw deleted the fleetserver-apmsampledtraces branch January 24, 2022 02:21
@joshdover
Copy link
Copy Markdown
Contributor

joshdover commented Jan 24, 2022

Apologize, I meant to approve this PR before 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ruflin ruflin ruflin approved these changes

@ywangd ywangd ywangd approved these changes

@ph ph Awaiting requested review from ph

@joshdover joshdover Awaiting requested review from joshdover

Assignees

No one assigned

Labels

>bug external-contributor Pull request authored by a developer outside the Elasticsearch team :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC Team:Security Meta label for security team v8.1.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Service tokens ignore privileges additionally defined by packages

7 participants

@axw @elasticmachine @ruflin @jsoriano @ywangd @joshdover @elasticsearchmachine