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:19 UTC
[logging-log4j2] 01/01: LOG4J2-3056 Refactored MD5 usage for
sharing sensitive information.
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 {