You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shiro.apache.org by GitBox <gi...@apache.org> on 2020/12/31 12:59:41 UTC
[GitHub] [shiro] bmarwell opened a new pull request #273: [SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)
bmarwell opened a new pull request #273:
URL: https://github.com/apache/shiro/pull/273
Following this checklist to help us incorporate your contribution quickly and easily:
- [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/SHIRO) filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without pulling in other changes.
- [ ] Each commit in the pull request should have a meaningful subject line and body.
- [ ] Format the pull request title like `[SHIRO-XXX] - Fixes bug in SessionManager`,
where you replace `SHIRO-XXX` with the appropriate JIRA issue. Best practice
is to use the JIRA issue title in the pull request title and in the first line of the commit message.
- [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [ ] Run `mvn clean install apache-rat:check` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
- [ ] If you have a group of commits related to the same change, please squash your commits into one and force push your branch using `git rebase -i`.
Trivial changes like typos do not require a JIRA issue (javadoc, comments...).
In this case, just format the pull request title like `(DOC) - Add javadoc in SessionManager`.
If this is your first contribution, you have to read the [Contribution Guidelines](https://github.com/apache/shiro/blob/master/CONTRIBUTING.md)
If your pull request is about ~20 lines of code you don't need to sign an [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf)
if you are unsure please ask on the developers list.
To make clear that you license your contribution under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [ ] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] bdemers commented on a change in pull request #273: [SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)
Posted by GitBox <gi...@apache.org>.
bdemers commented on a change in pull request #273:
URL: https://github.com/apache/shiro/pull/273#discussion_r554012683
##########
File path: crypto/hash/src/main/java/org/apache/shiro/crypto/hash/ConfigurableHashService.java
##########
@@ -18,44 +18,20 @@
*/
package org.apache.shiro.crypto.hash;
-import org.apache.shiro.crypto.RandomNumberGenerator;
-import org.apache.shiro.lang.util.ByteSource;
-
/**
* A {@code HashService} that allows configuration of its strategy via JavaBeans-compatible setter methods.
*
* @since 1.2
*/
public interface ConfigurableHashService extends HashService {
- /**
- * Sets the 'private' (internal) salt to be paired with a 'public' (random or supplied) salt during hash computation.
- *
- * @param privateSalt the 'private' internal salt to be paired with a 'public' (random or supplied) salt during
- * hash computation.
- */
- void setPrivateSalt(ByteSource privateSalt);
-
- /**
- * Sets the number of hash iterations that will be performed during hash computation.
- *
- * @param iterations the number of hash iterations that will be performed during hash computation.
- */
- void setHashIterations(int iterations);
-
/**
* Sets the name of the {@link java.security.MessageDigest MessageDigest} algorithm that will be used to compute
* hashes.
*
* @param name the name of the {@link java.security.MessageDigest MessageDigest} algorithm that will be used to
* compute hashes.
*/
- void setHashAlgorithmName(String name);
+ void setDefaultAlgorithmName(String name);
Review comment:
nit: might need to tweak the javadoc to go with this, I'm assuming this is the alg used if the hash cannot be determined?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] bmarwell commented on a change in pull request #273: [SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)
Posted by GitBox <gi...@apache.org>.
bmarwell commented on a change in pull request #273:
URL: https://github.com/apache/shiro/pull/273#discussion_r554347551
##########
File path: tools/hasher/pom.xml
##########
@@ -50,7 +50,11 @@
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
- <artifactId>slf4j-simple</artifactId>
+ <artifactId>slf4j-api</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>ch.qos.logback</groupId>
+ <artifactId>logback-classic</artifactId>
Review comment:
It is not so funny actually. See the chat in slack. To be able test things written to `System.out`, we need `logback-classic`, as the `simple-logger` cannot be intercepted.
Romain suggested to refactor the hasher utility a bit further, and I would like to do that in another PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] bmarwell closed pull request #273: [SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)
Posted by GitBox <gi...@apache.org>.
bmarwell closed pull request #273:
URL: https://github.com/apache/shiro/pull/273
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] bmarwell commented on a change in pull request #273: [SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)
Posted by GitBox <gi...@apache.org>.
bmarwell commented on a change in pull request #273:
URL: https://github.com/apache/shiro/pull/273#discussion_r554342422
##########
File path: crypto/hash/src/main/java/org/apache/shiro/crypto/hash/ConfigurableHashService.java
##########
@@ -18,44 +18,20 @@
*/
package org.apache.shiro.crypto.hash;
-import org.apache.shiro.crypto.RandomNumberGenerator;
-import org.apache.shiro.lang.util.ByteSource;
-
/**
* A {@code HashService} that allows configuration of its strategy via JavaBeans-compatible setter methods.
*
* @since 1.2
*/
public interface ConfigurableHashService extends HashService {
- /**
- * Sets the 'private' (internal) salt to be paired with a 'public' (random or supplied) salt during hash computation.
- *
- * @param privateSalt the 'private' internal salt to be paired with a 'public' (random or supplied) salt during
- * hash computation.
- */
- void setPrivateSalt(ByteSource privateSalt);
-
- /**
- * Sets the number of hash iterations that will be performed during hash computation.
- *
- * @param iterations the number of hash iterations that will be performed during hash computation.
- */
- void setHashIterations(int iterations);
-
/**
* Sets the name of the {@link java.security.MessageDigest MessageDigest} algorithm that will be used to compute
* hashes.
*
* @param name the name of the {@link java.security.MessageDigest MessageDigest} algorithm that will be used to
* compute hashes.
*/
- void setHashAlgorithmName(String name);
+ void setDefaultAlgorithmName(String name);
Review comment:
Yes, the javadocs are the unchecked checkbox ;) Next commit is on the way.
The default is being used when a `HashRequest` does not carry an algorithm name. Its old behaviour/semantics was at least similar, but I was not looking into making it a 100% compatible here.
##########
File path: crypto/hash/src/main/java/org/apache/shiro/crypto/hash/DefaultHashService.java
##########
@@ -123,222 +71,52 @@ public DefaultHashService() {
* <p/>
* A salt will be generated and used to compute the hash. The salt is generated as follows:
* <ol>
- * <li>Use the {@link #getRandomNumberGenerator() randomNumberGenerator} to generate a new random number.</li>
- * <li>{@link #combine(ByteSource, ByteSource) combine} this random salt with any configured
- * {@link #getPrivateSalt() privateSalt}
- * </li>
* <li>Use the combined value as the salt used during hash computation</li>
* </ol>
* </li>
* <li>
- * If the request salt is not null:
- * <p/>
- * This indicates that the hash computation is for comparison purposes (of a
- * previously computed hash). The request salt will be {@link #combine(ByteSource, ByteSource) combined} with any
- * configured {@link #getPrivateSalt() privateSalt} and used as the complete salt during hash computation.
- * </li>
- * </ul>
- * <p/>
- * The returned {@code Hash}'s {@link Hash#getSalt() salt} property
- * will contain <em>only</em> the 'public' part of the salt and <em>NOT</em> the privateSalt. See the class-level
- * JavaDoc explanation for more info.
*
* @param request the request to process
* @return the response containing the result of the hash computation, as well as any hash salt used that should be
* exposed to the caller.
*/
+ @Override
public Hash computeHash(HashRequest request) {
if (request == null || request.getSource() == null || request.getSource().isEmpty()) {
return null;
}
String algorithmName = getAlgorithmName(request);
- ByteSource source = request.getSource();
- int iterations = getIterations(request);
-
- ByteSource publicSalt = getPublicSalt(request);
- ByteSource privateSalt = getPrivateSalt();
- ByteSource salt = combine(privateSalt, publicSalt);
-
- Hash computed = new SimpleHash(algorithmName, source, salt, iterations);
-
- SimpleHash result = new SimpleHash(algorithmName);
- result.setBytes(computed.getBytes());
- result.setIterations(iterations);
- //Only expose the public salt - not the real/combined salt that might have been used:
- result.setSalt(publicSalt);
-
- return result;
- }
-
- protected String getAlgorithmName(HashRequest request) {
- String name = request.getAlgorithmName();
- if (name == null) {
- name = getHashAlgorithmName();
- }
- return name;
- }
-
- protected int getIterations(HashRequest request) {
- int iterations = Math.max(0, request.getIterations());
- if (iterations < 1) {
- iterations = Math.max(1, getHashIterations());
- }
- return iterations;
- }
-
- /**
- * Returns the public salt that should be used to compute a hash based on the specified request or
- * {@code null} if no public salt should be used.
- * <p/>
- * This implementation functions as follows:
- * <ol>
- * <li>If the request salt is not null and non-empty, this will be used, return it.</li>
- * <li>If the request salt is null or empty:
- * <ol>
- * <li>If a private salt has been set <em>OR</em> {@link #isGeneratePublicSalt()} is {@code true},
- * auto generate a random public salt via the configured
- * {@link #getRandomNumberGenerator() randomNumberGenerator}.</li>
- * <li>If a private salt has not been configured and {@link #isGeneratePublicSalt()} is {@code false},
- * do nothing - return {@code null} to indicate a salt should not be used during hash computation.</li>
- * </ol>
- * </li>
- * </ol>
- *
- * @param request request the request to process
- * @return the public salt that should be used to compute a hash based on the specified request or
- * {@code null} if no public salt should be used.
- */
- protected ByteSource getPublicSalt(HashRequest request) {
-
- ByteSource publicSalt = request.getSalt();
-
- if (publicSalt != null && !publicSalt.isEmpty()) {
- //a public salt was explicitly requested to be used - go ahead and use it:
- return publicSalt;
- }
-
- publicSalt = null;
- //check to see if we need to generate one:
- ByteSource privateSalt = getPrivateSalt();
- boolean privateSaltExists = privateSalt != null && !privateSalt.isEmpty();
+ Optional<HashSpi<? extends Hash>> kdfHash = HashProvider.getByAlgorithmName(algorithmName);
+ if (kdfHash.isPresent()) {
+ HashSpi<?> hashSpi = kdfHash.orElseThrow(NoSuchElementException::new);
- //If a private salt exists, we must generate a public salt to protect the integrity of the private salt.
- //Or generate it if the instance is explicitly configured to do so:
- if (privateSaltExists || isGeneratePublicSalt()) {
- publicSalt = getRandomNumberGenerator().nextBytes();
+ return hashSpi.newHashFactory(random).generate(request);
}
- return publicSalt;
+ throw new UnsupportedOperationException("Cannot create a hash with the given algorithm: " + algorithmName);
}
- /**
- * Combines the specified 'private' salt bytes with the specified additional extra bytes to use as the
- * total salt during hash computation. {@code privateSaltBytes} will be {@code null} }if no private salt has been
- * configured.
- *
- * @param privateSalt the (possibly {@code null}) 'private' salt to combine with the specified extra bytes
- * @param publicSalt the extra bytes to use in addition to the given private salt.
- * @return a combination of the specified private salt bytes and extra bytes that will be used as the total
- * salt during hash computation.
- */
- protected ByteSource combine(ByteSource privateSalt, ByteSource publicSalt) {
-
- byte[] privateSaltBytes = privateSalt != null ? privateSalt.getBytes() : null;
- int privateSaltLength = privateSaltBytes != null ? privateSaltBytes.length : 0;
-
- byte[] publicSaltBytes = publicSalt != null ? publicSalt.getBytes() : null;
- int extraBytesLength = publicSaltBytes != null ? publicSaltBytes.length : 0;
-
- int length = privateSaltLength + extraBytesLength;
- if (length <= 0) {
- return null;
- }
-
- byte[] combined = new byte[length];
-
- int i = 0;
- for (int j = 0; j < privateSaltLength; j++) {
- assert privateSaltBytes != null;
- combined[i++] = privateSaltBytes[j];
- }
- for (int j = 0; j < extraBytesLength; j++) {
- assert publicSaltBytes != null;
- combined[i++] = publicSaltBytes[j];
- }
-
- return ByteSource.Util.bytes(combined);
- }
-
- public void setHashAlgorithmName(String name) {
- this.algorithmName = name;
- }
-
- public String getHashAlgorithmName() {
- return this.algorithmName;
- }
-
- public void setPrivateSalt(ByteSource privateSalt) {
- this.privateSalt = privateSalt;
- }
-
- public ByteSource getPrivateSalt() {
- return this.privateSalt;
- }
-
- public void setHashIterations(int count) {
- this.iterations = count;
- }
-
- public int getHashIterations() {
- return this.iterations;
+ protected String getAlgorithmName(HashRequest request) {
+ return request.getAlgorithmName().orElseGet(this::getDefaultAlgorithmName);
}
- public void setRandomNumberGenerator(RandomNumberGenerator rng) {
- this.rng = rng;
+ @Override
+ public void setDefaultAlgorithmName(String name) {
+ this.defaultAlgorithmName = name;
}
- public RandomNumberGenerator getRandomNumberGenerator() {
- return this.rng;
+ public String getDefaultAlgorithmName() {
+ return this.defaultAlgorithmName;
}
- /**
- * Returns {@code true} if a public salt should be randomly generated and used to compute a hash if a
- * {@link HashRequest} does not specify a salt, {@code false} otherwise.
- * <p/>
- * The default value is {@code false} but should definitely be set to {@code true} if the
- * {@code HashService} instance is being used for password hashing.
- * <p/>
- * <b>NOTE:</b> this property only has an effect if a {@link #getPrivateSalt() privateSalt} is NOT configured. If a
- * private salt has been configured and a request does not provide a salt, a random salt will always be generated
- * to protect the integrity of the private salt (without a public salt, the private salt would be exposed as-is,
- * which is undesirable).
- *
- * @return {@code true} if a public salt should be randomly generated and used to compute a hash if a
- * {@link HashRequest} does not specify a salt, {@code false} otherwise.
- */
- public boolean isGeneratePublicSalt() {
- return generatePublicSalt;
+ public Random getRandom() {
+ return random;
}
- /**
- * Sets whether or not a public salt should be randomly generated and used to compute a hash if a
- * {@link HashRequest} does not specify a salt.
- * <p/>
- * The default value is {@code false} but should definitely be set to {@code true} if the
- * {@code HashService} instance is being used for password hashing.
- * <p/>
- * <b>NOTE:</b> this property only has an effect if a {@link #getPrivateSalt() privateSalt} is NOT configured. If a
- * private salt has been configured and a request does not provide a salt, a random salt will always be generated
- * to protect the integrity of the private salt (without a public salt, the private salt would be exposed as-is,
- * which is undesirable).
- *
- * @param generatePublicSalt whether or not a public salt should be randomly generated and used to compute a hash
- * if a {@link HashRequest} does not specify a salt.
- */
- public void setGeneratePublicSalt(boolean generatePublicSalt) {
- this.generatePublicSalt = generatePublicSalt;
+ public void setRandom(Random random) {
+ this.random = random;
Review comment:
Not even for testing. Removing it until we need it.
##########
File path: crypto/hash/src/main/java/org/apache/shiro/crypto/hash/Md2Hash.java
##########
@@ -30,10 +30,13 @@
* techniques and how the overloaded constructors function.
*
* @since 0.9
+ * @deprecated and will throw exceptions since 2.0.0, to be removed in 2.1.0.
Review comment:
+1 for ripping out. We should nag the users in 1.7+ then.
And not just for MD2. For all iteration+salt+pepper based hashes, even if there is no alternative in shiro 1. Seriously.
##########
File path: tools/hasher/pom.xml
##########
@@ -50,7 +50,11 @@
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
- <artifactId>slf4j-simple</artifactId>
+ <artifactId>slf4j-api</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>ch.qos.logback</groupId>
+ <artifactId>logback-classic</artifactId>
Review comment:
It is not so funny actually. See the chat in slack. To be able test things written to `System.out`, we need `logback-classic`, as the `simple-logger` cannot be intercepted.
Romain suggested to refactor the hasher utility a bit further, and I would like to do that in another PR.
##########
File path: crypto/hash/src/main/java/org/apache/shiro/crypto/hash/HashProvider.java
##########
@@ -0,0 +1,60 @@
+/*
+ * 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.shiro.crypto.hash;
+
+import java.util.Optional;
+import java.util.ServiceLoader;
+import java.util.stream.StreamSupport;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Hashes used by the Shiro2CryptFormat class.
+ *
+ * <p>Instead of maintaining them as an {@code Enum}, ServiceLoaders would provide a pluggable alternative.</p>
+ */
+public final class HashProvider {
+
+ private HashProvider() {
+ // utility class
+ }
+
+ /**
+ * Find a KDF implementation by searching the algorithms.
+ *
+ * @param algorithmName the algorithmName to match. This is case-sensitive.
+ * @return an instance of {@link HashProvider} if found, otherwise {@link Optional#empty()}.
+ * @throws NullPointerException if the given parameter algorithmName is {@code null}.
+ */
+ public static Optional<HashSpi<? extends Hash>> getByAlgorithmName(String algorithmName) {
+ requireNonNull(algorithmName, "algorithmName in HashProvider.getByAlgorithmName");
+ ServiceLoader<HashSpi<? extends Hash>> hashSpis = load();
+
+ return StreamSupport.stream(hashSpis.spliterator(), false)
+ .filter(hashSpi -> hashSpi.getImplementedAlgorithms().contains(algorithmName))
+ .findAny();
+ }
+
+ @SuppressWarnings("unchecked")
+ private static ServiceLoader<HashSpi<? extends Hash>> load() {
+ return (ServiceLoader<HashSpi<? extends Hash>>) (Object) ServiceLoader.load(HashSpi.class);
Review comment:
What bugs me most is the ugly casting to object and back. But with what I saw from your `jjwt` example, adding a parameter might help with that!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] bdemers commented on a change in pull request #273: [SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)
Posted by GitBox <gi...@apache.org>.
bdemers commented on a change in pull request #273:
URL: https://github.com/apache/shiro/pull/273#discussion_r554024642
##########
File path: crypto/support/hashes/bcrypt/src/main/java/org/apache/shiro/crypto/support/hashes/bcrypt/OpenBSDBase64.java
##########
@@ -0,0 +1,194 @@
+/*
+ * 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.shiro.crypto.support.hashes.bcrypt;
+
+
+/**
+ * Encoder for the custom Base64 variant of BCrypt (called Radix64 here). It has the same rules as Base64 but uses a
+ * different mapping table than the various RFCs
+ * <p>
+ * According to Wikipedia:
+ *
+ * <blockquote>
+ * Unix stores password hashes computed with crypt in the /etc/passwd file using radix-64 encoding called B64. It uses a
+ * mostly-alphanumeric set of characters, plus . and /. Its 64-character set is "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz".
+ * Padding is not used.
+ * </blockquote>
+ */
+public interface OpenBSDBase64 {
+
+
+ /**
+ * Encode given raw byte array to a Radix64 style, UTF-8 encoded byte array.
+ *
+ * @param rawBytes to encode
+ * @return UTF-8 encoded string representing radix64 encoded data
+ */
+ byte[] encode(byte[] rawBytes);
+
+ /**
+ * From a UTF-8 encoded string representing radix64 encoded data as byte array, decodes the raw bytes from it.
+ *
+ * @param utf8EncodedRadix64String from a string get it with <code>"m0CrhHm10qJ3lXRY.5zDGO".getBytes(StandardCharsets.UTF8)</code>
+ * @return the raw bytes encoded by this utf-8 radix4 string
+ */
+ byte[] decode(byte[] utf8EncodedRadix64String);
+
+ /*
+ * 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.
+ */
+
+ /**
+ * A mod of Square's Okio Base64 encoder
+ * <p>
+ * Original author: Alexander Y. Kleymenov
Review comment:
If not already make sure this gets added to the NOTICE for this module
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] bdemers commented on a change in pull request #273: [SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)
Posted by GitBox <gi...@apache.org>.
bdemers commented on a change in pull request #273:
URL: https://github.com/apache/shiro/pull/273#discussion_r554021392
##########
File path: crypto/hash/src/main/java/org/apache/shiro/crypto/hash/Md2Hash.java
##########
@@ -30,10 +30,13 @@
* techniques and how the overloaded constructors function.
*
* @since 0.9
+ * @deprecated and will throw exceptions since 2.0.0, to be removed in 2.1.0.
Review comment:
we can also mark them deprecated now, and then remove all the deprecated bits before the official 2.0 release if you are on the fence about anything.
Another idea, is we could nag users: print a log message: "WARN: Seriously, stop using MD2, here is why <insert some URL here>" 👺
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] bdemers commented on a change in pull request #273: [SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)
Posted by GitBox <gi...@apache.org>.
bdemers commented on a change in pull request #273:
URL: https://github.com/apache/shiro/pull/273#discussion_r554024642
##########
File path: crypto/support/hashes/bcrypt/src/main/java/org/apache/shiro/crypto/support/hashes/bcrypt/OpenBSDBase64.java
##########
@@ -0,0 +1,194 @@
+/*
+ * 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.shiro.crypto.support.hashes.bcrypt;
+
+
+/**
+ * Encoder for the custom Base64 variant of BCrypt (called Radix64 here). It has the same rules as Base64 but uses a
+ * different mapping table than the various RFCs
+ * <p>
+ * According to Wikipedia:
+ *
+ * <blockquote>
+ * Unix stores password hashes computed with crypt in the /etc/passwd file using radix-64 encoding called B64. It uses a
+ * mostly-alphanumeric set of characters, plus . and /. Its 64-character set is "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz".
+ * Padding is not used.
+ * </blockquote>
+ */
+public interface OpenBSDBase64 {
+
+
+ /**
+ * Encode given raw byte array to a Radix64 style, UTF-8 encoded byte array.
+ *
+ * @param rawBytes to encode
+ * @return UTF-8 encoded string representing radix64 encoded data
+ */
+ byte[] encode(byte[] rawBytes);
+
+ /**
+ * From a UTF-8 encoded string representing radix64 encoded data as byte array, decodes the raw bytes from it.
+ *
+ * @param utf8EncodedRadix64String from a string get it with <code>"m0CrhHm10qJ3lXRY.5zDGO".getBytes(StandardCharsets.UTF8)</code>
+ * @return the raw bytes encoded by this utf-8 radix4 string
+ */
+ byte[] decode(byte[] utf8EncodedRadix64String);
+
+ /*
+ * 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.
+ */
+
+ /**
+ * A mod of Square's Okio Base64 encoder
+ * <p>
+ * Original author: Alexander Y. Kleymenov
Review comment:
~~If not already make sure this gets added to the NOTICE for this module~~
never minde, i just needed to keep scrolling
##########
File path: crypto/support/hashes/bcrypt/src/main/java/org/apache/shiro/crypto/support/hashes/bcrypt/OpenBSDBase64.java
##########
@@ -0,0 +1,194 @@
+/*
+ * 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.shiro.crypto.support.hashes.bcrypt;
+
+
+/**
+ * Encoder for the custom Base64 variant of BCrypt (called Radix64 here). It has the same rules as Base64 but uses a
+ * different mapping table than the various RFCs
+ * <p>
+ * According to Wikipedia:
+ *
+ * <blockquote>
+ * Unix stores password hashes computed with crypt in the /etc/passwd file using radix-64 encoding called B64. It uses a
+ * mostly-alphanumeric set of characters, plus . and /. Its 64-character set is "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz".
+ * Padding is not used.
+ * </blockquote>
+ */
+public interface OpenBSDBase64 {
+
+
+ /**
+ * Encode given raw byte array to a Radix64 style, UTF-8 encoded byte array.
+ *
+ * @param rawBytes to encode
+ * @return UTF-8 encoded string representing radix64 encoded data
+ */
+ byte[] encode(byte[] rawBytes);
+
+ /**
+ * From a UTF-8 encoded string representing radix64 encoded data as byte array, decodes the raw bytes from it.
+ *
+ * @param utf8EncodedRadix64String from a string get it with <code>"m0CrhHm10qJ3lXRY.5zDGO".getBytes(StandardCharsets.UTF8)</code>
+ * @return the raw bytes encoded by this utf-8 radix4 string
+ */
+ byte[] decode(byte[] utf8EncodedRadix64String);
+
+ /*
+ * 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.
+ */
+
+ /**
+ * A mod of Square's Okio Base64 encoder
+ * <p>
+ * Original author: Alexander Y. Kleymenov
Review comment:
~~If not already make sure this gets added to the NOTICE for this module~~
never mind, i just needed to keep scrolling
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] bdemers commented on a change in pull request #273: [SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)
Posted by GitBox <gi...@apache.org>.
bdemers commented on a change in pull request #273:
URL: https://github.com/apache/shiro/pull/273#discussion_r554020300
##########
File path: crypto/hash/src/main/java/org/apache/shiro/crypto/hash/Md2Hash.java
##########
@@ -30,10 +30,13 @@
* techniques and how the overloaded constructors function.
*
* @since 0.9
+ * @deprecated and will throw exceptions since 2.0.0, to be removed in 2.1.0.
Review comment:
Rip anything out in 2.0, removing anything is technically a breaking change. (it's easy to add back in later if/when needed)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] bdemers commented on a change in pull request #273: [SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)
Posted by GitBox <gi...@apache.org>.
bdemers commented on a change in pull request #273:
URL: https://github.com/apache/shiro/pull/273#discussion_r554009848
##########
File path: core/pom.xml
##########
@@ -101,6 +101,14 @@
<groupId>org.apache.shiro</groupId>
<artifactId>shiro-crypto-hash</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.apache.shiro</groupId>
+ <artifactId>shiro-crypto-support-hashes-argon2</artifactId>
Review comment:
nit: he groupId could be `org.apache.shiro.hashes` and artifactId: `shiro-crypto-argon2`
Or, `org.apache.shiro.crypto:shiro-hashes-argon2`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] bmarwell commented on pull request #273: [SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)
Posted by GitBox <gi...@apache.org>.
bmarwell commented on pull request #273:
URL: https://github.com/apache/shiro/pull/273#issuecomment-756384834
About `HexFormat` and `Base64Format`:
> `HexFormat.java`
> HashFormat that outputs only The hash's digest bytes in hex format. It does not print out anything else (salt, iterations, etc). This implementation is mostly provided as a convenience for command-line hashing.
and
> `Base64Format.java`
> HashFormat that outputs only the hash's digest bytes in Base64 format. It does not print out anything else (salt, iterations, etc). This implementation is mostly provided as a convenience for command-line hashing.
my +1 for dropping them:
1. for 2.0.0 mark them as deprecated and log warnings when being used.
2. for 2.1.0 throw exceptions when being used
3. for 2.2.0 complete removal
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] bmarwell commented on a change in pull request #273: [SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)
Posted by GitBox <gi...@apache.org>.
bmarwell commented on a change in pull request #273:
URL: https://github.com/apache/shiro/pull/273#discussion_r556527645
##########
File path: crypto/hash/src/main/java/org/apache/shiro/crypto/hash/HashProvider.java
##########
@@ -0,0 +1,60 @@
+/*
+ * 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.shiro.crypto.hash;
+
+import java.util.Optional;
+import java.util.ServiceLoader;
+import java.util.stream.StreamSupport;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Hashes used by the Shiro2CryptFormat class.
+ *
+ * <p>Instead of maintaining them as an {@code Enum}, ServiceLoaders would provide a pluggable alternative.</p>
+ */
+public final class HashProvider {
+
+ private HashProvider() {
+ // utility class
+ }
+
+ /**
+ * Find a KDF implementation by searching the algorithms.
+ *
+ * @param algorithmName the algorithmName to match. This is case-sensitive.
+ * @return an instance of {@link HashProvider} if found, otherwise {@link Optional#empty()}.
+ * @throws NullPointerException if the given parameter algorithmName is {@code null}.
+ */
+ public static Optional<HashSpi<? extends Hash>> getByAlgorithmName(String algorithmName) {
+ requireNonNull(algorithmName, "algorithmName in HashProvider.getByAlgorithmName");
+ ServiceLoader<HashSpi<? extends Hash>> hashSpis = load();
+
+ return StreamSupport.stream(hashSpis.spliterator(), false)
+ .filter(hashSpi -> hashSpi.getImplementedAlgorithms().contains(algorithmName))
+ .findAny();
+ }
+
+ @SuppressWarnings("unchecked")
+ private static ServiceLoader<HashSpi<? extends Hash>> load() {
+ return (ServiceLoader<HashSpi<? extends Hash>>) (Object) ServiceLoader.load(HashSpi.class);
Review comment:
I removed the generic type.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] bmarwell commented on a change in pull request #273: [SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)
Posted by GitBox <gi...@apache.org>.
bmarwell commented on a change in pull request #273:
URL: https://github.com/apache/shiro/pull/273#discussion_r554344737
##########
File path: crypto/hash/src/main/java/org/apache/shiro/crypto/hash/DefaultHashService.java
##########
@@ -123,222 +71,52 @@ public DefaultHashService() {
* <p/>
* A salt will be generated and used to compute the hash. The salt is generated as follows:
* <ol>
- * <li>Use the {@link #getRandomNumberGenerator() randomNumberGenerator} to generate a new random number.</li>
- * <li>{@link #combine(ByteSource, ByteSource) combine} this random salt with any configured
- * {@link #getPrivateSalt() privateSalt}
- * </li>
* <li>Use the combined value as the salt used during hash computation</li>
* </ol>
* </li>
* <li>
- * If the request salt is not null:
- * <p/>
- * This indicates that the hash computation is for comparison purposes (of a
- * previously computed hash). The request salt will be {@link #combine(ByteSource, ByteSource) combined} with any
- * configured {@link #getPrivateSalt() privateSalt} and used as the complete salt during hash computation.
- * </li>
- * </ul>
- * <p/>
- * The returned {@code Hash}'s {@link Hash#getSalt() salt} property
- * will contain <em>only</em> the 'public' part of the salt and <em>NOT</em> the privateSalt. See the class-level
- * JavaDoc explanation for more info.
*
* @param request the request to process
* @return the response containing the result of the hash computation, as well as any hash salt used that should be
* exposed to the caller.
*/
+ @Override
public Hash computeHash(HashRequest request) {
if (request == null || request.getSource() == null || request.getSource().isEmpty()) {
return null;
}
String algorithmName = getAlgorithmName(request);
- ByteSource source = request.getSource();
- int iterations = getIterations(request);
-
- ByteSource publicSalt = getPublicSalt(request);
- ByteSource privateSalt = getPrivateSalt();
- ByteSource salt = combine(privateSalt, publicSalt);
-
- Hash computed = new SimpleHash(algorithmName, source, salt, iterations);
-
- SimpleHash result = new SimpleHash(algorithmName);
- result.setBytes(computed.getBytes());
- result.setIterations(iterations);
- //Only expose the public salt - not the real/combined salt that might have been used:
- result.setSalt(publicSalt);
-
- return result;
- }
-
- protected String getAlgorithmName(HashRequest request) {
- String name = request.getAlgorithmName();
- if (name == null) {
- name = getHashAlgorithmName();
- }
- return name;
- }
-
- protected int getIterations(HashRequest request) {
- int iterations = Math.max(0, request.getIterations());
- if (iterations < 1) {
- iterations = Math.max(1, getHashIterations());
- }
- return iterations;
- }
-
- /**
- * Returns the public salt that should be used to compute a hash based on the specified request or
- * {@code null} if no public salt should be used.
- * <p/>
- * This implementation functions as follows:
- * <ol>
- * <li>If the request salt is not null and non-empty, this will be used, return it.</li>
- * <li>If the request salt is null or empty:
- * <ol>
- * <li>If a private salt has been set <em>OR</em> {@link #isGeneratePublicSalt()} is {@code true},
- * auto generate a random public salt via the configured
- * {@link #getRandomNumberGenerator() randomNumberGenerator}.</li>
- * <li>If a private salt has not been configured and {@link #isGeneratePublicSalt()} is {@code false},
- * do nothing - return {@code null} to indicate a salt should not be used during hash computation.</li>
- * </ol>
- * </li>
- * </ol>
- *
- * @param request request the request to process
- * @return the public salt that should be used to compute a hash based on the specified request or
- * {@code null} if no public salt should be used.
- */
- protected ByteSource getPublicSalt(HashRequest request) {
-
- ByteSource publicSalt = request.getSalt();
-
- if (publicSalt != null && !publicSalt.isEmpty()) {
- //a public salt was explicitly requested to be used - go ahead and use it:
- return publicSalt;
- }
-
- publicSalt = null;
- //check to see if we need to generate one:
- ByteSource privateSalt = getPrivateSalt();
- boolean privateSaltExists = privateSalt != null && !privateSalt.isEmpty();
+ Optional<HashSpi<? extends Hash>> kdfHash = HashProvider.getByAlgorithmName(algorithmName);
+ if (kdfHash.isPresent()) {
+ HashSpi<?> hashSpi = kdfHash.orElseThrow(NoSuchElementException::new);
- //If a private salt exists, we must generate a public salt to protect the integrity of the private salt.
- //Or generate it if the instance is explicitly configured to do so:
- if (privateSaltExists || isGeneratePublicSalt()) {
- publicSalt = getRandomNumberGenerator().nextBytes();
+ return hashSpi.newHashFactory(random).generate(request);
}
- return publicSalt;
+ throw new UnsupportedOperationException("Cannot create a hash with the given algorithm: " + algorithmName);
}
- /**
- * Combines the specified 'private' salt bytes with the specified additional extra bytes to use as the
- * total salt during hash computation. {@code privateSaltBytes} will be {@code null} }if no private salt has been
- * configured.
- *
- * @param privateSalt the (possibly {@code null}) 'private' salt to combine with the specified extra bytes
- * @param publicSalt the extra bytes to use in addition to the given private salt.
- * @return a combination of the specified private salt bytes and extra bytes that will be used as the total
- * salt during hash computation.
- */
- protected ByteSource combine(ByteSource privateSalt, ByteSource publicSalt) {
-
- byte[] privateSaltBytes = privateSalt != null ? privateSalt.getBytes() : null;
- int privateSaltLength = privateSaltBytes != null ? privateSaltBytes.length : 0;
-
- byte[] publicSaltBytes = publicSalt != null ? publicSalt.getBytes() : null;
- int extraBytesLength = publicSaltBytes != null ? publicSaltBytes.length : 0;
-
- int length = privateSaltLength + extraBytesLength;
- if (length <= 0) {
- return null;
- }
-
- byte[] combined = new byte[length];
-
- int i = 0;
- for (int j = 0; j < privateSaltLength; j++) {
- assert privateSaltBytes != null;
- combined[i++] = privateSaltBytes[j];
- }
- for (int j = 0; j < extraBytesLength; j++) {
- assert publicSaltBytes != null;
- combined[i++] = publicSaltBytes[j];
- }
-
- return ByteSource.Util.bytes(combined);
- }
-
- public void setHashAlgorithmName(String name) {
- this.algorithmName = name;
- }
-
- public String getHashAlgorithmName() {
- return this.algorithmName;
- }
-
- public void setPrivateSalt(ByteSource privateSalt) {
- this.privateSalt = privateSalt;
- }
-
- public ByteSource getPrivateSalt() {
- return this.privateSalt;
- }
-
- public void setHashIterations(int count) {
- this.iterations = count;
- }
-
- public int getHashIterations() {
- return this.iterations;
+ protected String getAlgorithmName(HashRequest request) {
+ return request.getAlgorithmName().orElseGet(this::getDefaultAlgorithmName);
}
- public void setRandomNumberGenerator(RandomNumberGenerator rng) {
- this.rng = rng;
+ @Override
+ public void setDefaultAlgorithmName(String name) {
+ this.defaultAlgorithmName = name;
}
- public RandomNumberGenerator getRandomNumberGenerator() {
- return this.rng;
+ public String getDefaultAlgorithmName() {
+ return this.defaultAlgorithmName;
}
- /**
- * Returns {@code true} if a public salt should be randomly generated and used to compute a hash if a
- * {@link HashRequest} does not specify a salt, {@code false} otherwise.
- * <p/>
- * The default value is {@code false} but should definitely be set to {@code true} if the
- * {@code HashService} instance is being used for password hashing.
- * <p/>
- * <b>NOTE:</b> this property only has an effect if a {@link #getPrivateSalt() privateSalt} is NOT configured. If a
- * private salt has been configured and a request does not provide a salt, a random salt will always be generated
- * to protect the integrity of the private salt (without a public salt, the private salt would be exposed as-is,
- * which is undesirable).
- *
- * @return {@code true} if a public salt should be randomly generated and used to compute a hash if a
- * {@link HashRequest} does not specify a salt, {@code false} otherwise.
- */
- public boolean isGeneratePublicSalt() {
- return generatePublicSalt;
+ public Random getRandom() {
+ return random;
}
- /**
- * Sets whether or not a public salt should be randomly generated and used to compute a hash if a
- * {@link HashRequest} does not specify a salt.
- * <p/>
- * The default value is {@code false} but should definitely be set to {@code true} if the
- * {@code HashService} instance is being used for password hashing.
- * <p/>
- * <b>NOTE:</b> this property only has an effect if a {@link #getPrivateSalt() privateSalt} is NOT configured. If a
- * private salt has been configured and a request does not provide a salt, a random salt will always be generated
- * to protect the integrity of the private salt (without a public salt, the private salt would be exposed as-is,
- * which is undesirable).
- *
- * @param generatePublicSalt whether or not a public salt should be randomly generated and used to compute a hash
- * if a {@link HashRequest} does not specify a salt.
- */
- public void setGeneratePublicSalt(boolean generatePublicSalt) {
- this.generatePublicSalt = generatePublicSalt;
+ public void setRandom(Random random) {
+ this.random = random;
Review comment:
Not even for testing. Removing it until we need it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] bdemers commented on a change in pull request #273: [SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)
Posted by GitBox <gi...@apache.org>.
bdemers commented on a change in pull request #273:
URL: https://github.com/apache/shiro/pull/273#discussion_r554017501
##########
File path: crypto/hash/src/main/java/org/apache/shiro/crypto/hash/HashProvider.java
##########
@@ -0,0 +1,60 @@
+/*
+ * 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.shiro.crypto.hash;
+
+import java.util.Optional;
+import java.util.ServiceLoader;
+import java.util.stream.StreamSupport;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Hashes used by the Shiro2CryptFormat class.
+ *
+ * <p>Instead of maintaining them as an {@code Enum}, ServiceLoaders would provide a pluggable alternative.</p>
+ */
+public final class HashProvider {
+
+ private HashProvider() {
+ // utility class
+ }
+
+ /**
+ * Find a KDF implementation by searching the algorithms.
+ *
+ * @param algorithmName the algorithmName to match. This is case-sensitive.
+ * @return an instance of {@link HashProvider} if found, otherwise {@link Optional#empty()}.
+ * @throws NullPointerException if the given parameter algorithmName is {@code null}.
+ */
+ public static Optional<HashSpi<? extends Hash>> getByAlgorithmName(String algorithmName) {
+ requireNonNull(algorithmName, "algorithmName in HashProvider.getByAlgorithmName");
+ ServiceLoader<HashSpi<? extends Hash>> hashSpis = load();
+
+ return StreamSupport.stream(hashSpis.spliterator(), false)
+ .filter(hashSpi -> hashSpi.getImplementedAlgorithms().contains(algorithmName))
+ .findAny();
+ }
+
+ @SuppressWarnings("unchecked")
+ private static ServiceLoader<HashSpi<? extends Hash>> load() {
+ return (ServiceLoader<HashSpi<? extends Hash>>) (Object) ServiceLoader.load(HashSpi.class);
Review comment:
In JJWT, we ran into some issues with class loaders on various environments, IIRC depending on the env: vanilla java, JPMS, spring, OSGi, the actual classloader used could be different, so we ended up doing something like this:
https://github.com/jwtk/jjwt/blob/master/impl/src/main/java/io/jsonwebtoken/impl/lang/Services.java
(basically checking the three common classloaders)
I don't think this is blocking, as it shouldn't change the API of this class, but it's something to think about
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] bmarwell commented on a change in pull request #273: [SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)
Posted by GitBox <gi...@apache.org>.
bmarwell commented on a change in pull request #273:
URL: https://github.com/apache/shiro/pull/273#discussion_r554373598
##########
File path: crypto/hash/src/main/java/org/apache/shiro/crypto/hash/HashProvider.java
##########
@@ -0,0 +1,60 @@
+/*
+ * 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.shiro.crypto.hash;
+
+import java.util.Optional;
+import java.util.ServiceLoader;
+import java.util.stream.StreamSupport;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Hashes used by the Shiro2CryptFormat class.
+ *
+ * <p>Instead of maintaining them as an {@code Enum}, ServiceLoaders would provide a pluggable alternative.</p>
+ */
+public final class HashProvider {
+
+ private HashProvider() {
+ // utility class
+ }
+
+ /**
+ * Find a KDF implementation by searching the algorithms.
+ *
+ * @param algorithmName the algorithmName to match. This is case-sensitive.
+ * @return an instance of {@link HashProvider} if found, otherwise {@link Optional#empty()}.
+ * @throws NullPointerException if the given parameter algorithmName is {@code null}.
+ */
+ public static Optional<HashSpi<? extends Hash>> getByAlgorithmName(String algorithmName) {
+ requireNonNull(algorithmName, "algorithmName in HashProvider.getByAlgorithmName");
+ ServiceLoader<HashSpi<? extends Hash>> hashSpis = load();
+
+ return StreamSupport.stream(hashSpis.spliterator(), false)
+ .filter(hashSpi -> hashSpi.getImplementedAlgorithms().contains(algorithmName))
+ .findAny();
+ }
+
+ @SuppressWarnings("unchecked")
+ private static ServiceLoader<HashSpi<? extends Hash>> load() {
+ return (ServiceLoader<HashSpi<? extends Hash>>) (Object) ServiceLoader.load(HashSpi.class);
Review comment:
What bugs me most is the ugly casting to object and back. But with what I saw from your `jjwt` example, adding a parameter might help with that!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] bdemers commented on a change in pull request #273: [SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)
Posted by GitBox <gi...@apache.org>.
bdemers commented on a change in pull request #273:
URL: https://github.com/apache/shiro/pull/273#discussion_r554009848
##########
File path: core/pom.xml
##########
@@ -101,6 +101,14 @@
<groupId>org.apache.shiro</groupId>
<artifactId>shiro-crypto-hash</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.apache.shiro</groupId>
+ <artifactId>shiro-crypto-support-hashes-argon2</artifactId>
Review comment:
nit: he groupId could be `org.apache.shiro.hashes` and artifactId: `shiro-crypto-argon2`
Or, `org.apache.shiro.crypto:shiro-hashes-argon2`
##########
File path: crypto/hash/src/main/java/org/apache/shiro/crypto/hash/ConfigurableHashService.java
##########
@@ -18,44 +18,20 @@
*/
package org.apache.shiro.crypto.hash;
-import org.apache.shiro.crypto.RandomNumberGenerator;
-import org.apache.shiro.lang.util.ByteSource;
-
/**
* A {@code HashService} that allows configuration of its strategy via JavaBeans-compatible setter methods.
*
* @since 1.2
*/
public interface ConfigurableHashService extends HashService {
- /**
- * Sets the 'private' (internal) salt to be paired with a 'public' (random or supplied) salt during hash computation.
- *
- * @param privateSalt the 'private' internal salt to be paired with a 'public' (random or supplied) salt during
- * hash computation.
- */
- void setPrivateSalt(ByteSource privateSalt);
-
- /**
- * Sets the number of hash iterations that will be performed during hash computation.
- *
- * @param iterations the number of hash iterations that will be performed during hash computation.
- */
- void setHashIterations(int iterations);
-
/**
* Sets the name of the {@link java.security.MessageDigest MessageDigest} algorithm that will be used to compute
* hashes.
*
* @param name the name of the {@link java.security.MessageDigest MessageDigest} algorithm that will be used to
* compute hashes.
*/
- void setHashAlgorithmName(String name);
+ void setDefaultAlgorithmName(String name);
Review comment:
nit: might need to tweak the javadoc to go with this, I'm assuming this is the alg used if the hash cannot be determined?
##########
File path: crypto/hash/src/main/java/org/apache/shiro/crypto/hash/ConfigurableHashService.java
##########
@@ -18,44 +18,20 @@
*/
package org.apache.shiro.crypto.hash;
-import org.apache.shiro.crypto.RandomNumberGenerator;
-import org.apache.shiro.lang.util.ByteSource;
-
/**
* A {@code HashService} that allows configuration of its strategy via JavaBeans-compatible setter methods.
*
* @since 1.2
*/
public interface ConfigurableHashService extends HashService {
- /**
- * Sets the 'private' (internal) salt to be paired with a 'public' (random or supplied) salt during hash computation.
- *
- * @param privateSalt the 'private' internal salt to be paired with a 'public' (random or supplied) salt during
- * hash computation.
- */
- void setPrivateSalt(ByteSource privateSalt);
-
- /**
- * Sets the number of hash iterations that will be performed during hash computation.
- *
- * @param iterations the number of hash iterations that will be performed during hash computation.
- */
- void setHashIterations(int iterations);
-
/**
* Sets the name of the {@link java.security.MessageDigest MessageDigest} algorithm that will be used to compute
* hashes.
*
* @param name the name of the {@link java.security.MessageDigest MessageDigest} algorithm that will be used to
* compute hashes.
*/
- void setHashAlgorithmName(String name);
+ void setDefaultAlgorithmName(String name);
- /**
- * Sets a source of randomness used to generate public salts that will in turn be used during hash computation.
- *
- * @param rng a source of randomness used to generate public salts that will in turn be used during hash computation.
- */
- void setRandomNumberGenerator(RandomNumberGenerator rng);
Review comment:
+1 for removing this from the public API.
##########
File path: crypto/hash/src/main/java/org/apache/shiro/crypto/hash/DefaultHashService.java
##########
@@ -123,222 +71,52 @@ public DefaultHashService() {
* <p/>
* A salt will be generated and used to compute the hash. The salt is generated as follows:
* <ol>
- * <li>Use the {@link #getRandomNumberGenerator() randomNumberGenerator} to generate a new random number.</li>
- * <li>{@link #combine(ByteSource, ByteSource) combine} this random salt with any configured
- * {@link #getPrivateSalt() privateSalt}
- * </li>
* <li>Use the combined value as the salt used during hash computation</li>
* </ol>
* </li>
* <li>
- * If the request salt is not null:
- * <p/>
- * This indicates that the hash computation is for comparison purposes (of a
- * previously computed hash). The request salt will be {@link #combine(ByteSource, ByteSource) combined} with any
- * configured {@link #getPrivateSalt() privateSalt} and used as the complete salt during hash computation.
- * </li>
- * </ul>
- * <p/>
- * The returned {@code Hash}'s {@link Hash#getSalt() salt} property
- * will contain <em>only</em> the 'public' part of the salt and <em>NOT</em> the privateSalt. See the class-level
- * JavaDoc explanation for more info.
*
* @param request the request to process
* @return the response containing the result of the hash computation, as well as any hash salt used that should be
* exposed to the caller.
*/
+ @Override
public Hash computeHash(HashRequest request) {
if (request == null || request.getSource() == null || request.getSource().isEmpty()) {
return null;
}
String algorithmName = getAlgorithmName(request);
- ByteSource source = request.getSource();
- int iterations = getIterations(request);
-
- ByteSource publicSalt = getPublicSalt(request);
- ByteSource privateSalt = getPrivateSalt();
- ByteSource salt = combine(privateSalt, publicSalt);
-
- Hash computed = new SimpleHash(algorithmName, source, salt, iterations);
-
- SimpleHash result = new SimpleHash(algorithmName);
- result.setBytes(computed.getBytes());
- result.setIterations(iterations);
- //Only expose the public salt - not the real/combined salt that might have been used:
- result.setSalt(publicSalt);
-
- return result;
- }
-
- protected String getAlgorithmName(HashRequest request) {
- String name = request.getAlgorithmName();
- if (name == null) {
- name = getHashAlgorithmName();
- }
- return name;
- }
-
- protected int getIterations(HashRequest request) {
- int iterations = Math.max(0, request.getIterations());
- if (iterations < 1) {
- iterations = Math.max(1, getHashIterations());
- }
- return iterations;
- }
-
- /**
- * Returns the public salt that should be used to compute a hash based on the specified request or
- * {@code null} if no public salt should be used.
- * <p/>
- * This implementation functions as follows:
- * <ol>
- * <li>If the request salt is not null and non-empty, this will be used, return it.</li>
- * <li>If the request salt is null or empty:
- * <ol>
- * <li>If a private salt has been set <em>OR</em> {@link #isGeneratePublicSalt()} is {@code true},
- * auto generate a random public salt via the configured
- * {@link #getRandomNumberGenerator() randomNumberGenerator}.</li>
- * <li>If a private salt has not been configured and {@link #isGeneratePublicSalt()} is {@code false},
- * do nothing - return {@code null} to indicate a salt should not be used during hash computation.</li>
- * </ol>
- * </li>
- * </ol>
- *
- * @param request request the request to process
- * @return the public salt that should be used to compute a hash based on the specified request or
- * {@code null} if no public salt should be used.
- */
- protected ByteSource getPublicSalt(HashRequest request) {
-
- ByteSource publicSalt = request.getSalt();
-
- if (publicSalt != null && !publicSalt.isEmpty()) {
- //a public salt was explicitly requested to be used - go ahead and use it:
- return publicSalt;
- }
-
- publicSalt = null;
- //check to see if we need to generate one:
- ByteSource privateSalt = getPrivateSalt();
- boolean privateSaltExists = privateSalt != null && !privateSalt.isEmpty();
+ Optional<HashSpi<? extends Hash>> kdfHash = HashProvider.getByAlgorithmName(algorithmName);
+ if (kdfHash.isPresent()) {
+ HashSpi<?> hashSpi = kdfHash.orElseThrow(NoSuchElementException::new);
- //If a private salt exists, we must generate a public salt to protect the integrity of the private salt.
- //Or generate it if the instance is explicitly configured to do so:
- if (privateSaltExists || isGeneratePublicSalt()) {
- publicSalt = getRandomNumberGenerator().nextBytes();
+ return hashSpi.newHashFactory(random).generate(request);
}
- return publicSalt;
+ throw new UnsupportedOperationException("Cannot create a hash with the given algorithm: " + algorithmName);
}
- /**
- * Combines the specified 'private' salt bytes with the specified additional extra bytes to use as the
- * total salt during hash computation. {@code privateSaltBytes} will be {@code null} }if no private salt has been
- * configured.
- *
- * @param privateSalt the (possibly {@code null}) 'private' salt to combine with the specified extra bytes
- * @param publicSalt the extra bytes to use in addition to the given private salt.
- * @return a combination of the specified private salt bytes and extra bytes that will be used as the total
- * salt during hash computation.
- */
- protected ByteSource combine(ByteSource privateSalt, ByteSource publicSalt) {
-
- byte[] privateSaltBytes = privateSalt != null ? privateSalt.getBytes() : null;
- int privateSaltLength = privateSaltBytes != null ? privateSaltBytes.length : 0;
-
- byte[] publicSaltBytes = publicSalt != null ? publicSalt.getBytes() : null;
- int extraBytesLength = publicSaltBytes != null ? publicSaltBytes.length : 0;
-
- int length = privateSaltLength + extraBytesLength;
- if (length <= 0) {
- return null;
- }
-
- byte[] combined = new byte[length];
-
- int i = 0;
- for (int j = 0; j < privateSaltLength; j++) {
- assert privateSaltBytes != null;
- combined[i++] = privateSaltBytes[j];
- }
- for (int j = 0; j < extraBytesLength; j++) {
- assert publicSaltBytes != null;
- combined[i++] = publicSaltBytes[j];
- }
-
- return ByteSource.Util.bytes(combined);
- }
-
- public void setHashAlgorithmName(String name) {
- this.algorithmName = name;
- }
-
- public String getHashAlgorithmName() {
- return this.algorithmName;
- }
-
- public void setPrivateSalt(ByteSource privateSalt) {
- this.privateSalt = privateSalt;
- }
-
- public ByteSource getPrivateSalt() {
- return this.privateSalt;
- }
-
- public void setHashIterations(int count) {
- this.iterations = count;
- }
-
- public int getHashIterations() {
- return this.iterations;
+ protected String getAlgorithmName(HashRequest request) {
+ return request.getAlgorithmName().orElseGet(this::getDefaultAlgorithmName);
}
- public void setRandomNumberGenerator(RandomNumberGenerator rng) {
- this.rng = rng;
+ @Override
+ public void setDefaultAlgorithmName(String name) {
+ this.defaultAlgorithmName = name;
}
- public RandomNumberGenerator getRandomNumberGenerator() {
- return this.rng;
+ public String getDefaultAlgorithmName() {
+ return this.defaultAlgorithmName;
}
- /**
- * Returns {@code true} if a public salt should be randomly generated and used to compute a hash if a
- * {@link HashRequest} does not specify a salt, {@code false} otherwise.
- * <p/>
- * The default value is {@code false} but should definitely be set to {@code true} if the
- * {@code HashService} instance is being used for password hashing.
- * <p/>
- * <b>NOTE:</b> this property only has an effect if a {@link #getPrivateSalt() privateSalt} is NOT configured. If a
- * private salt has been configured and a request does not provide a salt, a random salt will always be generated
- * to protect the integrity of the private salt (without a public salt, the private salt would be exposed as-is,
- * which is undesirable).
- *
- * @return {@code true} if a public salt should be randomly generated and used to compute a hash if a
- * {@link HashRequest} does not specify a salt, {@code false} otherwise.
- */
- public boolean isGeneratePublicSalt() {
- return generatePublicSalt;
+ public Random getRandom() {
+ return random;
}
- /**
- * Sets whether or not a public salt should be randomly generated and used to compute a hash if a
- * {@link HashRequest} does not specify a salt.
- * <p/>
- * The default value is {@code false} but should definitely be set to {@code true} if the
- * {@code HashService} instance is being used for password hashing.
- * <p/>
- * <b>NOTE:</b> this property only has an effect if a {@link #getPrivateSalt() privateSalt} is NOT configured. If a
- * private salt has been configured and a request does not provide a salt, a random salt will always be generated
- * to protect the integrity of the private salt (without a public salt, the private salt would be exposed as-is,
- * which is undesirable).
- *
- * @param generatePublicSalt whether or not a public salt should be randomly generated and used to compute a hash
- * if a {@link HashRequest} does not specify a salt.
- */
- public void setGeneratePublicSalt(boolean generatePublicSalt) {
- this.generatePublicSalt = generatePublicSalt;
+ public void setRandom(Random random) {
+ this.random = random;
Review comment:
If possible make the random getter/setter package private (I'm guessing it's only used for testing?)
##########
File path: crypto/hash/src/main/java/org/apache/shiro/crypto/hash/HashProvider.java
##########
@@ -0,0 +1,60 @@
+/*
+ * 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.shiro.crypto.hash;
+
+import java.util.Optional;
+import java.util.ServiceLoader;
+import java.util.stream.StreamSupport;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Hashes used by the Shiro2CryptFormat class.
+ *
+ * <p>Instead of maintaining them as an {@code Enum}, ServiceLoaders would provide a pluggable alternative.</p>
+ */
+public final class HashProvider {
+
+ private HashProvider() {
+ // utility class
+ }
+
+ /**
+ * Find a KDF implementation by searching the algorithms.
+ *
+ * @param algorithmName the algorithmName to match. This is case-sensitive.
+ * @return an instance of {@link HashProvider} if found, otherwise {@link Optional#empty()}.
+ * @throws NullPointerException if the given parameter algorithmName is {@code null}.
+ */
+ public static Optional<HashSpi<? extends Hash>> getByAlgorithmName(String algorithmName) {
+ requireNonNull(algorithmName, "algorithmName in HashProvider.getByAlgorithmName");
+ ServiceLoader<HashSpi<? extends Hash>> hashSpis = load();
+
+ return StreamSupport.stream(hashSpis.spliterator(), false)
+ .filter(hashSpi -> hashSpi.getImplementedAlgorithms().contains(algorithmName))
+ .findAny();
+ }
+
+ @SuppressWarnings("unchecked")
+ private static ServiceLoader<HashSpi<? extends Hash>> load() {
+ return (ServiceLoader<HashSpi<? extends Hash>>) (Object) ServiceLoader.load(HashSpi.class);
Review comment:
In JJWT, we ran into some issues with class loaders on various environments, IIRC depending on the env: vanilla java, JPMS, spring, OSGi, the actual classloader used could be different, so we ended up doing something like this:
https://github.com/jwtk/jjwt/blob/master/impl/src/main/java/io/jsonwebtoken/impl/lang/Services.java
(basically checking the three common classloaders)
I don't think this is blocking, as it shouldn't change the API of this class, but it's something to think about
##########
File path: crypto/hash/src/main/java/org/apache/shiro/crypto/hash/Md2Hash.java
##########
@@ -30,10 +30,13 @@
* techniques and how the overloaded constructors function.
*
* @since 0.9
+ * @deprecated and will throw exceptions since 2.0.0, to be removed in 2.1.0.
Review comment:
Rip anything out in 2.0, removing anything is technically a breaking change. (it's easy to add back in later if/when needed)
##########
File path: crypto/hash/src/main/java/org/apache/shiro/crypto/hash/Md2Hash.java
##########
@@ -30,10 +30,13 @@
* techniques and how the overloaded constructors function.
*
* @since 0.9
+ * @deprecated and will throw exceptions since 2.0.0, to be removed in 2.1.0.
Review comment:
we can also mark them deprecated now, and then remove all the deprecated bits before the official 2.0 release if you are on the fence about anything.
Another idea, is we could nag users: print a log message: "WARN: Seriously, stop using MD2, here is why <insert some URL here>" 👺
##########
File path: crypto/support/hashes/bcrypt/src/main/java/org/apache/shiro/crypto/support/hashes/bcrypt/OpenBSDBase64.java
##########
@@ -0,0 +1,194 @@
+/*
+ * 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.shiro.crypto.support.hashes.bcrypt;
+
+
+/**
+ * Encoder for the custom Base64 variant of BCrypt (called Radix64 here). It has the same rules as Base64 but uses a
+ * different mapping table than the various RFCs
+ * <p>
+ * According to Wikipedia:
+ *
+ * <blockquote>
+ * Unix stores password hashes computed with crypt in the /etc/passwd file using radix-64 encoding called B64. It uses a
+ * mostly-alphanumeric set of characters, plus . and /. Its 64-character set is "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz".
+ * Padding is not used.
+ * </blockquote>
+ */
+public interface OpenBSDBase64 {
+
+
+ /**
+ * Encode given raw byte array to a Radix64 style, UTF-8 encoded byte array.
+ *
+ * @param rawBytes to encode
+ * @return UTF-8 encoded string representing radix64 encoded data
+ */
+ byte[] encode(byte[] rawBytes);
+
+ /**
+ * From a UTF-8 encoded string representing radix64 encoded data as byte array, decodes the raw bytes from it.
+ *
+ * @param utf8EncodedRadix64String from a string get it with <code>"m0CrhHm10qJ3lXRY.5zDGO".getBytes(StandardCharsets.UTF8)</code>
+ * @return the raw bytes encoded by this utf-8 radix4 string
+ */
+ byte[] decode(byte[] utf8EncodedRadix64String);
+
+ /*
+ * 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.
+ */
+
+ /**
+ * A mod of Square's Okio Base64 encoder
+ * <p>
+ * Original author: Alexander Y. Kleymenov
Review comment:
If not already make sure this gets added to the NOTICE for this module
##########
File path: crypto/support/hashes/bcrypt/src/main/java/org/apache/shiro/crypto/support/hashes/bcrypt/OpenBSDBase64.java
##########
@@ -0,0 +1,194 @@
+/*
+ * 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.shiro.crypto.support.hashes.bcrypt;
+
+
+/**
+ * Encoder for the custom Base64 variant of BCrypt (called Radix64 here). It has the same rules as Base64 but uses a
+ * different mapping table than the various RFCs
+ * <p>
+ * According to Wikipedia:
+ *
+ * <blockquote>
+ * Unix stores password hashes computed with crypt in the /etc/passwd file using radix-64 encoding called B64. It uses a
+ * mostly-alphanumeric set of characters, plus . and /. Its 64-character set is "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz".
+ * Padding is not used.
+ * </blockquote>
+ */
+public interface OpenBSDBase64 {
+
+
+ /**
+ * Encode given raw byte array to a Radix64 style, UTF-8 encoded byte array.
+ *
+ * @param rawBytes to encode
+ * @return UTF-8 encoded string representing radix64 encoded data
+ */
+ byte[] encode(byte[] rawBytes);
+
+ /**
+ * From a UTF-8 encoded string representing radix64 encoded data as byte array, decodes the raw bytes from it.
+ *
+ * @param utf8EncodedRadix64String from a string get it with <code>"m0CrhHm10qJ3lXRY.5zDGO".getBytes(StandardCharsets.UTF8)</code>
+ * @return the raw bytes encoded by this utf-8 radix4 string
+ */
+ byte[] decode(byte[] utf8EncodedRadix64String);
+
+ /*
+ * 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.
+ */
+
+ /**
+ * A mod of Square's Okio Base64 encoder
+ * <p>
+ * Original author: Alexander Y. Kleymenov
Review comment:
~~If not already make sure this gets added to the NOTICE for this module~~
never minde, i just needed to keep scrolling
##########
File path: crypto/support/hashes/bcrypt/src/main/java/org/apache/shiro/crypto/support/hashes/bcrypt/OpenBSDBase64.java
##########
@@ -0,0 +1,194 @@
+/*
+ * 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.shiro.crypto.support.hashes.bcrypt;
+
+
+/**
+ * Encoder for the custom Base64 variant of BCrypt (called Radix64 here). It has the same rules as Base64 but uses a
+ * different mapping table than the various RFCs
+ * <p>
+ * According to Wikipedia:
+ *
+ * <blockquote>
+ * Unix stores password hashes computed with crypt in the /etc/passwd file using radix-64 encoding called B64. It uses a
+ * mostly-alphanumeric set of characters, plus . and /. Its 64-character set is "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz".
+ * Padding is not used.
+ * </blockquote>
+ */
+public interface OpenBSDBase64 {
+
+
+ /**
+ * Encode given raw byte array to a Radix64 style, UTF-8 encoded byte array.
+ *
+ * @param rawBytes to encode
+ * @return UTF-8 encoded string representing radix64 encoded data
+ */
+ byte[] encode(byte[] rawBytes);
+
+ /**
+ * From a UTF-8 encoded string representing radix64 encoded data as byte array, decodes the raw bytes from it.
+ *
+ * @param utf8EncodedRadix64String from a string get it with <code>"m0CrhHm10qJ3lXRY.5zDGO".getBytes(StandardCharsets.UTF8)</code>
+ * @return the raw bytes encoded by this utf-8 radix4 string
+ */
+ byte[] decode(byte[] utf8EncodedRadix64String);
+
+ /*
+ * 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.
+ */
+
+ /**
+ * A mod of Square's Okio Base64 encoder
+ * <p>
+ * Original author: Alexander Y. Kleymenov
Review comment:
~~If not already make sure this gets added to the NOTICE for this module~~
never mind, i just needed to keep scrolling
##########
File path: crypto/support/pom.xml
##########
@@ -0,0 +1,42 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ 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.
+ -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <parent>
+ <groupId>org.apache.shiro</groupId>
+ <artifactId>shiro-crypto</artifactId>
+ <version>2.0.0-SNAPSHOT</version>
+ <relativePath>../pom.xml</relativePath>
+ </parent>
+
+ <artifactId>support</artifactId>
+ <name>Apache Shiro :: Cryptography :: Support</name>
+ <packaging>pom</packaging>
+
+ <modules>
+ <module>hashes</module>
Review comment:
nit: you can remove the extra empty parent poms by doing something like:
```xml
<modules>
<module>hashes/bcrypt</module>
<module>hashes/argon2</module>
</modules>
```
##########
File path: tools/hasher/pom.xml
##########
@@ -50,7 +50,11 @@
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
- <artifactId>slf4j-simple</artifactId>
+ <artifactId>slf4j-api</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>ch.qos.logback</groupId>
+ <artifactId>logback-classic</artifactId>
Review comment:
nice cleanup 😄
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] bmarwell commented on a change in pull request #273: [SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)
Posted by GitBox <gi...@apache.org>.
bmarwell commented on a change in pull request #273:
URL: https://github.com/apache/shiro/pull/273#discussion_r554342422
##########
File path: crypto/hash/src/main/java/org/apache/shiro/crypto/hash/ConfigurableHashService.java
##########
@@ -18,44 +18,20 @@
*/
package org.apache.shiro.crypto.hash;
-import org.apache.shiro.crypto.RandomNumberGenerator;
-import org.apache.shiro.lang.util.ByteSource;
-
/**
* A {@code HashService} that allows configuration of its strategy via JavaBeans-compatible setter methods.
*
* @since 1.2
*/
public interface ConfigurableHashService extends HashService {
- /**
- * Sets the 'private' (internal) salt to be paired with a 'public' (random or supplied) salt during hash computation.
- *
- * @param privateSalt the 'private' internal salt to be paired with a 'public' (random or supplied) salt during
- * hash computation.
- */
- void setPrivateSalt(ByteSource privateSalt);
-
- /**
- * Sets the number of hash iterations that will be performed during hash computation.
- *
- * @param iterations the number of hash iterations that will be performed during hash computation.
- */
- void setHashIterations(int iterations);
-
/**
* Sets the name of the {@link java.security.MessageDigest MessageDigest} algorithm that will be used to compute
* hashes.
*
* @param name the name of the {@link java.security.MessageDigest MessageDigest} algorithm that will be used to
* compute hashes.
*/
- void setHashAlgorithmName(String name);
+ void setDefaultAlgorithmName(String name);
Review comment:
Yes, the javadocs are the unchecked checkbox ;) Next commit is on the way.
The default is being used when a `HashRequest` does not carry an algorithm name. Its old behaviour/semantics was at least similar, but I was not looking into making it a 100% compatible here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] bmarwell commented on a change in pull request #273: [SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)
Posted by GitBox <gi...@apache.org>.
bmarwell commented on a change in pull request #273:
URL: https://github.com/apache/shiro/pull/273#discussion_r554345881
##########
File path: crypto/hash/src/main/java/org/apache/shiro/crypto/hash/Md2Hash.java
##########
@@ -30,10 +30,13 @@
* techniques and how the overloaded constructors function.
*
* @since 0.9
+ * @deprecated and will throw exceptions since 2.0.0, to be removed in 2.1.0.
Review comment:
+1 for ripping out. We should nag the users in 1.7+ then.
And not just for MD2. For all iteration+salt+pepper based hashes, even if there is no alternative in shiro 1. Seriously.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] bdemers commented on a change in pull request #273: [SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)
Posted by GitBox <gi...@apache.org>.
bdemers commented on a change in pull request #273:
URL: https://github.com/apache/shiro/pull/273#discussion_r554013599
##########
File path: crypto/hash/src/main/java/org/apache/shiro/crypto/hash/ConfigurableHashService.java
##########
@@ -18,44 +18,20 @@
*/
package org.apache.shiro.crypto.hash;
-import org.apache.shiro.crypto.RandomNumberGenerator;
-import org.apache.shiro.lang.util.ByteSource;
-
/**
* A {@code HashService} that allows configuration of its strategy via JavaBeans-compatible setter methods.
*
* @since 1.2
*/
public interface ConfigurableHashService extends HashService {
- /**
- * Sets the 'private' (internal) salt to be paired with a 'public' (random or supplied) salt during hash computation.
- *
- * @param privateSalt the 'private' internal salt to be paired with a 'public' (random or supplied) salt during
- * hash computation.
- */
- void setPrivateSalt(ByteSource privateSalt);
-
- /**
- * Sets the number of hash iterations that will be performed during hash computation.
- *
- * @param iterations the number of hash iterations that will be performed during hash computation.
- */
- void setHashIterations(int iterations);
-
/**
* Sets the name of the {@link java.security.MessageDigest MessageDigest} algorithm that will be used to compute
* hashes.
*
* @param name the name of the {@link java.security.MessageDigest MessageDigest} algorithm that will be used to
* compute hashes.
*/
- void setHashAlgorithmName(String name);
+ void setDefaultAlgorithmName(String name);
- /**
- * Sets a source of randomness used to generate public salts that will in turn be used during hash computation.
- *
- * @param rng a source of randomness used to generate public salts that will in turn be used during hash computation.
- */
- void setRandomNumberGenerator(RandomNumberGenerator rng);
Review comment:
+1 for removing this from the public API.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] bdemers commented on a change in pull request #273: [SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)
Posted by GitBox <gi...@apache.org>.
bdemers commented on a change in pull request #273:
URL: https://github.com/apache/shiro/pull/273#discussion_r554028518
##########
File path: tools/hasher/pom.xml
##########
@@ -50,7 +50,11 @@
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
- <artifactId>slf4j-simple</artifactId>
+ <artifactId>slf4j-api</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>ch.qos.logback</groupId>
+ <artifactId>logback-classic</artifactId>
Review comment:
nice cleanup 😄
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] bdemers commented on a change in pull request #273: [SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)
Posted by GitBox <gi...@apache.org>.
bdemers commented on a change in pull request #273:
URL: https://github.com/apache/shiro/pull/273#discussion_r554026985
##########
File path: crypto/support/pom.xml
##########
@@ -0,0 +1,42 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ 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.
+ -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <parent>
+ <groupId>org.apache.shiro</groupId>
+ <artifactId>shiro-crypto</artifactId>
+ <version>2.0.0-SNAPSHOT</version>
+ <relativePath>../pom.xml</relativePath>
+ </parent>
+
+ <artifactId>support</artifactId>
+ <name>Apache Shiro :: Cryptography :: Support</name>
+ <packaging>pom</packaging>
+
+ <modules>
+ <module>hashes</module>
Review comment:
nit: you can remove the extra empty parent poms by doing something like:
```xml
<modules>
<module>hashes/bcrypt</module>
<module>hashes/argon2</module>
</modules>
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] bdemers commented on a change in pull request #273: [SHIRO-290] Implement BCrypt (and possibly other unix crypt formats like scrypt)
Posted by GitBox <gi...@apache.org>.
bdemers commented on a change in pull request #273:
URL: https://github.com/apache/shiro/pull/273#discussion_r554014787
##########
File path: crypto/hash/src/main/java/org/apache/shiro/crypto/hash/DefaultHashService.java
##########
@@ -123,222 +71,52 @@ public DefaultHashService() {
* <p/>
* A salt will be generated and used to compute the hash. The salt is generated as follows:
* <ol>
- * <li>Use the {@link #getRandomNumberGenerator() randomNumberGenerator} to generate a new random number.</li>
- * <li>{@link #combine(ByteSource, ByteSource) combine} this random salt with any configured
- * {@link #getPrivateSalt() privateSalt}
- * </li>
* <li>Use the combined value as the salt used during hash computation</li>
* </ol>
* </li>
* <li>
- * If the request salt is not null:
- * <p/>
- * This indicates that the hash computation is for comparison purposes (of a
- * previously computed hash). The request salt will be {@link #combine(ByteSource, ByteSource) combined} with any
- * configured {@link #getPrivateSalt() privateSalt} and used as the complete salt during hash computation.
- * </li>
- * </ul>
- * <p/>
- * The returned {@code Hash}'s {@link Hash#getSalt() salt} property
- * will contain <em>only</em> the 'public' part of the salt and <em>NOT</em> the privateSalt. See the class-level
- * JavaDoc explanation for more info.
*
* @param request the request to process
* @return the response containing the result of the hash computation, as well as any hash salt used that should be
* exposed to the caller.
*/
+ @Override
public Hash computeHash(HashRequest request) {
if (request == null || request.getSource() == null || request.getSource().isEmpty()) {
return null;
}
String algorithmName = getAlgorithmName(request);
- ByteSource source = request.getSource();
- int iterations = getIterations(request);
-
- ByteSource publicSalt = getPublicSalt(request);
- ByteSource privateSalt = getPrivateSalt();
- ByteSource salt = combine(privateSalt, publicSalt);
-
- Hash computed = new SimpleHash(algorithmName, source, salt, iterations);
-
- SimpleHash result = new SimpleHash(algorithmName);
- result.setBytes(computed.getBytes());
- result.setIterations(iterations);
- //Only expose the public salt - not the real/combined salt that might have been used:
- result.setSalt(publicSalt);
-
- return result;
- }
-
- protected String getAlgorithmName(HashRequest request) {
- String name = request.getAlgorithmName();
- if (name == null) {
- name = getHashAlgorithmName();
- }
- return name;
- }
-
- protected int getIterations(HashRequest request) {
- int iterations = Math.max(0, request.getIterations());
- if (iterations < 1) {
- iterations = Math.max(1, getHashIterations());
- }
- return iterations;
- }
-
- /**
- * Returns the public salt that should be used to compute a hash based on the specified request or
- * {@code null} if no public salt should be used.
- * <p/>
- * This implementation functions as follows:
- * <ol>
- * <li>If the request salt is not null and non-empty, this will be used, return it.</li>
- * <li>If the request salt is null or empty:
- * <ol>
- * <li>If a private salt has been set <em>OR</em> {@link #isGeneratePublicSalt()} is {@code true},
- * auto generate a random public salt via the configured
- * {@link #getRandomNumberGenerator() randomNumberGenerator}.</li>
- * <li>If a private salt has not been configured and {@link #isGeneratePublicSalt()} is {@code false},
- * do nothing - return {@code null} to indicate a salt should not be used during hash computation.</li>
- * </ol>
- * </li>
- * </ol>
- *
- * @param request request the request to process
- * @return the public salt that should be used to compute a hash based on the specified request or
- * {@code null} if no public salt should be used.
- */
- protected ByteSource getPublicSalt(HashRequest request) {
-
- ByteSource publicSalt = request.getSalt();
-
- if (publicSalt != null && !publicSalt.isEmpty()) {
- //a public salt was explicitly requested to be used - go ahead and use it:
- return publicSalt;
- }
-
- publicSalt = null;
- //check to see if we need to generate one:
- ByteSource privateSalt = getPrivateSalt();
- boolean privateSaltExists = privateSalt != null && !privateSalt.isEmpty();
+ Optional<HashSpi<? extends Hash>> kdfHash = HashProvider.getByAlgorithmName(algorithmName);
+ if (kdfHash.isPresent()) {
+ HashSpi<?> hashSpi = kdfHash.orElseThrow(NoSuchElementException::new);
- //If a private salt exists, we must generate a public salt to protect the integrity of the private salt.
- //Or generate it if the instance is explicitly configured to do so:
- if (privateSaltExists || isGeneratePublicSalt()) {
- publicSalt = getRandomNumberGenerator().nextBytes();
+ return hashSpi.newHashFactory(random).generate(request);
}
- return publicSalt;
+ throw new UnsupportedOperationException("Cannot create a hash with the given algorithm: " + algorithmName);
}
- /**
- * Combines the specified 'private' salt bytes with the specified additional extra bytes to use as the
- * total salt during hash computation. {@code privateSaltBytes} will be {@code null} }if no private salt has been
- * configured.
- *
- * @param privateSalt the (possibly {@code null}) 'private' salt to combine with the specified extra bytes
- * @param publicSalt the extra bytes to use in addition to the given private salt.
- * @return a combination of the specified private salt bytes and extra bytes that will be used as the total
- * salt during hash computation.
- */
- protected ByteSource combine(ByteSource privateSalt, ByteSource publicSalt) {
-
- byte[] privateSaltBytes = privateSalt != null ? privateSalt.getBytes() : null;
- int privateSaltLength = privateSaltBytes != null ? privateSaltBytes.length : 0;
-
- byte[] publicSaltBytes = publicSalt != null ? publicSalt.getBytes() : null;
- int extraBytesLength = publicSaltBytes != null ? publicSaltBytes.length : 0;
-
- int length = privateSaltLength + extraBytesLength;
- if (length <= 0) {
- return null;
- }
-
- byte[] combined = new byte[length];
-
- int i = 0;
- for (int j = 0; j < privateSaltLength; j++) {
- assert privateSaltBytes != null;
- combined[i++] = privateSaltBytes[j];
- }
- for (int j = 0; j < extraBytesLength; j++) {
- assert publicSaltBytes != null;
- combined[i++] = publicSaltBytes[j];
- }
-
- return ByteSource.Util.bytes(combined);
- }
-
- public void setHashAlgorithmName(String name) {
- this.algorithmName = name;
- }
-
- public String getHashAlgorithmName() {
- return this.algorithmName;
- }
-
- public void setPrivateSalt(ByteSource privateSalt) {
- this.privateSalt = privateSalt;
- }
-
- public ByteSource getPrivateSalt() {
- return this.privateSalt;
- }
-
- public void setHashIterations(int count) {
- this.iterations = count;
- }
-
- public int getHashIterations() {
- return this.iterations;
+ protected String getAlgorithmName(HashRequest request) {
+ return request.getAlgorithmName().orElseGet(this::getDefaultAlgorithmName);
}
- public void setRandomNumberGenerator(RandomNumberGenerator rng) {
- this.rng = rng;
+ @Override
+ public void setDefaultAlgorithmName(String name) {
+ this.defaultAlgorithmName = name;
}
- public RandomNumberGenerator getRandomNumberGenerator() {
- return this.rng;
+ public String getDefaultAlgorithmName() {
+ return this.defaultAlgorithmName;
}
- /**
- * Returns {@code true} if a public salt should be randomly generated and used to compute a hash if a
- * {@link HashRequest} does not specify a salt, {@code false} otherwise.
- * <p/>
- * The default value is {@code false} but should definitely be set to {@code true} if the
- * {@code HashService} instance is being used for password hashing.
- * <p/>
- * <b>NOTE:</b> this property only has an effect if a {@link #getPrivateSalt() privateSalt} is NOT configured. If a
- * private salt has been configured and a request does not provide a salt, a random salt will always be generated
- * to protect the integrity of the private salt (without a public salt, the private salt would be exposed as-is,
- * which is undesirable).
- *
- * @return {@code true} if a public salt should be randomly generated and used to compute a hash if a
- * {@link HashRequest} does not specify a salt, {@code false} otherwise.
- */
- public boolean isGeneratePublicSalt() {
- return generatePublicSalt;
+ public Random getRandom() {
+ return random;
}
- /**
- * Sets whether or not a public salt should be randomly generated and used to compute a hash if a
- * {@link HashRequest} does not specify a salt.
- * <p/>
- * The default value is {@code false} but should definitely be set to {@code true} if the
- * {@code HashService} instance is being used for password hashing.
- * <p/>
- * <b>NOTE:</b> this property only has an effect if a {@link #getPrivateSalt() privateSalt} is NOT configured. If a
- * private salt has been configured and a request does not provide a salt, a random salt will always be generated
- * to protect the integrity of the private salt (without a public salt, the private salt would be exposed as-is,
- * which is undesirable).
- *
- * @param generatePublicSalt whether or not a public salt should be randomly generated and used to compute a hash
- * if a {@link HashRequest} does not specify a salt.
- */
- public void setGeneratePublicSalt(boolean generatePublicSalt) {
- this.generatePublicSalt = generatePublicSalt;
+ public void setRandom(Random random) {
+ this.random = random;
Review comment:
If possible make the random getter/setter package private (I'm guessing it's only used for testing?)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org