Re: Encrypt password in deployment plans and config.ser
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
Re: Encrypt password in deployment plans and config.ser
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 === ---
Re: Encrypt password in deployment plans and config.ser
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:
Re: Encrypt password in deployment plans and config.ser
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 11)
+++
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.EntryString, Object entry : attributes.entrySet()) {
String name =
Encrypt password in deployment plans and config.ser
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 11)
+++
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.EntryString, 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);
Re: Encrypt password in deployment plans and config.ser
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 11)
+++
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.EntryString, 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);
