From 9fe135ef0f3125adf955366540e4aa3e4d33b9ec Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 31 Dec 2014 01:21:08 +0100 Subject: [PATCH] - fixes #6, simplifies password verification - improves filename IV -> SIV using substring from sha256(secondaryKey + plaintextFilename). References #7 --- .../crypto/aes256/Aes256Cryptor.java | 99 ++++++++++++------- .../aes256/AesCryptographicConfiguration.java | 12 +-- .../org/cryptomator/crypto/aes256/Key.java | 16 +-- .../crypto/aes256/Aes256CryptorTest.java | 22 +++-- 4 files changed, 89 insertions(+), 60 deletions(-) diff --git a/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/Aes256Cryptor.java b/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/Aes256Cryptor.java index e9fecddc30..06f6a55415 100644 --- a/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/Aes256Cryptor.java +++ b/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/Aes256Cryptor.java @@ -19,6 +19,7 @@ import java.nio.file.Path; import java.security.InvalidAlgorithmParameterException; import java.security.InvalidKeyException; +import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; import java.security.spec.InvalidKeySpecException; @@ -30,6 +31,7 @@ import java.util.UUID; import java.util.zip.CRC32; +import javax.crypto.AEADBadTagException; import javax.crypto.BadPaddingException; import javax.crypto.Cipher; import javax.crypto.CipherInputStream; @@ -38,12 +40,14 @@ import javax.crypto.NoSuchPaddingException; import javax.crypto.SecretKey; import javax.crypto.SecretKeyFactory; +import javax.crypto.spec.GCMParameterSpec; import javax.crypto.spec.IvParameterSpec; import javax.crypto.spec.PBEKeySpec; import javax.crypto.spec.SecretKeySpec; import org.apache.commons.io.Charsets; import org.apache.commons.io.IOUtils; +import org.apache.commons.lang3.ArrayUtils; import org.apache.commons.lang3.StringUtils; import org.cryptomator.crypto.AbstractCryptor; import org.cryptomator.crypto.CryptorIOSupport; @@ -90,6 +94,11 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo */ private final byte[] masterKey = new byte[MASTER_KEY_LENGTH]; + /** + * If certain cryptographic operations need a second key, which is distinct to the masterKey + */ + private final byte[] secondaryKey = new byte[MASTER_KEY_LENGTH]; + private static final int SIZE_OF_LONG = Long.BYTES; static { @@ -109,6 +118,7 @@ public class Aes256Cryptor extends AbstractCryptor implements AesCryptographicCo public Aes256Cryptor() { SECURE_PRNG.setSeed(SECURE_PRNG.generateSeed(PRNG_SEED_LENGTH)); SECURE_PRNG.nextBytes(this.masterKey); + SECURE_PRNG.nextBytes(this.secondaryKey); } /** @@ -119,6 +129,7 @@ public Aes256Cryptor() { */ Aes256Cryptor(Random prng) { prng.nextBytes(this.masterKey); + prng.nextBytes(this.secondaryKey); } /** @@ -126,6 +137,9 @@ public Aes256Cryptor() { */ @Override public void encryptMasterKey(OutputStream out, CharSequence password) throws IOException { + final ByteBuffer combinedKey = ByteBuffer.allocate(this.masterKey.length + this.secondaryKey.length); + combinedKey.put(this.masterKey); + combinedKey.put(this.secondaryKey); try { // derive key: final byte[] userSalt = randomData(SALT_LENGTH); @@ -133,21 +147,21 @@ public void encryptMasterKey(OutputStream out, CharSequence password) throws IOE // encrypt: final byte[] iv = randomData(AES_BLOCK_LENGTH); - final Cipher encCipher = this.cipher(MASTERKEY_CIPHER, userKey, iv, Cipher.ENCRYPT_MODE); - byte[] encryptedUserKey = encCipher.doFinal(userKey.getEncoded()); - byte[] encryptedMasterKey = encCipher.doFinal(this.masterKey); + final Cipher encCipher = aesGcmCipher(userKey, iv, Cipher.ENCRYPT_MODE); + byte[] encryptedCombinedKey = encCipher.doFinal(combinedKey.array()); // save encrypted masterkey: final Key key = new Key(); key.setIterations(PBKDF2_PW_ITERATIONS); key.setIv(iv); key.setKeyLength(AES_KEY_LENGTH); - key.setMasterkey(encryptedMasterKey); + key.setMasterkey(encryptedCombinedKey); key.setSalt(userSalt); - key.setPwVerification(encryptedUserKey); objectMapper.writeValue(out, key); } catch (IllegalBlockSizeException | BadPaddingException ex) { - throw new IllegalStateException("Block size hard coded. Padding irrelevant in ENCRYPT_MODE. IV must exist in CBC mode.", ex); + throw new IllegalStateException("Block size hard coded. Padding irrelevant in ENCRYPT_MODE. IV must exist in CTR mode.", ex); + } finally { + Arrays.fill(combinedKey.array(), (byte) 0); } } @@ -162,7 +176,7 @@ public void encryptMasterKey(OutputStream out, CharSequence password) throws IOE */ @Override public void decryptMasterKey(InputStream in, CharSequence password) throws DecryptFailedException, WrongPasswordException, UnsupportedKeyLengthException, IOException { - byte[] decrypted = new byte[0]; + byte[] combinedKey = new byte[0]; try { // load encrypted masterkey: final Key key = objectMapper.readValue(in, Key.class); @@ -176,26 +190,22 @@ public void decryptMasterKey(InputStream in, CharSequence password) throws Decry // derive key: final SecretKey userKey = pbkdf2(password, key.getSalt(), key.getIterations(), key.getKeyLength()); - // check password: - final Cipher encCipher = this.cipher(MASTERKEY_CIPHER, userKey, key.getIv(), Cipher.ENCRYPT_MODE); - byte[] encryptedUserKey = encCipher.doFinal(userKey.getEncoded()); - if (!Arrays.equals(key.getPwVerification(), encryptedUserKey)) { - throw new WrongPasswordException(); - } - - // decrypt: - final Cipher decCipher = this.cipher(MASTERKEY_CIPHER, userKey, key.getIv(), Cipher.DECRYPT_MODE); - decrypted = decCipher.doFinal(key.getMasterkey()); + // decrypt and check password by catching AEAD exception + final Cipher decCipher = aesGcmCipher(userKey, key.getIv(), Cipher.DECRYPT_MODE); + combinedKey = decCipher.doFinal(key.getMasterkey()); - // everything ok, move decrypted data to masterkey: - final ByteBuffer masterKeyBuffer = ByteBuffer.wrap(this.masterKey); - masterKeyBuffer.put(decrypted); + // everything ok, split decrypted data to masterkey and secondary key: + final ByteBuffer combinedKeyBuffer = ByteBuffer.wrap(combinedKey); + combinedKeyBuffer.get(this.masterKey); + combinedKeyBuffer.get(this.secondaryKey); + } catch (AEADBadTagException e) { + throw new WrongPasswordException(); } catch (IllegalBlockSizeException | BadPaddingException | BufferOverflowException ex) { throw new DecryptFailedException(ex); } catch (NoSuchAlgorithmException ex) { throw new IllegalStateException("Algorithm should exist.", ex); } finally { - Arrays.fill(decrypted, (byte) 0); + Arrays.fill(combinedKey, (byte) 0); } } @@ -206,11 +216,24 @@ public void decryptMasterKey(InputStream in, CharSequence password) throws Decry @Override public void swipeSensitiveDataInternal() { Arrays.fill(this.masterKey, (byte) 0); + Arrays.fill(this.secondaryKey, (byte) 0); } - private Cipher cipher(String cipherTransformation, SecretKey key, byte[] iv, int cipherMode) { + private Cipher aesGcmCipher(SecretKey key, byte[] iv, int cipherMode) { try { - final Cipher cipher = Cipher.getInstance(cipherTransformation); + final Cipher cipher = Cipher.getInstance(AES_GCM_CIPHER); + cipher.init(cipherMode, key, new GCMParameterSpec(AES_GCM_TAG_LENGTH, iv)); + return cipher; + } catch (InvalidKeyException ex) { + throw new IllegalArgumentException("Invalid key.", ex); + } catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidAlgorithmParameterException ex) { + throw new IllegalStateException("Algorithm/Padding should exist and accept GCM specs.", ex); + } + } + + private Cipher aesCtrCipher(SecretKey key, byte[] iv, int cipherMode) { + try { + final Cipher cipher = Cipher.getInstance(AES_CTR_CIPHER); cipher.init(cipherMode, key, new IvParameterSpec(iv)); return cipher; } catch (InvalidKeyException ex) { @@ -268,6 +291,15 @@ private long crc32Sum(byte[] source) { return crc32.getValue(); } + private byte[] sha256(byte[] data) { + try { + final MessageDigest md = MessageDigest.getInstance("SHA-256"); + return md.digest(data); + } catch (NoSuchAlgorithmException e) { + throw new AssertionError("Every implementation of the Java platform is required to support SHA-256.", e); + } + } + @Override public String encryptPath(String cleartextPath, char encryptedPathSep, char cleartextPathSep, CryptorIOSupport ioSupport) { try { @@ -300,13 +332,14 @@ public String encryptPath(String cleartextPath, char encryptedPathSep, char clea * {@link FileNamingConventions#LONG_NAME_FILE_EXT}. */ private String encryptPathComponent(final String cleartext, final SecretKey key, CryptorIOSupport ioSupport) throws IllegalBlockSizeException, BadPaddingException, IOException { - final long cleartextHash = crc32Sum(cleartext.getBytes()); + final byte[] mac = sha256(ArrayUtils.addAll(secondaryKey, cleartext.getBytes())); + final byte[] partialIv = ArrayUtils.subarray(mac, 0, 10); final ByteBuffer iv = ByteBuffer.allocate(AES_BLOCK_LENGTH); - iv.putLong(cleartextHash); - final Cipher cipher = this.cipher(FILE_NAME_CIPHER, key, iv.array(), Cipher.ENCRYPT_MODE); + iv.put(partialIv); + final Cipher cipher = this.aesCtrCipher(key, iv.array(), Cipher.ENCRYPT_MODE); final byte[] cleartextBytes = cleartext.getBytes(Charsets.UTF_8); final byte[] encryptedBytes = cipher.doFinal(cleartextBytes); - final String ivAndCiphertext = Long.toHexString(cleartextHash) + IV_PREFIX_SEPARATOR + ENCRYPTED_FILENAME_CODEC.encodeAsString(encryptedBytes); + final String ivAndCiphertext = ENCRYPTED_FILENAME_CODEC.encodeAsString(partialIv) + IV_PREFIX_SEPARATOR + ENCRYPTED_FILENAME_CODEC.encodeAsString(encryptedBytes); if (ivAndCiphertext.length() + BASIC_FILE_EXT.length() > ENCRYPTED_FILENAME_LENGTH_LIMIT) { final String crc32 = Long.toHexString(crc32Sum(ivAndCiphertext.getBytes())); @@ -354,12 +387,12 @@ private String decryptPathComponent(final String encrypted, final SecretKey key, throw new IllegalArgumentException("Unsupported path component: " + encrypted); } - final String cleartextHash = StringUtils.substringBefore(ivAndCiphertext, IV_PREFIX_SEPARATOR); + final String partialIvStr = StringUtils.substringBefore(ivAndCiphertext, IV_PREFIX_SEPARATOR); final String ciphertext = StringUtils.substringAfter(ivAndCiphertext, IV_PREFIX_SEPARATOR); final ByteBuffer iv = ByteBuffer.allocate(AES_BLOCK_LENGTH); - iv.putLong(Long.parseLong(cleartextHash, 16)); + iv.put(ENCRYPTED_FILENAME_CODEC.decode(partialIvStr)); - final Cipher cipher = this.cipher(FILE_NAME_CIPHER, key, iv.array(), Cipher.DECRYPT_MODE); + final Cipher cipher = this.aesCtrCipher(key, iv.array(), Cipher.DECRYPT_MODE); final byte[] encryptedBytes = ENCRYPTED_FILENAME_CODEC.decode(ciphertext); final byte[] cleartextBytes = cipher.doFinal(encryptedBytes); return new String(cleartextBytes, Charsets.UTF_8); @@ -403,7 +436,7 @@ public Long decryptedFile(SeekableByteChannel encryptedFile, OutputStream plaint // derive secret key and generate cipher: final SecretKey key = this.deriveSecretKeyFromMasterKey(); - final Cipher cipher = this.cipher(FILE_CONTENT_CIPHER, key, countingIv.array(), Cipher.DECRYPT_MODE); + final Cipher cipher = this.aesCtrCipher(key, countingIv.array(), Cipher.DECRYPT_MODE); // read content final InputStream in = new SeekableByteChannelInputStream(encryptedFile); @@ -434,7 +467,7 @@ public Long decryptRange(SeekableByteChannel encryptedFile, OutputStream plainte // derive secret key and generate cipher: final SecretKey key = this.deriveSecretKeyFromMasterKey(); - final Cipher cipher = this.cipher(FILE_CONTENT_CIPHER, key, countingIv.array(), Cipher.DECRYPT_MODE); + final Cipher cipher = this.aesCtrCipher(key, countingIv.array(), Cipher.DECRYPT_MODE); // read content final InputStream in = new SeekableByteChannelInputStream(encryptedFile); @@ -454,7 +487,7 @@ public Long encryptFile(InputStream plaintextFile, SeekableByteChannel encrypted // derive secret key and generate cipher: final SecretKey key = this.deriveSecretKeyFromMasterKey(); - final Cipher cipher = this.cipher(FILE_CONTENT_CIPHER, key, countingIv.array(), Cipher.ENCRYPT_MODE); + final Cipher cipher = this.aesCtrCipher(key, countingIv.array(), Cipher.ENCRYPT_MODE); // 8 bytes (file size: temporarily -1): final ByteBuffer fileSize = ByteBuffer.allocate(SIZE_OF_LONG); diff --git a/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/AesCryptographicConfiguration.java b/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/AesCryptographicConfiguration.java index 4422d04b98..44d21ef327 100644 --- a/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/AesCryptographicConfiguration.java +++ b/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/AesCryptographicConfiguration.java @@ -55,21 +55,19 @@ interface AesCryptographicConfiguration { * * @see http://docs.oracle.com/javase/7/docs/technotes/guides/security/StandardNames.html#Cipher */ - String MASTERKEY_CIPHER = "AES/CTR/NoPadding"; + String AES_GCM_CIPHER = "AES/GCM/NoPadding"; /** - * Cipher specs for file name encryption. - * - * @see http://docs.oracle.com/javase/7/docs/technotes/guides/security/StandardNames.html#Cipher + * Length of authentication tag. */ - String FILE_NAME_CIPHER = "AES/CTR/NoPadding"; + int AES_GCM_TAG_LENGTH = 128; /** - * Cipher specs for content encryption. Using CTR-mode for random access. + * Cipher specs for file name and file content encryption. Using CTR-mode for random access. * * @see http://docs.oracle.com/javase/7/docs/technotes/guides/security/StandardNames.html#Cipher */ - String FILE_CONTENT_CIPHER = "AES/CTR/NoPadding"; + String AES_CTR_CIPHER = "AES/CTR/NoPadding"; /** * AES block size is 128 bit or 16 bytes. diff --git a/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/Key.java b/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/Key.java index c14bb7c7ea..65dce8d1e7 100644 --- a/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/Key.java +++ b/main/crypto-aes/src/main/java/org/cryptomator/crypto/aes256/Key.java @@ -4,7 +4,7 @@ import com.fasterxml.jackson.annotation.JsonPropertyOrder; -@JsonPropertyOrder(value = { "salt", "iv", "iterations", "keyLength", "masterkey" }) +@JsonPropertyOrder(value = {"salt", "iv", "iterations", "keyLength", "masterkey", "secondaryKey"}) public class Key implements Serializable { private static final long serialVersionUID = 8578363158959619885L; @@ -12,13 +12,12 @@ public class Key implements Serializable { private byte[] iv; private int iterations; private int keyLength; - private byte[] pwVerification; private byte[] masterkey; - + public byte[] getSalt() { return salt; } - + public void setSalt(byte[] salt) { this.salt = salt; } @@ -47,14 +46,6 @@ public void setKeyLength(int keyLength) { this.keyLength = keyLength; } - public byte[] getPwVerification() { - return pwVerification; - } - - public void setPwVerification(byte[] pwVerification) { - this.pwVerification = pwVerification; - } - public byte[] getMasterkey() { return masterkey; } @@ -63,5 +54,4 @@ public void setMasterkey(byte[] masterkey) { this.masterkey = masterkey; } - } \ No newline at end of file diff --git a/main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/Aes256CryptorTest.java b/main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/Aes256CryptorTest.java index 0c71c6382a..163306b5da 100644 --- a/main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/Aes256CryptorTest.java +++ b/main/crypto-aes/src/test/java/org/cryptomator/crypto/aes256/Aes256CryptorTest.java @@ -47,21 +47,29 @@ public void testCorrectPassword() throws IOException, WrongPasswordException, De IOUtils.closeQuietly(in); } - @Test(expected = WrongPasswordException.class) + @Test public void testWrongPassword() throws IOException, DecryptFailedException, WrongPasswordException, UnsupportedKeyLengthException { final String pw = "asd"; final Aes256Cryptor cryptor = new Aes256Cryptor(TEST_PRNG); final ByteArrayOutputStream out = new ByteArrayOutputStream(); cryptor.encryptMasterKey(out, pw); cryptor.swipeSensitiveData(); + IOUtils.closeQuietly(out); - final String wrongPw = "foo"; + // all these passwords are expected to fail. + final String[] wrongPws = {"a", "as", "asdf", "sdf", "das", "dsa", "foo", "bar", "baz"}; final Aes256Cryptor decryptor = new Aes256Cryptor(TEST_PRNG); - final InputStream in = new ByteArrayInputStream(out.toByteArray()); - decryptor.decryptMasterKey(in, wrongPw); - - IOUtils.closeQuietly(out); - IOUtils.closeQuietly(in); + for (final String wrongPw : wrongPws) { + final InputStream in = new ByteArrayInputStream(out.toByteArray()); + try { + decryptor.decryptMasterKey(in, wrongPw); + Assert.fail("should not succeed."); + } catch (WrongPasswordException e) { + continue; + } finally { + IOUtils.closeQuietly(in); + } + } } @Test