Skip to content

Commit

Permalink
Default resolveFromHash to Hasher.NOOP
Browse files Browse the repository at this point in the history
This changes the default behavior when resolving the hashing
algorithm from unrecognised hash strings, which was introduced in
 elastic#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 elastic#31697
Reverts 58cf95a
  • Loading branch information
jkakavas committed Jul 2, 2018
1 parent a7eaa40 commit 0434be4
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,8 @@ public static Hasher resolve(String name) {

/**
* 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
Expand All @@ -457,7 +458,8 @@ public static Hasher resolveFromHash(char[] hash) {
} 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;
}
}

Expand All @@ -471,13 +473,8 @@ public static Hasher resolveFromHash(char[] hash) {
* @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;
}
final Hasher hasher = resolveFromHash(hash);
return hasher.verify(data, hash);
}

private static char[] getPbkdf2Hash(SecureString data, int cost) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,11 @@ public void testAuthenticate() throws Exception {
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"});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,7 @@ public void testResolveFromHash() {
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) {
Expand Down

0 comments on commit 0434be4

Please sign in to comment.