[SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)

RELEASE-NOTES

@@ -17,9 +17,27 @@
 
 This is not an official release notes document.  It exists for Shiro developers
 to jot down their notes while working in the source code.  These notes will be
-combined with Jira's auto-generated release notes during a release for the
+combined with Jira’s auto-generated release notes during a release for the
 total set.
 
+###########################################################
+# 2.0.0
+###########################################################
+
+Improvement
+
+    [SHIRO-290] Implement bcrypt and argon2 KDF algorithms
+
+Backwards Incompatible Changes
+--------------------------------
+
+* Changed default DefaultPasswordService.java algorithm to "Argon2id".
+* PasswordService.encryptPassword(Object plaintext) will now throw a NullPointerException on null parameter.
+  It was never specified how this method would behave.
+* Made salt non-nullable.
+* Removed methods in PasswordMatcher.
+
+
 ###########################################################
 # 1.5.3
 ###########################################################

core/pom.xml

@@ -101,6 +101,16 @@
             <groupId>org.apache.shiro</groupId>
             <artifactId>shiro-crypto-hash</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.shiro.crypto</groupId>
+            <artifactId>shiro-hashes-argon2</artifactId>
+            <scope>runtime</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.shiro.crypto</groupId>
+            <artifactId>shiro-hashes-bcrypt</artifactId>
+            <scope>runtime</scope>
+        </dependency>
         <dependency>
             <groupId>org.apache.shiro</groupId>
             <artifactId>shiro-crypto-cipher</artifactId>

core/src/main/java/org/apache/shiro/authc/SimpleAuthenticationInfo.java

@@ -18,13 +18,15 @@
  */
 package org.apache.shiro.authc;
 
+import org.apache.shiro.lang.util.ByteSource;
+import org.apache.shiro.lang.util.SimpleByteSource;
 import org.apache.shiro.subject.MutablePrincipalCollection;
 import org.apache.shiro.subject.PrincipalCollection;
 import org.apache.shiro.subject.SimplePrincipalCollection;
-import org.apache.shiro.lang.util.ByteSource;
 
 import java.util.Collection;
 import java.util.HashSet;
+import java.util.Objects;
 import java.util.Set;
 
 
@@ -37,6 +39,7 @@
  */
 public class SimpleAuthenticationInfo implements MergableAuthenticationInfo, SaltedAuthenticationInfo {
 
+    private static final long serialVersionUID = 5390456512469696779L;
     /**
      * The principals identifying the account associated with this AuthenticationInfo instance.
      */
@@ -51,7 +54,7 @@ public class SimpleAuthenticationInfo implements MergableAuthenticationInfo, Sal
      *
      * @since 1.1
      */
-    protected ByteSource credentialsSalt;
+    protected ByteSource credentialsSalt = SimpleByteSource.empty();
 
     /**
      * Default no-argument constructor.
@@ -124,6 +127,7 @@ public SimpleAuthenticationInfo(PrincipalCollection principals, Object hashedCre
     }
 
 
+    @Override
     public PrincipalCollection getPrincipals() {
         return principals;
     }
@@ -137,6 +141,7 @@ public void setPrincipals(PrincipalCollection principals) {
         this.principals = principals;
     }
 
+    @Override
     public Object getCredentials() {
         return credentials;
     }
@@ -163,6 +168,7 @@ public void setCredentials(Object credentials) {
      *         hashed at all.
      * @since 1.1
      */
+    @Override
     public ByteSource getCredentialsSalt() {
         return credentialsSalt;
     }
@@ -189,6 +195,7 @@ public void setCredentialsSalt(ByteSource salt) {
      *
      * @param info the <code>AuthenticationInfo</code> to add into this instance.
      */
+    @Override
     @SuppressWarnings("unchecked")
     public void merge(AuthenticationInfo info) {
         if (info == null || info.getPrincipals() == null || info.getPrincipals().isEmpty()) {
@@ -249,14 +256,21 @@ public void merge(AuthenticationInfo info) {
      * @return <code>true</code> if the Object argument is an <code>instanceof SimpleAuthenticationInfo</code> and
      *         its {@link #getPrincipals() principals} are equal to this instance's principals, <code>false</code> otherwise.
      */
+    @Override
     public boolean equals(Object o) {
-        if (this == o) return true;
-        if (!(o instanceof SimpleAuthenticationInfo)) return false;
+        if (this == o) {
+            return true;
+        }
+        if (!(o instanceof SimpleAuthenticationInfo)) {
+            return false;
+        }
 
         SimpleAuthenticationInfo that = (SimpleAuthenticationInfo) o;
 
         //noinspection RedundantIfStatement
-        if (principals != null ? !principals.equals(that.principals) : that.principals != null) return false;
+        if (!Objects.equals(principals, that.principals)) {
+            return false;
+        }
 
         return true;
     }
@@ -266,6 +280,7 @@ public boolean equals(Object o) {
      *
      * @return the hashcode of the internal {@link #getPrincipals() principals} instance.
      */
+    @Override
     public int hashCode() {
         return (principals != null ? principals.hashCode() : 0);
     }
@@ -275,6 +290,7 @@ public int hashCode() {
      *
      * @return <code>{@link #getPrincipals() principals}.toString()</code>
      */
+    @Override
     public String toString() {
         return principals.toString();
     }

core/src/main/java/org/apache/shiro/authc/credential/DefaultPasswordService.java

@@ -18,32 +18,36 @@
  */
 package org.apache.shiro.authc.credential;
 
-import java.security.MessageDigest;
-
 import org.apache.shiro.crypto.hash.DefaultHashService;
 import org.apache.shiro.crypto.hash.Hash;
 import org.apache.shiro.crypto.hash.HashRequest;
 import org.apache.shiro.crypto.hash.HashService;
-import org.apache.shiro.crypto.hash.format.*;
+import org.apache.shiro.crypto.hash.format.DefaultHashFormatFactory;
+import org.apache.shiro.crypto.hash.format.HashFormat;
+import org.apache.shiro.crypto.hash.format.HashFormatFactory;
+import org.apache.shiro.crypto.hash.format.ParsableHashFormat;
+import org.apache.shiro.crypto.hash.format.Shiro2CryptFormat;
 import org.apache.shiro.lang.util.ByteSource;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.security.MessageDigest;
+
+import static java.util.Objects.requireNonNull;
+
 /**
  * Default implementation of the {@link PasswordService} interface that relies on an internal
  * {@link HashService}, {@link HashFormat}, and {@link HashFormatFactory} to function:
  * <h2>Hashing Passwords</h2>
  *
  * <h2>Comparing Passwords</h2>
- * All hashing operations are performed by the internal {@link #getHashService() hashService}.  After the hash
- * is computed, it is formatted into a String value via the internal {@link #getHashFormat() hashFormat}.
+ * All hashing operations are performed by the internal {@link #getHashService() hashService}.
  *
  * @since 1.2
  */
 public class DefaultPasswordService implements HashingPasswordService {
 
-    public static final String DEFAULT_HASH_ALGORITHM = "SHA-256";
-    public static final int DEFAULT_HASH_ITERATIONS = 500000; //500,000
+    public static final String DEFAULT_HASH_ALGORITHM = "argon2id";
 
     private static final Logger log = LoggerFactory.getLogger(DefaultPasswordService.class);
 
@@ -53,25 +57,33 @@ public class DefaultPasswordService implements HashingPasswordService {
 
     private volatile boolean hashFormatWarned; //used to avoid excessive log noise
 
+    /**
+     * Constructs a new PasswordService with a default hash service and the default
+     * algorithm name {@value #DEFAULT_HASH_ALGORITHM}, a default hash format (shiro2) and
+     * a default hashformat factory.
+     *
+     * <p>The default algorithm can change between minor versions and does not introduce
+     * API incompatibility by design.</p>
+     */
     public DefaultPasswordService() {
         this.hashFormatWarned = false;
 
         DefaultHashService hashService = new DefaultHashService();
-        hashService.setHashAlgorithmName(DEFAULT_HASH_ALGORITHM);
-        hashService.setHashIterations(DEFAULT_HASH_ITERATIONS);
-        hashService.setGeneratePublicSalt(true); //always want generated salts for user passwords to be most secure
+        hashService.setDefaultAlgorithmName(DEFAULT_HASH_ALGORITHM);
         this.hashService = hashService;
 
-        this.hashFormat = new Shiro1CryptFormat();
+        this.hashFormat = new Shiro2CryptFormat();
         this.hashFormatFactory = new DefaultHashFormatFactory();
     }
 
+    @Override
     public String encryptPassword(Object plaintext) {
-        Hash hash = hashPassword(plaintext);
+        Hash hash = hashPassword(requireNonNull(plaintext));
         checkHashFormatDurability();
         return this.hashFormat.format(hash);
     }
 
+    @Override
     public Hash hashPassword(Object plaintext) {
         ByteSource plaintextBytes = createByteSource(plaintext);
         if (plaintextBytes == null || plaintextBytes.isEmpty()) {
@@ -81,6 +93,7 @@ public Hash hashPassword(Object plaintext) {
         return hashService.computeHash(request);
     }
 
+    @Override
     public boolean passwordsMatch(Object plaintext, Hash saved) {
         ByteSource plaintextBytes = createByteSource(plaintext);
 
@@ -92,11 +105,7 @@ public boolean passwordsMatch(Object plaintext, Hash saved) {
             }
         }
 
-        HashRequest request = buildHashRequest(plaintextBytes, saved);
-
-        Hash computed = this.hashService.computeHash(request);
-
-        return constantEquals(saved.toString(), computed.toString());
+        return saved.matchesPassword(plaintextBytes);
     }
 
     private boolean constantEquals(String savedHash, String computedHash) {
@@ -133,6 +142,7 @@ protected ByteSource createByteSource(Object o) {
         return ByteSource.Util.bytes(o);
     }
 
+    @Override
     public boolean passwordsMatch(Object submittedPlaintext, String saved) {
         ByteSource plaintextBytes = createByteSource(submittedPlaintext);
 
@@ -151,9 +161,9 @@ public boolean passwordsMatch(Object submittedPlaintext, String saved) {
         //configuration changes.
         HashFormat discoveredFormat = this.hashFormatFactory.getInstance(saved);
 
-        if (discoveredFormat != null && discoveredFormat instanceof ParsableHashFormat) {
+        if (discoveredFormat instanceof ParsableHashFormat) {
 
-            ParsableHashFormat parsableHashFormat = (ParsableHashFormat)discoveredFormat;
+            ParsableHashFormat parsableHashFormat = (ParsableHashFormat) discoveredFormat;
             Hash savedHash = parsableHashFormat.parse(saved);
 
             return passwordsMatch(submittedPlaintext, savedHash);
@@ -174,16 +184,6 @@ public boolean passwordsMatch(Object submittedPlaintext, String saved) {
         return constantEquals(saved, formatted);
     }
 
-    protected HashRequest buildHashRequest(ByteSource plaintext, Hash saved) {
-        //keep everything from the saved hash except for the source:
-        return new HashRequest.Builder().setSource(plaintext)
-                //now use the existing saved data:
-                .setAlgorithmName(saved.getAlgorithmName())
-                .setSalt(saved.getSalt())
-                .setIterations(saved.getIterations())
-                .build();
-    }
-
     public HashService getHashService() {
         return hashService;
     }

core/src/main/java/org/apache/shiro/authc/credential/HashedCredentialsMatcher.java

@@ -21,13 +21,16 @@
 import org.apache.shiro.authc.AuthenticationInfo;
 import org.apache.shiro.authc.AuthenticationToken;
 import org.apache.shiro.authc.SaltedAuthenticationInfo;
-import org.apache.shiro.lang.codec.Base64;
-import org.apache.shiro.lang.codec.Hex;
 import org.apache.shiro.crypto.hash.AbstractHash;
 import org.apache.shiro.crypto.hash.Hash;
 import org.apache.shiro.crypto.hash.SimpleHash;
+import org.apache.shiro.lang.codec.Base64;
+import org.apache.shiro.lang.codec.Hex;
+import org.apache.shiro.lang.util.SimpleByteSource;
 import org.apache.shiro.lang.util.StringUtils;
 
+import static java.util.Objects.requireNonNull;
+
 /**
  * A {@code HashedCredentialMatcher} provides support for hashing of supplied {@code AuthenticationToken} credentials
  * before being compared to those in the {@code AuthenticationInfo} from the data store.
@@ -49,10 +52,7 @@
  * and multiple hash iterations.  Please read this excellent
  * <a href="http://www.owasp.org/index.php/Hashing_Java" _target="blank">Hashing Java article</a> to learn about
  * salting and multiple iterations and why you might want to use them. (Note of sections 5
- * &quot;Why add salt?&quot; and 6 "Hardening against the attacker's attack").   We should also note here that all of
- * Shiro's Hash implementations (for example, {@link org.apache.shiro.crypto.hash.Md5Hash Md5Hash},
- * {@link org.apache.shiro.crypto.hash.Sha1Hash Sha1Hash}, etc) support salting and multiple hash iterations via
- * overloaded constructors.
+ * &quot;Why add salt?&quot; and 6 "Hardening against the attacker's attack").</p>
  * <h4>Real World Case Study</h4>
  * In April 2010, some public Atlassian Jira and Confluence
  * installations (Apache Software Foundation, Codehaus, etc) were the target of account attacks and user accounts
@@ -112,8 +112,8 @@
  * two, if your application mandates high security, use the SHA-256 (or higher) hashing algorithms and their
  * supporting {@code CredentialsMatcher} implementations.
  *
- * @see org.apache.shiro.crypto.hash.Md5Hash
- * @see org.apache.shiro.crypto.hash.Sha1Hash
+ * @see org.apache.shiro.crypto.hash.Sha256Hash
+ * @see org.apache.shiro.crypto.hash.Sha384Hash
  * @see org.apache.shiro.crypto.hash.Sha256Hash
  * @since 0.9
  */
@@ -341,6 +341,7 @@ protected Object getSalt(AuthenticationToken token) {
      * @param info the AuthenticationInfo from which to retrieve the credentials which assumed to be in already-hashed form.
      * @return a {@link Hash Hash} instance representing the given AuthenticationInfo's stored credentials.
      */
+    @Override
     protected Object getCredentials(AuthenticationInfo info) {
         Object credentials = info.getCredentials();
 
@@ -400,14 +401,14 @@ public boolean doCredentialsMatch(AuthenticationToken token, AuthenticationInfo
      * @since 1.1
      */
     protected Object hashProvidedCredentials(AuthenticationToken token, AuthenticationInfo info) {
-        Object salt = null;
+        final Object salt;
         if (info instanceof SaltedAuthenticationInfo) {
             salt = ((SaltedAuthenticationInfo) info).getCredentialsSalt();
-        } else {
+        } else if (isHashSalted()) {
             //retain 1.0 backwards compatibility:
-            if (isHashSalted()) {
-                salt = getSalt(token);
-            }
+            salt = getSalt(token);
+        } else {
+            salt = SimpleByteSource.empty();
         }
         return hashProvidedCredentials(token.getCredentials(), salt, getHashIterations());
     }
@@ -435,14 +436,15 @@ private String assertHashAlgorithmName() throws IllegalStateException {
      * implementation/algorithm used is based on the {@link #getHashAlgorithmName() hashAlgorithmName} property.
      *
      * @param credentials    the submitted authentication token's credentials to hash
-     * @param salt           the value to salt the hash, or {@code null} if a salt will not be used.
+     * @param salt           the value to salt the hash. Cannot be {@code null}, but an empty ByteSource.
      * @param hashIterations the number of times to hash the credentials.  At least one hash will always occur though,
      *                       even if this argument is 0 or negative.
      * @return the hashed value of the provided credentials, according to the specified salt and hash iterations.
+     * @throws NullPointerException if salt is {@code null}.
      */
     protected Hash hashProvidedCredentials(Object credentials, Object salt, int hashIterations) {
         String hashAlgorithmName = assertHashAlgorithmName();
-        return new SimpleHash(hashAlgorithmName, credentials, salt, hashIterations);
+        return new SimpleHash(hashAlgorithmName, credentials, requireNonNull(salt, "salt cannot be null."), hashIterations);
     }
 
     /**