From 3b613c36f4da6a3c03ba5caebb68d26ae31a3ba6 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Wed, 11 Dec 2019 09:14:50 +0200 Subject: [PATCH] Always return 401 for not valid tokens (#49736) (#50042) Return a 401 in all cases when a request is submitted with an access token that we can't consume. Before this change, we would throw a 500 when a request came in with an access token that we had generated but was then invalidated/expired and deleted from the tokens index. Resolves: #38866 Backport of #49736 --- .../xpack/security/authc/TokenService.java | 8 +-- .../test/SecuritySettingsSource.java | 8 +++ .../security/authc/TokenAuthIntegTests.java | 52 ++++++++++++++++++- .../security/authc/TokenServiceTests.java | 40 +++++++++++++- 4 files changed, 103 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java index 0d5ca1fb275..50d5a87b9ad 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java @@ -422,7 +422,7 @@ public final class TokenService { } else { final GetRequest getRequest = client.prepareGet(tokensIndex.aliasName(), SINGLE_MAPPING_NAME, getTokenDocumentId(userTokenId)).request(); - final Consumer onFailure = ex -> listener.onFailure(traceLog("decode token", userTokenId, ex)); + final Consumer onFailure = ex -> listener.onFailure(traceLog("get token from id", userTokenId, ex)); tokensIndex.checkIndexVersionThenExecute( ex -> listener.onFailure(traceLog("prepare tokens index [" + tokensIndex.aliasName() +"]", userTokenId, ex)), () -> executeAsyncWithOrigin(client.threadPool().getThreadContext(), SECURITY_ORIGIN, getRequest, @@ -442,8 +442,10 @@ public final class TokenService { listener.onResponse(UserToken.fromSourceMap(userTokenSource)); } } else { - onFailure.accept( - new IllegalStateException("token document is missing and must be present")); + // The chances of a random token string decoding to something that we can read is minimal, so + // we assume that this was a token we have created but is now expired/revoked and deleted + logger.trace("The access token [{}] is expired and already deleted", userTokenId); + listener.onResponse(null); } }, e -> { // if the index or the shard is not there / available we assume that diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java index 491fd66c453..33477395056 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java @@ -7,6 +7,7 @@ package org.elasticsearch.test; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.analysis.common.CommonAnalysisPlugin; +import org.elasticsearch.client.RequestOptions; import org.elasticsearch.common.Strings; import org.elasticsearch.common.network.NetworkModule; import org.elasticsearch.common.settings.MockSecureSettings; @@ -30,11 +31,13 @@ import org.elasticsearch.xpack.security.audit.logfile.LoggingAuditTrail; import java.io.IOException; import java.io.InputStream; import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.util.ArrayList; import java.util.Arrays; +import java.util.Base64; import java.util.Collection; import java.util.List; import java.util.function.Consumer; @@ -57,6 +60,11 @@ public class SecuritySettingsSource extends NodeConfigurationSource { public static final String TEST_PASSWORD_HASHED = new String(Hasher.resolve(randomFrom("pbkdf2", "pbkdf2_1000", "bcrypt9", "bcrypt8", "bcrypt")). hash(new SecureString(SecuritySettingsSourceField.TEST_PASSWORD.toCharArray()))); + public static final RequestOptions SECURITY_REQUEST_OPTIONS = RequestOptions.DEFAULT.toBuilder() + .addHeader("Authorization", + "Basic " + Base64.getEncoder().encodeToString( + (TEST_USER_NAME + ":" + SecuritySettingsSourceField.TEST_PASSWORD).getBytes(StandardCharsets.UTF_8))) + .build(); public static final String TEST_ROLE = "user"; public static final String TEST_SUPERUSER = "test_superuser"; diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java index 07b37c1bf6e..61ee7e8c30f 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java @@ -8,14 +8,19 @@ package org.elasticsearch.xpack.security.authc; import org.apache.directory.api.util.Strings; import org.elasticsearch.ElasticsearchSecurityException; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.ElasticsearchStatusException; +import org.elasticsearch.Version; import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.action.support.WriteRequest; import org.elasticsearch.action.update.UpdateResponse; import org.elasticsearch.client.Client; +import org.elasticsearch.client.RequestOptions; +import org.elasticsearch.client.RestHighLevelClient; import org.elasticsearch.cluster.ack.ClusterStateUpdateResponse; import org.elasticsearch.common.settings.SecureString; +import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.index.query.QueryBuilders; @@ -53,6 +58,7 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; import static org.elasticsearch.index.mapper.MapperService.SINGLE_MAPPING_NAME; +import static org.elasticsearch.test.SecuritySettingsSource.SECURITY_REQUEST_OPTIONS; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoTimeout; import static org.hamcrest.Matchers.equalTo; @@ -75,6 +81,11 @@ public class TokenAuthIntegTests extends SecurityIntegTestCase { return defaultMaxNumberOfNodes() + 1; } + @Override + protected boolean addMockHttpTransport() { + return false; // need real http + } + public void testTokenServiceBootstrapOnNodeJoin() throws Exception { final Client client = client(); SecurityClient securityClient = new SecurityClient(client); @@ -186,6 +197,7 @@ public class TokenAuthIntegTests extends SecurityIntegTestCase { .actionGet(); } catch (ElasticsearchSecurityException e) { assertEquals("token malformed", e.getMessage()); + assertThat(e.status(), equalTo(RestStatus.UNAUTHORIZED)); } } client.admin().indices().prepareRefresh(RestrictedIndicesNames.SECURITY_TOKENS_ALIAS).get(); @@ -302,7 +314,6 @@ public class TokenAuthIntegTests extends SecurityIntegTestCase { assertNoTimeout(client() .filterWithHeader(Collections.singletonMap("Authorization", "Bearer " + createTokenResponse.getTokenString())) .admin().cluster().prepareHealth().get()); - CreateTokenResponse refreshResponse = securityClient.prepareRefreshToken(createTokenResponse.getRefreshToken()).get(); assertNotNull(refreshResponse.getRefreshToken()); assertNotEquals(refreshResponse.getRefreshToken(), createTokenResponse.getRefreshToken()); @@ -552,6 +563,36 @@ public class TokenAuthIntegTests extends SecurityIntegTestCase { }); } + public void testAuthenticateWithWrongToken() throws Exception { + final RestHighLevelClient restClient = new TestRestHighLevelClient(); + org.elasticsearch.client.security.CreateTokenResponse response = restClient.security().createToken( + org.elasticsearch.client.security.CreateTokenRequest.passwordGrant(SecuritySettingsSource.TEST_USER_NAME, + SecuritySettingsSourceField.TEST_PASSWORD.toCharArray()), SECURITY_REQUEST_OPTIONS); + assertNotNull(response.getAccessToken()); + // First authenticate with token + RequestOptions correctAuthOptions = + RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", "Bearer " + response.getAccessToken()).build(); + org.elasticsearch.client.security.AuthenticateResponse validResponse = restClient.security().authenticate(correctAuthOptions); + assertThat(validResponse.getUser().getUsername(), equalTo(SecuritySettingsSource.TEST_USER_NAME)); + // Now attempt to authenticate with an invalid access token string + RequestOptions wrongAuthOptions = + RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", "Bearer " + randomAlphaOfLengthBetween(0, 128)).build(); + ElasticsearchStatusException e = expectThrows(ElasticsearchStatusException.class, + () -> restClient.security().authenticate(wrongAuthOptions)); + assertThat(e.status(), equalTo(RestStatus.UNAUTHORIZED)); + // Now attempt to authenticate with an invalid access token with valid structure (pre 7.2) + RequestOptions wrongAuthOptionsPre72 = + RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", "Bearer " + generateAccessToken(Version.V_7_1_0)).build(); + ElasticsearchStatusException e1 = expectThrows(ElasticsearchStatusException.class, + () -> restClient.security().authenticate(wrongAuthOptionsPre72)); + assertThat(e1.status(), equalTo(RestStatus.UNAUTHORIZED)); + // Now attempt to authenticate with an invalid access token with valid structure (after 7.2) + RequestOptions wrongAuthOptionsAfter72 = + RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", "Bearer " + generateAccessToken(Version.V_7_4_0)).build(); + ElasticsearchStatusException e2 = expectThrows(ElasticsearchStatusException.class, + () -> restClient.security().authenticate(wrongAuthOptionsAfter72)); + assertThat(e2.status(), equalTo(RestStatus.UNAUTHORIZED)); + } @Before public void waitForSecurityIndexWritable() throws Exception { assertSecurityIndexActive(); @@ -570,4 +611,13 @@ public class TokenAuthIntegTests extends SecurityIntegTestCase { ClusterStateResponse clusterStateResponse = client().admin().cluster().prepareState().setCustoms(true).get(); assertFalse(clusterStateResponse.getState().customs().containsKey(TokenMetaData.TYPE)); } + + private String generateAccessToken(Version version) throws Exception { + TokenService tokenService = internalCluster().getInstance(TokenService.class); + String accessTokenString = UUIDs.randomBase64UUID(); + if (version.onOrAfter(TokenService.VERSION_ACCESS_TOKENS_AS_UUIDS)) { + accessTokenString = TokenService.hashTokenString(accessTokenString); + } + return tokenService.prependVersionAndEncodeAccessToken(version, accessTokenString); + } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java index 0f5325f8960..7fc958cfafb 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java @@ -600,7 +600,7 @@ public class TokenServiceTests extends ESTestCase { final int numBytes = randomIntBetween(1, TokenService.MINIMUM_BYTES + 32); final byte[] randomBytes = new byte[numBytes]; random().nextBytes(randomBytes); - TokenService tokenService = createTokenService(Settings.EMPTY, systemUTC()); + TokenService tokenService = createTokenService(tokenServiceEnabledSettings, systemUTC()); ThreadContext requestContext = new ThreadContext(Settings.EMPTY); storeTokenHeader(requestContext, Base64.getEncoder().encodeToString(randomBytes)); @@ -612,6 +612,36 @@ public class TokenServiceTests extends ESTestCase { } } + public void testNotValidPre72Tokens() throws Exception { + TokenService tokenService = createTokenService(tokenServiceEnabledSettings, systemUTC()); + // mock another random token so that we don't find a token in TokenService#getUserTokenFromId + Authentication authentication = new Authentication(new User("joe", "admin"), new RealmRef("native_realm", "native", "node1"), null); + mockGetTokenFromId(tokenService, UUIDs.randomBase64UUID(), authentication, false); + ThreadContext requestContext = new ThreadContext(Settings.EMPTY); + storeTokenHeader(requestContext, generateAccessToken(tokenService, Version.V_7_1_0)); + + try (ThreadContext.StoredContext ignore = requestContext.newStoredContext(true)) { + PlainActionFuture future = new PlainActionFuture<>(); + tokenService.getAndValidateToken(requestContext, future); + assertNull(future.get()); + } + } + + public void testNotValidAfter72Tokens() throws Exception { + TokenService tokenService = createTokenService(tokenServiceEnabledSettings, systemUTC()); + // mock another random token so that we don't find a token in TokenService#getUserTokenFromId + Authentication authentication = new Authentication(new User("joe", "admin"), new RealmRef("native_realm", "native", "node1"), null); + mockGetTokenFromId(tokenService, UUIDs.randomBase64UUID(), authentication, false); + ThreadContext requestContext = new ThreadContext(Settings.EMPTY); + storeTokenHeader(requestContext, generateAccessToken(tokenService, randomFrom(Version.V_7_2_0, Version.V_7_3_2))); + + try (ThreadContext.StoredContext ignore = requestContext.newStoredContext(true)) { + PlainActionFuture future = new PlainActionFuture<>(); + tokenService.getAndValidateToken(requestContext, future); + assertNull(future.get()); + } + } + public void testIndexNotAvailable() throws Exception { TokenService tokenService = createTokenService(tokenServiceEnabledSettings, systemUTC()); Authentication authentication = new Authentication(new User("joe", "admin"), new RealmRef("native_realm", "native", "node1"), null); @@ -823,4 +853,12 @@ public class TokenServiceTests extends ESTestCase { return anotherDataNode; } + private String generateAccessToken(TokenService tokenService, Version version) throws Exception { + String accessTokenString = UUIDs.randomBase64UUID(); + if (version.onOrAfter(TokenService.VERSION_ACCESS_TOKENS_AS_UUIDS)) { + accessTokenString = TokenService.hashTokenString(accessTokenString); + } + return tokenService.prependVersionAndEncodeAccessToken(version, accessTokenString); + } + }