From 112de8be22743116eee18a0cba9a40c19f7361c2 Mon Sep 17 00:00:00 2001 From: alutman Date: Wed, 12 Jul 2017 19:54:08 +1000 Subject: [PATCH] Improve encryption methods --- README.md | 11 +- src/main/Program.java | 2 +- src/model/ByteData.java | 8 +- src/model/CBCCryptographer.java | 166 ++++++++++++------ src/model/Cryptographer.java | 7 +- src/model/CryptographerHandler.java | 17 +- src/model/enums/CryptStatus.java | 3 +- .../exception/DecryptFailedException.java | 18 ++ src/model/exception/InvalidDataException.java | 10 ++ .../exception/InvalidEncryptionException.java | 10 -- src/model/exception/InvalidHMacException.java | 11 ++ test/model/CBCCryptographerTest.java | 20 ++- 12 files changed, 195 insertions(+), 88 deletions(-) create mode 100644 src/model/exception/DecryptFailedException.java create mode 100644 src/model/exception/InvalidDataException.java delete mode 100644 src/model/exception/InvalidEncryptionException.java create mode 100644 src/model/exception/InvalidHMacException.java diff --git a/README.md b/README.md index 1ec4be8..49fa4ab 100644 --- a/README.md +++ b/README.md @@ -17,8 +17,15 @@ Details * Raw text is handled in UTF-8 format * Text files when encrypted are by default encoded in base64 * Binary files when encrypted by default have no encoding - * Encryption uses AES, CBC with PKCS5Padding, salted with a hard-coded byte array - * Keys are hashed with PBKDF2WithHmacSHA1 + +### Encryption + * **Cipher**: `AES-128` + * **Block Mode**: `CBC` + * **Padding**: `PKCS5Padding` + * **HMAC**: `SHA256` + * **Key Derivation**: `PBKDF2WithHmacSHA1` + * **Salting**: `SHA1PRNG` + Sources ------- diff --git a/src/main/Program.java b/src/main/Program.java index d86dc9b..3a66aa7 100644 --- a/src/main/Program.java +++ b/src/main/Program.java @@ -23,7 +23,7 @@ * Start point */ public class Program { - public static final String VERSION = "1.10.1"; + public static final String VERSION = "1.12.0"; public static final String NAME = "AES Text Editor"; private static boolean verbose = false; diff --git a/src/model/ByteData.java b/src/model/ByteData.java index 18c87e5..75ad4ec 100644 --- a/src/model/ByteData.java +++ b/src/model/ByteData.java @@ -1,7 +1,7 @@ package model; import model.enums.FileStatus; -import model.exception.InvalidEncryptionException; +import model.exception.InvalidEncodingException; import com.sun.org.apache.xerces.internal.impl.dv.util.HexBin; import com.sun.org.apache.xml.internal.security.exceptions.Base64DecodingException; import com.sun.org.apache.xml.internal.security.utils.Base64; @@ -52,19 +52,19 @@ else if (encodingMode.equals(Encoding.NONE)) { //Don't bother encoding } } - public void decode() throws InvalidEncryptionException { + public void decode() throws InvalidEncodingException { if(encodingMode.equals(Encoding.BASE64)) { try { set(Base64.decode(text())); } catch(Base64DecodingException b64de) { - throw new InvalidEncryptionException(); + throw new InvalidEncodingException(); } } else if (encodingMode.equals(Encoding.HEX)) { byte[] b = HexBin.decode(text()); if (b == null) { - throw new InvalidEncryptionException(); + throw new InvalidEncodingException(); } set(HexBin.decode(text())); } diff --git a/src/model/CBCCryptographer.java b/src/model/CBCCryptographer.java index 1b73f4f..d871314 100644 --- a/src/model/CBCCryptographer.java +++ b/src/model/CBCCryptographer.java @@ -1,14 +1,18 @@ package model; +import model.exception.DecryptFailedException; +import model.exception.InvalidDataException; +import model.exception.InvalidHMacException; + import javax.crypto.*; import javax.crypto.spec.IvParameterSpec; import javax.crypto.spec.PBEKeySpec; import javax.crypto.spec.SecretKeySpec; -import java.security.InvalidAlgorithmParameterException; -import java.security.InvalidKeyException; -import java.security.NoSuchAlgorithmException; +import java.security.*; +import java.security.spec.InvalidKeySpecException; import java.security.spec.InvalidParameterSpecException; import java.security.spec.KeySpec; +import java.util.Arrays; /** * Created by alutman on 18/03/14. @@ -17,15 +21,19 @@ * */ public class CBCCryptographer implements Cryptographer{ - private final byte[] SALT = {(byte) 0xA9, (byte) 0x9B, (byte) 0xC8, (byte) 0x32, (byte) 0x56, (byte) 0x35, (byte) 0xE3, (byte) 0x03}; - private final int ITERATION_COUNT = 65536; - private final int KEY_LENGTH = 128; - private final int IV_LENGTH = 16; + private final int ITERATION_COUNT = 100000; + private final int BLOCK_SIZE = 128; + private final int KEY_LENGTH = BLOCK_SIZE; + private final int HMAC_LENGTH = 256; + private final int SALT_LENGTH = 160; + private Cipher cipher; - private final String ENCRYPTION_ALGORITHM = "AES"; + private final String ENCRYPTION_ALGORITHM = "AES"; //128 private final String MODE_ALGORITHM = "CBC"; + private final String RANDOM_ALGORITHM = "SHA1PRNG"; + private final String MAC_ALGORITHM = "HmacSHA256"; private final String PADDING_ALGORITHM = "PKCS5Padding"; - private final String HASH_ALGORITHM = "PBKDF2WithHmacSHA1"; + private final String KEY_DERIVATION_ALGORITHM = "PBKDF2WithHmacSHA1"; private final String CIPHER_ALGORITHM = ENCRYPTION_ALGORITHM+"/"+MODE_ALGORITHM+"/"+PADDING_ALGORITHM; public CBCCryptographer() { @@ -34,79 +42,123 @@ public CBCCryptographer() { } catch (NoSuchAlgorithmException | NoSuchPaddingException e) { //Hardcoded algorithm shouldn't ever throw this - e.printStackTrace(); - System.exit(1); + throw new RuntimeException(e); } } - - private SecretKey generateSecretKey(String key) { - try { - SecretKeyFactory secretKeyFactory = SecretKeyFactory.getInstance(HASH_ALGORITHM); - KeySpec keySpec = new PBEKeySpec(key.toCharArray(), SALT, ITERATION_COUNT, KEY_LENGTH); - SecretKey secretKeyTemp = secretKeyFactory.generateSecret(keySpec); - return new SecretKeySpec(secretKeyTemp.getEncoded(), "AES"); - } - catch (Exception e) { - e.printStackTrace(); - System.exit(1); - } - return null; + private byte[] deriveKey(String key, byte[] salt, int length) throws NoSuchAlgorithmException, InvalidKeySpecException { + PBEKeySpec ks = new PBEKeySpec(key.toCharArray(), salt, ITERATION_COUNT, length); + SecretKeyFactory skf = SecretKeyFactory.getInstance(KEY_DERIVATION_ALGORITHM); + return skf.generateSecret(ks).getEncoded(); } public String getMode() { return CIPHER_ALGORITHM; } + /* + Structure of encrypted data is + + hmacSalt + hmac + encSalt + iv + cipherText + */ + @Override public byte[] encrypt(byte[] key, byte[] data) { try { - SecretKey secretKey = generateSecretKey(new String(key)); - cipher.init(Cipher.ENCRYPT_MODE, secretKey); - byte[] iv = cipher.getParameters().getParameterSpec(IvParameterSpec.class).getIV(); + SecureRandom sr = SecureRandom.getInstance(RANDOM_ALGORITHM); - //encrypt data - byte[] encrypted = cipher.doFinal(data); - - //put iv in start of new array - byte[] cipherText = new byte[encrypted.length + iv.length]; - System.arraycopy(iv, 0, cipherText, 0, iv.length); + //generate key + byte[] encSalt = new byte[SALT_LENGTH / 8]; + sr.nextBytes(encSalt); + byte[] encKey = deriveKey(new String(key), encSalt, KEY_LENGTH); - //append encrypted data to new array, after iv - System.arraycopy(encrypted, 0, cipherText, iv.length, encrypted.length); + cipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(encKey, ENCRYPTION_ALGORITHM)); + byte[] iv = cipher.getParameters().getParameterSpec(IvParameterSpec.class).getIV(); - //Encode to B64 - return cipherText; + //encrypt data + byte[] cipherText = cipher.doFinal(data); + + // generate mac + byte[] hmacSalt = new byte[SALT_LENGTH / 8]; + sr.nextBytes(hmacSalt); + byte[] hmacKey = deriveKey(new String(key), hmacSalt, SALT_LENGTH); + Mac mac = Mac.getInstance(MAC_ALGORITHM); + mac.init(new SecretKeySpec(hmacKey, MAC_ALGORITHM)); + + // apply hmac + byte[] encrypted = new byte[iv.length + encSalt.length + cipherText.length]; + System.arraycopy(encSalt, 0, encrypted, 0, encSalt.length); + System.arraycopy(iv, 0, encrypted, encSalt.length, iv.length); + System.arraycopy(cipherText, 0, encrypted, encSalt.length+iv.length, cipherText.length); + + byte[] hmac = mac.doFinal(encrypted); + + //Copy everything together + byte[] output = new byte[hmacSalt.length + encrypted.length + hmac.length]; + System.arraycopy(hmacSalt, 0, output, 0, hmacSalt.length); + System.arraycopy(hmac, 0, output, hmacSalt.length, hmac.length); + System.arraycopy(encrypted, 0, output, hmacSalt.length+hmac.length, encrypted.length); + + return output; } - catch (IllegalBlockSizeException | BadPaddingException | InvalidParameterSpecException | InvalidKeyException e) { - //Shouldn't occur - e.printStackTrace(); - System.exit(1); + catch (IllegalBlockSizeException | BadPaddingException | InvalidParameterSpecException | InvalidKeyException + | NoSuchAlgorithmException | InvalidKeySpecException e) { + //Hardcoded algorithms and sizes shouldn't ever throw these + throw new RuntimeException(e); } - return null; } @Override - public byte[] decrypt(byte[] key, byte[] data) throws BadPaddingException, IllegalBlockSizeException { - SecretKey secretKey = generateSecretKey(new String(key)); + public byte[] decrypt(byte[] key, byte[] data) throws DecryptFailedException, InvalidDataException { + if(data.length < SALT_LENGTH/8+HMAC_LENGTH/8+SALT_LENGTH/8+BLOCK_SIZE/8) { + //Any encrypted data MUST have hmacSalt, hmac, encSalt & iv + throw new InvalidDataException("data not long enough"); + } try { - //pull out the iv from the encrypted data - byte[] iv = new byte[IV_LENGTH]; - System.arraycopy(data,0,iv,0,IV_LENGTH); - - //Initialize with extracted IV - cipher.init(Cipher.DECRYPT_MODE, secretKey, new IvParameterSpec(iv)); - - //Cut out data after IV - byte[] ciphertext = new byte[data.length - IV_LENGTH]; - System.arraycopy(data, IV_LENGTH, ciphertext, 0, ciphertext.length); - - //Decrypt - return cipher.doFinal(ciphertext); + int current = 0; + int encryptedStart = 0; + byte[] hmacSalt = Arrays.copyOfRange(data, current, SALT_LENGTH/8); + current += SALT_LENGTH/8; + byte[] hmac = Arrays.copyOfRange(data, current, current+HMAC_LENGTH/8); + current += HMAC_LENGTH/8; + encryptedStart = current; + byte[] encSalt = Arrays.copyOfRange(data, current, current+SALT_LENGTH/8); + current += SALT_LENGTH/8; + byte[] iv = Arrays.copyOfRange(data, current, current+BLOCK_SIZE/8); + current += BLOCK_SIZE/8; + byte[] cipherText = Arrays.copyOfRange(data, current, data.length); + + + byte[] hmacKey = deriveKey(new String(key), hmacSalt, SALT_LENGTH); + // Perform HMAC using SHA-256 + Mac mac = Mac.getInstance(MAC_ALGORITHM); + mac.init(new SecretKeySpec(hmacKey, MAC_ALGORITHM)); + byte[] checkHmac = mac.doFinal(Arrays.copyOfRange(data, encryptedStart, data.length)); + + // Compare Computed HMAC vs Recovered HMAC + if (MessageDigest.isEqual(hmac, checkHmac)) { + byte[] encKey = deriveKey(new String(key), encSalt, KEY_LENGTH); + cipher.init(Cipher.DECRYPT_MODE, new SecretKeySpec(encKey, ENCRYPTION_ALGORITHM), new IvParameterSpec(iv)); + return cipher.doFinal(cipherText); + } + else { + //Likely thrown for all invalid cases + throw new InvalidHMacException("hmac does not match the ciphertext"); + } } catch (InvalidAlgorithmParameterException | InvalidKeyException e) { //Shouldn't occur e.printStackTrace(); System.exit(1); + } catch (InvalidKeySpecException | NoSuchAlgorithmException e) { + //Shouldn't occur + e.printStackTrace(); + throw new RuntimeException(e); + } catch (BadPaddingException| IllegalBlockSizeException | InvalidHMacException e) { + throw new DecryptFailedException(e); } return null; diff --git a/src/model/Cryptographer.java b/src/model/Cryptographer.java index 982cce9..c5be226 100644 --- a/src/model/Cryptographer.java +++ b/src/model/Cryptographer.java @@ -1,7 +1,12 @@ package model; +import model.exception.DecryptFailedException; +import model.exception.InvalidDataException; +import model.exception.InvalidHMacException; + import javax.crypto.BadPaddingException; import javax.crypto.IllegalBlockSizeException; +import java.security.InvalidKeyException; /** * Created by alutman on 18/03/14. @@ -12,7 +17,7 @@ public interface Cryptographer { public byte[] encrypt (byte[] key, byte[] data); - public byte[] decrypt (byte[] key, byte[] data) throws BadPaddingException, IllegalBlockSizeException; + public byte[] decrypt (byte[] key, byte[] data) throws DecryptFailedException, InvalidDataException; public String getMode(); diff --git a/src/model/CryptographerHandler.java b/src/model/CryptographerHandler.java index ac0135f..d72f6ef 100644 --- a/src/model/CryptographerHandler.java +++ b/src/model/CryptographerHandler.java @@ -2,11 +2,11 @@ import model.enums.Encoding; import model.enums.FileStatus; -import model.exception.InvalidEncryptionException; +import model.exception.DecryptFailedException; +import model.exception.InvalidDataException; +import model.exception.InvalidEncodingException; import model.enums.CryptStatus; -import javax.crypto.BadPaddingException; -import javax.crypto.IllegalBlockSizeException; import java.io.*; /** @@ -92,12 +92,13 @@ public CryptStatus decrypt (String key) { data.set(aes.decrypt(key.getBytes(), data.bytes())); data.detectMode(); } - catch (InvalidEncryptionException iee) { - //InvalidEncryptionException is a result of trying to decrypt something that isn't encrypted - return CryptStatus.INVALID_FILE; + catch (InvalidEncodingException iee) { + return CryptStatus.INVALID_ENCODING; } - catch (BadPaddingException | IllegalBlockSizeException e) { - //BadPaddingException is a result of trying to decrypt with a incorrect key + catch (InvalidDataException e) { + return CryptStatus.INVALID_DATA; + } + catch (DecryptFailedException e) { data.encode(); return CryptStatus.INVALID_KEY_ENC; } diff --git a/src/model/enums/CryptStatus.java b/src/model/enums/CryptStatus.java index 5a35e77..47e811c 100644 --- a/src/model/enums/CryptStatus.java +++ b/src/model/enums/CryptStatus.java @@ -12,7 +12,8 @@ public enum CryptStatus { ENCRYPT_SUCCESS("ENCRYPT SUCCESSFUL"), DECRYPT_SUCCESS("DECRYPT SUCCESSFUL"), INVALID_KEY_ENC("INVALID KEY/ENCRYPTION"), - INVALID_FILE("NOT A VALID ENCRYPTED FILE"), + INVALID_ENCODING("INVALID ENCODING"), + INVALID_DATA("INVALID DATA"), ENCRYPT_FAILED("ENCRYPT FAILED"), DECRYPT_FAILED("DECRYPT FAILED"); diff --git a/src/model/exception/DecryptFailedException.java b/src/model/exception/DecryptFailedException.java new file mode 100644 index 0000000..28c0816 --- /dev/null +++ b/src/model/exception/DecryptFailedException.java @@ -0,0 +1,18 @@ +package model.exception; + +/** + * Created by Alex on 12-Jul-17. + */ +public class DecryptFailedException extends Exception{ + public DecryptFailedException(String message) { + super(message); + } + + public DecryptFailedException(String message, Throwable cause) { + super(message, cause); + } + + public DecryptFailedException(Throwable cause) { + super(cause); + } +} diff --git a/src/model/exception/InvalidDataException.java b/src/model/exception/InvalidDataException.java new file mode 100644 index 0000000..5561012 --- /dev/null +++ b/src/model/exception/InvalidDataException.java @@ -0,0 +1,10 @@ +package model.exception; + +/** + * Created by Alex on 12-Jul-17. + */ +public class InvalidDataException extends Exception { + public InvalidDataException(String message) { + super(message); + } +} diff --git a/src/model/exception/InvalidEncryptionException.java b/src/model/exception/InvalidEncryptionException.java deleted file mode 100644 index bbaa8fe..0000000 --- a/src/model/exception/InvalidEncryptionException.java +++ /dev/null @@ -1,10 +0,0 @@ -package model.exception; - -/** - * Created by alutman on 11-Sep-14. - */ -public class InvalidEncryptionException extends Exception{ - public InvalidEncryptionException() { - super(); - } -} diff --git a/src/model/exception/InvalidHMacException.java b/src/model/exception/InvalidHMacException.java new file mode 100644 index 0000000..3dc8305 --- /dev/null +++ b/src/model/exception/InvalidHMacException.java @@ -0,0 +1,11 @@ +package model.exception; + +/** + * Created by Alex on 12-Jul-17. + */ +public class InvalidHMacException extends Exception { + + public InvalidHMacException(String message) { + super(message); + } +} diff --git a/test/model/CBCCryptographerTest.java b/test/model/CBCCryptographerTest.java index 3a6f8b8..89e06c0 100644 --- a/test/model/CBCCryptographerTest.java +++ b/test/model/CBCCryptographerTest.java @@ -2,10 +2,10 @@ import com.sun.org.apache.xml.internal.security.exceptions.Base64DecodingException; import com.sun.org.apache.xml.internal.security.utils.Base64; +import model.exception.DecryptFailedException; +import model.exception.InvalidDataException; import org.junit.Test; -import javax.crypto.BadPaddingException; -import javax.crypto.IllegalBlockSizeException; import java.security.SecureRandom; /** @@ -17,7 +17,7 @@ public class CBCCryptographerTest { @Test - public void should_produce_identical_byte_data_after_encrypt_and_decrypt() throws BadPaddingException, IllegalBlockSizeException { + public void should_produce_identical_byte_data_after_encrypt_and_decrypt() throws DecryptFailedException, InvalidDataException { SecureRandom sr = new SecureRandom(); byte[] b = new byte[16]; byte[] b2 = new byte[200]; @@ -36,7 +36,7 @@ public void should_produce_identical_byte_data_after_encrypt_and_decrypt() throw } @Test - public void should_produce_identical_string_data_after_encrypt_and_decrypt() throws Base64DecodingException, BadPaddingException, IllegalBlockSizeException { + public void should_produce_identical_string_data_after_encrypt_and_decrypt() throws Base64DecodingException, DecryptFailedException, InvalidDataException { SecureRandom sr = new SecureRandom(); byte[] b = new byte[16]; sr.nextBytes(b); @@ -51,6 +51,18 @@ public void should_produce_identical_string_data_after_encrypt_and_decrypt() thr assert new String(data).equals(decryptedData); } + @Test + public void should_work_with_empty_arrays() throws DecryptFailedException, InvalidDataException { + byte[] key = new byte[0]; + byte[] data = new byte[0]; + + Cryptographer aes = new CBCCryptographer(); + byte[] encryptedData = aes.encrypt(key, data); + byte[] decryptedData = aes.decrypt(key, encryptedData); + + assert compare(data, decryptedData); + } + private static boolean compare(byte[] a, byte[] b) { if (a.length != b.length) { return false;