Thanks for this Maria and Kalle. Just a quick note though - the Hasher stuff I wrote is still in flux - it's not necessarily scoped out to how it should/could be before being 'releasable'. Any thoughts or feedback is appreciated!
Cheers, Les On Fri, Jun 3, 2011 at 7:52 AM, <[email protected]> wrote: > Author: kaosko > Date: Fri Jun 3 14:52:04 2011 > New Revision: 1131059 > > URL: http://svn.apache.org/viewvc?rev=1131059&view=rev > Log: > FIXED - issue SHIRO-302: DefaultHasher does not generate random salt > https://issues.apache.org/jira/browse/SHIRO-302 > - apply Maria Jurcovicova's patch as is, including unit tests > > Added: > shiro/trunk/core/src/test/java/org/apache/shiro/crypto/hash/ > > shiro/trunk/core/src/test/java/org/apache/shiro/crypto/hash/DefaultHasherTest.java > Modified: > > shiro/trunk/core/src/main/java/org/apache/shiro/crypto/hash/DefaultHasher.java > > Modified: > shiro/trunk/core/src/main/java/org/apache/shiro/crypto/hash/DefaultHasher.java > URL: > http://svn.apache.org/viewvc/shiro/trunk/core/src/main/java/org/apache/shiro/crypto/hash/DefaultHasher.java?rev=1131059&r1=1131058&r2=1131059&view=diff > ============================================================================== > --- > shiro/trunk/core/src/main/java/org/apache/shiro/crypto/hash/DefaultHasher.java > (original) > +++ > shiro/trunk/core/src/main/java/org/apache/shiro/crypto/hash/DefaultHasher.java > Fri Jun 3 14:52:04 2011 > @@ -153,7 +153,7 @@ public class DefaultHasher implements Co > publicSaltBytes = null; > } > if (publicSaltBytes == null) { > - getRandomNumberGenerator().nextBytes().getBytes(); > + publicSaltBytes = > getRandomNumberGenerator().nextBytes().getBytes(); > } > > String algorithmName = getHashAlgorithmName(); > > Added: > shiro/trunk/core/src/test/java/org/apache/shiro/crypto/hash/DefaultHasherTest.java > URL: > http://svn.apache.org/viewvc/shiro/trunk/core/src/test/java/org/apache/shiro/crypto/hash/DefaultHasherTest.java?rev=1131059&view=auto > ============================================================================== > --- > shiro/trunk/core/src/test/java/org/apache/shiro/crypto/hash/DefaultHasherTest.java > (added) > +++ > shiro/trunk/core/src/test/java/org/apache/shiro/crypto/hash/DefaultHasherTest.java > Fri Jun 3 14:52:04 2011 > @@ -0,0 +1,154 @@ > +package org.apache.shiro.crypto.hash; > + > +import java.util.Arrays; > + > +import junit.framework.TestCase; > + > +import org.apache.shiro.crypto.SecureRandomNumberGenerator; > +import org.apache.shiro.util.ByteSource; > +import org.junit.Test; > + > +/** > + * Test for {@link DefaultHasher} class. > + * > + */ > +public class DefaultHasherTest { > + > + /** > + * If the same string is hashed twice and no salt was supplied, hashed > + * result should be different in each case. > + */ > + @Test > + public void testOnlyRandomSaltRandomness() { > + Hasher hasher = createHasher(); > + > + HashResponse firstHash = hashString(hasher, "password"); > + HashResponse secondHash = hashString(hasher, "password"); > + > + assertNotEquals(firstHash.getHash().toBase64(), > secondHash.getHash().toBase64()); > + } > + > + /** > + * If a string is hashed and no salt was supplied, random salt is > generated. > + * Hash of the same string with generated random salt should return > the > + * same result. > + */ > + @Test > + public void testOnlyRandomSaltReturn() { > + Hasher hasher = createHasher(); > + > + HashResponse firstHash = hashString(hasher, "password"); > + HashResponse secondHash = hashString(hasher, "password", > firstHash.getSalt().getBytes()); > + > + TestCase.assertEquals(firstHash.getHash().toBase64(), > secondHash.getHash().toBase64()); > + } > + > + /** > + * Two different strings hashed with the same salt should result in > two different > + * hashes. > + */ > + @Test > + public void testOnlyRandomSaltHash() { > + Hasher hasher = createHasher(); > + > + HashResponse firstHash = hashString(hasher, "password"); > + HashResponse secondHash = hashString(hasher, "password2", > firstHash.getSalt().getBytes()); > + > + assertNotEquals(firstHash.getHash().toBase64(), > secondHash.getHash().toBase64()); > + } > + > + /** > + * If the same string is hashed twice and only base salt was > supplied, hashed > + * result should be different in each case. > + */ > + @Test > + public void testBothSaltsRandomness() { > + Hasher hasher = createHasherWithSalt(); > + > + HashResponse firstHash = hashString(hasher, "password"); > + HashResponse secondHash = hashString(hasher, "password"); > + > + assertNotEquals(firstHash.getHash().toBase64(), > secondHash.getHash().toBase64()); > + } > + > + /** > + * If a string is hashed and only base salt was supplied, random salt > is generated. > + * Hash of the same string with generated random salt should return > the > + * same result. > + */ > + @Test > + public void testBothSaltsReturn() { > + Hasher hasher = createHasherWithSalt(); > + > + HashResponse firstHash = hashString(hasher, "password"); > + HashResponse secondHash = hashString(hasher, "password", > firstHash.getSalt().getBytes()); > + > + TestCase.assertEquals(firstHash.getHash().toBase64(), > secondHash.getHash().toBase64()); > + } > + > + /** > + * Two different strings hashed with the same salt should result in > two different > + * hashes. > + */ > + @Test > + public void testBothSaltsHash() { > + Hasher hasher = createHasherWithSalt(); > + > + HashResponse firstHash = hashString(hasher, "password"); > + HashResponse secondHash = hashString(hasher, "password2", > firstHash.getSalt().getBytes()); > + > + assertNotEquals(firstHash.getHash().toBase64(), > secondHash.getHash().toBase64()); > + } > + > + /** > + * Hash result is different if the base salt is added. > + */ > + @Test > + public void testBaseSaltChangesResult() { > + Hasher saltedHasher = createHasherWithSalt(); > + Hasher hasher = createHasher(); > + > + HashResponse firstHash = hashStringPredictable(saltedHasher, > "password"); > + HashResponse secondHash = hashStringPredictable(hasher, > "password"); > + > + assertNotEquals(firstHash.getHash().toBase64(), > secondHash.getHash().toBase64()); > + } > + > + protected HashResponse hashString(Hasher hasher, String string) { > + return hasher.computeHash(new > SimpleHashRequest(ByteSource.Util.bytes(string))); > + } > + > + protected HashResponse hashString(Hasher hasher, String string, > byte[] salt) { > + return hasher.computeHash(new > SimpleHashRequest(ByteSource.Util.bytes(string), > ByteSource.Util.bytes(salt))); > + } > + > + private HashResponse hashStringPredictable(Hasher hasher, String > string) { > + byte[] salt = new byte[20]; > + Arrays.fill(salt, (byte) 2); > + return hasher.computeHash(new > SimpleHashRequest(ByteSource.Util.bytes(string), > ByteSource.Util.bytes(salt))); > + } > + > + private Hasher createHasher() { > + return new DefaultHasher(); > + } > + > + private Hasher createHasherWithSalt() { > + DefaultHasher defaultHasher = new DefaultHasher(); > + defaultHasher.setBaseSalt((new > SecureRandomNumberGenerator()).nextBytes().getBytes()); > + > + return defaultHasher; > + } > + > + private void assertNotEquals(String str1, String str2) { > + boolean equals = equals(str1, str2); > + if (equals) > + TestCase.fail("Strings are supposed to be > different."); > + } > + > + protected boolean equals(String str1, String str2) { > + if (str1 == null) > + return str2 == null; > + > + return str1.equals(str2); > + } > +}
