Thanks Ivan. So the way I'm going to do this is to capture the encryption meta-information in GBeanOverride whenever such information is passed into it. Currently there are three palces -
1. When a GBeanOverride is constructed with a given GBeanData. 2. When a GBeanOverride is called to do "applyOverride". 3. When an attribute is set into a GBeanOverride at runtime. This ensures that all the attributes for loaded GBeans will get the up-to-date encryption information. In addition to that, I will also retain the encryption information when reading config.xml. This ensures that even if a GBean is not loaded during one server run, the encrypted attribute will continue to be encrypted the server is stopped. With the above approach, the only occasion that an attribute does not get encrypted when saving into config.xml even though it's specified as "encrypted" in GBeanInfo is that - * the attribute value is written in clear text by user by editting the config.xml, and * the GBean containing this attribute is not loaded during server runs I suppose this is a reasonable exception. Comments are welcome. -Jack On Tue, Jul 21, 2009 at 10:50 PM, Ivan <xhh...@gmail.com> wrote: > 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 <greensi...@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 <greensi...@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 <jga...@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 <greensi...@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 >
