Skip to content

Allow population of Enrich indices to work with System Index protections #67406

Merged
AthenaEryma merged 13 commits intoelastic:masterfrom
AthenaEryma:si/multi-user-reindex
Jan 28, 2021
Merged

Allow population of Enrich indices to work with System Index protections #67406
AthenaEryma merged 13 commits intoelastic:masterfrom
AthenaEryma:si/multi-user-reindex

Conversation

@AthenaEryma
Copy link
Copy Markdown
Contributor

@AthenaEryma AthenaEryma commented Jan 13, 2021 •
edited
Loading

This PR does three things:

  1. Tweaks existing reindex infrastructure so that different clients can be used for the "search" part and the "index" part of a reindex operation, and
  2. Modifies Enrich to take advantage of this to perform the "search" part in the security context of the current user (so that DLS/FLS etc. are properly applied) while performing the "index" part in the security context of the Enrich plugin (so that access to system indices, and .enrich-* in particular, is allowed regardless of the permissions of the current user).
  3. Adds integration tests for the above, to verify that Enrich does not leak info protected by DLS and/or FLS.

Closes #62505

Co-authored-by: Jay Modi jay.modi@elastic.co

@AthenaEryma
Copy link
Copy Markdown
Contributor Author

AthenaEryma commented Jan 15, 2021

Note to myself: The remaining test failures are due to getting warnings from these uses of IndexNameExpressionResolver, which are called for validation in the context of the user's client:

elasticsearch/modules/reindex/src/main/java/org/elasticsearch/index/reindex/ReindexValidator.java

Lines 130 to 132 in f72b893

target = indexNameExpressionResolver.concreteWriteIndex(clusterState, destination).getName();
}
for (String sourceIndex : indexNameExpressionResolver.concreteIndexNames(clusterState, source)) {

We'll need to work around this, because those index name resolutions both being done in the same context will be problematic once we lock down system index access: we'll want to resolve the index names for the search side in the user's context, and the enrich indices in Enrich's context. I think the best way to do this is to pop the validation out into a protected method and override it in TransportEnrichReindexAction, which is going to involve duplicating some logic from ReindexValidator (or tweaking that class to expose its validations in such a way that we can hook in at the appropriate place to modify the context before resolving the enrich/destination index).

@AthenaEryma AthenaEryma marked this pull request as ready for review January 26, 2021 23:01
@AthenaEryma AthenaEryma added the :Distributed/Ingest Node Execution or management of Ingest Pipelines label Jan 26, 2021
@elasticmachine elasticmachine added the Team:Data Management (obsolete) DO NOT USE. This team no longer exists. label Jan 26, 2021
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Jan 26, 2021

Pinging @elastic/es-core-features (Team:Core/Features)

@AthenaEryma AthenaEryma removed the WIP label Jan 26, 2021
jaymode
jaymode approved these changes Jan 28, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

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

LGTM

@AthenaEryma AthenaEryma merged commit 4fe7a61 into elastic:master Jan 28, 2021
@AthenaEryma AthenaEryma deleted the si/multi-user-reindex branch January 28, 2021 17:17
@AthenaEryma AthenaEryma mentioned this pull request Jan 28, 2021
Merged
AthenaEryma added a commit to AthenaEryma/elasticsearch that referenced this pull request Jan 28, 2021
@AthenaEryma
Allow population of Enrich indices to work with System Index protecti…
3c5183c 
…ons (elastic#67406)

This PR does three things:
1) Tweaks existing reindex infrastructure so that different clients can be used for the "search" part and the "index" part of a reindex operation, and
2) Modifies Enrich to take advantage of this to perform the "search" part in the security context of the current user (so that DLS/FLS etc. are properly applied) while performing the "index" part in the security context of the Enrich plugin (so that access to system indices, and `.enrich-*` in particular, is allowed regardless of the permissions of the current user).
3) Adds integration tests for the above, to verify that Enrich does not leak info protected by DLS and/or FLS.

Co-authored-by: Jay Modi <jay.modi@elastic.co>
AthenaEryma added a commit that referenced this pull request Jan 28, 2021
@AthenaEryma
[7.x] Allow population of Enrich indices to work with System Index pr…
4cf0906 
…otections (#67406)

This PR does three things:
1) Tweaks existing reindex infrastructure so that different clients can be used for the "search" part and the "index" part of a reindex operation, and
2) Modifies Enrich to take advantage of this to perform the "search" part in the security context of the current user (so that DLS/FLS etc. are properly applied) while performing the "index" part in the security context of the Enrich plugin (so that access to system indices, and `.enrich-*` in particular, is allowed regardless of the permissions of the current user).
3) Adds integration tests for the above, to verify that Enrich does not leak info protected by DLS and/or FLS.

Co-authored-by: Jay Modi <jay.modi@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jaymode jaymode jaymode approved these changes

@williamrandolph williamrandolph Awaiting requested review from williamrandolph

Assignees

No one assigned

Labels

:Distributed/Ingest Node Execution or management of Ingest Pipelines >non-issue Team:Data Management (obsolete) DO NOT USE. This team no longer exists. v7.12.0 v8.0.0-alpha1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow population of Enrich indices to work with System Index protections

4 participants

@AthenaEryma @elasticmachine @jaymode @jakelandis