From d77987ee8a4e97e9c96526d5110cd2e85a033529 Mon Sep 17 00:00:00 2001 From: Thomas Quinot Date: Mon, 6 Feb 2023 23:27:38 +0100 Subject: [PATCH] Fix maximum password length enforcement NUL terminators play a role in keying only if the password is shorter than 72 bytes. For a password that is exactly 72 bytes, no cyclic repetition occurs in the key expansion phase, and no NUL is needed: the password can be used as-is; in other words, the NUL terminator should not be counted against the 72 bytes limit. Adjust test cases accordingly. Test vectors in testReferenceValuesWithoutNullTerminator have been tested against the Python bcrypt module. --- .../at/favre/lib/crypto/bcrypt/BCrypt.java | 19 ++++---- .../favre/lib/crypto/bcrypt/BcryptTest.java | 44 ++++++++++++++----- 2 files changed, 42 insertions(+), 21 deletions(-) diff --git a/modules/bcrypt/src/main/java/at/favre/lib/crypto/bcrypt/BCrypt.java b/modules/bcrypt/src/main/java/at/favre/lib/crypto/bcrypt/BCrypt.java index 2bfc361..ac7a3d7 100644 --- a/modules/bcrypt/src/main/java/at/favre/lib/crypto/bcrypt/BCrypt.java +++ b/modules/bcrypt/src/main/java/at/favre/lib/crypto/bcrypt/BCrypt.java @@ -299,11 +299,14 @@ public HashData hashRaw(int cost, byte[] salt, byte[] password) { throw new IllegalArgumentException("provided password must at least be length 1 if no null terminator is appended"); } - if (password.length > version.allowedMaxPwLength + (version.appendNullTerminator ? 0 : 1)) { + if (password.length > version.allowedMaxPwLength) { password = longPasswordStrategy.derive(password); } byte[] pwWithNullTerminator = version.appendNullTerminator ? Bytes.wrap(password).append((byte) 0).array() : Bytes.wrap(password).copy().array(); + // Note: pwWithNullTerminator may be longer than allowedMaxPwLength, which is not an issue because the NUL terminator + // only plays a part with shorter passwords that need to be cyclically repeated to provide the 72 bytes of keying + // material required by EksBlowfish key expansion. try { byte[] hash = new BCryptOpenBSDProtocol().cryptRaw(1L << (long) cost, salt, pwWithNullTerminator); return new HashData(cost, version, salt, version.useOnly23bytesForHash ? @@ -701,15 +704,13 @@ public static final class Version { private static final BCryptParser DEFAULT_PARSER = new BCryptParser.Default(new Radix64Encoder.Default(), BCrypt.DEFAULT_CHARSET); /** - * Absolutely maximum length bcrypt can support (18x32bit) + * Absolutely maximum length bcrypt can support (18x32bit). + * Shorter passwords are repeated cyclically, possibly with NUL bytes separating each occurrence + * (but the NUL byte should not count against the limite: if the password is exactly this length + * the all characters are used and no repetition needs to happen, so no NUL needs to be inserted). */ public static final int MAX_PW_LENGTH_BYTE = 72; - /** - * The max length of the password in bytes excluding lats null-terminator byte - */ - public static final int DEFAULT_MAX_PW_LENGTH_BYTE = MAX_PW_LENGTH_BYTE - 1; - /** * $2a$ *

@@ -759,7 +760,7 @@ public static final class Version { * This mirrors how Bouncy Castle creates bcrypt hashes: with 24 byte out and without null-terminator. Gets a fake * version descriptor. */ - public static final Version VERSION_BC = new Version(new byte[]{MAJOR_VERSION, 0x63}, false, false, DEFAULT_MAX_PW_LENGTH_BYTE, DEFAULT_FORMATTER, DEFAULT_PARSER); + public static final Version VERSION_BC = new Version(new byte[]{MAJOR_VERSION, 0x63}, false, false, MAX_PW_LENGTH_BYTE, DEFAULT_FORMATTER, DEFAULT_PARSER); /** * List of supported versions @@ -801,7 +802,7 @@ public static final class Version { public final BCryptParser parser; private Version(byte[] versionIdentifier, BCryptFormatter formatter, BCryptParser parser) { - this(versionIdentifier, true, true, DEFAULT_MAX_PW_LENGTH_BYTE, formatter, parser); + this(versionIdentifier, true, true, MAX_PW_LENGTH_BYTE, formatter, parser); } /** diff --git a/modules/bcrypt/src/test/java/at/favre/lib/crypto/bcrypt/BcryptTest.java b/modules/bcrypt/src/test/java/at/favre/lib/crypto/bcrypt/BcryptTest.java index 1978daa..3cb51cd 100644 --- a/modules/bcrypt/src/test/java/at/favre/lib/crypto/bcrypt/BcryptTest.java +++ b/modules/bcrypt/src/test/java/at/favre/lib/crypto/bcrypt/BcryptTest.java @@ -401,15 +401,15 @@ public void testHashDataWipe() { @Test public void testVersionPojoMethods() { assertEquals(BCrypt.Version.VERSION_2A, BCrypt.Version.VERSION_2A); - assertEquals(BCrypt.Version.VERSION_2A, new BCrypt.Version(new byte[]{MAJOR_VERSION, 0x61}, true, true, BCrypt.Version.DEFAULT_MAX_PW_LENGTH_BYTE, null, null)); - assertEquals(BCrypt.Version.VERSION_2Y, new BCrypt.Version(new byte[]{MAJOR_VERSION, 0x79}, true, true, BCrypt.Version.DEFAULT_MAX_PW_LENGTH_BYTE, null, null)); + assertEquals(BCrypt.Version.VERSION_2A, new BCrypt.Version(new byte[]{MAJOR_VERSION, 0x61}, true, true, BCrypt.Version.MAX_PW_LENGTH_BYTE, null, null)); + assertEquals(BCrypt.Version.VERSION_2Y, new BCrypt.Version(new byte[]{MAJOR_VERSION, 0x79}, true, true, BCrypt.Version.MAX_PW_LENGTH_BYTE, null, null)); assertEquals(BCrypt.Version.VERSION_2Y_NO_NULL_TERMINATOR, new BCrypt.Version(new byte[]{MAJOR_VERSION, 0x79}, true, false, BCrypt.Version.MAX_PW_LENGTH_BYTE, null, null)); assertNotEquals(BCrypt.Version.VERSION_2Y, BCrypt.Version.VERSION_2A); assertNotEquals(BCrypt.Version.VERSION_2A, BCrypt.Version.VERSION_2B); assertNotEquals(BCrypt.Version.VERSION_2X, BCrypt.Version.VERSION_2Y); assertEquals(BCrypt.Version.VERSION_2A.hashCode(), BCrypt.Version.VERSION_2A.hashCode()); - assertEquals(BCrypt.Version.VERSION_2A.hashCode(), new BCrypt.Version(new byte[]{MAJOR_VERSION, 0x61}, true, true, BCrypt.Version.DEFAULT_MAX_PW_LENGTH_BYTE, null, null).hashCode()); + assertEquals(BCrypt.Version.VERSION_2A.hashCode(), new BCrypt.Version(new byte[]{MAJOR_VERSION, 0x61}, true, true, BCrypt.Version.MAX_PW_LENGTH_BYTE, null, null).hashCode()); assertNotEquals(BCrypt.Version.VERSION_2Y.hashCode(), BCrypt.Version.VERSION_2A.hashCode()); assertNotEquals(BCrypt.Version.VERSION_2A.hashCode(), BCrypt.Version.VERSION_2B.hashCode()); @@ -424,7 +424,11 @@ public void testVerifierWithLongPasswordStrategy() { byte[] hash = BCrypt.with(truncate).hash(4, pw); assertTrue(BCrypt.verifyer(BCrypt.Version.VERSION_2A, truncate).verify(pw, hash).verified); - assertFalse(BCrypt.verifyer(BCrypt.Version.VERSION_2A, LongPasswordStrategies.none()).verify(pw, hash).verified); + assertTrue(BCrypt.verifyer(BCrypt.Version.VERSION_2A, LongPasswordStrategies.none()).verify(pw, hash).verified); + // Strategies "truncate" and "none" are equivalent because key expansion will just ignore extra data. + + assertFalse(BCrypt.verifyer(BCrypt.Version.VERSION_2A, LongPasswordStrategies.hashSha512(BCrypt.Version.VERSION_2A)).verify(pw, hash).verified); + // The SHA-512 strategy yields a different hash } @Test @@ -443,26 +447,42 @@ public void testWithNullTerminatorWithinPw_shouldNotTerminate() { @Test public void testVersionWithNullTerminator() { - char[] pw = "myverlongpasswordthatisatleast72charslongandlongnothisisnotlongenoughyou".toCharArray(); - assertEquals(72, pw.length); - assertEquals(72, Bytes.from(pw).length()); + char[] pw71 = "myverlongpasswordthatisatleast72charslongandlongnothisisnotlongenoughyo".toCharArray(); + assertEquals(71, pw71.length); + assertEquals(71, Bytes.from(pw71).length()); + // For a 71 characters password, with null terminator there is no repetition, whereas without null terminator + // the first character is used again as 72th byte. byte[] salt = Bytes.random(16).array(); - byte[] hash1 = BCrypt.with(BCrypt.Version.VERSION_2Y_NO_NULL_TERMINATOR, LongPasswordStrategies.truncate(BCrypt.Version.VERSION_2Y_NO_NULL_TERMINATOR)).hash(4, salt, Bytes.from(pw).array()); - byte[] hash2 = BCrypt.with(BCrypt.Version.VERSION_2Y, LongPasswordStrategies.truncate(BCrypt.Version.VERSION_2Y)).hash(4, salt, Bytes.from(pw).array()); + byte[] hash71_1 = BCrypt.with(BCrypt.Version.VERSION_2Y_NO_NULL_TERMINATOR, LongPasswordStrategies.truncate(BCrypt.Version.VERSION_2Y_NO_NULL_TERMINATOR)).hash(4, salt, Bytes.from(pw71).array()); + byte[] hash71_2 = BCrypt.with(BCrypt.Version.VERSION_2Y, LongPasswordStrategies.truncate(BCrypt.Version.VERSION_2Y)).hash(4, salt, Bytes.from(pw71).array()); + assertNotEquals(Bytes.wrap(hash71_1).encodeUtf8(), Bytes.wrap(hash71_2).encodeUtf8()); + - assertNotEquals(Bytes.wrap(hash1).encodeUtf8(), Bytes.wrap(hash2).encodeUtf8()); - System.out.println(Bytes.wrap(hash1).encodeUtf8() + "\n" + Bytes.wrap(hash2).encodeUtf8()); + char[] pw72 = "myverlongpasswordthatisatleast72charslongandlongnothisisnotlongenoughyom".toCharArray(); + assertEquals(72, pw72.length); + assertEquals(72, Bytes.from(pw72).length()); + + byte[] hash72_1 = BCrypt.with(BCrypt.Version.VERSION_2Y_NO_NULL_TERMINATOR, LongPasswordStrategies.truncate(BCrypt.Version.VERSION_2Y_NO_NULL_TERMINATOR)).hash(4, salt, Bytes.from(pw72).array()); + byte[] hash72_2 = BCrypt.with(BCrypt.Version.VERSION_2Y, LongPasswordStrategies.truncate(BCrypt.Version.VERSION_2Y)).hash(4, salt, Bytes.from(pw72).array()); + assertEquals(Bytes.wrap(hash72_1).encodeUtf8(), Bytes.wrap(hash72_2).encodeUtf8()); + + assertEquals(Bytes.wrap(hash71_1).encodeUtf8(), Bytes.wrap(hash72_1).encodeUtf8()); + System.out.println(Bytes.wrap(hash71_1).encodeUtf8() + "\n" + Bytes.wrap(hash71_2).encodeUtf8()); } @Test public void testReferenceValuesWithoutNullTerminator() { char[] pw = "myverlongpasswordthatisatleast72charslongandlongnothisisnotlongenoughyou".toCharArray(); + assertEquals(72, pw.length); + char[] pw71 = new char[71]; + System.arraycopy(pw, 0, pw71, 0, 71); assertTrue(BCrypt.verifyer(BCrypt.Version.VERSION_2Y_NO_NULL_TERMINATOR).verify(pw, "$2y$04$d4CIUbwyucxm87BQnDWyI.xHDm2vyIZfBDOzjASNkn/yB.6lzLwOG".toCharArray()).verified); assertTrue(BCrypt.verifyer(BCrypt.Version.VERSION_2Y_NO_NULL_TERMINATOR).verify(pw, "$2y$04$w8S7HTjIfG.8RRVOhLZWtuH6eei2l7NZ/VhYUrDJndAjDmOqK6E0W".toCharArray()).verified); - assertTrue(BCrypt.verifyer(BCrypt.Version.VERSION_2Y, LongPasswordStrategies.truncate(BCrypt.Version.VERSION_2Y)).verify(pw, "$2y$04$w8S7HTjIfG.8RRVOhLZWtu//55gj0VTX7XdNkQmDuPw.qQXsnvtkG".toCharArray()).verified); + assertTrue(BCrypt.verifyer(BCrypt.Version.VERSION_2Y, LongPasswordStrategies.truncate(BCrypt.Version.VERSION_2Y)).verify(pw, "$2y$04$w8S7HTjIfG.8RRVOhLZWtuH6eei2l7NZ/VhYUrDJndAjDmOqK6E0W".toCharArray()).verified); + assertTrue(BCrypt.verifyer(BCrypt.Version.VERSION_2Y, LongPasswordStrategies.truncate(BCrypt.Version.VERSION_2Y)).verify(pw71, "$2y$04$w8S7HTjIfG.8RRVOhLZWtu//55gj0VTX7XdNkQmDuPw.qQXsnvtkG".toCharArray()).verified); } @Test