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

Add 'create_doc' index privilege #45806

Merged
merged 21 commits into from
Oct 7, 2019

Conversation

bizybot
Copy link
Contributor

@bizybot bizybot commented Aug 21, 2019

Use case:
User with create_doc index privilege will be allowed to only index new documents
either via Index API or Bulk API.

There are two cases that we need to think:

  • User indexing a new document without specifying an Id.
    For this ES auto generates an Id and now ES version 7.5.0 onwards defaults to op_type create we just need to authorize on the op_type.
  • User indexing a new document with an Id.
    This is problematic as we do not know whether a document with Id exists or not.
    If the op_type is create then we can assume the user is trying to add a document, if it exists it is going to throw an error from the index engine.

Given these both cases, we can safely authorize based on the op_type value. If the value is create then the user with create_doc privilege is authorized to index new documents.

In the AuthorizationService when authorizing a bulk request, we check the implied action.
This code changes that to append the :op_type/index or :op_type/create
to indicate the implied index action.

`append_only` index privilege allows users to index new documents
but not update existing documents.
Wherever the op-type is `index` and `_id` is specified for the
document to be indexed, we would deny access even if the document
does not exist. We do not know in authz service whether this document
exists or not.
@bizybot bizybot added discuss :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC labels Aug 21, 2019
@bizybot bizybot requested a review from tvernum August 21, 2019 16:22
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

@jpountz
Copy link
Contributor

jpountz commented Sep 6, 2019

@bizybot Is there anything in particular that you wanted to be discussed with the rest of the team?

Some of us got curious about the limitation you mentioned, do we understand it correctly that index requests with an id (PUT {index}/_doc/{id}) will be rejected unless op_type is set to create?

@bizybot
Copy link
Contributor Author

bizybot commented Sep 7, 2019

@bizybot Is there anything in particular that you wanted to be discussed with the rest of the team?

I put this as discuss label as we were discussing with the other teams like Beats, Logstash, APM to notify the behavior of the privilege we are building.

Some of us got curious about the limitation you mentioned, do we understand it correctly that index requests with an id (PUT {index}/_doc/{id}) will be rejected unless op_type is set to create?

Yes, the teams understanding is correct. I wanted to make sure that the other teams (Beats/Logstash) know about this behavior when using this privilege. Beats informed that they do explicitly set op_type create when using id so it seems that is already accounted for.

Thank you for your feedback.

@bizybot bizybot removed the discuss label Sep 9, 2019
@bizybot bizybot changed the title [Draft] Append only index privilege [Draft] 'create_docs' index privilege Sep 23, 2019
@jasontedor
Copy link
Member

Sorry, I have to ask though, why is it create_docs and not create_doc like create_index, etc.? As far as I know, our privileges like this are singular, and it makes sense that way. The privilege authorizes me to create a single doc, and it's only APIs that we have that allow users to create more than one doc in a single go. So I'm authorized to create a doc, one doc at a time. 🤷‍♀

@bizybot
Copy link
Contributor Author

bizybot commented Sep 25, 2019

Sorry, I have to ask though, why is it create_docs and not create_doc like create_index, etc.? As far as I know, our privileges like this are singular, and it makes sense that way. The privilege authorizes me to create a single doc, and it's only APIs that we have that allow users to create more than one doc in a single go. So I'm authorized to create a doc, one doc at a time. 🤷‍♀

Yes, makes sense, I was focused more on the action. I will update it. Thank you.

@bizybot bizybot changed the title [Draft] 'create_docs' index privilege [Draft] 'create_doc' index privilege Sep 25, 2019
@bizybot bizybot marked this pull request as ready for review October 5, 2019 11:26
@bizybot bizybot requested a review from jkakavas October 5, 2019 11:30
@bizybot
Copy link
Contributor Author

bizybot commented Oct 5, 2019

@elasticmachine run elasticsearch-ci/bwc

1 similar comment
@bizybot
Copy link
Contributor Author

bizybot commented Oct 6, 2019

@elasticmachine run elasticsearch-ci/bwc

Copy link
Contributor

@tvernum tvernum left a comment

Choose a reason for hiding this comment

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

I left 2 suggestions (but 1 is a nit)

@bizybot bizybot requested a review from tvernum October 7, 2019 05:19
@bizybot
Copy link
Contributor Author

bizybot commented Oct 7, 2019

@elasticmachine run elasticsearch-ci/packaging-sample

@bizybot bizybot merged commit 924b298 into elastic:master Oct 7, 2019
bizybot added a commit to bizybot/elasticsearch that referenced this pull request Oct 7, 2019
Use case:
User with `create_doc` index privilege will be allowed to only index new documents
either via Index API or Bulk API.

There are two cases that we need to think:
- **User indexing a new document without specifying an Id.**
   For this ES auto generates an Id and now ES version 7.5.0 onwards defaults to `op_type` `create` we just need to authorize on the `op_type`.
- **User indexing a new document with an Id.**
   This is problematic as we do not know whether a document with Id exists or not.
   If the `op_type` is `create` then we can assume the user is trying to add a document, if it exists it is going to throw an error from the index engine.

Given these both cases, we can safely authorize based on the `op_type` value. If the value is `create` then the user with `create_doc` privilege is authorized to index new documents.

In the `AuthorizationService` when authorizing a bulk request, we check the implied action.
This code changes that to append the `:op_type/index` or `:op_type/create`
to indicate the implied index action.
bizybot added a commit that referenced this pull request Oct 7, 2019
Use case:
User with `create_doc` index privilege will be allowed to only index new documents
either via Index API or Bulk API.

There are two cases that we need to think:
- **User indexing a new document without specifying an Id.**
   For this ES auto generates an Id and now ES version 7.5.0 onwards defaults to `op_type` `create` we just need to authorize on the `op_type`.
- **User indexing a new document with an Id.**
   This is problematic as we do not know whether a document with Id exists or not.
   If the `op_type` is `create` then we can assume the user is trying to add a document, if it exists it is going to throw an error from the index engine.

Given these both cases, we can safely authorize based on the `op_type` value. If the value is `create` then the user with `create_doc` privilege is authorized to index new documents.

In the `AuthorizationService` when authorizing a bulk request, we check the implied action.
This code changes that to append the `:op_type/index` or `:op_type/create`
to indicate the implied index action.
@bizybot bizybot mentioned this pull request Oct 7, 2019
bizybot pushed a commit that referenced this pull request Oct 9, 2019
This commit adds documentation for new index privilege
create_doc which only allows indexing of new documents
but no updates to existing documents via Index or Bulk APIs.

Relates: #45806
bizybot pushed a commit to bizybot/elasticsearch that referenced this pull request Oct 9, 2019
This commit adds documentation for new index privilege
create_doc which only allows indexing of new documents
but no updates to existing documents via Index or Bulk APIs.

Relates: elastic#45806
bizybot added a commit that referenced this pull request Oct 9, 2019
This commit adds documentation for new index privilege
create_doc which only allows indexing of new documents
but no updates to existing documents via Index or Bulk APIs.

Relates: #45806
cwurm pushed a commit to elastic/beats that referenced this pull request Oct 14, 2019
Updates the writer role documentation based on #13847 and #13848. Also corrects some mistakes.
1. Changes `read from` to the correct `write to` (Beats does not read from indices).
2. Setting `setup.template.enabled` to `false` is no longer necessary after #13847.
3. Setting `setup.ilm.overwrite` to `false` is unnecessary if `setup.ilm.check_exists` is already `false` (even today).
4. Adds a note about only `monitor` and `create_doc` being always necessary, explicitly calling out the most secure configuration (following #13847 and #13848).
5. Correct what `monitor` is for: It's for checking things like cluster version and license, not "sending monitor info".
6. Replaces `manage_pipeline` with the read-only `cluster:admin/ingest/pipeline/get`. Unfortunately, there is no read-only cluster role for pipelines, so it requires this privilege. But better than the very permissive `manage_pipeline` that allows changing any pipeline.
7. Changes `index` to the more restrictive, append-only `create_doc` (introduced in elastic/elasticsearch#45806).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants