Hi Jacques, why don't we just discuss such changes before they got implemented?
I don't think it is a valid solution to change the key in a versioned class file during compile time, that's a really strange proposal.
The configuration through the properties was fine, every responsible person should take care of changing this when he is setting up a production environment. And using a database driven SystemProperty entry completely avoids having a productive key somewhere in a file.
I don't see how this is really a security problem.This also breaks configuration solutions which deal with property file manipulations like proposed by Gil and me.
I propose to revert this. Thanks, Michael Am 05.11.17 um 12:28 schrieb jler...@apache.org:
Author: jleroux Date: Sun Nov 5 11:28:42 2017 New Revision: 1814349 URL: http://svn.apache.org/viewvc?rev=1814349&view=rev Log: Fixed: Secure the login.secret_key_string (OFBIZ-9966) When OFBIZ-4983 was implemented I missed that we put the login.secret_key_string as a property in security properties. This should not have been because it eases attackers work. The recommended way is to have it as a private static final String that can be changed just when compiling using sed and uuidgen. So then the key is temporay and final and it gets quite harder for a possible attacker to use this mean. Modified: ofbiz/ofbiz-framework/trunk/applications/securityext/src/main/java/org/apache/ofbiz/securityext/login/LoginEvents.java ofbiz/ofbiz-framework/trunk/framework/security/config/security.properties Modified: ofbiz/ofbiz-framework/trunk/applications/securityext/src/main/java/org/apache/ofbiz/securityext/login/LoginEvents.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/securityext/src/main/java/org/apache/ofbiz/securityext/login/LoginEvents.java?rev=1814349&r1=1814348&r2=1814349&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/applications/securityext/src/main/java/org/apache/ofbiz/securityext/login/LoginEvents.java (original) +++ ofbiz/ofbiz-framework/trunk/applications/securityext/src/main/java/org/apache/ofbiz/securityext/login/LoginEvents.java Sun Nov 5 11:28:42 2017 @@ -69,7 +69,12 @@ public class LoginEvents { public static final String module = LoginEvents.class.getName(); public static final String resource = "SecurityextUiLabels"; public static final String usernameCookieName = "OFBiz.Username"; - private static final String keyValue = UtilProperties.getPropertyValue(LoginWorker.securityProperties, "login.secret_key_string"); + // OOTB the loginSecretKeyString is not properly initialised and can not be OOTB. + // The best way to create the loginSecretKeyString is to use a temporary way to load in a static final key when compiling. + // This is simple and most secure. One of the proposed way is to use sed and uuidgen to modify the loginSecretKeyString value + // The magic words here are TEMPORARY and FINAL! + private static final String loginSecretKeyString = "loginSecretKeyString"; + /** * Save USERNAME and PASSWORD for use by auth pages even if we start in non-auth pages. * @@ -253,7 +258,7 @@ public class LoginEvents { autoPassword = RandomStringUtils.randomAlphanumeric(EntityUtilProperties.getPropertyAsInteger("security", "password.length.min", 5).intValue()); EntityCrypto entityCrypto = new EntityCrypto(delegator,null); try { - passwordToSend = entityCrypto.encrypt(keyValue, EncryptMethod.TRUE, autoPassword); + passwordToSend = entityCrypto.encrypt(loginSecretKeyString, EncryptMethod.TRUE, autoPassword); } catch (GeneralException e) { Debug.logWarning(e, "Problem in encryption", module); } Modified: ofbiz/ofbiz-framework/trunk/framework/security/config/security.properties URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/security/config/security.properties?rev=1814349&r1=1814348&r2=1814349&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/security/config/security.properties (original) +++ ofbiz/ofbiz-framework/trunk/framework/security/config/security.properties Sun Nov 5 11:28:42 2017 @@ -126,7 +126,5 @@ protect-view.preprocessor=java.org.apach #default.error.response.view=none: default.error.response.view=view:viewBlocked-# If false, then no externalLoginKey parameters will be added to cross-webapp urls+# -- If false, then no externalLoginKey parameters will be added to cross-webapp urls security.login.externalLoginKey.enabled=true -#Security key used to encrypt and decrypt the autogenerated password in forgot password functionality. -login.secret_key_string=Secret Key \ No newline at end of file
smime.p7s
Description: S/MIME Cryptographic Signature
