This is an automated email from the ASF dual-hosted git repository. jleroux pushed a commit to branch trunk in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/trunk by this push: new 9963c46 Fixed: Static initialization vectors for encryption (OFBIZ-12281) 9963c46 is described below commit 9963c46c829b53af52f97f4dffc5756e820b4e00 Author: Jacques Le Roux <jacques.le.r...@les7arts.com> AuthorDate: Wed Jul 21 10:54:03 2021 +0200 Fixed: Static initialization vectors for encryption (OFBIZ-12281) (after discussing this on secur...@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 --- .../org/apache/ofbiz/base/crypto/DesCrypt.java | 53 ++++++++++++++++------ 1 file changed, 39 insertions(+), 14 deletions(-) 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.InvalidAlgorithmParameterException; 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.SecretKeyFactory; 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 @@ import org.apache.ofbiz.base.util.GeneralException; */ 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 class DesCrypt { } 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 @@ public class DesCrypt { 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; } }