Skip to content

Commit

Permalink
fixed several code smells
Browse files Browse the repository at this point in the history
  • Loading branch information
overheadhunter committed Aug 18, 2021
1 parent ba0755c commit 3347bc0
Show file tree
Hide file tree
Showing 16 changed files with 76 additions and 153 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import org.cryptomator.cryptolib.api.AuthenticationFailedException;
import org.cryptomator.cryptolib.api.Cryptor;
import org.cryptomator.cryptolib.api.FileHeader;
import org.cryptomator.cryptolib.common.ByteBuffers;

import java.io.EOFException;
import java.io.IOException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
*/
public class DestroyableSecretKey implements SecretKey, AutoCloseable, Cloneable {

private transient final byte[] key;
private final transient byte[] key;
private final String algorithm;
private boolean destroyed;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import org.cryptomator.cryptolib.api.Cryptor;
import org.cryptomator.cryptolib.api.FileHeader;
import org.cryptomator.cryptolib.common.ByteBuffers;

import java.io.IOException;
import java.nio.ByteBuffer;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,13 @@
*******************************************************************************/
package org.cryptomator.cryptolib.common;

import org.cryptomator.cryptolib.api.Cryptor;
import org.cryptomator.cryptolib.api.FileHeader;

import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.channels.WritableByteChannel;

import org.cryptomator.cryptolib.api.Cryptor;
import org.cryptomator.cryptolib.api.FileHeader;
import org.cryptomator.cryptolib.common.ByteBuffers;

public class EncryptingWritableByteChannel implements WritableByteChannel {

private final WritableByteChannel delegate;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ protected ReseedingSecureRandomProvider() {

private static class ReseedingSecureRandomSpi extends SecureRandomSpi {

private final SecureRandom seeder, csprng;
private final SecureRandom seeder;
private final SecureRandom csprng;
private final long reseedAfter;
private final int seedLength;
private long counter;
Expand Down
34 changes: 18 additions & 16 deletions src/main/java/org/cryptomator/cryptolib/common/Scrypt.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,17 @@ public class Scrypt {

private static final int P = 1; // scrypt parallelization parameter

private Scrypt() {
}

/**
* Derives a key from the given passphrase.
* This implementation makes sure, any copies of the passphrase used during key derivation are overwritten in memory asap (before next GC cycle).
*
* @param passphrase The passphrase, whose characters will get UTF-8 encoded during key derivation.
* @param salt Salt, ideally randomly generated
* @param costParam Cost parameter <code>N</code>, larger than 1, a power of 2 and less than <code>2^(128 * blockSize / 8)</code>
* @param blockSize Block size <code>r</code>
*
* @param passphrase The passphrase, whose characters will get UTF-8 encoded during key derivation.
* @param salt Salt, ideally randomly generated
* @param costParam Cost parameter <code>N</code>, larger than 1, a power of 2 and less than <code>2^(128 * blockSize / 8)</code>
* @param blockSize Block size <code>r</code>
* @param keyLengthInBytes Key output length <code>dkLen</code>
* @return Derived key
* @see <a href="https://tools.ietf.org/html/rfc7914#section-2">RFC 7914</a>
Expand All @@ -37,7 +40,6 @@ public static byte[] scrypt(CharSequence passphrase, byte[] salt, int costParam,
final byte[] pw = new byte[buf.remaining()];
buf.get(pw);
try {
// return SCrypt.generate(pw, salt, costParam, blockSize, 1, keyLengthInBytes);
return scrypt(pw, salt, costParam, blockSize, keyLengthInBytes);
} finally {
Arrays.fill(pw, (byte) 0); // overwrite bytes
Expand All @@ -49,15 +51,15 @@ public static byte[] scrypt(CharSequence passphrase, byte[] salt, int costParam,
/**
* Derives a key from the given passphrase.
* This implementation makes sure, any copies of the passphrase used during key derivation are overwritten in memory asap (before next GC cycle).
*
* @param passphrase The passphrase,
* @param salt Salt, ideally randomly generated
* @param costParam Cost parameter <code>N</code>, larger than 1, a power of 2 and less than <code>2^(128 * blockSize / 8)</code>
* @param blockSize Block size <code>r</code>
*
* @param passphrase The passphrase,
* @param salt Salt, ideally randomly generated
* @param costParam Cost parameter <code>N</code>, larger than 1, a power of 2 and less than <code>2^(128 * blockSize / 8)</code>
* @param blockSize Block size <code>r</code>
* @param keyLengthInBytes Key output length <code>dkLen</code>
* @return Derived key
* @see <a href="https://tools.ietf.org/html/rfc7914#section-2">RFC 7914</a>
* @author Derived from com.lambdaworks.crypto.SCrypt, Apache License 2.0, Copyright (C) 2011 - Will Glozer
* @see <a href="https://tools.ietf.org/html/rfc7914#section-2">RFC 7914</a>
*/
public static byte[] scrypt(byte[] passphrase, byte[] salt, int costParam, int blockSize, int keyLengthInBytes) {
if (costParam < 2 || (costParam & (costParam - 1)) != 0) {
Expand Down Expand Up @@ -93,10 +95,10 @@ public static byte[] scrypt(byte[] passphrase, byte[] salt, int costParam, int b
/**
* Implementation of PBKDF2 (RFC2898).
*
* @param mac Pre-initialized {@link Mac} instance to use.
* @param S Salt.
* @param c Iteration count.
* @param DK Byte array that derived key will be placed in.
* @param mac Pre-initialized {@link Mac} instance to use.
* @param S Salt.
* @param c Iteration count.
* @param DK Byte array that derived key will be placed in.
* @param dkLen Intended length, in octets, of the derived key.
* @author Derived from com.lambdaworks.crypto.PBKDF, Apache License 2.0, Copyright (C) 2011 - Will Glozer
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

import javax.security.auth.Destroyable;
import java.nio.ByteBuffer;
import java.util.Arrays;

class FileHeaderImpl implements FileHeader, Destroyable {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class FileNameCryptorImpl implements FileNameCryptor {
@Override
protected SivMode initialValue() {
return new SivMode();
};
}
};

private final Masterkey masterkey;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

import javax.security.auth.Destroyable;
import java.nio.ByteBuffer;
import java.util.Arrays;

class FileHeaderImpl implements FileHeader, Destroyable {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class FileNameCryptorImpl implements FileNameCryptor {
@Override
protected SivMode initialValue() {
return new SivMode();
};
}
};

private final Masterkey masterkey;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.cryptomator.cryptolib.common;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

Expand All @@ -12,22 +13,28 @@ public class DestroyablesTest {
public void testDestroySilently() throws DestroyFailedException {
Destroyable destroyable = Mockito.mock(Destroyable.class);

Destroyables.destroySilently(destroyable);
Assertions.assertDoesNotThrow(() -> {
Destroyables.destroySilently(destroyable);
});

Mockito.verify(destroyable).destroy();
}

@Test
public void testDestroySilentlyIgnoresNull() {
Destroyables.destroySilently(null);
Assertions.assertDoesNotThrow(() -> {
Destroyables.destroySilently(null);
});
}

@Test
public void testDestroySilentlySuppressesException() throws DestroyFailedException {
Destroyable destroyable = Mockito.mock(Destroyable.class);
Mockito.doThrow(new DestroyFailedException()).when(destroyable).destroy();

Destroyables.destroySilently(destroyable);
Assertions.assertDoesNotThrow(() -> {
Destroyables.destroySilently(destroyable);
});

Mockito.verify(destroyable).destroy();
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

public class BenchmarkTest {

@Disabled
@Disabled("only on demand")
@Test
public void runBenchmarks() throws RunnerException {
// Taken from http://stackoverflow.com/a/30486197/4014509:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.Mockito;

Expand Down Expand Up @@ -199,71 +200,31 @@ public void testDecryptionWithTooShortHeader() throws InterruptedException, IOEx
}
}

@Test
@DisplayName("decrypt chunk with unauthentic NONCE")
public void testChunkDecryptionWithUnauthenticNonce() {
ByteBuffer ciphertext = ByteBuffer.wrap(BaseEncoding.base64().decode("aAAAAAAAAAAAAAAAAAAAALTwrBTNYP7m3yTGKlhka9WPvX1Lpn5EYfVxlyX1ISgRXtdRnivM7r6F3Og="));
@DisplayName("decrypt unauthentic chunk")
@ParameterizedTest(name = "unauthentic {1}")
@CsvSource(value = {
"aAAAAAAAAAAAAAAAAAAAALTwrBTNYP7m3yTGKlhka9WPvX1Lpn5EYfVxlyX1ISgRXtdRnivM7r6F3Og=, NONCE",
"AAAAAAAAAAAAAAAAAAAAALTwrBTNYP7m3YTGKlhka9WPvX1Lpn5EYfVxlyX1ISgRXtdRnivM7r6F3Og=, CONTENT",
"AAAAAAAAAAAAAAAAAAAAALTwrBTNYP7m3yTGKlhka9WPvX1Lpn5EYfVxlyX1ISgRXtdRnivM7r6F3OG=, MAC",
})
public void testUnauthenticChunkDecryption(String chunkData, String ignored) {
ByteBuffer ciphertext = ByteBuffer.wrap(BaseEncoding.base64().decode(chunkData));

Assertions.assertThrows(AuthenticationFailedException.class, () -> {
fileContentCryptor.decryptChunk(ciphertext, 0, header, true);
});
}

@Test
@DisplayName("decrypt file with unauthentic NONCE in first chunk")
public void testDecryptionWithUnauthenticNonce() throws InterruptedException, IOException {
byte[] ciphertext = BaseEncoding.base64().decode("AAAAAAAAAAAAAAAAAAAAANyVwHiiQImCrUiiFJKEIIdTD4r7x0U2ualjtPHEy3OLzqdAPU1ga27XjlTjFxC1VCqZa+" //
+ "L2eH+xWVgrSLX+JkG35ZJxk5xXswAAAAAAAAAAABAAAAAAAAC08KwUzWD+5t8kxipYZGvVj719S6Z+RGH1cZcl9SEoEV7XUZ4rzO6+hdzo");
ReadableByteChannel ciphertextCh = Channels.newChannel(new ByteArrayInputStream(ciphertext));

try (ReadableByteChannel cleartextCh = new DecryptingReadableByteChannel(ciphertextCh, cryptor, true)) {
IOException thrown = Assertions.assertThrows(IOException.class, () -> {
cleartextCh.read(ByteBuffer.allocate(3));
});
MatcherAssert.assertThat(thrown.getCause(), CoreMatchers.instanceOf(AuthenticationFailedException.class));
}
}

@Test
@DisplayName("decrypt chunk with unauthentic CONTENT")
public void testChunkDecryptionWithUnauthenticContent() {
ByteBuffer ciphertext = ByteBuffer.wrap(BaseEncoding.base64().decode("AAAAAAAAAAAAAAAAAAAAALTwrBTNYP7m3YTGKlhka9WPvX1Lpn5EYfVxlyX1ISgRXtdRnivM7r6F3Og="));

Assertions.assertThrows(AuthenticationFailedException.class, () -> {
fileContentCryptor.decryptChunk(ciphertext, 0, header, true);
});
}

@Test
@DisplayName("decrypt file with unauthentic CONTENT in first chunk")
public void testDecryptionWithUnauthenticContent() throws InterruptedException, IOException {
byte[] ciphertext = BaseEncoding.base64().decode("AAAAAAAAAAAAAAAAAAAAANyVwHiiQImCrUiiFJKEIIdTD4r7x0U2ualjtPHEy3OLzqdAPU1ga27XjlTjFxC1VCqZa+" //
+ "L2eH+xWVgrSLX+JkG35ZJxk5xXswAAAAAAAAAAAAAAAAAAAAC08KwUZWD+5t8kxipYZGvVj719S6Z+RGH1cZcl9SEoEV7XUZ4rzO6+hdzo");
ReadableByteChannel ciphertextCh = Channels.newChannel(new ByteArrayInputStream(ciphertext));

try (ReadableByteChannel cleartextCh = new DecryptingReadableByteChannel(ciphertextCh, cryptor, true)) {
IOException thrown = Assertions.assertThrows(IOException.class, () -> {
cleartextCh.read(ByteBuffer.allocate(3));
});
MatcherAssert.assertThat(thrown.getCause(), CoreMatchers.instanceOf(AuthenticationFailedException.class));
}
}

@Test
@DisplayName("decrypt chunk with unauthentic MAC")
public void testChunkDecryptionWithUnauthenticMac() {
ByteBuffer ciphertext = ByteBuffer.wrap(BaseEncoding.base64().decode("AAAAAAAAAAAAAAAAAAAAALTwrBTNYP7m3yTGKlhka9WPvX1Lpn5EYfVxlyX1ISgRXtdRnivM7r6F3OG="));

Assertions.assertThrows(AuthenticationFailedException.class, () -> {
fileContentCryptor.decryptChunk(ciphertext, 0, header, true);
});
}
@DisplayName("decrypt unauthentic file")
@ParameterizedTest(name = "unauthentic {1} in first chunk")
@CsvSource(value = {
"AAAAAAAAAAAAAAAAAAAAANyVwHiiQImCrUiiFJKEIIdTD4r7x0U2ualjtPHEy3OLzqdAPU1ga27XjlTjFxC1VCqZa+L2eH+xWVgrSLX+JkG35ZJxk5xXswAAAAAAAAAAABAAAAAAAAC08KwUzWD+5t8kxipYZGvVj719S6Z+RGH1cZcl9SEoEV7XUZ4rzO6+hdzo, NONCE",
"AAAAAAAAAAAAAAAAAAAAANyVwHiiQImCrUiiFJKEIIdTD4r7x0U2ualjtPHEy3OLzqdAPU1ga27XjlTjFxC1VCqZa+L2eH+xWVgrSLX+JkG35ZJxk5xXswAAAAAAAAAAAAAAAAAAAAC08KwUZWD+5t8kxipYZGvVj719S6Z+RGH1cZcl9SEoEV7XUZ4rzO6+hdzo, CONTENT",
"AAAAAAAAAAAAAAAAAAAAANyVwHiiQImCrUiiFJKEIIdTD4r7x0U2ualjtPHEy3OLzqdAPU1ga27XjlTjFxC1VCqZa+L2eH+xWVgrSLX+JkG35ZJxk5xXswAAAAAAAAAAAAAAAAAAAAC08KwUzWD+5t8kxipYZGvVj719S6Z+RGH1cZcl9SEoEV7XUZ4rzO6+hdzO, MAC",
})
public void testDecryptionWithUnauthenticFirstChunk(String fileData, String ignored) throws IOException {
byte[] ciphertext = BaseEncoding.base64().decode(fileData);

@Test
@DisplayName("decrypt file with unauthentic MAC in first chunk")
public void testDecryptionWithUnauthenticMac() throws InterruptedException, IOException {
byte[] ciphertext = BaseEncoding.base64().decode("AAAAAAAAAAAAAAAAAAAAANyVwHiiQImCrUiiFJKEIIdTD4r7x0U2ualjtPHEy3OLzqdAPU1ga27XjlTjFxC1VCqZa+" //
+ "L2eH+xWVgrSLX+JkG35ZJxk5xXswAAAAAAAAAAAAAAAAAAAAC08KwUzWD+5t8kxipYZGvVj719S6Z+RGH1cZcl9SEoEV7XUZ4rzO6+hdzO");
ReadableByteChannel ciphertextCh = Channels.newChannel(new ByteArrayInputStream(ciphertext));

try (ReadableByteChannel cleartextCh = new DecryptingReadableByteChannel(ciphertextCh, cryptor, true)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

public class BenchmarkTest {

@Disabled
@Disabled("only on demand")
@Test
public void runBenchmarks() throws RunnerException {
// Taken from http://stackoverflow.com/a/30486197/4014509:
Expand Down
Loading

0 comments on commit 3347bc0

Please sign in to comment.