geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jack Cai <greensi...@gmail.com>
Subject Re: Encrypt password in deployment plans and config.ser
Date Tue, 21 Jul 2009 14:36:23 GMT
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);
>> >
>> >
>> >
>>
>
>

Mime
View raw message