There are multiple ways to implement this solution, by far my least favorite is hardcoding and then recompiling! Also hardening security and credentials is usually not done in the generic code but in the specific deployment environment which differs from person to person or company to company.
If you are not careful enough in securing your file system then you might as well consider your security a disaster. Many many frameworks store credentials in text in the file system e.g. drupal, suitecrm, rails, wordpress, django, joomla ... and the list goes on. Oh .. and where does OFBiz store JDBC passwords? Wouldn't that be in entityengine.xml? Oh and what about the socket for AdminServer? What about anything you want to keep secret? Do you hard code everything? Security is a strategy. It is a combination of approaches for encryption, networking, file systems, databases, templating and dozens of other factors and each person implements these factors differently. My recommendation is to revert and get community consensus on adopting any security strategy (or any strategy for that matter). On Nov 5, 2017 8:49 PM, "Jacques Le Roux" <jacques.le.r...@les7arts.com> wrote: > 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/mai >>> n/java/org/apache/ofbiz/securityext/login/LoginEvents.java >>> ofbiz/ofbiz-framework/trunk/framework/security/config/securi >>> ty.properties >>> >>> Modified: ofbiz/ofbiz-framework/trunk/applications/securityext/src/mai >>> n/java/org/apache/ofbiz/securityext/login/LoginEvents.java >>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/app >>> lications/securityext/src/main/java/org/apache/ofbiz/securit >>> yext/login/LoginEvents.java?rev=1814349&r1=1814348&r2=1814349&view=diff >>> ============================================================ >>> ================== >>> --- ofbiz/ofbiz-framework/trunk/applications/securityext/src/mai >>> n/java/org/apache/ofbiz/securityext/login/LoginEvents.java (original) >>> +++ ofbiz/ofbiz-framework/trunk/applications/securityext/src/mai >>> n/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.randomAlphan >>> umeric(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/securi >>> ty.properties >>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/fra >>> mework/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 >>> >>> >>> >> >> >
