Validate field permissions when creating a role

When creating a role, we do not check if the exceptions for
the field permissions are a subset of granted fields. If such
a role is assigned to a user then that user's authentication fails
for this reason.

We added a check to validate role query in elastic#46275 and on the same lines,
this commit adds check if the exceptions for the field
permissions is a subset of granted fields when parsing the
index privileges from the role descriptor.

Co-authored-by: Yogesh Gaikwad <[email protected]>

Backport of: elastic#50212

Author: tvernum

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java

@@ -7,6 +7,7 @@
 
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.Version;
+import org.elasticsearch.ElasticsearchSecurityException;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.Strings;
@@ -24,6 +25,7 @@
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
+import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissions;
 import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivilege;
 import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivileges;
 import org.elasticsearch.xpack.core.security.support.Validation;
@@ -539,6 +541,7 @@ private static RoleDescriptor.IndicesPrivileges parseIndex(String roleName, XCon
             throw new ElasticsearchParseException("failed to parse indices privileges for role [{}]. {} requires {} if {} is given",
                     roleName, Fields.FIELD_PERMISSIONS, Fields.GRANT_FIELDS, Fields.EXCEPT_FIELDS);
         }
+        checkIfExceptFieldsIsSubsetOfGrantedFields(roleName, grantedFields, deniedFields);
         return RoleDescriptor.IndicesPrivileges.builder()
                 .indices(names)
                 .privileges(privileges)
@@ -549,6 +552,14 @@ private static RoleDescriptor.IndicesPrivileges parseIndex(String roleName, XCon
                 .build();
     }
 
+    private static void checkIfExceptFieldsIsSubsetOfGrantedFields(String roleName, String[] grantedFields, String[] deniedFields) {
+        try {
+            FieldPermissions.buildPermittedFieldsAutomaton(grantedFields, deniedFields);
+        } catch (ElasticsearchSecurityException e) {
+            throw new ElasticsearchParseException("failed to parse indices privileges for role [{}] - {}", e, roleName, e.getMessage());
+        }
+    }
+
     private static ApplicationResourcePrivileges[] parseApplicationPrivileges(String roleName, XContentParser parser)
             throws IOException {
         if (parser.currentToken() != XContentParser.Token.START_ARRAY) {

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/FieldPermissions.java

@@ -119,12 +119,16 @@ public static Automaton initializePermittedFieldsAutomaton(FieldPermissionsDefin
         assert groups.size() > 0 : "there must always be a single group for field inclusion/exclusion";
         List<Automaton> automatonList =
                 groups.stream()
-                        .map(g -> FieldPermissions.initializePermittedFieldsAutomaton(g.getGrantedFields(), g.getExcludedFields()))
+                        .map(g -> FieldPermissions.buildPermittedFieldsAutomaton(g.getGrantedFields(), g.getExcludedFields()))
                         .collect(Collectors.toList());
         return Automatons.unionAndMinimize(automatonList);
     }
 
-    private static Automaton initializePermittedFieldsAutomaton(final String[] grantedFields, final String[] deniedFields) {
+    /**
+     * Construct a single automaton to represent the set of {@code grantedFields} except for the {@code deniedFields}.
+     * @throws ElasticsearchSecurityException If {@code deniedFields} is not a subset of {@code grantedFields}.
+     */
+    public static Automaton buildPermittedFieldsAutomaton(final String[] grantedFields, final String[] deniedFields) {
         Automaton grantedFieldsAutomaton;
         if (grantedFields == null || Arrays.stream(grantedFields).anyMatch(Regex::isMatchAllPattern)) {
             grantedFieldsAutomaton = Automatons.MATCH_ALL;

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RoleDescriptorTests.java

@@ -5,6 +5,7 @@
  */
 package org.elasticsearch.xpack.security.authz;
 
+import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.Version;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesArray;
@@ -19,6 +20,7 @@
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.TestMatchers;
 import org.elasticsearch.test.VersionUtils;
 import org.elasticsearch.xpack.core.XPackClientPlugin;
 import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
@@ -27,6 +29,7 @@
 import org.elasticsearch.xpack.core.security.support.MetadataUtils;
 import org.hamcrest.Matchers;
 
+import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.LinkedHashSet;
@@ -296,4 +299,30 @@ public void testParseIgnoresTransientMetadata() throws Exception {
         assertEquals(true, parsed.getTransientMetadata().get("enabled"));
     }
 
+    public void testParseIndicesPrivilegesSucceedsWhenExceptFieldsIsSubsetOfGrantedFields() throws IOException {
+        final boolean grantAll = randomBoolean();
+        final String grant = grantAll ? "\"*\"" : "\"f1\",\"f2\"";
+        final String except = grantAll ? "\"_fx\",\"f8\"" : "\"f1\"";
+
+        final String json = "{ \"indices\": [{\"names\": [\"idx1\",\"idx2\"], \"privileges\": [\"p1\", \"p2\"], \"field_security\" : { " +
+            "\"grant\" : [" + grant + "], \"except\" : [" + except + "] } }] }";
+        final RoleDescriptor rd = RoleDescriptor.parse("test",
+            new BytesArray(json), false, XContentType.JSON);
+        assertEquals("test", rd.getName());
+        assertEquals(1, rd.getIndicesPrivileges().length);
+        assertArrayEquals(new String[]{"idx1", "idx2"}, rd.getIndicesPrivileges()[0].getIndices());
+        assertArrayEquals((grantAll) ? new String[]{"*"} : new String[]{"f1", "f2"}, rd.getIndicesPrivileges()[0].getGrantedFields());
+        assertArrayEquals((grantAll) ? new String[]{"_fx", "f8"} : new String[]{"f1"}, rd.getIndicesPrivileges()[0].getDeniedFields());
+    }
+
+    public void testParseIndicesPrivilegesFailsWhenExceptFieldsAreNotSubsetOfGrantedFields() {
+        final String json = "{ \"indices\": [{\"names\": [\"idx1\",\"idx2\"], \"privileges\": [\"p1\", \"p2\"], \"field_security\" : { " +
+            "\"grant\" : [\"f1\",\"f2\"], \"except\" : [\"f3\"] } }] }";
+        final ElasticsearchParseException epe = expectThrows(ElasticsearchParseException.class, () -> RoleDescriptor.parse("test",
+            new BytesArray(json), false, XContentType.JSON));
+        assertThat(epe, TestMatchers.throwableWithMessage(containsString("must be a subset of the granted fields ")));
+        assertThat(epe, TestMatchers.throwableWithMessage(containsString("f1")));
+        assertThat(epe, TestMatchers.throwableWithMessage(containsString("f2")));
+        assertThat(epe, TestMatchers.throwableWithMessage(containsString("f3")));
+    }
 }