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:
[email protected]


Reply via email to