JSPWIKI-1299 see jira for details

jspwiki-main/pom.xml

@@ -235,6 +235,13 @@
             <scope>test</scope>
             <classifier>jdk8</classifier>
         </dependency>
+
+        <dependency>
+            <groupId>com.github.kirviq</groupId>
+            <artifactId>dumbster</artifactId>
+            <version>1.7.1</version>
+            <scope>test</scope>
+        </dependency>
 
         <dependency>
             <groupId>org.junit.jupiter</groupId>

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;

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);

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" );
 

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 );
     }
 
+    /**
+     * <p>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 <code>{PBKDF2-SHA256}<var>iterations</var>$base64(salt)$base64(hash)</code>, so the
+     * iteration count of stored entries can be raised in the future without breaking old entries.</p>
+     *
+     * @param password the password to be hashed
+     * @return the password entry, prepended by <code>{PBKDF2-SHA256}</code>
+     * @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 <code>{PBKDF2-SHA256}</code> 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 );

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" ) );
+    }
+
+}