From 97a72ecbf7dd6b6b2593342485a8440a56d84cf4 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Mon, 7 Dec 2015 14:59:12 +0100 Subject: [PATCH] make some better use of mocks during unit tests --- main/core/pom.xml | 5 - .../webdav/jackrabbit/CryptorMock.java | 103 ++++++++++++++++++ .../webdav/jackrabbit/RangeRequestTest.java | 4 +- main/crypto-aes/pom.xml | 11 ++ .../crypto/aes256/Aes256Cryptor.java | 13 +-- .../crypto/aes256/CryptoComponent.java | 15 +++ .../crypto/aes256/CryptoModule.java | 29 +++++ .../crypto/aes256/Aes256CryptorTest.java | 14 ++- .../crypto/aes256/CryptoTestComponent.java | 15 +++ .../crypto/aes256/CryptoTestModule.java | 25 +++++ .../crypto/aes256/InsecureRandomMock.java | 23 ++++ main/pom.xml | 4 +- .../org/cryptomator/ui/CryptomatorModule.java | 9 +- .../ui/controllers/WelcomeController.java | 7 +- .../cryptomator/ui/model/VaultFactory.java | 3 +- 15 files changed, 252 insertions(+), 28 deletions(-) create mode 100644 main/core/src/test/java/org/cryptomator/webdav/jackrabbit/CryptorMock.java create mode 100644 main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/CryptoComponent.java create mode 100644 main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/CryptoModule.java create mode 100644 main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/CryptoTestComponent.java create mode 100644 main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/CryptoTestModule.java create mode 100644 main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/InsecureRandomMock.java diff --git a/main/core/pom.xml b/main/core/pom.xml index f68443a432..5d55b77b54 100644 --- a/main/core/pom.xml +++ b/main/core/pom.xml @@ -27,11 +27,6 @@ org.cryptomator crypto-api - - org.cryptomator - crypto-aes - test - diff --git a/main/core/src/test/java/org/cryptomator/webdav/jackrabbit/CryptorMock.java b/main/core/src/test/java/org/cryptomator/webdav/jackrabbit/CryptorMock.java new file mode 100644 index 0000000000..aaa1a35e2a --- /dev/null +++ b/main/core/src/test/java/org/cryptomator/webdav/jackrabbit/CryptorMock.java @@ -0,0 +1,103 @@ +package org.cryptomator.webdav.jackrabbit; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.nio.ByteBuffer; +import java.nio.channels.SeekableByteChannel; + +import org.cryptomator.crypto.Cryptor; +import org.cryptomator.crypto.exceptions.DecryptFailedException; +import org.cryptomator.crypto.exceptions.EncryptFailedException; +import org.cryptomator.crypto.exceptions.MacAuthenticationFailedException; +import org.cryptomator.crypto.exceptions.UnsupportedKeyLengthException; +import org.cryptomator.crypto.exceptions.UnsupportedVaultException; +import org.cryptomator.crypto.exceptions.WrongPasswordException; + +class CryptorMock implements Cryptor { + + private static final int BUFSIZE = 32768; + + @Override + public void randomizeMasterKey() { + // noop + } + + @Override + public void encryptMasterKey(OutputStream out, CharSequence password) throws IOException { + // noop + } + + @Override + public void decryptMasterKey(InputStream in, CharSequence password) throws DecryptFailedException, WrongPasswordException, UnsupportedKeyLengthException, IOException, UnsupportedVaultException { + // noop + } + + @Override + public String encryptDirectoryPath(String cleartextDirectoryId, String nativePathSep) { + return cleartextDirectoryId; + } + + @Override + public String encryptFilename(String cleartextName) { + return cleartextName; + } + + @Override + public String decryptFilename(String ciphertextName) throws DecryptFailedException { + return ciphertextName; + } + + @Override + public Long decryptedContentLength(SeekableByteChannel encryptedFile) throws IOException, MacAuthenticationFailedException { + return encryptedFile.size(); + } + + @Override + public Long decryptFile(SeekableByteChannel encryptedFile, OutputStream plaintextFile, boolean authenticate) throws IOException, DecryptFailedException { + ByteBuffer buf = ByteBuffer.allocate(BUFSIZE); + long numReadTotal = 0; + int numRead = 0; + while ((numRead = encryptedFile.read(buf)) != -1) { + numReadTotal += numRead; + buf.flip(); + byte[] bytes = new byte[numRead]; + buf.get(bytes); + plaintextFile.write(bytes); + buf.rewind(); + } + return numReadTotal; + } + + @Override + public Long decryptRange(SeekableByteChannel encryptedFile, OutputStream plaintextFile, long pos, long length, boolean authenticate) throws IOException, DecryptFailedException { + encryptedFile.position(pos); + + ByteBuffer buf = ByteBuffer.allocate(BUFSIZE); + long numReadTotal = 0; + int numRead = 0; + while ((numRead = encryptedFile.read(buf)) != -1 && numReadTotal < length) { + int len = (int) Math.min(Math.min(numRead, BUFSIZE), length - numReadTotal); // known to fit into integer + numReadTotal += len; + buf.flip(); + byte[] bytes = new byte[len]; + buf.get(bytes); + plaintextFile.write(bytes); + buf.rewind(); + } + return numReadTotal; + } + + @Override + public Long encryptFile(InputStream plaintextFile, SeekableByteChannel encryptedFile) throws IOException, EncryptFailedException { + byte[] buf = new byte[BUFSIZE]; + long numWrittenTotal = 0; + int numRead = 0; + while ((numRead = plaintextFile.read(buf)) != -1) { + numWrittenTotal += numRead; + encryptedFile.write(ByteBuffer.wrap(buf, 0, numRead)); + } + return numWrittenTotal; + } + +} diff --git a/main/core/src/test/java/org/cryptomator/webdav/jackrabbit/RangeRequestTest.java b/main/core/src/test/java/org/cryptomator/webdav/jackrabbit/RangeRequestTest.java index 5c84d910d7..d54fbdb59f 100644 --- a/main/core/src/test/java/org/cryptomator/webdav/jackrabbit/RangeRequestTest.java +++ b/main/core/src/test/java/org/cryptomator/webdav/jackrabbit/RangeRequestTest.java @@ -26,7 +26,7 @@ import org.apache.commons.httpclient.methods.PutMethod; import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; -import org.cryptomator.crypto.aes256.Aes256Cryptor; +import org.cryptomator.crypto.Cryptor; import org.cryptomator.webdav.WebDavServer; import org.cryptomator.webdav.WebDavServer.ServletLifeCycleAdapter; import org.junit.AfterClass; @@ -41,7 +41,7 @@ public class RangeRequestTest { private static final Logger LOG = LoggerFactory.getLogger(RangeRequestTest.class); - private static final Aes256Cryptor CRYPTOR = new Aes256Cryptor(); + private static final Cryptor CRYPTOR = new CryptorMock(); private static final WebDavServer SERVER = new WebDavServer(); private static final File TMP_VAULT = Files.createTempDir(); private static ServletLifeCycleAdapter SERVLET; diff --git a/main/crypto-aes/pom.xml b/main/crypto-aes/pom.xml index 491e9446f0..eec989feb0 100644 --- a/main/crypto-aes/pom.xml +++ b/main/crypto-aes/pom.xml @@ -65,5 +65,16 @@ com.fasterxml.jackson.core jackson-databind + + + + com.google.dagger + dagger + + + com.google.dagger + dagger-compiler + provided + diff --git a/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/Aes256Cryptor.java b/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/Aes256Cryptor.java index e5ddd4dc56..46810bf326 100644 --- a/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/Aes256Cryptor.java +++ b/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/Aes256Cryptor.java @@ -103,14 +103,11 @@ public class Aes256Cryptor implements Cryptor, AesCryptographicConfiguration { /** * Creates a new Cryptor with a newly initialized PRNG. */ - public Aes256Cryptor() { - try { - securePrng = SecureRandom.getInstanceStrong(); - // No setSeed needed. See SecureRandom.getInstance(String): - // The first call to nextBytes will force the SecureRandom object to seed itself - } catch (NoSuchAlgorithmException e) { - throw new IllegalStateException("PRNG algorithm should exist.", e); - } + + Aes256Cryptor(SecureRandom securePrng) { + this.securePrng = securePrng; + // No setSeed needed. See SecureRandom.getInstance(String): + // The first call to nextBytes will force the SecureRandom object to seed itself } @Override diff --git a/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/CryptoComponent.java b/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/CryptoComponent.java new file mode 100644 index 0000000000..a3fccd27a0 --- /dev/null +++ b/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/CryptoComponent.java @@ -0,0 +1,15 @@ +package org.cryptomator.crypto.aes256; + +import javax.inject.Singleton; + +import org.cryptomator.crypto.Cryptor; + +import dagger.Component; + +@Singleton +@Component(modules = CryptoModule.class) +interface CryptoComponent { + + Cryptor cryptor(); + +} \ No newline at end of file diff --git a/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/CryptoModule.java b/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/CryptoModule.java new file mode 100644 index 0000000000..1bed4cf792 --- /dev/null +++ b/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/CryptoModule.java @@ -0,0 +1,29 @@ +package org.cryptomator.crypto.aes256; + +import java.security.NoSuchAlgorithmException; +import java.security.SecureRandom; + +import org.cryptomator.crypto.Cryptor; + +import dagger.Module; +import dagger.Provides; + +@Module +public class CryptoModule { + + @Provides + SecureRandom provideRandomNumberGenerator() { + try { + return SecureRandom.getInstanceStrong(); + } catch (NoSuchAlgorithmException e) { + // quote "Every implementation of the Java platform is required to support at least one strong SecureRandom implementation." + throw new AssertionError("No SecureRandom implementation available."); + } + } + + @Provides + public Cryptor provideCryptor(SecureRandom secureRandom) { + return new Aes256Cryptor(secureRandom); + } + +} diff --git a/main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/Aes256CryptorTest.java b/main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/Aes256CryptorTest.java index adfcaafd00..ef500daec9 100644 --- a/main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/Aes256CryptorTest.java +++ b/main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/Aes256CryptorTest.java @@ -19,6 +19,7 @@ import javax.security.auth.DestroyFailedException; import org.apache.commons.io.IOUtils; +import org.cryptomator.crypto.Cryptor; import org.cryptomator.crypto.exceptions.DecryptFailedException; import org.cryptomator.crypto.exceptions.EncryptFailedException; import org.cryptomator.crypto.exceptions.UnsupportedKeyLengthException; @@ -29,13 +30,18 @@ public class Aes256CryptorTest { - private final Aes256Cryptor cryptor; + private final Cryptor cryptor; public Aes256CryptorTest() { - cryptor = new Aes256Cryptor(); + cryptor = DaggerCryptoTestComponent.create().cryptor(); cryptor.randomizeMasterKey(); } + @Test + public void testMultipleCryptorInstances() { + Assert.assertNotSame(DaggerCryptoTestComponent.create().cryptor(), DaggerCryptoTestComponent.create().cryptor()); + } + @Test(timeout = 10000) public void testCorrectPassword() throws IOException, WrongPasswordException, DecryptFailedException, UnsupportedKeyLengthException, DestroyFailedException, UnsupportedVaultException { final String pw = "asd"; @@ -44,7 +50,7 @@ public void testCorrectPassword() throws IOException, WrongPasswordException, De cryptor.encryptMasterKey(out, pw); cryptor.destroy(); - final Aes256Cryptor decryptor = new Aes256Cryptor(); + final Cryptor decryptor = DaggerCryptoTestComponent.create().cryptor(); final InputStream in = new ByteArrayInputStream(out.toByteArray()); decryptor.decryptMasterKey(in, pw); @@ -63,7 +69,7 @@ public void testWrongPassword() throws IOException, DecryptFailedException, Wron // all these passwords are expected to fail. final String[] wrongPws = {"a", "as", "asdf", "sdf", "das", "dsa", "foo", "bar", "baz"}; - final Aes256Cryptor decryptor = new Aes256Cryptor(); + final Cryptor decryptor = DaggerCryptoTestComponent.create().cryptor(); for (final String wrongPw : wrongPws) { final InputStream in = new ByteArrayInputStream(out.toByteArray()); try { diff --git a/main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/CryptoTestComponent.java b/main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/CryptoTestComponent.java new file mode 100644 index 0000000000..bdde0cd2b9 --- /dev/null +++ b/main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/CryptoTestComponent.java @@ -0,0 +1,15 @@ +package org.cryptomator.crypto.aes256; + +import javax.inject.Singleton; + +import org.cryptomator.crypto.Cryptor; + +import dagger.Component; + +@Singleton +@Component(modules = CryptoTestModule.class) +public interface CryptoTestComponent { + + Cryptor cryptor(); + +} diff --git a/main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/CryptoTestModule.java b/main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/CryptoTestModule.java new file mode 100644 index 0000000000..ddad1670ac --- /dev/null +++ b/main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/CryptoTestModule.java @@ -0,0 +1,25 @@ +package org.cryptomator.crypto.aes256; + +import java.security.SecureRandom; + +import org.cryptomator.crypto.Cryptor; + +import dagger.Module; +import dagger.Provides; + +@Module +public class CryptoTestModule { + + @Provides + @SuppressWarnings("deprecation") + SecureRandom provideRandomNumberGenerator() { + // we use this class for testing only, as unit tests on CI servers tend to stall, if they rely on true randomness. + return new InsecureRandomMock(); + } + + @Provides + Cryptor provideCryptor(SecureRandom secureRandom) { + return new Aes256Cryptor(secureRandom); + } + +} diff --git a/main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/InsecureRandomMock.java b/main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/InsecureRandomMock.java new file mode 100644 index 0000000000..9d54108c53 --- /dev/null +++ b/main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/InsecureRandomMock.java @@ -0,0 +1,23 @@ +package org.cryptomator.crypto.aes256; + +import java.security.SecureRandom; +import java.util.Random; + +/** + * DO NOT USE + * + * This class is for testing only. + */ +@Deprecated // marked as deprecated and made package-private inside /src/test/java to avoid accidential use. +class InsecureRandomMock extends SecureRandom { + + private static final long serialVersionUID = 1505563778398085504L; + private final Random random = new Random(); + + @Override + public void nextBytes(byte[] bytes) { + // let the deterministic RNG do the work: + this.random.nextBytes(bytes); + } + +} diff --git a/main/pom.xml b/main/pom.xml index 9b64b48fdf..f0dddf8859 100644 --- a/main/pom.xml +++ b/main/pom.xml @@ -136,12 +136,12 @@ com.google.dagger dagger - 2.0.1 + 2.0.2 com.google.dagger dagger-compiler - 2.0.1 + 2.0.2 provided diff --git a/main/ui/src/main/java/org/cryptomator/ui/CryptomatorModule.java b/main/ui/src/main/java/org/cryptomator/ui/CryptomatorModule.java index 02c7075d02..34299fd2ff 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/CryptomatorModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/CryptomatorModule.java @@ -9,7 +9,7 @@ import org.cryptomator.crypto.Cryptor; import org.cryptomator.crypto.SamplingCryptorDecorator; -import org.cryptomator.crypto.aes256.Aes256Cryptor; +import org.cryptomator.crypto.aes256.CryptoModule; import org.cryptomator.ui.model.VaultObjectMapperProvider; import org.cryptomator.ui.settings.Settings; import org.cryptomator.ui.settings.SettingsProvider; @@ -26,7 +26,7 @@ import dagger.Provides; import javafx.application.Application; -@Module +@Module(includes = CryptoModule.class) class CryptomatorModule { private final Application application; @@ -90,8 +90,9 @@ WebDavServer provideWebDavServer() { } @Provides - Cryptor provideCryptor() { - return SamplingCryptorDecorator.decorate(new Aes256Cryptor()); + @Named("SamplingCryptor") + Cryptor provideCryptor(Cryptor cryptor) { + return SamplingCryptorDecorator.decorate(cryptor); } private T closeLater(T object, Closer closer) { diff --git a/main/ui/src/main/java/org/cryptomator/ui/controllers/WelcomeController.java b/main/ui/src/main/java/org/cryptomator/ui/controllers/WelcomeController.java index 61b1d5bbae..d64bdd94a1 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/controllers/WelcomeController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/controllers/WelcomeController.java @@ -9,6 +9,7 @@ package org.cryptomator.ui.controllers; import java.io.IOException; +import java.io.InputStream; import java.net.URL; import java.util.Comparator; import java.util.HashMap; @@ -25,6 +26,7 @@ import org.apache.commons.httpclient.HttpStatus; import org.apache.commons.httpclient.cookie.CookiePolicy; import org.apache.commons.httpclient.methods.GetMethod; +import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.SystemUtils; import org.cryptomator.ui.settings.Settings; import org.slf4j.Logger; @@ -140,8 +142,9 @@ private void checkForUpdates() { client.getParams().setConnectionManagerTimeout(5000); try { client.executeMethod(method); - if (method.getStatusCode() == HttpStatus.SC_OK) { - final byte[] responseData = method.getResponseBody(); + final InputStream responseBodyStream = method.getResponseBodyAsStream(); + if (method.getStatusCode() == HttpStatus.SC_OK && responseBodyStream != null) { + final byte[] responseData = IOUtils.toByteArray(responseBodyStream); final ObjectMapper mapper = new ObjectMapper(); final Map map = mapper.readValue(responseData, new TypeReference>() { }); diff --git a/main/ui/src/main/java/org/cryptomator/ui/model/VaultFactory.java b/main/ui/src/main/java/org/cryptomator/ui/model/VaultFactory.java index 5ff52adb4e..17ee6f891f 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/model/VaultFactory.java +++ b/main/ui/src/main/java/org/cryptomator/ui/model/VaultFactory.java @@ -3,6 +3,7 @@ import java.nio.file.Path; import javax.inject.Inject; +import javax.inject.Named; import javax.inject.Provider; import javax.inject.Singleton; @@ -20,7 +21,7 @@ public class VaultFactory { private final DeferredCloser closer; @Inject - public VaultFactory(WebDavServer server, Provider cryptorProvider, WebDavMounter mounter, DeferredCloser closer) { + public VaultFactory(WebDavServer server, @Named("SamplingCryptor") Provider cryptorProvider, WebDavMounter mounter, DeferredCloser closer) { this.server = server; this.cryptorProvider = cryptorProvider; this.mounter = mounter;