You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2021/07/23 07:32:22 UTC

[ofbiz-framework] branch trunk updated (c5a1a91 -> 4f14cb9)

This is an automated email from the ASF dual-hosted git repository.

jleroux pushed a change to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git.


    from c5a1a91  Fixed: Correct checkstyle typo
     new 4e0ed1e  Fixed: Static initialization vectors for encryption (OFBIZ-12281)
     new 4f14cb9  Fixed: Use platform specific failed on CompoundWidget (OFBIZ-12283)

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../thirdparty/valuelink/ValueLinkApi.java         | 142 +++++++++++++++------
 .../org/apache/ofbiz/base/crypto/DesCrypt.java     |   2 -
 .../org/apache/ofbiz/widget/model/HtmlWidget.java  |  40 +++---
 3 files changed, 124 insertions(+), 60 deletions(-)

[ofbiz-framework] 02/02: Fixed: Use platform specific failed on CompoundWidget (OFBIZ-12283)

Posted by jl...@apache.org.
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

commit 4f14cb96cf0b9d80bea4e363eb464d69027b4298
Author: Jacques Le Roux <ja...@les7arts.com>
AuthorDate: Fri Jul 23 08:49:35 2021 +0200

    Fixed: Use platform specific failed on CompoundWidget (OFBIZ-12283)
    
    Fixes a checkstyle issue
---
 .../org/apache/ofbiz/widget/model/HtmlWidget.java  | 40 ++++++++++++----------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/HtmlWidget.java b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/HtmlWidget.java
index 3fcc901..db206f1 100644
--- a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/HtmlWidget.java
+++ b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/HtmlWidget.java
@@ -18,15 +18,18 @@
  *******************************************************************************/
 package org.apache.ofbiz.widget.model;
 
-import freemarker.ext.beans.BeansWrapper;
-import freemarker.ext.beans.CollectionModel;
-import freemarker.ext.beans.StringModel;
-import freemarker.template.Configuration;
-import freemarker.template.Template;
-import freemarker.template.TemplateException;
-import freemarker.template.TemplateModel;
-import freemarker.template.TemplateModelException;
-import freemarker.template.Version;
+import java.io.IOException;
+import java.io.StringWriter;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Stack;
+
+import javax.servlet.http.HttpServletRequest;
+
 import org.apache.ofbiz.base.util.Debug;
 import org.apache.ofbiz.base.util.GeneralException;
 import org.apache.ofbiz.base.util.StringUtil;
@@ -48,16 +51,15 @@ import org.jsoup.parser.ParseError;
 import org.jsoup.select.Elements;
 import org.w3c.dom.Element;
 
-import javax.servlet.http.HttpServletRequest;
-import java.io.IOException;
-import java.io.StringWriter;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Stack;
+import freemarker.ext.beans.BeansWrapper;
+import freemarker.ext.beans.CollectionModel;
+import freemarker.ext.beans.StringModel;
+import freemarker.template.Configuration;
+import freemarker.template.Template;
+import freemarker.template.TemplateException;
+import freemarker.template.TemplateModel;
+import freemarker.template.TemplateModelException;
+import freemarker.template.Version;
 
 /**
  * Widget Library - Screen model HTML class.

[ofbiz-framework] 01/02: Fixed: Static initialization vectors for encryption (OFBIZ-12281)

Posted by jl...@apache.org.
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

commit 4e0ed1e6df0b8121d37144573c04bca4a5986c95
Author: Jacques Le Roux <ja...@les7arts.com>
AuthorDate: Fri Jul 23 08:45:51 2021 +0200

    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: this prepends the IV in order to decrypt the plaintexts. It's similar
    to what has been done for the DesCrypt class. But contrary to the DesCrypt class
    I did not test.
    
    Thanks: Artem Smotrakov
---
 .../thirdparty/valuelink/ValueLinkApi.java         | 142 +++++++++++++++------
 .../org/apache/ofbiz/base/crypto/DesCrypt.java     |   2 -
 2 files changed, 103 insertions(+), 41 deletions(-)

diff --git a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/valuelink/ValueLinkApi.java b/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/valuelink/ValueLinkApi.java
index 18fdff7..a07d9a3 100644
--- a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/valuelink/ValueLinkApi.java
+++ b/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/valuelink/ValueLinkApi.java
@@ -23,6 +23,7 @@ import java.math.BigInteger;
 import java.nio.charset.StandardCharsets;
 import java.security.InvalidAlgorithmParameterException;
 import java.security.InvalidKeyException;
+import java.security.Key;
 import java.security.KeyFactory;
 import java.security.KeyPair;
 import java.security.KeyPairGenerator;
@@ -59,7 +60,9 @@ import javax.crypto.spec.DHPrivateKeySpec;
 import javax.crypto.spec.DHPublicKeySpec;
 import javax.crypto.spec.IvParameterSpec;
 
+import org.apache.commons.codec.binary.Base64;
 import org.apache.ofbiz.base.util.Debug;
+import org.apache.ofbiz.base.util.GeneralException;
 import org.apache.ofbiz.base.util.HttpClient;
 import org.apache.ofbiz.base.util.HttpClientException;
 import org.apache.ofbiz.base.util.StringUtil;
@@ -154,14 +157,23 @@ public class ValueLinkApi {
      * @return Hex String of the encrypted pin (EAN) for transmission to ValueLink
      */
     public String encryptPin(String pin) {
+        byte[] rawIv = new byte[8];
+        SecureRandom random = new SecureRandom();
+        random.nextBytes(rawIv);
+        IvParameterSpec iv = new IvParameterSpec(rawIv);
         // get the Cipher
-        Cipher mwkCipher = this.getCipher(this.getMwkKey(), Cipher.ENCRYPT_MODE);
+        Cipher mwkCipher = null;
+        try {
+            mwkCipher = getCipher(this.getMwkKey(), Cipher.ENCRYPT_MODE, iv);
+        } catch (GeneralException e1) {
+            Debug.logError(e1, MODULE);
+        }
 
         // pin to bytes
         byte[] pinBytes = pin.getBytes(StandardCharsets.UTF_8);
 
         // 7 bytes of random data
-        byte[] random = this.getRandomBytes(7);
+        byte[] randomBytes = this.getRandomBytes(7);
 
         // pin checksum
         byte[] checkSum = this.getPinCheckSum(pinBytes);
@@ -169,8 +181,8 @@ public class ValueLinkApi {
         // put all together
         byte[] eanBlock = new byte[16];
         int i;
-        for (i = 0; i < random.length; i++) {
-            eanBlock[i] = random[i];
+        for (i = 0; i < randomBytes.length; i++) {
+            eanBlock[i] = randomBytes[i];
         }
         eanBlock[7] = checkSum[0];
         for (i = 0; i < pinBytes.length; i++) {
@@ -196,11 +208,26 @@ public class ValueLinkApi {
     /**
      * Decrypt an encrypted pin using the configured keys
      * @param pin Hex String of the encrypted pin (EAN)
-     * @return Plain text String of the pin
+     * @return Plain text String of the pin @
      */
     public String decryptPin(String pin) {
-        // get the Cipher
-        Cipher mwkCipher = this.getCipher(this.getMwkKey(), Cipher.DECRYPT_MODE);
+        // separate prefix with IV from the rest of encrypted data
+        byte[] encryptedPayload = Base64.decodeBase64(pin.getBytes());
+        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);
+
+        Cipher mwkCipher = null;
+        try {
+            mwkCipher = getCipher(getMwkKey(), Cipher.ENCRYPT_MODE, new IvParameterSpec(iv));
+        } catch (GeneralException e1) {
+            Debug.logError(e1, MODULE);
+        }
 
         // decrypt pin
         String decryptedPinString = null;
@@ -263,7 +290,7 @@ public class ValueLinkApi {
     }
 
     /**
-     * Output the creation of public/private keys + KEK to the console for manual database update
+     * Output the creation of public/private keys + KEK to the console for manual database update @
      */
     public StringBuffer outputKeyCreation(boolean kekOnly, String kekTest) {
         return this.outputKeyCreation(0, kekOnly, kekTest);
@@ -324,7 +351,17 @@ public class ValueLinkApi {
         byte[] loadKekBytes = loadedKek.getEncoded();
 
         // test the KEK
-        Cipher cipher = this.getCipher(this.getKekKey(), Cipher.ENCRYPT_MODE);
+        byte[] rawIv = new byte[8];
+        SecureRandom secRandom = new SecureRandom();
+        secRandom.nextBytes(rawIv);
+        IvParameterSpec iv = new IvParameterSpec(rawIv);
+
+        Cipher cipher = null;
+        try {
+            cipher = getCipher(this.getKekKey(), Cipher.ENCRYPT_MODE, iv);
+        } catch (GeneralException e1) {
+            Debug.logError(e1, MODULE);
+        }
         byte[] kekTestB = {0, 0, 0, 0, 0, 0, 0, 0 };
         byte[] kekTestC = new byte[0];
         if (kekTest != null) {
@@ -481,7 +518,7 @@ public class ValueLinkApi {
 
     /**
      * Generate a new MWK
-     * @return Hex String of the new encrypted MWK ready for transmission to ValueLink
+     * @return Hex String of the new encrypted MWK ready for transmission to ValueLink @
      */
     public byte[] generateMwk() {
         KeyGenerator keyGen = null;
@@ -520,7 +557,7 @@ public class ValueLinkApi {
     /**
      * Generate a new MWK
      * @param desBytes byte array of the DES key (24 bytes)
-     * @return Hex String of the new encrypted MWK ready for transmission to ValueLink
+     * @return Hex String of the new encrypted MWK ready for transmission to ValueLink @
      */
     public byte[] generateMwk(byte[] desBytes) {
         if (debug) {
@@ -555,7 +592,7 @@ public class ValueLinkApi {
     /**
      * Generate a new MWK
      * @param mwkdes3 pre-generated DES3 SecretKey
-     * @return Hex String of the new encrypted MWK ready for transmission to ValueLink
+     * @return Hex String of the new encrypted MWK ready for transmission to ValueLink @
      */
     public byte[] generateMwk(SecretKey mwkdes3) {
         // zeros for checksum
@@ -566,9 +603,18 @@ public class ValueLinkApi {
         Random ran = new SecureRandom();
         ran.nextBytes(random);
 
+        byte[] rawIv = new byte[8];
+        SecureRandom secRandom = new SecureRandom();
+        secRandom.nextBytes(rawIv);
+        IvParameterSpec iv = new IvParameterSpec(rawIv);
 
         // open a cipher using the new mwk
-        Cipher cipher = this.getCipher(mwkdes3, Cipher.ENCRYPT_MODE);
+        Cipher cipher = null;
+        try {
+            cipher = getCipher(mwkdes3, Cipher.ENCRYPT_MODE, iv);
+        } catch (GeneralException e1) {
+            Debug.logError(e1, MODULE);
+        }
 
         // make the checksum - encrypted 8 bytes of 0's
         byte[] encryptedZeros = new byte[0];
@@ -598,7 +644,12 @@ public class ValueLinkApi {
      * @return encrypted byte array
      */
     public byte[] encryptViaKek(byte[] content) {
-        return cryptoViaKek(content, Cipher.ENCRYPT_MODE);
+        try {
+            return cryptoViaKek(content, Cipher.ENCRYPT_MODE);
+        } catch (GeneralException e1) {
+            Debug.logError(e1, MODULE);
+        }
+        return null;
     }
 
     /**
@@ -607,7 +658,12 @@ public class ValueLinkApi {
      * @return decrypted byte array
      */
     public byte[] decryptViaKek(byte[] content) {
-        return cryptoViaKek(content, Cipher.DECRYPT_MODE);
+        try {
+            return cryptoViaKek(content, Cipher.DECRYPT_MODE);
+        } catch (GeneralException e1) {
+            Debug.logError(e1, MODULE);
+        }
+        return null;
     }
 
     /**
@@ -758,42 +814,50 @@ public class ValueLinkApi {
         return dhParamSpec;
     }
 
-    /** actual kek encryption/decryption code */
-    protected byte[] cryptoViaKek(byte[] content, int mode) {
+    /**
+     * actual kek encryption/decryption code
+     * @throws GeneralException
+     */
+    protected byte[] cryptoViaKek(byte[] content, int mode) throws GeneralException {
         // open a cipher using the kek for transport
-        Cipher cipher = this.getCipher(this.getKekKey(), mode);
-        byte[] dec = new byte[0];
+        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 = getCipher(getKekKey(), mode, iv);
         try {
-            dec = cipher.doFinal(content);
+            encBytes = cipher.doFinal(content);
         } catch (IllegalStateException | BadPaddingException | IllegalBlockSizeException e) {
             Debug.logError(e, MODULE);
         }
-        return dec;
-    }
+        // Prepend iv as a prefix to use it during decryption
+        byte[] combinedPayload = new byte[rawIv.length + encBytes.length];
 
-    /** return a cipher for a key - DESede/CBC/NoPadding IV = 0 */
-    protected Cipher getCipher(SecretKey key, int mode) {
-        byte[] zeros = {0, 0, 0, 0, 0, 0, 0, 0 };
-        IvParameterSpec iv = new IvParameterSpec(zeros);
+        // 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;
+    }
+
+    // return a cipher for a key - DESede/CBC/NoPadding with random IV
+    protected static Cipher getCipher(Key key, int mode, IvParameterSpec iv) throws GeneralException {
         // create the Cipher - DESede/CBC/NoPadding
-        Cipher mwkCipher = null;
+        Cipher cipher = null;
         try {
-            mwkCipher = Cipher.getInstance("DESede/CBC/NoPadding");
-        } catch (NoSuchAlgorithmException e) {
-            Debug.logError(e, MODULE);
-            return null;
-        } catch (NoSuchPaddingException e) {
-            Debug.logError(e, MODULE);
+            cipher = Cipher.getInstance("DESede/CBC/NoPadding");
+        } catch (NoSuchAlgorithmException | NoSuchPaddingException e) {
+            throw new GeneralException(e);
         }
         try {
-            mwkCipher.init(mode, key, iv);
-        } catch (InvalidKeyException e) {
-            Debug.logError(e, "Invalid key", MODULE);
-        } catch (InvalidAlgorithmParameterException e) {
-            Debug.logError(e, MODULE);
+            cipher.init(mode, key, iv);
+        } catch (InvalidKeyException | InvalidAlgorithmParameterException e) {
+            throw new GeneralException(e);
         }
-        return mwkCipher;
+        return cipher;
     }
 
     /**
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 d37ecb5..924b4b6 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
@@ -76,8 +76,6 @@ public class DesCrypt {
     }
 
     public static byte[] decrypt(Key key, byte[] bytes) throws GeneralException {
-        // 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];