Enforce authentication internal consistency on remote access inner au…

…thentication (#93894) This PR enhances the internal consistency check and runs it on the deserialized authentication object when performing authentication for remote access.
elastic · Mar 8, 2023 · 45a9252 · 45a9252
1 parent c572c6d
commit 45a9252
Show file tree

Hide file tree

Showing 10 changed files with 596 additions and 81 deletions.
diff --git a/...plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java b/...plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java
diff --git a/...test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationConsistencyTests.java b/...test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationConsistencyTests.java
diff --git a/...n/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java b/...n/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java
@@ -39,6 +39,7 @@
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
 
+import static java.util.Map.entry;
 import static org.elasticsearch.xpack.core.security.authc.RemoteAccessAuthenticationTests.randomRoleDescriptorsIntersection;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
@@ -355,10 +356,8 @@ public void testIsServiceAccount() {
 
         if (isServiceAccount) {
             assertThat(authentication.isServiceAccount(), is(true));
-            assertThat(authentication.isAuthenticatedWithServiceAccount(), is(true));
         } else {
             assertThat(authentication.isServiceAccount(), is(false));
-            assertThat(authentication.isAuthenticatedWithServiceAccount(), is(false));
         }
     }
 
@@ -637,9 +636,8 @@ public void testSupportsRunAs() {
         // Service account cannot run-as
         assertThat(AuthenticationTestHelper.builder().serviceAccount().build().supportsRunAs(anonymousUser), is(false));
 
-        // Neither internal user nor its token can run-as
+        // internal user cannot run-as
         assertThat(AuthenticationTestHelper.builder().internal().build().supportsRunAs(anonymousUser), is(false));
-        assertThat(AuthenticationTestHelper.builder().internal().build().token().supportsRunAs(anonymousUser), is(false));
 
         // Neither anonymous user nor its token can run-as
         assertThat(AuthenticationTestHelper.builder().anonymous(anonymousUser).build().supportsRunAs(anonymousUser), is(false));
@@ -900,13 +898,13 @@ public void testMaybeRewriteMetadataForApiKeyRoleDescriptorsWithRemoteAccess() {
         final String apiKeyId = randomAlphaOfLengthBetween(1, 10);
         final String apiKeyName = randomAlphaOfLengthBetween(1, 10);
         final Map<String, Object> metadata = Map.ofEntries(
-            Map.entry(AuthenticationField.API_KEY_ID_KEY, apiKeyId),
-            Map.entry(AuthenticationField.API_KEY_NAME_KEY, apiKeyName),
-            Map.entry(AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY, new BytesArray("""
+            entry(AuthenticationField.API_KEY_ID_KEY, apiKeyId),
+            entry(AuthenticationField.API_KEY_NAME_KEY, apiKeyName),
+            entry(AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY, new BytesArray("""
                 {"base_role":{"cluster":["all"],
                 "remote_indices":{"names":["logs-*"],"privileges":["read"],"clusters":["my_cluster*","other_cluster"]}}
                 }""")),
-            Map.entry(AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY, new BytesArray("""
+            entry(AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY, new BytesArray("""
                 {"limited_by_role":{"cluster":["*"],
                 "remote_indices":{"names":["logs-*-*"],"privileges":["write"],"clusters":["my_cluster*"]}}
                 }"""))

diff --git a/...urity/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java b/...urity/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java
@@ -1653,7 +1653,7 @@ LogEntryBuilder withAuthentication(Authentication authentication) {
                 }
             }
             // TODO: service token info is logged in a separate authentication field (#84394)
-            if (authentication.isAuthenticatedWithServiceAccount()) {
+            if (authentication.isServiceAccount()) {
                 logEntry.with(
                     SERVICE_TOKEN_NAME_FIELD_NAME,
                     (String) authentication.getAuthenticatingSubject().getMetadata().get(TOKEN_NAME_FIELD)

diff --git a/...c/main/java/org/elasticsearch/xpack/security/authc/RemoteAccessAuthenticationService.java b/...c/main/java/org/elasticsearch/xpack/security/authc/RemoteAccessAuthenticationService.java
@@ -148,7 +148,19 @@ public AuthenticationService getAuthenticationService() {
     }
 
     private void validate(final RemoteAccessAuthentication remoteAccessAuthentication) {
-        final Subject effectiveSubject = remoteAccessAuthentication.getAuthentication().getEffectiveSubject();
+        final Authentication authentication = remoteAccessAuthentication.getAuthentication();
+        authentication.checkConsistency();
+        final Subject effectiveSubject = authentication.getEffectiveSubject();
+        if (false == effectiveSubject.getType().equals(Subject.Type.USER)) {
+            throw new IllegalArgumentException(
+                "subject ["
+                    + effectiveSubject.getUser().principal()
+                    + "] has type ["
+                    + effectiveSubject.getType()
+                    + "] which is not supported for remote access"
+            );
+        }
+
         for (RemoteAccessAuthentication.RoleDescriptorsBytes roleDescriptorsBytes : remoteAccessAuthentication
             .getRoleDescriptorsBytesList()) {
             final Set<RoleDescriptor> roleDescriptors = roleDescriptorsBytes.toRoleDescriptors();
@@ -167,15 +179,6 @@ private void validate(final RemoteAccessAuthentication remoteAccessAuthenticatio
                 }
             }
         }
-        if (false == effectiveSubject.getType().equals(Subject.Type.USER)) {
-            throw new IllegalArgumentException(
-                "subject ["
-                    + effectiveSubject.getUser().principal()
-                    + "] has type ["
-                    + effectiveSubject.getType()
-                    + "] which is not supported for remote access"
-            );
-        }
     }
 
     private Version getMinNodeVersion() {

diff --git a/...y/src/main/java/org/elasticsearch/xpack/security/authc/service/ServiceAccountService.java b/...y/src/main/java/org/elasticsearch/xpack/security/authc/service/ServiceAccountService.java
@@ -163,7 +163,7 @@ public void findTokensFor(GetServiceAccountCredentialsRequest request, ActionLis
 
     // TODO: No production code usage
     public void getRoleDescriptor(Authentication authentication, ActionListener<RoleDescriptor> listener) {
-        assert authentication.isAuthenticatedWithServiceAccount() : "authentication is not for service account: " + authentication;
+        assert authentication.isServiceAccount() : "authentication is not for service account: " + authentication;
         final String principal = authentication.getEffectiveSubject().getUser().principal();
         getRoleDescriptorForPrincipal(principal, listener);
     }

diff --git a/...ity/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationDenialMessages.java b/...ity/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationDenialMessages.java
@@ -104,7 +104,7 @@ private static String remoteClusterText(@Nullable String clusterAlias) {
     }
 
     private static String authenticatedUserDescription(Authentication authentication) {
-        String userText = (authentication.isAuthenticatedWithServiceAccount() ? "service account" : "user")
+        String userText = (authentication.isServiceAccount() ? "service account" : "user")
             + " ["
             + authentication.getAuthenticatingSubject().getUser().principal()
             + "]";

diff --git a/.../src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java b/.../src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java
@@ -2100,7 +2100,7 @@ public void testAccessDenied() throws Exception {
             .put(LoggingAuditTrail.ACTION_FIELD_NAME, "_action/bar")
             .put(LoggingAuditTrail.REQUEST_NAME_FIELD_NAME, request.getClass().getSimpleName())
             .put(LoggingAuditTrail.REQUEST_ID_FIELD_NAME, requestId);
-        if (authentication.isAuthenticatedWithServiceAccount()) {
+        if (authentication.isServiceAccount()) {
             checkedFields.put(
                 LoggingAuditTrail.SERVICE_TOKEN_NAME_FIELD_NAME,
                 (String) authentication.getAuthenticatingSubject().getMetadata().get(TOKEN_NAME_FIELD)
@@ -2942,7 +2942,7 @@ private static void authentication(Authentication authentication, MapBuilder<Str
                 }
             }
         }
-        if (authentication.isAuthenticatedWithServiceAccount()) {
+        if (authentication.isServiceAccount()) {
             checkedFields.put(
                 LoggingAuditTrail.SERVICE_TOKEN_NAME_FIELD_NAME,
                 (String) authentication.getAuthenticatingSubject().getMetadata().get(TOKEN_NAME_FIELD)

diff --git a/...rity/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java b/...rity/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java
@@ -849,20 +849,20 @@ public void testAuthenticateRestAnonymous() throws Exception {
     public void testAuthenticateTransportFallback() throws Exception {
         when(firstRealm.token(threadContext)).thenReturn(null);
         when(secondRealm.token(threadContext)).thenReturn(null);
-        User user1 = new User("username", "r1", "r2");
+        User fallbackUser = AuthenticationTestHelper.randomInternalUser();
         boolean requestIdAlreadyPresent = randomBoolean();
         SetOnce<String> reqId = new SetOnce<>();
         if (requestIdAlreadyPresent) {
             reqId.set(AuditUtil.getOrGenerateRequestId(threadContext));
         }
 
-        authenticateBlocking("_action", transportRequest, user1, result -> {
+        authenticateBlocking("_action", transportRequest, fallbackUser, result -> {
             if (requestIdAlreadyPresent) {
                 assertThat(expectAuditRequestId(threadContext), is(reqId.get()));
             }
             assertThat(expectAuditRequestId(threadContext), is(result.v2()));
             assertThat(result, notNullValue());
-            assertThat(result.v1().getEffectiveSubject().getUser(), sameInstance(user1));
+            assertThat(result.v1().getEffectiveSubject().getUser(), sameInstance(fallbackUser));
             assertThat(result.v1().getAuthenticationType(), is(AuthenticationType.INTERNAL));
             assertThreadContextContainsAuthentication(result.v1());
             verify(operatorPrivilegesService).maybeMarkOperatorUser(eq(result.v1()), eq(threadContext));

diff --git a/...ecurity/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticatorChainTests.java b/...ecurity/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticatorChainTests.java
@@ -96,7 +96,7 @@ public void init() {
         when(realms.getUnlicensedRealms()).thenReturn(List.of());
         final User user = new User(randomAlphaOfLength(8));
         authentication = AuthenticationTestHelper.builder().user(user).build(false);
-        fallbackUser = mock(User.class);
+        fallbackUser = AuthenticationTestHelper.randomInternalUser();
         authenticatorChain = new AuthenticatorChain(
             settings,
             operatorPrivilegesService,
@@ -326,7 +326,6 @@ public void testRunAsIsIgnoredForUnsupportedAuthenticationTypes() throws Illegal
             AuthenticationTestHelper.builder().anonymous(anonymousUser).build(),
             AuthenticationTestHelper.builder().anonymous(anonymousUser).build().token(),
             AuthenticationTestHelper.builder().internal().build(),
-            AuthenticationTestHelper.builder().internal().build().token(),
             AuthenticationTestHelper.builder().realm().build(true),
             AuthenticationTestHelper.builder().realm().build(true).token(),
             AuthenticationTestHelper.builder().apiKey().runAs().build(),