resolveHasher defaults to NOOP (#31723)

* Default resolveFromHash to Hasher.NOOP

This changes the default behavior when resolving the hashing
algorithm from unrecognised hash strings, which was introduced in
 #31234

A hash string that doesn't start with an algorithm identifier can
either be a malformed/corrupted hash or a plaintext password when
Hasher.NOOP is used(against warnings).
Do not make assumptions about which of the two is true for such
strings and default to Hasher.NOOP. Hash verification will subsequently
fail for malformed hashes.
Finally, do not log the potentially malformed hash as this can very
well be a plaintext password.

Resolves #31697
Reverts 58cf95a06f
This commit is contained in:
Ioannis Kakavas 2018-07-03 11:31:48 +03:00 committed by GitHub
parent 3d53daeb2f
commit 49b977ba7c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 7 additions and 15 deletions

View File

@ -438,7 +438,8 @@ public enum Hasher {
/**
* Returns a {@link Hasher} instance that can be used to verify the {@code hash} by inspecting the
* hash prefix and determining the algorithm used for its generation.
* hash prefix and determining the algorithm used for its generation. If no specific algorithm
* prefix, can be determined {@code Hasher.NOOP} is returned.
*
* @param hash the char array from which the hashing algorithm is to be deduced
* @return the hasher that can be used for validation
@ -457,7 +458,8 @@ public enum Hasher {
} else if (CharArrays.charsBeginsWith(SSHA256_PREFIX, hash)) {
return Hasher.SSHA256;
} else {
throw new IllegalArgumentException("unknown hash format for hash [" + new String(hash) + "]");
// This is either a non hashed password from cache or a corrupted hash string.
return Hasher.NOOP;
}
}
@ -471,13 +473,8 @@ public enum Hasher {
* @return true if the hash corresponds to the data, false otherwise
*/
public static boolean verifyHash(SecureString data, char[] hash) {
try {
final Hasher hasher = resolveFromHash(hash);
return hasher.verify(data, hash);
} catch (IllegalArgumentException e) {
// The password hash format is invalid, we're unable to verify password
return false;
}
}
private static char[] getPbkdf2Hash(SecureString data, int cost) {

View File

@ -84,13 +84,11 @@ public class FileRealmTests extends ESTestCase {
assertThat(user.roles(), arrayContaining("role1", "role2"));
}
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/31697")
public void testAuthenticateCaching() throws Exception {
Settings settings = Settings.builder()
.put("cache.hash_algo", Hasher.values()[randomIntBetween(0, Hasher.values().length - 1)].name().toLowerCase(Locale.ROOT)).build();
RealmConfig config = new RealmConfig("file-test", settings, globalSettings, TestEnvironment.newEnvironment(globalSettings),
threadContext);
when(userPasswdStore.verifyPassword(eq("user1"), eq(new SecureString("test123")), any(Supplier.class)))
.thenAnswer(VERIFY_PASSWORD_ANSWER);
when(userRolesStore.roles("user1")).thenReturn(new String[]{"role1", "role2"});

View File

@ -128,10 +128,7 @@ public class HasherTests extends ESTestCase {
assertThat(Hasher.resolveFromHash(
"{PBKDF2}1000000$UuyhtjDEzWmE2wyY80akZKPWWpy2r2X50so41YML82U=$WFasYLelqbjQwt3EqFlUcwHiC38EZC45Iu/Iz0xL1GQ=".toCharArray()),
sameInstance(Hasher.PBKDF2_1000000));
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> {
Hasher.resolveFromHash("{GBGN}cGR8S2vr3FuFuOpQitR".toCharArray());
});
assertThat(e.getMessage(), containsString("unknown hash format for hash"));
assertThat(Hasher.resolveFromHash("notavalidhashformat".toCharArray()), sameInstance(Hasher.NOOP));
}
private static void testHasherSelfGenerated(Hasher hasher) {