Limit ObjectMessage deserialization with a package white list

ObjectMessages in JMS can be used to transfer arbitrary Java objects
which are then deserialized before or at the time of
being passed on to message listeners.

This opens message consumers up to a range of attacks
that exploit issues in Java object serialization.
Note: attacker must authenticate with RabbitMQ in order
to carry out the attack.

With this commit we introduce a way to white list packages
that can be deserialized when ObjectMessages are consumed:

 * Using a system property, com.rabbitmq.jms.TrustedPackagesPrefixes
 * Using RMQSession#setTrustedPackages(List<String>)

By default all packages are trusted for backwards compatibility
(as it's not possible for library maintainers to cover application-specific
packages).

A CVE has been allocated for this issue and will be announced
once a new release is published.

Related issues in other messaging libraries and services:

 * Spring AMQP ticket: https://jira.spring.io/browse/AMQP-590
 * ActiveMQ ticket: https://issues.apache.org/jira/browse/AMQ-6013

Note: this includes a number of small drive-by changes:

 * TLS tests are now executed conditionally and excluded by default
 * Many minor IDEA code analysis fixes
 * JavaDoc warning fixes

Author: michaelklishin

src/main/java/com/rabbitmq/jms/admin/RMQConnectionFactory.java

@@ -7,6 +7,7 @@
 
 import java.io.IOException;
 import java.io.Serializable;
+import java.util.List;
 import java.util.concurrent.TimeoutException;
 
 import javax.jms.Connection;
@@ -23,6 +24,7 @@
 import javax.naming.StringRefAddr;
 import javax.net.ssl.SSLException;
 
+import com.rabbitmq.jms.util.WhiteListObjectInputStream;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -64,6 +66,13 @@ public class RMQConnectionFactory implements ConnectionFactory, Referenceable, S
     /** The time to wait for threads/messages to terminate during {@link Connection#close()} */
     private volatile long terminationTimeout = Long.getLong("rabbit.jms.terminationTimeout", 15000);
 
+    /**
+     * Classes in these packages can be transferred via ObjectMessage.
+     *
+     * @see WhiteListObjectInputStream
+     */
+    private List<String> trustedPackages = WhiteListObjectInputStream.DEFAULT_TRUSTED_PACKAGES;
+
     /**
      * {@inheritDoc}
      */
@@ -144,7 +153,7 @@ public String toString() {
     /**
      * Set connection factory parameters by URI String.
      * @param uriString URI to use for instantiated connection
-     * @throws JMSException
+     * @throws JMSException if connection URI is invalid
      */
     public void setUri(String uriString) throws JMSException {
         logger.trace("Set connection factory parameters by URI '{}'", uriString);
@@ -160,7 +169,14 @@ public void setUri(String uriString) throws JMSException {
         this.virtualHost = factory.getVirtualHost();
     }
 
-    private static final void setRabbitUri(Logger logger, RMQConnectionFactory rmqFactory, com.rabbitmq.client.ConnectionFactory factory, String uriString) throws RMQJMSException {
+    /**
+     * @param value list of trusted package prefixes
+     */
+    public void setTrustedPackages(List<String> value) {
+        this.trustedPackages = value;
+    }
+
+    private static void setRabbitUri(Logger logger, RMQConnectionFactory rmqFactory, com.rabbitmq.client.ConnectionFactory factory, String uriString) throws RMQJMSException {
         if (uriString != null) { // we get the defaults if the uri is null
             try {
                 factory.setUri(uriString);
@@ -189,21 +205,21 @@ public void setSsl(boolean ssl) {
         this.ssl = ssl;
     }
 
-    private static final String scheme(boolean isSsl) {
+    private static String scheme(boolean isSsl) {
         return (isSsl ? "amqps" : "amqp");
     }
 
-    private static final String uriUInfoEscape(String user, String pass) {
-        if (null==user) return null;
-        if (null==pass) return encUserinfo(user, "UTF-8");
+    private static String uriUInfoEscape(String user, String pass) {
+        if (null == user) return null;
+        if (null == pass) return encUserinfo(user, "UTF-8");
         return encUserinfo(user + ":" + pass, "UTF-8");
     }
 
-    private static final String uriHostEscape(String host) {
+    private static String uriHostEscape(String host) {
         return encHost(host, "UTF-8");
     }
 
-    private static final String uriVirtualHostEscape(String vHost) {
+    private static String uriVirtualHostEscape(String vHost) {
         return encSegment(vHost, "UTF-8");
     }
 
@@ -224,9 +240,9 @@ public Reference getReference() throws NamingException {
      * @param propertyName - the name of the property
      * @param value - the value to store with the property
      */
-    private static final void addStringRefProperty(Reference ref,
-                                                   String propertyName,
-                                                   String value) {
+    private static void addStringRefProperty(Reference ref,
+                                             String propertyName,
+                                             String value) {
         if (value==null || propertyName==null) return;
         RefAddr ra = new StringRefAddr(propertyName, value);
         ref.add(ra);
@@ -238,10 +254,10 @@ private static final void addStringRefProperty(Reference ref,
      * @param propertyName - the name of the property
      * @param value - the value to store with the property
      */
-    private static final void addIntegerRefProperty(Reference ref,
-                                                    String propertyName,
-                                                    Integer value) {
-        if (value==null || propertyName==null) return;
+    private static void addIntegerRefProperty(Reference ref,
+                                              String propertyName,
+                                              Integer value) {
+        if (value == null || propertyName == null) return;
         RefAddr ra = new StringRefAddr(propertyName, String.valueOf(value));
         ref.add(ra);
     }

src/main/java/com/rabbitmq/jms/admin/RMQDestination.java

@@ -142,12 +142,16 @@ public boolean amqpReadable() {
     }
 
     /**
-     * @return <code>true</code> if this is an AMQP mapped resource, <code>false</code> otherwise
+     * @return <code>true</code> if this is an AMQP 0-9-1 mapped resource, <code>false</code> otherwise
      */
     public boolean isAmqp() {
         return this.amqp;
     }
-    /** For JNDI binding and Spring beans */
+
+    /**
+     * For JNDI binding and Spring beans
+     * @param amqp set to <code>true</code> if this is an AMQP 0-9-1 mapped resource, <code>false</code> otherwise
+     */
     public void setAmqp(boolean amqp) {
         if (this.isDeclared())
             throw new IllegalStateException();
@@ -158,7 +162,10 @@ public void setAmqp(boolean amqp) {
     public String getAmqpQueueName() {
         return this.amqpQueueName;
     }
-    /** For JNDI binding and Spring beans */
+    /**
+     * For JNDI binding and Spring beans
+     * @param amqpQueueName AMQP 0-9-1 queue name
+     */
     public void setAmqpQueueName(String amqpQueueName) {
         if (this.isDeclared())
             throw new IllegalStateException();
@@ -167,7 +174,10 @@ public void setAmqpQueueName(String amqpQueueName) {
     public String getAmqpExchangeName() {
         return this.amqpExchangeName;
     }
-    /** For JNDI binding and Spring beans */
+    /**
+     * For JNDI binding and Spring beans
+     * @param amqpExchangeName AMQP 0-9-1 exchange name to use
+     */
     public void setAmqpExchangeName(String amqpExchangeName) {
         if (this.isDeclared())
             throw new IllegalStateException();
@@ -176,23 +186,36 @@ public void setAmqpExchangeName(String amqpExchangeName) {
     public String getDestinationName() {
         return this.destinationName;
     }
-    /** For JNDI binding and Spring beans */
+    /**
+     * For JNDI binding and Spring beans
+     * @param destinationName JMS destination name
+     */
     public void setDestinationName(String destinationName) {
         if (isDeclared())
             throw new IllegalStateException();
         this.destinationName = destinationName;
     }
+
+    /**
+     * @return AMQP 0-9-1 routing key
+     */
     public String getAmqpRoutingKey() {
         return this.amqpRoutingKey;
     }
-    /** For JNDI binding and Spring beans */
+    /**
+     * For JNDI binding and Spring beans
+     * @param routingKey AMQP 0-9-1 routing key
+     */
     public void setAmqpRoutingKey(String routingKey) {
         if (isDeclared())
             throw new IllegalStateException();
         this.amqpRoutingKey = routingKey;
     }
 
-    /** Internal use only */
+    /**
+     * Internal use only
+     * @return AMQP 0-9-1 exchange type used
+     */
     public String amqpExchangeType() {
         return queueOrTopicExchangeType(this.isQueue);
     }

src/main/java/com/rabbitmq/jms/client/Completion.java

@@ -22,6 +22,7 @@ public void setComplete() {
 
     /**
      * Non-blocking snapshot test for completion.
+     * @return <code>true</code> if this operation has completed, <code>false</code> otherwise
      */
     public boolean isComplete() {
         return this.fb.isComplete();

src/main/java/com/rabbitmq/jms/client/DeliveryExecutor.java

@@ -53,7 +53,9 @@ public DeliveryExecutor(long onMessageTimeoutMs) {
      * only one <code>onMessage</code> call being executed at any one time.
      *
      * @param rmqMessage the message to deliver
+     * @param messageListener JMS message listener that will handle the delivery
      * @throws JMSException if the delivery takes too long and is aborted
+     * @throws InterruptedException if executing thread is interrupted
      */
     public void deliverMessageWithProtection(RMQMessage rmqMessage, MessageListener messageListener) throws JMSException, InterruptedException {
         try {

src/main/java/com/rabbitmq/jms/client/RMQConnection.java

@@ -27,6 +27,7 @@
 import javax.jms.TopicConnection;
 import javax.jms.TopicSession;
 
+import com.rabbitmq.jms.util.WhiteListObjectInputStream;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -77,6 +78,13 @@ public class RMQConnection implements Connection, QueueConnection, TopicConnecti
     /** This is used for JMSCTS test cases, as ClientID should only be configurable right after the connection has been created */
     private volatile boolean canSetClientID = true;
 
+    /**
+     * Classes in these packages can be transferred via ObjectMessage.
+     *
+     * @see WhiteListObjectInputStream
+     */
+    private List<String> trustedPackages = WhiteListObjectInputStream.DEFAULT_TRUSTED_PACKAGES;
+
     /**
      * Creates an RMQConnection object.
      * @param rabbitConnection the TCP connection wrapper to the RabbitMQ broker
@@ -113,6 +121,7 @@ public Session createSession(boolean transacted, int acknowledgeMode) throws JMS
         illegalStateExceptionIfClosed();
         freezeClientID();
         RMQSession session = new RMQSession(this, transacted, acknowledgeMode, this.subscriptions);
+        session.setTrustedPackages(this.trustedPackages);
         this.sessions.add(session);
         return session;
     }
@@ -154,6 +163,19 @@ public void setClientID(String clientID) throws JMSException {
 
     }
 
+    public List<String> getTrustedPackages() {
+        return trustedPackages;
+    }
+
+    /**
+     * @param value list of trusted package prefixes
+     *
+     * @see com.rabbitmq.jms.admin.RMQConnectionFactory#setTrustedPackages(List)
+     */
+    public void setTrustedPackages(List<String> value) {
+        this.trustedPackages = value;
+    }
+
     /**
      * {@inheritDoc}
      */