OFBiz › OFBiz - Dev

[jira] [Created] (OFBIZ-4824) Deprecated use of org.ofbiz.base.crypto.HashCrypt

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

4 messages Options

Nicolas Malin (Jira)

[jira] [Created] (OFBIZ-4824) Deprecated use of org.ofbiz.base.crypto.HashCrypt

Deprecated use of org.ofbiz.base.crypto.HashCrypt
-------------------------------------------------

Key: OFBIZ-4824
URL: https://issues.apache.org/jira/browse/OFBIZ-4824
Project: OFBiz
Issue Type: Bug
Components: framework
Affects Versions: SVN trunk
Reporter: Pierre Smits
Fix For: SVN trunk

Hash

classes:
[javac16] Compiling 140 source files to /ci/ofbiz/framework/entity/build/classes
[javac16] /ci/ofbiz/framework/entity/src/org/ofbiz/entity/GenericEntity.java:1299: warning: [deprecation] getDigestHash(java.lang.String) in org.ofbiz.base.crypto.HashCrypt has been deprecated
[javac16] curValue = HashCrypt.getDigestHash(encryptField);
[javac16] ^
[javac16] /ci/ofbiz/framework/entity/src/org/ofbiz/entity/util/EntityCrypto.java:122: warning: [deprecation] getDigestHashOldFunnyHexEncode(java.lang.String,java.lang.String) in org.ofbiz.base.crypto.HashCrypt has been deprecated
[javac16] String hashedKeyName = useOldFunnyKeyHash? HashCrypt.getDigestHashOldFunnyHexEncode(originalKeyName, null) : HashCrypt.getDigestHash(originalKeyName);
[javac16] ^
[javac16] /ci/ofbiz/framework/entity/src/org/ofbiz/entity/util/EntityCrypto.java:122: warning: [deprecation] getDigestHash(java.lang.String) in org.ofbiz.base.crypto.HashCrypt has been deprecated
[javac16] String hashedKeyName = useOldFunnyKeyHash? HashCrypt.getDigestHashOldFunnyHexEncode(originalKeyName, null) : HashCrypt.getDigestHash(originalKeyName);
[javac16] ^
[javac16] 3 warnings

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

Nicolas Malin (Jira)

[jira] [Assigned] (OFBIZ-4824) Deprecated use of org.ofbiz.base.crypto.HashCrypt

[ https://issues.apache.org/jira/browse/OFBIZ-4824?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Adam Heath reassigned OFBIZ-4824:
---------------------------------

Assignee: Adam Heath

> Deprecated use of org.ofbiz.base.crypto.HashCrypt
> -------------------------------------------------
>
> Key: OFBIZ-4824
> URL: https://issues.apache.org/jira/browse/OFBIZ-4824
> Project: OFBiz
> Issue Type: Bug
> Components: framework
> Affects Versions: SVN trunk
> Reporter: Pierre Smits
> Assignee: Adam Heath
> Fix For: SVN trunk
>
>
> Hash
> classes:
> [javac16] Compiling 140 source files to /ci/ofbiz/framework/entity/build/classes
> [javac16] /ci/ofbiz/framework/entity/src/org/ofbiz/entity/GenericEntity.java:1299: warning: [deprecation] getDigestHash(java.lang.String) in org.ofbiz.base.crypto.HashCrypt has been deprecated
> [javac16] curValue = HashCrypt.getDigestHash(encryptField);
> [javac16] ^
> [javac16] /ci/ofbiz/framework/entity/src/org/ofbiz/entity/util/EntityCrypto.java:122: warning: [deprecation] getDigestHashOldFunnyHexEncode(java.lang.String,java.lang.String) in org.ofbiz.base.crypto.HashCrypt has been deprecated
> [javac16] String hashedKeyName = useOldFunnyKeyHash? HashCrypt.getDigestHashOldFunnyHexEncode(originalKeyName, null) : HashCrypt.getDigestHash(originalKeyName);
> [javac16] ^
> [javac16] /ci/ofbiz/framework/entity/src/org/ofbiz/entity/util/EntityCrypto.java:122: warning: [deprecation] getDigestHash(java.lang.String) in org.ofbiz.base.crypto.HashCrypt has been deprecated
> [javac16] String hashedKeyName = useOldFunnyKeyHash? HashCrypt.getDigestHashOldFunnyHexEncode(originalKeyName, null) : HashCrypt.getDigestHash(originalKeyName);
> [javac16] ^
> [javac16] 3 warnings

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

Nicolas Malin (Jira)

[jira] [Commented] (OFBIZ-4824) Deprecated use of org.ofbiz.base.crypto.HashCrypt

In reply to this post by Nicolas Malin (Jira)

[ https://issues.apache.org/jira/browse/OFBIZ-4824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13257602#comment-13257602 ]

Adam Heath commented on OFBIZ-4824:
-----------------------------------

I have fixed GenericEntity locally. However, it means that every time GenericEntity.toString() is called, you will get a *different* string, as a random salt is prepended to encrypted fields. But really, no one should be comparing the exact string output anyways.

And, actually, that call in toString is really truly broken. It should do the same thing as EntityCrypto; to do anything less, would mean that PCI compliance is not being met.

Fixing EntityCrypto is a bit more difficult. I don't want to just @SuppressWarnings("deprecation") on these methods; they really need to use the new and improved methods. However, I can't just switch completely, 'cuz then all existing crypted fields will no longer load.

Stay tuned.

Nicolas Malin (Jira)

[jira] [Commented] (OFBIZ-4824) Deprecated use of org.ofbiz.base.crypto.HashCrypt

In reply to this post by Nicolas Malin (Jira)

[ https://issues.apache.org/jira/browse/OFBIZ-4824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13257723#comment-13257723 ]

Adam Heath commented on OFBIZ-4824:
-----------------------------------

Wow, EntityCrypt is full of all sorts of fun stuff.

First, the constructor creates 20 random keys if none are found in the database. Those 20 keys could never possibly be used. That loop should just be removed.

Second, getKey(String, boolean) has broken synchronization on the keyMap.get/put pair. We've been lucky that is hasn't entered into a loop inside HashMap. This is problably due to the first item above. 20 keys get stored in the map, which is enough to cause the map to *not* resize it's internal buckets, when later keys get requested.

Next, getRandomString() is not secure. That's the point of the SecureRandom class. This is a simple fix, however.

Also, the transaction suspending that it doesn't doesn't do the right thing if there is an OutOfMemory thrown, or other Error or RuntimeException. It really needs to do that cleanup inside a finally. Fortunately, that's what TransactionUtil.doNewTransaction is for.

Ideally, decrypt(), when it calls getKey(), shouldn't be creating *new* keys in the database, nor storing into the keyMap.