Skip to content

Commit

Permalink
Don't override SSLContext when explicitly set up
Browse files Browse the repository at this point in the history
Fixes #12
  • Loading branch information
acogoluegnes committed Jan 2, 2017
1 parent 1ea0f7a commit dfaaf43
Show file tree
Hide file tree
Showing 10 changed files with 261 additions and 18 deletions.
140 changes: 140 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@
<rabbitmq.dir>${deps.dir}/rabbit</rabbitmq.dir>
<rabbitmqctl.bin>${rabbitmq.dir}/scripts/rabbitmqctl</rabbitmqctl.bin>

<test-keystore.ca>${project.build.directory}/ca.keystore</test-keystore.ca>
<test-keystore.empty>${project.build.directory}/empty.keystore</test-keystore.empty>
<test-keystore.password>bunnies</test-keystore.password>

<test-broker.A.nodename>rabbit@localhost</test-broker.A.nodename>
<test-broker.A.node_port>5672</test-broker.A.node_port>
<test-broker.A.config_file>${project.build.directory}/test-classes/${test-broker.A.nodename}</test-broker.A.config_file>
Expand Down Expand Up @@ -119,6 +123,17 @@
</dependencies>

<build>
<!--
Those resources are a Java properties file and RabbitMQ
configuration files for the test brokers.
-->
<testResources>
<testResource>
<directory>${basedir}/src/test/resources</directory>
<filtering>true</filtering>
</testResource>
</testResources>

<plugins>

<plugin>
Expand Down Expand Up @@ -161,6 +176,13 @@
<make.bin>${make.bin}</make.bin>
<rabbitmq.dir>${rabbitmq.dir}</rabbitmq.dir>
<rabbitmqctl.bin>${rabbitmqctl.bin}</rabbitmqctl.bin>

<test-keystore.ca>${test-keystore.ca}</test-keystore.ca>
<test-keystore.empty>${test-keystore.empty}</test-keystore.empty>
<test-keystore.password>${test-keystore.password}</test-keystore.password>
<test-client-cert.path>${test-tls-certs.dir}/client/keycert.p12</test-client-cert.path>
<test-client-cert.password>changeme</test-client-cert.password>

<test-broker.A.nodename>${test-broker.A.nodename}</test-broker.A.nodename>
<test-broker.A.node_port>${test-broker.A.node_port}</test-broker.A.node_port>
<test-broker.A.config_file>${test-broker.A.config_file}</test-broker.A.config_file>
Expand Down Expand Up @@ -294,6 +316,70 @@
<releaseProfiles>releases</releaseProfiles>
</configuration>
</plugin>
<plugin>
<groupId>org.codehaus.gmaven</groupId>
<artifactId>groovy-maven-plugin</artifactId>
<version>2.0</version>
<executions>
<!--
We always remove the generated Java keystores because
keytool(1) complains if the destination file already exists
with the content we want to import.
-->
<execution>
<phase>generate-test-resources</phase>
<id>remove-old-test-keystores</id>
<goals>
<goal>execute</goal>
</goals>
<configuration>
<source>
${groovy-scripts.dir}/remove_old_test_keystores.groovy
</source>
</configuration>
</execution>
</executions>
</plugin>
<!-- Generate two Java keystores for the SSLTests testsuite. -->
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>keytool-maven-plugin</artifactId>
<version>1.5</version>
<configuration>
<skip>true</skip>
</configuration>
<executions>
<execution>
<id>generate-test-ca-keystore</id>
<phase>generate-test-resources</phase>
<goals>
<goal>importCertificate</goal>
</goals>
<configuration>
<file>${test-tls-certs.dir}/testca/cacert.pem</file>
<keystore>${test-keystore.ca}</keystore>
<storepass>${test-keystore.password}</storepass>
<noprompt>true</noprompt>
<alias>server1</alias>
</configuration>
</execution>
<execution>
<id>generate-test-empty-keystore</id>
<phase>generate-test-resources</phase>
<goals>
<goal>importCertificate</goal>
<goal>deleteAlias</goal>
</goals>
<configuration>
<file>${test-tls-certs.dir}/testca/cacert.pem</file>
<keystore>${test-keystore.empty}</keystore>
<storepass>${test-keystore.password}</storepass>
<noprompt>true</noprompt>
<alias>server1</alias>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>

Expand Down Expand Up @@ -754,6 +840,22 @@
<artifactId>groovy-maven-plugin</artifactId>
<version>2.0</version>
<executions>
<!--
We use "${rabbitmq.dir}/Makefile" to query the location of
the test TLS certificates.
-->
<execution>
<phase>generate-test-resources</phase>
<id>query-test-tls-certs-dir</id>
<goals>
<goal>execute</goal>
</goals>
<configuration>
<source>
${groovy-scripts.dir}/query_test_tls_certs_dir.groovy
</source>
</configuration>
</execution>
<!--
Start a broker.
Expand Down Expand Up @@ -798,7 +900,45 @@
</execution>
</executions>
</plugin>
<!--
Generate two Java keystores from the test TLS certificates.
Here, we only enable the plugin. The actual configuration is
in the main <build /> element.
-->
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>keytool-maven-plugin</artifactId>
<configuration>
<skip>false</skip>
</configuration>
</plugin>
</plugins>
</build>
</profile>
<profile>
<!--
If the caller decided to provide an external RabbitMQ
broker/cluster, he can specify the path to his test TLS
certificates with the ${test-tls-certs.dir} property.
Thus, if the CA certificate exists, we also enable the generation
of the two Java keystores.
-->
<id>use-provided-test-keystores</id>
<activation>
<file>
<exists>${test-tls-certs.dir}/testca/cacert.pem</exists>
</file>
</activation>
<build>
<plugins>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>keytool-maven-plugin</artifactId>
<configuration>
<skip>false</skip>
</configuration>
</plugin>
</plugins>
</build>
</profile>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ public Connection createConnection(String username, String password) throws JMSE
this.password = password;
// Create a new factory and set the properties
com.rabbitmq.client.ConnectionFactory factory = new com.rabbitmq.client.ConnectionFactory();
maybeEnableTLS(factory);
setRabbitUri(logger, this, factory, this.getUri());
maybeEnableTLS(factory);
com.rabbitmq.client.Connection rabbitConnection = instantiateNodeConnection(factory);

RMQConnection conn = new RMQConnection(new ConnectionParams()
Expand Down
25 changes: 25 additions & 0 deletions src/main/scripts/query_test_tls_certs_dir.groovy
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
String[] command = [
properties['make.bin'],
'-C', properties['rabbitmq.dir'],
'--no-print-directory',
'show-test-tls-certs-dir',
"DEPS_DIR=${properties['deps.dir']}",
]

def pb = new ProcessBuilder(command)
pb.redirectErrorStream(true)

def process = pb.start()

// We are only interested in the last line of output. Previous lines, if
// any, are related to the generation of the test certificates.
def whole_output = ""
process.inputStream.eachLine {
whole_output += it
project.properties['test-tls-certs.dir'] = it.trim()
}
process.waitFor()
if (process.exitValue() != 0) {
println(whole_output.trim())
fail("Failed to query test TLS certs directory with command: ${command.join(' ')}")
}
8 changes: 8 additions & 0 deletions src/main/scripts/remove_old_test_keystores.groovy
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
def dir = new File(project.build.directory)

// This pattern starts with `.*`. This is normally useless and even
// inefficient but the matching doesn't work without it...
def pattern = ~/.*\.keystore$/
dir.eachFileMatch(pattern) { file ->
file.delete()
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ public abstract class AbstractITTopicSSL {

@Before
public void beforeTests() throws Exception {
org.junit.Assume.assumeTrue(Boolean.getBoolean("com.rabbitmq.jms.TLSTests"));
this.connFactory =
(TopicConnectionFactory) AbstractTestConnectionFactory.getTestConnectionFactory(true, 0)
.getConnectionFactory();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
public class RabbitAPIConnectionFactory extends AbstractTestConnectionFactory {

private static final int RABBIT_PORT = 5672; // 5672 default; 5673 Tracer.
private static final int RABBIT_TLS_PORT = 5671;
private final boolean testssl;
private int qbrMax;

Expand All @@ -30,7 +31,11 @@ public ConnectionFactory getConnectionFactory() {
private static final long serialVersionUID = 1L;
@Override
public Connection createConnection(String userName, String password) throws JMSException {
if (!testssl) this.setPort(RABBIT_PORT);
if (!testssl) {
this.setPort(RABBIT_PORT);
} else {
this.setPort(RABBIT_TLS_PORT);
}
return super.createConnection(userName, password);
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import javax.jms.QueueSession;
import javax.jms.Session;

import org.junit.Before;
import org.junit.Test;

/**
Expand All @@ -21,11 +20,6 @@ public class SSLSimpleQueueMessageIT extends AbstractITQueueSSL {
private static final String QUEUE_NAME = "test.queue."+SSLSimpleQueueMessageIT.class.getCanonicalName();
private static final long TEST_RECEIVE_TIMEOUT = 1000; // one second

@Before
public void beforeTests() throws Exception {
org.junit.Assume.assumeTrue(Boolean.getBoolean("com.rabbitmq.jms.TLSTests"));
}

private void messageTestBase(MessageTestType mtt) throws Exception {
try {
queueConn.start();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import javax.jms.TopicSession;
import javax.jms.TopicSubscriber;

import org.junit.Before;
import org.junit.Test;

/**
Expand All @@ -20,11 +19,6 @@
public class SSLSimpleTopicMessageIT extends AbstractITTopicSSL {
private static final String TOPIC_NAME = "test.topic." + SSLSimpleTopicMessageIT.class.getCanonicalName();

@Before
public void beforeTests() throws Exception {
org.junit.Assume.assumeTrue(Boolean.getBoolean("com.rabbitmq.jms.TLSTests"));
}

@Test
public void testSendAndReceiveTextMessage() throws Exception {
final String MESSAGE2 = "Hello " + SSLSimpleTopicMessageIT.class.getName();
Expand Down
70 changes: 70 additions & 0 deletions src/test/java/com/rabbitmq/integration/tests/SslContextIT.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/* Copyright (c) 2016 Pivotal Software, Inc. All rights reserved. */

package com.rabbitmq.integration.tests;

import com.rabbitmq.jms.admin.RMQConnectionFactory;
import org.junit.Test;

import javax.jms.Connection;
import javax.net.ssl.SSLContext;
import javax.net.ssl.TrustManager;
import javax.net.ssl.X509TrustManager;
import java.security.NoSuchAlgorithmException;
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
import java.util.concurrent.atomic.AtomicInteger;

import static org.junit.Assert.assertEquals;


public class SslContextIT {

// https://github.com/rabbitmq/rabbitmq-jms-client/issues/12
// the set SSLContext isn't overridden
@Test public void sslContextShouldBeUsedWhenExplicitlySet() throws Exception {
Connection connection = null;
try {
RMQConnectionFactory connectionFactory = (RMQConnectionFactory) AbstractTestConnectionFactory.getTestConnectionFactory(true, 0)
.getConnectionFactory();
connectionFactory.setUri("amqps://guest:guest@localhost:5671/%2f");
SSLContext sslContext = createSslContext();
AlwaysTrustTrustManager trustManager = new AlwaysTrustTrustManager();
sslContext.init(null, new TrustManager[] {trustManager}, null);
connectionFactory.useSslProtocol(sslContext);
connection = connectionFactory.createConnection();
assertEquals(1, trustManager.checkServerTrustedCallCount.get());
} finally {
if(connection != null) {
connection.close();
}
}

}

private static SSLContext createSslContext() throws NoSuchAlgorithmException {
String[] protocols = SSLContext.getDefault().getSupportedSSLParameters().getProtocols();
String protocol = com.rabbitmq.client.ConnectionFactory.computeDefaultTlsProcotol(protocols);
return SSLContext.getInstance(protocol);
}

private static class AlwaysTrustTrustManager implements X509TrustManager {

private final AtomicInteger checkServerTrustedCallCount = new AtomicInteger();

@Override
public void checkClientTrusted(X509Certificate[] x509Certificates, String s) throws CertificateException {

}

@Override
public void checkServerTrusted(X509Certificate[] x509Certificates, String s) throws CertificateException {
checkServerTrustedCallCount.incrementAndGet();
}

@Override
public X509Certificate[] getAcceptedIssuers() {
return new X509Certificate[0];
}
}

}
14 changes: 11 additions & 3 deletions src/test/resources/rabbit@localhost.config
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
% vim:ft=erlang:

[
{rabbit, [
{auth_mechanisms, ['PLAIN', 'AMQPLAIN', 'EXTERNAL', 'RABBIT-CR-DEMO']}
{rabbit, [
{ssl_listeners, [5671]},
{ssl_options, [
{cacertfile, "${test-tls-certs.dir}/testca/cacert.pem"},
{certfile, "${test-tls-certs.dir}/server/cert.pem"},
{keyfile, "${test-tls-certs.dir}/server/key.pem"},
{verify, verify_peer},
{fail_if_no_peer_cert, false},
{honor_cipher_order, true}]},
{auth_mechanisms, ['PLAIN', 'AMQPLAIN', 'EXTERNAL', 'RABBIT-CR-DEMO']}
]}
].
].

0 comments on commit dfaaf43

Please sign in to comment.