Skip to content
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.

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

CODEC-182 - Sha2Crypt: tidy up salt Javadoc and validation. #301

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Changes from 1 commit
Commits
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
Next Next commit
CODEC-182 - check salt prefix for consistency
  • Loading branch information
sebbASF committed Jul 30, 2024
commit a9af84752b585aaa952338083927c1bfc14db788
30 changes: 17 additions & 13 deletions src/main/java/org/apache/commons/codec/digest/Sha2Crypt.java
Original file line number Diff line number Diff line change
@@ -69,7 +69,7 @@ public class Sha2Crypt {

/** The pattern to match valid salt values. */
private static final Pattern SALT_PATTERN = Pattern
.compile("^\\$([56])\\$(rounds=(\\d+)\\$)?([\\.\\/a-zA-Z0-9]{1,16}).*");
.compile("^(\\$[56]\\$)(rounds=(\\d+)\\$)?([\\.\\/a-zA-Z0-9]{1,16}).*");

/**
* Generates a libc crypt() compatible "$5$" hash value with random salt.
@@ -98,9 +98,9 @@ public static String sha256Crypt(final byte[] keyBytes) {
* @param keyBytes
* plaintext to hash. Each array element is set to {@code 0} before returning.
* @param salt
* real salt value without prefix or "rounds=". The salt may be null, in which case a salt
* is generated for you using {@link SecureRandom}. If one does not want to use {@link SecureRandom},
* you can pass your own {@link Random} in {@link #sha256Crypt(byte[], String, Random)}.
* salt value including prefix ($5$ or $6$) and optionally "rounds=".
* The salt may be null, in which case a salt is generated for you using {@link SecureRandom}.
* Or you can pass your own {@link Random} in {@link #sha256Crypt(byte[], String, Random)}.
* @return complete hash value including salt
* @throws IllegalArgumentException
* if the salt does not match the allowed pattern
@@ -122,7 +122,8 @@ public static String sha256Crypt(final byte[] keyBytes, String salt) {
* @param keyBytes
* plaintext to hash. Each array element is set to {@code 0} before returning.
* @param salt
* real salt value without prefix or "rounds=".
* salt value including prefix ($5$ or $6$) and optionally "rounds=".
* The salt may be null, in which case a salt is generated for you using the provided random generator
* @param random
* the instance of {@link Random} to use for generating the salt.
* Consider using {@link SecureRandom} for more secure salts.
@@ -153,9 +154,9 @@ public static String sha256Crypt(final byte[] keyBytes, String salt, final Rando
* @param keyBytes
* plaintext to hash. Each array element is set to {@code 0} before returning.
* @param salt
* real salt value without prefix or "rounds="; may not be null
* salt value including prefix ($5$ or $6$) and optionally "rounds=". Not null.
* @param saltPrefix
* either $5$ or $6$
* either $5$ or $6$; must agree with prefix in salt itself
* @param blocksize
* a value that differs between $5$ and $6$
* @param algorithm
@@ -183,6 +184,10 @@ private static String sha2Crypt(final byte[] keyBytes, final String salt, final
if (!m.find()) {
throw new IllegalArgumentException("Invalid salt value: " + salt);
}
final String saltLeader = m.group(1);
if (!saltLeader.equals(saltPrefix)) {
throw new IllegalArgumentException("Expected salt to start with: " + saltPrefix + " but was: " + salt);
}
if (m.group(3) != null) {
rounds = Integer.parseInt(m.group(3));
rounds = Math.max(ROUNDS_MIN, Math.min(ROUNDS_MAX, rounds));
@@ -570,10 +575,9 @@ public static String sha512Crypt(final byte[] keyBytes) {
* @param keyBytes
* plaintext to hash. Each array element is set to {@code 0} before returning.
* @param salt
* real salt value without prefix or "rounds=". The salt may be null, in which case a salt is generated
* for you using {@link SecureRandom}; if you want to use a {@link Random} object other than
* {@link SecureRandom} then we suggest you provide it using
* {@link #sha512Crypt(byte[], String, Random)}.
* salt value including prefix ($5$ or $6$) and optionally "rounds=".
* The salt may be null, in which case a salt is generated for you using {@link SecureRandom}.
* Or you can pass your own {@link Random} to {@link #sha512Crypt(byte[], String, Random)}.
* @return complete hash value including salt
* @throws IllegalArgumentException
* if the salt does not match the allowed pattern
@@ -595,8 +599,8 @@ public static String sha512Crypt(final byte[] keyBytes, String salt) {
* @param keyBytes
* plaintext to hash. Each array element is set to {@code 0} before returning.
* @param salt
* real salt value without prefix or "rounds=". The salt may be null, in which case a salt
* is generated for you using {@link SecureRandom}.
* salt value including prefix ($5$ or $6$) and optionally "rounds=".
* The salt may be null, in which case a salt is generated for you using the provided random generator
* @param random
* the instance of {@link Random} to use for generating the salt.
* Consider using {@link SecureRandom} for more secure salts.
Original file line number Diff line number Diff line change
@@ -19,6 +19,7 @@
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertThrowsExactly;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.nio.charset.StandardCharsets;
@@ -103,4 +104,8 @@ public void testZeroOutInput() {
assertArrayEquals(new byte[buffer.length], buffer);
}

@Test
public void testCodec182() {
assertThrowsExactly(IllegalArgumentException.class, () -> Sha2Crypt.sha256Crypt("secret".getBytes(StandardCharsets.UTF_8), "$6$abcd$"));
}
}
Original file line number Diff line number Diff line change
@@ -19,6 +19,7 @@
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertThrowsExactly;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.nio.charset.StandardCharsets;
@@ -112,4 +113,8 @@ public void testZeroOutInput() {
assertArrayEquals(new byte[buffer.length], buffer);
}

@Test
public void testCodec182() {
assertThrowsExactly(IllegalArgumentException.class, () -> Sha2Crypt.sha512Crypt("secret".getBytes(StandardCharsets.UTF_8), "$5$abcd$"));
}
}