You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by or...@apache.org on 2020/09/02 19:32:50 UTC
[qpid-broker-j] 03/10: QPID-8460: [Broker-j] Do not expose private
information to exception message (#55)
This is an automated email from the ASF dual-hosted git repository.
orudyy pushed a commit to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/qpid-broker-j.git
commit aa934369a6313dd26b501ed50405c4c200bd346b
Author: vavrtom <cz...@tiscali.cz>
AuthorDate: Thu Aug 6 23:41:42 2020 +0200
QPID-8460: [Broker-j] Do not expose private information to exception message (#55)
Co-authored-by: Tomas Vavricka <to...@deutsche-boerse.com>
(cherry picked from commit 734fb3994f74ae066b587d3563322f10e47e6727)
---
.../server/model/ConfiguredAutomatedAttribute.java | 3 +-
.../AbstractScramAuthenticationManager.java | 24 +++++++++-------
.../crammd5/CramMd5Base64HashedNegotiator.java | 25 +++++++---------
.../sasl/crammd5/CramMd5Base64HexNegotiator.java | 33 +++++++++-------------
.../java/org/apache/qpid/server/util/Strings.java | 24 +++++++++++++---
.../auth/BasicAuthPreemptiveAuthenticator.java | 24 ++++++++--------
6 files changed, 70 insertions(+), 63 deletions(-)
diff --git a/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredAutomatedAttribute.java b/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredAutomatedAttribute.java
index a5b3598..d98a96f 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredAutomatedAttribute.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/model/ConfiguredAutomatedAttribute.java
@@ -255,7 +255,8 @@ public class ConfiguredAutomatedAttribute<C extends ConfiguredObject, T> extend
Type returnType = getGetter().getGenericReturnType();
String simpleName = returnType instanceof Class ? ((Class) returnType).getSimpleName() : returnType.toString();
- throw new IllegalArgumentException("Cannot convert '" + value
+ throw new IllegalArgumentException("Cannot convert '" +
+ (isSecure() ? AbstractConfiguredObject.SECURED_STRING_VALUE : value)
+ "' into a " + simpleName
+ " for attribute " + getName()
+ " (" + iae.getMessage() + ")", iae);
diff --git a/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/AbstractScramAuthenticationManager.java b/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/AbstractScramAuthenticationManager.java
index e8bb81e..abc6cbf 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/AbstractScramAuthenticationManager.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/AbstractScramAuthenticationManager.java
@@ -159,17 +159,18 @@ public abstract class AbstractScramAuthenticationManager<X extends AbstractScram
final String[] passwordFields = user.getPassword().split(",");
if (passwordFields.length == 2)
{
- byte[] saltedPassword = Strings.decodeBase64(passwordFields[PasswordField.SALTED_PASSWORD.ordinal()]);
+ final byte[] saltedPassword = Strings.decodePrivateBase64(passwordFields[PasswordField.SALTED_PASSWORD.ordinal()],
+ "user '" + user.getName() + "' salted password");
try
{
- byte[] clientKey = computeHmac(saltedPassword, "Client Key");
+ final byte[] clientKey = computeHmac(saltedPassword, "Client Key");
- byte[] storedKey = MessageDigest.getInstance(getDigestName()).digest(clientKey);
+ final byte[] storedKey = MessageDigest.getInstance(getDigestName()).digest(clientKey);
- byte[] serverKey = computeHmac(saltedPassword, "Server Key");
+ final byte[] serverKey = computeHmac(saltedPassword, "Server Key");
- String password = passwordFields[PasswordField.SALT.ordinal()] + ","
+ final String password = passwordFields[PasswordField.SALT.ordinal()] + ","
+ "," // remove previously insecure salted password field
+ Base64.getEncoder().encodeToString(storedKey) + ","
+ Base64.getEncoder().encodeToString(serverKey) + ","
@@ -183,7 +184,7 @@ public abstract class AbstractScramAuthenticationManager<X extends AbstractScram
}
else if (passwordFields.length == 4)
{
- String password = passwordFields[PasswordField.SALT.ordinal()] + ","
+ final String password = passwordFields[PasswordField.SALT.ordinal()] + ","
+ "," // remove previously insecure salted password field
+ passwordFields[PasswordField.STORED_KEY.ordinal()] + ","
+ passwordFields[PasswordField.SERVER_KEY.ordinal()] + ","
@@ -296,7 +297,7 @@ public abstract class AbstractScramAuthenticationManager<X extends AbstractScram
@Override
public SaltAndPasswordKeys getSaltAndPasswordKeys(final String username)
{
- ManagedUser user = getUser(username);
+ final ManagedUser user = getUser(username);
final byte[] salt;
final byte[] storedKey;
@@ -318,9 +319,12 @@ public abstract class AbstractScramAuthenticationManager<X extends AbstractScram
{
updateStoredPasswordFormatIfNecessary(user);
final String[] passwordFields = user.getPassword().split(",");
- salt = Strings.decodeBase64(passwordFields[PasswordField.SALT.ordinal()]);
- storedKey = Strings.decodeBase64(passwordFields[PasswordField.STORED_KEY.ordinal()]);
- serverKey = Strings.decodeBase64(passwordFields[PasswordField.SERVER_KEY.ordinal()]);
+ salt = Strings.decodePrivateBase64(passwordFields[PasswordField.SALT.ordinal()],
+ "user '" + user.getName() + "' salt");
+ storedKey = Strings.decodePrivateBase64(passwordFields[PasswordField.STORED_KEY.ordinal()],
+ "user '" + user.getName() + "' stored key");
+ serverKey = Strings.decodePrivateBase64(passwordFields[PasswordField.SERVER_KEY.ordinal()],
+ "user '" + user.getName() + "' server key");
iterationCount = Integer.parseInt(passwordFields[PasswordField.ITERATION_COUNT.ordinal()]);
exception = null;
}
diff --git a/broker-core/src/main/java/org/apache/qpid/server/security/auth/sasl/crammd5/CramMd5Base64HashedNegotiator.java b/broker-core/src/main/java/org/apache/qpid/server/security/auth/sasl/crammd5/CramMd5Base64HashedNegotiator.java
index 3d3a551..91c9daa 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/security/auth/sasl/crammd5/CramMd5Base64HashedNegotiator.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/security/auth/sasl/crammd5/CramMd5Base64HashedNegotiator.java
@@ -27,21 +27,16 @@ import org.apache.qpid.server.util.Strings;
public class CramMd5Base64HashedNegotiator extends AbstractCramMd5Negotiator
{
public static final String MECHANISM = "CRAM-MD5-HASHED";
- private static final PasswordTransformer BASE64_PASSWORD_TRANSFORMER =
- new PasswordTransformer()
- {
- @Override
- public char[] transform(final char[] passwordData)
- {
- byte[] passwordBytes = Strings.decodeBase64(new String(passwordData));
- char[] password = new char[passwordBytes.length];
- for (int i = 0; i < passwordBytes.length; i++)
- {
- password[i] = (char) passwordBytes[i];
- }
- return password;
- }
- };
+ private static final PasswordTransformer BASE64_PASSWORD_TRANSFORMER = passwordData ->
+ {
+ final byte[] passwordBytes = Strings.decodePrivateBase64(new String(passwordData), "CRAM MD5 hashed password");
+ final char[] password = new char[passwordBytes.length];
+ for (int i = 0; i < passwordBytes.length; i++)
+ {
+ password[i] = (char) passwordBytes[i];
+ }
+ return password;
+ };
public CramMd5Base64HashedNegotiator(final PasswordCredentialManagingAuthenticationProvider<?> authenticationProvider,
final String localFQDN,
diff --git a/broker-core/src/main/java/org/apache/qpid/server/security/auth/sasl/crammd5/CramMd5Base64HexNegotiator.java b/broker-core/src/main/java/org/apache/qpid/server/security/auth/sasl/crammd5/CramMd5Base64HexNegotiator.java
index 22fc95c..edcbff5 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/security/auth/sasl/crammd5/CramMd5Base64HexNegotiator.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/security/auth/sasl/crammd5/CramMd5Base64HexNegotiator.java
@@ -27,26 +27,19 @@ import org.apache.qpid.server.util.Strings;
public class CramMd5Base64HexNegotiator extends AbstractCramMd5Negotiator
{
public static final String MECHANISM = "CRAM-MD5-HEX";
- private static final PasswordTransformer BASE64_HEX_PASSWORD_TRANSFORMER =
- new PasswordTransformer()
- {
- private final char[] HEX_CHARACTERS =
- {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f'};
-
- @Override
- public char[] transform(final char[] passwordData)
- {
- byte[] passwordBytes = Strings.decodeBase64(new String(passwordData));
- char[] password = new char[passwordBytes.length * 2];
-
- for (int i = 0; i < passwordBytes.length; i++)
- {
- password[2 * i] = HEX_CHARACTERS[(((int) passwordBytes[i]) & 0xf0) >> 4];
- password[(2 * i) + 1] = HEX_CHARACTERS[(((int) passwordBytes[i]) & 0x0f)];
- }
- return password;
- }
- };
+ private static final char[] HEX_CHARACTERS =
+ {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f'};
+ private static final PasswordTransformer BASE64_HEX_PASSWORD_TRANSFORMER = passwordData ->
+ {
+ final byte[] passwordBytes = Strings.decodePrivateBase64(new String(passwordData), "CRAM MD5 hex password");
+ final char[] password = new char[passwordBytes.length * 2];
+ for (int i = 0; i < passwordBytes.length; i++)
+ {
+ password[2 * i] = HEX_CHARACTERS[(((int) passwordBytes[i]) & 0xf0) >> 4];
+ password[(2 * i) + 1] = HEX_CHARACTERS[(((int) passwordBytes[i]) & 0x0f)];
+ }
+ return password;
+ };
public CramMd5Base64HexNegotiator(final PasswordCredentialManagingAuthenticationProvider<?> authenticationProvider,
final String localFQDN,
diff --git a/broker-core/src/main/java/org/apache/qpid/server/util/Strings.java b/broker-core/src/main/java/org/apache/qpid/server/util/Strings.java
index 0e468fb..b62fe7f 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/util/Strings.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/util/Strings.java
@@ -21,7 +21,6 @@
package org.apache.qpid.server.util;
import java.io.UnsupportedEncodingException;
-import java.io.Writer;
import java.nio.ByteBuffer;
import java.util.Arrays;
import java.util.Base64;
@@ -129,17 +128,34 @@ public final class Strings
return resolver;
}
+ public static byte[] decodePrivateBase64(String base64String, String description)
+ {
+ if (isInvalidBase64String(base64String))
+ {
+ // do not add base64String to exception message as it can contain private data
+ throw new IllegalArgumentException("Cannot convert " + description +
+ " string to a byte[] - it does not appear to be base64 data");
+ }
+
+ return Base64.getDecoder().decode(base64String);
+ }
+
public static byte[] decodeBase64(String base64String)
{
- base64String = base64String.replaceAll("\\s","");
- if(!base64String.matches("^(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?$"))
+ if (isInvalidBase64String(base64String))
{
- throw new IllegalArgumentException("Cannot convert string '"+ base64String+ "'to a byte[] - it does not appear to be base64 data");
+ throw new IllegalArgumentException("Cannot convert string '" + base64String +
+ "' to a byte[] - it does not appear to be base64 data");
}
return Base64.getDecoder().decode(base64String);
}
+ private static boolean isInvalidBase64String(String base64String)
+ {
+ return !base64String.replaceAll("\\s", "").matches("^(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?$");
+ }
+
public static interface Resolver
{
String resolve(String variable, final Resolver resolver);
diff --git a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/BasicAuthPreemptiveAuthenticator.java b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/BasicAuthPreemptiveAuthenticator.java
index 76353f8..c79d4bd 100644
--- a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/BasicAuthPreemptiveAuthenticator.java
+++ b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/BasicAuthPreemptiveAuthenticator.java
@@ -45,16 +45,16 @@ public class BasicAuthPreemptiveAuthenticator implements HttpRequestPreemptiveAu
@Override
public Subject attemptAuthentication(final HttpServletRequest request, final HttpManagementConfiguration managementConfiguration)
{
- String header = request.getHeader("Authorization");
+ final String header = request.getHeader("Authorization");
final Port<?> port = managementConfiguration.getPort(request);
final AuthenticationProvider<?> authenticationProvider = managementConfiguration.getAuthenticationProvider(request);
- SubjectCreator subjectCreator = port.getSubjectCreator(request.isSecure(), request.getServerName());
+ final SubjectCreator subjectCreator = port.getSubjectCreator(request.isSecure(), request.getServerName());
if (header != null && authenticationProvider instanceof UsernamePasswordAuthenticationProvider)
{
- UsernamePasswordAuthenticationProvider<?> namePasswdAuthProvider = (UsernamePasswordAuthenticationProvider<?>)authenticationProvider;
+ final UsernamePasswordAuthenticationProvider<?> namePasswdAuthProvider = (UsernamePasswordAuthenticationProvider<?>)authenticationProvider;
- String[] tokens = header.split("\\s");
+ final String[] tokens = header.split("\\s");
if (tokens.length >= 2 && "BASIC".equalsIgnoreCase(tokens[0]))
{
boolean isBasicAuthSupported = false;
@@ -68,19 +68,17 @@ public class BasicAuthPreemptiveAuthenticator implements HttpRequestPreemptiveAu
}
if (isBasicAuthSupported)
{
- String base64UsernameAndPassword = tokens[1];
- String[] credentials = (new String(Strings.decodeBase64(base64UsernameAndPassword),
- StandardCharsets.UTF_8)).split(":", 2);
+ final String base64UsernameAndPassword = tokens[1];
+ final String[] credentials = new String(Strings.decodePrivateBase64(base64UsernameAndPassword,
+ "basic authentication credentials"), StandardCharsets.UTF_8).split(":", 2);
if (credentials.length == 2)
{
- String username = credentials[0];
- String password = credentials[1];
- AuthenticationResult authenticationResult = namePasswdAuthProvider.authenticate(username, password);
- SubjectAuthenticationResult result = subjectCreator.createResultWithGroups(authenticationResult);
+ final String username = credentials[0];
+ final String password = credentials[1];
+ final AuthenticationResult authenticationResult = namePasswdAuthProvider.authenticate(username, password);
+ final SubjectAuthenticationResult result = subjectCreator.createResultWithGroups(authenticationResult);
return result.getSubject();
-
-
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org