Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement verification of API keys #35318

Merged
merged 9 commits into from
Nov 14, 2018
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,12 @@ private static Integer authSchemePriority(final String headerValue) {
return 0;
} else if (headerValue.regionMatches(true, 0, "bearer", 0, "bearer".length())) {
return 1;
} else if (headerValue.regionMatches(true, 0, "basic", 0, "basic".length())) {
} else if (headerValue.regionMatches(true, 0, "apikey", 0, "apikey".length())) {
return 2;
} else {
} else if (headerValue.regionMatches(true, 0, "basic", 0, "basic".length())) {
return 3;
} else {
return 4;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,9 @@ public void testSortsWWWAuthenticateHeaderValues() {
final String basicAuthScheme = "Basic realm=\"" + XPackField.SECURITY + "\" charset=\"UTF-8\"";
final String bearerAuthScheme = "Bearer realm=\"" + XPackField.SECURITY + "\"";
final String negotiateAuthScheme = randomFrom("Negotiate", "Negotiate Ijoijksdk");
final String apiKeyAuthScheme = "ApiKey";
final Map<String, List<String>> failureResponeHeaders = new HashMap<>();
final List<String> supportedSchemes = Arrays.asList(basicAuthScheme, bearerAuthScheme, negotiateAuthScheme);
final List<String> supportedSchemes = Arrays.asList(basicAuthScheme, bearerAuthScheme, negotiateAuthScheme, apiKeyAuthScheme);
Collections.shuffle(supportedSchemes, random());
failureResponeHeaders.put("WWW-Authenticate", supportedSchemes);
final DefaultAuthenticationFailureHandler failuerHandler = new DefaultAuthenticationFailureHandler(failureResponeHeaders);
Expand All @@ -123,7 +124,7 @@ public void testSortsWWWAuthenticateHeaderValues() {
assertThat(ese, is(notNullValue()));
assertThat(ese.getHeader("WWW-Authenticate"), is(notNullValue()));
assertThat(ese.getMessage(), equalTo("error attempting to authenticate request"));
assertWWWAuthenticateWithSchemes(ese, negotiateAuthScheme, bearerAuthScheme, basicAuthScheme);
assertWWWAuthenticateWithSchemes(ese, negotiateAuthScheme, bearerAuthScheme, apiKeyAuthScheme, basicAuthScheme);
}

private void assertWWWAuthenticateWithSchemes(final ElasticsearchSecurityException ese, final String... schemes) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@
import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
import static org.elasticsearch.cluster.metadata.IndexMetaData.INDEX_FORMAT_SETTING;
import static org.elasticsearch.xpack.core.XPackSettings.API_KEY_SERVICE_ENABLED_SETTING;
import static org.elasticsearch.xpack.core.XPackSettings.HTTP_SSL_ENABLED;
import static org.elasticsearch.xpack.security.support.SecurityIndexManager.INTERNAL_INDEX_FORMAT;
import static org.elasticsearch.xpack.security.support.SecurityIndexManager.SECURITY_INDEX_NAME;
Expand Down Expand Up @@ -540,6 +541,13 @@ private AuthenticationFailureHandler createAuthenticationFailureHandler(final Re
defaultFailureResponseHeaders.get("WWW-Authenticate").add(bearerScheme);
}
}
if (API_KEY_SERVICE_ENABLED_SETTING.get(settings)) {
final String apiKeyScheme = "ApiKey";
if (defaultFailureResponseHeaders.computeIfAbsent("WWW-Authenticate", x -> new ArrayList<>())
.contains(apiKeyScheme) == false) {
defaultFailureResponseHeaders.get("WWW-Authenticate").add(apiKeyScheme);
}
}
failureHandler = new DefaultAuthenticationFailureHandler(defaultFailureResponseHeaders);
} else {
logger.debug("Using authentication failure handler from extension [" + extensionName + "]");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,32 +10,44 @@
import org.apache.logging.log4j.Logger;
import org.elasticsearch.Version;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.get.GetRequest;
import org.elasticsearch.action.get.GetResponse;
import org.elasticsearch.action.index.IndexAction;
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.client.Client;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.CharArrays;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.UUIDs;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.xpack.core.XPackSettings;
import org.elasticsearch.xpack.core.security.action.CreateApiKeyRequest;
import org.elasticsearch.xpack.core.security.action.CreateApiKeyResponse;
import org.elasticsearch.xpack.core.security.authc.Authentication;
import org.elasticsearch.xpack.core.security.authc.AuthenticationResult;
import org.elasticsearch.xpack.core.security.authc.support.Hasher;
import org.elasticsearch.xpack.core.security.user.User;
import org.elasticsearch.xpack.security.support.SecurityIndexManager;

import javax.crypto.SecretKeyFactory;
import java.io.Closeable;
import java.io.IOException;
import java.security.NoSuchAlgorithmException;
import java.time.Clock;
import java.time.Instant;
import java.util.Arrays;
import java.util.Base64;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.function.Function;
import java.util.stream.Collectors;

import static org.elasticsearch.xpack.core.ClientHelper.SECURITY_ORIGIN;
import static org.elasticsearch.xpack.core.ClientHelper.executeAsyncWithOrigin;
Expand Down Expand Up @@ -142,6 +154,122 @@ public void createApiKey(Authentication authentication, CreateApiKeyRequest requ
}
}

/**
* Checks for the presence of a {@code Authorization} header with a value that starts with
* {@code ApiKey }. If found this will attempt to authenticate the key.
*/
void authenticateWithApiKeyIfPresent(ThreadContext ctx, ActionListener<AuthenticationResult> listener) {
if (enabled) {
final ApiKeyCredentials credentials;
try {
credentials = getCredentialsFromHeader(ctx);
bizybot marked this conversation as resolved.
Show resolved Hide resolved
} catch (IllegalArgumentException iae) {
listener.onResponse(AuthenticationResult.unsuccessful(iae.getMessage(), iae));
return;
}

if (credentials != null) {
final GetRequest getRequest = client.prepareGet(SecurityIndexManager.SECURITY_INDEX_NAME, TYPE, credentials.getId())
.setFetchSource(true).request();
executeAsyncWithOrigin(ctx, SECURITY_ORIGIN, getRequest, ActionListener.<GetResponse>wrap(response -> {
if (response.isExists()) {
try (ApiKeyCredentials ignore = credentials) {
validateApiKeyCredentials(response.getSource(), credentials, clock, listener);
}
} else {
credentials.close();
listener.onResponse(AuthenticationResult.unsuccessful("unable to authenticate", null));
jaymode marked this conversation as resolved.
Show resolved Hide resolved
}
}, e -> {
credentials.close();
listener.onResponse(AuthenticationResult.unsuccessful("apikey auth encountered a failure", e));
}), client::get);
} else {
listener.onResponse(AuthenticationResult.notHandled());
}
} else {
listener.onResponse(AuthenticationResult.notHandled());
}
}

/**
* Validates the ApiKey using the source map
* @param source the source map from a get of the ApiKey document
* @param credentials the credentials provided by the user
* @param listener the listener to notify after verification
*/
static void validateApiKeyCredentials(Map<String, Object> source, ApiKeyCredentials credentials, Clock clock,
ActionListener<AuthenticationResult> listener) {
final String apiKeyHash = (String) source.get("api_key_hash");
if (apiKeyHash == null) {
throw new IllegalStateException("api key hash is missing");
}
final boolean verified = verifyKeyAgainstHash(apiKeyHash, credentials);

if (verified) {
final Long expirationEpochMilli = (Long) source.get("expiration_time");
if (expirationEpochMilli == null || Instant.ofEpochMilli(expirationEpochMilli).isAfter(clock.instant())) {
final String principal = Objects.requireNonNull((String) source.get("principal"));
final Map<String, Object> metadata = (Map<String, Object>) source.get("metadata");
final List<Map<String, Object>> roleDescriptors = (List<Map<String, Object>>) source.get("role_descriptors");
final String[] roleNames = roleDescriptors.stream()
.map(rdSource -> (String) rdSource.get("name"))
.collect(Collectors.toList())
.toArray(Strings.EMPTY_ARRAY);
final User apiKeyUser = new User(principal, roleNames, null, null, metadata, true);
listener.onResponse(AuthenticationResult.success(apiKeyUser));
} else {
listener.onResponse(AuthenticationResult.unsuccessful("api key is expired", null));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

. A conscious decision has been made to always validate the hash
and then check expiration. This is done to prevent leaking that a given
key has expired.

Can you please elaborate on your reasoning behind this? I don't necessarily see a value in masking whether the key is invalid or expired.

Also wouldn't the AuthenticationResult message leak that to the requester anyhow?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please elaborate on your reasoning behind this?

My reasoning is that an attacker can currently differentiate between a non-existent api key and an existing one based on timing, which allows the attacker to obtain the ID of a key that exists and then begin a brute force attack on a single ID. If they can also differentiate between one that exists and is expired, this helps them as well. Smart guesses/a vulnerability in our key generation process could wind up making the ability to find a non-expired ID useful to an attacker.

That said, if you still feel there is not much value in this I am happy to reconsider.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I missed the second question.

Also wouldn't the AuthenticationResult message leak that to the requester anyhow?

No, the authentication result message is used for logging and is not returned to the user.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please elaborate on your reasoning behind this?

That said, if you still feel there is not much value in this I am happy to reconsider.

Thanks for the clarification! No, I agree with you that this is a valuable protection layer, let's keep it !

}
} else {
listener.onResponse(AuthenticationResult.unsuccessful("invalid credentials", null));
}
}

/**
* Gets the API Key from the <code>Authorization</code> header if the header begins with
* <code>ApiKey </code>
*/
static ApiKeyCredentials getCredentialsFromHeader(ThreadContext threadContext) {
String header = threadContext.getHeader("Authorization");
if (Strings.hasText(header) && header.regionMatches(true, 0, "ApiKey ", 0, "ApiKey ".length())
bizybot marked this conversation as resolved.
Show resolved Hide resolved
&& header.length() > "ApiKey ".length()) {
final byte[] decodedApiKeyCredBytes = Base64.getDecoder().decode(header.substring("ApiKey ".length()));
char[] apiKeyCredChars = null;
try {
apiKeyCredChars = CharArrays.utf8BytesToChars(decodedApiKeyCredBytes);
int colonIndex = -1;
for (int i = 0; i < apiKeyCredChars.length; i++) {
if (apiKeyCredChars[i] == ':') {
colonIndex = i;
break;
}
}

if (colonIndex < 1) {
throw new IllegalArgumentException("invalid ApiKey value");
}
return new ApiKeyCredentials(new String(Arrays.copyOfRange(apiKeyCredChars, 0, colonIndex)),
new SecureString(Arrays.copyOfRange(apiKeyCredChars, colonIndex + 1, apiKeyCredChars.length)));
} finally {
if (apiKeyCredChars != null) {
Arrays.fill(apiKeyCredChars, (char) 0);
}
}
}
return null;
}

private static boolean verifyKeyAgainstHash(String apiKeyHash, ApiKeyCredentials credentials) {
final char[] apiKeyHashChars = apiKeyHash.toCharArray();
try {
Hasher hasher = Hasher.resolveFromHash(apiKeyHash.toCharArray());
return hasher.verify(credentials.getKey(), apiKeyHashChars);
} finally {
Arrays.fill(apiKeyHashChars, (char) 0);
}
}

private Instant getApiKeyExpiration(Instant now, CreateApiKeyRequest request) {
if (request.getExpiration() != null) {
return now.plusSeconds(request.getExpiration().getSeconds());
Expand All @@ -155,4 +283,28 @@ private void ensureEnabled() {
throw new IllegalStateException("tokens are not enabled");
}
}

// package private class for testing
static final class ApiKeyCredentials implements Closeable {
private final String id;
private final SecureString key;

ApiKeyCredentials(String id, SecureString key) {
this.id = id;
this.key = key;
}

String getId() {
return id;
}

SecureString getKey() {
return key;
}

@Override
public void close() {
key.close();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

package org.elasticsearch.xpack.security.authc;

import org.elasticsearch.action.support.PlainActionFuture;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.core.security.authc.AuthenticationResult;
import org.elasticsearch.xpack.core.security.authc.support.Hasher;

import java.nio.charset.StandardCharsets;
import java.time.Clock;
import java.time.temporal.ChronoUnit;
import java.util.Base64;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

import static org.hamcrest.Matchers.arrayContaining;
import static org.hamcrest.Matchers.is;

public class ApiKeyServiceTests extends ESTestCase {

public void testGetCredentialsFromThreadContext() {
ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
assertNull(ApiKeyService.getCredentialsFromHeader(threadContext));

final String apiKeyAuthScheme = randomFrom("apikey", "apiKey", "ApiKey", "APikey", "APIKEY");
final String id = randomAlphaOfLength(12);
final String key = randomAlphaOfLength(16);
String headerValue = apiKeyAuthScheme + " " + Base64.getEncoder().encodeToString((id + ":" + key).getBytes(StandardCharsets.UTF_8));

try (ThreadContext.StoredContext ignore = threadContext.stashContext()) {
threadContext.putHeader("Authorization", headerValue);
ApiKeyService.ApiKeyCredentials creds = ApiKeyService.getCredentialsFromHeader(threadContext);
assertNotNull(creds);
assertEquals(id, creds.getId());
assertEquals(key, creds.getKey().toString());
}

// missing space
headerValue = apiKeyAuthScheme + Base64.getEncoder().encodeToString((id + ":" + key).getBytes(StandardCharsets.UTF_8));
try (ThreadContext.StoredContext ignore = threadContext.stashContext()) {
threadContext.putHeader("Authorization", headerValue);
ApiKeyService.ApiKeyCredentials creds = ApiKeyService.getCredentialsFromHeader(threadContext);
assertNull(creds);
}

// missing colon
headerValue = apiKeyAuthScheme + " " + Base64.getEncoder().encodeToString((id + key).getBytes(StandardCharsets.UTF_8));
try (ThreadContext.StoredContext ignore = threadContext.stashContext()) {
threadContext.putHeader("Authorization", headerValue);
IllegalArgumentException e =
expectThrows(IllegalArgumentException.class, () -> ApiKeyService.getCredentialsFromHeader(threadContext));
assertEquals("invalid ApiKey value", e.getMessage());
}
}

public void testValidateApiKey() 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<String, Object> sourceMap = new HashMap<>();
sourceMap.put("api_key_hash", new String(hash));
sourceMap.put("principal", "test_user");
sourceMap.put("metadata", Collections.emptyMap());
sourceMap.put("role_descriptors", Collections.singletonList(Collections.singletonMap("name", "a role")));


ApiKeyService.ApiKeyCredentials creds =
new ApiKeyService.ApiKeyCredentials(randomAlphaOfLength(12), new SecureString(apiKey.toCharArray()));
PlainActionFuture<AuthenticationResult> future = new PlainActionFuture<>();
ApiKeyService.validateApiKeyCredentials(sourceMap, creds, Clock.systemUTC(), future);
AuthenticationResult result = future.get();
assertNotNull(result);
assertTrue(result.isAuthenticated());
assertThat(result.getUser().principal(), is("test_user"));
assertThat(result.getUser().roles(), arrayContaining("a role"));
assertThat(result.getUser().metadata(), is(Collections.emptyMap()));

sourceMap.put("expiration_time", Clock.systemUTC().instant().plus(1L, ChronoUnit.HOURS).toEpochMilli());
future = new PlainActionFuture<>();
ApiKeyService.validateApiKeyCredentials(sourceMap, creds, Clock.systemUTC(), future);
result = future.get();
assertNotNull(result);
assertTrue(result.isAuthenticated());
assertThat(result.getUser().principal(), is("test_user"));
assertThat(result.getUser().roles(), arrayContaining("a role"));
assertThat(result.getUser().metadata(), is(Collections.emptyMap()));

sourceMap.put("expiration_time", Clock.systemUTC().instant().minus(1L, ChronoUnit.HOURS).toEpochMilli());
future = new PlainActionFuture<>();
ApiKeyService.validateApiKeyCredentials(sourceMap, creds, Clock.systemUTC(), future);
result = future.get();
assertNotNull(result);
assertFalse(result.isAuthenticated());

sourceMap.remove("expiration_time");
creds = new ApiKeyService.ApiKeyCredentials(randomAlphaOfLength(12), new SecureString(randomAlphaOfLength(15).toCharArray()));
future = new PlainActionFuture<>();
ApiKeyService.validateApiKeyCredentials(sourceMap, creds, Clock.systemUTC(), future);
result = future.get();
assertNotNull(result);
assertFalse(result.isAuthenticated());
}
}