Permission for restricted indices #37577

albertzaharovits · 2019-01-17T16:58:09Z

Supersedes #36765 .

This grants the capability to grant privileges over certain restricted indices (.security and .security-6 at the moment) . It also removes the special status of the superuser role.

IndicesPermission.Group is extended by adding the allow_restricted_indices boolean flag. By default the flag is false. When it is toggled, you acknowledge that the indices under the scope of the permission group can cover the restricted indices as well. Otherwise, by default, restricted indices are ignored when granting privileges, thus rendering them hidden for authorization purposes.
This effectively adds a confirmation "check-box" for roles that might grant privileges to restricted indices.

The "special status" of the superuser role has been removed and coded as any other role:

new RoleDescriptor("superuser",
    new String[] { "all" },
    new RoleDescriptor.IndicesPrivileges[] {
        RoleDescriptor.IndicesPrivileges.builder()
            .indices("*")
            .privileges("all")
            .allowRestrictedIndices(true)
// this ----^
            .build() },
            new RoleDescriptor.ApplicationResourcePrivileges[] {
                RoleDescriptor.ApplicationResourcePrivileges.builder()
                    .application("*")
                    .privileges("*")
                    .resources("*")
                    .build()
            },
            null, new String[] { "*" },
    MetadataUtils.DEFAULT_RESERVED_METADATA,
    Collections.emptyMap());

In the context of the Backup .security work, this would allow the creation of a curator role that would permit listing (get settings) for all indices (including the restricted ones). That way the curator role would be able to list and snapshot all indices, but not read or restore any of them.

Relates #34454

elasticmachine · 2019-01-17T16:58:11Z

Pinging @elastic/es-security

jaymode

This is looking pretty good

jaymode · 2019-01-18T16:40:38Z

...level/src/main/java/org/elasticsearch/client/security/user/privileges/IndicesPrivileges.java

-                              @Nullable String query) {
-        super(indices, privileges);
+    private IndicesPrivileges(Collection<String> indices, Collection<String> privileges, boolean allowRestrictedIndices,
+            @Nullable FieldSecurity fieldSecurity, @Nullable String query) {


nit: can you align the fields so that the @ on this line aligns with the C

jaymode · 2019-01-18T16:53:35Z

.../src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java

+            return check(action) && (indexNameMatcher.test(index)
+                    && ((false == RestrictedIndicesNames.NAMES_SET.contains(index)) // if it is not restricted no more checks are required
+                            || IndexPrivilege.MONITOR.predicate().test(action) // allow monitor as a special case, even for restricted
+                            || allowRestrictedIndices)


I suggest we reorder this check to:

(indexNameMatcher.test(index) && (allowRestrictedIndices || false == RestrictedIndicesNames.NAMES_SET.contains(index) || IndexPrivilege.MONITOR.predicate().test(action))

jaymode · 2019-01-18T17:01:15Z

.../src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java

+                    }
+                }
+            }
+            return indexMatcher(restrictedIndices)


I think it might be worth checking if we have any restricted or any ordinary

final Predicate<String> predicate; if (restricted.isEmpty()) { predicate = indexMatcher(ordinaryIndices) .and(index -> false == RestrictedIndicesNames.NAMES_SET.contains(index)); } else if (ordinary.isEmpty()) { predicate = indexMatcher(restrictedIndices); } else { predicate = indexMatcher(restrictedIndices) .or(indexMatcher(ordinaryIndices) .and(index -> false == RestrictedIndicesNames.NAMES_SET.contains(index))); }

also, shouldn't we take into account the monitor action case?

I think it might be worth checking if we have any restricted or any ordinary

Agreed will do!

shouldn't we take into account the monitor action case?

Ha! I noticed this too, but I decided to keep the original behavior. In the original behavior, monitoring for wildcards will not work, indices have to be named explicitly.

Out of the three options:

remove support all together

keep it as is

extend monitoring priv to wildcards

I am leaning towards 1, but only slightly. Maintaining the status quo was chosen so that this PR sail smoothly.

I think we can make any such change in a different PR (if it's breaking I'll take good care of it to catch the train!)

Let's maintain status quo in this PR and then do 1 in a followup targeted towards 7.0

jaymode · 2019-01-18T17:03:50Z

...n/core/src/main/java/org/elasticsearch/xpack/core/security/index/RestrictedIndicesNames.java

+import java.util.Collections;
+import java.util.Set;
+
+public class RestrictedIndicesNames {


make the class final and add private constructor

I'll learn, I'll learn, thanks for the patience!

albertzaharovits · 2019-01-18T18:53:44Z

Thanks for the speedy feedback @jaymode ! I have pushed the fixes.

jaymode

LGTM. Hopefully CI will agree

albertzaharovits · 2019-01-20T19:58:11Z

run gradle build tests 1

This grants the capability to grant privileges over certain restricted indices (.security and .security-6 at the moment). It also removes the special status of the superuser role. IndicesPermission.Group is extended by adding the `allow_restricted_indices` boolean flag. By default the flag is false. When it is toggled, you acknowledge that the indices under the scope of the permission group can cover the restricted indices as well. Otherwise, by default, restricted indices are ignored when granting privileges, thus rendering them hidden for authorization purposes. This effectively adds a confirmation "check-box" for roles that might grant privileges to restricted indices. The "special status" of the superuser role has been removed and coded as any other role: ``` new RoleDescriptor("superuser", new String[] { "all" }, new RoleDescriptor.IndicesPrivileges[] { RoleDescriptor.IndicesPrivileges.builder() .indices("*") .privileges("all") .allowRestrictedIndices(true) // this ----^ .build() }, new RoleDescriptor.ApplicationResourcePrivileges[] { RoleDescriptor.ApplicationResourcePrivileges.builder() .application("*") .privileges("*") .resources("*") .build() }, null, new String[] { "*" }, MetadataUtils.DEFAULT_RESERVED_METADATA, Collections.emptyMap()); ``` In the context of the Backup .security work, this allows the creation of a "curator role" that would permit listing (get settings) for all indices (including the restricted ones). That way the curator role would be able to ist and snapshot all indices, but not read or restore any of them. Supersedes #36765 Relates #34454

* elastic/master: (104 commits) Permission for restricted indices (elastic#37577) Remove Watcher Account "unsecure" settings (elastic#36736) Add cache cleaning task for ML snapshot (elastic#37505) Update jdk used by the docker builds (elastic#37621) Remove an unused constant in PutMappingRequest. Update get users to allow unknown fields (elastic#37593) Do not add index event listener if CCR disabled (elastic#37432) Add local session timeouts to leader node (elastic#37438) Add some deprecation optimizations (elastic#37597) refactor inner geogrid classes to own class files (elastic#37596) Remove obsolete deprecation checks (elastic#37510) ML: Add support for single bucket aggs in Datafeeds (elastic#37544) ML: creating ML State write alias and pointing writes there (elastic#37483) Deprecate types in the put mapping API. (elastic#37280) [ILM] Add unfollow action (elastic#36970) Packaging: Update marker used to allow ELASTIC_PASSWORD (elastic#37243) Fix setting openldap realm ssl config Document the need for JAVA11_HOME (elastic#37589) SQL: fix object extraction from sources (elastic#37502) Nit in settings.gradle for Eclipse ...

allowRestrictedIndices was only added in ES6.7 (see elastic#37577). For randomised testing we should not set that field if we are generating a role descriptor that will be serialized into an older version stream. Resolves: elastic#82216

allowRestrictedIndices was only added in ES6.7 (see #37577). For randomised testing we should not set that field if we are generating a role descriptor that will be serialized into an older version stream. Resolves: #82216

allowRestrictedIndices was only added in ES6.7 (see elastic#37577). For randomised testing we should not set that field if we are generating a role descriptor that will be serialized into an older version stream. Resolves: elastic#82216

allowRestrictedIndices was only added in ES6.7 (see #37577). For randomised testing we should not set that field if we are generating a role descriptor that will be serialized into an older version stream. Resolves: #82216

albertzaharovits added 17 commits January 15, 2019 22:47

RoleDescriptor allow_restricted_indices

b6b532d

Mapping

871b437

WIP

fc171e4

allow_restricted_indices_rename

85b4408

allowRestrictedIndices

ae55e0c

mhm

6ca5e67

V 7

b6cc231

Let's get it on!

e1ec82e

HasPrivilege woop woop

942b0f7

Client WIP

fe6fbcb

Merge branch 'master' into strict_privilege_take_4

2a97057

privileges API

f3b2753

AuthorizationService, AuthorizedIndices

68fe549

Trim

d8fc9d4

Test

6f6b62e

AuthorizedIndicesTests

9d7fc64

AuthorizedIndicesTests

615de74

albertzaharovits added >enhancement v7.0.0 :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC v6.7.0 labels Jan 17, 2019

albertzaharovits self-assigned this Jan 17, 2019

albertzaharovits requested review from tvernum and jaymode January 17, 2019 16:58

albertzaharovits mentioned this pull request Jan 17, 2019

WIldcard IndicesPermissions don't cover .security #36765

Closed

albertzaharovits added 4 commits January 17, 2019 19:26

Merge branch 'master' into strict_privilege_take_4

4d540cf

GetUserPrivilegesResponseTests

01764b2

Merge branch 'master' into strict_privilege_take_4

77effc2

PutRoleRequestTests fix

adace44

jaymode reviewed Jan 18, 2019

View reviewed changes

albertzaharovits added 2 commits January 18, 2019 20:13

Merge branch 'master' into strict_privilege_take_4

8c2623f

Address feedback

64a7bac

jaymode approved these changes Jan 18, 2019

View reviewed changes

albertzaharovits added 5 commits January 19, 2019 10:58

Checkstyle

c3d9a42

:client:rest-high-level:unitTest fix

6e09e78

Has Priv docs IT

afe946d

Fix ESNativeRealmMigrateToolTests

e104fa2

Fix RoleDescriptorTests

43c90fc

albertzaharovits merged commit ff0f540 into elastic:master Jan 20, 2019

albertzaharovits deleted the strict_privilege_take_4 branch January 20, 2019 21:19

albertzaharovits mentioned this pull request Jan 23, 2019

Remove implicit index monitor privilege #37774

Merged

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

albertzaharovits mentioned this pull request Apr 16, 2019

Backup .security #34454

Closed

3 tasks

andrershov mentioned this pull request Aug 29, 2019

Add documentation for allow_restricted_indices #46123

Closed

tvernum mentioned this pull request Jan 5, 2022

[TEST] Fix random RoleDescriptor compat with v6.7 #82242

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Permission for restricted indices #37577

Permission for restricted indices #37577

albertzaharovits commented Jan 17, 2019

elasticmachine commented Jan 17, 2019

jaymode left a comment

jaymode Jan 18, 2019

jaymode Jan 18, 2019

jaymode Jan 18, 2019

albertzaharovits Jan 18, 2019

jaymode Jan 18, 2019

jaymode Jan 18, 2019

albertzaharovits Jan 18, 2019

albertzaharovits commented Jan 18, 2019

jaymode left a comment

albertzaharovits commented Jan 20, 2019

Permission for restricted indices #37577

Permission for restricted indices #37577

Conversation

albertzaharovits commented Jan 17, 2019

elasticmachine commented Jan 17, 2019

jaymode left a comment

Choose a reason for hiding this comment

jaymode Jan 18, 2019

Choose a reason for hiding this comment

jaymode Jan 18, 2019

Choose a reason for hiding this comment

jaymode Jan 18, 2019

Choose a reason for hiding this comment

albertzaharovits Jan 18, 2019

Choose a reason for hiding this comment

jaymode Jan 18, 2019

Choose a reason for hiding this comment

jaymode Jan 18, 2019

Choose a reason for hiding this comment

albertzaharovits Jan 18, 2019

Choose a reason for hiding this comment

albertzaharovits commented Jan 18, 2019

jaymode left a comment

Choose a reason for hiding this comment

albertzaharovits commented Jan 20, 2019