From 0d2a6da9f8ff3d3785f3a7df6c7f32d6c7c61c14 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 3 Dec 2021 13:50:44 +0100 Subject: [PATCH 01/22] create object pool --- .../cryptolib/common/ObjectPool.java | 48 +++++++++++++++ .../cryptolib/common/ObjectPoolTest.java | 61 +++++++++++++++++++ 2 files changed, 109 insertions(+) create mode 100644 src/main/java/org/cryptomator/cryptolib/common/ObjectPool.java create mode 100644 src/test/java/org/cryptomator/cryptolib/common/ObjectPoolTest.java diff --git a/src/main/java/org/cryptomator/cryptolib/common/ObjectPool.java b/src/main/java/org/cryptomator/cryptolib/common/ObjectPool.java new file mode 100644 index 0000000..730a3f7 --- /dev/null +++ b/src/main/java/org/cryptomator/cryptolib/common/ObjectPool.java @@ -0,0 +1,48 @@ +package org.cryptomator.cryptolib.common; + +import java.lang.ref.WeakReference; +import java.util.Queue; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.function.Supplier; + +public class ObjectPool { + + private final Queue> pool; + private final Supplier factory; + + public ObjectPool(Supplier factory) { + this.pool = new ConcurrentLinkedQueue<>(); + this.factory = factory; + } + + public Lease get() { + WeakReference ref; + while ((ref = pool.poll()) != null) { + T cached = ref.get(); + if (cached != null) { + return new Lease(cached); + } + } + return new Lease(factory.get()); + } + + public class Lease implements AutoCloseable { + + private T obj; + + public Lease(T obj) { + this.obj = obj; + } + + public T get() { + return obj; + } + + @Override + public void close() { + pool.offer(new WeakReference<>(obj)); + obj = null; + } + } + +} diff --git a/src/test/java/org/cryptomator/cryptolib/common/ObjectPoolTest.java b/src/test/java/org/cryptomator/cryptolib/common/ObjectPoolTest.java new file mode 100644 index 0000000..2838d76 --- /dev/null +++ b/src/test/java/org/cryptomator/cryptolib/common/ObjectPoolTest.java @@ -0,0 +1,61 @@ +package org.cryptomator.cryptolib.common; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import java.util.function.Supplier; + +public class ObjectPoolTest { + + private Supplier factory = Mockito.mock(Supplier.class); + private ObjectPool pool = new ObjectPool<>(factory); + + @BeforeEach + public void setup() { + Mockito.doAnswer(invocation -> new Foo()).when(factory).get(); + } + + @Test + @DisplayName("new instance is created if pool is empty") + public void testCreateNewObjWhenPoolIsEmpty() { + try (ObjectPool.Lease lease1 = pool.get()) { + try (ObjectPool.Lease lease2 = pool.get()) { + Assertions.assertNotSame(lease1.get(), lease2.get()); + } + } + Mockito.verify(factory, Mockito.times(2)).get(); + } + + @Test + @DisplayName("recycle existing instance") + public void testRecycleExistingObj() { + Foo foo1; + try (ObjectPool.Lease lease = pool.get()) { + foo1 = lease.get(); + } + try (ObjectPool.Lease lease = pool.get()) { + Assertions.assertSame(foo1, lease.get()); + } + Mockito.verify(factory, Mockito.times(1)).get(); + } + + @Test + @DisplayName("create new instance when pool is GC'ed") + public void testGc() { + try (ObjectPool.Lease lease = pool.get()) { + Assertions.assertNotNull(lease.get()); + } + System.gc(); // seems to be reliable on Temurin 17 with @RepeatedTest(1000) + try (ObjectPool.Lease lease = pool.get()) { + Assertions.assertNotNull(lease.get()); + } + Mockito.verify(factory, Mockito.times(2)).get(); + } + + private static class Foo { + } + +} \ No newline at end of file From 05554f6b0a5326a5f10479b49e24b6ca55386dee Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 3 Dec 2021 14:28:29 +0100 Subject: [PATCH 02/22] use object pool in CipherSupplier --- .../cryptolib/common/CipherSupplier.java | 89 ++++++++++++++----- .../cryptolib/common/CipherSupplierTest.java | 4 +- 2 files changed, 71 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptolib/common/CipherSupplier.java b/src/main/java/org/cryptomator/cryptolib/common/CipherSupplier.java index edbc13e..45e1875 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/CipherSupplier.java +++ b/src/main/java/org/cryptomator/cryptolib/common/CipherSupplier.java @@ -15,7 +15,6 @@ import java.security.InvalidKeyException; import java.security.NoSuchAlgorithmException; import java.security.spec.AlgorithmParameterSpec; -import java.util.function.Function; public final class CipherSupplier { @@ -24,51 +23,101 @@ public final class CipherSupplier { public static final CipherSupplier RFC3394_KEYWRAP = new CipherSupplier("AESWrap"); private final String cipherAlgorithm; - private final ThreadLocal threadLocal; + private final ObjectPool cipherPool; public CipherSupplier(String cipherAlgorithm) { this.cipherAlgorithm = cipherAlgorithm; - this.threadLocal = new Provider(); - this.threadLocal.get(); // eagerly initialize to provoke exceptions + this.cipherPool = new ObjectPool<>(this::createCipher); + try (ObjectPool.Lease lease = cipherPool.get()) { + lease.get(); // eagerly initialize to provoke exceptions + } } - private class Provider extends ThreadLocal { - @Override - protected Cipher initialValue() { - try { - return Cipher.getInstance(cipherAlgorithm); - } catch (NoSuchAlgorithmException | NoSuchPaddingException e) { - throw new IllegalArgumentException("Invalid cipher algorithm or padding.", e); - } + private Cipher createCipher() { + try { + return Cipher.getInstance(cipherAlgorithm); + } catch (NoSuchAlgorithmException | NoSuchPaddingException e) { + throw new IllegalArgumentException("Invalid cipher algorithm or padding.", e); } } + public ReusableCipher encrypt(SecretKey key, AlgorithmParameterSpec params) { + ObjectPool.Lease lease = cipherPool.get(); + initMode(lease.get(), Cipher.ENCRYPT_MODE, key, params); + return new ReusableCipher(lease); + } + + @Deprecated public Cipher forEncryption(SecretKey key, AlgorithmParameterSpec params) { - return forMode(Cipher.ENCRYPT_MODE, key, params); + final Cipher cipher = createCipher(); + initMode(cipher, Cipher.ENCRYPT_MODE, key, params); + return cipher; } + public ReusableCipher decrypt(SecretKey key, AlgorithmParameterSpec params) { + ObjectPool.Lease lease = cipherPool.get(); + initMode(lease.get(), Cipher.DECRYPT_MODE, key, params); + return new ReusableCipher(lease); + } + + @Deprecated public Cipher forDecryption(SecretKey key, AlgorithmParameterSpec params) { - return forMode(Cipher.DECRYPT_MODE, key, params); + final Cipher cipher = createCipher(); + initMode(cipher, Cipher.DECRYPT_MODE, key, params); + return cipher; + } + + public ReusableCipher wrap(SecretKey kek) { + ObjectPool.Lease lease = cipherPool.get(); + initMode(lease.get(), Cipher.WRAP_MODE, kek, null); + return new ReusableCipher(lease); } + @Deprecated public Cipher forWrapping(SecretKey kek) { - return forMode(Cipher.WRAP_MODE, kek, null); + final Cipher cipher = createCipher(); + initMode(cipher, Cipher.WRAP_MODE, kek, null); + return cipher; + } + + public ReusableCipher unwrap(SecretKey kek) { + ObjectPool.Lease lease = cipherPool.get(); + initMode(lease.get(), Cipher.UNWRAP_MODE, kek, null); + return new ReusableCipher(lease); } + @Deprecated public Cipher forUnwrapping(SecretKey kek) { - return forMode(Cipher.UNWRAP_MODE, kek, null); + final Cipher cipher = createCipher(); + initMode(cipher, Cipher.UNWRAP_MODE, kek, null); + return cipher; } - // visible for testing - Cipher forMode(int ciphermode, SecretKey key, AlgorithmParameterSpec params) { - final Cipher cipher = threadLocal.get(); + private void initMode(Cipher cipher, int ciphermode, SecretKey key, AlgorithmParameterSpec params) { try { cipher.init(ciphermode, key, params); - return cipher; } catch (InvalidKeyException e) { throw new IllegalArgumentException("Invalid key.", e); } catch (InvalidAlgorithmParameterException e) { throw new IllegalArgumentException("Algorithm parameter not appropriate for " + cipher.getAlgorithm() + ".", e); } } + + public static class ReusableCipher implements AutoCloseable { + + private final ObjectPool.Lease lease; + + private ReusableCipher(ObjectPool.Lease lease) { + this.lease = lease; + } + + public Cipher get() { + return lease.get(); + } + + @Override + public void close() { + lease.close(); + } + } } \ No newline at end of file diff --git a/src/test/java/org/cryptomator/cryptolib/common/CipherSupplierTest.java b/src/test/java/org/cryptomator/cryptolib/common/CipherSupplierTest.java index 03a97eb..4142649 100644 --- a/src/test/java/org/cryptomator/cryptolib/common/CipherSupplierTest.java +++ b/src/test/java/org/cryptomator/cryptolib/common/CipherSupplierTest.java @@ -31,7 +31,7 @@ public void testGetUnknownCipher() { public void testGetCipherWithInvalidKey() { CipherSupplier supplier = new CipherSupplier("AES/CBC/PKCS5Padding"); IllegalArgumentException exception = Assertions.assertThrows(IllegalArgumentException.class, () -> { - supplier.forMode(Cipher.ENCRYPT_MODE, new DestroyableSecretKey(new byte[13], "AES"), new IvParameterSpec(new byte[16])); + supplier.forEncryption(new DestroyableSecretKey(new byte[13], "AES"), new IvParameterSpec(new byte[16])); }); MatcherAssert.assertThat(exception.getMessage(), CoreMatchers.containsString("Invalid key")); } @@ -40,7 +40,7 @@ public void testGetCipherWithInvalidKey() { public void testGetCipherWithInvalidAlgorithmParam() { CipherSupplier supplier = new CipherSupplier("AES/CBC/PKCS5Padding"); IllegalArgumentException exception = Assertions.assertThrows(IllegalArgumentException.class, () -> { - supplier.forMode(Cipher.ENCRYPT_MODE, new DestroyableSecretKey(new byte[16], "AES"), new RC5ParameterSpec(1, 1, 8)); + supplier.forEncryption(new DestroyableSecretKey(new byte[16], "AES"), new RC5ParameterSpec(1, 1, 8)); }); MatcherAssert.assertThat(exception.getMessage(), CoreMatchers.containsString("Algorithm parameter not appropriate for")); } From 94d629a4592e122be7c7e145003d396da26e44a4 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 3 Dec 2021 14:28:40 +0100 Subject: [PATCH 03/22] use new CipherSupplier API --- .../cryptolib/common/AesKeyWrap.java | 12 ++++---- .../cryptolib/ecies/GcmWithSecretNonce.java | 11 +++++--- .../cryptolib/v1/FileContentCryptorImpl.java | 17 ++++++----- .../cryptolib/v1/FileHeaderCryptorImpl.java | 17 +++++------ .../cryptolib/v2/FileContentCryptorImpl.java | 28 ++++++++++--------- .../cryptolib/v2/FileHeaderCryptorImpl.java | 15 +++++----- .../cryptolib/common/CipherSupplierTest.java | 4 +-- .../ecies/GcmWithSecretNonceTest.java | 5 +++- .../v2/FileHeaderCryptorImplTest.java | 7 ++++- 9 files changed, 67 insertions(+), 49 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptolib/common/AesKeyWrap.java b/src/main/java/org/cryptomator/cryptolib/common/AesKeyWrap.java index 0e9bcd9..619dd49 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/AesKeyWrap.java +++ b/src/main/java/org/cryptomator/cryptolib/common/AesKeyWrap.java @@ -22,9 +22,9 @@ public class AesKeyWrap { * @return Wrapped key */ public static byte[] wrap(DestroyableSecretKey kek, SecretKey key) { - try (DestroyableSecretKey kekCopy = kek.copy()) { - final Cipher cipher = CipherSupplier.RFC3394_KEYWRAP.forWrapping(kekCopy); - return cipher.wrap(key); + try (DestroyableSecretKey kekCopy = kek.copy(); + CipherSupplier.ReusableCipher cipher = CipherSupplier.RFC3394_KEYWRAP.wrap(kekCopy)) { + return cipher.get().wrap(key); } catch (InvalidKeyException | IllegalBlockSizeException e) { throw new IllegalArgumentException("Unable to wrap key.", e); } @@ -43,9 +43,9 @@ public static DestroyableSecretKey unwrap(DestroyableSecretKey kek, byte[] wrapp // visible for testing static DestroyableSecretKey unwrap(DestroyableSecretKey kek, byte[] wrappedKey, String wrappedKeyAlgorithm, int wrappedKeyType) throws InvalidKeyException { - try (DestroyableSecretKey kekCopy = kek.copy()) { - final Cipher cipher = CipherSupplier.RFC3394_KEYWRAP.forUnwrapping(kekCopy); - return DestroyableSecretKey.from(cipher.unwrap(wrappedKey, wrappedKeyAlgorithm, wrappedKeyType)); + try (DestroyableSecretKey kekCopy = kek.copy(); + CipherSupplier.ReusableCipher cipher = CipherSupplier.RFC3394_KEYWRAP.unwrap(kekCopy)) { + return DestroyableSecretKey.from(cipher.get().unwrap(wrappedKey, wrappedKeyAlgorithm, wrappedKeyType)); } catch (NoSuchAlgorithmException e) { throw new IllegalArgumentException("Invalid algorithm: " + wrappedKeyAlgorithm, e); } diff --git a/src/main/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonce.java b/src/main/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonce.java index 891ef83..3b053c1 100644 --- a/src/main/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonce.java +++ b/src/main/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonce.java @@ -9,6 +9,7 @@ import javax.crypto.Cipher; import javax.crypto.IllegalBlockSizeException; import javax.crypto.spec.GCMParameterSpec; +import javax.crypto.spec.IvParameterSpec; import java.util.Arrays; class GcmWithSecretNonce implements AuthenticatedEncryption { @@ -26,8 +27,9 @@ public int requiredSecretBytes() { public byte[] encrypt(byte[] secret, byte[] plaintext) { try (DestroyableSecretKey key = new DestroyableSecretKey(secret, 0, GCM_KEY_SIZE, "AES")) { byte[] nonce = Arrays.copyOfRange(secret, GCM_KEY_SIZE, GCM_KEY_SIZE + GCM_NONCE_SIZE); - Cipher cipher = CipherSupplier.AES_GCM.forEncryption(key, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce)); - return cipher.doFinal(plaintext); + try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_GCM.encrypt(key, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { + return cipher.get().doFinal(plaintext); + } } catch (IllegalBlockSizeException | BadPaddingException e) { throw new IllegalStateException("Unexpected exception during GCM decryption.", e); } @@ -37,8 +39,9 @@ public byte[] encrypt(byte[] secret, byte[] plaintext) { public byte[] decrypt(byte[] secret, byte[] ciphertext) throws AEADBadTagException { try (DestroyableSecretKey key = new DestroyableSecretKey(secret, 0, GCM_KEY_SIZE, "AES")) { byte[] nonce = Arrays.copyOfRange(secret, GCM_KEY_SIZE, GCM_KEY_SIZE + GCM_NONCE_SIZE); - Cipher cipher = CipherSupplier.AES_GCM.forDecryption(key, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce)); - return cipher.doFinal(ciphertext); + try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_GCM.decrypt(key, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { + return cipher.get().doFinal(ciphertext); + } } catch (IllegalBlockSizeException | BadPaddingException e) { Throwables.throwIfInstanceOf(e, AEADBadTagException.class); throw new IllegalStateException("Unexpected exception during GCM decryption.", e); diff --git a/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java index cad0d0b..ee024ef 100644 --- a/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java @@ -107,12 +107,14 @@ void encryptChunk(ByteBuffer cleartextChunk, ByteBuffer ciphertextChunk, long ch // nonce: byte[] nonce = new byte[NONCE_SIZE]; random.nextBytes(nonce); + ciphertextChunk.put(nonce); // payload: - final Cipher cipher = CipherSupplier.AES_CTR.forEncryption(fk, new IvParameterSpec(nonce)); - ciphertextChunk.put(nonce); - assert ciphertextChunk.remaining() >= cipher.getOutputSize(cleartextChunk.remaining()) + MAC_SIZE; - int bytesEncrypted = cipher.doFinal(cleartextChunk, ciphertextChunk); + int bytesEncrypted; + try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_CTR.encrypt(fk, new IvParameterSpec(nonce))) { + assert ciphertextChunk.remaining() >= cipher.get().getOutputSize(cleartextChunk.remaining()) + MAC_SIZE; + bytesEncrypted = cipher.get().doFinal(cleartextChunk, ciphertextChunk); + } // mac: final ByteBuffer ciphertextBuf = ciphertextChunk.asReadOnlyBuffer(); @@ -143,9 +145,10 @@ void decryptChunk(ByteBuffer ciphertextChunk, ByteBuffer cleartextChunk, Destroy payloadBuf.position(NONCE_SIZE).limit(ciphertextChunk.limit() - MAC_SIZE); // payload: - final Cipher cipher = CipherSupplier.AES_CTR.forDecryption(fk, new IvParameterSpec(nonce)); - assert cleartextChunk.remaining() >= cipher.getOutputSize(payloadBuf.remaining()); - cipher.doFinal(payloadBuf, cleartextChunk); + try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_CTR.encrypt(fk, new IvParameterSpec(nonce))) { + assert cleartextChunk.remaining() >= cipher.get().getOutputSize(payloadBuf.remaining()); + cipher.get().doFinal(payloadBuf, cleartextChunk); + } } catch (ShortBufferException e) { throw new IllegalStateException("Buffer allocated for reported output size apparently not big enough.", e); } catch (IllegalBlockSizeException | BadPaddingException e) { diff --git a/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java index aeaa7ee..6064188 100644 --- a/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java @@ -17,7 +17,6 @@ import org.cryptomator.cryptolib.common.MacSupplier; import javax.crypto.BadPaddingException; -import javax.crypto.Cipher; import javax.crypto.IllegalBlockSizeException; import javax.crypto.Mac; import javax.crypto.ShortBufferException; @@ -61,9 +60,10 @@ public ByteBuffer encryptHeader(FileHeader header) { result.put(headerImpl.getNonce()); // encrypt payload: - Cipher cipher = CipherSupplier.AES_CTR.forEncryption(ek, new IvParameterSpec(headerImpl.getNonce())); - int encrypted = cipher.doFinal(payloadCleartextBuf, result); - assert encrypted == FileHeaderImpl.Payload.SIZE; + try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_CTR.encrypt(ek, new IvParameterSpec(headerImpl.getNonce()))) { + int encrypted = cipher.get().doFinal(payloadCleartextBuf, result); + assert encrypted == FileHeaderImpl.Payload.SIZE; + } // mac nonce and ciphertext: ByteBuffer nonceAndCiphertextBuf = result.duplicate(); @@ -114,10 +114,11 @@ public FileHeader decryptHeader(ByteBuffer ciphertextHeaderBuf) throws Authentic ByteBuffer payloadCleartextBuf = ByteBuffer.allocate(FileHeaderImpl.Payload.SIZE); try (DestroyableSecretKey ek = masterkey.getEncKey()) { // decrypt payload: - Cipher cipher = CipherSupplier.AES_CTR.forDecryption(ek, new IvParameterSpec(nonce)); - assert cipher.getOutputSize(ciphertextPayload.length) == payloadCleartextBuf.remaining(); - int decrypted = cipher.doFinal(ByteBuffer.wrap(ciphertextPayload), payloadCleartextBuf); - assert decrypted == FileHeaderImpl.Payload.SIZE; + try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_CTR.encrypt(ek, new IvParameterSpec(nonce))) { + assert cipher.get().getOutputSize(ciphertextPayload.length) == payloadCleartextBuf.remaining(); + int decrypted = cipher.get().doFinal(ByteBuffer.wrap(ciphertextPayload), payloadCleartextBuf); + assert decrypted == FileHeaderImpl.Payload.SIZE; + } payloadCleartextBuf.flip(); FileHeaderImpl.Payload payload = FileHeaderImpl.Payload.decode(payloadCleartextBuf); diff --git a/src/main/java/org/cryptomator/cryptolib/v2/FileContentCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v2/FileContentCryptorImpl.java index a03eea9..5e7673f 100644 --- a/src/main/java/org/cryptomator/cryptolib/v2/FileContentCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v2/FileContentCryptorImpl.java @@ -103,13 +103,14 @@ void encryptChunk(ByteBuffer cleartextChunk, ByteBuffer ciphertextChunk, long ch random.nextBytes(nonce); // payload: - final Cipher cipher = CipherSupplier.AES_GCM.forEncryption(fk, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce)); - final byte[] chunkNumberBigEndian = longToBigEndianByteArray(chunkNumber); - cipher.updateAAD(chunkNumberBigEndian); - cipher.updateAAD(headerNonce); - ciphertextChunk.put(nonce); - assert ciphertextChunk.remaining() >= cipher.getOutputSize(cleartextChunk.remaining()); - cipher.doFinal(cleartextChunk, ciphertextChunk); + try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_GCM.encrypt(fk, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { + final byte[] chunkNumberBigEndian = longToBigEndianByteArray(chunkNumber); + cipher.get().updateAAD(chunkNumberBigEndian); + cipher.get().updateAAD(headerNonce); + ciphertextChunk.put(nonce); + assert ciphertextChunk.remaining() >= cipher.get().getOutputSize(cleartextChunk.remaining()); + cipher.get().doFinal(cleartextChunk, ciphertextChunk); + } } catch (ShortBufferException e) { throw new IllegalStateException("Buffer allocated for reported output size apparently not big enough.", e); } catch (IllegalBlockSizeException | BadPaddingException e) { @@ -134,12 +135,13 @@ void decryptChunk(ByteBuffer ciphertextChunk, ByteBuffer cleartextChunk, long ch assert payloadBuf.remaining() >= GCM_TAG_SIZE; // payload: - final Cipher cipher = CipherSupplier.AES_GCM.forDecryption(fk, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce)); - final byte[] chunkNumberBigEndian = longToBigEndianByteArray(chunkNumber); - cipher.updateAAD(chunkNumberBigEndian); - cipher.updateAAD(headerNonce); - assert cleartextChunk.remaining() >= cipher.getOutputSize(payloadBuf.remaining()); - cipher.doFinal(payloadBuf, cleartextChunk); + try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_GCM.decrypt(fk, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { + final byte[] chunkNumberBigEndian = longToBigEndianByteArray(chunkNumber); + cipher.get().updateAAD(chunkNumberBigEndian); + cipher.get().updateAAD(headerNonce); + assert cleartextChunk.remaining() >= cipher.get().getOutputSize(payloadBuf.remaining()); + cipher.get().doFinal(payloadBuf, cleartextChunk); + } } catch (AEADBadTagException e) { throw new AuthenticationFailedException("Content tag mismatch.", e); } catch (ShortBufferException e) { diff --git a/src/main/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImpl.java index 509a5b9..c2a5ae5 100644 --- a/src/main/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImpl.java @@ -61,10 +61,10 @@ public ByteBuffer encryptHeader(FileHeader header) { result.put(headerImpl.getNonce()); // encrypt payload: - Cipher cipher = CipherSupplier.AES_GCM.forEncryption(ek, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, headerImpl.getNonce())); - int encrypted = cipher.doFinal(payloadCleartextBuf, result); - assert encrypted == FileHeaderImpl.PAYLOAD_LEN + FileHeaderImpl.TAG_LEN; - + try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_GCM.encrypt(ek, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, headerImpl.getNonce()))) { + int encrypted = cipher.get().doFinal(payloadCleartextBuf, result); + assert encrypted == FileHeaderImpl.PAYLOAD_LEN + FileHeaderImpl.TAG_LEN; + } result.flip(); return result; } catch (ShortBufferException e) { @@ -93,9 +93,10 @@ public FileHeader decryptHeader(ByteBuffer ciphertextHeaderBuf) throws Authentic ByteBuffer payloadCleartextBuf = ByteBuffer.allocate(FileHeaderImpl.Payload.SIZE + GCM_TAG_SIZE); try (DestroyableSecretKey ek = masterkey.getEncKey()) { // decrypt payload: - Cipher cipher = CipherSupplier.AES_GCM.forDecryption(ek, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce)); - int decrypted = cipher.doFinal(ByteBuffer.wrap(ciphertextAndTag), payloadCleartextBuf); - assert decrypted == FileHeaderImpl.Payload.SIZE; + try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_GCM.decrypt(ek, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { + int decrypted = cipher.get().doFinal(ByteBuffer.wrap(ciphertextAndTag), payloadCleartextBuf); + assert decrypted == FileHeaderImpl.Payload.SIZE; + } payloadCleartextBuf.flip(); FileHeaderImpl.Payload payload = FileHeaderImpl.Payload.decode(payloadCleartextBuf); diff --git a/src/test/java/org/cryptomator/cryptolib/common/CipherSupplierTest.java b/src/test/java/org/cryptomator/cryptolib/common/CipherSupplierTest.java index 4142649..0c62435 100644 --- a/src/test/java/org/cryptomator/cryptolib/common/CipherSupplierTest.java +++ b/src/test/java/org/cryptomator/cryptolib/common/CipherSupplierTest.java @@ -31,7 +31,7 @@ public void testGetUnknownCipher() { public void testGetCipherWithInvalidKey() { CipherSupplier supplier = new CipherSupplier("AES/CBC/PKCS5Padding"); IllegalArgumentException exception = Assertions.assertThrows(IllegalArgumentException.class, () -> { - supplier.forEncryption(new DestroyableSecretKey(new byte[13], "AES"), new IvParameterSpec(new byte[16])); + supplier.encrypt(new DestroyableSecretKey(new byte[13], "AES"), new IvParameterSpec(new byte[16])); }); MatcherAssert.assertThat(exception.getMessage(), CoreMatchers.containsString("Invalid key")); } @@ -40,7 +40,7 @@ public void testGetCipherWithInvalidKey() { public void testGetCipherWithInvalidAlgorithmParam() { CipherSupplier supplier = new CipherSupplier("AES/CBC/PKCS5Padding"); IllegalArgumentException exception = Assertions.assertThrows(IllegalArgumentException.class, () -> { - supplier.forEncryption(new DestroyableSecretKey(new byte[16], "AES"), new RC5ParameterSpec(1, 1, 8)); + supplier.encrypt(new DestroyableSecretKey(new byte[16], "AES"), new RC5ParameterSpec(1, 1, 8)); }); MatcherAssert.assertThat(exception.getMessage(), CoreMatchers.containsString("Algorithm parameter not appropriate for")); } diff --git a/src/test/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonceTest.java b/src/test/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonceTest.java index 91ed77a..6873e2b 100644 --- a/src/test/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonceTest.java +++ b/src/test/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonceTest.java @@ -10,6 +10,7 @@ import org.junit.jupiter.params.provider.CsvSource; import javax.crypto.AEADBadTagException; +import javax.crypto.spec.GCMParameterSpec; /** * Test vectors from https://csrc.nist.rip/groups/ST/toolkit/BCM/documents/proposedmodes/gcm/gcm-spec.pdf @@ -22,7 +23,9 @@ public class GcmWithSecretNonceTest { public void setup() { // reset cipher state to avoid InvalidAlgorithmParameterExceptions due to IV-reuse GcmTestHelper.reset((mode, key, params) -> { - CipherSupplier.AES_GCM.forEncryption(key, params); + try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_GCM.encrypt(key, params)) { + cipher.get(); + } }); } diff --git a/src/test/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImplTest.java b/src/test/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImplTest.java index 1cfc50f..fd4796c 100644 --- a/src/test/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImplTest.java +++ b/src/test/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImplTest.java @@ -19,9 +19,12 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import javax.crypto.spec.GCMParameterSpec; import java.nio.ByteBuffer; import java.security.SecureRandom; +import static org.cryptomator.cryptolib.v2.Constants.GCM_TAG_SIZE; + public class FileHeaderCryptorImplTest { private static final SecureRandom RANDOM_MOCK = SecureRandomMock.NULL_RANDOM; @@ -35,7 +38,9 @@ public void setup() { // reset cipher state to avoid InvalidAlgorithmParameterExceptions due to IV-reuse GcmTestHelper.reset((mode, key, params) -> { - CipherSupplier.AES_GCM.forEncryption(key, params); + try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_GCM.encrypt(key, params)) { + cipher.get(); + } }); } From bf462146d6a770446b493e310a22c0e943ae5e2b Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 3 Dec 2021 14:44:23 +0100 Subject: [PATCH 04/22] use object pool in MacSupplier --- .../cryptolib/common/MacSupplier.java | 59 ++++++++++++++----- .../cryptolib/common/MacSupplierTest.java | 21 ++++--- 2 files changed, 54 insertions(+), 26 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptolib/common/MacSupplier.java b/src/main/java/org/cryptomator/cryptolib/common/MacSupplier.java index f58514b..54615e9 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/MacSupplier.java +++ b/src/main/java/org/cryptomator/cryptolib/common/MacSupplier.java @@ -8,43 +8,72 @@ *******************************************************************************/ package org.cryptomator.cryptolib.common; -import java.security.InvalidKeyException; -import java.security.NoSuchAlgorithmException; - +import javax.crypto.Cipher; import javax.crypto.Mac; import javax.crypto.SecretKey; +import java.security.InvalidKeyException; +import java.security.NoSuchAlgorithmException; public final class MacSupplier { public static final MacSupplier HMAC_SHA256 = new MacSupplier("HmacSHA256"); private final String macAlgorithm; - private final ThreadLocal threadLocal; + private final ObjectPool macPool; public MacSupplier(String macAlgorithm) { this.macAlgorithm = macAlgorithm; - this.threadLocal = new Provider(); + this.macPool = new ObjectPool<>(this::createMac); + try (ObjectPool.Lease lease = macPool.get()) { + lease.get(); // eagerly initialize to provoke exceptions + } } - private class Provider extends ThreadLocal { - @Override - protected Mac initialValue() { - try { - return Mac.getInstance(macAlgorithm); - } catch (NoSuchAlgorithmException e) { - throw new IllegalArgumentException("Invalid MAC algorithm.", e); - } + private Mac createMac() { + try { + return Mac.getInstance(macAlgorithm); + } catch (NoSuchAlgorithmException e) { + throw new IllegalArgumentException("Invalid MAC algorithm.", e); } } + public ReusableMac keyed(SecretKey key) { + ObjectPool.Lease lease = macPool.get(); + init(lease.get(), key); + return new ReusableMac(lease); + } + + @Deprecated public Mac withKey(SecretKey key) { + final Mac mac = createMac(); + init(mac, key); + return mac; + } + + private void init(Mac mac, SecretKey key) { try { - final Mac mac = threadLocal.get(); mac.init(key); - return mac; } catch (InvalidKeyException e) { throw new IllegalArgumentException("Invalid key.", e); } } + public static class ReusableMac implements AutoCloseable { + + private final ObjectPool.Lease lease; + + private ReusableMac(ObjectPool.Lease lease) { + this.lease = lease; + } + + public Mac get() { + return lease.get(); + } + + @Override + public void close() { + lease.close(); + } + } + } \ No newline at end of file diff --git a/src/test/java/org/cryptomator/cryptolib/common/MacSupplierTest.java b/src/test/java/org/cryptomator/cryptolib/common/MacSupplierTest.java index 9a3458d..4aefda4 100644 --- a/src/test/java/org/cryptomator/cryptolib/common/MacSupplierTest.java +++ b/src/test/java/org/cryptomator/cryptolib/common/MacSupplierTest.java @@ -11,11 +11,9 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; -import javax.crypto.Mac; import javax.crypto.SecretKey; import javax.crypto.spec.SecretKeySpec; import java.lang.reflect.Method; -import java.security.InvalidKeyException; import java.security.Key; import java.security.KeyPairGenerator; import java.security.NoSuchAlgorithmException; @@ -24,27 +22,28 @@ public class MacSupplierTest { @Test public void testConstructorWithInvalidDigest() { - SecretKey key = new SecretKeySpec(new byte[16], "HmacSHA256"); Assertions.assertThrows(IllegalArgumentException.class, () -> { - new MacSupplier("FOO3000").withKey(key); + new MacSupplier("FOO3000"); }); } @Test - public void testGetMac() throws InvalidKeyException, NoSuchAlgorithmException { + public void testGetMac() { SecretKey key = new SecretKeySpec(new byte[16], "HmacSHA256"); - Mac mac1 = MacSupplier.HMAC_SHA256.withKey(key); - Assertions.assertNotNull(mac1); + try (MacSupplier.ReusableMac mac1 = MacSupplier.HMAC_SHA256.keyed(key)) { + Assertions.assertNotNull(mac1); + } - Mac mac2 = MacSupplier.HMAC_SHA256.withKey(key); - Assertions.assertSame(mac1, mac2); + try (MacSupplier.ReusableMac mac2 = MacSupplier.HMAC_SHA256.keyed(key)) { + Assertions.assertNotNull(mac2); + } } @Test - public void testGetMacWithInvalidKey() throws InvalidKeyException, NoSuchAlgorithmException, ReflectiveOperationException { + public void testGetMacWithInvalidKey() throws NoSuchAlgorithmException, ReflectiveOperationException { Key key = KeyPairGenerator.getInstance("RSA").generateKeyPair().getPrivate(); // invoked via reflection, as we can not cast Key to SecretKey. - Method m = MacSupplier.class.getMethod("withKey", SecretKey.class); + Method m = MacSupplier.class.getMethod("keyed", SecretKey.class); Assertions.assertThrows(IllegalArgumentException.class, () -> { m.invoke(MacSupplier.HMAC_SHA256, key); }); From 3b9b00c0d41862be26748df5fcac259765b3200c Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 3 Dec 2021 14:44:38 +0100 Subject: [PATCH 05/22] use new MacSupplier API --- .../cryptolib/common/MasterkeyFileAccess.java | 7 ++++--- .../org/cryptomator/cryptolib/common/Scrypt.java | 8 ++++---- .../cryptolib/v1/FileContentCryptorImpl.java | 14 +++++++------- .../cryptolib/v1/FileHeaderCryptorImpl.java | 15 ++++++++------- 4 files changed, 23 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptolib/common/MasterkeyFileAccess.java b/src/main/java/org/cryptomator/cryptolib/common/MasterkeyFileAccess.java index 45e3e92..be920b9 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/MasterkeyFileAccess.java +++ b/src/main/java/org/cryptomator/cryptolib/common/MasterkeyFileAccess.java @@ -192,9 +192,10 @@ MasterkeyFile lock(Masterkey masterkey, CharSequence passphrase, int vaultVersio final byte[] salt = new byte[DEFAULT_SCRYPT_SALT_LENGTH]; csprng.nextBytes(salt); - try (DestroyableSecretKey kek = scrypt(passphrase, salt, pepper, scryptCostParam, DEFAULT_SCRYPT_BLOCK_SIZE)) { - final Mac mac = MacSupplier.HMAC_SHA256.withKey(masterkey.getMacKey()); - final byte[] versionMac = mac.doFinal(ByteBuffer.allocate(Integer.SIZE / Byte.SIZE).putInt(vaultVersion).array()); + try (DestroyableSecretKey kek = scrypt(passphrase, salt, pepper, scryptCostParam, DEFAULT_SCRYPT_BLOCK_SIZE); + DestroyableSecretKey macKey = masterkey.getMacKey(); + MacSupplier.ReusableMac mac = MacSupplier.HMAC_SHA256.keyed(macKey)) { + final byte[] versionMac = mac.get().doFinal(ByteBuffer.allocate(Integer.SIZE / Byte.SIZE).putInt(vaultVersion).array()); MasterkeyFile result = new MasterkeyFile(); result.version = vaultVersion; result.versionMac = versionMac; diff --git a/src/main/java/org/cryptomator/cryptolib/common/Scrypt.java b/src/main/java/org/cryptomator/cryptolib/common/Scrypt.java index 4d35aaf..09c0d44 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/Scrypt.java +++ b/src/main/java/org/cryptomator/cryptolib/common/Scrypt.java @@ -72,21 +72,21 @@ public static byte[] scrypt(byte[] passphrase, byte[] salt, int costParam, int b throw new IllegalArgumentException("Parameter r is too large"); } - try (DestroyableSecretKey key = new DestroyableSecretKey(passphrase, "HmacSHA256")) { - Mac mac = MacSupplier.HMAC_SHA256.withKey(key); + try (DestroyableSecretKey key = new DestroyableSecretKey(passphrase, "HmacSHA256"); + MacSupplier.ReusableMac mac = MacSupplier.HMAC_SHA256.keyed(key)) { byte[] DK = new byte[keyLengthInBytes]; byte[] B = new byte[128 * blockSize * P]; byte[] XY = new byte[256 * blockSize]; byte[] V = new byte[128 * blockSize * costParam]; - pbkdf2(mac, salt, 1, B, P * 128 * blockSize); + pbkdf2(mac.get(), salt, 1, B, P * 128 * blockSize); for (int i = 0; i < P; i++) { smix(B, i * 128 * blockSize, blockSize, costParam, V, XY); } - pbkdf2(mac, B, 1, DK, keyLengthInBytes); + pbkdf2(mac.get(), B, 1, DK, keyLengthInBytes); return DK; } diff --git a/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java index ee024ef..9154fd2 100644 --- a/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java @@ -184,14 +184,14 @@ boolean checkChunkMac(byte[] headerNonce, long chunkNumber, ByteBuffer chunkBuf) } private byte[] calcChunkMac(byte[] headerNonce, long chunkNumber, byte[] chunkNonce, ByteBuffer ciphertext) { - try (DestroyableSecretKey mk = masterkey.getMacKey()) { + try (DestroyableSecretKey mk = masterkey.getMacKey(); + MacSupplier.ReusableMac mac = MacSupplier.HMAC_SHA256.keyed(mk)) { final byte[] chunkNumberBigEndian = ByteBuffer.allocate(Long.SIZE / Byte.SIZE).order(ByteOrder.BIG_ENDIAN).putLong(chunkNumber).array(); - final Mac mac = MacSupplier.HMAC_SHA256.withKey(mk); - mac.update(headerNonce); - mac.update(chunkNumberBigEndian); - mac.update(chunkNonce); - mac.update(ciphertext); - return mac.doFinal(); + mac.get().update(headerNonce); + mac.get().update(chunkNumberBigEndian); + mac.get().update(chunkNonce); + mac.get().update(ciphertext); + return mac.get().doFinal(); } } diff --git a/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java index 6064188..6b01851 100644 --- a/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java @@ -68,9 +68,10 @@ public ByteBuffer encryptHeader(FileHeader header) { // mac nonce and ciphertext: ByteBuffer nonceAndCiphertextBuf = result.duplicate(); nonceAndCiphertextBuf.flip(); - Mac mac = MacSupplier.HMAC_SHA256.withKey(mk); - mac.update(nonceAndCiphertextBuf); - result.put(mac.doFinal()); + try (MacSupplier.ReusableMac mac = MacSupplier.HMAC_SHA256.keyed(mk)) { + mac.get().update(nonceAndCiphertextBuf); + result.put(mac.get().doFinal()); + } result.flip(); return result; @@ -100,12 +101,12 @@ public FileHeader decryptHeader(ByteBuffer ciphertextHeaderBuf) throws Authentic buf.get(expectedMac); // check mac: - try (DestroyableSecretKey mk = masterkey.getMacKey()) { + try (DestroyableSecretKey mk = masterkey.getMacKey(); + MacSupplier.ReusableMac mac = MacSupplier.HMAC_SHA256.keyed(mk)) { ByteBuffer nonceAndCiphertextBuf = buf.duplicate(); nonceAndCiphertextBuf.position(FileHeaderImpl.NONCE_POS).limit(FileHeaderImpl.NONCE_POS + FileHeaderImpl.NONCE_LEN + FileHeaderImpl.PAYLOAD_LEN); - Mac mac = MacSupplier.HMAC_SHA256.withKey(mk); - mac.update(nonceAndCiphertextBuf); - byte[] calculatedMac = mac.doFinal(); + mac.get().update(nonceAndCiphertextBuf); + byte[] calculatedMac = mac.get().doFinal(); if (!MessageDigest.isEqual(expectedMac, calculatedMac)) { throw new AuthenticationFailedException("Header MAC doesn't match."); } From a414d6680a4bd1d75adef1aadccf464d7353eecb Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 3 Dec 2021 15:00:06 +0100 Subject: [PATCH 06/22] use object pool in MessageDigestSupplier --- .../common/MessageDigestSupplier.java | 46 ++++++++++++++----- .../common/MessageDigestSupplierTest.java | 14 +++--- 2 files changed, 42 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptolib/common/MessageDigestSupplier.java b/src/main/java/org/cryptomator/cryptolib/common/MessageDigestSupplier.java index 12c3ca7..cb8f602 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/MessageDigestSupplier.java +++ b/src/main/java/org/cryptomator/cryptolib/common/MessageDigestSupplier.java @@ -17,28 +17,52 @@ public final class MessageDigestSupplier { public static final MessageDigestSupplier SHA256 = new MessageDigestSupplier("SHA-256"); private final String digestAlgorithm; - private final ThreadLocal threadLocal; + private final ObjectPool mdPool; public MessageDigestSupplier(String digestAlgorithm) { this.digestAlgorithm = digestAlgorithm; - this.threadLocal = new Provider(); + this.mdPool = new ObjectPool<>(this::createMessageDigest); + try (ObjectPool.Lease lease = mdPool.get()) { + lease.get(); // eagerly initialize to provoke exceptions + } } - private class Provider extends ThreadLocal { - @Override - protected MessageDigest initialValue() { - try { - return MessageDigest.getInstance(digestAlgorithm); - } catch (NoSuchAlgorithmException e) { - throw new IllegalArgumentException("Invalid digest algorithm.", e); - } + private MessageDigest createMessageDigest() { + try { + return MessageDigest.getInstance(digestAlgorithm); + } catch (NoSuchAlgorithmException e) { + throw new IllegalArgumentException("Invalid digest algorithm.", e); } } + public ReusableMessageDigest instance() { + ObjectPool.Lease lease = mdPool.get(); + lease.get().reset(); + return new ReusableMessageDigest(lease); + } + + @Deprecated public MessageDigest get() { - final MessageDigest result = threadLocal.get(); + final MessageDigest result = createMessageDigest(); result.reset(); return result; } + public static class ReusableMessageDigest implements AutoCloseable { + + private final ObjectPool.Lease lease; + + private ReusableMessageDigest(ObjectPool.Lease lease) { + this.lease = lease; + } + + public MessageDigest get() { + return lease.get(); + } + + @Override + public void close() { + lease.close(); + } + } } \ No newline at end of file diff --git a/src/test/java/org/cryptomator/cryptolib/common/MessageDigestSupplierTest.java b/src/test/java/org/cryptomator/cryptolib/common/MessageDigestSupplierTest.java index ad67c66..aab00fc 100644 --- a/src/test/java/org/cryptomator/cryptolib/common/MessageDigestSupplierTest.java +++ b/src/test/java/org/cryptomator/cryptolib/common/MessageDigestSupplierTest.java @@ -11,24 +11,24 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; -import java.security.MessageDigest; - public class MessageDigestSupplierTest { @Test public void testConstructorWithInvalidDigest() { Assertions.assertThrows(IllegalArgumentException.class, () -> { - new MessageDigestSupplier("FOO3000").get(); + new MessageDigestSupplier("FOO3000"); }); } @Test public void testGetSha1() { - MessageDigest digest1 = MessageDigestSupplier.SHA1.get(); - Assertions.assertNotNull(digest1); + try (MessageDigestSupplier.ReusableMessageDigest digest = MessageDigestSupplier.SHA1.instance()) { + Assertions.assertNotNull(digest); + } - MessageDigest digest2 = MessageDigestSupplier.SHA1.get(); - Assertions.assertSame(digest1, digest2); + try (MessageDigestSupplier.ReusableMessageDigest digest = MessageDigestSupplier.SHA1.instance()) { + Assertions.assertNotNull(digest); + } } } From ba9c1e2a447e701f89764d1735450980f24fed62 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 3 Dec 2021 15:00:29 +0100 Subject: [PATCH 07/22] use new MessageDigestSupplier API --- .../cryptolib/ecies/KeyDerivationFunction.java | 15 +++++++-------- .../cryptolib/v1/FileNameCryptorImpl.java | 5 +++-- .../cryptolib/v2/FileNameCryptorImpl.java | 5 +++-- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptolib/ecies/KeyDerivationFunction.java b/src/main/java/org/cryptomator/cryptolib/ecies/KeyDerivationFunction.java index f7a6b95..9a9c65e 100644 --- a/src/main/java/org/cryptomator/cryptolib/ecies/KeyDerivationFunction.java +++ b/src/main/java/org/cryptomator/cryptolib/ecies/KeyDerivationFunction.java @@ -6,7 +6,6 @@ import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.security.DigestException; -import java.security.MessageDigest; import java.util.Arrays; @FunctionalInterface @@ -32,8 +31,8 @@ public interface KeyDerivationFunction { * @return key data */ static byte[] ansiX963sha256Kdf(byte[] sharedSecret, byte[] sharedInfo, int keyDataLen) { - MessageDigest digest = MessageDigestSupplier.SHA256.get(); // max input length is 2^64 - 1, see https://doi.org/10.6028/NIST.SP.800-56Cr2, Table 1 - int hashLen = digest.getDigestLength(); + // max input length is 2^64 - 1, see https://doi.org/10.6028/NIST.SP.800-56Cr2, Table 1 + int hashLen = 32; // fixed digest length for SHA-256 in bytes // These two checks must be performed according to spec. However with 32 bit integers, we can't exceed any limits anyway: assert BigInteger.valueOf(4L + sharedSecret.length + sharedInfo.length).compareTo(BigInteger.ONE.shiftLeft(64).subtract(BigInteger.ONE)) < 0 : "input larger than hashmaxlen"; @@ -43,15 +42,15 @@ static byte[] ansiX963sha256Kdf(byte[] sharedSecret, byte[] sharedInfo, int keyD assert ByteOrder.BIG_ENDIAN.equals(counter.order()); int n = (keyDataLen + hashLen - 1) / hashLen; byte[] buffer = new byte[n * hashLen]; - try { + try (MessageDigestSupplier.ReusableMessageDigest sha256 = MessageDigestSupplier.SHA256.instance()) { for (int i = 0; i < n; i++) { - digest.update(sharedSecret); + sha256.get().update(sharedSecret); counter.clear(); counter.putInt(i + 1); counter.flip(); - digest.update(counter); - digest.update(sharedInfo); - digest.digest(buffer, i * hashLen, hashLen); + sha256.get().update(counter); + sha256.get().update(sharedInfo); + sha256.get().digest(buffer, i * hashLen, hashLen); } return Arrays.copyOf(buffer, keyDataLen); } catch (DigestException e) { diff --git a/src/main/java/org/cryptomator/cryptolib/v1/FileNameCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v1/FileNameCryptorImpl.java index ad99625..af6d16d 100644 --- a/src/main/java/org/cryptomator/cryptolib/v1/FileNameCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v1/FileNameCryptorImpl.java @@ -39,10 +39,11 @@ protected SivMode initialValue() { @Override public String hashDirectoryId(String cleartextDirectoryId) { - try (DestroyableSecretKey ek = masterkey.getEncKey(); DestroyableSecretKey mk = masterkey.getMacKey()) { + try (DestroyableSecretKey ek = masterkey.getEncKey(); DestroyableSecretKey mk = masterkey.getMacKey(); + MessageDigestSupplier.ReusableMessageDigest sha1 = MessageDigestSupplier.SHA1.instance()) { byte[] cleartextBytes = cleartextDirectoryId.getBytes(UTF_8); byte[] encryptedBytes = AES_SIV.get().encrypt(ek, mk, cleartextBytes); - byte[] hashedBytes = MessageDigestSupplier.SHA1.get().digest(encryptedBytes); + byte[] hashedBytes = sha1.get().digest(encryptedBytes); return BASE32.encode(hashedBytes); } } diff --git a/src/main/java/org/cryptomator/cryptolib/v2/FileNameCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v2/FileNameCryptorImpl.java index 9f14680..fe21abb 100644 --- a/src/main/java/org/cryptomator/cryptolib/v2/FileNameCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v2/FileNameCryptorImpl.java @@ -39,10 +39,11 @@ protected SivMode initialValue() { @Override public String hashDirectoryId(String cleartextDirectoryId) { - try (DestroyableSecretKey ek = masterkey.getEncKey(); DestroyableSecretKey mk = masterkey.getMacKey()) { + try (DestroyableSecretKey ek = masterkey.getEncKey(); DestroyableSecretKey mk = masterkey.getMacKey(); + MessageDigestSupplier.ReusableMessageDigest sha1 = MessageDigestSupplier.SHA1.instance()) { byte[] cleartextBytes = cleartextDirectoryId.getBytes(UTF_8); byte[] encryptedBytes = AES_SIV.get().encrypt(ek, mk, cleartextBytes); - byte[] hashedBytes = MessageDigestSupplier.SHA1.get().digest(encryptedBytes); + byte[] hashedBytes = sha1.get().digest(encryptedBytes); return BASE32.encode(hashedBytes); } } From 597d537f690f6fdfea7fb972c6f8fb74e5fc3191 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 3 Dec 2021 15:40:39 +0100 Subject: [PATCH 08/22] use correct method (despite being the same for CTR) --- .../org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java | 2 +- .../org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java index 9154fd2..6f05b14 100644 --- a/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java @@ -145,7 +145,7 @@ void decryptChunk(ByteBuffer ciphertextChunk, ByteBuffer cleartextChunk, Destroy payloadBuf.position(NONCE_SIZE).limit(ciphertextChunk.limit() - MAC_SIZE); // payload: - try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_CTR.encrypt(fk, new IvParameterSpec(nonce))) { + try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_CTR.decrypt(fk, new IvParameterSpec(nonce))) { assert cleartextChunk.remaining() >= cipher.get().getOutputSize(payloadBuf.remaining()); cipher.get().doFinal(payloadBuf, cleartextChunk); } diff --git a/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java index 6b01851..55efed4 100644 --- a/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java @@ -115,7 +115,7 @@ public FileHeader decryptHeader(ByteBuffer ciphertextHeaderBuf) throws Authentic ByteBuffer payloadCleartextBuf = ByteBuffer.allocate(FileHeaderImpl.Payload.SIZE); try (DestroyableSecretKey ek = masterkey.getEncKey()) { // decrypt payload: - try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_CTR.encrypt(ek, new IvParameterSpec(nonce))) { + try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_CTR.decrypt(ek, new IvParameterSpec(nonce))) { assert cipher.get().getOutputSize(ciphertextPayload.length) == payloadCleartextBuf.remaining(); int decrypted = cipher.get().doFinal(ByteBuffer.wrap(ciphertextPayload), payloadCleartextBuf); assert decrypted == FileHeaderImpl.Payload.SIZE; From d0141e3fdc19bdcbf1e1c1ad9d99a6f6a2048b4a Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Mon, 6 Dec 2021 09:20:08 +0100 Subject: [PATCH 09/22] add javadoc --- .../cryptolib/common/CipherSupplier.java | 56 +++++++++++++++++++ .../cryptolib/common/MacSupplier.java | 13 +++++ .../common/MessageDigestSupplier.java | 11 ++++ 3 files changed, 80 insertions(+) diff --git a/src/main/java/org/cryptomator/cryptolib/common/CipherSupplier.java b/src/main/java/org/cryptomator/cryptolib/common/CipherSupplier.java index 45e1875..0dc4ce9 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/CipherSupplier.java +++ b/src/main/java/org/cryptomator/cryptolib/common/CipherSupplier.java @@ -41,12 +41,27 @@ private Cipher createCipher() { } } + /** + * Leases a reusable cipher object initialized for encryption. + * + * @param key Encryption key + * @param params Params such as IV/Nonce + * @return A ReusableCipher instance holding a refurbished Cipher + */ public ReusableCipher encrypt(SecretKey key, AlgorithmParameterSpec params) { ObjectPool.Lease lease = cipherPool.get(); initMode(lease.get(), Cipher.ENCRYPT_MODE, key, params); return new ReusableCipher(lease); } + /** + * Creates a new Cipher object initialized for encryption. + * + * @param key Encryption key + * @param params Params such as IV/Nonce + * @return New Cipher instance + * @deprecated Use {@link #encrypt(SecretKey, AlgorithmParameterSpec)} instead. + */ @Deprecated public Cipher forEncryption(SecretKey key, AlgorithmParameterSpec params) { final Cipher cipher = createCipher(); @@ -54,12 +69,27 @@ public Cipher forEncryption(SecretKey key, AlgorithmParameterSpec params) { return cipher; } + /** + * Leases a reusable cipher object initialized for decryption. + * + * @param key Decryption key + * @param params Params such as IV/Nonce + * @return A ReusableCipher instance holding a refurbished Cipher + */ public ReusableCipher decrypt(SecretKey key, AlgorithmParameterSpec params) { ObjectPool.Lease lease = cipherPool.get(); initMode(lease.get(), Cipher.DECRYPT_MODE, key, params); return new ReusableCipher(lease); } + /** + * Creates a new Cipher object initialized for decryption. + * + * @param key Encryption key + * @param params Params such as IV/Nonce + * @return New Cipher instance + * @deprecated Use {@link #decrypt(SecretKey, AlgorithmParameterSpec)} instead. + */ @Deprecated public Cipher forDecryption(SecretKey key, AlgorithmParameterSpec params) { final Cipher cipher = createCipher(); @@ -67,12 +97,25 @@ public Cipher forDecryption(SecretKey key, AlgorithmParameterSpec params) { return cipher; } + /** + * Leases a reusable cipher object initialized for wrapping a key. + * + * @param kek Key encryption key + * @return A ReusableCipher instance holding a refurbished Cipher + */ public ReusableCipher wrap(SecretKey kek) { ObjectPool.Lease lease = cipherPool.get(); initMode(lease.get(), Cipher.WRAP_MODE, kek, null); return new ReusableCipher(lease); } + /** + * Creates a new Cipher object initialized for wrapping a key. + * + * @param kek Key encryption key + * @return New Cipher instance + * @deprecated Use {@link #wrap(SecretKey)} instead. + */ @Deprecated public Cipher forWrapping(SecretKey kek) { final Cipher cipher = createCipher(); @@ -80,12 +123,25 @@ public Cipher forWrapping(SecretKey kek) { return cipher; } + /** + * Leases a reusable cipher object initialized for unwrapping a key. + * + * @param kek Key encryption key + * @return A ReusableCipher instance holding a refurbished Cipher + */ public ReusableCipher unwrap(SecretKey kek) { ObjectPool.Lease lease = cipherPool.get(); initMode(lease.get(), Cipher.UNWRAP_MODE, kek, null); return new ReusableCipher(lease); } + /** + * Creates a new Cipher object initialized for unwrapping a key. + * + * @param kek Key encryption key + * @return New Cipher instance + * @deprecated Use {@link #unwrap(SecretKey)} instead. + */ @Deprecated public Cipher forUnwrapping(SecretKey kek) { final Cipher cipher = createCipher(); diff --git a/src/main/java/org/cryptomator/cryptolib/common/MacSupplier.java b/src/main/java/org/cryptomator/cryptolib/common/MacSupplier.java index 54615e9..c54d386 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/MacSupplier.java +++ b/src/main/java/org/cryptomator/cryptolib/common/MacSupplier.java @@ -37,12 +37,25 @@ private Mac createMac() { } } + /** + * Leases a reusable MAC object initialized with the given key. + * + * @param key Key to use in keyed MAC + * @return A ReusableMac instance holding a refurbished MAC + */ public ReusableMac keyed(SecretKey key) { ObjectPool.Lease lease = macPool.get(); init(lease.get(), key); return new ReusableMac(lease); } + /** + * Creates a new MAC + * + * @param key Key to use in keyed MAC + * @return New Mac instance + * @deprecated Use {@link #keyed(SecretKey)} instead + */ @Deprecated public Mac withKey(SecretKey key) { final Mac mac = createMac(); diff --git a/src/main/java/org/cryptomator/cryptolib/common/MessageDigestSupplier.java b/src/main/java/org/cryptomator/cryptolib/common/MessageDigestSupplier.java index cb8f602..6f82e8d 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/MessageDigestSupplier.java +++ b/src/main/java/org/cryptomator/cryptolib/common/MessageDigestSupplier.java @@ -35,12 +35,23 @@ private MessageDigest createMessageDigest() { } } + /** + * Leases a reusable MessageDigest. + * + * @return A ReusableMessageDigest instance holding a refurbished MessageDigest + */ public ReusableMessageDigest instance() { ObjectPool.Lease lease = mdPool.get(); lease.get().reset(); return new ReusableMessageDigest(lease); } + /** + * Creates a new MessageDigest. + * + * @deprecated Use {@link #instance()} + * @return New MessageDigest instance + */ @Deprecated public MessageDigest get() { final MessageDigest result = createMessageDigest(); From c532025ba94059dd51e5ba2ed8557f536dabf079 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Mon, 6 Dec 2021 09:20:22 +0100 Subject: [PATCH 10/22] remove unused imports --- src/main/java/org/cryptomator/cryptolib/common/MacSupplier.java | 1 - .../org/cryptomator/cryptolib/common/MasterkeyFileAccess.java | 1 - .../org/cryptomator/cryptolib/ecies/GcmWithSecretNonce.java | 2 -- .../org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java | 2 -- .../org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java | 1 - .../org/cryptomator/cryptolib/v2/FileContentCryptorImpl.java | 1 - .../org/cryptomator/cryptolib/v2/FileHeaderCryptorImpl.java | 1 - 7 files changed, 9 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptolib/common/MacSupplier.java b/src/main/java/org/cryptomator/cryptolib/common/MacSupplier.java index c54d386..fcd62c6 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/MacSupplier.java +++ b/src/main/java/org/cryptomator/cryptolib/common/MacSupplier.java @@ -8,7 +8,6 @@ *******************************************************************************/ package org.cryptomator.cryptolib.common; -import javax.crypto.Cipher; import javax.crypto.Mac; import javax.crypto.SecretKey; import java.security.InvalidKeyException; diff --git a/src/main/java/org/cryptomator/cryptolib/common/MasterkeyFileAccess.java b/src/main/java/org/cryptomator/cryptolib/common/MasterkeyFileAccess.java index be920b9..f3e902f 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/MasterkeyFileAccess.java +++ b/src/main/java/org/cryptomator/cryptolib/common/MasterkeyFileAccess.java @@ -5,7 +5,6 @@ import org.cryptomator.cryptolib.api.Masterkey; import org.cryptomator.cryptolib.api.MasterkeyLoadingFailedException; -import javax.crypto.Mac; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; diff --git a/src/main/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonce.java b/src/main/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonce.java index 3b053c1..9d7d9b1 100644 --- a/src/main/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonce.java +++ b/src/main/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonce.java @@ -6,10 +6,8 @@ import javax.crypto.AEADBadTagException; import javax.crypto.BadPaddingException; -import javax.crypto.Cipher; import javax.crypto.IllegalBlockSizeException; import javax.crypto.spec.GCMParameterSpec; -import javax.crypto.spec.IvParameterSpec; import java.util.Arrays; class GcmWithSecretNonce implements AuthenticatedEncryption { diff --git a/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java index 6f05b14..1f4d086 100644 --- a/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java @@ -17,9 +17,7 @@ import org.cryptomator.cryptolib.common.MacSupplier; import javax.crypto.BadPaddingException; -import javax.crypto.Cipher; import javax.crypto.IllegalBlockSizeException; -import javax.crypto.Mac; import javax.crypto.ShortBufferException; import javax.crypto.spec.IvParameterSpec; import java.nio.ByteBuffer; diff --git a/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java index 55efed4..e4df32b 100644 --- a/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java @@ -18,7 +18,6 @@ import javax.crypto.BadPaddingException; import javax.crypto.IllegalBlockSizeException; -import javax.crypto.Mac; import javax.crypto.ShortBufferException; import javax.crypto.spec.IvParameterSpec; import java.nio.ByteBuffer; diff --git a/src/main/java/org/cryptomator/cryptolib/v2/FileContentCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v2/FileContentCryptorImpl.java index 5e7673f..6d38e1a 100644 --- a/src/main/java/org/cryptomator/cryptolib/v2/FileContentCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v2/FileContentCryptorImpl.java @@ -16,7 +16,6 @@ import javax.crypto.AEADBadTagException; import javax.crypto.BadPaddingException; -import javax.crypto.Cipher; import javax.crypto.IllegalBlockSizeException; import javax.crypto.ShortBufferException; import javax.crypto.spec.GCMParameterSpec; diff --git a/src/main/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImpl.java index c2a5ae5..ae365e3 100644 --- a/src/main/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImpl.java @@ -17,7 +17,6 @@ import javax.crypto.AEADBadTagException; import javax.crypto.BadPaddingException; -import javax.crypto.Cipher; import javax.crypto.IllegalBlockSizeException; import javax.crypto.ShortBufferException; import javax.crypto.spec.GCMParameterSpec; From fb50af96143feca7d18ad2b1e2456645c19e5d97 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Mon, 6 Dec 2021 10:07:36 +0100 Subject: [PATCH 11/22] Removed redundant classes --- .../cryptolib/common/AesKeyWrap.java | 4 +- .../cryptolib/common/CipherSupplier.java | 51 +++++++------------ .../cryptolib/common/MacSupplier.java | 28 ++-------- .../cryptolib/common/MasterkeyFileAccess.java | 3 +- .../common/MessageDigestSupplier.java | 27 ++-------- .../cryptolib/common/ObjectPool.java | 27 ++++++---- .../cryptomator/cryptolib/common/Scrypt.java | 2 +- .../cryptolib/ecies/GcmWithSecretNonce.java | 6 ++- .../ecies/KeyDerivationFunction.java | 4 +- .../cryptolib/v1/FileContentCryptorImpl.java | 9 ++-- .../cryptolib/v1/FileHeaderCryptorImpl.java | 11 ++-- .../cryptolib/v1/FileNameCryptorImpl.java | 5 +- .../cryptolib/v2/FileContentCryptorImpl.java | 6 ++- .../cryptolib/v2/FileHeaderCryptorImpl.java | 6 ++- .../cryptolib/v2/FileNameCryptorImpl.java | 5 +- .../cryptolib/common/MacSupplierTest.java | 5 +- .../common/MessageDigestSupplierTest.java | 6 ++- .../cryptolib/common/ObjectPoolTest.java | 12 ++--- .../ecies/GcmWithSecretNonceTest.java | 4 +- .../v2/FileHeaderCryptorImplTest.java | 4 +- 20 files changed, 105 insertions(+), 120 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptolib/common/AesKeyWrap.java b/src/main/java/org/cryptomator/cryptolib/common/AesKeyWrap.java index 619dd49..e00480d 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/AesKeyWrap.java +++ b/src/main/java/org/cryptomator/cryptolib/common/AesKeyWrap.java @@ -23,7 +23,7 @@ public class AesKeyWrap { */ public static byte[] wrap(DestroyableSecretKey kek, SecretKey key) { try (DestroyableSecretKey kekCopy = kek.copy(); - CipherSupplier.ReusableCipher cipher = CipherSupplier.RFC3394_KEYWRAP.wrap(kekCopy)) { + ObjectPool.Lease cipher = CipherSupplier.RFC3394_KEYWRAP.wrap(kekCopy)) { return cipher.get().wrap(key); } catch (InvalidKeyException | IllegalBlockSizeException e) { throw new IllegalArgumentException("Unable to wrap key.", e); @@ -44,7 +44,7 @@ public static DestroyableSecretKey unwrap(DestroyableSecretKey kek, byte[] wrapp // visible for testing static DestroyableSecretKey unwrap(DestroyableSecretKey kek, byte[] wrappedKey, String wrappedKeyAlgorithm, int wrappedKeyType) throws InvalidKeyException { try (DestroyableSecretKey kekCopy = kek.copy(); - CipherSupplier.ReusableCipher cipher = CipherSupplier.RFC3394_KEYWRAP.unwrap(kekCopy)) { + ObjectPool.Lease cipher = CipherSupplier.RFC3394_KEYWRAP.unwrap(kekCopy)) { return DestroyableSecretKey.from(cipher.get().unwrap(wrappedKey, wrappedKeyAlgorithm, wrappedKeyType)); } catch (NoSuchAlgorithmException e) { throw new IllegalArgumentException("Invalid algorithm: " + wrappedKeyAlgorithm, e); diff --git a/src/main/java/org/cryptomator/cryptolib/common/CipherSupplier.java b/src/main/java/org/cryptomator/cryptolib/common/CipherSupplier.java index 0dc4ce9..b3af7d9 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/CipherSupplier.java +++ b/src/main/java/org/cryptomator/cryptolib/common/CipherSupplier.java @@ -28,7 +28,7 @@ public final class CipherSupplier { public CipherSupplier(String cipherAlgorithm) { this.cipherAlgorithm = cipherAlgorithm; this.cipherPool = new ObjectPool<>(this::createCipher); - try (ObjectPool.Lease lease = cipherPool.get()) { + try (ObjectPool.Lease lease = cipherPool.get()) { lease.get(); // eagerly initialize to provoke exceptions } } @@ -46,12 +46,12 @@ private Cipher createCipher() { * * @param key Encryption key * @param params Params such as IV/Nonce - * @return A ReusableCipher instance holding a refurbished Cipher + * @return A lease supplying a refurbished Cipher */ - public ReusableCipher encrypt(SecretKey key, AlgorithmParameterSpec params) { - ObjectPool.Lease lease = cipherPool.get(); + public ObjectPool.Lease encrypt(SecretKey key, AlgorithmParameterSpec params) { + ObjectPool.Lease lease = cipherPool.get(); initMode(lease.get(), Cipher.ENCRYPT_MODE, key, params); - return new ReusableCipher(lease); + return lease; } /** @@ -74,12 +74,12 @@ public Cipher forEncryption(SecretKey key, AlgorithmParameterSpec params) { * * @param key Decryption key * @param params Params such as IV/Nonce - * @return A ReusableCipher instance holding a refurbished Cipher + * @return A lease supplying a refurbished Cipher */ - public ReusableCipher decrypt(SecretKey key, AlgorithmParameterSpec params) { - ObjectPool.Lease lease = cipherPool.get(); + public ObjectPool.Lease decrypt(SecretKey key, AlgorithmParameterSpec params) { + ObjectPool.Lease lease = cipherPool.get(); initMode(lease.get(), Cipher.DECRYPT_MODE, key, params); - return new ReusableCipher(lease); + return lease; } /** @@ -101,12 +101,12 @@ public Cipher forDecryption(SecretKey key, AlgorithmParameterSpec params) { * Leases a reusable cipher object initialized for wrapping a key. * * @param kek Key encryption key - * @return A ReusableCipher instance holding a refurbished Cipher + * @return A lease supplying a refurbished Cipher */ - public ReusableCipher wrap(SecretKey kek) { - ObjectPool.Lease lease = cipherPool.get(); + public ObjectPool.Lease wrap(SecretKey kek) { + ObjectPool.Lease lease = cipherPool.get(); initMode(lease.get(), Cipher.WRAP_MODE, kek, null); - return new ReusableCipher(lease); + return lease; } /** @@ -127,12 +127,12 @@ public Cipher forWrapping(SecretKey kek) { * Leases a reusable cipher object initialized for unwrapping a key. * * @param kek Key encryption key - * @return A ReusableCipher instance holding a refurbished Cipher + * @return A lease supplying a refurbished Cipher */ - public ReusableCipher unwrap(SecretKey kek) { - ObjectPool.Lease lease = cipherPool.get(); + public ObjectPool.Lease unwrap(SecretKey kek) { + ObjectPool.Lease lease = cipherPool.get(); initMode(lease.get(), Cipher.UNWRAP_MODE, kek, null); - return new ReusableCipher(lease); + return lease; } /** @@ -159,21 +159,4 @@ private void initMode(Cipher cipher, int ciphermode, SecretKey key, AlgorithmPar } } - public static class ReusableCipher implements AutoCloseable { - - private final ObjectPool.Lease lease; - - private ReusableCipher(ObjectPool.Lease lease) { - this.lease = lease; - } - - public Cipher get() { - return lease.get(); - } - - @Override - public void close() { - lease.close(); - } - } } \ No newline at end of file diff --git a/src/main/java/org/cryptomator/cryptolib/common/MacSupplier.java b/src/main/java/org/cryptomator/cryptolib/common/MacSupplier.java index fcd62c6..6093489 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/MacSupplier.java +++ b/src/main/java/org/cryptomator/cryptolib/common/MacSupplier.java @@ -23,7 +23,7 @@ public final class MacSupplier { public MacSupplier(String macAlgorithm) { this.macAlgorithm = macAlgorithm; this.macPool = new ObjectPool<>(this::createMac); - try (ObjectPool.Lease lease = macPool.get()) { + try (ObjectPool.Lease lease = macPool.get()) { lease.get(); // eagerly initialize to provoke exceptions } } @@ -40,12 +40,12 @@ private Mac createMac() { * Leases a reusable MAC object initialized with the given key. * * @param key Key to use in keyed MAC - * @return A ReusableMac instance holding a refurbished MAC + * @return A lease supplying a refurbished MAC */ - public ReusableMac keyed(SecretKey key) { - ObjectPool.Lease lease = macPool.get(); + public ObjectPool.Lease keyed(SecretKey key) { + ObjectPool.Lease lease = macPool.get(); init(lease.get(), key); - return new ReusableMac(lease); + return lease; } /** @@ -70,22 +70,4 @@ private void init(Mac mac, SecretKey key) { } } - public static class ReusableMac implements AutoCloseable { - - private final ObjectPool.Lease lease; - - private ReusableMac(ObjectPool.Lease lease) { - this.lease = lease; - } - - public Mac get() { - return lease.get(); - } - - @Override - public void close() { - lease.close(); - } - } - } \ No newline at end of file diff --git a/src/main/java/org/cryptomator/cryptolib/common/MasterkeyFileAccess.java b/src/main/java/org/cryptomator/cryptolib/common/MasterkeyFileAccess.java index f3e902f..dc43d97 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/MasterkeyFileAccess.java +++ b/src/main/java/org/cryptomator/cryptolib/common/MasterkeyFileAccess.java @@ -5,6 +5,7 @@ import org.cryptomator.cryptolib.api.Masterkey; import org.cryptomator.cryptolib.api.MasterkeyLoadingFailedException; +import javax.crypto.Mac; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -193,7 +194,7 @@ MasterkeyFile lock(Masterkey masterkey, CharSequence passphrase, int vaultVersio csprng.nextBytes(salt); try (DestroyableSecretKey kek = scrypt(passphrase, salt, pepper, scryptCostParam, DEFAULT_SCRYPT_BLOCK_SIZE); DestroyableSecretKey macKey = masterkey.getMacKey(); - MacSupplier.ReusableMac mac = MacSupplier.HMAC_SHA256.keyed(macKey)) { + ObjectPool.Lease mac = MacSupplier.HMAC_SHA256.keyed(macKey)) { final byte[] versionMac = mac.get().doFinal(ByteBuffer.allocate(Integer.SIZE / Byte.SIZE).putInt(vaultVersion).array()); MasterkeyFile result = new MasterkeyFile(); result.version = vaultVersion; diff --git a/src/main/java/org/cryptomator/cryptolib/common/MessageDigestSupplier.java b/src/main/java/org/cryptomator/cryptolib/common/MessageDigestSupplier.java index 6f82e8d..5be3de5 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/MessageDigestSupplier.java +++ b/src/main/java/org/cryptomator/cryptolib/common/MessageDigestSupplier.java @@ -22,7 +22,7 @@ public final class MessageDigestSupplier { public MessageDigestSupplier(String digestAlgorithm) { this.digestAlgorithm = digestAlgorithm; this.mdPool = new ObjectPool<>(this::createMessageDigest); - try (ObjectPool.Lease lease = mdPool.get()) { + try (ObjectPool.Lease lease = mdPool.get()) { lease.get(); // eagerly initialize to provoke exceptions } } @@ -40,17 +40,17 @@ private MessageDigest createMessageDigest() { * * @return A ReusableMessageDigest instance holding a refurbished MessageDigest */ - public ReusableMessageDigest instance() { - ObjectPool.Lease lease = mdPool.get(); + public ObjectPool.Lease instance() { + ObjectPool.Lease lease = mdPool.get(); lease.get().reset(); - return new ReusableMessageDigest(lease); + return lease; } /** * Creates a new MessageDigest. * - * @deprecated Use {@link #instance()} * @return New MessageDigest instance + * @deprecated Use {@link #instance()} */ @Deprecated public MessageDigest get() { @@ -59,21 +59,4 @@ public MessageDigest get() { return result; } - public static class ReusableMessageDigest implements AutoCloseable { - - private final ObjectPool.Lease lease; - - private ReusableMessageDigest(ObjectPool.Lease lease) { - this.lease = lease; - } - - public MessageDigest get() { - return lease.get(); - } - - @Override - public void close() { - lease.close(); - } - } } \ No newline at end of file diff --git a/src/main/java/org/cryptomator/cryptolib/common/ObjectPool.java b/src/main/java/org/cryptomator/cryptolib/common/ObjectPool.java index 730a3f7..715f07d 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/ObjectPool.java +++ b/src/main/java/org/cryptomator/cryptolib/common/ObjectPool.java @@ -7,30 +7,39 @@ public class ObjectPool { - private final Queue> pool; + private final Queue> returnedInstances; private final Supplier factory; public ObjectPool(Supplier factory) { - this.pool = new ConcurrentLinkedQueue<>(); + this.returnedInstances = new ConcurrentLinkedQueue<>(); this.factory = factory; } - public Lease get() { + public Lease get() { WeakReference ref; - while ((ref = pool.poll()) != null) { + while ((ref = returnedInstances.poll()) != null) { T cached = ref.get(); if (cached != null) { - return new Lease(cached); + return new Lease<>(this, cached); } } - return new Lease(factory.get()); + return new Lease<>(this, factory.get()); } - public class Lease implements AutoCloseable { + /** + * A holder for resource leased from an {@link ObjectPool}. + * This is basically an {@link AutoCloseable autocloseable} {@link Supplier} that is intended to be used + * via try-with-resource blocks. + * + * @param Type of the leased instance + */ + public static class Lease implements AutoCloseable, Supplier { + private final ObjectPool pool; private T obj; - public Lease(T obj) { + private Lease(ObjectPool pool, T obj) { + this.pool = pool; this.obj = obj; } @@ -40,7 +49,7 @@ public T get() { @Override public void close() { - pool.offer(new WeakReference<>(obj)); + pool.returnedInstances.add(new WeakReference<>(obj)); obj = null; } } diff --git a/src/main/java/org/cryptomator/cryptolib/common/Scrypt.java b/src/main/java/org/cryptomator/cryptolib/common/Scrypt.java index 09c0d44..05134a0 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/Scrypt.java +++ b/src/main/java/org/cryptomator/cryptolib/common/Scrypt.java @@ -73,7 +73,7 @@ public static byte[] scrypt(byte[] passphrase, byte[] salt, int costParam, int b } try (DestroyableSecretKey key = new DestroyableSecretKey(passphrase, "HmacSHA256"); - MacSupplier.ReusableMac mac = MacSupplier.HMAC_SHA256.keyed(key)) { + ObjectPool.Lease mac = MacSupplier.HMAC_SHA256.keyed(key)) { byte[] DK = new byte[keyLengthInBytes]; byte[] B = new byte[128 * blockSize * P]; diff --git a/src/main/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonce.java b/src/main/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonce.java index 9d7d9b1..9cfc015 100644 --- a/src/main/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonce.java +++ b/src/main/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonce.java @@ -3,9 +3,11 @@ import com.google.common.base.Throwables; import org.cryptomator.cryptolib.common.CipherSupplier; import org.cryptomator.cryptolib.common.DestroyableSecretKey; +import org.cryptomator.cryptolib.common.ObjectPool; import javax.crypto.AEADBadTagException; import javax.crypto.BadPaddingException; +import javax.crypto.Cipher; import javax.crypto.IllegalBlockSizeException; import javax.crypto.spec.GCMParameterSpec; import java.util.Arrays; @@ -25,7 +27,7 @@ public int requiredSecretBytes() { public byte[] encrypt(byte[] secret, byte[] plaintext) { try (DestroyableSecretKey key = new DestroyableSecretKey(secret, 0, GCM_KEY_SIZE, "AES")) { byte[] nonce = Arrays.copyOfRange(secret, GCM_KEY_SIZE, GCM_KEY_SIZE + GCM_NONCE_SIZE); - try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_GCM.encrypt(key, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.encrypt(key, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { return cipher.get().doFinal(plaintext); } } catch (IllegalBlockSizeException | BadPaddingException e) { @@ -37,7 +39,7 @@ public byte[] encrypt(byte[] secret, byte[] plaintext) { public byte[] decrypt(byte[] secret, byte[] ciphertext) throws AEADBadTagException { try (DestroyableSecretKey key = new DestroyableSecretKey(secret, 0, GCM_KEY_SIZE, "AES")) { byte[] nonce = Arrays.copyOfRange(secret, GCM_KEY_SIZE, GCM_KEY_SIZE + GCM_NONCE_SIZE); - try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_GCM.decrypt(key, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.decrypt(key, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { return cipher.get().doFinal(ciphertext); } } catch (IllegalBlockSizeException | BadPaddingException e) { diff --git a/src/main/java/org/cryptomator/cryptolib/ecies/KeyDerivationFunction.java b/src/main/java/org/cryptomator/cryptolib/ecies/KeyDerivationFunction.java index 9a9c65e..0b8ecf0 100644 --- a/src/main/java/org/cryptomator/cryptolib/ecies/KeyDerivationFunction.java +++ b/src/main/java/org/cryptomator/cryptolib/ecies/KeyDerivationFunction.java @@ -1,11 +1,13 @@ package org.cryptomator.cryptolib.ecies; import org.cryptomator.cryptolib.common.MessageDigestSupplier; +import org.cryptomator.cryptolib.common.ObjectPool; import java.math.BigInteger; import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.security.DigestException; +import java.security.MessageDigest; import java.util.Arrays; @FunctionalInterface @@ -42,7 +44,7 @@ static byte[] ansiX963sha256Kdf(byte[] sharedSecret, byte[] sharedInfo, int keyD assert ByteOrder.BIG_ENDIAN.equals(counter.order()); int n = (keyDataLen + hashLen - 1) / hashLen; byte[] buffer = new byte[n * hashLen]; - try (MessageDigestSupplier.ReusableMessageDigest sha256 = MessageDigestSupplier.SHA256.instance()) { + try (ObjectPool.Lease sha256 = MessageDigestSupplier.SHA256.instance()) { for (int i = 0; i < n; i++) { sha256.get().update(sharedSecret); counter.clear(); diff --git a/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java index 1f4d086..807e686 100644 --- a/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java @@ -15,9 +15,12 @@ import org.cryptomator.cryptolib.common.CipherSupplier; import org.cryptomator.cryptolib.common.DestroyableSecretKey; import org.cryptomator.cryptolib.common.MacSupplier; +import org.cryptomator.cryptolib.common.ObjectPool; import javax.crypto.BadPaddingException; +import javax.crypto.Cipher; import javax.crypto.IllegalBlockSizeException; +import javax.crypto.Mac; import javax.crypto.ShortBufferException; import javax.crypto.spec.IvParameterSpec; import java.nio.ByteBuffer; @@ -109,7 +112,7 @@ void encryptChunk(ByteBuffer cleartextChunk, ByteBuffer ciphertextChunk, long ch // payload: int bytesEncrypted; - try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_CTR.encrypt(fk, new IvParameterSpec(nonce))) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_CTR.encrypt(fk, new IvParameterSpec(nonce))) { assert ciphertextChunk.remaining() >= cipher.get().getOutputSize(cleartextChunk.remaining()) + MAC_SIZE; bytesEncrypted = cipher.get().doFinal(cleartextChunk, ciphertextChunk); } @@ -143,7 +146,7 @@ void decryptChunk(ByteBuffer ciphertextChunk, ByteBuffer cleartextChunk, Destroy payloadBuf.position(NONCE_SIZE).limit(ciphertextChunk.limit() - MAC_SIZE); // payload: - try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_CTR.decrypt(fk, new IvParameterSpec(nonce))) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_CTR.decrypt(fk, new IvParameterSpec(nonce))) { assert cleartextChunk.remaining() >= cipher.get().getOutputSize(payloadBuf.remaining()); cipher.get().doFinal(payloadBuf, cleartextChunk); } @@ -183,7 +186,7 @@ boolean checkChunkMac(byte[] headerNonce, long chunkNumber, ByteBuffer chunkBuf) private byte[] calcChunkMac(byte[] headerNonce, long chunkNumber, byte[] chunkNonce, ByteBuffer ciphertext) { try (DestroyableSecretKey mk = masterkey.getMacKey(); - MacSupplier.ReusableMac mac = MacSupplier.HMAC_SHA256.keyed(mk)) { + ObjectPool.Lease mac = MacSupplier.HMAC_SHA256.keyed(mk)) { final byte[] chunkNumberBigEndian = ByteBuffer.allocate(Long.SIZE / Byte.SIZE).order(ByteOrder.BIG_ENDIAN).putLong(chunkNumber).array(); mac.get().update(headerNonce); mac.get().update(chunkNumberBigEndian); diff --git a/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java index e4df32b..d5b127b 100644 --- a/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java @@ -15,9 +15,12 @@ import org.cryptomator.cryptolib.common.CipherSupplier; import org.cryptomator.cryptolib.common.DestroyableSecretKey; import org.cryptomator.cryptolib.common.MacSupplier; +import org.cryptomator.cryptolib.common.ObjectPool; import javax.crypto.BadPaddingException; +import javax.crypto.Cipher; import javax.crypto.IllegalBlockSizeException; +import javax.crypto.Mac; import javax.crypto.ShortBufferException; import javax.crypto.spec.IvParameterSpec; import java.nio.ByteBuffer; @@ -59,7 +62,7 @@ public ByteBuffer encryptHeader(FileHeader header) { result.put(headerImpl.getNonce()); // encrypt payload: - try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_CTR.encrypt(ek, new IvParameterSpec(headerImpl.getNonce()))) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_CTR.encrypt(ek, new IvParameterSpec(headerImpl.getNonce()))) { int encrypted = cipher.get().doFinal(payloadCleartextBuf, result); assert encrypted == FileHeaderImpl.Payload.SIZE; } @@ -67,7 +70,7 @@ public ByteBuffer encryptHeader(FileHeader header) { // mac nonce and ciphertext: ByteBuffer nonceAndCiphertextBuf = result.duplicate(); nonceAndCiphertextBuf.flip(); - try (MacSupplier.ReusableMac mac = MacSupplier.HMAC_SHA256.keyed(mk)) { + try (ObjectPool.Lease mac = MacSupplier.HMAC_SHA256.keyed(mk)) { mac.get().update(nonceAndCiphertextBuf); result.put(mac.get().doFinal()); } @@ -101,7 +104,7 @@ public FileHeader decryptHeader(ByteBuffer ciphertextHeaderBuf) throws Authentic // check mac: try (DestroyableSecretKey mk = masterkey.getMacKey(); - MacSupplier.ReusableMac mac = MacSupplier.HMAC_SHA256.keyed(mk)) { + ObjectPool.Lease mac = MacSupplier.HMAC_SHA256.keyed(mk)) { ByteBuffer nonceAndCiphertextBuf = buf.duplicate(); nonceAndCiphertextBuf.position(FileHeaderImpl.NONCE_POS).limit(FileHeaderImpl.NONCE_POS + FileHeaderImpl.NONCE_LEN + FileHeaderImpl.PAYLOAD_LEN); mac.get().update(nonceAndCiphertextBuf); @@ -114,7 +117,7 @@ public FileHeader decryptHeader(ByteBuffer ciphertextHeaderBuf) throws Authentic ByteBuffer payloadCleartextBuf = ByteBuffer.allocate(FileHeaderImpl.Payload.SIZE); try (DestroyableSecretKey ek = masterkey.getEncKey()) { // decrypt payload: - try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_CTR.decrypt(ek, new IvParameterSpec(nonce))) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_CTR.decrypt(ek, new IvParameterSpec(nonce))) { assert cipher.get().getOutputSize(ciphertextPayload.length) == payloadCleartextBuf.remaining(); int decrypted = cipher.get().doFinal(ByteBuffer.wrap(ciphertextPayload), payloadCleartextBuf); assert decrypted == FileHeaderImpl.Payload.SIZE; diff --git a/src/main/java/org/cryptomator/cryptolib/v1/FileNameCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v1/FileNameCryptorImpl.java index af6d16d..309d1d6 100644 --- a/src/main/java/org/cryptomator/cryptolib/v1/FileNameCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v1/FileNameCryptorImpl.java @@ -14,11 +14,14 @@ import org.cryptomator.cryptolib.api.Masterkey; import org.cryptomator.cryptolib.common.DestroyableSecretKey; import org.cryptomator.cryptolib.common.MessageDigestSupplier; +import org.cryptomator.cryptolib.common.ObjectPool; import org.cryptomator.siv.SivMode; import org.cryptomator.siv.UnauthenticCiphertextException; import javax.crypto.IllegalBlockSizeException; +import java.security.MessageDigest; + import static java.nio.charset.StandardCharsets.UTF_8; class FileNameCryptorImpl implements FileNameCryptor { @@ -40,7 +43,7 @@ protected SivMode initialValue() { @Override public String hashDirectoryId(String cleartextDirectoryId) { try (DestroyableSecretKey ek = masterkey.getEncKey(); DestroyableSecretKey mk = masterkey.getMacKey(); - MessageDigestSupplier.ReusableMessageDigest sha1 = MessageDigestSupplier.SHA1.instance()) { + ObjectPool.Lease sha1 = MessageDigestSupplier.SHA1.instance()) { byte[] cleartextBytes = cleartextDirectoryId.getBytes(UTF_8); byte[] encryptedBytes = AES_SIV.get().encrypt(ek, mk, cleartextBytes); byte[] hashedBytes = sha1.get().digest(encryptedBytes); diff --git a/src/main/java/org/cryptomator/cryptolib/v2/FileContentCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v2/FileContentCryptorImpl.java index 6d38e1a..98bdf61 100644 --- a/src/main/java/org/cryptomator/cryptolib/v2/FileContentCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v2/FileContentCryptorImpl.java @@ -13,9 +13,11 @@ import org.cryptomator.cryptolib.api.FileHeader; import org.cryptomator.cryptolib.common.CipherSupplier; import org.cryptomator.cryptolib.common.DestroyableSecretKey; +import org.cryptomator.cryptolib.common.ObjectPool; import javax.crypto.AEADBadTagException; import javax.crypto.BadPaddingException; +import javax.crypto.Cipher; import javax.crypto.IllegalBlockSizeException; import javax.crypto.ShortBufferException; import javax.crypto.spec.GCMParameterSpec; @@ -102,7 +104,7 @@ void encryptChunk(ByteBuffer cleartextChunk, ByteBuffer ciphertextChunk, long ch random.nextBytes(nonce); // payload: - try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_GCM.encrypt(fk, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.encrypt(fk, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { final byte[] chunkNumberBigEndian = longToBigEndianByteArray(chunkNumber); cipher.get().updateAAD(chunkNumberBigEndian); cipher.get().updateAAD(headerNonce); @@ -134,7 +136,7 @@ void decryptChunk(ByteBuffer ciphertextChunk, ByteBuffer cleartextChunk, long ch assert payloadBuf.remaining() >= GCM_TAG_SIZE; // payload: - try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_GCM.decrypt(fk, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.decrypt(fk, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { final byte[] chunkNumberBigEndian = longToBigEndianByteArray(chunkNumber); cipher.get().updateAAD(chunkNumberBigEndian); cipher.get().updateAAD(headerNonce); diff --git a/src/main/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImpl.java index ae365e3..f9e13c4 100644 --- a/src/main/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImpl.java @@ -14,9 +14,11 @@ import org.cryptomator.cryptolib.api.Masterkey; import org.cryptomator.cryptolib.common.CipherSupplier; import org.cryptomator.cryptolib.common.DestroyableSecretKey; +import org.cryptomator.cryptolib.common.ObjectPool; import javax.crypto.AEADBadTagException; import javax.crypto.BadPaddingException; +import javax.crypto.Cipher; import javax.crypto.IllegalBlockSizeException; import javax.crypto.ShortBufferException; import javax.crypto.spec.GCMParameterSpec; @@ -60,7 +62,7 @@ public ByteBuffer encryptHeader(FileHeader header) { result.put(headerImpl.getNonce()); // encrypt payload: - try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_GCM.encrypt(ek, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, headerImpl.getNonce()))) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.encrypt(ek, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, headerImpl.getNonce()))) { int encrypted = cipher.get().doFinal(payloadCleartextBuf, result); assert encrypted == FileHeaderImpl.PAYLOAD_LEN + FileHeaderImpl.TAG_LEN; } @@ -92,7 +94,7 @@ public FileHeader decryptHeader(ByteBuffer ciphertextHeaderBuf) throws Authentic ByteBuffer payloadCleartextBuf = ByteBuffer.allocate(FileHeaderImpl.Payload.SIZE + GCM_TAG_SIZE); try (DestroyableSecretKey ek = masterkey.getEncKey()) { // decrypt payload: - try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_GCM.decrypt(ek, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.decrypt(ek, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { int decrypted = cipher.get().doFinal(ByteBuffer.wrap(ciphertextAndTag), payloadCleartextBuf); assert decrypted == FileHeaderImpl.Payload.SIZE; } diff --git a/src/main/java/org/cryptomator/cryptolib/v2/FileNameCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v2/FileNameCryptorImpl.java index fe21abb..1f4a573 100644 --- a/src/main/java/org/cryptomator/cryptolib/v2/FileNameCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v2/FileNameCryptorImpl.java @@ -14,11 +14,14 @@ import org.cryptomator.cryptolib.api.Masterkey; import org.cryptomator.cryptolib.common.DestroyableSecretKey; import org.cryptomator.cryptolib.common.MessageDigestSupplier; +import org.cryptomator.cryptolib.common.ObjectPool; import org.cryptomator.siv.SivMode; import org.cryptomator.siv.UnauthenticCiphertextException; import javax.crypto.IllegalBlockSizeException; +import java.security.MessageDigest; + import static java.nio.charset.StandardCharsets.UTF_8; class FileNameCryptorImpl implements FileNameCryptor { @@ -40,7 +43,7 @@ protected SivMode initialValue() { @Override public String hashDirectoryId(String cleartextDirectoryId) { try (DestroyableSecretKey ek = masterkey.getEncKey(); DestroyableSecretKey mk = masterkey.getMacKey(); - MessageDigestSupplier.ReusableMessageDigest sha1 = MessageDigestSupplier.SHA1.instance()) { + ObjectPool.Lease sha1 = MessageDigestSupplier.SHA1.instance()) { byte[] cleartextBytes = cleartextDirectoryId.getBytes(UTF_8); byte[] encryptedBytes = AES_SIV.get().encrypt(ek, mk, cleartextBytes); byte[] hashedBytes = sha1.get().digest(encryptedBytes); diff --git a/src/test/java/org/cryptomator/cryptolib/common/MacSupplierTest.java b/src/test/java/org/cryptomator/cryptolib/common/MacSupplierTest.java index 4aefda4..2155138 100644 --- a/src/test/java/org/cryptomator/cryptolib/common/MacSupplierTest.java +++ b/src/test/java/org/cryptomator/cryptolib/common/MacSupplierTest.java @@ -11,6 +11,7 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import javax.crypto.Mac; import javax.crypto.SecretKey; import javax.crypto.spec.SecretKeySpec; import java.lang.reflect.Method; @@ -30,11 +31,11 @@ public void testConstructorWithInvalidDigest() { @Test public void testGetMac() { SecretKey key = new SecretKeySpec(new byte[16], "HmacSHA256"); - try (MacSupplier.ReusableMac mac1 = MacSupplier.HMAC_SHA256.keyed(key)) { + try (ObjectPool.Lease mac1 = MacSupplier.HMAC_SHA256.keyed(key)) { Assertions.assertNotNull(mac1); } - try (MacSupplier.ReusableMac mac2 = MacSupplier.HMAC_SHA256.keyed(key)) { + try (ObjectPool.Lease mac2 = MacSupplier.HMAC_SHA256.keyed(key)) { Assertions.assertNotNull(mac2); } } diff --git a/src/test/java/org/cryptomator/cryptolib/common/MessageDigestSupplierTest.java b/src/test/java/org/cryptomator/cryptolib/common/MessageDigestSupplierTest.java index aab00fc..6c4d290 100644 --- a/src/test/java/org/cryptomator/cryptolib/common/MessageDigestSupplierTest.java +++ b/src/test/java/org/cryptomator/cryptolib/common/MessageDigestSupplierTest.java @@ -11,6 +11,8 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import java.security.MessageDigest; + public class MessageDigestSupplierTest { @Test @@ -22,11 +24,11 @@ public void testConstructorWithInvalidDigest() { @Test public void testGetSha1() { - try (MessageDigestSupplier.ReusableMessageDigest digest = MessageDigestSupplier.SHA1.instance()) { + try (ObjectPool.Lease digest = MessageDigestSupplier.SHA1.instance()) { Assertions.assertNotNull(digest); } - try (MessageDigestSupplier.ReusableMessageDigest digest = MessageDigestSupplier.SHA1.instance()) { + try (ObjectPool.Lease digest = MessageDigestSupplier.SHA1.instance()) { Assertions.assertNotNull(digest); } } diff --git a/src/test/java/org/cryptomator/cryptolib/common/ObjectPoolTest.java b/src/test/java/org/cryptomator/cryptolib/common/ObjectPoolTest.java index 2838d76..a435c7d 100644 --- a/src/test/java/org/cryptomator/cryptolib/common/ObjectPoolTest.java +++ b/src/test/java/org/cryptomator/cryptolib/common/ObjectPoolTest.java @@ -21,8 +21,8 @@ public void setup() { @Test @DisplayName("new instance is created if pool is empty") public void testCreateNewObjWhenPoolIsEmpty() { - try (ObjectPool.Lease lease1 = pool.get()) { - try (ObjectPool.Lease lease2 = pool.get()) { + try (ObjectPool.Lease lease1 = pool.get()) { + try (ObjectPool.Lease lease2 = pool.get()) { Assertions.assertNotSame(lease1.get(), lease2.get()); } } @@ -33,10 +33,10 @@ public void testCreateNewObjWhenPoolIsEmpty() { @DisplayName("recycle existing instance") public void testRecycleExistingObj() { Foo foo1; - try (ObjectPool.Lease lease = pool.get()) { + try (ObjectPool.Lease lease = pool.get()) { foo1 = lease.get(); } - try (ObjectPool.Lease lease = pool.get()) { + try (ObjectPool.Lease lease = pool.get()) { Assertions.assertSame(foo1, lease.get()); } Mockito.verify(factory, Mockito.times(1)).get(); @@ -45,11 +45,11 @@ public void testRecycleExistingObj() { @Test @DisplayName("create new instance when pool is GC'ed") public void testGc() { - try (ObjectPool.Lease lease = pool.get()) { + try (ObjectPool.Lease lease = pool.get()) { Assertions.assertNotNull(lease.get()); } System.gc(); // seems to be reliable on Temurin 17 with @RepeatedTest(1000) - try (ObjectPool.Lease lease = pool.get()) { + try (ObjectPool.Lease lease = pool.get()) { Assertions.assertNotNull(lease.get()); } Mockito.verify(factory, Mockito.times(2)).get(); diff --git a/src/test/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonceTest.java b/src/test/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonceTest.java index 6873e2b..d9fcbc9 100644 --- a/src/test/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonceTest.java +++ b/src/test/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonceTest.java @@ -2,6 +2,7 @@ import org.cryptomator.cryptolib.common.CipherSupplier; import org.cryptomator.cryptolib.common.GcmTestHelper; +import org.cryptomator.cryptolib.common.ObjectPool; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -10,6 +11,7 @@ import org.junit.jupiter.params.provider.CsvSource; import javax.crypto.AEADBadTagException; +import javax.crypto.Cipher; import javax.crypto.spec.GCMParameterSpec; /** @@ -23,7 +25,7 @@ public class GcmWithSecretNonceTest { public void setup() { // reset cipher state to avoid InvalidAlgorithmParameterExceptions due to IV-reuse GcmTestHelper.reset((mode, key, params) -> { - try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_GCM.encrypt(key, params)) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.encrypt(key, params)) { cipher.get(); } }); diff --git a/src/test/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImplTest.java b/src/test/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImplTest.java index fd4796c..25ce394 100644 --- a/src/test/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImplTest.java +++ b/src/test/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImplTest.java @@ -14,11 +14,13 @@ import org.cryptomator.cryptolib.api.Masterkey; import org.cryptomator.cryptolib.common.CipherSupplier; import org.cryptomator.cryptolib.common.GcmTestHelper; +import org.cryptomator.cryptolib.common.ObjectPool; import org.cryptomator.cryptolib.common.SecureRandomMock; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import javax.crypto.Cipher; import javax.crypto.spec.GCMParameterSpec; import java.nio.ByteBuffer; import java.security.SecureRandom; @@ -38,7 +40,7 @@ public void setup() { // reset cipher state to avoid InvalidAlgorithmParameterExceptions due to IV-reuse GcmTestHelper.reset((mode, key, params) -> { - try (CipherSupplier.ReusableCipher cipher = CipherSupplier.AES_GCM.encrypt(key, params)) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.encrypt(key, params)) { cipher.get(); } }); From 0c2871f83799eb1182ce21ac3f152c2a03a71660 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Mon, 6 Dec 2021 10:18:27 +0100 Subject: [PATCH 12/22] added more javadoc --- .../cryptomator/cryptolib/common/ObjectPool.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/main/java/org/cryptomator/cryptolib/common/ObjectPool.java b/src/main/java/org/cryptomator/cryptolib/common/ObjectPool.java index 715f07d..00de730 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/ObjectPool.java +++ b/src/main/java/org/cryptomator/cryptolib/common/ObjectPool.java @@ -5,6 +5,20 @@ import java.util.concurrent.ConcurrentLinkedQueue; import java.util.function.Supplier; +/** + * A simple object pool for resources that are expensive to create but are needed frequently. + *

+ * Example Usage: + * 

{@code
+ *     Supplier fooFactory = () -> new Foo();
+ *     ObjectPool fooPool = new ObjectPool(fooFactory);
+ *     try (ObjectPool.Lease lease = fooPool.get()) { // attempts to get a pooled Foo or invokes factory
+ *         lease.get().foo(); // exclusively use Foo instance
+ *     } // releases instance back to the pool when done
+ * }
+ * + * @param Type of the pooled objects + */ public class ObjectPool { private final Queue> returnedInstances; From 5bffe87e1116859c852ee221d876bf944d43d50a Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Mon, 6 Dec 2021 10:25:08 +0100 Subject: [PATCH 13/22] renamed methods in CipherSupplier --- .../cryptomator/cryptolib/common/AesKeyWrap.java | 4 ++-- .../cryptolib/common/CipherSupplier.java | 16 ++++++++-------- .../cryptolib/ecies/GcmWithSecretNonce.java | 4 ++-- .../cryptolib/v1/FileContentCryptorImpl.java | 4 ++-- .../cryptolib/v1/FileHeaderCryptorImpl.java | 4 ++-- .../cryptolib/v2/FileContentCryptorImpl.java | 4 ++-- .../cryptolib/v2/FileHeaderCryptorImpl.java | 4 ++-- .../cryptolib/common/CipherSupplierTest.java | 6 ++---- .../cryptolib/ecies/GcmWithSecretNonceTest.java | 3 +-- .../cryptolib/v2/FileHeaderCryptorImplTest.java | 5 +---- 10 files changed, 24 insertions(+), 30 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptolib/common/AesKeyWrap.java b/src/main/java/org/cryptomator/cryptolib/common/AesKeyWrap.java index e00480d..5b603e7 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/AesKeyWrap.java +++ b/src/main/java/org/cryptomator/cryptolib/common/AesKeyWrap.java @@ -23,7 +23,7 @@ public class AesKeyWrap { */ public static byte[] wrap(DestroyableSecretKey kek, SecretKey key) { try (DestroyableSecretKey kekCopy = kek.copy(); - ObjectPool.Lease cipher = CipherSupplier.RFC3394_KEYWRAP.wrap(kekCopy)) { + ObjectPool.Lease cipher = CipherSupplier.RFC3394_KEYWRAP.keyWrapCipher(kekCopy)) { return cipher.get().wrap(key); } catch (InvalidKeyException | IllegalBlockSizeException e) { throw new IllegalArgumentException("Unable to wrap key.", e); @@ -44,7 +44,7 @@ public static DestroyableSecretKey unwrap(DestroyableSecretKey kek, byte[] wrapp // visible for testing static DestroyableSecretKey unwrap(DestroyableSecretKey kek, byte[] wrappedKey, String wrappedKeyAlgorithm, int wrappedKeyType) throws InvalidKeyException { try (DestroyableSecretKey kekCopy = kek.copy(); - ObjectPool.Lease cipher = CipherSupplier.RFC3394_KEYWRAP.unwrap(kekCopy)) { + ObjectPool.Lease cipher = CipherSupplier.RFC3394_KEYWRAP.keyUnwrapCipher(kekCopy)) { return DestroyableSecretKey.from(cipher.get().unwrap(wrappedKey, wrappedKeyAlgorithm, wrappedKeyType)); } catch (NoSuchAlgorithmException e) { throw new IllegalArgumentException("Invalid algorithm: " + wrappedKeyAlgorithm, e); diff --git a/src/main/java/org/cryptomator/cryptolib/common/CipherSupplier.java b/src/main/java/org/cryptomator/cryptolib/common/CipherSupplier.java index b3af7d9..36c3b22 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/CipherSupplier.java +++ b/src/main/java/org/cryptomator/cryptolib/common/CipherSupplier.java @@ -48,7 +48,7 @@ private Cipher createCipher() { * @param params Params such as IV/Nonce * @return A lease supplying a refurbished Cipher */ - public ObjectPool.Lease encrypt(SecretKey key, AlgorithmParameterSpec params) { + public ObjectPool.Lease encryptionCipher(SecretKey key, AlgorithmParameterSpec params) { ObjectPool.Lease lease = cipherPool.get(); initMode(lease.get(), Cipher.ENCRYPT_MODE, key, params); return lease; @@ -60,7 +60,7 @@ public ObjectPool.Lease encrypt(SecretKey key, AlgorithmParameterSpec pa * @param key Encryption key * @param params Params such as IV/Nonce * @return New Cipher instance - * @deprecated Use {@link #encrypt(SecretKey, AlgorithmParameterSpec)} instead. + * @deprecated Use {@link #encryptionCipher(SecretKey, AlgorithmParameterSpec)} instead. */ @Deprecated public Cipher forEncryption(SecretKey key, AlgorithmParameterSpec params) { @@ -76,7 +76,7 @@ public Cipher forEncryption(SecretKey key, AlgorithmParameterSpec params) { * @param params Params such as IV/Nonce * @return A lease supplying a refurbished Cipher */ - public ObjectPool.Lease decrypt(SecretKey key, AlgorithmParameterSpec params) { + public ObjectPool.Lease decryptionCipher(SecretKey key, AlgorithmParameterSpec params) { ObjectPool.Lease lease = cipherPool.get(); initMode(lease.get(), Cipher.DECRYPT_MODE, key, params); return lease; @@ -88,7 +88,7 @@ public ObjectPool.Lease decrypt(SecretKey key, AlgorithmParameterSpec pa * @param key Encryption key * @param params Params such as IV/Nonce * @return New Cipher instance - * @deprecated Use {@link #decrypt(SecretKey, AlgorithmParameterSpec)} instead. + * @deprecated Use {@link #decryptionCipher(SecretKey, AlgorithmParameterSpec)} instead. */ @Deprecated public Cipher forDecryption(SecretKey key, AlgorithmParameterSpec params) { @@ -103,7 +103,7 @@ public Cipher forDecryption(SecretKey key, AlgorithmParameterSpec params) { * @param kek Key encryption key * @return A lease supplying a refurbished Cipher */ - public ObjectPool.Lease wrap(SecretKey kek) { + public ObjectPool.Lease keyWrapCipher(SecretKey kek) { ObjectPool.Lease lease = cipherPool.get(); initMode(lease.get(), Cipher.WRAP_MODE, kek, null); return lease; @@ -114,7 +114,7 @@ public ObjectPool.Lease wrap(SecretKey kek) { * * @param kek Key encryption key * @return New Cipher instance - * @deprecated Use {@link #wrap(SecretKey)} instead. + * @deprecated Use {@link #keyWrapCipher(SecretKey)} instead. */ @Deprecated public Cipher forWrapping(SecretKey kek) { @@ -129,7 +129,7 @@ public Cipher forWrapping(SecretKey kek) { * @param kek Key encryption key * @return A lease supplying a refurbished Cipher */ - public ObjectPool.Lease unwrap(SecretKey kek) { + public ObjectPool.Lease keyUnwrapCipher(SecretKey kek) { ObjectPool.Lease lease = cipherPool.get(); initMode(lease.get(), Cipher.UNWRAP_MODE, kek, null); return lease; @@ -140,7 +140,7 @@ public ObjectPool.Lease unwrap(SecretKey kek) { * * @param kek Key encryption key * @return New Cipher instance - * @deprecated Use {@link #unwrap(SecretKey)} instead. + * @deprecated Use {@link #keyUnwrapCipher(SecretKey)} instead. */ @Deprecated public Cipher forUnwrapping(SecretKey kek) { diff --git a/src/main/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonce.java b/src/main/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonce.java index 9cfc015..093af59 100644 --- a/src/main/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonce.java +++ b/src/main/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonce.java @@ -27,7 +27,7 @@ public int requiredSecretBytes() { public byte[] encrypt(byte[] secret, byte[] plaintext) { try (DestroyableSecretKey key = new DestroyableSecretKey(secret, 0, GCM_KEY_SIZE, "AES")) { byte[] nonce = Arrays.copyOfRange(secret, GCM_KEY_SIZE, GCM_KEY_SIZE + GCM_NONCE_SIZE); - try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.encrypt(key, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.encryptionCipher(key, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { return cipher.get().doFinal(plaintext); } } catch (IllegalBlockSizeException | BadPaddingException e) { @@ -39,7 +39,7 @@ public byte[] encrypt(byte[] secret, byte[] plaintext) { public byte[] decrypt(byte[] secret, byte[] ciphertext) throws AEADBadTagException { try (DestroyableSecretKey key = new DestroyableSecretKey(secret, 0, GCM_KEY_SIZE, "AES")) { byte[] nonce = Arrays.copyOfRange(secret, GCM_KEY_SIZE, GCM_KEY_SIZE + GCM_NONCE_SIZE); - try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.decrypt(key, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.decryptionCipher(key, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { return cipher.get().doFinal(ciphertext); } } catch (IllegalBlockSizeException | BadPaddingException e) { diff --git a/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java index 807e686..74b62c6 100644 --- a/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java @@ -112,7 +112,7 @@ void encryptChunk(ByteBuffer cleartextChunk, ByteBuffer ciphertextChunk, long ch // payload: int bytesEncrypted; - try (ObjectPool.Lease cipher = CipherSupplier.AES_CTR.encrypt(fk, new IvParameterSpec(nonce))) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_CTR.encryptionCipher(fk, new IvParameterSpec(nonce))) { assert ciphertextChunk.remaining() >= cipher.get().getOutputSize(cleartextChunk.remaining()) + MAC_SIZE; bytesEncrypted = cipher.get().doFinal(cleartextChunk, ciphertextChunk); } @@ -146,7 +146,7 @@ void decryptChunk(ByteBuffer ciphertextChunk, ByteBuffer cleartextChunk, Destroy payloadBuf.position(NONCE_SIZE).limit(ciphertextChunk.limit() - MAC_SIZE); // payload: - try (ObjectPool.Lease cipher = CipherSupplier.AES_CTR.decrypt(fk, new IvParameterSpec(nonce))) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_CTR.decryptionCipher(fk, new IvParameterSpec(nonce))) { assert cleartextChunk.remaining() >= cipher.get().getOutputSize(payloadBuf.remaining()); cipher.get().doFinal(payloadBuf, cleartextChunk); } diff --git a/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java index d5b127b..6721814 100644 --- a/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java @@ -62,7 +62,7 @@ public ByteBuffer encryptHeader(FileHeader header) { result.put(headerImpl.getNonce()); // encrypt payload: - try (ObjectPool.Lease cipher = CipherSupplier.AES_CTR.encrypt(ek, new IvParameterSpec(headerImpl.getNonce()))) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_CTR.encryptionCipher(ek, new IvParameterSpec(headerImpl.getNonce()))) { int encrypted = cipher.get().doFinal(payloadCleartextBuf, result); assert encrypted == FileHeaderImpl.Payload.SIZE; } @@ -117,7 +117,7 @@ public FileHeader decryptHeader(ByteBuffer ciphertextHeaderBuf) throws Authentic ByteBuffer payloadCleartextBuf = ByteBuffer.allocate(FileHeaderImpl.Payload.SIZE); try (DestroyableSecretKey ek = masterkey.getEncKey()) { // decrypt payload: - try (ObjectPool.Lease cipher = CipherSupplier.AES_CTR.decrypt(ek, new IvParameterSpec(nonce))) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_CTR.decryptionCipher(ek, new IvParameterSpec(nonce))) { assert cipher.get().getOutputSize(ciphertextPayload.length) == payloadCleartextBuf.remaining(); int decrypted = cipher.get().doFinal(ByteBuffer.wrap(ciphertextPayload), payloadCleartextBuf); assert decrypted == FileHeaderImpl.Payload.SIZE; diff --git a/src/main/java/org/cryptomator/cryptolib/v2/FileContentCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v2/FileContentCryptorImpl.java index 98bdf61..4abaf68 100644 --- a/src/main/java/org/cryptomator/cryptolib/v2/FileContentCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v2/FileContentCryptorImpl.java @@ -104,7 +104,7 @@ void encryptChunk(ByteBuffer cleartextChunk, ByteBuffer ciphertextChunk, long ch random.nextBytes(nonce); // payload: - try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.encrypt(fk, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.encryptionCipher(fk, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { final byte[] chunkNumberBigEndian = longToBigEndianByteArray(chunkNumber); cipher.get().updateAAD(chunkNumberBigEndian); cipher.get().updateAAD(headerNonce); @@ -136,7 +136,7 @@ void decryptChunk(ByteBuffer ciphertextChunk, ByteBuffer cleartextChunk, long ch assert payloadBuf.remaining() >= GCM_TAG_SIZE; // payload: - try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.decrypt(fk, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.decryptionCipher(fk, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { final byte[] chunkNumberBigEndian = longToBigEndianByteArray(chunkNumber); cipher.get().updateAAD(chunkNumberBigEndian); cipher.get().updateAAD(headerNonce); diff --git a/src/main/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImpl.java index f9e13c4..ac294bf 100644 --- a/src/main/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImpl.java @@ -62,7 +62,7 @@ public ByteBuffer encryptHeader(FileHeader header) { result.put(headerImpl.getNonce()); // encrypt payload: - try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.encrypt(ek, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, headerImpl.getNonce()))) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.encryptionCipher(ek, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, headerImpl.getNonce()))) { int encrypted = cipher.get().doFinal(payloadCleartextBuf, result); assert encrypted == FileHeaderImpl.PAYLOAD_LEN + FileHeaderImpl.TAG_LEN; } @@ -94,7 +94,7 @@ public FileHeader decryptHeader(ByteBuffer ciphertextHeaderBuf) throws Authentic ByteBuffer payloadCleartextBuf = ByteBuffer.allocate(FileHeaderImpl.Payload.SIZE + GCM_TAG_SIZE); try (DestroyableSecretKey ek = masterkey.getEncKey()) { // decrypt payload: - try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.decrypt(ek, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.decryptionCipher(ek, new GCMParameterSpec(GCM_TAG_SIZE * Byte.SIZE, nonce))) { int decrypted = cipher.get().doFinal(ByteBuffer.wrap(ciphertextAndTag), payloadCleartextBuf); assert decrypted == FileHeaderImpl.Payload.SIZE; } diff --git a/src/test/java/org/cryptomator/cryptolib/common/CipherSupplierTest.java b/src/test/java/org/cryptomator/cryptolib/common/CipherSupplierTest.java index 0c62435..777ef95 100644 --- a/src/test/java/org/cryptomator/cryptolib/common/CipherSupplierTest.java +++ b/src/test/java/org/cryptomator/cryptolib/common/CipherSupplierTest.java @@ -13,10 +13,8 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; -import javax.crypto.Cipher; import javax.crypto.spec.IvParameterSpec; import javax.crypto.spec.RC5ParameterSpec; -import javax.crypto.spec.SecretKeySpec; public class CipherSupplierTest { @@ -31,7 +29,7 @@ public void testGetUnknownCipher() { public void testGetCipherWithInvalidKey() { CipherSupplier supplier = new CipherSupplier("AES/CBC/PKCS5Padding"); IllegalArgumentException exception = Assertions.assertThrows(IllegalArgumentException.class, () -> { - supplier.encrypt(new DestroyableSecretKey(new byte[13], "AES"), new IvParameterSpec(new byte[16])); + supplier.encryptionCipher(new DestroyableSecretKey(new byte[13], "AES"), new IvParameterSpec(new byte[16])); }); MatcherAssert.assertThat(exception.getMessage(), CoreMatchers.containsString("Invalid key")); } @@ -40,7 +38,7 @@ public void testGetCipherWithInvalidKey() { public void testGetCipherWithInvalidAlgorithmParam() { CipherSupplier supplier = new CipherSupplier("AES/CBC/PKCS5Padding"); IllegalArgumentException exception = Assertions.assertThrows(IllegalArgumentException.class, () -> { - supplier.encrypt(new DestroyableSecretKey(new byte[16], "AES"), new RC5ParameterSpec(1, 1, 8)); + supplier.encryptionCipher(new DestroyableSecretKey(new byte[16], "AES"), new RC5ParameterSpec(1, 1, 8)); }); MatcherAssert.assertThat(exception.getMessage(), CoreMatchers.containsString("Algorithm parameter not appropriate for")); } diff --git a/src/test/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonceTest.java b/src/test/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonceTest.java index d9fcbc9..48cb923 100644 --- a/src/test/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonceTest.java +++ b/src/test/java/org/cryptomator/cryptolib/ecies/GcmWithSecretNonceTest.java @@ -12,7 +12,6 @@ import javax.crypto.AEADBadTagException; import javax.crypto.Cipher; -import javax.crypto.spec.GCMParameterSpec; /** * Test vectors from https://csrc.nist.rip/groups/ST/toolkit/BCM/documents/proposedmodes/gcm/gcm-spec.pdf @@ -25,7 +24,7 @@ public class GcmWithSecretNonceTest { public void setup() { // reset cipher state to avoid InvalidAlgorithmParameterExceptions due to IV-reuse GcmTestHelper.reset((mode, key, params) -> { - try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.encrypt(key, params)) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.encryptionCipher(key, params)) { cipher.get(); } }); diff --git a/src/test/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImplTest.java b/src/test/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImplTest.java index 25ce394..c9e1c3b 100644 --- a/src/test/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImplTest.java +++ b/src/test/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImplTest.java @@ -21,12 +21,9 @@ import org.junit.jupiter.api.Test; import javax.crypto.Cipher; -import javax.crypto.spec.GCMParameterSpec; import java.nio.ByteBuffer; import java.security.SecureRandom; -import static org.cryptomator.cryptolib.v2.Constants.GCM_TAG_SIZE; - public class FileHeaderCryptorImplTest { private static final SecureRandom RANDOM_MOCK = SecureRandomMock.NULL_RANDOM; @@ -40,7 +37,7 @@ public void setup() { // reset cipher state to avoid InvalidAlgorithmParameterExceptions due to IV-reuse GcmTestHelper.reset((mode, key, params) -> { - try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.encrypt(key, params)) { + try (ObjectPool.Lease cipher = CipherSupplier.AES_GCM.encryptionCipher(key, params)) { cipher.get(); } }); From a0954d48548da1701aa3d37dde1bb200271636ce Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Mon, 6 Dec 2021 12:37:13 +0100 Subject: [PATCH 14/22] Add benchmark for pooled suppliers --- .../common/PooledSuppliersBenchmarkTest.java | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 src/test/java/org/cryptomator/cryptolib/common/PooledSuppliersBenchmarkTest.java diff --git a/src/test/java/org/cryptomator/cryptolib/common/PooledSuppliersBenchmarkTest.java b/src/test/java/org/cryptomator/cryptolib/common/PooledSuppliersBenchmarkTest.java new file mode 100644 index 0000000..471515a --- /dev/null +++ b/src/test/java/org/cryptomator/cryptolib/common/PooledSuppliersBenchmarkTest.java @@ -0,0 +1,99 @@ +package org.cryptomator.cryptolib.common; + +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; +import org.openjdk.jmh.runner.Runner; +import org.openjdk.jmh.runner.RunnerException; +import org.openjdk.jmh.runner.options.Options; +import org.openjdk.jmh.runner.options.OptionsBuilder; + +import javax.crypto.Cipher; +import javax.crypto.Mac; +import javax.crypto.SecretKey; +import javax.crypto.spec.GCMParameterSpec; +import javax.crypto.spec.SecretKeySpec; +import java.security.MessageDigest; +import java.util.Random; +import java.util.concurrent.TimeUnit; + +@State(Scope.Thread) +@Warmup(iterations = 2) +@Measurement(iterations = 2) +@BenchmarkMode(value = {Mode.AverageTime}) +@OutputTimeUnit(TimeUnit.MICROSECONDS) +public class PooledSuppliersBenchmarkTest { + + private static final Random RNG = new Random(42); + private static final CipherSupplier CIPHER_SUPPLIER = CipherSupplier.AES_GCM; + private static final MessageDigestSupplier MD_SUPPLIER = MessageDigestSupplier.SHA256; + private static final MacSupplier MAC_SUPPLIER = MacSupplier.HMAC_SHA256; + private SecretKey key; + private GCMParameterSpec gcmParams; + + @Disabled("only on demand") + @Test + public void runBenchmarks() throws RunnerException { + Options opt = new OptionsBuilder() // + .include(getClass().getName()) // + .threads(2).forks(1) // + .shouldFailOnError(true).shouldDoGC(true) // + .build(); + new Runner(opt).run(); + } + + @Setup(Level.Invocation) + public void shuffleData() { + byte[] bytes = new byte[28]; + RNG.nextBytes(bytes); + this.key = new SecretKeySpec(bytes, 0, 16, "AES"); + this.gcmParams = new GCMParameterSpec(128, bytes, 16, 12); + } + + @Benchmark + public void createCipher(Blackhole blackHole) { + blackHole.consume(CIPHER_SUPPLIER.forEncryption(key, gcmParams)); + } + + @Benchmark + public void recycleCipher(Blackhole blackHole) { + try (ObjectPool.Lease lease = CIPHER_SUPPLIER.encryptionCipher(key, gcmParams)) { + blackHole.consume(lease.get()); + } + } + + @Benchmark + public void createMac(Blackhole blackHole) { + blackHole.consume(MAC_SUPPLIER.withKey(key)); + } + + @Benchmark + public void recycleMac(Blackhole blackHole) { + try (ObjectPool.Lease lease = MAC_SUPPLIER.keyed(key)) { + blackHole.consume(lease.get()); + } + } + + @Benchmark + public void createMd(Blackhole blackHole) { + blackHole.consume(MD_SUPPLIER.get()); + } + + @Benchmark + public void recycleMd(Blackhole blackHole) { + try (ObjectPool.Lease lease = MD_SUPPLIER.instance()) { + blackHole.consume(lease.get()); + } + } + +} From 96f00c9dddbe246f579336381edcf305b874d96c Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Tue, 25 Jan 2022 21:38:29 +0100 Subject: [PATCH 15/22] cleanup --- .../cryptolib/api/CryptoLibIntegrationTest.java | 4 ---- .../cryptolib/common/SeekableByteChannelMock.java | 10 +++++----- .../cryptolib/v1/FileContentCryptorImplTest.java | 1 - 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/test/java/org/cryptomator/cryptolib/api/CryptoLibIntegrationTest.java b/src/test/java/org/cryptomator/cryptolib/api/CryptoLibIntegrationTest.java index 0b8f78f..f0c57f8 100644 --- a/src/test/java/org/cryptomator/cryptolib/api/CryptoLibIntegrationTest.java +++ b/src/test/java/org/cryptomator/cryptolib/api/CryptoLibIntegrationTest.java @@ -8,10 +8,6 @@ *******************************************************************************/ package org.cryptomator.cryptolib.api; -import org.cryptomator.cryptolib.api.AuthenticationFailedException; -import org.cryptomator.cryptolib.api.Cryptor; -import org.cryptomator.cryptolib.api.CryptorProvider; -import org.cryptomator.cryptolib.api.Masterkey; import org.cryptomator.cryptolib.common.DecryptingReadableByteChannel; import org.cryptomator.cryptolib.common.EncryptingWritableByteChannel; import org.cryptomator.cryptolib.common.SecureRandomMock; diff --git a/src/test/java/org/cryptomator/cryptolib/common/SeekableByteChannelMock.java b/src/test/java/org/cryptomator/cryptolib/common/SeekableByteChannelMock.java index 9f2db2f..8aa9116 100644 --- a/src/test/java/org/cryptomator/cryptolib/common/SeekableByteChannelMock.java +++ b/src/test/java/org/cryptomator/cryptolib/common/SeekableByteChannelMock.java @@ -27,7 +27,7 @@ public boolean isOpen() { } @Override - public void close() throws IOException { + public void close() { open = false; } @@ -46,7 +46,7 @@ public int read(ByteBuffer dst) throws IOException { } @Override - public int write(ByteBuffer src) throws IOException { + public int write(ByteBuffer src) { int num = Math.min(buf.remaining(), src.remaining()); ByteBuffer limitedSrc = src.asReadOnlyBuffer(); limitedSrc.limit(limitedSrc.position() + num); @@ -55,12 +55,12 @@ public int write(ByteBuffer src) throws IOException { } @Override - public long position() throws IOException { + public long position() { return buf.position(); } @Override - public SeekableByteChannel position(long newPosition) throws IOException { + public SeekableByteChannel position(long newPosition) { assert newPosition < Integer.MAX_VALUE; buf.position((int) newPosition); return this; @@ -72,7 +72,7 @@ public long size() throws IOException { } @Override - public SeekableByteChannel truncate(long size) throws IOException { + public SeekableByteChannel truncate(long size) { assert size < Integer.MAX_VALUE; if (size < buf.position()) { buf.position((int) size); diff --git a/src/test/java/org/cryptomator/cryptolib/v1/FileContentCryptorImplTest.java b/src/test/java/org/cryptomator/cryptolib/v1/FileContentCryptorImplTest.java index 007fe3f..ea6fc9a 100644 --- a/src/test/java/org/cryptomator/cryptolib/v1/FileContentCryptorImplTest.java +++ b/src/test/java/org/cryptomator/cryptolib/v1/FileContentCryptorImplTest.java @@ -38,7 +38,6 @@ import java.nio.channels.ReadableByteChannel; import java.nio.channels.SeekableByteChannel; import java.nio.channels.WritableByteChannel; -import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; import java.util.Arrays; From 08308819a44bc4da1b143fcbb5098830d38d993a Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Mon, 7 Feb 2022 16:29:05 +0100 Subject: [PATCH 16/22] Avoid `ByteBuffer#asReadOnlyBuffer()` (#29) --- .../cryptolib/v1/FileContentCryptorImpl.java | 35 +++++++------------ .../cryptolib/v1/FileHeaderCryptorImpl.java | 2 +- .../cryptolib/v2/FileContentCryptorImpl.java | 6 ++-- .../cryptolib/v2/FileHeaderCryptorImpl.java | 2 +- 4 files changed, 17 insertions(+), 28 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java index cad0d0b..986e78f 100644 --- a/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v1/FileContentCryptorImpl.java @@ -112,12 +112,12 @@ void encryptChunk(ByteBuffer cleartextChunk, ByteBuffer ciphertextChunk, long ch final Cipher cipher = CipherSupplier.AES_CTR.forEncryption(fk, new IvParameterSpec(nonce)); ciphertextChunk.put(nonce); assert ciphertextChunk.remaining() >= cipher.getOutputSize(cleartextChunk.remaining()) + MAC_SIZE; - int bytesEncrypted = cipher.doFinal(cleartextChunk, ciphertextChunk); + cipher.doFinal(cleartextChunk, ciphertextChunk); // mac: - final ByteBuffer ciphertextBuf = ciphertextChunk.asReadOnlyBuffer(); - ciphertextBuf.position(NONCE_SIZE).limit(NONCE_SIZE + bytesEncrypted); - byte[] authenticationCode = calcChunkMac(headerNonce, chunkNumber, nonce, ciphertextBuf); + ByteBuffer nonceAndPayload = ciphertextChunk.duplicate(); + nonceAndPayload.flip(); + byte[] authenticationCode = calcChunkMac(headerNonce, chunkNumber, nonceAndPayload); assert authenticationCode.length == MAC_SIZE; ciphertextChunk.put(authenticationCode); } catch (ShortBufferException e) { @@ -134,12 +134,10 @@ void decryptChunk(ByteBuffer ciphertextChunk, ByteBuffer cleartextChunk, Destroy try (DestroyableSecretKey fk = fileKey.copy()) { // nonce: final byte[] nonce = new byte[NONCE_SIZE]; - final ByteBuffer chunkNonceBuf = ciphertextChunk.asReadOnlyBuffer(); - chunkNonceBuf.position(0).limit(NONCE_SIZE); - chunkNonceBuf.get(nonce); + ciphertextChunk.get(nonce, 0, NONCE_SIZE); // payload: - final ByteBuffer payloadBuf = ciphertextChunk.asReadOnlyBuffer(); + final ByteBuffer payloadBuf = ciphertextChunk.duplicate(); payloadBuf.position(NONCE_SIZE).limit(ciphertextChunk.limit() - MAC_SIZE); // payload: @@ -157,37 +155,30 @@ void decryptChunk(ByteBuffer ciphertextChunk, ByteBuffer cleartextChunk, Destroy boolean checkChunkMac(byte[] headerNonce, long chunkNumber, ByteBuffer chunkBuf) { assert chunkBuf.remaining() >= NONCE_SIZE + MAC_SIZE; - // get three components: nonce + payload + mac - final ByteBuffer chunkNonceBuf = chunkBuf.asReadOnlyBuffer(); - chunkNonceBuf.position(0).limit(NONCE_SIZE); - final ByteBuffer payloadBuf = chunkBuf.asReadOnlyBuffer(); - payloadBuf.position(NONCE_SIZE).limit(chunkBuf.limit() - MAC_SIZE); - final ByteBuffer expectedMacBuf = chunkBuf.asReadOnlyBuffer(); + // get nonce + payload + final ByteBuffer nonceAndPayload = chunkBuf.duplicate(); + nonceAndPayload.position(0).limit(chunkBuf.limit() - MAC_SIZE); + final ByteBuffer expectedMacBuf = chunkBuf.duplicate(); expectedMacBuf.position(chunkBuf.limit() - MAC_SIZE); - // get nonce: - final byte[] chunkNonce = new byte[NONCE_SIZE]; - chunkNonceBuf.get(chunkNonce); - // get expected MAC: final byte[] expectedMac = new byte[MAC_SIZE]; expectedMacBuf.get(expectedMac); // get actual MAC: - final byte[] calculatedMac = calcChunkMac(headerNonce, chunkNumber, chunkNonce, payloadBuf); + final byte[] calculatedMac = calcChunkMac(headerNonce, chunkNumber, nonceAndPayload); // time-constant equality check of two MACs: return MessageDigest.isEqual(expectedMac, calculatedMac); } - private byte[] calcChunkMac(byte[] headerNonce, long chunkNumber, byte[] chunkNonce, ByteBuffer ciphertext) { + private byte[] calcChunkMac(byte[] headerNonce, long chunkNumber, ByteBuffer nonceAndCiphertext) { try (DestroyableSecretKey mk = masterkey.getMacKey()) { final byte[] chunkNumberBigEndian = ByteBuffer.allocate(Long.SIZE / Byte.SIZE).order(ByteOrder.BIG_ENDIAN).putLong(chunkNumber).array(); final Mac mac = MacSupplier.HMAC_SHA256.withKey(mk); mac.update(headerNonce); mac.update(chunkNumberBigEndian); - mac.update(chunkNonce); - mac.update(ciphertext); + mac.update(nonceAndCiphertext); return mac.doFinal(); } } diff --git a/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java index aeaa7ee..46e8d4f 100644 --- a/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImpl.java @@ -88,7 +88,7 @@ public FileHeader decryptHeader(ByteBuffer ciphertextHeaderBuf) throws Authentic if (ciphertextHeaderBuf.remaining() < FileHeaderImpl.SIZE) { throw new IllegalArgumentException("Malformed ciphertext header"); } - ByteBuffer buf = ciphertextHeaderBuf.asReadOnlyBuffer(); + ByteBuffer buf = ciphertextHeaderBuf.duplicate(); byte[] nonce = new byte[FileHeaderImpl.NONCE_LEN]; buf.position(FileHeaderImpl.NONCE_POS); buf.get(nonce); diff --git a/src/main/java/org/cryptomator/cryptolib/v2/FileContentCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v2/FileContentCryptorImpl.java index a03eea9..e59beff 100644 --- a/src/main/java/org/cryptomator/cryptolib/v2/FileContentCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v2/FileContentCryptorImpl.java @@ -124,12 +124,10 @@ void decryptChunk(ByteBuffer ciphertextChunk, ByteBuffer cleartextChunk, long ch try (DestroyableSecretKey fk = fileKey.copy()) { // nonce: final byte[] nonce = new byte[GCM_NONCE_SIZE]; - final ByteBuffer chunkNonceBuf = ciphertextChunk.asReadOnlyBuffer(); - chunkNonceBuf.position(0).limit(GCM_NONCE_SIZE); - chunkNonceBuf.get(nonce); + ciphertextChunk.get(nonce, 0, GCM_NONCE_SIZE); // payload: - final ByteBuffer payloadBuf = ciphertextChunk.asReadOnlyBuffer(); + final ByteBuffer payloadBuf = ciphertextChunk.duplicate(); payloadBuf.position(GCM_NONCE_SIZE); assert payloadBuf.remaining() >= GCM_TAG_SIZE; diff --git a/src/main/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImpl.java index 509a5b9..2182e7b 100644 --- a/src/main/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImpl.java @@ -81,7 +81,7 @@ public FileHeader decryptHeader(ByteBuffer ciphertextHeaderBuf) throws Authentic if (ciphertextHeaderBuf.remaining() < FileHeaderImpl.SIZE) { throw new IllegalArgumentException("Malformed ciphertext header"); } - ByteBuffer buf = ciphertextHeaderBuf.asReadOnlyBuffer(); + ByteBuffer buf = ciphertextHeaderBuf.duplicate(); byte[] nonce = new byte[FileHeaderImpl.NONCE_LEN]; buf.position(FileHeaderImpl.NONCE_POS); buf.get(nonce); From 3134896b74faa4d80d4b3dd704d4f5d6e712bab8 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Tue, 8 Feb 2022 10:05:11 +0100 Subject: [PATCH 17/22] replace ThreadLocal with ObjectPool for SivMode instances --- .../cryptolib/v1/FileNameCryptorImpl.java | 23 ++++++++----------- .../cryptolib/v2/FileNameCryptorImpl.java | 23 ++++++++----------- 2 files changed, 20 insertions(+), 26 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptolib/v1/FileNameCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v1/FileNameCryptorImpl.java index 309d1d6..5104aa5 100644 --- a/src/main/java/org/cryptomator/cryptolib/v1/FileNameCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v1/FileNameCryptorImpl.java @@ -19,7 +19,6 @@ import org.cryptomator.siv.UnauthenticCiphertextException; import javax.crypto.IllegalBlockSizeException; - import java.security.MessageDigest; import static java.nio.charset.StandardCharsets.UTF_8; @@ -27,12 +26,7 @@ class FileNameCryptorImpl implements FileNameCryptor { private static final BaseEncoding BASE32 = BaseEncoding.base32(); - private static final ThreadLocal AES_SIV = new ThreadLocal() { - @Override - protected SivMode initialValue() { - return new SivMode(); - } - }; + private static final ObjectPool AES_SIV = new ObjectPool<>(SivMode::new); private final Masterkey masterkey; @@ -43,9 +37,10 @@ protected SivMode initialValue() { @Override public String hashDirectoryId(String cleartextDirectoryId) { try (DestroyableSecretKey ek = masterkey.getEncKey(); DestroyableSecretKey mk = masterkey.getMacKey(); - ObjectPool.Lease sha1 = MessageDigestSupplier.SHA1.instance()) { + ObjectPool.Lease sha1 = MessageDigestSupplier.SHA1.instance(); + ObjectPool.Lease siv = AES_SIV.get()) { byte[] cleartextBytes = cleartextDirectoryId.getBytes(UTF_8); - byte[] encryptedBytes = AES_SIV.get().encrypt(ek, mk, cleartextBytes); + byte[] encryptedBytes = siv.get().encrypt(ek, mk, cleartextBytes); byte[] hashedBytes = sha1.get().digest(encryptedBytes); return BASE32.encode(hashedBytes); } @@ -53,18 +48,20 @@ public String hashDirectoryId(String cleartextDirectoryId) { @Override public String encryptFilename(BaseEncoding encoding, String cleartextName, byte[]... associatedData) { - try (DestroyableSecretKey ek = masterkey.getEncKey(); DestroyableSecretKey mk = masterkey.getMacKey()) { + try (DestroyableSecretKey ek = masterkey.getEncKey(); DestroyableSecretKey mk = masterkey.getMacKey(); + ObjectPool.Lease siv = AES_SIV.get()) { byte[] cleartextBytes = cleartextName.getBytes(UTF_8); - byte[] encryptedBytes = AES_SIV.get().encrypt(ek, mk, cleartextBytes, associatedData); + byte[] encryptedBytes = siv.get().encrypt(ek, mk, cleartextBytes, associatedData); return encoding.encode(encryptedBytes); } } @Override public String decryptFilename(BaseEncoding encoding, String ciphertextName, byte[]... associatedData) throws AuthenticationFailedException { - try (DestroyableSecretKey ek = masterkey.getEncKey(); DestroyableSecretKey mk = masterkey.getMacKey()) { + try (DestroyableSecretKey ek = masterkey.getEncKey(); DestroyableSecretKey mk = masterkey.getMacKey(); + ObjectPool.Lease siv = AES_SIV.get()) { byte[] encryptedBytes = encoding.decode(ciphertextName); - byte[] cleartextBytes = AES_SIV.get().decrypt(ek, mk, encryptedBytes, associatedData); + byte[] cleartextBytes = siv.get().decrypt(ek, mk, encryptedBytes, associatedData); return new String(cleartextBytes, UTF_8); } catch (UnauthenticCiphertextException | IllegalArgumentException | IllegalBlockSizeException e) { throw new AuthenticationFailedException("Invalid Ciphertext.", e); diff --git a/src/main/java/org/cryptomator/cryptolib/v2/FileNameCryptorImpl.java b/src/main/java/org/cryptomator/cryptolib/v2/FileNameCryptorImpl.java index 1f4a573..0498afe 100644 --- a/src/main/java/org/cryptomator/cryptolib/v2/FileNameCryptorImpl.java +++ b/src/main/java/org/cryptomator/cryptolib/v2/FileNameCryptorImpl.java @@ -19,7 +19,6 @@ import org.cryptomator.siv.UnauthenticCiphertextException; import javax.crypto.IllegalBlockSizeException; - import java.security.MessageDigest; import static java.nio.charset.StandardCharsets.UTF_8; @@ -27,12 +26,7 @@ class FileNameCryptorImpl implements FileNameCryptor { private static final BaseEncoding BASE32 = BaseEncoding.base32(); - private static final ThreadLocal AES_SIV = new ThreadLocal() { - @Override - protected SivMode initialValue() { - return new SivMode(); - } - }; + private static final ObjectPool AES_SIV = new ObjectPool<>(SivMode::new); private final Masterkey masterkey; @@ -43,9 +37,10 @@ protected SivMode initialValue() { @Override public String hashDirectoryId(String cleartextDirectoryId) { try (DestroyableSecretKey ek = masterkey.getEncKey(); DestroyableSecretKey mk = masterkey.getMacKey(); - ObjectPool.Lease sha1 = MessageDigestSupplier.SHA1.instance()) { + ObjectPool.Lease sha1 = MessageDigestSupplier.SHA1.instance(); + ObjectPool.Lease siv = AES_SIV.get()) { byte[] cleartextBytes = cleartextDirectoryId.getBytes(UTF_8); - byte[] encryptedBytes = AES_SIV.get().encrypt(ek, mk, cleartextBytes); + byte[] encryptedBytes = siv.get().encrypt(ek, mk, cleartextBytes); byte[] hashedBytes = sha1.get().digest(encryptedBytes); return BASE32.encode(hashedBytes); } @@ -53,18 +48,20 @@ public String hashDirectoryId(String cleartextDirectoryId) { @Override public String encryptFilename(BaseEncoding encoding, String cleartextName, byte[]... associatedData) { - try (DestroyableSecretKey ek = masterkey.getEncKey(); DestroyableSecretKey mk = masterkey.getMacKey()) { + try (DestroyableSecretKey ek = masterkey.getEncKey(); DestroyableSecretKey mk = masterkey.getMacKey(); + ObjectPool.Lease siv = AES_SIV.get()) { byte[] cleartextBytes = cleartextName.getBytes(UTF_8); - byte[] encryptedBytes = AES_SIV.get().encrypt(ek, mk, cleartextBytes, associatedData); + byte[] encryptedBytes = siv.get().encrypt(ek, mk, cleartextBytes, associatedData); return encoding.encode(encryptedBytes); } } @Override public String decryptFilename(BaseEncoding encoding, String ciphertextName, byte[]... associatedData) throws AuthenticationFailedException { - try (DestroyableSecretKey ek = masterkey.getEncKey(); DestroyableSecretKey mk = masterkey.getMacKey()) { + try (DestroyableSecretKey ek = masterkey.getEncKey(); DestroyableSecretKey mk = masterkey.getMacKey(); + ObjectPool.Lease siv = AES_SIV.get()) { byte[] encryptedBytes = encoding.decode(ciphertextName); - byte[] cleartextBytes = AES_SIV.get().decrypt(ek, mk, encryptedBytes, associatedData); + byte[] cleartextBytes = siv.get().decrypt(ek, mk, encryptedBytes, associatedData); return new String(cleartextBytes, UTF_8); } catch (IllegalArgumentException | UnauthenticCiphertextException | IllegalBlockSizeException e) { throw new AuthenticationFailedException("Invalid Ciphertext.", e); From 4991b5695bdaeeb4f0b6fc21d094b7eab037a567 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Tue, 8 Feb 2022 15:03:51 +0100 Subject: [PATCH 18/22] fixed code smells --- .../cryptomator/cryptolib/common/AesKeyWrap.java | 7 +++++-- .../cryptomator/cryptolib/common/ByteBuffers.java | 3 +++ .../cryptolib/common/DestroyableSecretKey.java | 10 ++++++---- .../cryptolib/common/Destroyables.java | 3 +++ .../org/cryptomator/cryptolib/v1/Constants.java | 3 +++ .../org/cryptomator/cryptolib/v2/Constants.java | 3 +++ .../cryptolib/common/CipherSupplierTest.java | 10 ++++++++-- .../cryptolib/v1/FileHeaderCryptorImplTest.java | 12 ++++++------ .../cryptolib/v1/FileNameCryptorImplTest.java | 6 ++++-- .../cryptolib/v2/FileHeaderCryptorImplTest.java | 15 +++++++++------ .../cryptolib/v2/FileNameCryptorImplTest.java | 6 ++++-- 11 files changed, 54 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptolib/common/AesKeyWrap.java b/src/main/java/org/cryptomator/cryptolib/common/AesKeyWrap.java index 5b603e7..86e7489 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/AesKeyWrap.java +++ b/src/main/java/org/cryptomator/cryptolib/common/AesKeyWrap.java @@ -16,6 +16,9 @@ public class AesKeyWrap { + private AesKeyWrap() { + } + /** * @param kek Key encrypting key * @param key Key to be wrapped @@ -31,8 +34,8 @@ public static byte[] wrap(DestroyableSecretKey kek, SecretKey key) { } /** - * @param kek Key encrypting key - * @param wrappedKey Key to be unwrapped + * @param kek Key encrypting key + * @param wrappedKey Key to be unwrapped * @param wrappedKeyAlgorithm Key designation, i.e. algorithm to be associated with the unwrapped key. * @return Unwrapped key * @throws InvalidKeyException If unwrapping failed (i.e. wrong kek) diff --git a/src/main/java/org/cryptomator/cryptolib/common/ByteBuffers.java b/src/main/java/org/cryptomator/cryptolib/common/ByteBuffers.java index d30ee36..d80662e 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/ByteBuffers.java +++ b/src/main/java/org/cryptomator/cryptolib/common/ByteBuffers.java @@ -14,6 +14,9 @@ public class ByteBuffers { + private ByteBuffers() { + } + /** * Copies as many bytes as possible from the given source to the destination buffer. * The position of both buffers will be incremented by as many bytes as have been copied. diff --git a/src/main/java/org/cryptomator/cryptolib/common/DestroyableSecretKey.java b/src/main/java/org/cryptomator/cryptolib/common/DestroyableSecretKey.java index 4f91ea2..f28baeb 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/DestroyableSecretKey.java +++ b/src/main/java/org/cryptomator/cryptolib/common/DestroyableSecretKey.java @@ -29,6 +29,8 @@ */ public class DestroyableSecretKey implements SecretKey, AutoCloseable { + private static final String KEY_DESTROYED_ERROR = "Key has been destroyed"; + private final transient byte[] key; private final String algorithm; private boolean destroyed; @@ -95,13 +97,13 @@ public static DestroyableSecretKey generate(SecureRandom csprng, String algorith @Override public String getAlgorithm() { - Preconditions.checkState(!destroyed, "Key has been destroyed"); + Preconditions.checkState(!destroyed, KEY_DESTROYED_ERROR); return algorithm; } @Override public String getFormat() { - Preconditions.checkState(!destroyed, "Key has been destroyed"); + Preconditions.checkState(!destroyed, KEY_DESTROYED_ERROR); return "RAW"; } @@ -115,7 +117,7 @@ public String getFormat() { */ @Override public byte[] getEncoded() { - Preconditions.checkState(!destroyed, "Key has been destroyed"); + Preconditions.checkState(!destroyed, KEY_DESTROYED_ERROR); return key; } @@ -124,7 +126,7 @@ public byte[] getEncoded() { * @return New copy of this */ public DestroyableSecretKey copy() { - Preconditions.checkState(!destroyed, "Key has been destroyed"); + Preconditions.checkState(!destroyed, KEY_DESTROYED_ERROR); return new DestroyableSecretKey(key, algorithm); // key will get copied by the constructor as per contract } diff --git a/src/main/java/org/cryptomator/cryptolib/common/Destroyables.java b/src/main/java/org/cryptomator/cryptolib/common/Destroyables.java index 163a00f..84fe04d 100644 --- a/src/main/java/org/cryptomator/cryptolib/common/Destroyables.java +++ b/src/main/java/org/cryptomator/cryptolib/common/Destroyables.java @@ -5,6 +5,9 @@ public class Destroyables { + private Destroyables() { + } + public static void destroySilently(Destroyable destroyable) { if (destroyable == null) { return; diff --git a/src/main/java/org/cryptomator/cryptolib/v1/Constants.java b/src/main/java/org/cryptomator/cryptolib/v1/Constants.java index e4cc9d5..4662fae 100644 --- a/src/main/java/org/cryptomator/cryptolib/v1/Constants.java +++ b/src/main/java/org/cryptomator/cryptolib/v1/Constants.java @@ -10,6 +10,9 @@ final class Constants { + private Constants() { + } + static final String CONTENT_ENC_ALG = "AES"; static final int NONCE_SIZE = 16; diff --git a/src/main/java/org/cryptomator/cryptolib/v2/Constants.java b/src/main/java/org/cryptomator/cryptolib/v2/Constants.java index b0a7380..f6c0f4d 100644 --- a/src/main/java/org/cryptomator/cryptolib/v2/Constants.java +++ b/src/main/java/org/cryptomator/cryptolib/v2/Constants.java @@ -10,6 +10,9 @@ final class Constants { + private Constants() { + } + static final String CONTENT_ENC_ALG = "AES"; static final int GCM_NONCE_SIZE = 12; // 96 bit IVs strongly recommended for GCM diff --git a/src/test/java/org/cryptomator/cryptolib/common/CipherSupplierTest.java b/src/test/java/org/cryptomator/cryptolib/common/CipherSupplierTest.java index 777ef95..9923cce 100644 --- a/src/test/java/org/cryptomator/cryptolib/common/CipherSupplierTest.java +++ b/src/test/java/org/cryptomator/cryptolib/common/CipherSupplierTest.java @@ -13,8 +13,10 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import javax.crypto.SecretKey; import javax.crypto.spec.IvParameterSpec; import javax.crypto.spec.RC5ParameterSpec; +import java.security.spec.AlgorithmParameterSpec; public class CipherSupplierTest { @@ -28,8 +30,10 @@ public void testGetUnknownCipher() { @Test public void testGetCipherWithInvalidKey() { CipherSupplier supplier = new CipherSupplier("AES/CBC/PKCS5Padding"); + SecretKey key = new DestroyableSecretKey(new byte[13], "AES"); + AlgorithmParameterSpec params = new IvParameterSpec(new byte[16]); IllegalArgumentException exception = Assertions.assertThrows(IllegalArgumentException.class, () -> { - supplier.encryptionCipher(new DestroyableSecretKey(new byte[13], "AES"), new IvParameterSpec(new byte[16])); + supplier.encryptionCipher(key, params); }); MatcherAssert.assertThat(exception.getMessage(), CoreMatchers.containsString("Invalid key")); } @@ -37,8 +41,10 @@ public void testGetCipherWithInvalidKey() { @Test public void testGetCipherWithInvalidAlgorithmParam() { CipherSupplier supplier = new CipherSupplier("AES/CBC/PKCS5Padding"); + SecretKey key = new DestroyableSecretKey(new byte[16], "AES"); + AlgorithmParameterSpec params = new RC5ParameterSpec(1, 1, 8); IllegalArgumentException exception = Assertions.assertThrows(IllegalArgumentException.class, () -> { - supplier.encryptionCipher(new DestroyableSecretKey(new byte[16], "AES"), new RC5ParameterSpec(1, 1, 8)); + supplier.encryptionCipher(key, params); }); MatcherAssert.assertThat(exception.getMessage(), CoreMatchers.containsString("Algorithm parameter not appropriate for")); } diff --git a/src/test/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImplTest.java b/src/test/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImplTest.java index 11fa682..5605700 100644 --- a/src/test/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImplTest.java +++ b/src/test/java/org/cryptomator/cryptolib/v1/FileHeaderCryptorImplTest.java @@ -69,28 +69,28 @@ public void testDecryption() throws AuthenticationFailedException { @Test public void testDecryptionWithTooShortHeader() { - byte[] ciphertext = new byte[7]; + ByteBuffer ciphertext = ByteBuffer.allocate(7); Assertions.assertThrows(IllegalArgumentException.class, () -> { - headerCryptor.decryptHeader(ByteBuffer.wrap(ciphertext)); + headerCryptor.decryptHeader(ciphertext); }); } @Test public void testDecryptionWithInvalidMac1() { - byte[] ciphertext = BaseEncoding.base64().decode("AAAAAAAAAAAAAAAAAAAAANyVwHiiQImjrUiiFJKEIIdTD4r7x0U2ualjtPHEy3OLzqdAPU1ga26lJzstK9RUv1hj5zDC4wC9FgMfoVE1mD0HnuENuYXkJa=="); + ByteBuffer ciphertext = ByteBuffer.wrap(BaseEncoding.base64().decode("AAAAAAAAAAAAAAAAAAAAANyVwHiiQImjrUiiFJKEIIdTD4r7x0U2ualjtPHEy3OLzqdAPU1ga26lJzstK9RUv1hj5zDC4wC9FgMfoVE1mD0HnuENuYXkJa==")); Assertions.assertThrows(AuthenticationFailedException.class, () -> { - headerCryptor.decryptHeader(ByteBuffer.wrap(ciphertext)); + headerCryptor.decryptHeader(ciphertext); }); } @Test public void testDecryptionWithInvalidMac2() { - byte[] ciphertext = BaseEncoding.base64().decode("aAAAAAAAAAAAAAAAAAAAANyVwHiiQImjrUiiFJKEIIdTD4r7x0U2ualjtPHEy3OLzqdAPU1ga26lJzstK9RUv1hj5zDC4wC9FgMfoVE1mD0HnuENuYXkJA=="); + ByteBuffer ciphertext = ByteBuffer.wrap(BaseEncoding.base64().decode("aAAAAAAAAAAAAAAAAAAAANyVwHiiQImjrUiiFJKEIIdTD4r7x0U2ualjtPHEy3OLzqdAPU1ga26lJzstK9RUv1hj5zDC4wC9FgMfoVE1mD0HnuENuYXkJA==")); Assertions.assertThrows(AuthenticationFailedException.class, () -> { - headerCryptor.decryptHeader(ByteBuffer.wrap(ciphertext)); + headerCryptor.decryptHeader(ciphertext); }); } diff --git a/src/test/java/org/cryptomator/cryptolib/v1/FileNameCryptorImplTest.java b/src/test/java/org/cryptomator/cryptolib/v1/FileNameCryptorImplTest.java index ec77ab0..99ef409 100644 --- a/src/test/java/org/cryptomator/cryptolib/v1/FileNameCryptorImplTest.java +++ b/src/test/java/org/cryptomator/cryptolib/v1/FileNameCryptorImplTest.java @@ -97,9 +97,10 @@ public void testDecryptionOfMalformedFilename() { public void testDecryptionOfManipulatedFilename() { final byte[] encrypted = filenameCryptor.encryptFilename(BASE32, "test").getBytes(UTF_8); encrypted[0] ^= (byte) 0x01; // change 1 bit in first byte + String ciphertextName = new String(encrypted, UTF_8); AuthenticationFailedException e = Assertions.assertThrows(AuthenticationFailedException.class, () -> { - filenameCryptor.decryptFilename(BASE32, new String(encrypted, UTF_8)); + filenameCryptor.decryptFilename(BASE32, ciphertextName); }); MatcherAssert.assertThat(e.getCause(), CoreMatchers.instanceOf(UnauthenticCiphertextException.class)); } @@ -124,9 +125,10 @@ public void testDeterministicEncryptionOfFilenamesWithAssociatedData() throws Au @DisplayName("decrypt ciphertext with incorrect AD") public void testDeterministicEncryptionOfFilenamesWithWrongAssociatedData() { final String encrypted = filenameCryptor.encryptFilename(BASE32, "test", "right".getBytes(UTF_8)); + final byte[] ad = "wrong".getBytes(UTF_8); Assertions.assertThrows(AuthenticationFailedException.class, () -> { - filenameCryptor.decryptFilename(BASE32, encrypted, "wrong".getBytes(UTF_8)); + filenameCryptor.decryptFilename(BASE32, encrypted, ad); }); } diff --git a/src/test/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImplTest.java b/src/test/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImplTest.java index c9e1c3b..baea1a5 100644 --- a/src/test/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImplTest.java +++ b/src/test/java/org/cryptomator/cryptolib/v2/FileHeaderCryptorImplTest.java @@ -78,25 +78,28 @@ public void testDecryption() throws AuthenticationFailedException { @Test public void testDecryptionWithTooShortHeader() { - byte[] ciphertext = new byte[7]; + ByteBuffer ciphertext = ByteBuffer.allocate(7); + Assertions.assertThrows(IllegalArgumentException.class, () -> { - headerCryptor.decryptHeader(ByteBuffer.wrap(ciphertext)); + headerCryptor.decryptHeader(ciphertext); }); } @Test public void testDecryptionWithInvalidTag1() { - byte[] ciphertext = BaseEncoding.base64().decode("AAAAAAAAAAAAAAAAMVi/wrKflJEHTsXTuvOdGHJgA8o3pip00aL1jnUGNY7dSrEoTUrhey+maVG6P0F2RBmZR74SjUA="); + ByteBuffer ciphertext = ByteBuffer.wrap(BaseEncoding.base64().decode("AAAAAAAAAAAAAAAAMVi/wrKflJEHTsXTuvOdGHJgA8o3pip00aL1jnUGNY7dSrEoTUrhey+maVG6P0F2RBmZR74SjUA=")); + Assertions.assertThrows(AuthenticationFailedException.class, () -> { - headerCryptor.decryptHeader(ByteBuffer.wrap(ciphertext)); + headerCryptor.decryptHeader(ciphertext); }); } @Test public void testDecryptionWithInvalidTag2() { - byte[] ciphertext = BaseEncoding.base64().decode("AAAAAAAAAAAAAAAAMVi/wrKflJEHTsXTuvOdGHJgA8o3pip00aL1jnUGNY7dSrEoTUrhey+maVG6P0F2RBmZR74SjUa="); + ByteBuffer ciphertext = ByteBuffer.wrap(BaseEncoding.base64().decode("AAAAAAAAAAAAAAAAMVi/wrKflJEHTsXTuvOdGHJgA8o3pip00aL1jnUGNY7dSrEoTUrhey+maVG6P0F2RBmZR74SjUa=")); + Assertions.assertThrows(AuthenticationFailedException.class, () -> { - headerCryptor.decryptHeader(ByteBuffer.wrap(ciphertext)); + headerCryptor.decryptHeader(ciphertext); }); } diff --git a/src/test/java/org/cryptomator/cryptolib/v2/FileNameCryptorImplTest.java b/src/test/java/org/cryptomator/cryptolib/v2/FileNameCryptorImplTest.java index daa5da1..d808f89 100644 --- a/src/test/java/org/cryptomator/cryptolib/v2/FileNameCryptorImplTest.java +++ b/src/test/java/org/cryptomator/cryptolib/v2/FileNameCryptorImplTest.java @@ -98,9 +98,10 @@ public void testDecryptionOfMalformedFilename() { public void testDecryptionOfManipulatedFilename() { final byte[] encrypted = filenameCryptor.encryptFilename(BASE32, "test").getBytes(UTF_8); encrypted[0] ^= (byte) 0x01; // change 1 bit in first byte + String ciphertextName = new String(encrypted, UTF_8); AuthenticationFailedException e = Assertions.assertThrows(AuthenticationFailedException.class, () -> { - filenameCryptor.decryptFilename(BASE32, new String(encrypted, UTF_8)); + filenameCryptor.decryptFilename(BASE32, ciphertextName); }); MatcherAssert.assertThat(e.getCause(), CoreMatchers.instanceOf(UnauthenticCiphertextException.class)); } @@ -125,9 +126,10 @@ public void testDeterministicEncryptionOfFilenamesWithAssociatedData() throws Au @DisplayName("decrypt ciphertext with incorrect AD") public void testDeterministicEncryptionOfFilenamesWithWrongAssociatedData() { final String encrypted = filenameCryptor.encryptFilename(BASE32, "test", "right".getBytes(UTF_8)); + final byte[] ad = "wrong".getBytes(UTF_8); Assertions.assertThrows(AuthenticationFailedException.class, () -> { - filenameCryptor.decryptFilename(BASE32, encrypted, "wrong".getBytes(UTF_8)); + filenameCryptor.decryptFilename(BASE32, encrypted, ad); }); } From cb45c0b6901b3887f4396262a055624394579a2b Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Tue, 8 Feb 2022 15:44:20 +0100 Subject: [PATCH 19/22] `com.google.common.io.BaseEncoding` is part of the public API of FileNameCryptor --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 61629df..127e8e9 100644 --- a/pom.xml +++ b/pom.xml @@ -213,7 +213,7 @@ module org.cryptomator.cryptolib { requires org.cryptomator.siv; requires com.google.gson; - requires com.google.common; + requires transitive com.google.common; requires org.slf4j; exports org.cryptomator.cryptolib.api; From e0416458795a9eb519a325a534e3c6c24a6b457a Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Tue, 8 Feb 2022 16:47:41 +0100 Subject: [PATCH 20/22] use a new key for each test run --- .../cryptomator/cryptolib/api/CryptoLibIntegrationTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/cryptomator/cryptolib/api/CryptoLibIntegrationTest.java b/src/test/java/org/cryptomator/cryptolib/api/CryptoLibIntegrationTest.java index f0c57f8..f846629 100644 --- a/src/test/java/org/cryptomator/cryptolib/api/CryptoLibIntegrationTest.java +++ b/src/test/java/org/cryptomator/cryptolib/api/CryptoLibIntegrationTest.java @@ -30,12 +30,11 @@ public class CryptoLibIntegrationTest { private static final SecureRandom RANDOM_MOCK = SecureRandomMock.PRNG_RANDOM; - private static final Masterkey MASTERKEY = Masterkey.generate(RANDOM_MOCK); private static Stream getCryptors() { return Stream.of( - CryptorProvider.forScheme(CryptorProvider.Scheme.SIV_CTRMAC).provide(MASTERKEY, RANDOM_MOCK), - CryptorProvider.forScheme(CryptorProvider.Scheme.SIV_GCM).provide(MASTERKEY, RANDOM_MOCK) + CryptorProvider.forScheme(CryptorProvider.Scheme.SIV_CTRMAC).provide(Masterkey.generate(RANDOM_MOCK), RANDOM_MOCK), + CryptorProvider.forScheme(CryptorProvider.Scheme.SIV_GCM).provide(Masterkey.generate(RANDOM_MOCK), RANDOM_MOCK) ); } From eccaa3b64535126f26c58fcf702648379c0c2f17 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Tue, 8 Feb 2022 16:47:50 +0100 Subject: [PATCH 21/22] bump dependencies --- pom.xml | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/pom.xml b/pom.xml index 127e8e9..5c14f35 100644 --- a/pom.xml +++ b/pom.xml @@ -19,19 +19,19 @@ 2.8.9 - 30.1.1-jre - 1.4.3 - 1.69 - 1.7.31 + 31.0.1-jre + 1.4.4 + 1.70 + 1.7.35 - 5.7.2 - 3.11.2 + 5.8.2 + 3.12.4 2.2 - 1.32 + 1.34 - 6.2.2 + 6.5.3 0.8.7 1.6.8 @@ -131,7 +131,7 @@ org.apache.maven.plugins maven-enforcer-plugin - 3.0.0-M3 + 3.0.0 enforce-java @@ -151,7 +151,7 @@ maven-compiler-plugin - 3.8.1 + 3.9.0 UTF-8 true @@ -197,7 +197,7 @@ org.moditect moditect-maven-plugin - 1.0.0.RC1 + 1.0.0.RC2 add-module-infos @@ -238,12 +238,12 @@ org.apache.maven.plugins maven-surefire-plugin - 2.22.2 + 3.0.0-M5 org.apache.maven.plugins maven-jar-plugin - 3.2.0 + 3.2.2 @@ -267,7 +267,7 @@ maven-javadoc-plugin - 3.3.0 + 3.3.1 attach-javadocs From 6eb83a863c690ea55dd3dad71961518302239e38 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Tue, 8 Feb 2022 17:10:23 +0100 Subject: [PATCH 22/22] update to Mockito 4.x --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 5c14f35..a2240bb 100644 --- a/pom.xml +++ b/pom.xml @@ -26,7 +26,7 @@ 5.8.2 - 3.12.4 + 4.3.1 2.2 1.34