You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by cs...@apache.org on 2022/04/05 17:51:16 UTC
[activemq] branch activemq-5.17.x updated: AMQ-8550 - Check for null keystore/truststore passwords
This is an automated email from the ASF dual-hosted git repository.
cshannon pushed a commit to branch activemq-5.17.x
in repository https://gitbox.apache.org/repos/asf/activemq.git
The following commit(s) were added to refs/heads/activemq-5.17.x by this push:
new aa10747e9 AMQ-8550 - Check for null keystore/truststore passwords
aa10747e9 is described below
commit aa10747e941f9e6d26483dc9f56a86892cd36765
Author: Christopher L. Shannon <ch...@gmail.com>
AuthorDate: Tue Apr 5 12:11:10 2022 -0400
AMQ-8550 - Check for null keystore/truststore passwords
Inside ActiveMQSslConnectionFactory the passwords should be checked for
null so a NPE isn't thrown. Null will be passed to the factories instead
and the keystore/truststore factories will try and load the keystores
using null for the password which may or may not work depending on the
implementation and if password is set.
(cherry picked from commit b93d58259c86d5e1141d1004cf8d75cf4a4376a6)
---
.../activemq/ActiveMQSslConnectionFactory.java | 9 +-
.../activemq/ActiveMQSslConnectionFactoryTest.java | 109 ++++++++++++++++++++-
2 files changed, 114 insertions(+), 4 deletions(-)
diff --git a/activemq-client/src/main/java/org/apache/activemq/ActiveMQSslConnectionFactory.java b/activemq-client/src/main/java/org/apache/activemq/ActiveMQSslConnectionFactory.java
index 8bee79114..c7ba74885 100644
--- a/activemq-client/src/main/java/org/apache/activemq/ActiveMQSslConnectionFactory.java
+++ b/activemq-client/src/main/java/org/apache/activemq/ActiveMQSslConnectionFactory.java
@@ -132,7 +132,7 @@ public class ActiveMQSslConnectionFactory extends ActiveMQConnectionFactory {
if (trustStore != null) {
try(InputStream tsStream = getInputStream(trustStore)) {
- trustedCertStore.load(tsStream, trustStorePassword.toCharArray());
+ trustedCertStore.load(tsStream, trustStorePassword != null ? trustStorePassword.toCharArray() : null);
TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
tmf.init(trustedCertStore);
@@ -151,8 +151,11 @@ public class ActiveMQSslConnectionFactory extends ActiveMQConnectionFactory {
if (sslCert != null && sslCert.length > 0) {
try(ByteArrayInputStream bin = new ByteArrayInputStream(sslCert)) {
- ks.load(bin, keyStorePassword.toCharArray());
- kmf.init(ks, keyStoreKeyPassword !=null ? keyStoreKeyPassword.toCharArray() : keyStorePassword.toCharArray());
+ //A null password may not be allowed depending on implementation
+ //Check for null so an UnrecoverableKeyException is thrown if not supported or wrong instead of NullPointerException here
+ final char[] keyStorePass = keyStorePassword != null ? keyStorePassword.toCharArray() : null;
+ ks.load(bin, keyStorePass);
+ kmf.init(ks, keyStoreKeyPassword != null ? keyStoreKeyPassword.toCharArray() : keyStorePass);
keystoreManagers = kmf.getKeyManagers();
}
}
diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/ActiveMQSslConnectionFactoryTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/ActiveMQSslConnectionFactoryTest.java
index 979f37789..34c9147af 100644
--- a/activemq-unit-tests/src/test/java/org/apache/activemq/ActiveMQSslConnectionFactoryTest.java
+++ b/activemq-unit-tests/src/test/java/org/apache/activemq/ActiveMQSslConnectionFactoryTest.java
@@ -22,6 +22,7 @@ import java.io.FileInputStream;
import java.io.IOException;
import java.security.KeyStore;
import java.security.SecureRandom;
+import java.security.UnrecoverableKeyException;
import javax.net.ssl.KeyManager;
import javax.net.ssl.KeyManagerFactory;
@@ -102,6 +103,26 @@ public class ActiveMQSslConnectionFactoryTest extends CombinationTestSupport {
brokerStop();
}
+ public void testCreateSslConnectionKeyStore() throws Exception {
+ // Create SSL/TLS connection with trusted cert from truststore.
+ String sslUri = "ssl://localhost:61611";
+ broker = createSslBroker(sslUri);
+ assertNotNull(broker);
+
+ // This should create the connection.
+ ActiveMQSslConnectionFactory cf = getFactory(sslUri);
+ cf.setKeyStore("server.keystore");
+ cf.setKeyStorePassword("password");
+ cf.setTrustStore("server.keystore");
+ cf.setTrustStorePassword("password");
+ connection = (ActiveMQConnection)cf.createConnection();
+ LOG.info("Created client connection");
+ assertNotNull(connection);
+ connection.start();
+ connection.stop();
+ brokerStop();
+ }
+
public void testFailoverSslConnection() throws Exception {
// Create SSL/TLS connection with trusted cert from truststore.
String sslUri = "ssl://localhost:61611";
@@ -137,7 +158,7 @@ public class ActiveMQSslConnectionFactoryTest extends CombinationTestSupport {
brokerStop();
}
- public void testNegativeCreateSslConnectionWithWrongPassword() throws Exception {
+ public void testNegativeCreateSslConnectionWithWrongTrustStorePassword() throws Exception {
// Create SSL/TLS connection with trusted cert from truststore.
String sslUri = "ssl://localhost:61611";
broker = createSslBroker(sslUri);
@@ -151,6 +172,8 @@ public class ActiveMQSslConnectionFactoryTest extends CombinationTestSupport {
connection = (ActiveMQConnection)cf.createConnection();
}
catch (javax.jms.JMSException ignore) {
+ //Make sure it's an UnrecoverableKeyException
+ assertTrue(getRootCause(ignore) instanceof UnrecoverableKeyException);
// Expected exception
LOG.info("Expected java.io.Exception [" + ignore + "]");
}
@@ -159,6 +182,80 @@ public class ActiveMQSslConnectionFactoryTest extends CombinationTestSupport {
brokerStop();
}
+ public void testCreateSslConnectionWithNullTrustStorePassword() throws Exception {
+ // Create SSL/TLS connection with trusted cert from truststore.
+ String sslUri = "ssl://localhost:61611";
+ broker = createSslBroker(sslUri);
+ assertNotNull(broker);
+
+ // This should create the connection.
+ ActiveMQSslConnectionFactory cf = getFactory(sslUri);
+ cf.setTrustStore("server.keystore");
+ //don't set a truststore password so it's null, this caused an NPE
+ //before AMQ-8550. truststore password is used to protect the integrity
+ //but isn't needed to read the truststore
+ connection = (ActiveMQConnection)cf.createConnection();
+ LOG.info("Created client connection");
+ assertNotNull(connection);
+ connection.start();
+ connection.stop();
+ brokerStop();
+ }
+
+ public void testNegativeCreateSslConnectionWithWrongKeyStorePassword() throws Exception {
+ // Create SSL/TLS connection with keystore and trusted cert from truststore.
+ String sslUri = "ssl://localhost:61611";
+ broker = createSslBroker(sslUri);
+ assertNotNull(broker);
+
+ // This should FAIL to connect, due to wrong keystore password.
+ ActiveMQSslConnectionFactory cf = getFactory(sslUri);
+ cf.setKeyStore("server.keystore");
+ cf.setKeyStorePassword("badPassword");
+ cf.setTrustStore("server.keystore");
+ cf.setTrustStorePassword("password");
+ try {
+ connection = (ActiveMQConnection)cf.createConnection();
+ }
+ catch (javax.jms.JMSException ignore) {
+ //Make sure it's an UnrecoverableKeyException
+ assertTrue(getRootCause(ignore) instanceof UnrecoverableKeyException);
+ // Expected exception
+ LOG.info("Expected java.io.Exception [" + ignore + "]");
+ }
+ assertNull(connection);
+
+ brokerStop();
+ }
+
+ public void testNegativeCreateSslConnectionWithNullKeyStorePassword() throws Exception {
+ // Create SSL/TLS connection with keystore and trusted cert from truststore.
+ String sslUri = "ssl://localhost:61611";
+ broker = createSslBroker(sslUri);
+ assertNotNull(broker);
+
+ // This should FAIL to connect, due to null password for keystore.
+ //Before AMQ-8550 this would fail with a NPE
+ ActiveMQSslConnectionFactory cf = getFactory(sslUri);
+ cf.setKeyStore("server.keystore");
+ //don't set keystore password so it's null
+ cf.setTrustStore("server.keystore");
+ cf.setTrustStorePassword("password");
+ try {
+ connection = (ActiveMQConnection)cf.createConnection();
+ }
+ catch (javax.jms.JMSException ignore) {
+ //Make sure it's an UnrecoverableKeyException and not NullPointerException
+ assertTrue(getRootCause(ignore) instanceof UnrecoverableKeyException);
+ // Expected exception
+ LOG.info("Expected java.io.Exception [" + ignore + "]");
+ }
+ assertNull(connection);
+
+ brokerStop();
+ }
+
+
public void testNegativeCreateSslConnectionWithWrongCert() throws Exception {
// Create SSL/TLS connection with trusted cert from truststore.
String sslUri = "ssl://localhost:61611";
@@ -264,4 +361,14 @@ public class ActiveMQSslConnectionFactoryTest extends CombinationTestSupport {
return out.toByteArray();
}
+ private static Throwable getRootCause(Throwable throwable) {
+ Throwable cause = throwable.getCause();
+ Throwable rootCause = null;
+ while (cause != null) {
+ rootCause = cause;
+ cause = cause.getCause();
+ }
+ return rootCause;
+ }
+
}