commit 9963c46c829b53af52f97f4dffc5756e820b4e00
author Jacques Le Roux <jacques.le.roux@les7arts.com> Wed Jul 21 10:54:03 2021 +0200
committer Jacques Le Roux <jacques.le.roux@les7arts.com> Wed Jul 21 10:55:49 2021 +0200
tree 1acd211630b8640cd296b0520e0a11fee743a378
parent 2c421cedfba41f021fa75916e0fbd8f9d65f13e1

Fixed: Static initialization vectors for encryption (OFBIZ-12281)

(after discussing this on security@ofbiz.apache.org, it was decided to open a
Jira issue for that)

I've noticed that OFBiz Framework sometimes uses static initialization vectors
(IV) while creating a cipher:

https://s.apache.org/vhlbj
https://s.apache.org/nyndk

IVs should be unique and ideally unpredictable to avoid producing the same
ciphertexts for the same plaintexts.

The issues can be fixed with something like the following:

byte[] rawIV = new byte[8];
SecureRandom random = new SecureRandom();
random.nextBytes(rawIV).
IvParameterSpec iv = new IvParameterSpec(rawIV);

jleroux: we prepend the IV in order to decrypt the plaintexts. I'll do
something similar in ValueLinkApi class ASAP

Thanks: Artem Smotrakov

framework/base/src/main/java/org/apache/ofbiz/base/crypto/DesCrypt.java

diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/crypto/DesCrypt.java b/framework/base/src/main/java/org/apache/ofbiz/base/crypto/DesCrypt.java
index 6b7b4a6..d37ecb5 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/crypto/DesCrypt.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/crypto/DesCrypt.java
@@ -22,6 +22,7 @@
 import java.security.InvalidKeyException;
 import java.security.Key;
 import java.security.NoSuchAlgorithmException;
+import java.security.SecureRandom;
 import java.security.spec.InvalidKeySpecException;
 
 import javax.crypto.BadPaddingException;
@@ -33,6 +34,7 @@
 import javax.crypto.spec.DESedeKeySpec;
 import javax.crypto.spec.IvParameterSpec;
 
+import org.apache.commons.codec.binary.Base64;
 import org.apache.ofbiz.base.util.GeneralException;
 
 /**
@@ -41,8 +43,6 @@
  */
 public class DesCrypt {
 
-    private static final String MODULE = DesCrypt.class.getName();
-
     public static Key generateKey() throws NoSuchAlgorithmException {
         KeyGenerator keyGen = KeyGenerator.getInstance("DESede");
 
@@ -51,24 +51,52 @@
     }
 
     public static byte[] encrypt(Key key, byte[] bytes) throws GeneralException {
-        Cipher cipher = DesCrypt.getCipher(key, Cipher.ENCRYPT_MODE);
+        byte[] rawIv = new byte[8];
+        SecureRandom random = new SecureRandom();
+        random.nextBytes(rawIv);
+        IvParameterSpec iv = new IvParameterSpec(rawIv);
+
+        // Create the Cipher - DESede/CBC/PKCS5Padding
         byte[] encBytes = null;
+        Cipher cipher = DesCrypt.getCipher(key, Cipher.ENCRYPT_MODE, iv);
         try {
             encBytes = cipher.doFinal(bytes);
         } catch (IllegalStateException | IllegalBlockSizeException | BadPaddingException e) {
             throw new GeneralException(e);
         }
+
+        // Prepend iv as a prefix to use it during decryption
+        byte[] combinedPayload = new byte[rawIv.length + encBytes.length];
+
+        // populate payload with prefix iv and encrypted data
+        System.arraycopy(iv, 0, combinedPayload, 0, rawIv.length);
+        System.arraycopy(cipher, 0, combinedPayload, 8, encBytes.length);
+
         return encBytes;
     }
 
     public static byte[] decrypt(Key key, byte[] bytes) throws GeneralException {
-        Cipher cipher = DesCrypt.getCipher(key, Cipher.DECRYPT_MODE);
+        // create the Cipher - DESede/CBC/PKCS5Padding
+
+        // separate prefix with IV from the rest of encrypted data
+        byte[] encryptedPayload = Base64.decodeBase64(bytes);
+        byte[] iv = new byte[8];
+        byte[] encryptedBytes = new byte[encryptedPayload.length - iv.length];
+
+        // populate iv with bytes:
+        System.arraycopy(encryptedPayload, 0, iv, 0, iv.length);
+
+        // populate encryptedBytes with bytes:
+        System.arraycopy(encryptedPayload, iv.length, encryptedBytes, 0, encryptedBytes.length);
+
         byte[] decBytes = null;
+        Cipher cipher = DesCrypt.getCipher(key, Cipher.ENCRYPT_MODE, new IvParameterSpec(iv));
         try {
             decBytes = cipher.doFinal(bytes);
         } catch (IllegalStateException | IllegalBlockSizeException | BadPaddingException e) {
             throw new GeneralException(e);
         }
+
         return decBytes;
     }
 
@@ -101,23 +129,20 @@
         throw new GeneralException("Not a valid DESede key!");
     }
 
-    // return a cipher for a key - DESede/CBC/PKCS5Padding IV = 0
-    protected static Cipher getCipher(Key key, int mode) throws GeneralException {
-        byte[] zeros = {0, 0, 0, 0, 0, 0, 0, 0 };
-        IvParameterSpec iv = new IvParameterSpec(zeros);
-
-        // create the Cipher - DESede/CBC/NoPadding
-        Cipher encCipher = null;
+    // return a cipher for a key - DESede/CBC/PKCS5Padding with random IV
+    protected static Cipher getCipher(Key key, int mode, IvParameterSpec iv) throws GeneralException {
+        // create the Cipher - DESede/CBC/PKCS5Padding
+        Cipher cipher = null;
         try {
-            encCipher = Cipher.getInstance("DESede/CBC/PKCS5Padding");
+            cipher = Cipher.getInstance("DESede/CBC/PKCS5Padding");
         } catch (NoSuchAlgorithmException | NoSuchPaddingException e) {
             throw new GeneralException(e);
         }
         try {
-            encCipher.init(mode, key, iv);
+            cipher.init(mode, key, iv);
         } catch (InvalidKeyException | InvalidAlgorithmParameterException e) {
             throw new GeneralException(e);
         }
-        return encCipher;
+        return cipher;
     }
 }