ofbiz-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jacques Le Roux <jacques.le.r...@les7arts.com>
Subject Re: svn commit: r1814349 - in /ofbiz/ofbiz-framework/trunk: applications/securityext/src/main/java/org/apache/ofbiz/securityext/login/L oginEvents.java framework/security/config/security.properties
Date Sun, 05 Nov 2017 18:50:39 GMT
Le 05/11/2017 à 19:35, Taher Alkhateeb a écrit :
> There are multiple ways to implement this solution, by far my least
> favorite is hardcoding and then recompiling!
It's not harcoding it's automatically generating using uuidgen while compiling. So you get
a temporary uuid that only you know.
Of course you should not keep the source files on the production server after compiling.

> 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.
Of course this is was is intended to do with this way.
It's not the only way, and maybe not  the best, we need a ML brainstorming and a consensus...

> 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?
Yes that would be an easy secure way to also for those things IMO

> 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.
Yes I don't prevent you to use your own :) I just suggest this as it's better than what we
have currently and is easy to use.
And by proposing something OOTB we help our users to build more secure production environments
and be aware about it.

> My recommendation is to revert and get community consensus on adopting any
> security strategy (or any strategy for that matter).
There is no hurry to revert in trunk. I'm ready to discuss a security strategy and get a consensus
for our next releases.

Jacques
>
> On Nov 5, 2017 8:49 PM, "Jacques Le Roux" <jacques.le.roux@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 jleroux@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
>>>>
>>>>
>>>>
>>>


Mime
View raw message