You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by vy...@apache.org on 2021/04/08 14:36:18 UTC

[logging-log4j2] branch LOG4J2-3056 created (now 483bd3a)

This is an automated email from the ASF dual-hosted git repository.

vy pushed a change to branch LOG4J2-3056
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git.


      at 483bd3a  LOG4J2-3056 Refactored MD5 usage for sharing sensitive information.

This branch includes the following new commits:

     new 483bd3a  LOG4J2-3056 Refactored MD5 usage for sharing sensitive information.

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


[logging-log4j2] 01/01: LOG4J2-3056 Refactored MD5 usage for sharing sensitive information.

Posted by vy...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

vy pushed a commit to branch LOG4J2-3056
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git

commit 483bd3af2e2d4938a5ecfb3bef2eb29612bb02fc
Author: Volkan Yazici <vo...@gmail.com>
AuthorDate: Thu Apr 8 16:02:42 2021 +0200

    LOG4J2-3056 Refactored MD5 usage for sharing sensitive information.
---
 .../plugins/visitors/PluginAttributeVisitor.java   |  3 +-
 .../visitors/PluginBuilderAttributeVisitor.java    |  3 +-
 .../apache/logging/log4j/core/net/SmtpManager.java | 29 +++++++++++++++---
 .../apache/logging/log4j/core/util/NameUtil.java   | 35 ++++++++++++++--------
 .../logging/log4j/couchdb/CouchDbProvider.java     | 11 +++----
 .../log4j/flume/appender/FlumeEmbeddedManager.java | 21 +++++++++----
 .../logging/log4j/mongodb3/MongoDbProvider.java    |  4 +--
 7 files changed, 70 insertions(+), 36 deletions(-)

diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginAttributeVisitor.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginAttributeVisitor.java
index f4da42b..3cca001 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginAttributeVisitor.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginAttributeVisitor.java
@@ -23,7 +23,6 @@ import org.apache.logging.log4j.core.LogEvent;
 import org.apache.logging.log4j.core.config.Configuration;
 import org.apache.logging.log4j.core.config.Node;
 import org.apache.logging.log4j.core.config.plugins.PluginAttribute;
-import org.apache.logging.log4j.core.util.NameUtil;
 import org.apache.logging.log4j.util.StringBuilders;
 
 /**
@@ -43,7 +42,7 @@ public class PluginAttributeVisitor extends AbstractPluginVisitor<PluginAttribut
         final String replacedValue = this.substitutor.replace(event, rawValue);
         final Object defaultValue = findDefaultValue(event);
         final Object value = convert(replacedValue, defaultValue);
-        final Object debugValue = this.annotation.sensitive() ? NameUtil.md5(value + this.getClass().getName()) : value;
+        final Object debugValue = this.annotation.sensitive() ? "*****" : value;
         StringBuilders.appendKeyDqValue(log, name, debugValue);
         return value;
     }
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginBuilderAttributeVisitor.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginBuilderAttributeVisitor.java
index e951456..ae222a6 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginBuilderAttributeVisitor.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginBuilderAttributeVisitor.java
@@ -23,7 +23,6 @@ import org.apache.logging.log4j.core.LogEvent;
 import org.apache.logging.log4j.core.config.Configuration;
 import org.apache.logging.log4j.core.config.Node;
 import org.apache.logging.log4j.core.config.plugins.PluginBuilderAttribute;
-import org.apache.logging.log4j.core.util.NameUtil;
 import org.apache.logging.log4j.util.StringBuilders;
 
 /**
@@ -48,7 +47,7 @@ public class PluginBuilderAttributeVisitor extends AbstractPluginVisitor<PluginB
         final String rawValue = removeAttributeValue(attributes, name, this.aliases);
         final String replacedValue = this.substitutor.replace(event, rawValue);
         final Object value = convert(replacedValue, null);
-        final Object debugValue = this.annotation.sensitive() ? NameUtil.md5(value + this.getClass().getName()) : value;
+        final Object debugValue = this.annotation.sensitive() ? "*****" : value;
         StringBuilders.appendKeyDqValue(log, name, debugValue);
         return value;
     }
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/SmtpManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/SmtpManager.java
index c637859..323c5a8 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/SmtpManager.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/SmtpManager.java
@@ -107,7 +107,30 @@ public class SmtpManager extends AbstractManager {
             protocol = "smtp";
         }
 
+        final String name = createManagerName(to, cc, bcc, from, replyTo, subject, protocol, host, username, password, isDebug, filterName);
+        final Serializer subjectSerializer = PatternLayout.newSerializerBuilder().setConfiguration(config).setPattern(subject).build();
+
+        return getManager(name, FACTORY, new FactoryData(to, cc, bcc, from, replyTo, subjectSerializer,
+            protocol, host, port, username, password, isDebug, numElements, sslConfiguration));
+
+    }
+
+    private static String createManagerName(
+            final String to,
+            final String cc,
+            final String bcc,
+            final String from,
+            final String replyTo,
+            final String subject,
+            final String protocol,
+            final String host,
+            final String username,
+            final String password,
+            final boolean isDebug,
+            final String filterName) {
+
         final StringBuilder sb = new StringBuilder();
+
         if (to != null) {
             sb.append(to);
         }
@@ -143,11 +166,9 @@ public class SmtpManager extends AbstractManager {
         sb.append(isDebug ? ":debug:" : "::");
         sb.append(filterName);
 
-        final String name = "SMTP:" + NameUtil.md5(sb.toString());
-        final Serializer subjectSerializer = PatternLayout.newSerializerBuilder().setConfiguration(config).setPattern(subject).build();
+        final String hash = NameUtil.md5(sb.toString());
+        return "SMTP:" + hash;
 
-        return getManager(name, FACTORY, new FactoryData(to, cc, bcc, from, replyTo, subjectSerializer,
-            protocol, host, port, username, password, isDebug, numElements, sslConfiguration));
     }
 
     /**
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NameUtil.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NameUtil.java
index 4d4cfcc..dd255c8 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NameUtil.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NameUtil.java
@@ -16,44 +16,55 @@
  */
 package org.apache.logging.log4j.core.util;
 
-import java.security.MessageDigest;
-
 import org.apache.logging.log4j.util.Strings;
 
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.util.Objects;
+
 /**
  *
  */
 public final class NameUtil {
 
-    private static final int MASK = 0xff;
-
-    private NameUtil() {
-    }
+    private NameUtil() {}
 
     public static String getSubName(final String name) {
-        if (Strings.isEmpty(name)) {
+        if (Strings.isBlank(name)) {
             return null;
         }
         final int i = name.lastIndexOf('.');
         return i > 0 ? name.substring(0, i) : Strings.EMPTY;
     }
 
-    public static String md5(final String string) {
+    /**
+     * Calculates the <a href="https://en.wikipedia.org/wiki/MD5">MD5</a> hash
+     * of the given input string.
+     * <p>
+     * <b>MD5 has severe vulnerabilities and should not be used for sharing any
+     * sensitive information.</b> This function should only be used to create
+     * unique identifiers, e.g., configuration element names.
+     *
+     * @return string composed of 32 hexadecimal digits of the calculated hash
+     */
+    public static String md5(final String input) {
+        Objects.requireNonNull(input, "input");
         try {
             final MessageDigest digest = MessageDigest.getInstance("MD5");
-            digest.update(string.getBytes());
+            digest.update(input.getBytes());
             final byte[] bytes = digest.digest();
             final StringBuilder md5 = new StringBuilder();
             for (final byte b : bytes) {
-                final String hex = Integer.toHexString(MASK & b);
+                final String hex = Integer.toHexString(0xFF & b);
                 if (hex.length() == 1) {
                     md5.append('0');
                 }
                 md5.append(hex);
             }
             return md5.toString();
-        } catch (final Exception ex) {
-            return string;
+        } catch (final NoSuchAlgorithmException error) {
+            throw new RuntimeException(error);
         }
     }
+
 }
diff --git a/log4j-couchdb/src/main/java/org/apache/logging/log4j/couchdb/CouchDbProvider.java b/log4j-couchdb/src/main/java/org/apache/logging/log4j/couchdb/CouchDbProvider.java
index 942b292..3cd56e1 100644
--- a/log4j-couchdb/src/main/java/org/apache/logging/log4j/couchdb/CouchDbProvider.java
+++ b/log4j-couchdb/src/main/java/org/apache/logging/log4j/couchdb/CouchDbProvider.java
@@ -16,23 +16,22 @@
  */
 package org.apache.logging.log4j.couchdb;
 
-import java.lang.reflect.Method;
-
 import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.core.appender.nosql.NoSqlProvider;
 import org.apache.logging.log4j.core.config.plugins.Plugin;
 import org.apache.logging.log4j.core.config.plugins.PluginAttribute;
 import org.apache.logging.log4j.core.config.plugins.PluginFactory;
 import org.apache.logging.log4j.core.config.plugins.convert.TypeConverters;
 import org.apache.logging.log4j.core.config.plugins.validation.constraints.ValidHost;
 import org.apache.logging.log4j.core.config.plugins.validation.constraints.ValidPort;
-import org.apache.logging.log4j.core.util.NameUtil;
-import org.apache.logging.log4j.core.appender.nosql.NoSqlProvider;
 import org.apache.logging.log4j.status.StatusLogger;
 import org.apache.logging.log4j.util.LoaderUtil;
 import org.apache.logging.log4j.util.Strings;
 import org.lightcouch.CouchDbClient;
 import org.lightcouch.CouchDbProperties;
 
+import java.lang.reflect.Method;
+
 /**
  * The Apache CouchDB implementation of {@link NoSqlProvider}.
  */
@@ -106,7 +105,6 @@ public final class CouchDbProvider implements NoSqlProvider<CouchDbConnection> {
                     final CouchDbProperties properties = (CouchDbProperties) object;
                     client = new CouchDbClient(properties);
                     description = "uri=" + client.getDBUri() + ", username=" + properties.getUsername()
-                            + ", passwordHash=" + NameUtil.md5(password + CouchDbProvider.class.getName())
                             + ", maxConnections=" + properties.getMaxConnections() + ", connectionTimeout="
                             + properties.getConnectionTimeout() + ", socketTimeout=" + properties.getSocketTimeout();
                 } else {
@@ -150,8 +148,7 @@ public final class CouchDbProvider implements NoSqlProvider<CouchDbConnection> {
             }
 
             client = new CouchDbClient(databaseName, false, protocol, server, portInt, username, password);
-            description = "uri=" + client.getDBUri() + ", username=" + username + ", passwordHash="
-                    + NameUtil.md5(password + CouchDbProvider.class.getName());
+            description = "uri=" + client.getDBUri() + ", username=" + username;
         } else {
             LOGGER.error("No factory method was provided so the database name is required.");
             return null;
diff --git a/log4j-flume-ng/src/main/java/org/apache/logging/log4j/flume/appender/FlumeEmbeddedManager.java b/log4j-flume-ng/src/main/java/org/apache/logging/log4j/flume/appender/FlumeEmbeddedManager.java
index 7c6840a..c98ee59 100644
--- a/log4j-flume-ng/src/main/java/org/apache/logging/log4j/flume/appender/FlumeEmbeddedManager.java
+++ b/log4j-flume-ng/src/main/java/org/apache/logging/log4j/flume/appender/FlumeEmbeddedManager.java
@@ -32,16 +32,13 @@ import org.apache.logging.log4j.core.util.NameUtil;
 import org.apache.logging.log4j.util.PropertiesUtil;
 import org.apache.logging.log4j.util.Strings;
 
-/**
- *
- */
 public class FlumeEmbeddedManager extends AbstractFlumeManager {
 
     private static final String FILE_SEP = PropertiesUtil.getProperties().getStringProperty("file.separator");
 
     private static final String IN_MEMORY = "InMemory";
 
-    private static FlumeManagerFactory factory = new FlumeManagerFactory();
+    private static final FlumeManagerFactory FACTORY = new FlumeManagerFactory();
 
     private final EmbeddedAgent agent;
 
@@ -82,6 +79,17 @@ public class FlumeEmbeddedManager extends AbstractFlumeManager {
             throw new IllegalArgumentException("Cannot configure both Agents and Properties.");
         }
 
+        final String extendedName = extendManagerName(name, agents, properties);
+        return getManager(extendedName, FACTORY,
+                new FactoryData(name, agents, properties, batchSize, dataDir));
+
+    }
+
+    private static String extendManagerName(
+            final String name,
+            final Agent[] agents,
+            final Property[] properties) {
+
         final StringBuilder sb = new StringBuilder();
         boolean first = true;
 
@@ -106,8 +114,9 @@ public class FlumeEmbeddedManager extends AbstractFlumeManager {
             }
             sb.append(NameUtil.md5(props.toString()));
         }
-        return getManager(sb.toString(), factory,
-                new FactoryData(name, agents, properties, batchSize, dataDir));
+
+        return sb.toString();
+
     }
 
     @Override
diff --git a/log4j-mongodb3/src/main/java/org/apache/logging/log4j/mongodb3/MongoDbProvider.java b/log4j-mongodb3/src/main/java/org/apache/logging/log4j/mongodb3/MongoDbProvider.java
index 05ff99d..ce9fe98 100644
--- a/log4j-mongodb3/src/main/java/org/apache/logging/log4j/mongodb3/MongoDbProvider.java
+++ b/log4j-mongodb3/src/main/java/org/apache/logging/log4j/mongodb3/MongoDbProvider.java
@@ -30,7 +30,6 @@ import org.apache.logging.log4j.core.config.plugins.validation.constraints.Requi
 import org.apache.logging.log4j.core.config.plugins.validation.constraints.ValidHost;
 import org.apache.logging.log4j.core.config.plugins.validation.constraints.ValidPort;
 import org.apache.logging.log4j.core.filter.AbstractFilterable;
-import org.apache.logging.log4j.core.util.NameUtil;
 import org.apache.logging.log4j.status.StatusLogger;
 import org.apache.logging.log4j.util.LoaderUtil;
 import org.apache.logging.log4j.util.Strings;
@@ -178,8 +177,7 @@ public final class MongoDbProvider implements NoSqlProvider<MongoDbConnection> {
                 MongoCredential mongoCredential = null;
                 description = "database=" + databaseName;
                 if (Strings.isNotEmpty(userName) && Strings.isNotEmpty(password)) {
-                    description += ", username=" + userName + ", passwordHash="
-                            + NameUtil.md5(password + MongoDbProvider.class.getName());
+                    description += ", username=" + userName;
                     mongoCredential = MongoCredential.createCredential(userName, databaseName, password.toCharArray());
                 }
                 try {