diff --git a/main/filesystem-crypto/pom.xml b/main/filesystem-crypto/pom.xml index 77656a929..8bed43dfd 100644 --- a/main/filesystem-crypto/pom.xml +++ b/main/filesystem-crypto/pom.xml @@ -44,6 +44,12 @@ ${bouncycastle.version} + + + com.google.guava + guava + + org.apache.commons diff --git a/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/ByteRange.java b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/ByteRange.java index 642e34e20..362048854 100644 --- a/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/ByteRange.java +++ b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/ByteRange.java @@ -16,7 +16,7 @@ public class ByteRange { this.length = length; } - static ByteRange of(long start, long length) { + public static ByteRange of(long start, long length) { return new ByteRange(start, length); } diff --git a/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/FileContentDecryptor.java b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/FileContentDecryptor.java index dc334cc44..343e0b0ae 100644 --- a/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/FileContentDecryptor.java +++ b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/FileContentDecryptor.java @@ -1,12 +1,13 @@ package org.cryptomator.crypto.engine; import java.nio.ByteBuffer; -import java.util.concurrent.BlockingQueue; + +import javax.security.auth.Destroyable; /** * Stateful, thus not thread-safe. */ -public interface FileContentDecryptor { +public interface FileContentDecryptor extends Destroyable { public static final ByteBuffer EOF = ByteBuffer.allocate(0); @@ -24,14 +25,14 @@ public interface FileContentDecryptor { void append(ByteBuffer ciphertext); /** - * Returns a queue containing cleartext in byte-by-byte FIFO order, meaning in the order ciphertext has been appended to this encryptor. - * However the number and size of the ciphertext byte buffers doesn't need to resemble the ciphertext buffers. + * Returns the next decrypted cleartext in byte-by-byte FIFO order, meaning in the order ciphertext has been appended to this encryptor. + * However the number and size of the cleartext byte buffers doesn't need to resemble the ciphertext buffers. * - * The queue returns {@link #EOF}, when all ciphertext has been processed. + * This method might block if no cleartext is available yet. * - * @return A queue from which decrypted data can be {@link BlockingQueue#take() taken}. + * @return Decrypted cleartext or {@link #EOF}. */ - BlockingQueue cleartext(); + ByteBuffer cleartext() throws InterruptedException; /** * Calculates the ciphertext bytes required to perform a partial decryption of a requested cleartext byte range. @@ -50,4 +51,10 @@ public interface FileContentDecryptor { */ void skipToPosition(long nextCiphertextByte) throws IllegalArgumentException; + /** + * Clears file-specific sensitive information. + */ + @Override + void destroy(); + } diff --git a/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/FileContentEncryptor.java b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/FileContentEncryptor.java index 76928c4b6..3ca09c4a8 100644 --- a/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/FileContentEncryptor.java +++ b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/FileContentEncryptor.java @@ -1,12 +1,13 @@ package org.cryptomator.crypto.engine; import java.nio.ByteBuffer; -import java.util.concurrent.BlockingQueue; + +import javax.security.auth.Destroyable; /** * Stateful, thus not thread-safe. */ -public interface FileContentEncryptor { +public interface FileContentEncryptor extends Destroyable { public static final ByteBuffer EOF = ByteBuffer.allocate(0); @@ -26,14 +27,14 @@ public interface FileContentEncryptor { void append(ByteBuffer cleartext); /** - * Returns a queue containing ciphertext in byte-by-byte FIFO order, meaning in the order cleartext has been appended to this encryptor. + * Returns the next ciphertext in byte-by-byte FIFO order, meaning in the order cleartext has been appended to this encryptor. * However the number and size of the ciphertext byte buffers doesn't need to resemble the cleartext buffers. * - * The queue returns {@link #EOF}, when all cleartext has been processed. + * This method might block if no ciphertext is available yet. * - * @return A queue from which encrypted data can be {@link BlockingQueue#take() taken}. + * @return Encrypted ciphertext of {@link #EOF}. */ - BlockingQueue ciphertext(); + ByteBuffer ciphertext() throws InterruptedException; /** * Calculates the cleartext bytes required to perform a partial encryption of a specific cleartext byte range. @@ -52,4 +53,10 @@ public interface FileContentEncryptor { */ void skipToPosition(long nextCleartextByte) throws IllegalArgumentException; + /** + * Clears file-specific sensitive information. + */ + @Override + void destroy(); + } diff --git a/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/BytesWithSequenceNumber.java b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/BytesWithSequenceNumber.java new file mode 100644 index 000000000..b835c475f --- /dev/null +++ b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/BytesWithSequenceNumber.java @@ -0,0 +1,31 @@ +package org.cryptomator.crypto.engine.impl; + +import java.nio.ByteBuffer; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; + +class BytesWithSequenceNumber implements Comparable { + + private final Future byteBuffer; + private final long sequenceNumber; + + public BytesWithSequenceNumber(Future byteBuffer, long sequenceNumber) { + this.byteBuffer = byteBuffer; + this.sequenceNumber = sequenceNumber; + } + + public ByteBuffer get() throws InterruptedException { + try { + return byteBuffer.get(); + } catch (ExecutionException e) { + assert e.getCause() instanceof RuntimeException; + throw (RuntimeException) e.getCause(); + } + } + + @Override + public int compareTo(BytesWithSequenceNumber other) { + return Long.compare(this.sequenceNumber, other.sequenceNumber); + } + +} \ No newline at end of file diff --git a/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/CryptorImpl.java b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/CryptorImpl.java index e5cd54f20..0b4c7bdba 100644 --- a/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/CryptorImpl.java +++ b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/CryptorImpl.java @@ -14,6 +14,7 @@ import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; import java.util.Arrays; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Supplier; import javax.crypto.SecretKey; import javax.crypto.spec.SecretKeySpec; @@ -42,44 +43,69 @@ public class CryptorImpl implements Cryptor { private final AtomicReference fileContentCryptor = new AtomicReference<>(); private final SecureRandom randomSource; - public CryptorImpl(SecureRandom randomSource) { + /** + * Designated constructor. + * + * Package-visible for testing only, use secondary constructors otherwise to ensure a proper PRNG. + */ + CryptorImpl(SecureRandom randomSource) { this.randomSource = randomSource; } - @Override - public FilenameCryptor getFilenameCryptor() { - // lazy initialization pattern as proposed here http://stackoverflow.com/a/30247202/4014509 - final FilenameCryptor existingCryptor = filenameCryptor.get(); - if (existingCryptor != null) { - return existingCryptor; - } else { - final FilenameCryptor newCryptor = new FilenameCryptorImpl(encryptionKey, macKey); - if (filenameCryptor.compareAndSet(null, newCryptor)) { - return newCryptor; - } else { - // CAS failed: other thread set an object - return filenameCryptor.get(); - } + public CryptorImpl() { + this(getStrongSecureRandom()); + } + + private static SecureRandom getStrongSecureRandom() { + try { + return SecureRandom.getInstanceStrong(); + } catch (NoSuchAlgorithmException e) { + throw new IllegalStateException("No strong PRNGs available.", e); } } + @Override + public FilenameCryptor getFilenameCryptor() { + assertKeysExist(); + return initializeLazily(filenameCryptor, () -> { + return new FilenameCryptorImpl(encryptionKey, macKey); + }); + } + @Override public FileContentCryptor getFileContentCryptor() { - // lazy initialization pattern as proposed here http://stackoverflow.com/a/30247202/4014509 - final FileContentCryptor existingCryptor = fileContentCryptor.get(); - if (existingCryptor != null) { - return existingCryptor; + assertKeysExist(); + return initializeLazily(fileContentCryptor, () -> { + return new FileContentCryptorImpl(encryptionKey, macKey, randomSource); + }); + } + + /** + * threadsafe lazy initialization pattern as proposed on http://stackoverflow.com/a/30247202/4014509 + */ + private T initializeLazily(AtomicReference reference, Supplier factory) { + final T existingInstance = reference.get(); + if (existingInstance != null) { + return existingInstance; } else { - final FileContentCryptor newCryptor = new FileContentCryptorImpl(encryptionKey, macKey); - if (fileContentCryptor.compareAndSet(null, newCryptor)) { - return newCryptor; + final T newInstance = factory.get(); + if (reference.compareAndSet(null, newInstance)) { + return newInstance; } else { - // CAS failed: other thread set an object - return fileContentCryptor.get(); + return reference.get(); } } } + private void assertKeysExist() { + if (encryptionKey == null || encryptionKey.isDestroyed()) { + throw new IllegalStateException("No or invalid encryptionKey."); + } + if (macKey == null || macKey.isDestroyed()) { + throw new IllegalStateException("No or invalid MAC key."); + } + } + @Override public void randomizeMasterkey() { final byte[] randomBytes = new byte[KEYLENGTH_IN_BYTES]; diff --git a/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/FileContentCryptorImpl.java b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/FileContentCryptorImpl.java index 3448c3427..2aa46c3ed 100644 --- a/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/FileContentCryptorImpl.java +++ b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/FileContentCryptorImpl.java @@ -1,6 +1,7 @@ package org.cryptomator.crypto.engine.impl; import java.nio.ByteBuffer; +import java.security.SecureRandom; import java.util.Optional; import javax.crypto.SecretKey; @@ -11,20 +12,22 @@ import org.cryptomator.crypto.engine.FileContentEncryptor; class FileContentCryptorImpl implements FileContentCryptor { + // 16 header IV, 8 content nonce, 48 sensitive header data, 32 headerMac + static final int HEADER_SIZE = 104; + private final SecretKey encryptionKey; private final SecretKey macKey; + private final SecureRandom randomSource; - public FileContentCryptorImpl(SecretKey encryptionKey, SecretKey macKey) { - if (encryptionKey == null || macKey == null) { - throw new IllegalArgumentException("Key must not be null"); - } + public FileContentCryptorImpl(SecretKey encryptionKey, SecretKey macKey, SecureRandom randomSource) { this.encryptionKey = encryptionKey; this.macKey = macKey; + this.randomSource = randomSource; } @Override public int getHeaderSize() { - throw new UnsupportedOperationException("Method not implemented"); + return HEADER_SIZE; } @Override @@ -34,7 +37,7 @@ class FileContentCryptorImpl implements FileContentCryptor { @Override public FileContentEncryptor createFileContentEncryptor(Optional header) { - throw new UnsupportedOperationException("Method not implemented"); + return new FileContentEncryptorImpl(encryptionKey, macKey, randomSource); } } diff --git a/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/FileContentEncryptorImpl.java b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/FileContentEncryptorImpl.java new file mode 100644 index 000000000..fe55fa0df --- /dev/null +++ b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/FileContentEncryptorImpl.java @@ -0,0 +1,206 @@ +package org.cryptomator.crypto.engine.impl; + +import java.nio.ByteBuffer; +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +import java.security.NoSuchAlgorithmException; +import java.security.SecureRandom; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.PriorityBlockingQueue; + +import javax.crypto.BadPaddingException; +import javax.crypto.Cipher; +import javax.crypto.IllegalBlockSizeException; +import javax.crypto.Mac; +import javax.crypto.NoSuchPaddingException; +import javax.crypto.SecretKey; +import javax.crypto.ShortBufferException; +import javax.crypto.spec.IvParameterSpec; +import javax.crypto.spec.SecretKeySpec; +import javax.security.auth.DestroyFailedException; + +import org.cryptomator.crypto.engine.ByteRange; +import org.cryptomator.crypto.engine.FileContentEncryptor; +import org.cryptomator.io.ByteBuffers; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.util.concurrent.Futures; + +public class FileContentEncryptorImpl implements FileContentEncryptor { + + private static final Logger LOG = LoggerFactory.getLogger(FileContentEncryptorImpl.class); + private static final String AES = "AES"; + private static final int AES_BLOCK_LENGTH_IN_BYTES = 16; + private static final String AES_CBC = "AES/CBC/PKCS5Padding"; + private static final String HMAC_SHA256 = "HmacSHA256"; + private static final int CHUNK_SIZE = 32 * 1024; + private static final int NUM_WORKERS = Runtime.getRuntime().availableProcessors(); + + private final BlockingQueue ciphertextQueue = new PriorityBlockingQueue<>(); + private final ExecutorService executorService = Executors.newFixedThreadPool(NUM_WORKERS); + private final ThreadLocal hmacSha256; + private final long cleartextBytesEncrypted = 0; + private final SecretKey headerKey; + private final SecretKey contentKey; + private final byte[] iv; + private final byte[] nonce; + private ByteBuffer cleartextBuffer = ByteBuffer.allocate(CHUNK_SIZE); + private long chunkNumber = 0; + + public FileContentEncryptorImpl(SecretKey headerKey, SecretKey macKey, SecureRandom randomSource) { + this.hmacSha256 = new ThreadLocalMac(macKey, HMAC_SHA256); + this.headerKey = headerKey; + this.iv = new byte[16]; + this.nonce = new byte[8]; + final byte[] contentKeyBytes = new byte[32]; + randomSource.nextBytes(iv); + randomSource.nextBytes(nonce); + randomSource.nextBytes(contentKeyBytes); + this.contentKey = new SecretKeySpec(contentKeyBytes, AES); + } + + private ByteBuffer getCleartextSensitiveHeaderData() { + ByteBuffer header = ByteBuffer.allocate(104); + header.putLong(cleartextBytesEncrypted); + header.put(contentKey.getEncoded()); + header.flip(); + return header; + } + + private ByteBuffer getCiphertextSensitiveHeaderData() { + final ByteBuffer cleartext = getCleartextSensitiveHeaderData(); + try { + final Cipher cipher = Cipher.getInstance(AES_CBC); + cipher.init(Cipher.ENCRYPT_MODE, headerKey, new IvParameterSpec(iv)); + final int ciphertextLength = cipher.getOutputSize(cleartext.remaining()); + assert ciphertextLength == 48 : "8 byte long and 32 byte file key should fit into 3 blocks"; + final ByteBuffer ciphertext = ByteBuffer.allocate(ciphertextLength); + cipher.doFinal(cleartext, ciphertext); + return ciphertext; + } catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException | InvalidAlgorithmParameterException | ShortBufferException | IllegalBlockSizeException | BadPaddingException e) { + throw new IllegalStateException("Unable to compute encrypted header.", e); + } + } + + private byte[] mac(ByteBuffer what) { + Mac mac = hmacSha256.get(); + mac.update(what); + return mac.doFinal(); + } + + @Override + public ByteBuffer getHeader() { + final ByteBuffer header = ByteBuffer.allocate(FileContentCryptorImpl.HEADER_SIZE); + header.put(iv); + header.put(nonce); + final ByteBuffer sensitiveHeaderData = getCiphertextSensitiveHeaderData(); + header.put(sensitiveHeaderData); + final ByteBuffer headerSoFar = header.asReadOnlyBuffer(); + headerSoFar.flip(); + header.put(mac(headerSoFar)); + header.flip(); + return header; + } + + @Override + public void append(ByteBuffer cleartext) { + if (cleartext == FileContentEncryptor.EOF) { + submitCleartextBuffer(); + submitEof(); + } else { + while (cleartext.hasRemaining()) { + ByteBuffers.copy(cleartext, cleartextBuffer); + submitCleartextBufferIfFull(); + } + } + } + + private void submitCleartextBufferIfFull() { + if (!cleartextBuffer.hasRemaining()) { + submitCleartextBuffer(); + cleartextBuffer = ByteBuffer.allocate(CHUNK_SIZE); + } + } + + private void submitCleartextBuffer() { + cleartextBuffer.flip(); + long myChunkNumber = chunkNumber++; + Future result = executorService.submit(new EncryptionJob(cleartextBuffer, myChunkNumber)); + ciphertextQueue.offer(new BytesWithSequenceNumber(result, myChunkNumber)); + } + + private void submitEof() { + Future resolvedFuture = Futures.immediateFuture(FileContentEncryptor.EOF); + ciphertextQueue.offer(new BytesWithSequenceNumber(resolvedFuture, Long.MAX_VALUE)); + } + + @Override + public ByteBuffer ciphertext() throws InterruptedException { + return ciphertextQueue.take().get(); + } + + @Override + public ByteRange cleartextRequiredToEncryptRange(ByteRange cleartextRange) { + return ByteRange.of(0, Long.MAX_VALUE); + } + + @Override + public void skipToPosition(long nextCleartextByte) throws IllegalArgumentException { + throw new UnsupportedOperationException("partial encryption not supported."); + } + + @Override + public void destroy() { + try { + contentKey.destroy(); + } catch (DestroyFailedException e) { + LOG.warn("Could not destroy file-specific key", e); + } + } + + private class EncryptionJob implements Callable { + + private final ByteBuffer cleartextBlock; + private final byte[] nonceAndCtr; + + public EncryptionJob(ByteBuffer cleartext, long chunkNumber) { + this.cleartextBlock = cleartext; + + final ByteBuffer nonceAndCounterBuf = ByteBuffer.allocate(AES_BLOCK_LENGTH_IN_BYTES); + nonceAndCounterBuf.put(nonce); + nonceAndCounterBuf.putLong(chunkNumber * CHUNK_SIZE / AES_BLOCK_LENGTH_IN_BYTES); + this.nonceAndCtr = nonceAndCounterBuf.array(); + } + + @Override + public ByteBuffer call() { + try { + Cipher cipher = ThreadLocalAesCtrCipher.get(); + cipher.init(Cipher.ENCRYPT_MODE, contentKey, new IvParameterSpec(nonceAndCtr)); + Mac mac = hmacSha256.get(); + ByteBuffer ciphertextBlock = ByteBuffer.allocate(cipher.getOutputSize(cleartextBlock.remaining()) + mac.getMacLength()); + cipher.update(cleartextBlock, ciphertextBlock); + ByteBuffer ciphertextSoFar = ciphertextBlock.asReadOnlyBuffer(); + ciphertextSoFar.flip(); + mac.update(ciphertextSoFar); + byte[] authenticationCode = mac.doFinal(); + ciphertextBlock.put(authenticationCode); + ciphertextBlock.flip(); + return ciphertextBlock; + } catch (InvalidKeyException e) { + throw new IllegalStateException("File content key created by current class invalid.", e); + } catch (ShortBufferException e) { + throw new IllegalStateException("Buffer allocated for reported output size apparently not big enought.", e); + } catch (InvalidAlgorithmParameterException e) { + throw new IllegalStateException("CTR mode known to accept an IV (aka. nonce).", e); + } + } + + } + +} diff --git a/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/FilenameCryptorImpl.java b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/FilenameCryptorImpl.java index cd6695b3d..fdf573413 100644 --- a/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/FilenameCryptorImpl.java +++ b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/FilenameCryptorImpl.java @@ -32,9 +32,6 @@ class FilenameCryptorImpl implements FilenameCryptor { private final SecretKey macKey; FilenameCryptorImpl(SecretKey encryptionKey, SecretKey macKey) { - if (encryptionKey == null || macKey == null) { - throw new IllegalArgumentException("Key must not be null"); - } this.encryptionKey = encryptionKey; this.macKey = macKey; } diff --git a/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/ThreadLocalAesCtrCipher.java b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/ThreadLocalAesCtrCipher.java new file mode 100644 index 000000000..7c805955f --- /dev/null +++ b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/ThreadLocalAesCtrCipher.java @@ -0,0 +1,28 @@ +package org.cryptomator.crypto.engine.impl; + +import java.security.NoSuchAlgorithmException; + +import javax.crypto.Cipher; +import javax.crypto.NoSuchPaddingException; + +final class ThreadLocalAesCtrCipher { + + private ThreadLocalAesCtrCipher() { + } + + private static final String AES_CTR = "AES/CTR/NoPadding"; + private static final ThreadLocal THREAD_LOCAL_CIPHER = ThreadLocal.withInitial(ThreadLocalAesCtrCipher::newCipherInstance); + + private static Cipher newCipherInstance() { + try { + return Cipher.getInstance(AES_CTR); + } catch (NoSuchAlgorithmException | NoSuchPaddingException e) { + throw new IllegalStateException("Could not create MAC.", e); + } + } + + public static Cipher get() { + return THREAD_LOCAL_CIPHER.get(); + } + +} \ No newline at end of file diff --git a/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/ThreadLocalMac.java b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/ThreadLocalMac.java new file mode 100644 index 000000000..7402146b7 --- /dev/null +++ b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/ThreadLocalMac.java @@ -0,0 +1,37 @@ +package org.cryptomator.crypto.engine.impl; + +import java.security.InvalidKeyException; +import java.security.NoSuchAlgorithmException; + +import javax.crypto.Mac; +import javax.crypto.SecretKey; + +class ThreadLocalMac extends ThreadLocal { + + private final SecretKey macKey; + private final String macAlgorithm; + + ThreadLocalMac(SecretKey macKey, String macAlgorithm) { + this.macKey = macKey; + this.macAlgorithm = macAlgorithm; + } + + @Override + protected Mac initialValue() { + try { + Mac mac = Mac.getInstance(macAlgorithm); + mac.init(macKey); + return mac; + } catch (NoSuchAlgorithmException | InvalidKeyException e) { + throw new IllegalStateException("Could not create MAC.", e); + } + } + + @Override + public Mac get() { + Mac mac = super.get(); + mac.reset(); + return mac; + } + +} \ No newline at end of file diff --git a/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/fs/CryptoReadableFile.java b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/fs/CryptoReadableFile.java index 45e2b1d9e..a84c0b227 100644 --- a/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/fs/CryptoReadableFile.java +++ b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/fs/CryptoReadableFile.java @@ -37,7 +37,6 @@ class CryptoReadableFile implements ReadableFile { if (readAheadTask != null) { readAheadTask.cancel(true); bufferedCleartext = EMPTY_BUFFER; - decryptor.cleartext().clear(); } readAheadTask = executorService.submit(new Reader(pos)); } @@ -56,7 +55,7 @@ class CryptoReadableFile implements ReadableFile { private void bufferCleartext() throws InterruptedException { if (!bufferedCleartext.hasRemaining()) { - bufferedCleartext = decryptor.cleartext().take(); + bufferedCleartext = decryptor.cleartext(); } } diff --git a/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/fs/CryptoWritableFile.java b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/fs/CryptoWritableFile.java index 591d29f1d..d4c806d47 100644 --- a/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/fs/CryptoWritableFile.java +++ b/main/filesystem-crypto/src/main/java/org/cryptomator/crypto/fs/CryptoWritableFile.java @@ -95,7 +95,7 @@ class CryptoWritableFile implements WritableFile { public Void call() { try { ByteBuffer ciphertext; - while ((ciphertext = encryptor.ciphertext().take()) != FileContentEncryptor.EOF) { + while ((ciphertext = encryptor.ciphertext()) != FileContentEncryptor.EOF) { file.write(ciphertext); } } catch (InterruptedException e) { diff --git a/main/filesystem-crypto/src/test/java/org/cryptomator/crypto/engine/NoFileContentCryptor.java b/main/filesystem-crypto/src/test/java/org/cryptomator/crypto/engine/NoFileContentCryptor.java index 2e28ec236..149b82333 100644 --- a/main/filesystem-crypto/src/test/java/org/cryptomator/crypto/engine/NoFileContentCryptor.java +++ b/main/filesystem-crypto/src/test/java/org/cryptomator/crypto/engine/NoFileContentCryptor.java @@ -54,8 +54,8 @@ class NoFileContentCryptor implements FileContentCryptor { } @Override - public BlockingQueue cleartext() { - return cleartextQueue; + public ByteBuffer cleartext() throws InterruptedException { + return cleartextQueue.take(); } @Override @@ -68,6 +68,11 @@ class NoFileContentCryptor implements FileContentCryptor { // no-op } + @Override + public void destroy() { + // no-op + } + } private class Encryptor implements FileContentEncryptor { @@ -98,8 +103,8 @@ class NoFileContentCryptor implements FileContentCryptor { } @Override - public BlockingQueue ciphertext() { - return ciphertextQueue; + public ByteBuffer ciphertext() throws InterruptedException { + return ciphertextQueue.take(); } @Override @@ -112,6 +117,11 @@ class NoFileContentCryptor implements FileContentCryptor { // no-op } + @Override + public void destroy() { + // no-op + } + } } diff --git a/main/filesystem-crypto/src/test/java/org/cryptomator/crypto/engine/impl/CryptorImplTest.java b/main/filesystem-crypto/src/test/java/org/cryptomator/crypto/engine/impl/CryptorImplTest.java index f4127a45f..d7370fcdb 100644 --- a/main/filesystem-crypto/src/test/java/org/cryptomator/crypto/engine/impl/CryptorImplTest.java +++ b/main/filesystem-crypto/src/test/java/org/cryptomator/crypto/engine/impl/CryptorImplTest.java @@ -9,34 +9,19 @@ package org.cryptomator.crypto.engine.impl; import java.io.IOException; -import java.security.SecureRandom; -import java.util.Arrays; -import java.util.concurrent.atomic.AtomicReference; import org.cryptomator.crypto.engine.Cryptor; -import org.cryptomator.crypto.engine.FilenameCryptor; import org.junit.Assert; import org.junit.Test; public class CryptorImplTest { - private static final SecureRandom RANDOM_MOCK = new SecureRandom() { - - private static final long serialVersionUID = 1505563778398085504L; - - @Override - public void nextBytes(byte[] bytes) { - Arrays.fill(bytes, (byte) 0x00); - } - - }; - @Test public void testMasterkeyDecryption() throws IOException { final String testMasterKey = "{\"version\":3,\"scryptSalt\":\"AAAAAAAAAAA=\",\"scryptCostParam\":2,\"scryptBlockSize\":8," // + "\"primaryMasterKey\":\"mM+qoQ+o0qvPTiDAZYt+flaC3WbpNAx1sTXaUzxwpy0M9Ctj6Tih/Q==\"," // + "\"hmacMasterKey\":\"mM+qoQ+o0qvPTiDAZYt+flaC3WbpNAx1sTXaUzxwpy0M9Ctj6Tih/Q==\"}"; - final Cryptor cryptor = new CryptorImpl(RANDOM_MOCK); + final Cryptor cryptor = TestCryptorImplFactory.insecureCryptorImpl(); Assert.assertFalse(cryptor.readKeysFromMasterkeyFile(testMasterKey.getBytes(), "qwe")); Assert.assertTrue(cryptor.readKeysFromMasterkeyFile(testMasterKey.getBytes(), "asd")); } @@ -46,52 +31,25 @@ public class CryptorImplTest { final String expectedMasterKey = "{\"version\":3,\"scryptSalt\":\"AAAAAAAAAAA=\",\"scryptCostParam\":16384,\"scryptBlockSize\":8," // + "\"primaryMasterKey\":\"BJPIq5pvhN24iDtPJLMFPLaVJWdGog9k4n0P03j4ru+ivbWY9OaRGQ==\"," // + "\"hmacMasterKey\":\"BJPIq5pvhN24iDtPJLMFPLaVJWdGog9k4n0P03j4ru+ivbWY9OaRGQ==\"}"; - final Cryptor cryptor = new CryptorImpl(RANDOM_MOCK); + final Cryptor cryptor = TestCryptorImplFactory.insecureCryptorImpl(); cryptor.randomizeMasterkey(); final byte[] masterkeyFile = cryptor.writeKeysToMasterkeyFile("asd"); Assert.assertArrayEquals(expectedMasterKey.getBytes(), masterkeyFile); } @Test - public void testGetFilenameCryptorAfterUnlocking() { - final String testMasterKey = "{\"version\":3,\"scryptSalt\":\"AAAAAAAAAAA=\",\"scryptCostParam\":2,\"scryptBlockSize\":8," // - + "\"primaryMasterKey\":\"mM+qoQ+o0qvPTiDAZYt+flaC3WbpNAx1sTXaUzxwpy0M9Ctj6Tih/Q==\"," // - + "\"hmacMasterKey\":\"mM+qoQ+o0qvPTiDAZYt+flaC3WbpNAx1sTXaUzxwpy0M9Ctj6Tih/Q==\"}"; - final Cryptor cryptor = new CryptorImpl(RANDOM_MOCK); - cryptor.readKeysFromMasterkeyFile(testMasterKey.getBytes(), "asd"); - Assert.assertNotNull(cryptor.getFilenameCryptor()); + public void testGetFilenameAndFileContentCryptor() throws InterruptedException { + final Cryptor cryptor = TestCryptorImplFactory.insecureCryptorImpl(); + cryptor.randomizeMasterkey(); + + Assert.assertSame(cryptor.getFilenameCryptor(), cryptor.getFilenameCryptor()); + Assert.assertSame(cryptor.getFileContentCryptor(), cryptor.getFileContentCryptor()); } - @Test(expected = RuntimeException.class) - public void testGetFilenameCryptorBeforeUnlocking() { - final Cryptor cryptor = new CryptorImpl(RANDOM_MOCK); + @Test(expected = IllegalStateException.class) + public void testGetFilenameAndFileContentCryptorWithoutKeys() throws InterruptedException { + final Cryptor cryptor = TestCryptorImplFactory.insecureCryptorImpl(); cryptor.getFilenameCryptor(); } - @Test - public void testConcurrentGetFilenameCryptor() throws InterruptedException { - final String testMasterKey = "{\"version\":3,\"scryptSalt\":\"AAAAAAAAAAA=\",\"scryptCostParam\":2,\"scryptBlockSize\":8," // - + "\"primaryMasterKey\":\"mM+qoQ+o0qvPTiDAZYt+flaC3WbpNAx1sTXaUzxwpy0M9Ctj6Tih/Q==\"," // - + "\"hmacMasterKey\":\"mM+qoQ+o0qvPTiDAZYt+flaC3WbpNAx1sTXaUzxwpy0M9Ctj6Tih/Q==\"}"; - final Cryptor cryptor = new CryptorImpl(RANDOM_MOCK); - cryptor.readKeysFromMasterkeyFile(testMasterKey.getBytes(), "asd"); - - final AtomicReference receivedByT1 = new AtomicReference<>(); - final Thread t1 = new Thread(() -> { - receivedByT1.set(cryptor.getFilenameCryptor()); - }); - - final AtomicReference receivedByT2 = new AtomicReference<>(); - final Thread t2 = new Thread(() -> { - receivedByT2.set(cryptor.getFilenameCryptor()); - }); - t1.start(); - t2.start(); - t1.join(); - t2.join(); - // It is not guaranteed, both threads will enter getFilenameCryptor() exactly simultaneously. (But logging shows it is very likely) - // In any case both threads should receive the same FilenameCryptor - Assert.assertSame(receivedByT1.get(), receivedByT2.get()); - } - } diff --git a/main/filesystem-crypto/src/test/java/org/cryptomator/crypto/engine/impl/FileContentEncryptorImplTest.java b/main/filesystem-crypto/src/test/java/org/cryptomator/crypto/engine/impl/FileContentEncryptorImplTest.java new file mode 100644 index 000000000..dd720d43d --- /dev/null +++ b/main/filesystem-crypto/src/test/java/org/cryptomator/crypto/engine/impl/FileContentEncryptorImplTest.java @@ -0,0 +1,50 @@ +package org.cryptomator.crypto.engine.impl; + +import java.nio.ByteBuffer; +import java.security.SecureRandom; +import java.util.Arrays; + +import javax.crypto.SecretKey; +import javax.crypto.spec.SecretKeySpec; + +import org.bouncycastle.util.encoders.Base64; +import org.cryptomator.crypto.engine.FileContentEncryptor; +import org.cryptomator.io.ByteBuffers; +import org.junit.Assert; +import org.junit.Test; + +public class FileContentEncryptorImplTest { + + private static final SecureRandom RANDOM_MOCK = new SecureRandom() { + + private static final long serialVersionUID = 1505563778398085504L; + + @Override + public void nextBytes(byte[] bytes) { + Arrays.fill(bytes, (byte) 0x00); + } + + }; + + @Test + public void testEncryption() throws InterruptedException { + final byte[] keyBytes = new byte[32]; + final SecretKey headerKey = new SecretKeySpec(keyBytes, "AES"); + final SecretKey macKey = new SecretKeySpec(keyBytes, "AES"); + FileContentEncryptor encryptor = new FileContentEncryptorImpl(headerKey, macKey, RANDOM_MOCK); + + encryptor.append(ByteBuffer.wrap("hello world".getBytes())); + encryptor.append(FileContentEncryptor.EOF); + + ByteBuffer result = ByteBuffer.allocate(11); // we just care about the first 11 bytes, as this is the ciphertext. + ByteBuffer buf; + while ((buf = encryptor.ciphertext()) != FileContentEncryptor.EOF) { + ByteBuffers.copy(buf, result); + } + + // echo -n "hello world" | openssl enc -aes-256-ctr -K 0000000000000000000000000000000000000000000000000000000000000000 -iv 00000000000000000000000000000000 | base64 + final String expected = "tPCsFM1g/ubfJMY="; + Assert.assertArrayEquals(Base64.decode(expected), result.array()); + } + +} diff --git a/main/filesystem-crypto/src/test/java/org/cryptomator/crypto/engine/impl/TestCryptorImplFactory.java b/main/filesystem-crypto/src/test/java/org/cryptomator/crypto/engine/impl/TestCryptorImplFactory.java new file mode 100644 index 000000000..efba2a189 --- /dev/null +++ b/main/filesystem-crypto/src/test/java/org/cryptomator/crypto/engine/impl/TestCryptorImplFactory.java @@ -0,0 +1,26 @@ +package org.cryptomator.crypto.engine.impl; + +import java.security.SecureRandom; +import java.util.Arrays; + +public class TestCryptorImplFactory { + + private static final SecureRandom RANDOM_MOCK = new SecureRandom() { + + private static final long serialVersionUID = 1505563778398085504L; + + @Override + public void nextBytes(byte[] bytes) { + Arrays.fill(bytes, (byte) 0x00); + } + + }; + + /** + * @return A CryptorImpl with a mocked PRNG, that can be used during tests without the need of "real" random numbers. + */ + public static CryptorImpl insecureCryptorImpl() { + return new CryptorImpl(RANDOM_MOCK); + } + +} diff --git a/main/filesystem-crypto/src/test/java/org/cryptomator/crypto/fs/EncryptAndShortenIntegrationTest.java b/main/filesystem-crypto/src/test/java/org/cryptomator/crypto/fs/EncryptAndShortenIntegrationTest.java index ee05654e3..37dd19fc8 100644 --- a/main/filesystem-crypto/src/test/java/org/cryptomator/crypto/fs/EncryptAndShortenIntegrationTest.java +++ b/main/filesystem-crypto/src/test/java/org/cryptomator/crypto/fs/EncryptAndShortenIntegrationTest.java @@ -1,10 +1,7 @@ package org.cryptomator.crypto.fs; -import java.security.SecureRandom; -import java.util.Arrays; - import org.cryptomator.crypto.engine.Cryptor; -import org.cryptomator.crypto.engine.impl.CryptorImpl; +import org.cryptomator.crypto.engine.impl.TestCryptorImplFactory; import org.cryptomator.filesystem.FileSystem; import org.cryptomator.filesystem.Folder; import org.cryptomator.filesystem.FolderCreateMode; @@ -20,22 +17,11 @@ public class EncryptAndShortenIntegrationTest { private static final Logger LOG = LoggerFactory.getLogger(EncryptAndShortenIntegrationTest.class); - private static final SecureRandom RANDOM_MOCK = new SecureRandom() { - - private static final long serialVersionUID = 1505563778398085504L; - - @Override - public void nextBytes(byte[] bytes) { - Arrays.fill(bytes, (byte) 0x00); - } - - }; - @Test public void testEncryptionOfLongFolderNames() { final FileSystem physicalFs = new InMemoryFileSystem(); final FileSystem shorteningFs = new ShorteningFileSystem(physicalFs, physicalFs.folder("m"), 70); - final Cryptor cryptor = new CryptorImpl(RANDOM_MOCK); + final Cryptor cryptor = TestCryptorImplFactory.insecureCryptorImpl(); cryptor.randomizeMasterkey(); final FileSystem fs = new CryptoFileSystem(shorteningFs, cryptor, "foo"); fs.create(FolderCreateMode.FAIL_IF_PARENT_IS_MISSING);