diff --git a/jspwiki-main/pom.xml b/jspwiki-main/pom.xml index 06a04f26ae..e858ae4bc9 100644 --- a/jspwiki-main/pom.xml +++ b/jspwiki-main/pom.xml @@ -235,6 +235,13 @@ test jdk8 + + + com.github.kirviq + dumbster + 1.7.1 + test + org.junit.jupiter diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/user/AbstractUserDatabase.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/user/AbstractUserDatabase.java index 1f222589e5..b407be546a 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/user/AbstractUserDatabase.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/user/AbstractUserDatabase.java @@ -200,26 +200,26 @@ public UserProfile newProfile() { */ @Override public boolean validatePassword( final String loginName, final String password ) { - final String hashedPassword; try { final UserProfile profile = findByLoginName( loginName ); - String storedPassword = profile.getPassword(); + final String storedPassword = profile.getPassword(); boolean verified = false; - // If the password is stored as SHA-256 or SSHA, verify the hash - if( storedPassword.startsWith( SHA256_PREFIX ) || storedPassword.startsWith( SSHA_PREFIX ) ) { + if( storedPassword.startsWith( CryptoUtil.PBKDF2_PREFIX ) ) { + // current format: iterated, salted PBKDF2-HMAC-SHA256 + verified = CryptoUtil.verifyPbkdf2SaltedPassword( password.getBytes( StandardCharsets.UTF_8 ), storedPassword ); + } else if( storedPassword.startsWith( SHA256_PREFIX ) || storedPassword.startsWith( SSHA_PREFIX ) ) { + // legacy salted, single-iteration digests verified = CryptoUtil.verifySaltedPassword( password.getBytes( StandardCharsets.UTF_8 ), storedPassword ); + } else if( storedPassword.startsWith( SHA_PREFIX ) ) { + // legacy unsalted SHA-1; compare in constant time while this format survives + final String hashedPassword = getShaHash( password ); + verified = MessageDigest.isEqual( hashedPassword.getBytes( StandardCharsets.UTF_8 ), + storedPassword.substring( SHA_PREFIX.length() ).getBytes( StandardCharsets.UTF_8 ) ); } - // Use older verification algorithm if password is stored as SHA - if( storedPassword.startsWith( SHA_PREFIX ) ) { - storedPassword = storedPassword.substring( SHA_PREFIX.length() ); - hashedPassword = getShaHash( password ); - verified = hashedPassword.equals( storedPassword ); - } - - // If in the old format and password verified, upgrade the hash to SSHA - if( verified && !storedPassword.startsWith( SHA256_PREFIX ) ) { + // If verified against anything but the current KDF, upgrade the stored hash on this successful login + if( verified && !storedPassword.startsWith( CryptoUtil.PBKDF2_PREFIX ) ) { profile.setPassword( password ); save( profile ); } @@ -243,6 +243,12 @@ public boolean validatePasswordReuse( final String loginName, final String passw // If the password is stored as SHA-256 or SSHA, verify the hash for (String storedPassword : profile.getPreviousHashedCredentials()) { + if (storedPassword.startsWith(CryptoUtil.PBKDF2_PREFIX)) { + boolean match = CryptoUtil.verifyPbkdf2SaltedPassword(password.getBytes(StandardCharsets.UTF_8), storedPassword); + if (match) { + return false; + } + } if (storedPassword.startsWith(SHA256_PREFIX) || storedPassword.startsWith(SSHA_PREFIX)) { boolean match = CryptoUtil.verifySaltedPassword(password.getBytes(StandardCharsets.UTF_8), storedPassword); if (match) { @@ -304,7 +310,7 @@ protected static String generateUid( final UserDatabase db ) { */ protected String getHash( final String text ) { try { - return CryptoUtil.getSaltedPassword( text.getBytes(StandardCharsets.UTF_8), SHA256_PREFIX ); + return CryptoUtil.getPbkdf2SaltedPassword( text.getBytes( StandardCharsets.UTF_8 ) ); } catch( final NoSuchAlgorithmException e ) { LOG.error( "Error creating salted password hash: {}", e.getMessage() ); return text; diff --git a/jspwiki-main/src/test/java/org/apache/wiki/auth/AbstractPasswordReuseTest.java b/jspwiki-main/src/test/java/org/apache/wiki/auth/AbstractPasswordReuseTest.java index c5cb56825f..1066039e96 100644 --- a/jspwiki-main/src/test/java/org/apache/wiki/auth/AbstractPasswordReuseTest.java +++ b/jspwiki-main/src/test/java/org/apache/wiki/auth/AbstractPasswordReuseTest.java @@ -15,6 +15,7 @@ */ package org.apache.wiki.auth; +import com.dumbster.smtp.SimpleSmtpServer; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpSession; import java.util.Properties; @@ -24,7 +25,9 @@ import org.apache.wiki.api.core.Context; import org.apache.wiki.api.core.Session; import org.apache.wiki.auth.user.UserProfile; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -33,12 +36,28 @@ * */ public abstract class AbstractPasswordReuseTest { + private static SimpleSmtpServer dumbster = null; + + @BeforeAll + public static void startTestEmailServer() throws Exception { + + dumbster = SimpleSmtpServer.start(SimpleSmtpServer.AUTO_SMTP_PORT); + + } + + @AfterAll + public static void stopTestEmailServer() { + if (dumbster != null) { + dumbster.close(); + } + } public abstract Properties getTestProps() throws Exception; @Test public void verifyPasswordReusePolicies() throws Exception { Properties props = getTestProps(); + props.setProperty("mail.smtp.port", dumbster.getPort()+""); final HttpSession httpSession = mock(HttpSession.class); @@ -158,7 +177,7 @@ public void verifyPasswordReusePolicies() throws Exception { public void verifyPasswordReusePoliciesWithItOff() throws Exception { Properties props = getTestProps(); - + props.setProperty("mail.smtp.port", dumbster.getPort()+""); final HttpSession httpSession = mock(HttpSession.class); HttpServletRequest request = mock(HttpServletRequest.class); diff --git a/jspwiki-main/src/test/java/org/apache/wiki/auth/UserManagerTest.java b/jspwiki-main/src/test/java/org/apache/wiki/auth/UserManagerTest.java index 97ba5c0a71..81bf1378fe 100644 --- a/jspwiki-main/src/test/java/org/apache/wiki/auth/UserManagerTest.java +++ b/jspwiki-main/src/test/java/org/apache/wiki/auth/UserManagerTest.java @@ -18,6 +18,7 @@ Licensed to the Apache Software Foundation (ASF) under one */ package org.apache.wiki.auth; +import com.dumbster.smtp.SimpleSmtpServer; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpSession; import java.io.File; @@ -56,10 +57,26 @@ Licensed to the Apache Software Foundation (ASF) under one import java.util.UUID; import org.apache.commons.io.FileUtils; import org.apache.wiki.WikiEngine; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; class UserManagerTest { + private static SimpleSmtpServer dumbster = null; + @BeforeAll + public static void startTestEmailServer() throws Exception { + + dumbster = SimpleSmtpServer.start(SimpleSmtpServer.AUTO_SMTP_PORT); + + } + + @AfterAll + public static void stopTestEmailServer() { + if (dumbster != null) { + dumbster.close(); + } + } TestEngine m_engine; UserManager m_mgr; UserDatabase m_db; @@ -71,7 +88,7 @@ class UserManagerTest { @BeforeEach void setUp() throws Exception { final Properties props = TestEngine.getTestProperties(); - + props.setProperty("mail.smtp.port", dumbster.getPort()+""); // Make sure user profile save workflow is OFF props.remove( "jspwiki.approver" + WorkflowManager.WF_UP_CREATE_SAVE_APPROVER ); @@ -97,7 +114,7 @@ void tearDown() throws Exception { /** Call this setup program to use the save-profile workflow. */ protected void setUpWithWorkflow() throws Exception { final Properties props = TestEngine.getTestProperties(); - + props.setProperty("mail.smtp.port", dumbster.getPort()+""); // Turn on user profile saves by the Admin group props.put( "jspwiki.approver." + WorkflowManager.WF_UP_CREATE_SAVE_APPROVER, "Admin" ); diff --git a/jspwiki-util/src/main/java/org/apache/wiki/util/CryptoUtil.java b/jspwiki-util/src/main/java/org/apache/wiki/util/CryptoUtil.java index 722f23c6ae..a39e593ab3 100644 --- a/jspwiki-util/src/main/java/org/apache/wiki/util/CryptoUtil.java +++ b/jspwiki-util/src/main/java/org/apache/wiki/util/CryptoUtil.java @@ -18,10 +18,13 @@ Licensed to the Apache Software Foundation (ASF) under one */ package org.apache.wiki.util; +import javax.crypto.SecretKeyFactory; +import javax.crypto.spec.PBEKeySpec; import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; +import java.security.spec.InvalidKeySpecException; import java.util.Base64; import java.util.Random; @@ -42,6 +45,16 @@ public final class CryptoUtil { private static final int DEFAULT_SALT_SIZE = 8; + /** Prefix of PBKDF2-HMAC-SHA256 password entries, the current password-storage format. */ + public static final String PBKDF2_PREFIX = "{PBKDF2-SHA256}"; + + /** Iteration count for new PBKDF2-HMAC-SHA256 hashes, per current OWASP password-storage guidance. */ + private static final int PBKDF2_ITERATIONS = 600_000; + + private static final int PBKDF2_KEY_LENGTH_BYTES = 32; + + private static final int PBKDF2_SALT_SIZE = 16; + private static final Object HELP = "--help"; private static final Object HASH = "--hash"; @@ -174,6 +187,64 @@ static String getSaltedPassword( final byte[] password, final byte[] salt, final return algorithm + new String( base64, StandardCharsets.UTF_8 ); } + /** + *

Creates an iterated, salted PBKDF2-HMAC-SHA256 hash of the given password, suitable for storage. Unlike the + * single-iteration digests above, the work factor makes offline cracking of a disclosed user database + * expensive. The format is {PBKDF2-SHA256}iterations$base64(salt)$base64(hash), so the + * iteration count of stored entries can be raised in the future without breaking old entries.

+ * + * @param password the password to be hashed + * @return the password entry, prepended by {PBKDF2-SHA256} + * @throws NoSuchAlgorithmException If your JVM does not supply the necessary algorithm. Should not happen. + */ + public static String getPbkdf2SaltedPassword( final byte[] password ) throws NoSuchAlgorithmException { + final byte[] salt = new byte[ PBKDF2_SALT_SIZE ]; + RANDOM.nextBytes( salt ); + return getPbkdf2SaltedPassword( password, salt, PBKDF2_ITERATIONS ); + } + + static String getPbkdf2SaltedPassword( final byte[] password, final byte[] salt, final int iterations ) throws NoSuchAlgorithmException { + final byte[] hash = pbkdf2( password, salt, iterations ); + final Base64.Encoder encoder = Base64.getEncoder(); + return PBKDF2_PREFIX + iterations + "$" + encoder.encodeToString( salt ) + "$" + encoder.encodeToString( hash ); + } + + /** + * Verifies a password against a {PBKDF2-SHA256} entry created by + * {@link #getPbkdf2SaltedPassword(byte[])}. The comparison is constant-time. + * + * @param password the password to verify + * @param entry the stored password entry + * @return true if the password matches the entry + * @throws NoSuchAlgorithmException If your JVM does not supply the necessary algorithm. Should not happen. + */ + public static boolean verifyPbkdf2SaltedPassword( final byte[] password, final String entry ) throws NoSuchAlgorithmException { + if( !entry.startsWith( PBKDF2_PREFIX ) ) { + throw new IllegalArgumentException( "Hash not prefixed by expected algorithm; is it really a PBKDF2 hash?" ); + } + final String[] fields = entry.substring( PBKDF2_PREFIX.length() ).split( "\\$" ); + if( fields.length != 3 ) { + throw new IllegalArgumentException( "Malformed PBKDF2 password entry" ); + } + final int iterations = Integer.parseInt( fields[ 0 ] ); + final byte[] salt = Base64.getDecoder().decode( fields[ 1 ] ); + final byte[] expected = Base64.getDecoder().decode( fields[ 2 ] ); + final byte[] hash = pbkdf2( password, salt, iterations ); + return MessageDigest.isEqual( expected, hash ); + } + + private static byte[] pbkdf2( final byte[] password, final byte[] salt, final int iterations ) throws NoSuchAlgorithmException { + final char[] chars = new String( password, StandardCharsets.UTF_8 ).toCharArray(); + final PBEKeySpec spec = new PBEKeySpec( chars, salt, iterations, PBKDF2_KEY_LENGTH_BYTES * 8 ); + try { + return SecretKeyFactory.getInstance( "PBKDF2WithHmacSHA256" ).generateSecret( spec ).getEncoded(); + } catch( final InvalidKeySpecException e ) { + throw new NoSuchAlgorithmException( "Unable to compute PBKDF2 hash", e ); + } finally { + spec.clearPassword(); + } + } + /** * Compares a password to a given entry and returns true, if it matches. * @@ -183,11 +254,23 @@ static String getSaltedPassword( final byte[] password, final byte[] salt, final * @throws NoSuchAlgorithmException If there is no SHA available. */ public static boolean verifySaltedPassword( final byte[] password, final String entry ) throws NoSuchAlgorithmException { - if( !entry.startsWith( SSHA ) && !entry.startsWith( SHA256 ) ) { + if( !entry.startsWith( SSHA ) && !entry.startsWith( SHA256 ) && !entry.startsWith( CryptoUtil.PBKDF2_PREFIX ) ) { throw new IllegalArgumentException( "Hash not prefixed by expected algorithm; is it really a salted hash?" ); } - final String algorithm = entry.startsWith( SSHA ) ? SSHA : SHA256; - final byte[] challenge = Base64.getDecoder().decode( entry.substring( algorithm.length() ).getBytes( StandardCharsets.UTF_8 ) ); + final String algorithm; + if (entry.startsWith(PBKDF2_PREFIX)) { + return verifyPbkdf2SaltedPassword(password, entry); + } else if (entry.startsWith(SSHA)) { + algorithm = SSHA; + } else if (entry.startsWith(SHA256)) { + algorithm = SHA256; + } else { + throw new NoSuchAlgorithmException("unknown hash algorithm prefix"); + } + + String hash2 = entry.substring( algorithm.length() ); + byte[] bits = hash2.getBytes( StandardCharsets.UTF_8 ); + final byte[] challenge = Base64.getDecoder().decode(bits); // Extract the password hash and salt final byte[] passwordHash = extractPasswordHash( challenge, algorithm.equals( SSHA ) ? 20 : 32 ); diff --git a/jspwiki-util/src/test/java/org/apache/wiki/util/CryptoUtilPbkdf2Test.java b/jspwiki-util/src/test/java/org/apache/wiki/util/CryptoUtilPbkdf2Test.java new file mode 100644 index 0000000000..b8a94ba602 --- /dev/null +++ b/jspwiki-util/src/test/java/org/apache/wiki/util/CryptoUtilPbkdf2Test.java @@ -0,0 +1,58 @@ +/* + 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 + + http://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.wiki.util; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.nio.charset.StandardCharsets; + + +/** + * Regression tests for the PBKDF2 password-storage format. + */ +public class CryptoUtilPbkdf2Test { + + @Test + public void testRoundTrip() throws Exception { + final String entry = CryptoUtil.getPbkdf2SaltedPassword( "test128".getBytes( StandardCharsets.UTF_8 ) ); + Assertions.assertTrue( entry.startsWith( CryptoUtil.PBKDF2_PREFIX ) ); + Assertions.assertTrue( CryptoUtil.verifyPbkdf2SaltedPassword( "test128".getBytes( StandardCharsets.UTF_8 ), entry ) ); + } + + @Test + public void testWrongPasswordFails() throws Exception { + final String entry = CryptoUtil.getPbkdf2SaltedPassword( "test128".getBytes( StandardCharsets.UTF_8 ) ); + Assertions.assertFalse( CryptoUtil.verifyPbkdf2SaltedPassword( "TEST128".getBytes( StandardCharsets.UTF_8 ), entry ) ); + } + + @Test + public void testSaltsDiffer() throws Exception { + final String one = CryptoUtil.getPbkdf2SaltedPassword( "test128".getBytes( StandardCharsets.UTF_8 ) ); + final String two = CryptoUtil.getPbkdf2SaltedPassword( "test128".getBytes( StandardCharsets.UTF_8 ) ); + Assertions.assertNotEquals( one, two ); + } + + @Test + public void testNonPbkdf2EntryIsRejected() { + Assertions.assertThrows( IllegalArgumentException.class, + () -> CryptoUtil.verifyPbkdf2SaltedPassword( "x".getBytes( StandardCharsets.UTF_8 ), "{SSHA}abc" ) ); + } + +}