Improve encryption methods

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
 -------

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;

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()));
         }

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;
 

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();
 

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;
         }

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");
 

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);
+    }
+}

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);
+    }
+}

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);
+    }
+}