Skip to content

Reduce api key verification cost #112878

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

Reduce api key verification cost #112878

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c967d3a
Use ssha256 by default
albertzaharovits Sep 13, 2024
4e4a867
Extra 6 bits
albertzaharovits Sep 13, 2024
b1dfe28
18 byte count for non-cross-cluster API keys
albertzaharovits Sep 13, 2024
76a041b
allow ssha256
albertzaharovits Sep 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/common/SecureRandomHolder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

import java.security.SecureRandom;

class SecureRandomHolder {
public class SecureRandomHolder {
// class loading is atomic - this is a lazy & safe singleton to be used by this package
public static final SecureRandom INSTANCE = new SecureRandom();
}
2 changes: 1 addition & 1 deletion ...plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/Hasher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,7 @@ public static List<String> getAvailableAlgoStoredHash() {
return Arrays.stream(Hasher.values())
.map(Hasher::name)
.map(name -> name.toLowerCase(Locale.ROOT))
.filter(name -> (name.startsWith("pbkdf2") || name.startsWith("bcrypt")))
.filter(name -> (name.startsWith("pbkdf2") || name.startsWith("bcrypt") || "ssha256".equals(name)))
Copy link
Contributor Author

@albertzaharovits albertzaharovits Sep 13, 2024

Choose a reason for hiding this comment

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

This needs to be allowed only for API keys, not for passwords (api keys are random long secrets).

.collect(Collectors.toList());
}

Expand Down
26 changes: 23 additions & 3 deletions ...k/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@
import org.elasticsearch.client.internal.Client;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.SecureRandomHolder;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.UUIDs;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.cache.Cache;
Expand Down Expand Up @@ -160,7 +160,7 @@ public class ApiKeyService implements Closeable {

public static final Setting<String> PASSWORD_HASHING_ALGORITHM = XPackSettings.defaultStoredHashAlgorithmSetting(
"xpack.security.authc.api_key.hashing.algorithm",
(s) -> Hasher.PBKDF2.name()
(s) -> Hasher.SSHA256.name()
Copy link
Contributor Author

@albertzaharovits albertzaharovits Sep 13, 2024

Choose a reason for hiding this comment

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

There is currently no PBKDF2 option with cost lower than 1000. If we add a new hasher type/cost we need to worry about BWC (old node not understanding keys generated by new nodes).

);
public static final Setting<TimeValue> DELETE_TIMEOUT = Setting.timeSetting(
"xpack.security.authc.api_key.delete.timeout",
Expand Down Expand Up @@ -541,7 +541,9 @@ private void createApiKeyAndIndexIt(
) {
final Instant created = clock.instant();
final Instant expiration = getApiKeyExpiration(created, request.getExpiration());
final SecureString apiKey = UUIDs.randomBase64UUIDSecureString();
// the difference between 16 and 18 effectively results in the same "encoded" API Key that's sent in HTTP request headers,
// dues to base64 padding
final SecureString apiKey = getBase64SecureRandomString(request.getType() == ApiKey.Type.CROSS_CLUSTER ? 16 : 18);
assert ApiKey.Type.CROSS_CLUSTER != request.getType() || API_KEY_SECRET_LENGTH == apiKey.length()
: "Invalid API key (name=[" + request.getName() + "], type=[" + request.getType() + "], length=[" + apiKey.length() + "])";
Comment on lines +544 to 548
Copy link
Contributor Author

@albertzaharovits albertzaharovits Sep 13, 2024

Choose a reason for hiding this comment

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

As @n1v0lg pointed out, we'd need to support the current length for cross cluster API keys.
Maybe other (external, i.e. non-ES) consumers rely on the "api_key" length not changing, but hopefully they use the "encoded" one in the API response, which doesn't change.

Copy link
Contributor Author

@albertzaharovits albertzaharovits Sep 13, 2024

Choose a reason for hiding this comment

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

e.g.:

{
  "id" : "bZ0Y7JEBuvKKBRFbe4xD",
  "name" : "test",
  "api_key" : "BGEQu0kTIU8x7b63k9Np41dE",
  "encoded" : "YlowWTdKRUJ1dktLQlJGYmU0eEQ6QkdFUXUwa1RJVTh4N2I2M2s5TnA0MWRF"
}

The encoded is still 60 chars, but api_key changes from 22 to 24.


Expand Down Expand Up @@ -2725,4 +2727,22 @@ public void invalidateAll() {
roleDescriptorsBytesCache.invalidateAll();
}
}

private static SecureString getBase64SecureRandomString(int randomBytesCount) {
byte[] randomBytes = null;
byte[] encodedBytes = null;
try {
randomBytes = new byte[randomBytesCount];
SecureRandomHolder.INSTANCE.nextBytes(randomBytes);
encodedBytes = Base64.getUrlEncoder().withoutPadding().encode(randomBytes);
return new SecureString(CharArrays.utf8BytesToChars(encodedBytes));
} finally {
if (randomBytes != null) {
Arrays.fill(randomBytes, (byte) 0);
}
if (encodedBytes != null) {
Arrays.fill(encodedBytes, (byte) 0);
}
}
}
}