Skip to content

Commit

Permalink
Merge branch 'hotfix/2.0.1'
Browse files Browse the repository at this point in the history
  • Loading branch information
overheadhunter committed Aug 20, 2021
2 parents 8e5a822 + 7b180ca commit cad2d7d
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 38 deletions.
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>org.cryptomator</groupId>
<artifactId>cryptolib</artifactId>
<version>2.0.0</version>
<version>2.0.1</version>
<name>Cryptomator Crypto Library</name>
<description>This library contains all cryptographic functions that are used by Cryptomator.</description>
<url>https://github.com/cryptomator/cryptolib</url>
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/cryptomator/cryptolib/api/Masterkey.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public static Masterkey from(DestroyableSecretKey encKey, DestroyableSecretKey m
}

@Override
public Masterkey clone() {
public Masterkey copy() {
return new Masterkey(getEncoded());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ public class AesKeyWrap {
* @return Wrapped key
*/
public static byte[] wrap(DestroyableSecretKey kek, SecretKey key) {
try {
final Cipher cipher = CipherSupplier.RFC3394_KEYWRAP.forWrapping(kek);
try (DestroyableSecretKey kekCopy = kek.copy()) {
final Cipher cipher = CipherSupplier.RFC3394_KEYWRAP.forWrapping(kekCopy);
return cipher.wrap(key);
} catch (InvalidKeyException | IllegalBlockSizeException e) {
throw new IllegalArgumentException("Unable to wrap key.", e);
Expand All @@ -43,8 +43,8 @@ public static DestroyableSecretKey unwrap(DestroyableSecretKey kek, byte[] wrapp

// visible for testing
static DestroyableSecretKey unwrap(DestroyableSecretKey kek, byte[] wrappedKey, String wrappedKeyAlgorithm, int wrappedKeyType) throws InvalidKeyException {
final Cipher cipher = CipherSupplier.RFC3394_KEYWRAP.forUnwrapping(kek);
try {
try (DestroyableSecretKey kekCopy = kek.copy()) {
final Cipher cipher = CipherSupplier.RFC3394_KEYWRAP.forUnwrapping(kekCopy);
return DestroyableSecretKey.from(cipher.unwrap(wrappedKey, wrappedKeyAlgorithm, wrappedKeyType));
} catch (NoSuchAlgorithmException e) {
throw new IllegalArgumentException("Invalid algorithm: " + wrappedKeyAlgorithm, e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@

import javax.crypto.Cipher;
import javax.crypto.NoSuchPaddingException;
import javax.crypto.SecretKey;
import java.security.InvalidAlgorithmParameterException;
import java.security.InvalidKeyException;
import java.security.NoSuchAlgorithmException;
import java.security.spec.AlgorithmParameterSpec;
import java.util.function.Function;

public final class CipherSupplier {

Expand Down Expand Up @@ -41,27 +43,27 @@ protected Cipher initialValue() {
}
}

public Cipher forEncryption(DestroyableSecretKey key, AlgorithmParameterSpec params) {
public Cipher forEncryption(SecretKey key, AlgorithmParameterSpec params) {
return forMode(Cipher.ENCRYPT_MODE, key, params);
}

public Cipher forDecryption(DestroyableSecretKey key, AlgorithmParameterSpec params) {
public Cipher forDecryption(SecretKey key, AlgorithmParameterSpec params) {
return forMode(Cipher.DECRYPT_MODE, key, params);
}

public Cipher forWrapping(DestroyableSecretKey kek) {
public Cipher forWrapping(SecretKey kek) {
return forMode(Cipher.WRAP_MODE, kek, null);
}

public Cipher forUnwrapping(DestroyableSecretKey kek) {
public Cipher forUnwrapping(SecretKey kek) {
return forMode(Cipher.UNWRAP_MODE, kek, null);
}

// visible for testing
Cipher forMode(int ciphermode, DestroyableSecretKey key, AlgorithmParameterSpec params) {
Cipher forMode(int ciphermode, SecretKey key, AlgorithmParameterSpec params) {
final Cipher cipher = threadLocal.get();
try (DestroyableSecretKey clone = key.clone()) {
cipher.init(ciphermode, clone, params); // use cloned key, as this may destroy key.getEncoded()
try {
cipher.init(ciphermode, key, params);
return cipher;
} catch (InvalidKeyException e) {
throw new IllegalArgumentException("Invalid key.", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,19 @@
* actually implements {@link Destroyable}.
* <p>
* Furthermore, this implementation will not create copies when accessing {@link #getEncoded()}.
* Instead it implements {@link AutoCloseable} and {@link Cloneable} in an exception-free manner. To prevent mutation of the exposed key,
* Instead it implements {@link #copy} and {@link AutoCloseable} in an exception-free manner. To prevent mutation of the exposed key,
* you would want to make sure to always work on scoped copies, such as in this example:
*
* <pre>
* // clone "key" to protect it from unwanted modifications:
* try (DestroyableSecretKey k = key.clone()) {
* // copy "key" to protect it from unwanted modifications:
* try (DestroyableSecretKey k = key.copy()) {
* // use "k":
* Cipher cipher = Cipher.init(k, ...)
* cipher.doFinal(...)
* } // "k" will get destroyed here
* </pre>
*/
public class DestroyableSecretKey implements SecretKey, AutoCloseable, Cloneable {
public class DestroyableSecretKey implements SecretKey, AutoCloseable {

private transient final byte[] key;
private final String algorithm;
Expand Down Expand Up @@ -109,7 +109,7 @@ public String getFormat() {
* Returns the raw key bytes this instance wraps.
* <p>
* <b>Important:</b> Any change to the returned array will reflect in this key. Make sure to
* {@link #clone() make a local copy} if you can't rule out mutations.
* {@link #copy() make a local copy} if you can't rule out mutations.
*
* @return A byte array holding the secret key
*/
Expand All @@ -119,8 +119,11 @@ public byte[] getEncoded() {
return key;
}

@Override
public DestroyableSecretKey clone() {
/**
* Returns an independent copy of this key
* @return New copy of <code>this</code>
*/
public DestroyableSecretKey copy() {
Preconditions.checkState(!destroyed, "Key has been destroyed");
return new DestroyableSecretKey(key, algorithm); // key will get copied by the constructor as per contract
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public void decryptChunk(ByteBuffer ciphertextChunk, ByteBuffer cleartextChunk,

// visible for testing
void encryptChunk(ByteBuffer cleartextChunk, ByteBuffer ciphertextChunk, long chunkNumber, byte[] headerNonce, DestroyableSecretKey fileKey) {
try (DestroyableSecretKey fk = fileKey.clone()) {
try (DestroyableSecretKey fk = fileKey.copy()) {
// nonce:
byte[] nonce = new byte[NONCE_SIZE];
random.nextBytes(nonce);
Expand Down Expand Up @@ -131,7 +131,7 @@ void encryptChunk(ByteBuffer cleartextChunk, ByteBuffer ciphertextChunk, long ch
void decryptChunk(ByteBuffer ciphertextChunk, ByteBuffer cleartextChunk, DestroyableSecretKey fileKey) {
assert ciphertextChunk.remaining() >= NONCE_SIZE + MAC_SIZE;

try (DestroyableSecretKey fk = fileKey.clone()) {
try (DestroyableSecretKey fk = fileKey.copy()) {
// nonce:
final byte[] nonce = new byte[NONCE_SIZE];
final ByteBuffer chunkNonceBuf = ciphertextChunk.asReadOnlyBuffer();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public void decryptChunk(ByteBuffer ciphertextChunk, ByteBuffer cleartextChunk,

// visible for testing
void encryptChunk(ByteBuffer cleartextChunk, ByteBuffer ciphertextChunk, long chunkNumber, byte[] headerNonce, DestroyableSecretKey fileKey) {
try (DestroyableSecretKey fk = fileKey.clone()) {
try (DestroyableSecretKey fk = fileKey.copy()) {
// nonce:
byte[] nonce = new byte[GCM_NONCE_SIZE];
random.nextBytes(nonce);
Expand All @@ -121,7 +121,7 @@ void encryptChunk(ByteBuffer cleartextChunk, ByteBuffer ciphertextChunk, long ch
void decryptChunk(ByteBuffer ciphertextChunk, ByteBuffer cleartextChunk, long chunkNumber, byte[] headerNonce, DestroyableSecretKey fileKey) throws AuthenticationFailedException {
assert ciphertextChunk.remaining() >= GCM_NONCE_SIZE + GCM_TAG_SIZE;

try (DestroyableSecretKey fk = fileKey.clone()) {
try (DestroyableSecretKey fk = fileKey.copy()) {
// nonce:
final byte[] nonce = new byte[GCM_NONCE_SIZE];
final ByteBuffer chunkNonceBuf = ciphertextChunk.asReadOnlyBuffer();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public void testConstructorCreatesLocalCopy() {
}

@Test
public void testConstructorClonesKey() {
public void testConstructorCopiesKey() {
byte[] empty = new byte[32];
byte[] rawKey = new byte[32];
new Random(42).nextBytes(rawKey);
Expand Down Expand Up @@ -135,12 +135,12 @@ public void testGetEncoded() {
}

@Test
@DisplayName("clone() returns equal copy")
public void testClone() {
DestroyableSecretKey clone = key.clone();
@DisplayName("copy() returns equal copy")
public void testCopy() {
DestroyableSecretKey copy = key.copy();

Assertions.assertEquals(key, clone);
Assertions.assertNotSame(key, clone);
Assertions.assertEquals(key, copy);
Assertions.assertNotSame(key, copy);
}

@Test
Expand Down Expand Up @@ -194,9 +194,9 @@ public void testGetEncoded() {
}

@Test
@DisplayName("clone() throws IllegalStateException")
public void testClone() {
Assertions.assertThrows(IllegalStateException.class, key::clone);
@DisplayName("copy() throws IllegalStateException")
public void testCopy() {
Assertions.assertThrows(IllegalStateException.class, key::copy);
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,16 @@ public void testGetMacKey() {
}

@Test
public void testClone() {
public void testCopy() {
byte[] raw = new byte[64];
Arrays.fill(raw, (byte) 0x55);
Masterkey original = new Masterkey(raw);

Masterkey clone = original.clone();
Masterkey copy = original.copy();

Assertions.assertEquals(original, clone);
clone.destroy();
Assertions.assertNotEquals(original, clone);
Assertions.assertEquals(original, copy);
copy.destroy();
Assertions.assertNotEquals(original, copy);
}

}

0 comments on commit cad2d7d

Please sign in to comment.