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:23 UTC
[ofbiz-framework] 01/02: Fixed: Static initialization vectors for
encryption (OFBIZ-12281)
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];