From e12a0bcb34f0cf9bd8158e17f91f7705815c3a00 Mon Sep 17 00:00:00 2001
From: Ioannis Kakavas <ioannis@elastic.co>
Date: Fri, 17 Jan 2020 15:50:40 +0200
Subject: [PATCH 1/2] Don't fallback to anonymous for tokens/apikeys (#51042)

This commit changes our behavior so that when we receive a
request with an invalid/expired/wrong access token or API Key
we do not fallback to authenticating as the anonymous user even if
anonymous access is enabled for Elasticsearch.
---
 .../security/authc/AuthenticationService.java | 17 +++++-
 .../authc/AuthenticationServiceTests.java     | 54 +++++++++++++++++++
 2 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java
index 20289c5f09e91..f94e198c0e359 100644
--- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java
+++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java
@@ -460,6 +460,7 @@ private List<Realm> getRealmList(String principal) {
          * <ul>
          * <li>this is an initial request from a client without preemptive authentication, so we must return an authentication
          * challenge</li>
+         * <li>this is a request that contained an Authorization Header that we can't validate </li>
          * <li>this is a request made internally within a node and there is a fallback user, which is typically the
          * {@link SystemUser}</li>
          * <li>anonymous access is enabled and this will be considered an anonymous request</li>
@@ -475,7 +476,7 @@ void handleNullToken() {
                 RealmRef authenticatedBy = new RealmRef("__fallback", "__fallback", nodeName);
                 authentication = new Authentication(fallbackUser, authenticatedBy, null, Version.CURRENT, AuthenticationType.INTERNAL,
                     Collections.emptyMap());
-            } else if (isAnonymousUserEnabled) {
+            } else if (isAnonymousUserEnabled && shouldFallbackToAnonymous()) {
                 logger.trace("No valid credentials found in request [{}], using anonymous [{}]", request, anonymousUser.principal());
                 RealmRef authenticatedBy = new RealmRef("__anonymous", "__anonymous", nodeName);
                 authentication = new Authentication(anonymousUser, authenticatedBy, null, Version.CURRENT, AuthenticationType.ANONYMOUS,
@@ -499,6 +500,20 @@ void handleNullToken() {
             action.run();
         }
 
+        /**
+         * When an API Key or an Elasticsearch Token Service token is used for authentication and authentication fails (as indicated by
+         * a null AuthenticationToken) we should not fallback to the anonymous user.
+         */
+        boolean shouldFallbackToAnonymous(){
+            String header = threadContext.getHeader("Authorization");
+            if (Strings.hasText(header) &&
+                ((header.regionMatches(true, 0, "Bearer ", 0, "Bearer ".length()) && header.length() > "Bearer ".length()) ||
+                (header.regionMatches(true, 0, "ApiKey ", 0, "ApiKey ".length()) && header.length() > "ApiKey ".length()))) {
+                return false;
+            }
+            return true;
+        }
+
         /**
          * Consumes the {@link User} that resulted from attempting to authenticate a token against the {@link Realms}. When the user is
          * {@code null}, authentication fails and does not proceed. When there is a user, the request is inspected to see if the run as
diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java
index fcde6aad488dc..7f7d00d61c0b6 100644
--- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java
+++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java
@@ -99,6 +99,7 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Consumer;
 
+import static org.elasticsearch.index.seqno.SequenceNumbers.UNASSIGNED_PRIMARY_TERM;
 import static org.elasticsearch.test.SecurityTestsUtils.assertAuthenticationException;
 import static org.elasticsearch.xpack.core.security.support.Exceptions.authenticationError;
 import static org.elasticsearch.xpack.security.authc.TokenServiceTests.mockGetTokenFromId;
@@ -744,6 +745,59 @@ public void testAuthenticateTamperedUser() throws Exception {
         }
     }
 
+    public void testWrongTokenDoesNotFallbackToAnonymous() {
+        String username = randomBoolean() ? AnonymousUser.DEFAULT_ANONYMOUS_USERNAME : "user1";
+        Settings.Builder builder = Settings.builder()
+            .putList(AnonymousUser.ROLES_SETTING.getKey(), "r1", "r2", "r3");
+        if (username.equals(AnonymousUser.DEFAULT_ANONYMOUS_USERNAME) == false) {
+            builder.put(AnonymousUser.USERNAME_SETTING.getKey(), username);
+        }
+        Settings anonymousEnabledSettings = builder.build();
+        final AnonymousUser anonymousUser = new AnonymousUser(anonymousEnabledSettings);
+        service = new AuthenticationService(anonymousEnabledSettings, realms, auditTrail,
+            new DefaultAuthenticationFailureHandler(Collections.emptyMap()), threadPool, anonymousUser, tokenService, apiKeyService);
+
+        try (ThreadContext.StoredContext ignore = threadContext.stashContext()) {
+            final String reqId = AuditUtil.getOrGenerateRequestId(threadContext);
+            threadContext.putHeader("Authorization", "Bearer thisisaninvalidtoken");
+            ElasticsearchSecurityException e =
+                expectThrows(ElasticsearchSecurityException.class, () -> authenticateBlocking("_action", message, null));
+            verify(auditTrail).anonymousAccessDenied(reqId, "_action", message);
+            verifyNoMoreInteractions(auditTrail);
+            assertAuthenticationException(e);
+        }
+    }
+
+    public void testWrongApiKeyDoesNotFallbackToAnonymous() {
+        String username = randomBoolean() ? AnonymousUser.DEFAULT_ANONYMOUS_USERNAME : "user1";
+        Settings.Builder builder = Settings.builder()
+            .putList(AnonymousUser.ROLES_SETTING.getKey(), "r1", "r2", "r3");
+        if (username.equals(AnonymousUser.DEFAULT_ANONYMOUS_USERNAME) == false) {
+            builder.put(AnonymousUser.USERNAME_SETTING.getKey(), username);
+        }
+        Settings anonymousEnabledSettings = builder.build();
+        final AnonymousUser anonymousUser = new AnonymousUser(anonymousEnabledSettings);
+        service = new AuthenticationService(anonymousEnabledSettings, realms, auditTrail,
+            new DefaultAuthenticationFailureHandler(Collections.emptyMap()), threadPool, anonymousUser, tokenService, apiKeyService);
+        doAnswer(invocationOnMock -> {
+            final GetRequest request = (GetRequest) invocationOnMock.getArguments()[0];
+            final ActionListener<GetResponse> listener = (ActionListener<GetResponse>) invocationOnMock.getArguments()[1];
+            listener.onResponse(new GetResponse(new GetResult(request.index(), request.id(),
+                SequenceNumbers.UNASSIGNED_SEQ_NO, UNASSIGNED_PRIMARY_TERM, -1L, false, null,
+                Collections.emptyMap(), Collections.emptyMap())));
+            return Void.TYPE;
+        }).when(client).get(any(GetRequest.class), any(ActionListener.class));
+        try (ThreadContext.StoredContext ignore = threadContext.stashContext()) {
+            final String reqId = AuditUtil.getOrGenerateRequestId(threadContext);
+            threadContext.putHeader("Authorization", "ApiKey dGhpc2lzYW5pbnZhbGlkaWQ6dGhpc2lzYW5pbnZhbGlkc2VjcmV0");
+            ElasticsearchSecurityException e =
+                expectThrows(ElasticsearchSecurityException.class, () -> authenticateBlocking("_action", message, null));
+            verify(auditTrail).anonymousAccessDenied(reqId, "_action", message);
+            verifyNoMoreInteractions(auditTrail);
+            assertAuthenticationException(e);
+        }
+    }
+
     public void testAnonymousUserRest() throws Exception {
         String username = randomBoolean() ? AnonymousUser.DEFAULT_ANONYMOUS_USERNAME : "user1";
         Settings.Builder builder = Settings.builder()

From 2e97aade7f6fa113dab3745dcb0221bd29cdfbc2 Mon Sep 17 00:00:00 2001
From: Ioannis Kakavas <ioannis@elastic.co>
Date: Fri, 17 Jan 2020 16:14:44 +0200
Subject: [PATCH 2/2] Add type parameter to GetResult

---
 .../xpack/security/authc/AuthenticationServiceTests.java        | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java
index 7f7d00d61c0b6..0737087429d5a 100644
--- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java
+++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java
@@ -782,7 +782,7 @@ public void testWrongApiKeyDoesNotFallbackToAnonymous() {
         doAnswer(invocationOnMock -> {
             final GetRequest request = (GetRequest) invocationOnMock.getArguments()[0];
             final ActionListener<GetResponse> listener = (ActionListener<GetResponse>) invocationOnMock.getArguments()[1];
-            listener.onResponse(new GetResponse(new GetResult(request.index(), request.id(),
+            listener.onResponse(new GetResponse(new GetResult(request.index(), request.type(), request.id(),
                 SequenceNumbers.UNASSIGNED_SEQ_NO, UNASSIGNED_PRIMARY_TERM, -1L, false, null,
                 Collections.emptyMap(), Collections.emptyMap())));
             return Void.TYPE;