You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by sh...@apache.org on 2016/12/05 01:32:51 UTC

svn commit: r1772589 - in /ofbiz/trunk/framework: base/src/main/java/org/apache/ofbiz/base/crypto/HashCrypt.java security/config/security.properties security/data/PasswordSecurityDemoData.xml security/entitydef/entitymodel.xml

Author: shijh
Date: Mon Dec  5 01:32:51 2016
New Revision: 1772589

URL: http://svn.apache.org/viewvc?rev=1772589&view=rev
Log:
Implemented: LoginWorker HashCrypt the type of hash for one-way encryption

Added PBKDF2 hash type to encrypt password.

Thanks: wangjunyuan for this contribution.

Modified:
    ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/crypto/HashCrypt.java
    ofbiz/trunk/framework/security/config/security.properties
    ofbiz/trunk/framework/security/data/PasswordSecurityDemoData.xml
    ofbiz/trunk/framework/security/entitydef/entitymodel.xml

Modified: ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/crypto/HashCrypt.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/crypto/HashCrypt.java?rev=1772589&r1=1772588&r2=1772589&view=diff
==============================================================================
--- ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/crypto/HashCrypt.java (original)
+++ ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/crypto/HashCrypt.java Mon Dec  5 01:32:51 2016
@@ -22,6 +22,10 @@ import java.io.UnsupportedEncodingExcept
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.security.SecureRandom;
+import java.security.spec.InvalidKeySpecException;
+
+import javax.crypto.SecretKeyFactory;
+import javax.crypto.spec.PBEKeySpec;
 
 import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.codec.binary.Hex;
@@ -29,11 +33,12 @@ import org.apache.commons.lang.RandomStr
 import org.apache.ofbiz.base.util.Debug;
 import org.apache.ofbiz.base.util.GeneralRuntimeException;
 import org.apache.ofbiz.base.util.StringUtil;
-import org.apache.ofbiz.base.util.UtilValidate;
 import org.apache.ofbiz.base.util.UtilIO;
+import org.apache.ofbiz.base.util.UtilProperties;
+import org.apache.ofbiz.base.util.UtilValidate;
 
 /**
- * Utility class for doing SHA-1/MD5 One-Way Hash Encryption
+ * Utility class for doing SHA-1/MD5/PBKDF2 One-Way Hash Encryption
  *
  */
 public class HashCrypt {
@@ -41,6 +46,16 @@ public class HashCrypt {
     public static final String module = HashCrypt.class.getName();
     public static final String CRYPT_CHAR_SET = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789./";
 
+    private static final String PBKDF2_SHA1 ="pbkdf2_sha1";
+    
+    private static final String PBKDF2_SHA256 ="pbkdf2_sha256"; 
+    
+    private static final String PBKDF2_SHA384 ="pbkdf2_sha384";
+    
+    private static final String PBKDF2_SHA512 ="pbkdf2_sha512";
+
+    private static final int PBKDF2_ITERATIONS = UtilProperties.getPropertyAsInteger("security.properties", "password.encrypt.pbkdf2.iterations", 1000);
+    
     public static MessageDigest getMessageDigest(String type) {
         try {
             return MessageDigest.getInstance(type);
@@ -55,6 +70,8 @@ public class HashCrypt {
             return doCompareTypePrefix(crypted, defaultCrypt, password.getBytes());
         } else if (crypted.startsWith("$")) {
             return doComparePosix(crypted, defaultCrypt, password.getBytes(UtilIO.getUtf8()));
+        } else if (crypted.startsWith("pbkdf2")) {
+            return doComparePbkdf2(crypted, password);
         } else {
             // FIXME: should have been getBytes("UTF-8") originally
             return doCompareBare(crypted, defaultCrypt, password.getBytes());
@@ -106,15 +123,24 @@ public class HashCrypt {
      */
     @Deprecated
     public static String cryptPassword(String hashType, String salt, String password) {
+    	if (hashType.startsWith("PBKDF2")) {
+            return password != null ? pbkdf2HashCrypt(hashType, salt, password) : null;
+        }
         // FIXME: should have been getBytes("UTF-8") originally
         return password != null ? cryptBytes(hashType, salt, password.getBytes()) : null;
     }
 
     public static String cryptUTF8(String hashType, String salt, String value) {
+    	if (hashType.startsWith("PBKDF2")) {
+            return value != null ? pbkdf2HashCrypt(hashType, salt, value) : null;
+        }
         return value != null ? cryptBytes(hashType, salt, value.getBytes(UtilIO.getUtf8())) : null;
     }
 
     public static String cryptValue(String hashType, String salt, String value) {
+    	if (hashType.startsWith("PBKDF2")) {
+            return value != null ? pbkdf2HashCrypt(hashType, salt, value) : null;
+        }
         return value != null ? cryptBytes(hashType, salt, value.getBytes()) : null;
     }
 
@@ -142,6 +168,90 @@ public class HashCrypt {
         }
     }
 
+    public static String pbkdf2HashCrypt(String hashType, String salt, String value){
+        char[] chars = value.toCharArray();
+        if (UtilValidate.isEmpty(salt)) {
+            salt = getSalt();
+        }
+        try {
+            PBEKeySpec spec = new PBEKeySpec(chars, salt.getBytes(UtilIO.getUtf8()), PBKDF2_ITERATIONS, 64 * 4);
+            SecretKeyFactory skf = SecretKeyFactory.getInstance(hashType);
+            byte[] hash = Base64.encodeBase64(skf.generateSecret(spec).getEncoded());
+            String pbkdf2Type = null;
+            switch (hashType) {
+                case "PBKDF2WithHmacSHA1":
+                    pbkdf2Type = PBKDF2_SHA1;
+                    break;
+                case "PBKDF2WithHmacSHA256":
+                    pbkdf2Type = PBKDF2_SHA256;
+                    break;
+                case "PBKDF2WithHmacSHA384":
+                    pbkdf2Type = PBKDF2_SHA384;
+                    break;
+                case "PBKDF2WithHmacSHA512":
+                    pbkdf2Type = PBKDF2_SHA512;
+                    break;
+                default: 
+                    pbkdf2Type = PBKDF2_SHA1; 
+            }
+            return pbkdf2Type + "$" + PBKDF2_ITERATIONS + "$" + salt + "$" + new String(hash);
+        } catch (InvalidKeySpecException e) {
+            throw new GeneralRuntimeException("Error while creating SecretKey", e);
+        } catch (NoSuchAlgorithmException e) {
+            throw new GeneralRuntimeException("Error while computing SecretKeyFactory", e);
+        }
+    }
+    
+    public static boolean doComparePbkdf2(String storedPassword, String originalPassword){
+        try {
+            String[] parts = storedPassword.split("\\$");
+            String hashHead = parts[0];
+            int iterations = Integer.parseInt(parts[1]);
+            byte[] salt = parts[2].getBytes();
+            byte[] hash = Base64.decodeBase64(parts[3].getBytes());
+            
+            PBEKeySpec spec = new PBEKeySpec(originalPassword.toCharArray(), salt, iterations, hash.length * 8);
+            String hashType = null;
+            switch (hashHead.substring(hashHead.indexOf("_")+4)) {
+                case "256":
+                    hashType = "PBKDF2WithHmacSHA256";
+                    break;
+                case "384":
+                    hashType = "PBKDF2WithHmacSHA384";
+                    break;
+                case "512":
+                    hashType = "PBKDF2WithHmacSHA512";
+                    break;
+                default:  
+                    hashType = "PBKDF2WithHmacSHA1";
+            }
+            SecretKeyFactory skf = SecretKeyFactory.getInstance(hashType);
+            byte[] testHash = skf.generateSecret(spec).getEncoded();
+            int diff = hash.length ^ testHash.length;
+            
+            for (int i = 0; i < hash.length && i < testHash.length; i++) {
+                diff |= hash[i] ^ testHash[i];
+            }
+            
+            return diff == 0;
+        } catch (NoSuchAlgorithmException e) {
+            throw new GeneralRuntimeException("Error while computing SecretKeyFactory", e);
+        } catch (InvalidKeySpecException e) {
+            throw new GeneralRuntimeException("Error while creating SecretKey", e);
+        }
+    }
+    
+    private static String getSalt() {
+        try {
+            SecureRandom sr = SecureRandom.getInstance("SHA1PRNG");
+            byte[] salt = new byte[16];
+            sr.nextBytes(salt);
+            return salt.toString();
+        } catch (NoSuchAlgorithmException e) {
+            throw new GeneralRuntimeException("Error while creating salt", e);
+        }
+    }
+    
     /**
      * @deprecated use digestHash("SHA", null, str)
      */

Modified: ofbiz/trunk/framework/security/config/security.properties
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/security/config/security.properties?rev=1772589&r1=1772588&r2=1772589&view=diff
==============================================================================
--- ofbiz/trunk/framework/security/config/security.properties (original)
+++ ofbiz/trunk/framework/security/config/security.properties Mon Dec  5 01:32:51 2016
@@ -82,9 +82,13 @@ password.encrypt=true
 password.email_password.require_password_change=true
 
 # -- specify the type of hash to use for one-way encryption, will be passed to java.security.MessageDigest.getInstance() --
-# -- options may include: SHA, MD5, etc
+# -- options may include: SHA, MD5, PBKDF2WithHmacSHA1, PBKDF2WithHmacSHA256, PBKDF2WithHmacSHA384, PBKDF2WithHmacSHA512 and etc
 password.encrypt.hash.type=SHA
 
+# -- if the type of hash to use for one-way encryption is PBKDF2WithHmacSHA1 or PBKDF2WithHmacSHA256 or PBKDF2WithHmacSHA384 or PBKDF2WithHmacSHA512
+# -- the type of hash to use for one-way encryption needs iteration
+password.encrypt.pbkdf2.iterations=1000
+
 # -- this is helpful to recover old accounts or to be able to login at all sometimes --
 # -- SHOULD GENERALLY NOT BE TRUE FOR PRODUCTION SITES, but is useful for interim periods when going to password encryption --
 password.accept.encrypted.and.plain=false

Modified: ofbiz/trunk/framework/security/data/PasswordSecurityDemoData.xml
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/security/data/PasswordSecurityDemoData.xml?rev=1772589&r1=1772588&r2=1772589&view=diff
==============================================================================
--- ofbiz/trunk/framework/security/data/PasswordSecurityDemoData.xml (original)
+++ ofbiz/trunk/framework/security/data/PasswordSecurityDemoData.xml Mon Dec  5 01:32:51 2016
@@ -21,7 +21,7 @@ under the License.
 <entity-engine-xml>
     <!-- from the securityext component: SecurityExtData.xml -->
     <UserLogin userLoginId="admin" currentPassword="{SHA}47b56994cbc2b6d10aa1be30f70165adb305a41a" passwordHint=""/>
-    <UserLogin userLoginId="flexadmin" currentPassword="{SHA}47b56994cbc2b6d10aa1be30f70165adb305a41a" passwordHint=""/>
+    <UserLogin userLoginId="flexadmin" currentPassword="pbkdf2_sha1$1000$[B@4cb8144c$uAmNaxG3cmfr/VOWPuniw7J/Rw7VM7WFpax3PELBq2Q=" passwordHint=""/>
     <UserLogin userLoginId="demoadmin" currentPassword="{SHA}47b56994cbc2b6d10aa1be30f70165adb305a41a" passwordHint=""/>
     <UserLogin userLoginId="ltdadmin" currentPassword="{SHA}47b56994cbc2b6d10aa1be30f70165adb305a41a" passwordHint=""/>
     <UserLogin userLoginId="ltdadmin1" currentPassword="{SHA}47b56994cbc2b6d10aa1be30f70165adb305a41a" passwordHint=""/>

Modified: ofbiz/trunk/framework/security/entitydef/entitymodel.xml
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/security/entitydef/entitymodel.xml?rev=1772589&r1=1772588&r2=1772589&view=diff
==============================================================================
--- ofbiz/trunk/framework/security/entitydef/entitymodel.xml (original)
+++ ofbiz/trunk/framework/security/entitydef/entitymodel.xml Mon Dec  5 01:32:51 2016
@@ -62,7 +62,7 @@ under the License.
             package-name="org.apache.ofbiz.security.login"
             title="User Login Entity">
       <field name="userLoginId" type="id-vlong-ne"></field>
-      <field name="currentPassword" type="short-varchar"></field>
+      <field name="currentPassword" type="long-varchar"></field>
       <field name="passwordHint" type="description"></field>
       <field name="isSystem" type="indicator"></field>
       <field name="enabled" type="indicator"></field>
@@ -89,7 +89,7 @@ under the License.
       <field name="userLoginId" type="id-vlong-ne"></field>
       <field name="fromDate" type="date-time"></field>
       <field name="thruDate" type="date-time"></field>
-      <field name="currentPassword" type="short-varchar"></field>
+      <field name="currentPassword" type="long-varchar"></field>
       <prim-key field="userLoginId"/>
       <prim-key field="fromDate"/>
       <relation type="one" fk-name="USER_LPH_USER" rel-entity-name="UserLogin">



Re: svn commit: r1772589 - in /ofbiz/trunk/framework: base/src/main/java/org/apache/ofbiz/base/crypto/HashCrypt.java security/config/security.properties security/data/PasswordSecurityDemoData.xml security/entitydef/entitymodel.xml

Posted by Jacques Le Roux <ja...@les7arts.com>.
Why not changing other admin logins pwd encryption? Production sites should not use this pwd anyway (even if they are based on trunk).

We need to provide a simple mean to increase the currentPassword field size.

Could be as simple as an entry in https://cwiki.apache.org/confluence/display/OFBIZ/Revisions+Requiring+Data+Migration+-+upgrade+ofbiz

with ALTER TABLE UserLogin ALTER COLUMN currentPassword varchar(255)

Also nitpicking but I see no reasons to have blank lines between privates in HashCrypt ;)

Jacques


Le 05/12/2016 � 09:44, Nicolas Malin a �crit :
> hello Shi
>
> I think it's better to use a StringBuilder to build this.
>
> Cheers,
> Nicolas
> Le 05/12/2016 � 02:32, shijh@apache.org a �crit :
>> +            return pbkdf2Type + "$" + PBKDF2_ITERATIONS + "$" + salt + "$" + new String(hash);
>
>


Re: svn commit: r1772589 - in /ofbiz/trunk/framework: base/src/main/java/org/apache/ofbiz/base/crypto/HashCrypt.java security/config/security.properties security/data/PasswordSecurityDemoData.xml security/entitydef/entitymodel.xml

Posted by Nicolas Malin <ni...@nereide.fr>.
hello Shi

I think it's better to use a StringBuilder to build this.

Cheers,
Nicolas
Le 05/12/2016 � 02:32, shijh@apache.org a �crit :
> +            return pbkdf2Type + "$" + PBKDF2_ITERATIONS + "$" + salt + "$" + new String(hash);