geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ivan <xhh...@gmail.com>
Subject Re: Encrypt password in deployment plans and config.ser
Date Tue, 21 Jul 2009 14:50:09 GMT
gbeanInfo is used to delare the class name of the gbean, it is used to add
new gbean in the config.xml.
eg. <gbean name="..." gbeanInfo="org.apache.myGBean">

2009/7/21 Jack Cai <greensight@gmail.com>

> I've completed most of the coding for this small feature. Now I need to
> figure out a way to get the "encrypted" meta-information from GAttributeInfo
> in the GBeanOverride class so that it will encrypt attributes according to
> the meta-information when saved to config.xml. This turns out to be a
> non-trivial work, as GBeanOverride does not maintain any direct/indirect
> reference to GAttributeInfo! It does contain a "gbeanInfo" field, but its
> type is String and I cannot understand how its value could be used in any
> way.
>
> One possible solution is to duplicate the "encrypted“ meta-information in
> the config.xml, in a similar way as what we are doing with "null"
> attributes. But apparently this is not the ideal way. Does anybody have a
> better idea? Thanks a lot!
>
> -Jack
>
>
>
> On Wed, May 13, 2009 at 4:04 PM, Jack Cai <greensight@gmail.com> wrote:
>
>> Thanks Jarek for your comments! I should have provided more description
>> about the fix -
>>
>> 1. setAttribute() is usually called by the unmarshaller, so it should
>> actually do the decryption. Since the decryption will automatically bypass
>> unecrypted values, it won't do any harm if the passed-in attribute value is
>> not encrypted.
>>
>> 2. getAttribute() is usually called by various consumers that wants to
>> read the real value (decrypted), so it just returns what is stored in the
>> GBeanData (already decrypted value).
>>
>> 3. The current patch does do encryption for writeExternal(). The reason
>> decryption is not done for readExternal() directly is because readExternal()
>> calls setAttribute() which does the decryption.
>>
>> I agree that it's not optimal to hard-code to encrypt "password"
>> attributes, and better to use the GAttributeInfo to make this configurable.
>> My suggestion is to set encryption on for all attributes whose name contains
>> "password" and off for the rest. We can thus eliminate the hard-code logic
>> in GBeanOverride too (for encrption in config.xml).
>>
>> I'm going to create a patch if no objection.
>>
>> -Jack
>>
>> 2009/5/13 Jarek Gawor <jgawor@gmail.com>
>>
>> Jack,
>>>
>>> I was thinking about this a while ago and had a slightly different
>>> approach. I was thinking of adding a 'protected' or 'encrypted'
>>> attribute to the GAttributeInfo (for example, just like the
>>> 'persistent' attribute) to indicate that this attribute should be
>>> "encrypted" in the serialized file. I wanted to allow for any
>>> attribute to be encrypted even if it doesn't have the "password" name
>>> in it. And I think with this way we could also expose that setting via
>>> our GBean annotations.
>>>
>>> Btw, hmm... shouldn't encrypt be called in setAttribute() and decrypt
>>> in getAttribute() or at least encrypt in writeExternal() and decrypt
>>> in readExternal() ?
>>>
>>> Jarek
>>>
>>> On Tue, May 12, 2009 at 4:06 AM, Jack Cai <greensight@gmail.com> wrote:
>>> > Recently someone pointed out to us that password specified in the
>>> deployment
>>> > plan is stored in clear text in the config.ser after deployment, for
>>> > example, when deploying a datasource with the database password
>>> specified in
>>> > the deployment plan. I notice that there was another user mentioning
>>> exactly
>>> > the same problem in the geronimo-user list two years ago [1].
>>> >
>>> > I did a little more dig and also found this JIRA [2] along with this
>>> > discussion [3] on encrypting passwords in deployment plans.
>>> >
>>> > I understand that there are different arguments on what is "real"
>>> security.
>>> > But I also well appreciate users' concerns on having clear text
>>> password
>>> > appearing in the system. In China, we have a saying "guard against the
>>> good
>>> > guys but not the bad guys", meaning the guard is there just to prevent
>>> the
>>> > good guys from doing bad. Taking the same example as in the old thread
>>> [3],
>>> > if we lock a bicycle, then the good guys won't steal it, while the bad
>>> guys
>>> > with the intention to steal it can still find ways to steal it. But if
>>> we
>>> > leave the bicycle unlocked, then the good guys are tempted to steal the
>>> > bicycle too, because it's so easy.
>>> >
>>> > Back to JIRA [2], I think an alternative is to let user input the
>>> password
>>> > in encrypted form in the deployment plan at the very beginning. We can
>>> > provide a small command line tool to let user ecrypt the password
>>> > beforehands. If this is acceptable, then there is a very simple way to
>>> > satisfy requirement [1] & [2]. We can simply add a little encryption
>>> logic
>>> > in the class org.apache.geronimo.gbean.GBeanData [4], similar to what
>>> we did
>>> > in GBeanOverride for config.xml.
>>> >
>>> > Comments are welcome.
>>> >
>>> > -Jack
>>> >
>>> >
>>> > [1]
>>> http://www.nabble.com/plaintext-password-in-config.ser-to9834211.html
>>> > [2] http://issues.apache.org/jira/browse/GERONIMO-3003
>>> > [3]
>>> >
>>> http://www.nabble.com/Plaintext-passwords-in-Geronimo-plans-and-config-files-td9100565s134.html
>>> > [4]
>>> > Index:
>>> >
>>> D:/Dev/s/wasce_v2.1.0.1/server/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/gbean/GBeanData.java
>>> > ===================================================================
>>> > ---
>>> >
>>> D:/Dev/s/wasce_v2.1.0.1/server/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/gbean/GBeanData.java
>>> > (revision 111111)
>>> > +++
>>> >
>>> D:/Dev/s/wasce_v2.1.0.1/server/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/gbean/GBeanData.java
>>> > (working copy)
>>> > @@ -27,6 +27,8 @@
>>> >  import java.util.Map;
>>> >  import java.util.Set;
>>> >
>>> > +import org.apache.geronimo.crypto.EncryptionManager;
>>> > +
>>> >  /**
>>> >   * @version $Rev: 556119 $ $Date: 2007-07-13 15:34:02 -0400 (Fri, 13
>>> Jul
>>> > 2007) $
>>> >   */
>>> > @@ -112,6 +114,10 @@
>>> >      }
>>> >
>>> >      public void setAttribute(String name, Object value) {
>>> > +        if (name.toLowerCase().indexOf("password") > -1
>>> > +                && value instanceof String) {
>>> > +            value = EncryptionManager.decrypt((String) value);
>>> > +        }
>>> >          attributes.put(name, value);
>>> >      }
>>> >
>>> > @@ -207,6 +213,10 @@
>>> >          for (Map.Entry<String, Object> entry : attributes.entrySet())
>>> {
>>> >              String name = entry.getKey();
>>> >              Object value = entry.getValue();
>>> > +            if (name.toLowerCase().indexOf("password") > -1
>>> > +                    && value instanceof String) {
>>> > +                value = EncryptionManager.encrypt((String) value);
>>> > +            }
>>> >              try {
>>> >                  out.writeObject(name);
>>> >                  out.writeObject(value);
>>> >
>>> >
>>> >
>>>
>>
>>
>


-- 
Ivan

Mime
View raw message