From 5f7fc0eb265f97fe720ec1ad6b4021001101a2a9 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Thu, 17 Jun 2021 09:14:05 -0600 Subject: [PATCH] Improve Upgrading Closes gh-11259 --- .../security/crypto/bcrypt/BCrypt.java | 47 ++++++++++++++----- .../bcrypt/BCryptPasswordEncoderTests.java | 14 ++++++ .../security/crypto/bcrypt/BCryptTests.java | 7 +++ 3 files changed, 55 insertions(+), 13 deletions(-) diff --git a/crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCrypt.java b/crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCrypt.java index 1b545e5977..559bcbcf24 100644 --- a/crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCrypt.java +++ b/crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCrypt.java @@ -526,35 +526,47 @@ public class BCrypt { * @param safety bit 16 is set when the safety measure is requested * @return an array containing the binary hashed password */ - private byte[] crypt_raw(byte password[], byte salt[], int log_rounds, boolean sign_ext_bug, int safety) { - int rounds, i, j; + private byte[] crypt_raw(byte password[], byte salt[], int log_rounds, boolean sign_ext_bug, int safety, + boolean for_check) { int cdata[] = bf_crypt_ciphertext.clone(); int clen = cdata.length; - byte ret[]; + long rounds; if (log_rounds < 4 || log_rounds > 31) { - throw new IllegalArgumentException("Bad number of rounds"); + if (!for_check) { + throw new IllegalArgumentException("Bad number of rounds"); + } + if (log_rounds != 0) { + throw new IllegalArgumentException("Bad number of rounds"); + } + rounds = 0; } - rounds = 1 << log_rounds; + else { + rounds = roundsForLogRounds(log_rounds); + if (rounds < 16 || rounds > Integer.MAX_VALUE) { + throw new IllegalArgumentException("Bad number of rounds"); + } + } + if (salt.length != BCRYPT_SALT_LEN) { throw new IllegalArgumentException("Bad salt length"); } init_key(); ekskey(salt, password, sign_ext_bug, safety); - for (i = 0; i < rounds; i++) { + for (int i = 0; i < rounds; i++) { key(password, sign_ext_bug, safety); key(salt, false, safety); } - for (i = 0; i < 64; i++) { - for (j = 0; j < (clen >> 1); j++) { + for (int i = 0; i < 64; i++) { + for (int j = 0; j < (clen >> 1); j++) { encipher(cdata, j << 1); } } - ret = new byte[clen * 4]; - for (i = 0, j = 0; i < clen; i++) { + byte[] ret = new byte[clen * 4]; + for (int i = 0, j = 0; i < clen; i++) { ret[j++] = (byte) ((cdata[i] >> 24) & 0xff); ret[j++] = (byte) ((cdata[i] >> 16) & 0xff); ret[j++] = (byte) ((cdata[i] >> 8) & 0xff); @@ -563,6 +575,10 @@ public class BCrypt { return ret; } + private static String hashpwforcheck(byte[] passwordb, String salt) { + return hashpw(passwordb, salt, true); + } + /** * Hash a password using the OpenBSD bcrypt scheme * @param password the password to hash @@ -584,6 +600,10 @@ public class BCrypt { * @return the hashed password */ public static String hashpw(byte passwordb[], String salt) { + return hashpw(passwordb, salt, false); + } + + private static String hashpw(byte passwordb[], String salt, boolean for_check) { BCrypt B; String real_salt; byte saltb[], hashed[]; @@ -633,7 +653,7 @@ public class BCrypt { } B = new BCrypt(); - hashed = B.crypt_raw(passwordb, saltb, rounds, minor == 'x', minor == 'a' ? 0x10000 : 0); + hashed = B.crypt_raw(passwordb, saltb, rounds, minor == 'x', minor == 'a' ? 0x10000 : 0, for_check); rs.append("$2"); if (minor >= 'a') { @@ -740,7 +760,8 @@ public class BCrypt { * @return true if the passwords match, false otherwise */ public static boolean checkpw(String plaintext, String hashed) { - return equalsNoEarlyReturn(hashed, hashpw(plaintext, hashed)); + byte[] passwordb = plaintext.getBytes(StandardCharsets.UTF_8); + return equalsNoEarlyReturn(hashed, hashpwforcheck(passwordb, hashed)); } /** @@ -751,7 +772,7 @@ public class BCrypt { * @since 5.3 */ public static boolean checkpw(byte[] passwordb, String hashed) { - return equalsNoEarlyReturn(hashed, hashpw(passwordb, hashed)); + return equalsNoEarlyReturn(hashed, hashpwforcheck(passwordb, hashed)); } static boolean equalsNoEarlyReturn(String a, String b) { diff --git a/crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoderTests.java b/crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoderTests.java index 9039dc9657..e4851ab1d2 100644 --- a/crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoderTests.java +++ b/crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoderTests.java @@ -208,4 +208,18 @@ public class BCryptPasswordEncoderTests { assertThatIllegalArgumentException().isThrownBy(() -> encoder.matches(null, "does-not-matter")); } + @Test + public void upgradeWhenNoRoundsThenTrue() { + BCryptPasswordEncoder encoder = new BCryptPasswordEncoder(); + assertThat(encoder.upgradeEncoding("$2a$00$9N8N35BVs5TLqGL3pspAte5OWWA2a2aZIs.EGp7At7txYakFERMue")).isTrue(); + } + + @Test + public void checkWhenNoRoundsThenTrue() { + BCryptPasswordEncoder encoder = new BCryptPasswordEncoder(); + assertThat(encoder.matches("password", "$2a$00$9N8N35BVs5TLqGL3pspAte5OWWA2a2aZIs.EGp7At7txYakFERMue")) + .isTrue(); + assertThat(encoder.matches("wrong", "$2a$00$9N8N35BVs5TLqGL3pspAte5OWWA2a2aZIs.EGp7At7txYakFERMue")).isFalse(); + } + } diff --git a/crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptTests.java b/crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptTests.java index 2e11b0fe4e..d1d60a1626 100644 --- a/crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptTests.java +++ b/crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptTests.java @@ -456,4 +456,11 @@ public class BCryptTests { assertThat(BCrypt.equalsNoEarlyReturn("test", "pass")).isFalse(); } + @Test + public void checkpwWhenZeroRoundsThenMatches() { + String password = "$2a$00$9N8N35BVs5TLqGL3pspAte5OWWA2a2aZIs.EGp7At7txYakFERMue"; + assertThat(BCrypt.checkpw("password", password)).isTrue(); + assertThat(BCrypt.checkpw("wrong", password)).isFalse(); + } + }