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); >> > >> > >> > >> > >
