Fix can access resource checks for API Keys with run as (#84431)

* Fix can access resource checks for API Keys with run as (#84277)

This fixes two things for the "can access" authz check: * API Keys
running as, have access to the resources created by the effective run as
user * tokens created by API Keys (with the client credentials) have
access to the API Key's resources

In addition, this PR moves some of the authz plumbing code from the
Async and Scroll services classes under the Security Context class (as a
minor refactoring).

* spotless

* Merge fallout

* AuthenticationTests

Co-authored-by: Elastic Machine <[email protected]>

Author: albertzaharovits

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/async/AsyncTaskIndexService.java

@@ -57,8 +57,6 @@
 import org.elasticsearch.xpack.core.search.action.AsyncSearchResponse;
 import org.elasticsearch.xpack.core.search.action.SearchStatusResponse;
 import org.elasticsearch.xpack.core.security.SecurityContext;
-import org.elasticsearch.xpack.core.security.authc.Authentication;
-import org.elasticsearch.xpack.core.security.authc.support.AuthenticationContextSerializer;
 
 import java.io.FilterOutputStream;
 import java.io.IOException;
@@ -78,7 +76,6 @@
 import static org.elasticsearch.search.SearchService.MAX_ASYNC_SEARCH_RESPONSE_SIZE_SETTING;
 import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
 import static org.elasticsearch.xpack.core.ClientHelper.ASYNC_SEARCH_ORIGIN;
-import static org.elasticsearch.xpack.core.security.authc.AuthenticationField.AUTHENTICATION_KEY;
 
 /**
  * A service that exposes the CRUD operations for the async task-specific index.
@@ -203,8 +200,8 @@ public Client getClient() {
     /**
      * Returns the authentication information, or null if the current context has no authentication info.
      **/
-    public Authentication getAuthentication() {
-        return securityContext.getAuthentication();
+    public SecurityContext getSecurityContext() {
+        return securityContext;
     }
 
     /**
@@ -397,8 +394,7 @@ public <T extends AsyncTask> T getTaskAndCheckAuthentication(
             return null;
         }
         // Check authentication for the user
-        final Authentication auth = securityContext.getAuthentication();
-        if (ensureAuthenticatedUserIsSame(asyncTask.getOriginHeaders(), auth) == false) {
+        if (false == securityContext.canIAccessResourcesCreatedWithHeaders(asyncTask.getOriginHeaders())) {
             throw new ResourceNotFoundException(asyncExecutionId.getEncoded() + " not found");
         }
         return asyncTask;
@@ -471,7 +467,7 @@ private R parseResponseFromIndex(
                         @SuppressWarnings("unchecked")
                         final Map<String, String> headers = (Map<String, String>) XContentParserUtils.parseFieldsValue(parser);
                         // check the authentication of the current user against the user that initiated the async task
-                        if (checkAuthentication && ensureAuthenticatedUserIsSame(headers, securityContext.getAuthentication()) == false) {
+                        if (checkAuthentication && false == securityContext.canIAccessResourcesCreatedWithHeaders(headers)) {
                             throw new ResourceNotFoundException(asyncExecutionId.getEncoded());
                         }
                     }
@@ -540,8 +536,7 @@ public <T extends AsyncTask, SR extends SearchStatusResponse> void retrieveStatu
     }
 
     /**
-     * Checks if the current user's authentication matches the original authentication stored
-     * in the async search index.
+     * Checks if the current user can access the async search result of the original user.
      **/
     void ensureAuthenticatedUserCanDeleteFromIndex(AsyncExecutionId executionId, ActionListener<Void> listener) {
         GetRequest internalGet = new GetRequest(index).preference(executionId.getEncoded())
@@ -556,32 +551,14 @@ void ensureAuthenticatedUserCanDeleteFromIndex(AsyncExecutionId executionId, Act
             // Check authentication for the user
             @SuppressWarnings("unchecked")
             Map<String, String> headers = (Map<String, String>) get.getSource().get(HEADERS_FIELD);
-            if (ensureAuthenticatedUserIsSame(headers, securityContext.getAuthentication())) {
+            if (securityContext.canIAccessResourcesCreatedWithHeaders(headers)) {
                 listener.onResponse(null);
             } else {
                 listener.onFailure(new ResourceNotFoundException(executionId.getEncoded()));
             }
         }, exc -> listener.onFailure(new ResourceNotFoundException(executionId.getEncoded()))));
     }
 
-    /**
-     * Extracts the authentication from the original headers and checks that it matches
-     * the current user. This function returns always <code>true</code> if the provided
-     * <code>headers</code> do not contain any authentication.
-     */
-    boolean ensureAuthenticatedUserIsSame(Map<String, String> originHeaders, Authentication current) throws IOException {
-        if (originHeaders == null || originHeaders.containsKey(AUTHENTICATION_KEY) == false) {
-            // no authorization attached to the original request
-            return true;
-        }
-        if (current == null) {
-            // origin is an authenticated user but current is not
-            return false;
-        }
-        Authentication origin = AuthenticationContextSerializer.decode(originHeaders.get(AUTHENTICATION_KEY));
-        return origin.canAccessResourcesOf(current);
-    }
-
     private void writeResponse(R response, OutputStream os) throws IOException {
         os = new FilterOutputStream(os) {
             @Override

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/async/DeleteAsyncResultsService.java

@@ -53,7 +53,7 @@ public void deleteResponse(DeleteAsyncResultRequest request, ActionListener<Ackn
      * delete async search submitted by another user.
      */
     private void hasCancelTaskPrivilegeAsync(Consumer<Boolean> consumer) {
-        final Authentication current = store.getAuthentication();
+        final Authentication current = store.getSecurityContext().getAuthentication();
         if (current != null) {
             HasPrivilegesRequest req = new HasPrivilegesRequest();
             req.username(current.getUser().principal());

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

@@ -38,6 +38,7 @@
 import static org.elasticsearch.xpack.core.security.authc.Authentication.VERSION_API_KEY_ROLES_AS_BYTES;
 import static org.elasticsearch.xpack.core.security.authc.AuthenticationField.ATTACH_REALM_NAME;
 import static org.elasticsearch.xpack.core.security.authc.AuthenticationField.ATTACH_REALM_TYPE;
+import static org.elasticsearch.xpack.core.security.authc.AuthenticationField.AUTHENTICATION_KEY;
 
 /**
  * A lightweight utility that can find the current user and authentication information for the local thread.
@@ -121,15 +122,6 @@ public void setUser(User user, Version version) {
         );
     }
 
-    /** Writes the authentication to the thread context */
-    private void setAuthentication(Authentication authentication) {
-        try {
-            authentication.writeToContext(threadContext);
-        } catch (IOException e) {
-            throw new AssertionError("how can we have a IOException with a user we set", e);
-        }
-    }
-
     /**
      * Runs the consumer in a new context as the provided user. The original context is provided to the consumer. When this method
      * returns, the original context is restored.
@@ -221,16 +213,61 @@ private Map<String, Object> rewriteMetadataForApiKeyRoleDescriptors(Version stre
         return metadata;
     }
 
-    private Map<String, Object> convertRoleDescriptorsBytesToMap(BytesReference roleDescriptorsBytes) {
+    private static Map<String, Object> convertRoleDescriptorsBytesToMap(BytesReference roleDescriptorsBytes) {
         return XContentHelper.convertToMap(roleDescriptorsBytes, false, XContentType.JSON).v2();
     }
 
-    private BytesReference convertRoleDescriptorsMapToBytes(Map<String, Object> roleDescriptorsMap) {
+    private static BytesReference convertRoleDescriptorsMapToBytes(Map<String, Object> roleDescriptorsMap) {
         try (XContentBuilder builder = XContentBuilder.builder(XContentType.JSON.xContent())) {
             builder.map(roleDescriptorsMap);
             return BytesReference.bytes(builder);
         } catch (IOException e) {
             throw new UncheckedIOException(e);
         }
     }
+
+    /**
+     * Checks whether the user or API key of the passed in authentication can access the resources owned by the user
+     * or API key of this authentication. The rules are as follows:
+     *   * True if the authentications are for the same API key (same API key ID)
+     *   * True if they are the same username from the same realm
+     *      - For file and native realm, same realm means the same realm type
+     *      - For all other realms, same realm means same realm type plus same realm name
+     *   * An user and its API key cannot access each other's resources
+     *   * An user and its token can access each other's resources
+     *   * Two API keys are never able to access each other's resources regardless of their ownership.
+     *
+     *  This check is a best effort and it does not account for certain static and external changes.
+     *  See also <a href="https://www.elastic.co/guide/en/elasticsearch/reference/master/security-limitations.html">
+     *      security limitations</a>
+     */
+    public boolean canIAccessResourcesCreatedBy(@Nullable Authentication resourceCreatorAuthentication) {
+        if (resourceCreatorAuthentication == null) {
+            // resource creation was not authenticated (security was disabled); anyone can access such resources
+            return true;
+        }
+        final Authentication myAuthentication = getAuthentication();
+        if (myAuthentication == null) {
+            // unauthenticated users cannot access any resources created by authenticated users, even anonymously authenticated ones
+            return false;
+        }
+        return myAuthentication.canAccessResourcesOf(resourceCreatorAuthentication);
+    }
+
+    public boolean canIAccessResourcesCreatedWithHeaders(Map<String, String> resourceCreateRequestHeaders) throws IOException {
+        Authentication resourceCreatorAuthentication = null;
+        if (resourceCreateRequestHeaders != null && resourceCreateRequestHeaders.containsKey(AUTHENTICATION_KEY)) {
+            resourceCreatorAuthentication = AuthenticationContextSerializer.decode(resourceCreateRequestHeaders.get(AUTHENTICATION_KEY));
+        }
+        return canIAccessResourcesCreatedBy(resourceCreatorAuthentication);
+    }
+
+    /** Writes the authentication to the thread context */
+    private void setAuthentication(Authentication authentication) {
+        try {
+            authentication.writeToContext(threadContext);
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
+    }
 }

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java

@@ -32,6 +32,7 @@
 import java.util.Objects;
 
 import static org.elasticsearch.xcontent.ConstructingObjectParser.constructorArg;
+import static org.elasticsearch.xpack.core.security.authc.Subject.Type.API_KEY;
 
 // TODO(hub-cap) Clean this up after moving User over - This class can re-inherit its field AUTHENTICATION_KEY in AuthenticationField.
 // That interface can be removed
@@ -196,40 +197,55 @@ public void writeTo(StreamOutput out) throws IOException {
      *      security limitations</a>
      */
     public boolean canAccessResourcesOf(Authentication other) {
-        if (isApiKey() && other.isApiKey()) {
-            final boolean sameKeyId = getMetadata().get(AuthenticationField.API_KEY_ID_KEY)
-                .equals(other.getMetadata().get(AuthenticationField.API_KEY_ID_KEY));
-            if (sameKeyId) {
-                assert getUser().principal().equals(getUser().principal())
-                    : "The same API key ID cannot be attributed to two different usernames";
-            }
+        // if we introduce new authentication types in the future, it is likely that we'll need to revisit this method
+        assert EnumSet.of(
+            Authentication.AuthenticationType.REALM,
+            Authentication.AuthenticationType.API_KEY,
+            Authentication.AuthenticationType.TOKEN,
+            Authentication.AuthenticationType.ANONYMOUS,
+            Authentication.AuthenticationType.INTERNAL
+        ).containsAll(EnumSet.of(getAuthenticationType(), other.getAuthenticationType()))
+            : "cross AuthenticationType comparison for canAccessResourcesOf is not applicable for: "
+                + EnumSet.of(getAuthenticationType(), other.getAuthenticationType());
+        final AuthenticationContext myAuthContext = AuthenticationContext.fromAuthentication(this);
+        final AuthenticationContext creatorAuthContext = AuthenticationContext.fromAuthentication(other);
+        if (API_KEY.equals(myAuthContext.getEffectiveSubject().getType())
+            && API_KEY.equals(creatorAuthContext.getEffectiveSubject().getType())) {
+            final boolean sameKeyId = myAuthContext.getEffectiveSubject()
+                .getMetadata()
+                .get(AuthenticationField.API_KEY_ID_KEY)
+                .equals(creatorAuthContext.getEffectiveSubject().getMetadata().get(AuthenticationField.API_KEY_ID_KEY));
+            assert false == sameKeyId
+                || myAuthContext.getEffectiveSubject()
+                    .getUser()
+                    .principal()
+                    .equals(creatorAuthContext.getEffectiveSubject().getUser().principal())
+                : "The same API key ID cannot be attributed to two different usernames";
             return sameKeyId;
-        }
-
-        if (getAuthenticationType().equals(other.getAuthenticationType())
-            || (AuthenticationType.REALM == getAuthenticationType() && AuthenticationType.TOKEN == other.getAuthenticationType())
-            || (AuthenticationType.TOKEN == getAuthenticationType() && AuthenticationType.REALM == other.getAuthenticationType())) {
-            if (false == getUser().principal().equals(other.getUser().principal())) {
-                return false;
-            }
-            final RealmRef thisRealm = getSourceRealm();
-            final RealmRef otherRealm = other.getSourceRealm();
-            if (FileRealmSettings.TYPE.equals(thisRealm.getType()) || NativeRealmSettings.TYPE.equals(thisRealm.getType())) {
-                return thisRealm.getType().equals(otherRealm.getType());
-            }
-            return thisRealm.getName().equals(otherRealm.getName()) && thisRealm.getType().equals(otherRealm.getType());
-        } else {
-            assert EnumSet.of(
-                AuthenticationType.REALM,
-                AuthenticationType.API_KEY,
-                AuthenticationType.TOKEN,
-                AuthenticationType.ANONYMOUS,
-                AuthenticationType.INTERNAL
-            ).containsAll(EnumSet.of(getAuthenticationType(), other.getAuthenticationType()))
-                : "cross AuthenticationType comparison for canAccessResourcesOf is not applicable for: "
-                    + EnumSet.of(getAuthenticationType(), other.getAuthenticationType());
-            return false;
-        }
+        } else if ((API_KEY.equals(myAuthContext.getEffectiveSubject().getType())
+            && false == API_KEY.equals(creatorAuthContext.getEffectiveSubject().getType()))
+            || (false == API_KEY.equals(myAuthContext.getEffectiveSubject().getType())
+                && API_KEY.equals(creatorAuthContext.getEffectiveSubject().getType()))) {
+                    // an API Key cannot access resources created by non-API Keys or vice-versa
+                    return false;
+                } else {
+                    if (false == myAuthContext.getEffectiveSubject()
+                        .getUser()
+                        .principal()
+                        .equals(creatorAuthContext.getEffectiveSubject().getUser().principal())) {
+                        return false;
+                    }
+                    final Authentication.RealmRef myAuthRealm = myAuthContext.getEffectiveSubject().getRealm();
+                    final Authentication.RealmRef creatorAuthRealm = creatorAuthContext.getEffectiveSubject().getRealm();
+                    if (FileRealmSettings.TYPE.equals(myAuthRealm.getType()) || NativeRealmSettings.TYPE.equals(myAuthRealm.getType())) {
+                        // file and native realms can be renamed...
+                        // nonetheless, they are singleton realms, only one such realm of each type can exist
+                        return myAuthRealm.getType().equals(creatorAuthRealm.getType());
+                    } else {
+                        return myAuthRealm.getName().equals(creatorAuthRealm.getName())
+                            && myAuthRealm.getType().equals(creatorAuthRealm.getType());
+                    }
+                }
     }
 
     @Override