Skip to content

Commit

Permalink
[SHIRO-290] Implement BCrypt and Argon2
Browse files Browse the repository at this point in the history 
[SHIRO-290] WIP: Implement Unix crypt format, starting with bcrypt.

  - TBD: HashRequest
  - TBD: PasswortMatcher doesn’t know about the new hash format yet

[SHIRO-290] Rework to use existing Shiro1CryptFormat.

  - Hashes can now compare themselves to a given password.
    Reviwers: Review method placement and HAsh class description.
  - removed hashrequest
  - removed UnixCryptFormat
  - API change: made salt not-nullable.
    Additional constructor is supplied for hashing without or with
    default salt, the former and other methods/fields using
    SimpleByteSource.empty().
    Reviewers: Pay attention to method logic, so no empty salt is
    being used where a former `null` value would have created a
    new, random salt.
  - Modified tests to not expect exceptions in certain cases.
  - Modified tests to not expect passwordService calls when supplying an
    existing hash.
  - TBD: Fix Javadocs
  - TBD: Fix Hasher utility
  - TBD: Deprecate old non-KDF hash classes

[SHIRO-290] Prepare argon2 implementation.

 - BCrypt iterations vs cost: make iterations return iterations
 - add validate methods

[SHIRO-290] Implement Argon2Hash.java.

 - expand iterations field to take a comma separated list. Maybe just create a Shiro2CryptFormat instead?
 - Hex and Base64 formats are not fixed. Maybe we can drop them?
 - Fixed parameter "algorithm name" not taken into account for bcrypt.
 - Allow Hasher to read from stdin
 - Added a short test for Hasher.java.
 - Changed default DefaultPasswordService.java algorithm to "Argon2id".

[SHIRO-290] Implement Shiro2CryptFormat.java.

 - Only fields 1 and two are defined, rest is defined by the hash implementation
 - Therefore fully backwards-compatible to Shiro1CryptFormat.java.
 - Loads formats from ProvidedKdfHashes.java.
   We could also think of a pluggable mechanism, like using service loaders to hide classes
   like OpenBSDBase64.
 - In AbstractCryptHash.java, renamed `version` to `algorithmName`.
 - Removed iterations from AbstractCryptHash.java, they are possibly an implementation detail not
   present in other implementations (like bcrypt).
 - Signature change: `PasswordService.encryptPassword(Object plaintext)` will now throw
   a NullPointerException on `null` parameter. It was never specified how this method would behave.

[SHIRO-290] Add hasher tests

 - fix invalid cost factor for bcrypt when input is 0.
 - output Hasher messages using slf4j.

[SHIRO-290] ServiceLoadable KDF algorithms.

  - Move BCrypt and Argon2 into their own modules
  - Add a SPI
  - Remove hardcoded parameters, replace with ParameterMap for the hashRequest

[SHIRO-290] implemented review comments

 - remove at least MD2, MD5 and Sha1
 - Remove unused support-hashes module
 - changed group and artifact-ids for new modules
 - fixed compilation issue in Hasher (needs more work though)
 - add "since 2.0" comments

[SHIRO-290] add some javadoc, make implementation classes package-private.

[SHIRO-290] doc updates
  • Loading branch information
@bmarwell
bmarwell committed Jan 13, 2021
1 parent cc1933a commit 1e15994
Show file tree
Hide file tree
Showing 72 changed files with 3,077 additions and 1,156 deletions.
20 changes: 19 additions & 1 deletion RELEASE-NOTES
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
###########################################################
Expand Down
10 changes: 10 additions & 0 deletions core/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
26 changes: 21 additions & 5 deletions core/src/main/java/org/apache/shiro/authc/SimpleAuthenticationInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;


Expand All @@ -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.
*/
Expand All @@ -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.
Expand Down Expand Up @@ -124,6 +127,7 @@ public SimpleAuthenticationInfo(PrincipalCollection principals, Object hashedCre
}


@Override
public PrincipalCollection getPrincipals() {
return principals;
}
Expand All @@ -137,6 +141,7 @@ public void setPrincipals(PrincipalCollection principals) {
this.principals = principals;
}

@Override
public Object getCredentials() {
return credentials;
}
Expand All @@ -163,6 +168,7 @@ public void setCredentials(Object credentials) {
* hashed at all.
* @since 1.1
*/
@Override
public ByteSource getCredentialsSalt() {
return credentialsSalt;
}
Expand All @@ -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()) {
Expand Down Expand Up @@ -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;
}
Expand All @@ -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);
}
Expand All @@ -275,6 +290,7 @@ public int hashCode() {
*
* @return <code>{@link #getPrincipals() principals}.toString()</code>
*/
@Override
public String toString() {
return principals.toString();
}
Expand Down
58 changes: 29 additions & 29 deletions core/src/main/java/org/apache/shiro/authc/credential/DefaultPasswordService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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()) {
Expand All @@ -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);

Expand All @@ -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) {
Expand Down Expand Up @@ -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);

Expand All @@ -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);
Expand All @@ -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;
}
Expand Down
32 changes: 17 additions & 15 deletions core/src/main/java/org/apache/shiro/authc/credential/HashedCredentialsMatcher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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
*/
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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);
}

/**
Expand Down
Loading

0 comments on commit 1e15994

Please sign in to comment.