diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java index 5bf1569a9a293..d1e4644d1b532 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java @@ -302,27 +302,16 @@ void authenticateWithApiKeyIfPresent(ThreadContext ctx, ActionListenerwrap(response -> { - if (response.isExists()) { - try (ApiKeyCredentials ignore = credentials) { - final Map source = response.getSource(); - validateApiKeyCredentials(docId, source, credentials, clock, listener); - } - } else { + loadApiKeyAndValidateCredentials(ctx, credentials, ActionListener.wrap( + response -> { credentials.close(); - listener.onResponse( - AuthenticationResult.unsuccessful("unable to find apikey with id " + credentials.getId(), null)); + listener.onResponse(response); + }, + e -> { + credentials.close(); + listener.onFailure(e); } - }, e -> { - credentials.close(); - listener.onResponse(AuthenticationResult.unsuccessful("apikey authentication for id " + credentials.getId() + - " encountered a failure", e)); - }), client::get); + )); } else { listener.onResponse(AuthenticationResult.notHandled()); } @@ -331,6 +320,27 @@ void authenticateWithApiKeyIfPresent(ThreadContext ctx, ActionListener listener) { + final String docId = credentials.getId(); + final GetRequest getRequest = client + .prepareGet(SECURITY_MAIN_ALIAS, SINGLE_MAPPING_NAME, docId) + .setFetchSource(true) + .request(); + executeAsyncWithOrigin(ctx, SECURITY_ORIGIN, getRequest, ActionListener.wrap(response -> { + if (response.isExists()) { + final Map source = response.getSource(); + validateApiKeyCredentials(docId, source, credentials, clock, listener); + } else { + listener.onResponse( + AuthenticationResult.unsuccessful("unable to find apikey with id " + credentials.getId(), null)); + } + }, + e -> listener.onResponse(AuthenticationResult.unsuccessful( + "apikey authentication for id " + credentials.getId() + " encountered a failure", e))), + client::get); + } + /** * The current request has been authenticated by an API key and this method enables the * retrieval of role descriptors that are associated with the api key @@ -543,7 +553,8 @@ static ApiKeyCredentials getCredentialsFromHeader(ThreadContext threadContext) { return null; } - private static boolean verifyKeyAgainstHash(String apiKeyHash, ApiKeyCredentials credentials) { + // Protected instance method so this can be mocked + protected boolean verifyKeyAgainstHash(String apiKeyHash, ApiKeyCredentials credentials) { final char[] apiKeyHashChars = apiKeyHash.toCharArray(); try { Hasher hasher = Hasher.resolveFromHash(apiKeyHash.toCharArray()); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java index 88cbc0a80695d..38ab9a772f589 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java @@ -13,6 +13,7 @@ import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -41,6 +42,7 @@ import org.elasticsearch.xpack.security.test.SecurityMocks; import org.junit.After; import org.junit.Before; +import org.mockito.Mockito; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -54,6 +56,8 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.Semaphore; +import java.util.concurrent.atomic.AtomicInteger; import static org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR; import static org.hamcrest.Matchers.arrayContaining; @@ -431,16 +435,7 @@ public void testApiKeyCache() { Hasher hasher = randomFrom(Hasher.PBKDF2, Hasher.BCRYPT4, Hasher.BCRYPT); final char[] hash = hasher.hash(new SecureString(apiKey.toCharArray())); - Map sourceMap = new HashMap<>(); - sourceMap.put("doc_type", "api_key"); - sourceMap.put("api_key_hash", new String(hash)); - sourceMap.put("role_descriptors", Collections.singletonMap("a role", Collections.singletonMap("cluster", "all"))); - sourceMap.put("limited_by_role_descriptors", Collections.singletonMap("limited role", Collections.singletonMap("cluster", "all"))); - Map creatorMap = new HashMap<>(); - creatorMap.put("principal", "test_user"); - creatorMap.put("metadata", Collections.emptyMap()); - sourceMap.put("creator", creatorMap); - sourceMap.put("api_key_invalidated", false); + Map sourceMap = buildApiKeySourceDoc(hash); ApiKeyService service = createApiKeyService(Settings.EMPTY); ApiKeyCredentials creds = new ApiKeyCredentials(randomAlphaOfLength(12), new SecureString(apiKey.toCharArray())); @@ -488,6 +483,64 @@ public void testApiKeyCache() { assertThat(service.getFromCache(creds.getId()).success, is(true)); } + public void testAuthenticateWhileCacheBeingPopulated() throws Exception { + final String apiKey = randomAlphaOfLength(16); + Hasher hasher = randomFrom(Hasher.PBKDF2, Hasher.BCRYPT4, Hasher.BCRYPT); + final char[] hash = hasher.hash(new SecureString(apiKey.toCharArray())); + + Map sourceMap = buildApiKeySourceDoc(hash); + + ApiKeyService realService = createApiKeyService(Settings.EMPTY); + ApiKeyService service = Mockito.spy(realService); + + // Used to block the hashing of the first api-key secret so that we can guarantee + // that a second api key authentication takes place while hashing is "in progress". + final Semaphore hashWait = new Semaphore(0); + final AtomicInteger hashCounter = new AtomicInteger(0); + doAnswer(invocationOnMock -> { + hashCounter.incrementAndGet(); + hashWait.acquire(); + return invocationOnMock.callRealMethod(); + }).when(service).verifyKeyAgainstHash(any(String.class), any(ApiKeyCredentials.class)); + + final ApiKeyCredentials creds = new ApiKeyCredentials(randomAlphaOfLength(12), new SecureString(apiKey.toCharArray())); + final PlainActionFuture future1 = new PlainActionFuture<>(); + + // Call the top level authenticate... method because it has been known to be buggy in async situations + writeCredentialsToThreadContext(creds); + mockSourceDocument(creds.getId(), sourceMap); + + // This needs to be done in another thread, because we need it to not complete until we say so, but it should not block this test + this.threadPool.generic().execute(() -> service.authenticateWithApiKeyIfPresent(threadPool.getThreadContext(), future1)); + + // Wait for the first credential validation to get to the blocked state + assertBusy(() -> assertThat(hashCounter.get(), equalTo(1))); + if (future1.isDone()) { + // We do this [ rather than assertFalse(isDone) ] so we can get a reasonable failure message + fail("Expected authentication to be blocked, but was " + future1.actionGet()); + } + + // The second authentication should pass (but not immediately, but will not block) + PlainActionFuture future2 = new PlainActionFuture<>(); + + service.authenticateWithApiKeyIfPresent(threadPool.getThreadContext(), future2); + + assertThat(hashCounter.get(), equalTo(1)); + if (future2.isDone()) { + // We do this [ rather than assertFalse(isDone) ] so we can get a reasonable failure message + fail("Expected authentication to be blocked, but was " + future2.actionGet()); + } + + hashWait.release(); + + assertThat(future1.actionGet(TimeValue.timeValueSeconds(2)).isAuthenticated(), is(true)); + assertThat(future2.actionGet(TimeValue.timeValueMillis(100)).isAuthenticated(), is(true)); + + CachedApiKeyHashResult cachedApiKeyHashResult = service.getFromCache(creds.getId()); + assertNotNull(cachedApiKeyHashResult); + assertThat(cachedApiKeyHashResult.success, is(true)); + } + public void testApiKeyCacheDisabled() { final String apiKey = randomAlphaOfLength(16); Hasher hasher = randomFrom(Hasher.PBKDF2, Hasher.BCRYPT4, Hasher.BCRYPT); @@ -496,16 +549,7 @@ public void testApiKeyCacheDisabled() { .put(ApiKeyService.CACHE_TTL_SETTING.getKey(), "0s") .build(); - Map sourceMap = new HashMap<>(); - sourceMap.put("doc_type", "api_key"); - sourceMap.put("api_key_hash", new String(hash)); - sourceMap.put("role_descriptors", Collections.singletonMap("a role", Collections.singletonMap("cluster", "all"))); - sourceMap.put("limited_by_role_descriptors", Collections.singletonMap("limited role", Collections.singletonMap("cluster", "all"))); - Map creatorMap = new HashMap<>(); - creatorMap.put("principal", "test_user"); - creatorMap.put("metadata", Collections.emptyMap()); - sourceMap.put("creator", creatorMap); - sourceMap.put("api_key_invalidated", false); + Map sourceMap = buildApiKeySourceDoc(hash); ApiKeyService service = createApiKeyService(settings); ApiKeyCredentials creds = new ApiKeyCredentials(randomAlphaOfLength(12), new SecureString(apiKey.toCharArray())); @@ -517,10 +561,40 @@ public void testApiKeyCacheDisabled() { assertNull(cachedApiKeyHashResult); } - private ApiKeyService createApiKeyService(Settings settings) { + private ApiKeyService createApiKeyService(Settings baseSettings) { + final Settings settings = Settings.builder() + .put(XPackSettings.API_KEY_SERVICE_ENABLED_SETTING.getKey(), true) + .put(baseSettings) + .build(); return new ApiKeyService(settings, Clock.systemUTC(), client, licenseState, securityIndex, ClusterServiceUtils.createClusterService(threadPool), threadPool); } + private Map buildApiKeySourceDoc(char[] hash) { + Map sourceMap = new HashMap<>(); + sourceMap.put("doc_type", "api_key"); + sourceMap.put("api_key_hash", new String(hash)); + sourceMap.put("role_descriptors", Collections.singletonMap("a role", Collections.singletonMap("cluster", "all"))); + sourceMap.put("limited_by_role_descriptors", Collections.singletonMap("limited role", Collections.singletonMap("cluster", "all"))); + Map creatorMap = new HashMap<>(); + creatorMap.put("principal", "test_user"); + creatorMap.put("metadata", Collections.emptyMap()); + sourceMap.put("creator", creatorMap); + sourceMap.put("api_key_invalidated", false); + return sourceMap; + } + + private void writeCredentialsToThreadContext(ApiKeyCredentials creds) { + final String credentialString = creds.getId() + ":" + creds.getKey(); + this.threadPool.getThreadContext().putHeader("Authorization", + "ApiKey " + Base64.getEncoder().encodeToString(credentialString.getBytes(StandardCharsets.US_ASCII))); + } + + private void mockSourceDocument(String id, Map sourceMap) throws IOException { + try (XContentBuilder builder = JsonXContent.contentBuilder()) { + builder.map(sourceMap); + SecurityMocks.mockGetRequest(client, id, BytesReference.bytes(builder)); + } + } }