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

Do not rewrite aliases on remove-index from aliases requests #46989

Conversation

jasontedor
Copy link
Member

@jasontedor jasontedor commented Sep 23, 2019

When we rewrite alias requests, after filtering down to only those that the user is authorized to see, it can be that there are no aliases remaining in the request. However, core Elasticsearch interprets this as _all so the user would see more than they are authorized for. To address this, we previously rewrote all such requests to have aliases "*", "-*", which would be interpreted when aliases are resolved as nome. Yet, this is only needed for get aliases requests and we were applying it to all alias requests, including remove index requests. If such a request was sent to a coordinating node that is not the master node, the request would be rewritten to include "*" and "-*", and then the master would authorize the user for these. If the user had limited permissions, the request would fail, even if they were authorized on the index that the remove index action was over. This commit addresses this by only rewriting for get aliases and remove aliases request types but not for the remove index.

Co-authored-by: Albert Zaharovits <albert.zaharovits@elastic.co>
Co-authored-by: Tim Vernum <tim@adjective.org>

When we rewrite alias requests, if there are no aliases remaining in the
request after filtering down to only those that the user is authorized
to see, it can be that there are no aliases remaining in the
request. However, core Elasticsearch interprets this as _all so the user
would see more than they are authorized for. To address this, we
previously rewrote all such requests to have aliases "*", "-*", which
would be interpreted when aliases are resolved as no aliases. Yet, this
is only needed for get aliases requests and we were applying it to all
alias requests, including remove index requests. If such a request was
sent to a coordinating node that is not the master node, the request
would be rewritten to include "*" and "-*", and then the master would
authorize the user for these. If the user had limited permissions, the
request would fail, even if they were authorized on the index that the
remove index action was over. This commit addresses this by only
rewriting for get aliases requests.

Co-authored-by: Albert Zaharovits <albert.zaharovits@elastic.co>
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

Copy link
Contributor

@albertzaharovits albertzaharovits left a comment

Choose a reason for hiding this comment

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

LGTM.

@albertzaharovits
Copy link
Contributor

14:29:04 Fetching upstream changes from git@github.com:elastic/elasticsearch.git
14:29:04 using GIT_SSH to set credentials GitHub user @elasticmachine SSH key
14:29:04  > git fetch --tags --progress git@github.com:elastic/elasticsearch.git +refs/pull/46989/*:refs/remotes/origin/pr/46989/*
14:29:09 ERROR: Error fetching remote repo 'origin'
14:29:09 hudson.plugins.git.GitException: Failed to fetch from git@github.com:elastic/elasticsearch.git
14:29:09 	at hudson.plugins.git.GitSCM.fetchFrom(GitSCM.java:894)
14:29:09 	at hudson.plugins.git.GitSCM.retrieveChanges(GitSCM.java:1161)
14:29:09 	at hudson.plugins.git.GitSCM.checkout(GitSCM.java:1192)
14:29:09 	at hudson.scm.SCM.checkout(SCM.java:504)
14:29:09 	at hudson.model.AbstractProject.checkout(AbstractProject.java:1208)
14:29:09 	at hudson.model.AbstractBuild$AbstractBuildExecution.defaultCheckout(AbstractBuild.java:574)
14:29:09 	at jenkins.scm.SCMCheckoutStrategy.checkout(SCMCheckoutStrategy.java:86)
14:29:09 	at hudson.model.AbstractBuild$AbstractBuildExecution.run(AbstractBuild.java:499)
14:29:09 	at hudson.model.Run.execute(Run.java:1815)
14:29:09 	at hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:43)
14:29:09 	at hudson.model.ResourceController.execute(ResourceController.java:97)
14:29:09 	at hudson.model.Executor.run(Executor.java:429)
14:29:09 Caused by: hudson.plugins.git.GitException: Command "git fetch --tags --progress git@github.com:elastic/elasticsearch.git +refs/pull/46989

@elasticmachine test this please

@albertzaharovits albertzaharovits changed the title Only rewrite aliases on get alias requests Do not rewrite aliases on remove-index from aliases requests Sep 24, 2019
@albertzaharovits
Copy link
Contributor

@elasticmachine test this please

@jasontedor
Copy link
Member Author

@elasticmachine update branch

@jasontedor
Copy link
Member Author

@elasticmachine test this please

@jasontedor jasontedor merged commit 66b0346 into elastic:master Sep 24, 2019
albertzaharovits added a commit to albertzaharovits/elasticsearch that referenced this pull request Sep 24, 2019
…#46989)

When we rewrite alias requests, after filtering down to only those that
the user is authorized to see, it can be that there are no aliases
remaining in the request. However, core Elasticsearch interprets this as
_all so the user would see more than they are authorized for. To address
this, we previously rewrote all such requests to have aliases `"*"`,
`"-*"`, which would be interpreted when aliases are resolved as
nome. Yet, this is only needed for get aliases requests and we were
applying it to all alias requests, including remove index requests. If
such a request was sent to a coordinating node that is not the master
node, the request would be rewritten to include `"*"` and `"-*"`, and
then the master would authorize the user for these. If the user had
limited permissions, the request would fail, even if they were
authorized on the index that the remove index action was over. This
commit addresses this by ~only~ rewriting for get aliases and remove
aliases request types but not for the remove index.

Co-authored-by: Albert Zaharovits <albert.zaharovits@elastic.co>
Co-authored-by: Tim Vernum <tim@adjective.org>
albertzaharovits added a commit that referenced this pull request Sep 24, 2019
…#47020)

When we rewrite alias requests, after filtering down to only those that
the user is authorized to see, it can be that there are no aliases
remaining in the request. However, core Elasticsearch interprets this as
_all so the user would see more than they are authorized for. To address
this, we previously rewrote all such requests to have aliases `"*"`,
`"-*"`, which would be interpreted when aliases are resolved as
nome. Yet, this is only needed for get aliases requests and we were
applying it to all alias requests, including remove index requests. If
such a request was sent to a coordinating node that is not the master
node, the request would be rewritten to include `"*"` and `"-*"`, and
then the master would authorize the user for these. If the user had
limited permissions, the request would fail, even if they were
authorized on the index that the remove index action was over. This
commit addresses this by rewriting for get aliases and remove
aliases request types but not for the remove index.

Co-authored-by: Albert Zaharovits <albert.zaharovits@elastic.co>
Co-authored-by: Tim Vernum <tim@adjective.org>
albertzaharovits added a commit that referenced this pull request Sep 24, 2019
…#47018)

When we rewrite alias requests, after filtering down to only those that
the user is authorized to see, it can be that there are no aliases
remaining in the request. However, core Elasticsearch interprets this as
_all so the user would see more than they are authorized for. To address
this, we previously rewrote all such requests to have aliases `"*"`,
`"-*"`, which would be interpreted when aliases are resolved as
nome. Yet, this is only needed for get aliases requests and we were
applying it to all alias requests, including remove index requests. If
such a request was sent to a coordinating node that is not the master
node, the request would be rewritten to include `"*"` and `"-*"`, and
then the master would authorize the user for these. If the user had
limited permissions, the request would fail, even if they were
authorized on the index that the remove index action was over. This
commit addresses this by rewriting for get aliases and remove
aliases request types but not for the remove index.

Co-authored-by: Albert Zaharovits <albert.zaharovits@elastic.co>
Co-authored-by: Tim Vernum <tim@adjective.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants