OK before I revert that you might consider reading
https://stackoverflow.com/questions/13991100/where-do-you-store-your-secret-key-in-a-java-web-application
http://movingfast.io/articles/environment-variables-considered-harmful/
https://security.stackexchange.com/questions/49725/is-it-really-secure-to-store-api-keys-in-environment-variables
https://stackoverflow.com/questions/13991100/where-do-you-store-your-secret-key-in-a-java-web-application
But it's up to you of course, if you are still OK to use this way I'm totally
OK to revert it
Jacques
Le 05/11/2017 à 15:10, Michael Brohl a écrit :
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