DeltaSpike
  1. DeltaSpike
  2. DELTASPIKE-382

mask out passwords and other credentials in our Configuration logs

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.4
    • Fix Version/s: 0.5
    • Component/s: Configuration
    • Labels:
      None

      Description

      Our configuration mechanism currently logs all the configured values.
      This makes it hard to use it for passwords and stuff.

      I suggest we introduce some specific prefix property to configure configs which contain sensitive information.

      For the key 'some.random.password' this could look like:

      deltaspike_config.mask.some.random.password=true

      In the log we would in this case just output the information whether and where we did find some value, but not print the details for all configs which start with all of the configured masks.

      I'm not yet sure though how to configure this best. Suggestions appreciated!

        Activity

        Mark Struberg created issue -
        Hide
        Romain Manni-Bucau added a comment -

        i'd see a SPI "PropertyMasker" or sthg like that called before logging (isLoggable). By default the impl would be: if key contains pwd or password return false; return true;

        Show
        Romain Manni-Bucau added a comment - i'd see a SPI "PropertyMasker" or sthg like that called before logging (isLoggable). By default the impl would be: if key contains pwd or password return false; return true;
        Hide
        Gerhard Petracek added a comment -

        i would never store passwords as plain text

        Show
        Gerhard Petracek added a comment - i would never store passwords as plain text
        Hide
        Mark Struberg added a comment -

        Gerhard, what are the other options?
        It's not perfect but it's fine for most cases to store credentials, etc in JNDI.
        If there is some foreign bytecode running on your server, then you never will have 100% safety.
        For DB one should e.g. use a DataSource provided by the Server, but for other connectors it's not as easy.

        Romain Manni-Bucau I like the SPI idea.

        Show
        Mark Struberg added a comment - Gerhard, what are the other options? It's not perfect but it's fine for most cases to store credentials, etc in JNDI. If there is some foreign bytecode running on your server, then you never will have 100% safety. For DB one should e.g. use a DataSource provided by the Server, but for other connectors it's not as easy. Romain Manni-Bucau I like the SPI idea.
        Hide
        Denis Forveille added a comment -

        I agree with Gerhard. never store a pwd in plain text. at a minimun, encrypt it and decrypt (3DES..) it when needed, at least those are not immediately readable by anyone..

        Show
        Denis Forveille added a comment - I agree with Gerhard. never store a pwd in plain text. at a minimun, encrypt it and decrypt (3DES..) it when needed, at least those are not immediately readable by anyone..
        Hide
        Mark Struberg added a comment -

        Denis, this all is hopefully stored in JNDI thus only maintained by system administrators and only readable by the code on this box. And if someone manages to execute code on your box, then you have much heavier problems than a password.
        This whole masking is to make sure we don't output some sensitive information (might be other stuff than passwords as well) into some random log file which might be read by many other users.

        Show
        Mark Struberg added a comment - Denis, this all is hopefully stored in JNDI thus only maintained by system administrators and only readable by the code on this box. And if someone manages to execute code on your box, then you have much heavier problems than a password. This whole masking is to make sure we don't output some sensitive information (might be other stuff than passwords as well) into some random log file which might be read by many other users.
        Hide
        Gerhard Petracek added a comment - - edited

        @denis: yes (or a hash-code created by a cryptographic hash-function)
        imo we shouldn't support such very questionable approaches.

        Show
        Gerhard Petracek added a comment - - edited @denis: yes (or a hash-code created by a cryptographic hash-function) imo we shouldn't support such very questionable approaches.
        Hide
        Gerhard Petracek added a comment -

        @mark: i know what you mean, but i can't agree - the same is true for logs. if you store a password in plain text (in db, jndi,...), anybody who would/should get access to the logs could also read and expose the password manually. if you expose your logs to external people, you have to check them anyway.

        Show
        Gerhard Petracek added a comment - @mark: i know what you mean, but i can't agree - the same is true for logs. if you store a password in plain text (in db, jndi,...), anybody who would/should get access to the logs could also read and expose the password manually. if you expose your logs to external people, you have to check them anyway.
        Hide
        Romain Manni-Bucau added a comment -

        in openejb we support encryption. Basically i think you are both right so we need to handle it. That said a custom log category should be enough to prevent it to be logged in not dev environment.

        Show
        Romain Manni-Bucau added a comment - in openejb we support encryption. Basically i think you are both right so we need to handle it. That said a custom log category should be enough to prevent it to be logged in not dev environment.
        Hide
        Christian Kaltepoth added a comment - - edited

        I think we should support some way of preventing passwords to be logged. Of cause it would be great if everyone uses encryption, but that's not realistic. And even if you use encryption, you have to take special care to keep the key secret, which is difficult without something like a HSM. If the key isn't safe (for example compiled into the source), even encrypted passwords are insecure and shouldn't be logged.

        I like the idea of an SPI for that.

        Just one thought: What about an SPI instead that tells which config properties shouldn't be logged at all (instead of just masking them). This could be useful even in other situations. For example if people have MANY config entries and only want to log a subset of them. Just a thought.

        Show
        Christian Kaltepoth added a comment - - edited I think we should support some way of preventing passwords to be logged. Of cause it would be great if everyone uses encryption, but that's not realistic. And even if you use encryption, you have to take special care to keep the key secret, which is difficult without something like a HSM. If the key isn't safe (for example compiled into the source), even encrypted passwords are insecure and shouldn't be logged. I like the idea of an SPI for that. Just one thought: What about an SPI instead that tells which config properties shouldn't be logged at all (instead of just masking them). This could be useful even in other situations. For example if people have MANY config entries and only want to log a subset of them. Just a thought.
        Hide
        Gerhard Petracek added a comment -

        the only thing which sounds useful to me is a thin logging facade/factory/... to integrate any logging framework (or such questionable concepts) without additional dependency.
        imo if users don't care about their passwords/logs/..., most of them also don't care about masking configs and/or don't check if they get logged.
        or just a custom category as mentioned by romain.

        Show
        Gerhard Petracek added a comment - the only thing which sounds useful to me is a thin logging facade/factory/... to integrate any logging framework (or such questionable concepts) without additional dependency. imo if users don't care about their passwords/logs/..., most of them also don't care about masking configs and/or don't check if they get logged. or just a custom category as mentioned by romain.
        Hide
        Mark Struberg added a comment -

        Gerhard, SOME don't care but others DO!

        Also your argument is really not well funded. There is only 1 solution to be completely safe: to let the OS take care about security ans use a trusted key infrastructure. But once someone can inject/run foreign code on your server, then ALL bets are off.

        Sometimes you do need to keep a password around for your application to log in into another system. And regardless whether you do use a synchronous encryption or not - there is always a way to reconstruct this password. Storing it into JNDI should be sufficiently safe for most users. If it's not enough for them then they shall not use it, but for most people I've talked it it's really sufficient. The only important thing is that this configuration must never leave the server. And this is not guaranteed if we log it natively.

        Show
        Mark Struberg added a comment - Gerhard, SOME don't care but others DO! Also your argument is really not well funded. There is only 1 solution to be completely safe: to let the OS take care about security ans use a trusted key infrastructure. But once someone can inject/run foreign code on your server, then ALL bets are off. Sometimes you do need to keep a password around for your application to log in into another system. And regardless whether you do use a synchronous encryption or not - there is always a way to reconstruct this password. Storing it into JNDI should be sufficiently safe for most users. If it's not enough for them then they shall not use it, but for most people I've talked it it's really sufficient. The only important thing is that this configuration must never leave the server. And this is not guaranteed if we log it natively.
        Hide
        Gerhard Petracek added a comment - - edited

        i agree with romain and that also solves the cases you mentioned. or we don't log the config per default and just log it based on a system property (which is also easy enough to enable).
        i also know lots of people who agree with me, since we had this topic several times in environments of different security levels. you will never get an approach everybody agrees with, if you enable it per default.

        storing passwords as plain text and at the same time don't secure your logs and/or check them before exposing them is highly careless since projects usually use x libs as direct dependency or shipped in the server. if only one just logs all values of system-properties/jndi/... and you don't check the content of your logs before you expose them, you have the same issue.

        Show
        Gerhard Petracek added a comment - - edited i agree with romain and that also solves the cases you mentioned. or we don't log the config per default and just log it based on a system property (which is also easy enough to enable). i also know lots of people who agree with me, since we had this topic several times in environments of different security levels. you will never get an approach everybody agrees with, if you enable it per default. storing passwords as plain text and at the same time don't secure your logs and/or check them before exposing them is highly careless since projects usually use x libs as direct dependency or shipped in the server. if only one just logs all values of system-properties/jndi/... and you don't check the content of your logs before you expose them, you have the same issue.
        Hide
        Mark Struberg added a comment -

        Logging the configured values is something all my operations guy ever wanted. I don't agree with the X libs argument. If those other X libs read deltaspike configuration, well then that's up to them...

        In practice there is NO way to hide this info from code which is running on your server. Well you could activate the SecurityManager, but then 99% of all apps would even refuse to start... It doesn't care at all whether you have it store symmetrically encrypted or not. At some point in time you need to uncrypt it. End then you have all the pwd in plaintext as well.

        Once again: the only way which would technologically be valid is to use Java PKI and just utilize the private key stored in the OS or some HSM. But this would need the transport to support it. It would work for https, but e.g. not for SOAP WS-Security (regardless if https or http), nor for many other login mechanism.

        Proposed solution:
        I like Romains idea with the SPI. What about an interface 'ConfigFilter' which provides 2 different methods, one for 'real' and the other one for 'logged'/'masked' values?
        This ConfigFilter class could e.g. also used to transparently do the symmetric decryption for configuration which gets stored encrypted.

        Show
        Mark Struberg added a comment - Logging the configured values is something all my operations guy ever wanted. I don't agree with the X libs argument. If those other X libs read deltaspike configuration, well then that's up to them... In practice there is NO way to hide this info from code which is running on your server. Well you could activate the SecurityManager, but then 99% of all apps would even refuse to start... It doesn't care at all whether you have it store symmetrically encrypted or not. At some point in time you need to uncrypt it. End then you have all the pwd in plaintext as well. Once again: the only way which would technologically be valid is to use Java PKI and just utilize the private key stored in the OS or some HSM. But this would need the transport to support it. It would work for https, but e.g. not for SOAP WS-Security (regardless if https or http), nor for many other login mechanism. Proposed solution: I like Romains idea with the SPI. What about an interface 'ConfigFilter' which provides 2 different methods, one for 'real' and the other one for 'logged'/'masked' values? This ConfigFilter class could e.g. also used to transparently do the symmetric decryption for configuration which gets stored encrypted.
        Hide
        John D. Ament added a comment -

        What if it were even simpler?

        public interface ConfigPrintChecker

        { //or some other name public boolean displayKey(String key); public boolean displayMasked(String key); }

        Then let whoever create an implementation that matches their needs

        Show
        John D. Ament added a comment - What if it were even simpler? public interface ConfigPrintChecker { //or some other name public boolean displayKey(String key); public boolean displayMasked(String key); } Then let whoever create an implementation that matches their needs
        Hide
        Gerhard Petracek added a comment -

        @mark:
        in config-sources like jndi you don't only have values configured via/for deltaspike.
        exposing info from jndi is way more "common", because it doesn't even have to be the goal to expose sensitive info.
        like you said - if e.g. an operations guy is interested in all config-values (not just configured via/for ds) and just logs everything, any masking done in ds is useless.

        -> i don't agree because (again):
        checking logs before exposing them is imo by no way optional esp. if you store passwords as plain text in a common config-source which might be logged for many other reasons.
        that would be just highly careless, since there might be even other logged confidential info which isn't stored in a config.

        Show
        Gerhard Petracek added a comment - @mark: in config-sources like jndi you don't only have values configured via/for deltaspike. exposing info from jndi is way more "common", because it doesn't even have to be the goal to expose sensitive info. like you said - if e.g. an operations guy is interested in all config-values (not just configured via/for ds) and just logs everything, any masking done in ds is useless. -> i don't agree because (again): checking logs before exposing them is imo by no way optional esp. if you store passwords as plain text in a common config-source which might be logged for many other reasons. that would be just highly careless, since there might be even other logged confidential info which isn't stored in a config.
        Hide
        Mark Struberg added a comment -

        Gerhard Petracek
        > checking logs before exposing them is imo by no way optional
        sorry that's crap. I'm not talking about externally exposing them, but in a well setup scenario all Developers have access to the logs. This is one of the main parts all the DevOps movement is about: to care about all those logs TOGETHER with the operations team. Thus the developers must take care that sensitive info doesn't end up in the log at all.

        I think the ConfigFilter would provide all this and even more functionality like on the fly decryption.

        John D. Ament
        Kind of, but instead I'd return the filtered Strings

        public interface ConfigFilter

        { String filterValue(String key, String value); String filterValueForLog(String key, String value); }

        plus provide a default impl wich does only return value for the first method and the logic Romain suggested in the ForLog method.

        Show
        Mark Struberg added a comment - Gerhard Petracek > checking logs before exposing them is imo by no way optional sorry that's crap. I'm not talking about externally exposing them, but in a well setup scenario all Developers have access to the logs. This is one of the main parts all the DevOps movement is about: to care about all those logs TOGETHER with the operations team. Thus the developers must take care that sensitive info doesn't end up in the log at all. I think the ConfigFilter would provide all this and even more functionality like on the fly decryption. John D. Ament Kind of, but instead I'd return the filtered Strings public interface ConfigFilter { String filterValue(String key, String value); String filterValueForLog(String key, String value); } plus provide a default impl wich does only return value for the first method and the logic Romain suggested in the ForLog method.
        Hide
        Romain Manni-Bucau added a comment -

        Guys couldn't we simply get:

        public interface SecretPropertySelector

        { boolean isSecret(String key); }

        then handle 2 properties object: properties (like today) and secretProperties (with parent = properties). We add a method getSecretProperty() in ConfigResolver and that's all.

        The logging and all safe operations are done only on properties.

        Show
        Romain Manni-Bucau added a comment - Guys couldn't we simply get: public interface SecretPropertySelector { boolean isSecret(String key); } then handle 2 properties object: properties (like today) and secretProperties (with parent = properties). We add a method getSecretProperty() in ConfigResolver and that's all. The logging and all safe operations are done only on properties.
        Hide
        Romain Manni-Bucau added a comment -

        An alternative would be to fire an event when container is started and log with an observer at this moment, the observer would be overridable so deactivable

        Show
        Romain Manni-Bucau added a comment - An alternative would be to fire an event when container is started and log with an observer at this moment, the observer would be overridable so deactivable
        Hide
        Gerhard Petracek added a comment - - edited

        @mark:
        so far i haven't said something about the spi (that would be a different jira-ticket). i just don't agree with the out-of-the-box masking which actively supports a questionable approach.

        yes - depending on the use-case you might need to decrypt a password at some point, but without code-reviews, security audits,... you have to trust your developers anyway.
        -> ensure security in a better way. there are enough approaches >like< https://wiki.jenkins-ci.org/display/JENKINS/Mask+Passwords+Plugin which do that in a general fashion on a higher or lower level and not only within ds.
        (this one for sure not for prod. - it's just an open example which shows that you have to do it on a different level and not in a single ds-api. i can't ref. closed examples like specific handlers which would be for sure more appropriate.)

        btw. using strong language doesn't make anything more accurate and i refuse such a style in any discussion here.

        Show
        Gerhard Petracek added a comment - - edited @mark: so far i haven't said something about the spi (that would be a different jira-ticket). i just don't agree with the out-of-the-box masking which actively supports a questionable approach. yes - depending on the use-case you might need to decrypt a password at some point, but without code-reviews, security audits,... you have to trust your developers anyway. -> ensure security in a better way. there are enough approaches >like< https://wiki.jenkins-ci.org/display/JENKINS/Mask+Passwords+Plugin which do that in a general fashion on a higher or lower level and not only within ds. (this one for sure not for prod. - it's just an open example which shows that you have to do it on a different level and not in a single ds-api. i can't ref. closed examples like specific handlers which would be for sure more appropriate.) btw. using strong language doesn't make anything more accurate and i refuse such a style in any discussion here.
        Hide
        Romain Manni-Bucau added a comment -

        @Gerhard: the point is: we can't by default let pwd be logged. It is simple enough to be done by default today (ConfigurableDS will surely rely on it pretty quickly). We can't encourage no security so we need to handle it. I agree it should be only "good practices" but we can't only hope it as library providers. Jenkins mask pwd plugin is generic only on jenkins, if you have the same for any app (maybe a custom JUL handler?) it would work, without it this jira needs to be solved.

        Show
        Romain Manni-Bucau added a comment - @Gerhard: the point is: we can't by default let pwd be logged. It is simple enough to be done by default today (ConfigurableDS will surely rely on it pretty quickly). We can't encourage no security so we need to handle it. I agree it should be only "good practices" but we can't only hope it as library providers. Jenkins mask pwd plugin is generic only on jenkins, if you have the same for any app (maybe a custom JUL handler?) it would work, without it this jira needs to be solved.
        Hide
        Gerhard Petracek added a comment - - edited

        @romain:
        yes - if we change the default logging, we don't have an issue with that. i haven't said that there is nothing to change. as i said before, i haven't said something about the suggested spi which is a different story (and doesn't fit to the topic of this ticket which is about masking something out-of-the-box). i just don't agree with the need to mask something which would be only masked for accessing it via ds-apis.
        (i just can't point to closed solutions out there and so the jenkins plugin was just one of many examples how things can be done in a general fashion instead of relying on a specific api of one framework. for sure it wouldn't be active on your prod. system and a custom handler makes more sense, but i don't have a ref. to an open implementation out there.)

        Show
        Gerhard Petracek added a comment - - edited @romain: yes - if we change the default logging, we don't have an issue with that. i haven't said that there is nothing to change. as i said before, i haven't said something about the suggested spi which is a different story (and doesn't fit to the topic of this ticket which is about masking something out-of-the-box). i just don't agree with the need to mask something which would be only masked for accessing it via ds-apis. (i just can't point to closed solutions out there and so the jenkins plugin was just one of many examples how things can be done in a general fashion instead of relying on a specific api of one framework. for sure it wouldn't be active on your prod. system and a custom handler makes more sense, but i don't have a ref. to an open implementation out there.)
        Hide
        Gerhard Petracek added a comment - - edited

        to summarize it:
        #1 i don't agree with masking values for the access via a single ds-api (because it needs to be solved on a different level)
        #2 i agree that we can improve the current default handling
        #3 i agree that it should be possible to skip logging (and not masking) in some cases/stages/... (without agreeing with the password use-case -> i don't agree with the title of this ticket and the initially used use-case - see #1)
        #4 if someone uses it for use-cases not everybody here agrees with, they are responsible for it (but imo we shouldn't actively support/document it)

        Show
        Gerhard Petracek added a comment - - edited to summarize it: #1 i don't agree with masking values for the access via a single ds-api (because it needs to be solved on a different level) #2 i agree that we can improve the current default handling #3 i agree that it should be possible to skip logging (and not masking) in some cases/stages/... (without agreeing with the password use-case -> i don't agree with the title of this ticket and the initially used use-case - see #1) #4 if someone uses it for use-cases not everybody here agrees with, they are responsible for it (but imo we shouldn't actively support/document it)
        Mark Struberg made changes -
        Field Original Value New Value
        Summary mask out passwords and other credentials mask out passwords and other credentials in our Configuration logs
        Hide
        Mark Struberg added a comment -

        parsing ALL log output in production is imo not a good idea. This is way too expensive and could seriously blast performance in many situations. For jenkins it doesn't harm anyone, but in a production environment this is a no-go from a performance pov.

        ad #1: it's not a single DS method, it really affects the whole configuration area.
        ad #3: don't see it as a pure password issue, the solution with the SPI is much more flexible. As outlined above you could e.g. also use it for a decryption handler.
        ad #4: yes, I'll hack it and maintain it.

        Show
        Mark Struberg added a comment - parsing ALL log output in production is imo not a good idea. This is way too expensive and could seriously blast performance in many situations. For jenkins it doesn't harm anyone, but in a production environment this is a no-go from a performance pov. ad #1: it's not a single DS method, it really affects the whole configuration area. ad #3: don't see it as a pure password issue, the solution with the SPI is much more flexible. As outlined above you could e.g. also use it for a decryption handler. ad #4: yes, I'll hack it and maintain it.
        Hide
        Gerhard Petracek added a comment -

        again: i never said something (pro or con) about the spi. i just don't agree with masking values in our case. skipping the log in some cases is by far enough.
        and yes - it's a single api since the ConfigResolver api is affected. a proper approach for the initial use-case would handle it on a different level, because it allows to keep the logs clean (independent of the part which produces it).
        it's your responsibility, if you are using it in your project/s for passwords. skipping logging (instead of masking) without even implying a security context for other users should be enough (even for whatever you are going to do in your project).

        (it isn't about who is doing it - it's about how it's done and which message is transported to users actively via docs/supported syntax/... .)

        Show
        Gerhard Petracek added a comment - again: i never said something (pro or con) about the spi. i just don't agree with masking values in our case. skipping the log in some cases is by far enough. and yes - it's a single api since the ConfigResolver api is affected. a proper approach for the initial use-case would handle it on a different level, because it allows to keep the logs clean (independent of the part which produces it). it's your responsibility, if you are using it in your project/s for passwords. skipping logging (instead of masking) without even implying a security context for other users should be enough (even for whatever you are going to do in your project). (it isn't about who is doing it - it's about how it's done and which message is transported to users actively via docs/supported syntax/... .)
        Mark Struberg made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Gerhard Petracek made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Mark Struberg
            Reporter:
            Mark Struberg
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development