svn commit: r670731 - in /ofbiz/trunk/framework: base/src/base/org/ofbiz/base/crypto/HashCrypt.java base/src/base/org/ofbiz/base/util/StringUtil.java entity/src/org/ofbiz/entity/util/EntityCrypto.java

[jonesde]

Author: jonesde
Date: Mon Jun 23 13:15:27 2008
New Revision: 670731

URL: http://svn.apache.org/viewvc?rev=670731&view=rev
Log:
Removed old funny hex encode and decode routines from StringUtil, as they produced the same results as the commons library; found and fixed the real problem with the old entity engine encryption so that encrypted fields, including credit card numbers, couldn't be decrypted with the new code and it was the hashed key names no longer matched so the wrong DES key was used, ie a different one from the original key used to encrypt even if the original key was still in there; now it tries both, which is necessary because for systems updated before this went in there will be entries using the old DES key and some using the new DES key, so we can't just try one or the other; this should fix problems with credit cards and other encrypted value decryption after updating to fix the password hashing problem from a few weeks ago

Modified:
    ofbiz/trunk/framework/base/src/base/org/ofbiz/base/crypto/HashCrypt.java
    ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/StringUtil.java
    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntityCrypto.java

Modified: ofbiz/trunk/framework/base/src/base/org/ofbiz/base/crypto/HashCrypt.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/base/org/ofbiz/base/crypto/HashCrypt.java?rev=670731&r1=670730&r2=670731&view=diff
==============================================================================
--- ofbiz/trunk/framework/base/src/base/org/ofbiz/base/crypto/HashCrypt.java (original)
+++ ofbiz/trunk/framework/base/src/base/org/ofbiz/base/crypto/HashCrypt.java Mon Jun 23 13:15:27 2008
@@ -89,6 +89,7 @@
     }
     
     public static String getDigestHashOldFunnyHexEncode(String str, String hashType) {
+        if (UtilValidate.isEmpty(hashType)) hashType = "SHA";
         if (str == null) return null;
         try {
             MessageDigest messagedigest = MessageDigest.getInstance(hashType);

Modified: ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/StringUtil.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/StringUtil.java?rev=670731&r1=670730&r2=670731&view=diff
==============================================================================
--- ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/StringUtil.java (original)
+++ ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/StringUtil.java Mon Jun 23 13:15:27 2008
@@ -353,22 +353,6 @@
         }
     }
     
-    public static byte[] fromHexStringOldFunnyVariety(String str) {
-        str = cleanHexString(str);
-        int stringLength = str.length();
-        if ((stringLength & 0x1) != 0) {
-            throw new IllegalArgumentException("fromHexString requires an even number of hex characters");
-        }
-        byte[] b = new byte[stringLength / 2];
-
-        for (int i = 0, j = 0; i < stringLength; i+= 2, j++) {
-            int high = convertChar(str.charAt(i));
-            int low = convertChar(str.charAt(i+1));
-            b[j] = (byte) ((high << 4) | low);
-        }
-        return b;
-    }
-    
     private static char[] hexChar = { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' };
     public static int convertChar(char c) {
         if ( '0' <= c && c <= '9' ) {

Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntityCrypto.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntityCrypto.java?rev=670731&r1=670730&r2=670731&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntityCrypto.java (original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntityCrypto.java Mon Jun 23 13:15:27 2008
@@ -59,7 +59,7 @@
                 if (size == 0) {
                     for (int i = 0; i < 20; i++) {
                         String randomName = this.getRandomString();
-                        this.getKeyFromStore(randomName);
+                        this.getKeyFromStore(randomName, false);
                     }
                 }
             } catch (GenericEntityException e) {
@@ -68,10 +68,10 @@
         }
     }
 
-    /** Encrypts a String into an encrypted hex encoded byte array */
+    /** Encrypts an Object into an encrypted hex encoded String */
     public String encrypt(String keyName, Object obj) throws EntityCryptoException {
         try {
-            byte[] encryptedBytes = DesCrypt.encrypt(this.getKey(keyName), UtilObject.getBytes(obj));
+            byte[] encryptedBytes = DesCrypt.encrypt(this.getKey(keyName, false), UtilObject.getBytes(obj));
             String hexString = StringUtil.toHexString(encryptedBytes);
             return hexString;
         } catch (GeneralException e) {
@@ -79,47 +79,51 @@
         }
     }
 
-    /** Decrypts a hex encoded byte array into a String */
-    public Object decrypt(String keyName, String str) throws EntityCryptoException {
+    /** Decrypts a hex encoded String into an Object */
+    public Object decrypt(String keyName, String encryptedString) throws EntityCryptoException {
+        Object decryptedObj = null;
+        byte[] encryptedBytes = StringUtil.fromHexString(encryptedString);
         try {
-            byte[] encryptedBytes = StringUtil.fromHexString(str);
-            byte[] decryptedBytes = DesCrypt.decrypt(this.getKey(keyName), encryptedBytes);
-            if (encryptedBytes.equals(decryptedBytes)) {
-                // try using the old/bad hex encoding approach
-                encryptedBytes = StringUtil.fromHexStringOldFunnyVariety(str);
-                decryptedBytes = DesCrypt.decrypt(this.getKey(keyName), encryptedBytes);
-            }
+            SecretKey decryptKey = this.getKey(keyName, false);
+            byte[] decryptedBytes = DesCrypt.decrypt(decryptKey, encryptedBytes);
             
-            return UtilObject.getObject(decryptedBytes);
+            decryptedObj = UtilObject.getObject(decryptedBytes);
         } catch (GeneralException e) {
             try {
                 // try using the old/bad hex encoding approach; this is another path the code may take, ie if there is an exception thrown in decrypt
-                byte[] encryptedBytes = StringUtil.fromHexStringOldFunnyVariety(str);
-                byte[] decryptedBytes = DesCrypt.decrypt(this.getKey(keyName), encryptedBytes);
-                return UtilObject.getObject(decryptedBytes);
+                Debug.logInfo("Decrypt with DES key from standard key name hash failed, trying old/funny variety of key name hash", module);
+                SecretKey decryptKey = this.getKey(keyName, true);
+                byte[] decryptedBytes = DesCrypt.decrypt(decryptKey, encryptedBytes);
+                decryptedObj = UtilObject.getObject(decryptedBytes);
+                //Debug.logInfo("Old/funny variety succeeded: Decrypted value [" + encryptedString + "]", module);
             } catch (GeneralException e1) {
                 // NOTE: this throws the original exception back, not the new one if it fails using the other approach
                 throw new EntityCryptoException(e);
             }
         }
+        
+        // NOTE: this is definitely for debugging purposes only, do not uncomment in production server for security reasons: Debug.logInfo("Decrypted value [" + encryptedString + "] to result: " + decryptedObj, module);
+        return decryptedObj;
     }
 
-    protected SecretKey getKey(String name) throws EntityCryptoException {
-        SecretKey key = keyMap.get(name);
+    protected SecretKey getKey(String name, boolean useOldFunnyKeyHash) throws EntityCryptoException {
+        String keyMapName = name + useOldFunnyKeyHash;
+        SecretKey key = keyMap.get(keyMapName);
         if (key == null) {
             synchronized(this) {
-                String keyName = HashCrypt.getDigestHash(name);
-                key = this.getKeyFromStore(keyName);
-                keyMap.put(name, key);
+                key = this.getKeyFromStore(name, useOldFunnyKeyHash);
+                keyMap.put(keyMapName, key);
             }
         }
         return key;
     }
 
-    protected SecretKey getKeyFromStore(String keyName) throws EntityCryptoException {
+    protected SecretKey getKeyFromStore(String originalKeyName, boolean useOldFunnyKeyHash) throws EntityCryptoException {
+        String hashedKeyName = useOldFunnyKeyHash? HashCrypt.getDigestHashOldFunnyHexEncode(originalKeyName, null) : HashCrypt.getDigestHash(originalKeyName);
+
         GenericValue keyValue = null;
         try {
-            keyValue = delegator.findOne("EntityKeyStore", false, "keyName", keyName);
+            keyValue = delegator.findOne("EntityKeyStore", false, "keyName", hashedKeyName);
         } catch (GenericEntityException e) {
             throw new EntityCryptoException(e);
         }
@@ -132,7 +136,7 @@
             }
             GenericValue newValue = delegator.makeValue("EntityKeyStore");
             newValue.set("keyText", StringUtil.toHexString(key.getEncoded()));
-            newValue.set("keyName", keyName);
+            newValue.set("keyName", hashedKeyName);
 
             Transaction parentTransaction = null;
             boolean beganTrans = false;