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.

Sign up for GitHub

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

[DOCS] Clarifies API key breaking change #54522

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7d7a1c4
[DOCS] Clarify API key breaking change
lcawl Mar 31, 2020
fb195a1
[DOCS] More edits
lcawl Mar 31, 2020
b1387ac
[DOCS] Skips snippet test
lcawl Mar 31, 2020
ce0a26f
[DOCS] Addresses feedback
lcawl Apr 1, 2020
12b34e9
[DOCS] Clarify role descriptor object
lcawl Apr 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletion docs/reference/migration/migrate_7_6.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,44 @@ See also <<release-highlights>> and <<es-release-notes>>.
//Installation and Upgrade Guide

//tag::notable-breaking-changes[]
[discrete]
[[breaking_76_security_changes]]
=== 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.


If you use the {es} API key service to generate API keys, the behavior of new
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!

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

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

Copy link

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.

the {ref}/security-api-authenticate.html[authenticate API]. This is no longer
the default behavior.

To create derived keys with no privileges, specify an empty object in the
`role_descriptors` array when you run the
{ref}/security-api-create-api-key.html[create API key API]. For example:

[source,js]
----
...
"role_descriptors": {
"no-privilege": {
}
}
...
----
// NOTCONSOLE

//end::notable-breaking-changes[]

[discrete]
[[breaking_76_search_changes]]
=== Search Changes
=== Search changes

[discrete]
==== Deprecation of sparse vector fields
Expand Down
10 changes: 7 additions & 3 deletions docs/reference/release-notes/7.6.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,13 @@

Also see <<breaking-changes-7.6,Breaking changes in 7.6>>.

[[breaking-7.6.2]]
[float]
=== 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!


[[bug-7.6.2]]
[float]
=== Bug fixes
Expand All @@ -13,9 +20,6 @@ Allocation::
Authentication::
* Fix potential bug in concurrent token refresh support {pull}53668[#53668]

Authorization::
* Explicitly require that delegate API keys have no privileges {pull}53647[#53647]

CCR::
* Handle no such remote cluster exception in ccr {pull}53415[#53415] (issue: {issue}53225[#53225])

Expand Down