This is an automated email from the ASF dual-hosted git repository. btellier pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit bc10bab19ea489f6146d4c735c9df42ecec23852 Author: Benoit Tellier <[email protected]> AuthorDate: Fri Dec 3 11:29:53 2021 +0700 JAMES-3674 PBKDF2 hashing --- .../org/apache/james/user/jpa/model/JPAUser.java | 3 + .../org/apache/james/user/lib/model/Algorithm.java | 96 +++++++++++++++++++++- .../apache/james/user/lib/model/DefaultUser.java | 58 ++----------- .../apache/james/user/lib/model/AlgorithmTest.java | 73 +++++++++++++++- .../james/user/lib/model/DefaultUserTest.java | 55 ++++++++++++- 5 files changed, 233 insertions(+), 52 deletions(-) diff --git a/server/data/data-jpa/src/main/java/org/apache/james/user/jpa/model/JPAUser.java b/server/data/data-jpa/src/main/java/org/apache/james/user/jpa/model/JPAUser.java index c906b39..dad1fa0 100644 --- a/server/data/data-jpa/src/main/java/org/apache/james/user/jpa/model/JPAUser.java +++ b/server/data/data-jpa/src/main/java/org/apache/james/user/jpa/model/JPAUser.java @@ -60,6 +60,9 @@ public class JPAUser implements User { @VisibleForTesting static String hashPassword(String password, String nullableSalt, String nullableAlgorithm) { Algorithm algorithm = Algorithm.of(Optional.ofNullable(nullableAlgorithm).orElse("SHA-512")); + if (algorithm.isPBKDF2()) { + return algorithm.digest(password, nullableSalt); + } String credentials = password; if (algorithm.isSalted() && nullableSalt != null) { credentials = nullableSalt + password; diff --git a/server/data/data-library/src/main/java/org/apache/james/user/lib/model/Algorithm.java b/server/data/data-library/src/main/java/org/apache/james/user/lib/model/Algorithm.java index 6e0222d..5c01c68 100644 --- a/server/data/data-library/src/main/java/org/apache/james/user/lib/model/Algorithm.java +++ b/server/data/data-library/src/main/java/org/apache/james/user/lib/model/Algorithm.java @@ -21,21 +21,32 @@ package org.apache.james.user.lib.model; import static java.nio.charset.StandardCharsets.ISO_8859_1; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.OutputStream; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.security.spec.InvalidKeySpecException; +import java.security.spec.KeySpec; import java.util.Arrays; import java.util.List; import java.util.Objects; +import java.util.Optional; +import javax.crypto.SecretKeyFactory; +import javax.crypto.spec.PBEKeySpec; +import javax.mail.MessagingException; +import javax.mail.internet.MimeUtility; + +import com.google.common.base.Preconditions; import com.google.common.base.Splitter; import com.google.common.collect.ImmutableList; public class Algorithm { public interface Hasher { static Hasher from(Algorithm algorithm) { - return new RegularHashingSpec(algorithm); + return PBKDF2Hasher.from(algorithm) + .orElseGet(() -> new RegularHashingSpec(algorithm)); } byte[] digestString(String pass, String salt) throws IOException, NoSuchAlgorithmException, InvalidKeySpecException; @@ -64,6 +75,67 @@ public class Algorithm { } } + public static class PBKDF2Hasher implements Hasher { + public static Optional<Hasher> from(Algorithm algorithm) { + if (algorithm.getName().startsWith("PBKDF2")) { + List<String> parts = Splitter.on('-').splitToList(algorithm.getName()); + return Optional.of(new PBKDF2Hasher(parseIterationCount(parts), parseKeySize(parts))); + } else { + return Optional.empty(); + } + } + + private static int parseKeySize(List<String> parts) { + if (parts.size() >= 3) { + return Integer.parseInt(parts.get(2)); + } else { + return 512; + } + } + + private static int parseIterationCount(List<String> parts) { + if (parts.size() >= 2) { + return Integer.parseInt(parts.get(1)); + } else { + return 1000; + } + } + + private final int iterationCount; + private final int keySize; + + public PBKDF2Hasher(int iterationCount, int keySize) { + Preconditions.checkArgument(iterationCount > 0, "'iterationCount' should be greater than 0"); + Preconditions.checkArgument(keySize > 0, "'keySize' should be greater than 0"); + + this.iterationCount = iterationCount; + this.keySize = keySize; + } + + public byte[] digestString(String pass, String salt) throws NoSuchAlgorithmException, InvalidKeySpecException { + KeySpec spec = new PBEKeySpec(pass.toCharArray(), salt.getBytes(ISO_8859_1), iterationCount, keySize); + SecretKeyFactory factory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1"); + + return factory.generateSecret(spec).getEncoded(); + } + + @Override + public final boolean equals(Object o) { + if (o instanceof PBKDF2Hasher) { + PBKDF2Hasher that = (PBKDF2Hasher) o; + + return Objects.equals(this.iterationCount, that.iterationCount) + && Objects.equals(this.keySize, that.keySize); + } + return false; + } + + @Override + public final int hashCode() { + return Objects.hash(iterationCount, keySize); + } + } + public enum HashingMode { PLAIN, SALTED, @@ -121,6 +193,10 @@ public class Algorithm { return hashingMode == HashingMode.LEGACY || hashingMode == HashingMode.LEGACY_SALTED; } + public boolean isPBKDF2() { + return hasher instanceof PBKDF2Hasher; + } + public boolean isSalted() { return hashingMode == HashingMode.SALTED || hashingMode == HashingMode.LEGACY_SALTED; } @@ -129,6 +205,24 @@ public class Algorithm { return hasher; } + public String digest(String pass, String salt) { + try { + return encodeInBase64(hasher().digestString(pass, salt)); + } catch (MessagingException | IOException | NoSuchAlgorithmException | InvalidKeySpecException e) { + throw new RuntimeException("Fatal error when hashing password", e); + } + } + + public String encodeInBase64(byte[] digest) throws MessagingException, IOException { + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + OutputStream encodedStream = MimeUtility.encode(bos, "base64"); + encodedStream.write(digest); + if (!isLegacy()) { + encodedStream.close(); + } + return bos.toString(ISO_8859_1); + } + @Override public final boolean equals(Object o) { if (o instanceof Algorithm) { diff --git a/server/data/data-library/src/main/java/org/apache/james/user/lib/model/DefaultUser.java b/server/data/data-library/src/main/java/org/apache/james/user/lib/model/DefaultUser.java index 4a802de..9be1106 100644 --- a/server/data/data-library/src/main/java/org/apache/james/user/lib/model/DefaultUser.java +++ b/server/data/data-library/src/main/java/org/apache/james/user/lib/model/DefaultUser.java @@ -19,17 +19,7 @@ package org.apache.james.user.lib.model; -import static java.nio.charset.StandardCharsets.ISO_8859_1; - -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.OutputStream; import java.io.Serializable; -import java.security.NoSuchAlgorithmException; -import java.security.spec.InvalidKeySpecException; - -import javax.mail.MessagingException; -import javax.mail.internet.MimeUtility; import org.apache.james.core.Username; import org.apache.james.user.api.model.User; @@ -90,23 +80,15 @@ public class DefaultUser implements User, Serializable { @Override public boolean verifyPassword(String pass) { - try { - String hashGuess = digestString(pass, currentAlgorithm, userName.asString()); - return hashedPassword.equals(hashGuess); - } catch (NoSuchAlgorithmException nsae) { - throw new RuntimeException("Security error: " + nsae); - } + String hashGuess = digestString(pass, currentAlgorithm, userName.asString()); + return hashedPassword.equals(hashGuess); } @Override public boolean setPassword(String newPass) { - try { - hashedPassword = digestString(newPass, preferredAlgorithm, userName.asString()); - currentAlgorithm = preferredAlgorithm; - return true; - } catch (NoSuchAlgorithmException nsae) { - throw new RuntimeException("Security error: " + nsae); - } + hashedPassword = digestString(newPass, preferredAlgorithm, userName.asString()); + currentAlgorithm = preferredAlgorithm; + return true; } @@ -129,37 +111,15 @@ public class DefaultUser implements User, Serializable { return currentAlgorithm; } - - /** * Calculate digest of given String using given algorithm. Encode digest in * MIME-like base64. * - * @param pass - * the String to be hashed - * @param algorithm - * the algorithm to be used + * @param pass the String to be hashed + * @param algorithm the algorithm to be used * @return String Base-64 encoding of digest - * - * @throws NoSuchAlgorithmException - * if the algorithm passed in cannot be found */ - static String digestString(String pass, Algorithm algorithm, String salt) throws NoSuchAlgorithmException { - try { - byte[] digest = algorithm.hasher().digestString(pass, salt); - return encodeInBase64(algorithm, digest); - } catch (IOException | MessagingException | InvalidKeySpecException e) { - throw new RuntimeException("Fatal error", e); - } - } - - private static String encodeInBase64(Algorithm algorithm, byte[] digest) throws MessagingException, IOException { - ByteArrayOutputStream bos = new ByteArrayOutputStream(); - OutputStream encodedStream = MimeUtility.encode(bos, "base64"); - encodedStream.write(digest); - if (!algorithm.isLegacy()) { - encodedStream.close(); - } - return bos.toString(ISO_8859_1); + static String digestString(String pass, Algorithm algorithm, String salt) { + return algorithm.digest(pass, salt); } } diff --git a/server/data/data-library/src/test/java/org/apache/james/user/lib/model/AlgorithmTest.java b/server/data/data-library/src/test/java/org/apache/james/user/lib/model/AlgorithmTest.java index 0498583..5da7daa 100644 --- a/server/data/data-library/src/test/java/org/apache/james/user/lib/model/AlgorithmTest.java +++ b/server/data/data-library/src/test/java/org/apache/james/user/lib/model/AlgorithmTest.java @@ -19,6 +19,9 @@ package org.apache.james.user.lib.model; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + import org.assertj.core.api.SoftAssertions; import org.junit.jupiter.api.Test; @@ -27,7 +30,9 @@ import nl.jqno.equalsverifier.EqualsVerifier; class AlgorithmTest { @Test void shouldMatchBeanContract() { - EqualsVerifier.forClass(Algorithm.class).verify(); + EqualsVerifier.forClass(Algorithm.class) + .withIgnoredFields("hasher") + .verify(); } @Test @@ -139,4 +144,70 @@ class AlgorithmTest { softly.assertThat(Algorithm.of("SHA-1/legacy_salted", "plain").isSalted()).isTrue(); }); } + + @Test + void ofShouldParseIterationAndKeySize() { + assertThat(Algorithm.of("PBKDF2-10-20", "plain").hasher()) + .isEqualTo(new Algorithm.PBKDF2Hasher(10, 20)); + } + + @Test + void ofShouldParseIteration() { + assertThat(Algorithm.of("PBKDF2-10", "plain").hasher()) + .isEqualTo(new Algorithm.PBKDF2Hasher(10, 1024)); + } + + @Test + void ofShouldAcceptDefaultPBKDF2() { + assertThat(Algorithm.of("PBKDF2", "plain").hasher()) + .isEqualTo(new Algorithm.PBKDF2Hasher(1000, 1024)); + } + + @Test + void ofShouldThrowOnBadIteration() { + assertThatThrownBy(() -> Algorithm.of("PBKDF2-bad", "plain")) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + void ofShouldThrowOnEmptyIteration() { + assertThatThrownBy(() -> Algorithm.of("PBKDF2-", "plain")) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + void ofShouldThrowOnNegativeIteration() { + assertThatThrownBy(() -> Algorithm.of("PBKDF2--1", "plain")) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + void ofShouldThrowOnEmptyKeySize() { + assertThatThrownBy(() -> Algorithm.of("PBKDF2-1-", "plain")) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + void ofShouldThrowOnBadKeySize() { + assertThatThrownBy(() -> Algorithm.of("PBKDF2-1-bad", "plain")) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + void ofShouldThrowOnZeroIteration() { + assertThatThrownBy(() -> Algorithm.of("PBKDF2-0", "plain")) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + void ofShouldThrowOnZeroKeySize() { + assertThatThrownBy(() -> Algorithm.of("PBKDF2-1-0", "plain")) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + void ofShouldThrowOnNegativeKeySize() { + assertThatThrownBy(() -> Algorithm.of("PBKDF2-1--1", "plain")) + .isInstanceOf(IllegalArgumentException.class); + } } \ No newline at end of file diff --git a/server/data/data-library/src/test/java/org/apache/james/user/lib/model/DefaultUserTest.java b/server/data/data-library/src/test/java/org/apache/james/user/lib/model/DefaultUserTest.java index 87f9e2a..98b37f1 100644 --- a/server/data/data-library/src/test/java/org/apache/james/user/lib/model/DefaultUserTest.java +++ b/server/data/data-library/src/test/java/org/apache/james/user/lib/model/DefaultUserTest.java @@ -110,7 +110,7 @@ public class DefaultUserTest { @ParameterizedTest @MethodSource("sha1TestBed") void testSha1(String password, String expectedHash) throws Exception { - assertThat(DefaultUser.digestString(Optional.ofNullable(password).orElse(""), Algorithm.of("SHA-1"), "salt")) + assertThat(DefaultUser.digestString(password, Algorithm.of("SHA-1"), "salt")) .isEqualTo(expectedHash); } @@ -128,4 +128,57 @@ public class DefaultUserTest { assertThat(DefaultUser.digestString(password, Algorithm.of("SHA-512"), "salt")) .isEqualTo(expectedHash); } + + private static Stream<Arguments> PBKDF2TestBed() { + return Stream.of( + Arguments.of("myPassword", "Ag2g49zxor0w11yguLUMQ7EKBokU81LkvLyDqubWtQq7R5V21HVqZ+CEjEQxBLGfi35RFyesJtxb\r\n" + + "L5/VRCpI3g==\r\n"), + Arguments.of("otherPassword", "4KFfGIjbZqhaqZfr1rKWcoY5vkeps3/+x5BwU342kUbGGoW30kaP98R5iY6SNGg0yOaPBcB8EWqJ\r\n" + + "96RtIMnIYQ==\r\n"), + Arguments.of("", "6grdNX1hpxA5wJPXhBUJhz4qUoUSRZE0F3rqoPR+PYedDklDomJ0LPRV5f1SMNAX0fRgmQ8WDe6k\r\n" + + "2qr1Nc/orA==\r\n"), + Arguments.of("a", "WxpwqV5V9L3QR8xi8D8INuH0UH5oLeq+ZuXb6J1bAfhHp3urVOtAr+bwksC3JQRyC7QHE9MLfn61\r\n" + + "nTXo5johrQ==\r\n")); + } + + @ParameterizedTest + @MethodSource("PBKDF2TestBed") + void testPBKDF2(String password, String expectedHash) { + assertThat(DefaultUser.digestString(password, Algorithm.of("PBKDF2"), "salt")) + .isEqualTo(expectedHash); + } + + private static Stream<Arguments> PBKDF210IterationTestBed() { + return Stream.of( + Arguments.of("myPassword", "AoNuFZ7ZI6vHU8obYASuuLPaQcr8fGentKWsOawBNTIc7MNJMbo4yNjo0pcCVK6J/XAfbISEKugt\r\n" + + "HwDeSUA10A==\r\n"), + Arguments.of("otherPassword", "e4+swbwo1s3X665INoRVsENXrgJtC7SMws9G3Y0GBoLZBkqZQzE2aT2WLd+hOlf3s/wwQe10MA0Q\r\n" + + "xMJQIcIosQ==\r\n"), + Arguments.of("", "ZBXj9rrLc4L9hHXOBPpDd5ot9DDB6qaq1g2mbAMOivpZe3eYw1ehdFXbU9pwpI4y/+MZlLkG3E1S\r\n" + + "WRQXuUZqag==\r\n"), + Arguments.of("a", "i1iWZzuaqsFotT998+stRqyrcyUrZ0diBJf9RJ52mUo0a074ykh8joWdrxhEsyd2Fh2DNO38TWxC\r\n" + + "KkIK6taLxA==\r\n")); + } + + @ParameterizedTest + @MethodSource("PBKDF210IterationTestBed") + void testPBKDF210Iteration(String password, String expectedHash) { + assertThat(DefaultUser.digestString(password, Algorithm.of("PBKDF2-10"), "salt")) + .isEqualTo(expectedHash); + } + + private static Stream<Arguments> PBKDF210Iteration128KeySizeTestBed() { + return Stream.of( + Arguments.of("myPassword", "AoNuFZ7ZI6vHU8obYASuuA==\r\n"), + Arguments.of("otherPassword", "e4+swbwo1s3X665INoRVsA==\r\n"), + Arguments.of("", "ZBXj9rrLc4L9hHXOBPpDdw==\r\n"), + Arguments.of("a", "i1iWZzuaqsFotT998+stRg==\r\n")); + } + + @ParameterizedTest + @MethodSource("PBKDF210Iteration128KeySizeTestBed") + void testPBKDF210Iteration128KeySize(String password, String expectedHash) { + assertThat(DefaultUser.digestString(password, Algorithm.of("PBKDF2-10-128"), "salt")) + .isEqualTo(expectedHash); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
