You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@logging.apache.org by Matt Sicker <bo...@gmail.com> on 2017/06/26 14:56:28 UTC

Re: logging-log4j2 git commit: [LOG4J2-1956] JMS Appender broker password should be a char[], not a String.

This makes me think that char[] fields/params should have the
sensitive=true set by default.

On 25 June 2017 at 21:31, <gg...@apache.org> wrote:

> Repository: logging-log4j2
> Updated Branches:
>   refs/heads/master 07248cdbb -> bcd60cc6d
>
>
> [LOG4J2-1956] JMS Appender broker password should be a char[], not a
> String.
>
> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
> Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/
> commit/bcd60cc6
> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/bcd60cc6
> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/bcd60cc6
>
> Branch: refs/heads/master
> Commit: bcd60cc6d1b6d2cb314be7a8d31c8d534fd51f4f
> Parents: 07248cd
> Author: Gary Gregory <gg...@apache.org>
> Authored: Sun Jun 25 19:31:40 2017 -0700
> Committer: Gary Gregory <gg...@apache.org>
> Committed: Sun Jun 25 19:31:40 2017 -0700
>
> ----------------------------------------------------------------------
>  .../mom/activemq/JmsAppenderConnectPostStartupIT.java        | 2 +-
>  .../appender/mom/activemq/JmsAppenderConnectReConnectIT.java | 2 +-
>  .../appender/mom/activemq/JmsAppenderITcpConnectionIT.java   | 2 +-
>  .../core/appender/mom/activemq/JmsClientTestConfig.java      | 6 +++---
>  .../core/appender/mom/activemq/JmsClientTestConfigRule.java  | 8 ++++----
>  5 files changed, 10 insertions(+), 10 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/
> bcd60cc6/log4j-core-its/src/test/java/org/apache/logging/
> log4j/core/appender/mom/activemq/JmsAppenderConnectPostStartupIT.java
> ----------------------------------------------------------------------
> diff --git a/log4j-core-its/src/test/java/org/apache/logging/log4j/
> core/appender/mom/activemq/JmsAppenderConnectPostStartupIT.java
> b/log4j-core-its/src/test/java/org/apache/logging/log4j/
> core/appender/mom/activemq/JmsAppenderConnectPostStartupIT.java
> index a7391ba..bf3d50c 100644
> --- a/log4j-core-its/src/test/java/org/apache/logging/log4j/
> core/appender/mom/activemq/JmsAppenderConnectPostStartupIT.java
> +++ b/log4j-core-its/src/test/java/org/apache/logging/log4j/
> core/appender/mom/activemq/JmsAppenderConnectPostStartupIT.java
> @@ -55,7 +55,7 @@ public class JmsAppenderConnectPostStartupIT extends
> AbstractJmsAppenderIT {
>
>         // "admin"/"admin" are the default Apache Active MQ creds.
>         private static final JmsClientTestConfigRule
> jmsClientTestConfigRule = new JmsClientTestConfigRule(
> -                       ActiveMQInitialContextFactory.class.getName(),
> "tcp://localhost:" + portRule.getPort(), "admin", "admin");
> +                       ActiveMQInitialContextFactory.class.getName(),
> "tcp://localhost:" + portRule.getPort(), "admin", "admin".toCharArray());
>
>         /**
>          * Assign the port and client ONCE for the whole test suite.
>
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/
> bcd60cc6/log4j-core-its/src/test/java/org/apache/logging/
> log4j/core/appender/mom/activemq/JmsAppenderConnectReConnectIT.java
> ----------------------------------------------------------------------
> diff --git a/log4j-core-its/src/test/java/org/apache/logging/log4j/
> core/appender/mom/activemq/JmsAppenderConnectReConnectIT.java
> b/log4j-core-its/src/test/java/org/apache/logging/log4j/
> core/appender/mom/activemq/JmsAppenderConnectReConnectIT.java
> index 82b8f39..8944d5b 100644
> --- a/log4j-core-its/src/test/java/org/apache/logging/log4j/
> core/appender/mom/activemq/JmsAppenderConnectReConnectIT.java
> +++ b/log4j-core-its/src/test/java/org/apache/logging/log4j/
> core/appender/mom/activemq/JmsAppenderConnectReConnectIT.java
> @@ -70,7 +70,7 @@ public class JmsAppenderConnectReConnectIT {
>                                 .startBrokerService(
> JmsAppenderConnectReConnectIT.class.getName(), brokerUrlString, port);
>                 // Start appender
>                 final JmsClientTestConfig jmsClientTestConfig = new
> JmsClientTestConfig(ActiveMQInitialContextFactory.class.getName(),
> -                               brokerUrlString, "admin", "admin");
> +                               brokerUrlString, "admin",
> "admin".toCharArray());
>                 jmsClientTestConfig.start();
>                 final JmsAppender appender = jmsClientTestConfig.
> createAppender(MessageLayout.createLayout());
>                 // Log message
>
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/
> bcd60cc6/log4j-core-its/src/test/java/org/apache/logging/
> log4j/core/appender/mom/activemq/JmsAppenderITcpConnectionIT.java
> ----------------------------------------------------------------------
> diff --git a/log4j-core-its/src/test/java/org/apache/logging/log4j/
> core/appender/mom/activemq/JmsAppenderITcpConnectionIT.java
> b/log4j-core-its/src/test/java/org/apache/logging/log4j/
> core/appender/mom/activemq/JmsAppenderITcpConnectionIT.java
> index c32f911..f88d588 100644
> --- a/log4j-core-its/src/test/java/org/apache/logging/log4j/
> core/appender/mom/activemq/JmsAppenderITcpConnectionIT.java
> +++ b/log4j-core-its/src/test/java/org/apache/logging/log4j/
> core/appender/mom/activemq/JmsAppenderITcpConnectionIT.java
> @@ -50,7 +50,7 @@ public class JmsAppenderITcpConnectionIT extends
> AbstractJmsAppenderIT {
>
>         // "admin"/"admin" are the default Apache Active MQ creds.
>         public static final JmsClientTestConfigRule
> jmsClientTestConfigRule = new JmsClientTestConfigRule(
> -                       activeMqBrokerServiceRule,
> ActiveMQInitialContextFactory.class.getName(), "admin", "admin");
> +                       activeMqBrokerServiceRule,
> ActiveMQInitialContextFactory.class.getName(), "admin",
> "admin".toCharArray());
>
>         /**
>          * We assign a port only ONCE ands start the broker ONCE for the
> whole test
>
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/
> bcd60cc6/log4j-core-its/src/test/java/org/apache/logging/
> log4j/core/appender/mom/activemq/JmsClientTestConfig.java
> ----------------------------------------------------------------------
> diff --git a/log4j-core-its/src/test/java/org/apache/logging/log4j/
> core/appender/mom/activemq/JmsClientTestConfig.java
> b/log4j-core-its/src/test/java/org/apache/logging/log4j/
> core/appender/mom/activemq/JmsClientTestConfig.java
> index 17ac788..e65238d 100644
> --- a/log4j-core-its/src/test/java/org/apache/logging/log4j/
> core/appender/mom/activemq/JmsClientTestConfig.java
> +++ b/log4j-core-its/src/test/java/org/apache/logging/log4j/
> core/appender/mom/activemq/JmsClientTestConfig.java
> @@ -16,11 +16,11 @@ class JmsClientTestConfig {
>         private JmsAppender jmsAppender;
>         private final String jmsInitialContextFactoryClassName;
>         private JmsManager jmsManager;
> -       private final String jmsPassword;
> +       private final char[] jmsPassword;
>         private final String jmsProviderUrlStr;
>         private final String jmsUserName;
>
> -       JmsClientTestConfig(final String jmsInitialContextFactoryClassName,
> final String jmsProviderUrlStr, final String jmsUserName, final String
> jmsPassword) {
> +       JmsClientTestConfig(final String jmsInitialContextFactoryClassName,
> final String jmsProviderUrlStr, final String jmsUserName, final char[]
> jmsPassword) {
>                 this.jmsInitialContextFactoryClassName =
> jmsInitialContextFactoryClassName;
>                 this.jmsProviderUrlStr = jmsProviderUrlStr;
>                 this.jmsUserName = jmsUserName;
> @@ -52,7 +52,7 @@ class JmsClientTestConfig {
>                 return jmsManager;
>         }
>
> -       String getJmsPassword() {
> +       char[] getJmsPassword() {
>                 return jmsPassword;
>         }
>
>
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/
> bcd60cc6/log4j-core-its/src/test/java/org/apache/logging/
> log4j/core/appender/mom/activemq/JmsClientTestConfigRule.java
> ----------------------------------------------------------------------
> diff --git a/log4j-core-its/src/test/java/org/apache/logging/log4j/
> core/appender/mom/activemq/JmsClientTestConfigRule.java
> b/log4j-core-its/src/test/java/org/apache/logging/log4j/
> core/appender/mom/activemq/JmsClientTestConfigRule.java
> index 9cb2e8f..8c48d40 100644
> --- a/log4j-core-its/src/test/java/org/apache/logging/log4j/
> core/appender/mom/activemq/JmsClientTestConfigRule.java
> +++ b/log4j-core-its/src/test/java/org/apache/logging/log4j/
> core/appender/mom/activemq/JmsClientTestConfigRule.java
> @@ -15,11 +15,11 @@ class JmsClientTestConfigRule implements TestRule {
>         final String brokerUrlStr;
>         private JmsClientTestConfig jmsClientTestConfig;
>         final String jmsInitialContextFactoryClassName;
> -       final String password;
> +       final char[] password;
>         final String userName;
>
>         public JmsClientTestConfigRule(final ActiveMqBrokerServiceRule
> activeMqBrokerServiceRule,
> -                       final String jmsInitialContextFactoryClassName,
> final String userName, final String password) {
> +                       final String jmsInitialContextFactoryClassName,
> final String userName, final char[] password) {
>                 this.activeMqBrokerServiceRule = activeMqBrokerServiceRule;
>                 this.jmsInitialContextFactoryClassName =
> jmsInitialContextFactoryClassName;
>                 this.brokerUrlStr = null;
> @@ -28,7 +28,7 @@ class JmsClientTestConfigRule implements TestRule {
>         }
>
>         public JmsClientTestConfigRule(final String
> jmsInitialContextFactoryClassName, final String brokerUrlStr, final
> String userName,
> -                       final String password) {
> +                       final char[] password) {
>                 this.activeMqBrokerServiceRule = null;
>                 this.jmsInitialContextFactoryClassName =
> jmsInitialContextFactoryClassName;
>                 this.brokerUrlStr = brokerUrlStr;
> @@ -73,7 +73,7 @@ class JmsClientTestConfigRule implements TestRule {
>                 return jmsInitialContextFactoryClassName;
>         }
>
> -       String getPassword() {
> +       char[] getPassword() {
>                 return password;
>         }
>
>
>


-- 
Matt Sicker <bo...@gmail.com>

Re: logging-log4j2 git commit: [LOG4J2-1956] JMS Appender broker password should be a char[], not a String.

Posted by Gary Gregory <ga...@gmail.com>.
Could be. That falls into the better safe than sorry category. If some
plugin uses a char[] because it wants to/need to and it is not a sensitive
item, then it's not that big of a deal to say 'sensitve = true' if you
realize you do want that. I am not sure how often anyone would use a char[]
for anything other than a password.

Gary

On Mon, Jun 26, 2017 at 7:56 AM, Matt Sicker <bo...@gmail.com> wrote:

> This makes me think that char[] fields/params should have the
> sensitive=true set by default.
>
> On 25 June 2017 at 21:31, <gg...@apache.org> wrote:
>
> > Repository: logging-log4j2
> > Updated Branches:
> >   refs/heads/master 07248cdbb -> bcd60cc6d
> >
> >
> > [LOG4J2-1956] JMS Appender broker password should be a char[], not a
> > String.
> >
> > Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
> > Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/
> > commit/bcd60cc6
> > Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/
> bcd60cc6
> > Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/
> bcd60cc6
> >
> > Branch: refs/heads/master
> > Commit: bcd60cc6d1b6d2cb314be7a8d31c8d534fd51f4f
> > Parents: 07248cd
> > Author: Gary Gregory <gg...@apache.org>
> > Authored: Sun Jun 25 19:31:40 2017 -0700
> > Committer: Gary Gregory <gg...@apache.org>
> > Committed: Sun Jun 25 19:31:40 2017 -0700
> >
> > ----------------------------------------------------------------------
> >  .../mom/activemq/JmsAppenderConnectPostStartupIT.java        | 2 +-
> >  .../appender/mom/activemq/JmsAppenderConnectReConnectIT.java | 2 +-
> >  .../appender/mom/activemq/JmsAppenderITcpConnectionIT.java   | 2 +-
> >  .../core/appender/mom/activemq/JmsClientTestConfig.java      | 6 +++---
> >  .../core/appender/mom/activemq/JmsClientTestConfigRule.java  | 8
> ++++----
> >  5 files changed, 10 insertions(+), 10 deletions(-)
> > ----------------------------------------------------------------------
> >
> >
> > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/
> > bcd60cc6/log4j-core-its/src/test/java/org/apache/logging/
> > log4j/core/appender/mom/activemq/JmsAppenderConnectPostStartupIT.java
> > ----------------------------------------------------------------------
> > diff --git a/log4j-core-its/src/test/java/org/apache/logging/log4j/
> > core/appender/mom/activemq/JmsAppenderConnectPostStartupIT.java
> > b/log4j-core-its/src/test/java/org/apache/logging/log4j/
> > core/appender/mom/activemq/JmsAppenderConnectPostStartupIT.java
> > index a7391ba..bf3d50c 100644
> > --- a/log4j-core-its/src/test/java/org/apache/logging/log4j/
> > core/appender/mom/activemq/JmsAppenderConnectPostStartupIT.java
> > +++ b/log4j-core-its/src/test/java/org/apache/logging/log4j/
> > core/appender/mom/activemq/JmsAppenderConnectPostStartupIT.java
> > @@ -55,7 +55,7 @@ public class JmsAppenderConnectPostStartupIT extends
> > AbstractJmsAppenderIT {
> >
> >         // "admin"/"admin" are the default Apache Active MQ creds.
> >         private static final JmsClientTestConfigRule
> > jmsClientTestConfigRule = new JmsClientTestConfigRule(
> > -                       ActiveMQInitialContextFactory.class.getName(),
> > "tcp://localhost:" + portRule.getPort(), "admin", "admin");
> > +                       ActiveMQInitialContextFactory.class.getName(),
> > "tcp://localhost:" + portRule.getPort(), "admin", "admin".toCharArray());
> >
> >         /**
> >          * Assign the port and client ONCE for the whole test suite.
> >
> > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/
> > bcd60cc6/log4j-core-its/src/test/java/org/apache/logging/
> > log4j/core/appender/mom/activemq/JmsAppenderConnectReConnectIT.java
> > ----------------------------------------------------------------------
> > diff --git a/log4j-core-its/src/test/java/org/apache/logging/log4j/
> > core/appender/mom/activemq/JmsAppenderConnectReConnectIT.java
> > b/log4j-core-its/src/test/java/org/apache/logging/log4j/
> > core/appender/mom/activemq/JmsAppenderConnectReConnectIT.java
> > index 82b8f39..8944d5b 100644
> > --- a/log4j-core-its/src/test/java/org/apache/logging/log4j/
> > core/appender/mom/activemq/JmsAppenderConnectReConnectIT.java
> > +++ b/log4j-core-its/src/test/java/org/apache/logging/log4j/
> > core/appender/mom/activemq/JmsAppenderConnectReConnectIT.java
> > @@ -70,7 +70,7 @@ public class JmsAppenderConnectReConnectIT {
> >                                 .startBrokerService(
> > JmsAppenderConnectReConnectIT.class.getName(), brokerUrlString, port);
> >                 // Start appender
> >                 final JmsClientTestConfig jmsClientTestConfig = new
> > JmsClientTestConfig(ActiveMQInitialContextFactory.class.getName(),
> > -                               brokerUrlString, "admin", "admin");
> > +                               brokerUrlString, "admin",
> > "admin".toCharArray());
> >                 jmsClientTestConfig.start();
> >                 final JmsAppender appender = jmsClientTestConfig.
> > createAppender(MessageLayout.createLayout());
> >                 // Log message
> >
> > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/
> > bcd60cc6/log4j-core-its/src/test/java/org/apache/logging/
> > log4j/core/appender/mom/activemq/JmsAppenderITcpConnectionIT.java
> > ----------------------------------------------------------------------
> > diff --git a/log4j-core-its/src/test/java/org/apache/logging/log4j/
> > core/appender/mom/activemq/JmsAppenderITcpConnectionIT.java
> > b/log4j-core-its/src/test/java/org/apache/logging/log4j/
> > core/appender/mom/activemq/JmsAppenderITcpConnectionIT.java
> > index c32f911..f88d588 100644
> > --- a/log4j-core-its/src/test/java/org/apache/logging/log4j/
> > core/appender/mom/activemq/JmsAppenderITcpConnectionIT.java
> > +++ b/log4j-core-its/src/test/java/org/apache/logging/log4j/
> > core/appender/mom/activemq/JmsAppenderITcpConnectionIT.java
> > @@ -50,7 +50,7 @@ public class JmsAppenderITcpConnectionIT extends
> > AbstractJmsAppenderIT {
> >
> >         // "admin"/"admin" are the default Apache Active MQ creds.
> >         public static final JmsClientTestConfigRule
> > jmsClientTestConfigRule = new JmsClientTestConfigRule(
> > -                       activeMqBrokerServiceRule,
> > ActiveMQInitialContextFactory.class.getName(), "admin", "admin");
> > +                       activeMqBrokerServiceRule,
> > ActiveMQInitialContextFactory.class.getName(), "admin",
> > "admin".toCharArray());
> >
> >         /**
> >          * We assign a port only ONCE ands start the broker ONCE for the
> > whole test
> >
> > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/
> > bcd60cc6/log4j-core-its/src/test/java/org/apache/logging/
> > log4j/core/appender/mom/activemq/JmsClientTestConfig.java
> > ----------------------------------------------------------------------
> > diff --git a/log4j-core-its/src/test/java/org/apache/logging/log4j/
> > core/appender/mom/activemq/JmsClientTestConfig.java
> > b/log4j-core-its/src/test/java/org/apache/logging/log4j/
> > core/appender/mom/activemq/JmsClientTestConfig.java
> > index 17ac788..e65238d 100644
> > --- a/log4j-core-its/src/test/java/org/apache/logging/log4j/
> > core/appender/mom/activemq/JmsClientTestConfig.java
> > +++ b/log4j-core-its/src/test/java/org/apache/logging/log4j/
> > core/appender/mom/activemq/JmsClientTestConfig.java
> > @@ -16,11 +16,11 @@ class JmsClientTestConfig {
> >         private JmsAppender jmsAppender;
> >         private final String jmsInitialContextFactoryClassName;
> >         private JmsManager jmsManager;
> > -       private final String jmsPassword;
> > +       private final char[] jmsPassword;
> >         private final String jmsProviderUrlStr;
> >         private final String jmsUserName;
> >
> > -       JmsClientTestConfig(final String jmsInitialContextFactoryClassN
> ame,
> > final String jmsProviderUrlStr, final String jmsUserName, final String
> > jmsPassword) {
> > +       JmsClientTestConfig(final String jmsInitialContextFactoryClassN
> ame,
> > final String jmsProviderUrlStr, final String jmsUserName, final char[]
> > jmsPassword) {
> >                 this.jmsInitialContextFactoryClassName =
> > jmsInitialContextFactoryClassName;
> >                 this.jmsProviderUrlStr = jmsProviderUrlStr;
> >                 this.jmsUserName = jmsUserName;
> > @@ -52,7 +52,7 @@ class JmsClientTestConfig {
> >                 return jmsManager;
> >         }
> >
> > -       String getJmsPassword() {
> > +       char[] getJmsPassword() {
> >                 return jmsPassword;
> >         }
> >
> >
> > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/
> > bcd60cc6/log4j-core-its/src/test/java/org/apache/logging/
> > log4j/core/appender/mom/activemq/JmsClientTestConfigRule.java
> > ----------------------------------------------------------------------
> > diff --git a/log4j-core-its/src/test/java/org/apache/logging/log4j/
> > core/appender/mom/activemq/JmsClientTestConfigRule.java
> > b/log4j-core-its/src/test/java/org/apache/logging/log4j/
> > core/appender/mom/activemq/JmsClientTestConfigRule.java
> > index 9cb2e8f..8c48d40 100644
> > --- a/log4j-core-its/src/test/java/org/apache/logging/log4j/
> > core/appender/mom/activemq/JmsClientTestConfigRule.java
> > +++ b/log4j-core-its/src/test/java/org/apache/logging/log4j/
> > core/appender/mom/activemq/JmsClientTestConfigRule.java
> > @@ -15,11 +15,11 @@ class JmsClientTestConfigRule implements TestRule {
> >         final String brokerUrlStr;
> >         private JmsClientTestConfig jmsClientTestConfig;
> >         final String jmsInitialContextFactoryClassName;
> > -       final String password;
> > +       final char[] password;
> >         final String userName;
> >
> >         public JmsClientTestConfigRule(final ActiveMqBrokerServiceRule
> > activeMqBrokerServiceRule,
> > -                       final String jmsInitialContextFactoryClassName,
> > final String userName, final String password) {
> > +                       final String jmsInitialContextFactoryClassName,
> > final String userName, final char[] password) {
> >                 this.activeMqBrokerServiceRule =
> activeMqBrokerServiceRule;
> >                 this.jmsInitialContextFactoryClassName =
> > jmsInitialContextFactoryClassName;
> >                 this.brokerUrlStr = null;
> > @@ -28,7 +28,7 @@ class JmsClientTestConfigRule implements TestRule {
> >         }
> >
> >         public JmsClientTestConfigRule(final String
> > jmsInitialContextFactoryClassName, final String brokerUrlStr, final
> > String userName,
> > -                       final String password) {
> > +                       final char[] password) {
> >                 this.activeMqBrokerServiceRule = null;
> >                 this.jmsInitialContextFactoryClassName =
> > jmsInitialContextFactoryClassName;
> >                 this.brokerUrlStr = brokerUrlStr;
> > @@ -73,7 +73,7 @@ class JmsClientTestConfigRule implements TestRule {
> >                 return jmsInitialContextFactoryClassName;
> >         }
> >
> > -       String getPassword() {
> > +       char[] getPassword() {
> >                 return password;
> >         }
> >
> >
> >
>
>
> --
> Matt Sicker <bo...@gmail.com>
>