Optimize cross cluster access role descriptor validation (#94968)

This PR avoids an extra de-serialization of role descriptors received in a cross cluster access request, by pushing the validation down to the role building step (where we necessarily de-serialize the received role descriptors). This also has the effect that we return a `400` instead of a `401`. I could wrap the exception so that we return a `403` instead, but I think a `400` makes the most sense, since we received a bad payload. Currently, this failure is _not_ audited. I can add logic to detect it in [`authorize()`](https://github.com/elastic/elasticsearch/blob/b17dfc77b9c48313921aaafa9a9e3da3e2739fd8/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java#L317) and emit an audit event, in a follow up, or in this PR. Just didn't want that to block review across time-zones.
elastic · Apr 5, 2023 · b2225da · b2225da
1 parent 8a43667
commit b2225da
Show file tree

Hide file tree

Showing 16 changed files with 230 additions and 191 deletions.
diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Subject.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Subject.java
@@ -18,6 +18,7 @@
 import org.elasticsearch.xpack.core.security.authz.store.RoleReference;
 import org.elasticsearch.xpack.core.security.authz.store.RoleReferenceIntersection;
 import org.elasticsearch.xpack.core.security.user.AnonymousUser;
+import org.elasticsearch.xpack.core.security.user.CrossClusterAccessUser;
 import org.elasticsearch.xpack.core.security.user.User;
 
 import java.util.ArrayList;
@@ -270,17 +271,25 @@ private RoleReferenceIntersection buildRoleReferencesForCrossClusterAccess() {
         final var crossClusterAccessRoleDescriptorsBytes = (List<RoleDescriptorsBytes>) metadata.get(
             AuthenticationField.CROSS_CLUSTER_ACCESS_ROLE_DESCRIPTORS_KEY
         );
-        if (crossClusterAccessRoleDescriptorsBytes.isEmpty()) {
+        final var innerAuthentication = (Authentication) metadata.get(CROSS_CLUSTER_ACCESS_AUTHENTICATION_KEY);
+        final User innerUser = innerAuthentication.getEffectiveSubject().getUser();
+        if (CrossClusterAccessUser.is(innerUser)) {
+            assert crossClusterAccessRoleDescriptorsBytes.isEmpty()
+                : "role descriptors bytes list for internal cross cluster access user must be empty";
+            roleReferences.add(
+                new RoleReference.FixedRoleReference(CrossClusterAccessUser.ROLE_DESCRIPTOR, "cross_cluster_access_internal")
+            );
+        } else if (crossClusterAccessRoleDescriptorsBytes.isEmpty()) {
             // If the cross cluster access role descriptors are empty, the remote user has no privileges. We need to add an empty role to
             // restrict access of the overall intersection accordingly
-            roleReferences.add(new RoleReference.CrossClusterAccessRoleReference(RoleDescriptorsBytes.EMPTY));
+            roleReferences.add(new RoleReference.CrossClusterAccessRoleReference(innerUser.principal(), RoleDescriptorsBytes.EMPTY));
         } else {
             // This is just a sanity check, since we should never have more than 2 role descriptors.
             // We can have max two role descriptors in case when API key is used for cross cluster access.
             assert crossClusterAccessRoleDescriptorsBytes.size() <= 2
                 : "not expected to have list of cross cluster access role descriptors bytes which have more than 2 elements";
             for (RoleDescriptorsBytes roleDescriptorsBytes : crossClusterAccessRoleDescriptorsBytes) {
-                roleReferences.add(new RoleReference.CrossClusterAccessRoleReference(roleDescriptorsBytes));
+                roleReferences.add(new RoleReference.CrossClusterAccessRoleReference(innerUser.principal(), roleDescriptorsBytes));
             }
         }
         roleReferences.addAll(buildRoleReferencesForApiKey().getRoleReferences());

diff --git a/...plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java b/...plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java
@@ -219,6 +219,14 @@ public boolean hasRunAs() {
         return runAs.length != 0;
     }
 
+    public boolean hasPrivilegesOtherThanIndex() {
+        return hasClusterPrivileges()
+            || hasConfigurableClusterPrivileges()
+            || hasApplicationPrivileges()
+            || hasRunAs()
+            || hasRemoteIndicesPrivileges();
+    }
+
     public String[] getRunAs() {
         return this.runAs;
     }

diff --git a/...n/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/RoleReference.java b/...n/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/RoleReference.java
@@ -11,6 +11,7 @@
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.hash.MessageDigests;
 import org.elasticsearch.xpack.core.security.authc.CrossClusterAccessSubjectInfo;
+import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
 
 import java.util.HashSet;
 import java.util.List;
@@ -121,8 +122,13 @@ final class CrossClusterAccessRoleReference implements RoleReference {
 
         private final CrossClusterAccessSubjectInfo.RoleDescriptorsBytes roleDescriptorsBytes;
         private RoleKey id = null;
+        private final String userPrincipal;
 
-        public CrossClusterAccessRoleReference(CrossClusterAccessSubjectInfo.RoleDescriptorsBytes roleDescriptorsBytes) {
+        public CrossClusterAccessRoleReference(
+            String userPrincipal,
+            CrossClusterAccessSubjectInfo.RoleDescriptorsBytes roleDescriptorsBytes
+        ) {
+            this.userPrincipal = userPrincipal;
             this.roleDescriptorsBytes = roleDescriptorsBytes;
         }
 
@@ -141,10 +147,36 @@ public void resolve(RoleReferenceResolver resolver, ActionListener<RolesRetrieva
             resolver.resolveCrossClusterAccessRoleReference(this, listener);
         }
 
+        public String getUserPrincipal() {
+            return userPrincipal;
+        }
+
         public CrossClusterAccessSubjectInfo.RoleDescriptorsBytes getRoleDescriptorsBytes() {
             return roleDescriptorsBytes;
         }
+    }
+
+    final class FixedRoleReference implements RoleReference {
+
+        private final RoleDescriptor roleDescriptor;
+        private final String source;
+
+        public FixedRoleReference(RoleDescriptor roleDescriptor, String source) {
+            this.roleDescriptor = roleDescriptor;
+            this.source = source;
+        }
 
+        @Override
+        public RoleKey id() {
+            return new RoleKey(Set.of(roleDescriptor.getName()), source);
+        }
+
+        @Override
+        public void resolve(RoleReferenceResolver resolver, ActionListener<RolesRetrievalResult> listener) {
+            final RolesRetrievalResult rolesRetrievalResult = new RolesRetrievalResult();
+            rolesRetrievalResult.addDescriptors(Set.of(roleDescriptor));
+            listener.onResponse(rolesRetrievalResult);
+        }
     }
 
     /**

diff --git a/...core/src/main/java/org/elasticsearch/xpack/core/security/user/CrossClusterAccessUser.java b/...core/src/main/java/org/elasticsearch/xpack/core/security/user/CrossClusterAccessUser.java
@@ -11,6 +11,7 @@
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.xpack.core.security.authc.Authentication;
 import org.elasticsearch.xpack.core.security.authc.CrossClusterAccessSubjectInfo;
+import org.elasticsearch.xpack.core.security.authc.Subject;
 import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
 import org.elasticsearch.xpack.core.security.authz.RoleDescriptorsIntersection;
 
@@ -20,7 +21,7 @@
 public class CrossClusterAccessUser extends User {
     public static final String NAME = UsernamesField.CROSS_CLUSTER_ACCESS_NAME;
 
-    private static final RoleDescriptor ROLE_DESCRIPTOR = new RoleDescriptor(
+    public static final RoleDescriptor ROLE_DESCRIPTOR = new RoleDescriptor(
         UsernamesField.CROSS_CLUSTER_ACCESS_ROLE,
         new String[] { "cross_cluster_access" },
         null,
@@ -56,23 +57,16 @@ public static boolean is(User user) {
         return INSTANCE.equals(user);
     }
 
-    public static CrossClusterAccessSubjectInfo subjectInfoWithRoleDescriptors(TransportVersion transportVersion, String nodeName) {
-        return subjectInfo(transportVersion, nodeName, new RoleDescriptorsIntersection(ROLE_DESCRIPTOR));
-    }
-
-    public static CrossClusterAccessSubjectInfo subjectInfoWithEmptyRoleDescriptors(TransportVersion transportVersion, String nodeName) {
-        return subjectInfo(transportVersion, nodeName, RoleDescriptorsIntersection.EMPTY);
-    }
-
-    private static CrossClusterAccessSubjectInfo subjectInfo(
-        TransportVersion transportVersion,
-        String nodeName,
-        RoleDescriptorsIntersection roleDescriptorsIntersection
-    ) {
+    /**
+     * The role descriptor intersection in the returned subject info is always empty. Because the privileges of the cross cluster access
+     * internal user are static, we set them during role reference resolution instead of needlessly deserializing the role descriptor
+     * intersection (see flow starting at {@link Subject#getRoleReferenceIntersection(AnonymousUser)})
+     */
+    public static CrossClusterAccessSubjectInfo subjectInfo(TransportVersion transportVersion, String nodeName) {
         try {
             return new CrossClusterAccessSubjectInfo(
                 Authentication.newInternalAuthentication(INSTANCE, transportVersion, nodeName),
-                roleDescriptorsIntersection
+                RoleDescriptorsIntersection.EMPTY
             );
         } catch (IOException e) {
             throw new UncheckedIOException(e);

diff --git a/...e/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTestHelper.java b/...e/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTestHelper.java
@@ -252,17 +252,12 @@ public static CrossClusterAccessSubjectInfo randomCrossClusterAccessSubjectInfo(
         }
     }
 
-    public static CrossClusterAccessSubjectInfo crossClusterAccessSubjectInfoForInternalUser(boolean emptyRoleDescriptors) {
+    public static CrossClusterAccessSubjectInfo crossClusterAccessSubjectInfoForInternalUser() {
         final Authentication authentication = AuthenticationTestHelper.builder().internal(CrossClusterAccessUser.INSTANCE).build();
-        return emptyRoleDescriptors
-            ? CrossClusterAccessUser.subjectInfoWithEmptyRoleDescriptors(
-                authentication.getEffectiveSubject().getTransportVersion(),
-                authentication.getEffectiveSubject().getRealm().getNodeName()
-            )
-            : CrossClusterAccessUser.subjectInfoWithRoleDescriptors(
-                authentication.getEffectiveSubject().getTransportVersion(),
-                authentication.getEffectiveSubject().getRealm().getNodeName()
-            );
+        return CrossClusterAccessUser.subjectInfo(
+            authentication.getEffectiveSubject().getTransportVersion(),
+            authentication.getEffectiveSubject().getRealm().getNodeName()
+        );
     }
 
     private static Authentication randomCrossClusterAccessSupportedAuthenticationSubject(boolean allowInternalUser) {
@@ -290,7 +285,7 @@ public static CrossClusterAccessSubjectInfo randomCrossClusterAccessSubjectInfo(
 
     public static CrossClusterAccessSubjectInfo randomCrossClusterAccessSubjectInfo(final Authentication authentication) {
         if (CrossClusterAccessUser.is(authentication.getEffectiveSubject().getUser())) {
-            return crossClusterAccessSubjectInfoForInternalUser(false);
+            return crossClusterAccessSubjectInfoForInternalUser();
         }
         final int numberOfRoleDescriptors;
         if (authentication.isApiKey()) {