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

[DOCS] Clarifies API key breaking change #54522

Merged
merged 5 commits into from
Apr 1, 2020
Merged

Conversation

lcawl
Copy link
Contributor

@lcawl lcawl commented Mar 31, 2020

@lcawl lcawl added >docs General docs changes :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v7.6.3 labels Mar 31, 2020
@elasticmachine
Copy link
Collaborator

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

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-docs (>docs)

When you make a request to create API keys, you can specify an expiration and
permissions for the API key. Previously, if you used an API key to create
another API key (sometimes called a _derived key_), by default it had no
privileges. It could nonetheless perform some operations such as running
Copy link
Contributor Author

Choose a reason for hiding this comment

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

by default it had no privileges...

The documentation for the create API key API (https://www.elastic.co/guide/en/elasticsearch/reference/master/security-api-create-api-key.html) says "When [role_descriptors] is not specified or is an empty array, then the API key will have a point in time snapshot of permissions of the authenticated user. If you supply role descriptors then the resultant permissions would be an intersection of API keys permissions and authenticated user’s permissions thereby limiting the access scope for API keys...". Is that still correct? Or does it need a clarification that this behaviour differs when you're creating a derived key?

Copy link
Member

Choose a reason for hiding this comment

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

This is still correct for "non-derived" keys. For any derived keys, the role descriptors must be specified and its member must be an empty descriptor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and its member must be an empty descriptor

Sorry, but I don't understand this qualifier. Could you please clarify?

Copy link
Member

Choose a reason for hiding this comment

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

The basically means the example you put into the breaking change section:

"role_descriptors": {
    "no_privileges": {}
}

The internal "no_privileges" is an empty member of the outer "role_descriptors". It cannot be skipped.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I'll create a separate PR to augment the description in the create API key API reference

Choose a reason for hiding this comment

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

@ywangd what is the point of having a derived key with no privilege? I mean is there any way to have a admin-key that can be used to create other derived keys with limited but non-empty privileges. Currently I can create only empty privilege derived key and could not found a way to associate roles with it?

Copy link
Member

Choose a reason for hiding this comment

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

@baturozgur Previously we had a bug that derived keys are always created with no privileges regardless of what you specify in the role descriptors at creation time. We'd like to support derived keys properly and have an issue #52244 to track it. But we are still working out details on how we should approach the implementation. In the mean time, we decided to fix the bug temporarily by explicitly require no privilege should be specified at creation time for derived keys. This avoids the confusion that our users used to have.

In summary, currently there is no way to create derived keys with non-empty privileges. This has been the case since day 1 of the API key feature. The fix did not change this behaviour. It only tightens what you can specify in the input to avoid the false impression that you could create derived keys with non-empty privileges.

Derived keys can still be used for the authentication API, i.e. GET /_security/_authenticate. I understand it might not be something you would be interested. But a 3rd party application could leverage it as an authentication mechanism but manages privileges on its own.

@lcawl lcawl marked this pull request as ready for review March 31, 2020 17:36
@lcawl lcawl requested a review from ywangd March 31, 2020 17:36
Copy link
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.

I left a few comments for your to consider. Thanks


When you make a request to create API keys, you can specify an expiration and
permissions for the API key. Previously, if you used an API key to create
another API key (sometimes called a _derived key_), by default it had no
Copy link
Member

Choose a reason for hiding this comment

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

Should we be more explicity about the previous behaviour? that is the derived key had no privilege regardless of the role descriptors specified at creation time

API keys is impacted by the fix for
https://www.elastic.co/community/security[CVE-2020-7009].

When you make a request to create API keys, you can specify an expiration and
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible or necessary to emphasize in the beginning that the change only impact derived keys? Since most users are probably not using them and they can skip the details.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, done!

=== Breaking changes

Authorization::
* Creation of child API keys (keys created by existing keys) now requires explicit "no privileges" configuration {pull}53647[#53647], https://www.elastic.co/community/security[CVE-2020-7009]
Copy link
Member

Choose a reason for hiding this comment

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

It's called "child" keys here. Probably better to use a consistent name. I have no preference over either "derived" or "child" keys.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, thanks!

=== Security changes

[discrete]
==== {es} authentication API key privileges
Copy link
Member

Choose a reason for hiding this comment

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

The word "authentication" does not seem to be necessary here. Also privileges are in the scope of authorization.

the {ref}/security-api-create-api-key.html[create API key API].

As of 7.6.2, this behavior changes. To create derived keys with no privileges,
you must explicitly specify an empty object in the `role_descriptors` array.
Copy link
Member

Choose a reason for hiding this comment

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

The role_descriptors is technically an object not an array.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I'll fix that in the API ref in a separate PR too. For here, I've changed the sentence to "...you must explicitly specify an empty role descriptor".

Copy link
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.

Other than "role_descriptors" should be an object, rest LGTM.

@lcawl lcawl merged commit d02fded into elastic:7.6 Apr 1, 2020
@lcawl lcawl deleted the 762-security branch April 1, 2020 14:34
lcawl added a commit that referenced this pull request Apr 1, 2020
@lcawl lcawl added the v7.7.0 label Apr 1, 2020
lcawl added a commit that referenced this pull request Apr 1, 2020
@lcawl lcawl added the v7.8.0 label Apr 1, 2020
tvernum added a commit to tvernum/elasticsearch that referenced this pull request Sep 15, 2020
This updates the Create API Key reference document with
information about the limitations of derived API keys.

Since ES v7.6.0, API keys that are created from an API key (what we
refer to as "derived API keys" must be created with an empty
privileges list (to explicitly match the effective behaviour of all
earlier versions).

This information was included in the release notes, but didn't get
added to the API reference.

Relates: elastic#53647, elastic#54522, elastic#60154
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>docs General docs changes :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v7.6.3 v7.7.0 v7.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants