This is an automated email from the ASF dual-hosted git repository.
juanpablo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/jspwiki.git
commit 40f59e63201171623aed9d7d7a28f36e082876c4
Author: Juan Pablo Santos RodrÃguez <juanpablo.san...@gmail.com>
AuthorDate: Sun Apr 13 15:33:02 2025 +0200
Proper digest comparison on CryptoUtil.verifySaltedPassword
---
.../main/java/org/apache/wiki/util/CryptoUtil.java | 97 ++++++++--------------
.../java/org/apache/wiki/util/CryptoUtilTest.java | 3 +-
2 files changed, 35 insertions(+), 65 deletions(-)
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 b79854cd4..722f23c6a 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
@@ -22,7 +22,6 @@ import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
-import java.util.Arrays;
import java.util.Base64;
import java.util.Random;
@@ -31,8 +30,8 @@ import java.util.Random;
* Hashes and verifies salted SHA-1 passwords, which are compliant with RFC
* 2307.
*/
-public final class CryptoUtil
-{
+public final class CryptoUtil {
+
private static final String SSHA = "{SSHA}";
private static final String SHA1 = "{SHA-1}";
@@ -52,9 +51,7 @@ public final class CryptoUtil
/**
* Private constructor to prevent direct instantiation.
*/
- private CryptoUtil()
- {
- }
+ private CryptoUtil() {}
/**
* <p>
@@ -80,45 +77,34 @@ public final class CryptoUtil
* @param args arguments for this method as described above
* @throws Exception Catches nothing; throws everything up.
*/
- public static void main( final String[] args ) throws Exception
- {
+ public static void main( final String[] args ) throws Exception {
// Print help if the user requested it, or if no arguments
- if( args.length == 0 || (args.length == 1 && HELP.equals( args[0] )) )
- {
+ if( args.length == 0 || ( args.length == 1 && HELP.equals( args[0] ) )
) {
System.out.println( "Usage: CryptoUtil [options] " );
System.out.println( " --hash password algorithm
create hash for password" );
System.out.println( " --verify password digest algorithm
verify password for digest" );
System.out.println( "Valid algorithm options are {SSHA} and
{SHA-256}. If no algorithm is specified or an unsupported algorithm is
specified, SHA-256 is used." );
}
+ if( HASH.equals( args[0] ) ) {
// User wants to hash the password
- if( HASH.equals( args[0] ) )
- {
- if( args.length < 2 )
- {
+ if( args.length < 2 ) {
throw new IllegalArgumentException( "Error: --hash requires a
'password' argument." );
}
final String password = args[1].trim();
final String algorithm = args.length > 2 ? args[2].trim() : SHA256;
System.out.println( CryptoUtil.getSaltedPassword(
password.getBytes( StandardCharsets.UTF_8 ), algorithm ) );
- }
-
+ } else if( VERIFY.equals( args[0] ) ) {
// User wants to verify an existing password
- else if( VERIFY.equals( args[0] ) )
- {
- if( args.length < 3 )
- {
+ if( args.length < 3 ) {
throw new IllegalArgumentException( "Error: --hash requires
'password' and 'digest' arguments." );
}
final String password = args[1].trim();
final String digest = args[2].trim();
System.out.println( CryptoUtil.verifySaltedPassword(
password.getBytes( StandardCharsets.UTF_8 ), digest ) );
- }
-
- else
- {
+ } else {
System.out.println( "Wrong usage. Try --help." );
}
}
@@ -133,8 +119,7 @@ public final class CryptoUtil
* </p>
* <p>
* In layman's terms, the formula is
- * <code>digest( secret + salt ) + salt</code>. The resulting digest is
- * Base64-encoded.
+ * <code>digest( secret + salt ) + salt</code>. The resulting digest is
Base64-encoded.
* </p>
* <p>
* Note that successive invocations of this method with the same password
@@ -146,9 +131,8 @@ public final class CryptoUtil
* <code>{SSHA}</code> or <code>{SHA256}</code>.
* @throws NoSuchAlgorithmException If your JVM does not supply the
necessary algorithm. Should not happen.
*/
- public static String getSaltedPassword(final byte[] password, final String
algorithm ) throws NoSuchAlgorithmException
- {
- final byte[] salt = new byte[DEFAULT_SALT_SIZE];
+ public static String getSaltedPassword( final byte[] password, final
String algorithm ) throws NoSuchAlgorithmException {
+ final byte[] salt = new byte[ DEFAULT_SALT_SIZE ];
RANDOM.nextBytes( salt );
return getSaltedPassword( password, salt, algorithm );
@@ -172,8 +156,7 @@ public final class CryptoUtil
* @return the Base64-encoded password hash, prepended by
<code>{SSHA}</code> or <code>{SHA256}</code>.
* @throws NoSuchAlgorithmException If your JVM does not supply the
necessary algorithm. Should not happen.
*/
- static String getSaltedPassword(final byte[] password, final byte[] salt,
final String algorithm ) throws NoSuchAlgorithmException
- {
+ static String getSaltedPassword( final byte[] password, final byte[] salt,
final String algorithm ) throws NoSuchAlgorithmException {
//The term SSHA is used as a password prefix for backwards
compatibility, but we use SHA-1 when fetching an instance
//of MessageDigest, as it is the guaranteed option. We also need to
remove curly braces surrounding the string for
//backwards compatibility.
@@ -199,72 +182,59 @@ public final class CryptoUtil
* @return True, if the password matches.
* @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 ) )
- {
+ public static boolean verifySaltedPassword( final byte[] password, final
String entry ) throws NoSuchAlgorithmException {
+ if( !entry.startsWith( SSHA ) && !entry.startsWith( SHA256 ) ) {
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 byte[] challenge = Base64.getDecoder().decode( entry.substring(
algorithm.length() ).getBytes( StandardCharsets.UTF_8 ) );
// Extract the password hash and salt
- final byte[] passwordHash = extractPasswordHash( challenge,
algorithm.equals(SSHA) ? 20 : 32 );
- final byte[] salt = extractSalt( challenge, algorithm.equals(SSHA) ?
20 : 32 );
+ final byte[] passwordHash = extractPasswordHash( challenge,
algorithm.equals( SSHA ) ? 20 : 32 );
+ final byte[] salt = extractSalt( challenge, algorithm.equals( SSHA ) ?
20 : 32 );
// Re-create the hash using the password and the extracted salt
// The term SSHA is used as a password prefix for backwards
compatibility, but we use SHA-1 when fetching an instance
// of MessageDigest, as it is the guaranteed option. We also need to
remove curly braces surrounding the string for
// backwards compatibility.
- final String algorithmToUse = algorithm.equals(SSHA) ? SHA1 :
algorithm;
+ final String algorithmToUse = algorithm.equals( SSHA ) ? SHA1 :
algorithm;
final MessageDigest digest = MessageDigest.getInstance(
algorithmToUse.substring( 1, algorithmToUse.length() -1 ) );
digest.update( password );
final byte[] hash = digest.digest( salt );
// See if our extracted hash matches what we just re-created
- return Arrays.equals( passwordHash, hash );
+ return MessageDigest.isEqual( passwordHash, hash );
}
/**
* Helper method that extracts the hashed password fragment from a
supplied salted SHA-1 or SHA-256 digest
- * by taking all of the characters before position 20 or 32 depending on
algorithm.
+ * by taking all the characters before position 20 or 32 depending on
algorithm.
*
- * @param digest the salted digest, which is assumed to have been
- * previously decoded from Base64.
+ * @param digest the salted digest, which is assumed to have been
previously decoded from Base64.
* @return the password hash
- * @throws IllegalArgumentException if the length of the supplied digest is
- * less than or equal to 20 bytes
+ * @throws IllegalArgumentException if the length of the supplied digest
is less than or equal to 20 bytes
*/
- static byte[] extractPasswordHash(final byte[] digest, final int
hashLength ) throws IllegalArgumentException
- {
- if( digest.length < hashLength )
- {
+ static byte[] extractPasswordHash( final byte[] digest, final int
hashLength ) throws IllegalArgumentException {
+ if( digest.length < hashLength ) {
throw new IllegalArgumentException( "Hash was shorter than
expected; could not extract password hash!" );
}
// Extract the password hash
- final byte[] hash = new byte[hashLength];
- System.arraycopy(digest, 0, hash, 0, hashLength);
+ final byte[] hash = new byte[ hashLength ];
+ System.arraycopy( digest, 0, hash, 0, hashLength );
return hash;
}
/**
- * Helper method that extracts the salt from supplied salted digest by
taking all of the
- * characters after a given index.
+ * Helper method that extracts the salt from supplied salted digest by
taking all the characters after a given index.
*
- * @param digest the salted digest, which is assumed to have been
previously
- * decoded from Base64.
+ * @param digest the salted digest, which is assumed to have been
previously decoded from Base64.
* @return the salt
- * @throws IllegalArgumentException if the length of the supplied digest is
- * less than given length.
+ * @throws IllegalArgumentException if the length of the supplied digest
is less than given length.
*/
- static byte[] extractSalt(final byte[] digest, final int hashLength )
throws IllegalArgumentException
- {
- if( digest.length <= hashLength )
- {
+ static byte[] extractSalt( final byte[] digest, final int hashLength )
throws IllegalArgumentException {
+ if( digest.length <= hashLength ) {
throw new IllegalArgumentException( "Hash was shorter than
expected; we found no salt!" );
}
@@ -274,4 +244,5 @@ public final class CryptoUtil
return salt;
}
+
}
diff --git
a/jspwiki-util/src/test/java/org/apache/wiki/util/CryptoUtilTest.java
b/jspwiki-util/src/test/java/org/apache/wiki/util/CryptoUtilTest.java
index 58d7ce70d..b1724adaf 100644
--- a/jspwiki-util/src/test/java/org/apache/wiki/util/CryptoUtilTest.java
+++ b/jspwiki-util/src/test/java/org/apache/wiki/util/CryptoUtilTest.java
@@ -150,8 +150,7 @@ public class CryptoUtilTest {
Assertions.assertTrue( CryptoUtil.verifySaltedPassword( password,
"{SSHA}tAVisOOQGAeVyP8UMFQY9qi83lxsb09e" ) );
Assertions.assertTrue( CryptoUtil.verifySaltedPassword( password,
"{SSHA}BZaDYvB8czmNW3MjR2j7/mklODV0ZXN0eQ==" ) );
- // Verify with three consecutive random generations (based on
- // slappasswd)
+ // Verify with three consecutive random generations (based on
slappasswd)
password = "testPassword".getBytes();
Assertions.assertTrue( CryptoUtil.verifySaltedPassword( password,
"{SSHA}t2tfJHm/QZYUh0OZ8tkm05l2LLbuc3ZF" ) );
Assertions.assertTrue( CryptoUtil.verifySaltedPassword( password,
"{SSHA}0FKV9iM2cA5bAMws7mSgwg+zik/GT+wy" ) );
