diff --git a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/BasicAuthUtils.java b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/BasicAuthUtils.java index 67e9c4055389..5169ae56d8e6 100644 --- a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/BasicAuthUtils.java +++ b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/BasicAuthUtils.java @@ -23,10 +23,8 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.base.Predicate; import org.apache.druid.java.util.common.ISE; -import org.apache.druid.java.util.common.RE; import org.apache.druid.java.util.common.RetryUtils; import org.apache.druid.java.util.common.StringUtils; -import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.security.basic.authentication.entity.BasicAuthenticatorUser; import org.apache.druid.security.basic.authorization.entity.BasicAuthorizerGroupMapping; import org.apache.druid.security.basic.authorization.entity.BasicAuthorizerRole; @@ -35,21 +33,15 @@ import org.apache.druid.security.basic.authorization.entity.UserAndRoleMap; import javax.annotation.Nullable; -import javax.crypto.SecretKey; -import javax.crypto.SecretKeyFactory; -import javax.crypto.spec.PBEKeySpec; import javax.servlet.http.HttpServletRequest; import java.io.IOException; -import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; -import java.security.spec.InvalidKeySpecException; import java.util.HashMap; import java.util.Map; public class BasicAuthUtils { - private static final Logger log = new Logger(BasicAuthUtils.class); private static final SecureRandom SECURE_RANDOM = new SecureRandom(); public static final String ADMIN_NAME = "admin"; public static final String ADMIN_GROUP_MAPPING_NAME = "adminGroupMapping"; @@ -66,8 +58,6 @@ public class BasicAuthUtils public static final int DEFAULT_CREDENTIAL_VERIFY_DURATION_SECONDS = 600; public static final int DEFAULT_CREDENTIAL_MAX_DURATION_SECONDS = 3600; public static final int DEFAULT_CREDENTIAL_CACHE_SIZE = 100; - public static final int KEY_LENGTH = 512; - public static final String ALGORITHM = "PBKDF2WithHmacSHA512"; public static final int MAX_INIT_RETRIES = 2; public static final Predicate SHOULD_RETRY_INIT = (throwable) -> throwable instanceof BasicSecurityDBResourceException; @@ -102,30 +92,6 @@ public class BasicAuthUtils { }; - public static byte[] hashPassword(final char[] password, final byte[] salt, final int iterations) - { - try { - SecretKeyFactory keyFactory = SecretKeyFactory.getInstance(ALGORITHM); - SecretKey key = keyFactory.generateSecret( - new PBEKeySpec( - password, - salt, - iterations, - KEY_LENGTH - ) - ); - return key.getEncoded(); - } - catch (InvalidKeySpecException ikse) { - log.error("Invalid keyspec"); - throw new RuntimeException("Invalid keyspec", ikse); - } - catch (NoSuchAlgorithmException nsae) { - log.error("%s not supported on this system.", ALGORITHM); - throw new RE(nsae, "%s not supported on this system.", ALGORITHM); - } - } - public static byte[] generateSalt() { byte[] salt = new byte[SALT_LENGTH]; diff --git a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/LdapUserPrincipal.java b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/LdapUserPrincipal.java index d185b02e0f89..bb8845b8625d 100644 --- a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/LdapUserPrincipal.java +++ b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/LdapUserPrincipal.java @@ -22,8 +22,8 @@ import com.google.common.annotations.VisibleForTesting; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.logger.Logger; -import org.apache.druid.security.basic.BasicAuthUtils; import org.apache.druid.security.basic.authentication.entity.BasicAuthenticatorCredentials; +import org.apache.druid.security.basic.authentication.validator.PasswordHashGenerator; import javax.naming.directory.SearchResult; import java.security.Principal; @@ -94,7 +94,7 @@ public Instant getLastVerified() public boolean hasSameCredentials(char[] password) { - byte[] recalculatedHash = BasicAuthUtils.hashPassword( + byte[] recalculatedHash = PasswordHashGenerator.computePasswordHash( password, this.credentials.getSalt(), this.credentials.getIterations() @@ -138,6 +138,7 @@ public String toString() name, searchResult, createdAt, - lastVerified); + lastVerified + ); } } diff --git a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/entity/BasicAuthenticatorCredentials.java b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/entity/BasicAuthenticatorCredentials.java index e31bb7ca0833..1a6b16b7ddf3 100644 --- a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/entity/BasicAuthenticatorCredentials.java +++ b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/entity/BasicAuthenticatorCredentials.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; import org.apache.druid.security.basic.BasicAuthUtils; +import org.apache.druid.security.basic.authentication.validator.PasswordHashGenerator; import java.util.Arrays; @@ -50,7 +51,7 @@ public BasicAuthenticatorCredentials(BasicAuthenticatorCredentialUpdate update) { this.iterations = update.getIterations(); this.salt = BasicAuthUtils.generateSalt(); - this.hash = BasicAuthUtils.hashPassword(update.getPassword().toCharArray(), salt, iterations); + this.hash = PasswordHashGenerator.computePasswordHash(update.getPassword().toCharArray(), salt, iterations); } @JsonProperty diff --git a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/validator/LDAPCredentialsValidator.java b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/validator/LDAPCredentialsValidator.java index 45b6801d30b9..226904165fad 100644 --- a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/validator/LDAPCredentialsValidator.java +++ b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/validator/LDAPCredentialsValidator.java @@ -46,7 +46,6 @@ import javax.naming.directory.SearchControls; import javax.naming.directory.SearchResult; import javax.naming.ldap.LdapName; - import java.util.HashMap; import java.util.LinkedHashMap; import java.util.Map; @@ -60,6 +59,7 @@ public class LDAPCredentialsValidator implements CredentialsValidator private static final ReentrantLock LOCK = new ReentrantLock(); private final LruBlockCache cache; + private final PasswordHashGenerator hashGenerator = new PasswordHashGenerator(); private final BasicAuthLDAPConfig ldapConfig; // Custom overrides that can be passed via tests @@ -199,7 +199,7 @@ public AuthenticationResult validateCredentials( } byte[] salt = BasicAuthUtils.generateSalt(); - byte[] hash = BasicAuthUtils.hashPassword(password, salt, this.ldapConfig.getCredentialIterations()); + byte[] hash = hashGenerator.getOrComputePasswordHash(password, salt, this.ldapConfig.getCredentialIterations()); LdapUserPrincipal newPrincipal = new LdapUserPrincipal( username, new BasicAuthenticatorCredentials(salt, hash, this.ldapConfig.getCredentialIterations()), diff --git a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/validator/MetadataStoreCredentialsValidator.java b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/validator/MetadataStoreCredentialsValidator.java index 0975c1da9432..80ead35c2382 100644 --- a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/validator/MetadataStoreCredentialsValidator.java +++ b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/validator/MetadataStoreCredentialsValidator.java @@ -25,7 +25,6 @@ import com.google.inject.Provider; import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.logger.Logger; -import org.apache.druid.security.basic.BasicAuthUtils; import org.apache.druid.security.basic.BasicSecurityAuthenticationException; import org.apache.druid.security.basic.authentication.db.cache.BasicAuthenticatorCacheManager; import org.apache.druid.security.basic.authentication.entity.BasicAuthenticatorCredentials; @@ -42,6 +41,7 @@ public class MetadataStoreCredentialsValidator implements CredentialsValidator { private static final Logger LOG = new Logger(MetadataStoreCredentialsValidator.class); private final Provider cacheManager; + private final PasswordHashGenerator hashGenerator = new PasswordHashGenerator(); @JsonCreator public MetadataStoreCredentialsValidator( @@ -74,7 +74,7 @@ public AuthenticationResult validateCredentials( return null; } - byte[] recalculatedHash = BasicAuthUtils.hashPassword( + byte[] recalculatedHash = hashGenerator.getOrComputePasswordHash( password, credentials.getSalt(), credentials.getIterations() diff --git a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/validator/PasswordHashGenerator.java b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/validator/PasswordHashGenerator.java new file mode 100644 index 000000000000..10b30540ef54 --- /dev/null +++ b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/validator/PasswordHashGenerator.java @@ -0,0 +1,163 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.security.basic.authentication.validator; + +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheStats; +import com.google.common.hash.Hashing; +import org.apache.druid.error.DruidException; +import org.apache.druid.java.util.common.RE; +import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.java.util.common.logger.Logger; +import org.apache.druid.security.basic.BasicAuthUtils; + +import javax.crypto.SecretKey; +import javax.crypto.SecretKeyFactory; +import javax.crypto.spec.PBEKeySpec; +import java.security.NoSuchAlgorithmException; +import java.security.spec.InvalidKeySpecException; +import java.time.Duration; +import java.util.Arrays; +import java.util.Objects; +import java.util.concurrent.ExecutionException; + +/** + * Generates a hash of passwords using the {@link #HASH_ALGORITHM}. Multiple + * iterations may be used to enhance security of the generated hash. + *

+ * Hashes once computed are cached for an hour so that they need not be recomputed + * for every API invocation. + */ +public class PasswordHashGenerator +{ + private static final Logger log = new Logger(PasswordHashGenerator.class); + + public static final int KEY_LENGTH = 512; + public static final String HASH_ALGORITHM = "PBKDF2WithHmacSHA512"; + + /** + * Salt used to compute a quick sha-256 hash of the password for the cache. + */ + private final byte[] shaSalt = BasicAuthUtils.generateSalt(); + + private final Cache cache = CacheBuilder.newBuilder() + .maximumSize(1000) + .recordStats() + .expireAfterAccess(Duration.ofMinutes(60)) + .build(); + + /** + * Hashes the given password using the {@link #HASH_ALGORITHM}. + */ + public byte[] getOrComputePasswordHash(char[] password, byte[] salt, int numIterations) + { + try { + return cache.get( + CacheKey.of(password, salt, numIterations, shaSalt), + () -> computePasswordHash(password, salt, numIterations) + ); + } + catch (ExecutionException e) { + throw DruidException.defensive().build(e, "Could not compute hash of password"); + } + } + + public CacheStats getCacheStats() + { + return cache.stats(); + } + + /** + * Utility method to compuate hash of the given password using the {@link #HASH_ALGORITHM}. + * Callers should use the non-static method instead to leverage caching. + */ + public static byte[] computePasswordHash(final char[] password, final byte[] salt, final int iterations) + { + try { + SecretKeyFactory keyFactory = SecretKeyFactory.getInstance(HASH_ALGORITHM); + SecretKey key = keyFactory.generateSecret(new PBEKeySpec(password, salt, iterations, KEY_LENGTH)); + return key.getEncoded(); + } + catch (InvalidKeySpecException ikse) { + log.error("Invalid keyspec"); + throw new RuntimeException("Invalid keyspec", ikse); + } + catch (NoSuchAlgorithmException nsae) { + log.error("Hash algorithm[%s] is not supported on this system.", HASH_ALGORITHM); + throw new RE(nsae, "Hash algorithm[%s] is not supported on this system.", HASH_ALGORITHM); + } + } + + /** + * Key used in the {@link #cache}. An SHA-256 hash of the password is used + * instead of the actual string so that the passwords are never exposed, even + * in a heap dump. + */ + private static class CacheKey + { + final byte[] passwordSha; + final byte[] salt; + final int numIterations; + + CacheKey(byte[] passwordSha, byte[] salt, int numIterations) + { + this.passwordSha = passwordSha; + this.salt = salt; + this.numIterations = numIterations; + } + + static CacheKey of(char[] password, byte[] salt, int numIterations, byte[] md5Salt) + { + @SuppressWarnings("UnstableApiUsage") + byte[] passwordSha = Hashing.sha256().newHasher() + .putBytes(StringUtils.toUtf8(new String(password))) + .putBytes(md5Salt) + .hash() + .asBytes(); + + return new CacheKey(passwordSha, salt, numIterations); + } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + CacheKey cacheKey = (CacheKey) o; + return numIterations == cacheKey.numIterations + && Arrays.equals(passwordSha, cacheKey.passwordSha) + && Arrays.equals(salt, cacheKey.salt); + } + + @Override + public int hashCode() + { + int result = Objects.hash(numIterations); + result = 31 * result + Arrays.hashCode(passwordSha); + result = 31 * result + Arrays.hashCode(salt); + return result; + } + } +} diff --git a/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/BasicAuthUtilsTest.java b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/BasicAuthUtilsTest.java index 60235fbf84bc..3aff7d003c24 100644 --- a/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/BasicAuthUtilsTest.java +++ b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/BasicAuthUtilsTest.java @@ -40,18 +40,6 @@ public class BasicAuthUtilsTest { - @Test - public void testHashPassword() - { - char[] password = "HELLO".toCharArray(); - int iterations = BasicAuthUtils.DEFAULT_KEY_ITERATIONS; - byte[] salt = BasicAuthUtils.generateSalt(); - byte[] hash = BasicAuthUtils.hashPassword(password, salt, iterations); - - Assert.assertEquals(BasicAuthUtils.SALT_LENGTH, salt.length); - Assert.assertEquals(BasicAuthUtils.KEY_LENGTH / 8, hash.length); - } - @Test public void testPermissionSerdeIsChillAboutUnknownEnumStuffs() throws JsonProcessingException { diff --git a/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/CoordinatorBasicAuthenticatorMetadataStorageUpdaterTest.java b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/CoordinatorBasicAuthenticatorMetadataStorageUpdaterTest.java index 256a415597cc..71b52087a228 100644 --- a/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/CoordinatorBasicAuthenticatorMetadataStorageUpdaterTest.java +++ b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/CoordinatorBasicAuthenticatorMetadataStorageUpdaterTest.java @@ -32,6 +32,7 @@ import org.apache.druid.security.basic.authentication.entity.BasicAuthenticatorCredentialUpdate; import org.apache.druid.security.basic.authentication.entity.BasicAuthenticatorCredentials; import org.apache.druid.security.basic.authentication.entity.BasicAuthenticatorUser; +import org.apache.druid.security.basic.authentication.validator.PasswordHashGenerator; import org.apache.druid.server.security.AuthenticatorMapper; import org.junit.After; import org.junit.Assert; @@ -163,7 +164,7 @@ public void setCredentials() ); BasicAuthenticatorCredentials credentials = userMap.get("druid").getCredentials(); - byte[] recalculatedHash = BasicAuthUtils.hashPassword( + byte[] recalculatedHash = PasswordHashGenerator.computePasswordHash( "helloworld".toCharArray(), credentials.getSalt(), credentials.getIterations() diff --git a/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/CoordinatorBasicAuthenticatorResourceTest.java b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/CoordinatorBasicAuthenticatorResourceTest.java index 5dec8646573b..1f5e968adbd4 100644 --- a/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/CoordinatorBasicAuthenticatorResourceTest.java +++ b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/CoordinatorBasicAuthenticatorResourceTest.java @@ -36,6 +36,7 @@ import org.apache.druid.security.basic.authentication.entity.BasicAuthenticatorCredentialUpdate; import org.apache.druid.security.basic.authentication.entity.BasicAuthenticatorCredentials; import org.apache.druid.security.basic.authentication.entity.BasicAuthenticatorUser; +import org.apache.druid.security.basic.authentication.validator.PasswordHashGenerator; import org.apache.druid.server.security.AuthConfig; import org.apache.druid.server.security.AuthValidator; import org.apache.druid.server.security.AuthenticationResult; @@ -354,10 +355,10 @@ public void testUserCredentials() byte[] hash = credentials.getHash(); int iterations = credentials.getIterations(); Assert.assertEquals(BasicAuthUtils.SALT_LENGTH, salt.length); - Assert.assertEquals(BasicAuthUtils.KEY_LENGTH / 8, hash.length); + Assert.assertEquals(PasswordHashGenerator.KEY_LENGTH / 8, hash.length); Assert.assertEquals(BasicAuthUtils.DEFAULT_KEY_ITERATIONS, iterations); - byte[] recalculatedHash = BasicAuthUtils.hashPassword( + byte[] recalculatedHash = PasswordHashGenerator.computePasswordHash( "helloworld".toCharArray(), salt, iterations @@ -377,10 +378,10 @@ public void testUserCredentials() hash = cachedUserCredentials.getHash(); iterations = cachedUserCredentials.getIterations(); Assert.assertEquals(BasicAuthUtils.SALT_LENGTH, salt.length); - Assert.assertEquals(BasicAuthUtils.KEY_LENGTH / 8, hash.length); + Assert.assertEquals(PasswordHashGenerator.KEY_LENGTH / 8, hash.length); Assert.assertEquals(BasicAuthUtils.DEFAULT_KEY_ITERATIONS, iterations); - recalculatedHash = BasicAuthUtils.hashPassword( + recalculatedHash = PasswordHashGenerator.computePasswordHash( "helloworld".toCharArray(), salt, iterations diff --git a/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/basic/authentication/validator/PasswordHashGeneratorTest.java b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/basic/authentication/validator/PasswordHashGeneratorTest.java new file mode 100644 index 000000000000..42bf6c161597 --- /dev/null +++ b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/basic/authentication/validator/PasswordHashGeneratorTest.java @@ -0,0 +1,70 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.security.basic.authentication.validator; + +import com.google.common.cache.CacheStats; +import org.apache.druid.security.basic.BasicAuthUtils; +import org.junit.Assert; +import org.junit.Test; + +public class PasswordHashGeneratorTest +{ + + @Test + public void testHashPassword() + { + char[] password = "HELLO".toCharArray(); + int iterations = BasicAuthUtils.DEFAULT_KEY_ITERATIONS; + byte[] salt = BasicAuthUtils.generateSalt(); + byte[] hash = PasswordHashGenerator.computePasswordHash(password, salt, iterations); + + Assert.assertEquals(BasicAuthUtils.SALT_LENGTH, salt.length); + Assert.assertEquals(PasswordHashGenerator.KEY_LENGTH / 8, hash.length); + } + + @Test(timeout = 60_000L) + public void testHashIsNotRecomputedWhenCached() + { + final PasswordHashGenerator hashGenerator = new PasswordHashGenerator(); + + final char[] password = "this_is_a_long_password".toCharArray(); + final int iterations = BasicAuthUtils.DEFAULT_KEY_ITERATIONS; + final byte[] salt = BasicAuthUtils.generateSalt(); + + final byte[] expectedHash = PasswordHashGenerator.computePasswordHash(password, salt, iterations); + + // Verify that the first computation takes a few ms + byte[] firstHash = hashGenerator.getOrComputePasswordHash(password, salt, iterations); + Assert.assertArrayEquals(expectedHash, firstHash); + CacheStats stats = hashGenerator.getCacheStats(); + Assert.assertEquals(0, stats.hitCount()); + Assert.assertEquals(1, stats.missCount()); + + // Verify that each subsequent computation takes less than 1ms + for (int i = 0; i < 10; ++i) { + byte[] recomputedHash = hashGenerator.getOrComputePasswordHash(password, salt, iterations); + Assert.assertArrayEquals(expectedHash, recomputedHash); + stats = hashGenerator.getCacheStats(); + Assert.assertEquals(i + 1, stats.hitCount()); + Assert.assertEquals(1, stats.missCount()); + } + } + +}