Skip to content

Fix maximum password length enforcement #44

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
patrickfav merged 1 commit into from
Feb 9, 2023
Merged

Fix maximum password length enforcement #44

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions modules/bcrypt/src/main/java/at/favre/lib/crypto/bcrypt/BCrypt.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 ?
Expand Down Expand Up @@ -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$
* <p>
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

/**
Expand Down
44 changes: 32 additions & 12 deletions modules/bcrypt/src/test/java/at/favre/lib/crypto/bcrypt/BcryptTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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
Expand All @@ -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
Expand Down