This is an automated email from the ASF dual-hosted git repository. garydgregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-codec.git
commit 4f4a8a74fec32246d5e50559dc744bc19fc71b37 Author: Gary Gregory <[email protected]> AuthorDate: Sat Sep 19 06:01:10 2026 -0700 Bound Sha2Crypt password length to prevent quadratic CPU DoS Reject SHA-256/SHA-512 plaintexts over 4096 bytes by default to bound the quadratic SHA-crypt step, with a trusted JVM system-property override for compatibility. Document the limit in Sha2Crypt and Crypt, and add regression coverage for direct and Crypt entry points, boundary lengths, and override restoration. --- pom.xml | 5 ++ src/changes/changes.xml | 1 + .../org/apache/commons/codec/digest/Crypt.java | 4 ++ .../org/apache/commons/codec/digest/Sha2Crypt.java | 54 ++++++++++++++------- .../codec/digest/Sha2CryptKeyLengthTest.java | 56 ++++++++++++++++++++++ .../apache/commons/codec/digest/Sha2CryptTest.java | 14 +++--- 6 files changed, 111 insertions(+), 23 deletions(-) diff --git a/pom.xml b/pom.xml index 51997703..585c07bc 100644 --- a/pom.xml +++ b/pom.xml @@ -81,6 +81,11 @@ limitations under the License. <artifactId>junit-jupiter-params</artifactId> <scope>test</scope> </dependency> + <dependency> + <groupId>org.junit-pioneer</groupId> + <artifactId>junit-pioneer</artifactId> + <scope>test</scope> + </dependency> </dependencies> <properties> <maven.compiler.source>1.8</maven.compiler.source> diff --git a/src/changes/changes.xml b/src/changes/changes.xml index da5537ae..d80445e4 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -66,6 +66,7 @@ The <action> type attribute can be add,update,fix,remove. <action type="fix" dev="ggregory" due-to="Gary Gregory">Reject oversized Beider-Morse input before language guessing.</action> <action type="fix" dev="ggregory" due-to="Gary Gregory">QuotedPrintableCodec decoding now preserves hard CRLF line breaks and, leniently, unpaired CR and LF bytes instead of discarding them. Soft line breaks require the full =CRLF sequence; previously accepted =CR without LF now throws DecoderException. This affects both constructor modes and QCodec, which shares the decoder.</action> <action type="fix" dev="ggregory" due-to="Gary Gregory">Bound Sha2Crypt rounds from caller-supplied salts.</action> + <action type="fix" dev="ggregory" due-to="Gary Gregory">Bound Sha2Crypt password length to prevent quadratic CPU DoS.</action> <!-- ADD --> <action type="add" dev="ggregory" due-to="Gary Gregory">Add and use PhoneticEngine.Builder and deprecate old constructors.</action> <action type="add" dev="ggregory" due-to="Gary Gregory">Add BeiderMorseEncoder.Builder and deprecate old constructor.</action> diff --git a/src/main/java/org/apache/commons/codec/digest/Crypt.java b/src/main/java/org/apache/commons/codec/digest/Crypt.java index b7514248..2f055233 100644 --- a/src/main/java/org/apache/commons/codec/digest/Crypt.java +++ b/src/main/java/org/apache/commons/codec/digest/Crypt.java @@ -128,6 +128,10 @@ public class Crypt { * their own authentication timeouts and resource controls. * </p> * <p> + * SHA-256 and SHA-512 plaintext is limited to 4096 bytes by default because SHA-crypt has a quadratic input-length step. Applications that deliberately need a + * higher limit can set the {@code org.apache.commons.codec.digest.Sha2Crypt.keyMax} system property. This property is intended for trusted JVM configuration only. + * </p> + * <p> * The resulting string starts with the marker string ({@code $n$}), where n is the same as the input salt. The salt is then appended, followed by a * {@code "$"} sign. This is followed by the actual hash value. For DES the string only contains the salt and actual hash. The total length is dependent on * the algorithm used: diff --git a/src/main/java/org/apache/commons/codec/digest/Sha2Crypt.java b/src/main/java/org/apache/commons/codec/digest/Sha2Crypt.java index ab70935e..0ec62f6a 100644 --- a/src/main/java/org/apache/commons/codec/digest/Sha2Crypt.java +++ b/src/main/java/org/apache/commons/codec/digest/Sha2Crypt.java @@ -38,11 +38,23 @@ import java.util.regex.Pattern; * <p> * This class is immutable and thread-safe. * </p> + * <p> + * SHA-crypt hashing has a quadratic input-length step. To bound CPU and memory consumption when plaintext is supplied by an + * untrusted caller, plaintext is limited to 4096 bytes by default. The limit can be changed with the + * {@code org.apache.commons.codec.digest.Sha2Crypt.keyMax} system property; this property is intended for trusted JVM + * configuration only. + * </p> * * @since 1.7 */ public class Sha2Crypt { + /** Default maximum plaintext (key) length in bytes. */ + private static final int KEY_MAX_DEFAULT = 4096; + + /** System property used to override the default maximum plaintext (key) length. */ + static final String KEY_MAX_PROPERTY = "org.apache.commons.codec.digest.Sha2Crypt.keyMax"; + /** Default number of rounds if not explicitly specified. */ private static final int ROUNDS_DEFAULT = 5000; @@ -77,6 +89,20 @@ public class Sha2Crypt { private static final Pattern SALT_PATTERN = Pattern .compile("^\\$([56])\\$(rounds=(\\d+)\\$)?([\\.\\/a-zA-Z0-9]{1,16}).*"); + /** + * Finds the first non-zero digit, retaining one zero for an all-zero value. + * + * @param value a non-empty decimal string + * @return the index of the first significant digit + */ + private static int firstNonZeroIndex(final String value) { + int index = 0; + while (index < value.length() - 1 && value.charAt(index) == '0') { + index++; + } + return index; + } + /** * Generates a libc crypt() compatible "$5$" hash value with random salt. * @@ -89,6 +115,7 @@ public class Sha2Crypt { * * @param keyBytes Plaintext to hash. Each array element is set to {@code 0} before returning. * @return The Complete hash value. + * @throws IllegalArgumentException if {@code keyBytes} exceeds the configured maximum length * @throws IllegalArgumentException Thrown if a {@link NoSuchAlgorithmException} is caught. */ public static String sha256Crypt(final byte[] keyBytes) { @@ -105,6 +132,7 @@ public class Sha2Crypt { * @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)}. * @return The Complete hash value including salt. + * @throws IllegalArgumentException if {@code keyBytes} exceeds the configured maximum length * @throws IllegalArgumentException Thrown if the salt does not match the allowed pattern. * @throws IllegalArgumentException Thrown if a {@link NoSuchAlgorithmException} is caught. */ @@ -125,6 +153,7 @@ public class Sha2Crypt { * @param salt real salt value without prefix or "rounds=". * @param random The instance of {@link Random} to use for generating the salt. Consider using {@link SecureRandom} for more secure salts. * @return The Complete hash value including salt. + * @throws IllegalArgumentException if {@code keyBytes} exceeds the configured maximum length * @throws IllegalArgumentException Thrown if the salt does not match the allowed pattern. * @throws IllegalArgumentException Thrown if a {@link NoSuchAlgorithmException} is caught. * @since 1.12 @@ -160,6 +189,11 @@ public class Sha2Crypt { final int blocksize, final String algorithm) { final int keyLen = keyBytes.length; + final int keyMax = Math.max(0, Integer.getInteger(KEY_MAX_PROPERTY, KEY_MAX_DEFAULT)); + if (keyLen > keyMax) { + throw new IllegalArgumentException("Key length " + keyLen + " exceeds the maximum of " + keyMax + " bytes; " + + "raise it with the " + KEY_MAX_PROPERTY + " system property if intended"); + } // Extracts effective salt and the number of rounds from the given salt. int rounds = ROUNDS_DEFAULT; @@ -179,8 +213,8 @@ public class Sha2Crypt { final int firstNonZero = firstNonZeroIndex(roundsString); final String normalizedRounds = roundsString.substring(firstNonZero); final String roundsMaxString = Integer.toString(roundsMax); - if (normalizedRounds.length() > roundsMaxString.length() - || normalizedRounds.length() == roundsMaxString.length() && normalizedRounds.compareTo(roundsMaxString) > 0) { + if (normalizedRounds.length() > roundsMaxString.length() || + normalizedRounds.length() == roundsMaxString.length() && normalizedRounds.compareTo(roundsMaxString) > 0) { throw new IllegalArgumentException("Rounds value in salt exceeds the maximum of " + roundsMax + ": " + salt); } rounds = Math.max(ROUNDS_MIN, Integer.parseInt(normalizedRounds)); @@ -541,20 +575,6 @@ public class Sha2Crypt { return buffer.toString(); } - /** - * Finds the first non-zero digit, retaining one zero for an all-zero value. - * - * @param value a non-empty decimal string - * @return the index of the first significant digit - */ - private static int firstNonZeroIndex(final String value) { - int index = 0; - while (index < value.length() - 1 && value.charAt(index) == '0') { - index++; - } - return index; - } - /** * Generates a libc crypt() compatible "$6$" hash value with random salt. * @@ -567,6 +587,7 @@ public class Sha2Crypt { * * @param keyBytes Plaintext to hash. Each array element is set to {@code 0} before returning. * @return Complete hash value. + * @throws IllegalArgumentException if {@code keyBytes} exceeds the configured maximum length * @throws IllegalArgumentException Thrown if a {@link NoSuchAlgorithmException} is caught. */ public static String sha512Crypt(final byte[] keyBytes) { @@ -585,6 +606,7 @@ public class Sha2Crypt { * 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)}. * @return Complete hash value including salt. + * @throws IllegalArgumentException if {@code keyBytes} exceeds the configured maximum length * @throws IllegalArgumentException Thrown if the salt does not match the allowed pattern. * @throws IllegalArgumentException Thrown if a {@link NoSuchAlgorithmException} is caught. */ diff --git a/src/test/java/org/apache/commons/codec/digest/Sha2CryptKeyLengthTest.java b/src/test/java/org/apache/commons/codec/digest/Sha2CryptKeyLengthTest.java new file mode 100644 index 00000000..313b8a96 --- /dev/null +++ b/src/test/java/org/apache/commons/codec/digest/Sha2CryptKeyLengthTest.java @@ -0,0 +1,56 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.commons.codec.digest; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrowsExactly; + +import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.SetSystemProperty; + +class Sha2CryptKeyLengthTest { + + private static final String SHA256_SALT = "$5$abcdefghijklmnop"; + + private static final String SHA512_SALT = "$6$abcdefghijklmnop"; + + @Test + void testKeyLengthAboveCeilingRejectedByCrypt() { + assertThrowsExactly(IllegalArgumentException.class, () -> Crypt.crypt(new byte[4097], SHA256_SALT)); + assertThrowsExactly(IllegalArgumentException.class, () -> Crypt.crypt(new byte[4097], SHA512_SALT)); + assertThrowsExactly(IllegalArgumentException.class, () -> Crypt.crypt(new byte[4097])); + } + + @Test + void testKeyLengthAboveCeilingRejectedBySha2Crypt() { + assertThrowsExactly(IllegalArgumentException.class, () -> Sha2Crypt.sha256Crypt(new byte[4097], SHA256_SALT)); + assertThrowsExactly(IllegalArgumentException.class, () -> Sha2Crypt.sha512Crypt(new byte[4097], SHA512_SALT)); + } + + @Test + void testKeyLengthAtCeilingAccepted() { + assertNotNull(Sha2Crypt.sha256Crypt(new byte[4096], SHA256_SALT)); + assertNotNull(Sha2Crypt.sha512Crypt(new byte[4096], SHA512_SALT)); + } + + @Test + @SetSystemProperty(key = Sha2Crypt.KEY_MAX_PROPERTY, value = "8192") + void testKeyLengthCeilingOverrideRestoresPreviousValue() { + assertNotNull(Sha2Crypt.sha512Crypt(new byte[8192], SHA512_SALT)); + } +} diff --git a/src/test/java/org/apache/commons/codec/digest/Sha2CryptTest.java b/src/test/java/org/apache/commons/codec/digest/Sha2CryptTest.java index 13e7507b..e94d4e6a 100644 --- a/src/test/java/org/apache/commons/codec/digest/Sha2CryptTest.java +++ b/src/test/java/org/apache/commons/codec/digest/Sha2CryptTest.java @@ -51,13 +51,6 @@ class Sha2CryptTest { () -> Sha2Crypt.sha512Crypt("secret".getBytes(StandardCharsets.UTF_8), "$6$rounds=99999999999$abcdefghijklmnop")); } - @Test - void testRoundsLeadingZeroes() { - final String expected = Sha2Crypt.sha512Crypt("secret".getBytes(StandardCharsets.UTF_8), "$6$rounds=1000$abcdefghijklmnop"); - final String actual = Sha2Crypt.sha512Crypt("secret".getBytes(StandardCharsets.UTF_8), "$6$rounds=0000001000$abcdefghijklmnop"); - assertEquals(expected, actual); - } - @Test void testRoundsCeilingOverride() { final String previous = System.getProperty(Sha2Crypt.ROUNDS_MAX_PROPERTY); @@ -72,4 +65,11 @@ class Sha2CryptTest { } } } + + @Test + void testRoundsLeadingZeroes() { + final String expected = Sha2Crypt.sha512Crypt("secret".getBytes(StandardCharsets.UTF_8), "$6$rounds=1000$abcdefghijklmnop"); + final String actual = Sha2Crypt.sha512Crypt("secret".getBytes(StandardCharsets.UTF_8), "$6$rounds=0000001000$abcdefghijklmnop"); + assertEquals(expected, actual); + } }
