Prevent a VM transport from being used with BrokerView (#1845)

This prevents a VM transport URI from being used when adding a connector
or a network connector through the Broker MBean

(cherry picked from commit ef53b5b)

Author: cshannon

activemq-broker/src/main/java/org/apache/activemq/broker/jmx/BrokerView.java

@@ -19,6 +19,7 @@
 import java.io.File;
 import java.io.IOException;
 import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.*;
 import java.util.concurrent.atomic.AtomicInteger;
 
@@ -35,6 +36,7 @@
 import org.apache.activemq.command.*;
 import org.apache.activemq.network.NetworkConnector;
 import org.apache.activemq.util.BrokerSupport;
+import org.apache.activemq.util.URISupport;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -370,6 +372,8 @@ public ObjectName[] getDynamicDestinationProducers() {
 
     @Override
     public String addConnector(String discoveryAddress) throws Exception {
+        // Verify VM transport is not used
+        validateAllowedUrl(discoveryAddress);
         TransportConnector connector = brokerService.addConnector(discoveryAddress);
         if (connector == null) {
             throw new NoSuchElementException("Not connector matched the given name: " + discoveryAddress);
@@ -380,6 +384,8 @@ public String addConnector(String discoveryAddress) throws Exception {
 
     @Override
     public String addNetworkConnector(String discoveryAddress) throws Exception {
+        // Verify VM transport is not used
+        validateAllowedUrl(discoveryAddress);
         NetworkConnector connector = brokerService.addNetworkConnector(discoveryAddress);
         if (connector == null) {
             throw new NoSuchElementException("Not connector matched the given name: " + discoveryAddress);
@@ -556,4 +562,27 @@ public void setMaxUncommittedCount(int maxUncommittedCount) {
     public long getTotalMaxUncommittedExceededCount() {
         return safeGetBroker().getDestinationStatistics().getMaxUncommittedExceededCount().getCount();
 	}
+
+
+    // Validate the Url does not contain VM transport
+    private static void validateAllowedUrl(String uriString) throws URISyntaxException {
+        URI uri = new URI(uriString);
+        // First check the main URI scheme
+        validateAllowedScheme(uri.getScheme());
+
+        // If composite, also check all schemes for each component
+        if (URISupport.isCompositeURI(uri)) {
+            URISupport.CompositeData data = URISupport.parseComposite(uri);
+            for (URI component : data.getComponents()) {
+                validateAllowedScheme(component.getScheme());
+            }
+        }
+    }
+
+    // We don't allow VM transport scheme to be used
+    private static void validateAllowedScheme(String scheme) {
+        if (scheme.equals("vm")) {
+            throw new IllegalArgumentException("VM scheme is not allowed");
+        }
+    }
 }

activemq-unit-tests/src/test/java/org/apache/activemq/broker/jmx/MBeanTest.java

@@ -16,6 +16,9 @@
  */
 package org.apache.activemq.broker.jmx;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
 import java.io.BufferedReader;
 import java.io.InputStreamReader;
 import java.net.URI;
@@ -2055,4 +2058,26 @@ public void testSubscriptionViewProperties() throws Exception {
         assertFalse(subscription.isRetroactive());
         assertTrue(subscription.isExclusive());
     }
-}
+
+    // Test to verify VM transport is not allowed to be added as a connector
+    // through the Broker MBean
+    public void testAddVmConnectorBlockedBrokerView() throws Exception {
+        ObjectName brokerName = assertRegisteredObjectName(domain + ":type=Broker,brokerName=localhost");
+        BrokerViewMBean brokerView = MBeanServerInvocationHandler.newProxyInstance(mbeanServer, brokerName, BrokerViewMBean.class, true);
+
+        try {
+            brokerView.addConnector("vm://localhost");
+            fail("Should have failed trying to add vm connector bridge");
+        } catch (IllegalArgumentException e) {
+            assertEquals("VM scheme is not allowed", e.getMessage());
+        }
+
+        try {
+            // verify any composite URI is blocked as well
+            brokerView.addConnector("failover:(tcp://0.0.0.0:0,vm://" + brokerName + ")");
+            fail("Should have failed trying to add vm connector bridge");
+        } catch (IllegalArgumentException e) {
+            assertEquals("VM scheme is not allowed", e.getMessage());
+        }
+    }
+}

activemq-unit-tests/src/test/java/org/apache/activemq/jmx/JmxCreateNCTest.java

@@ -19,12 +19,16 @@
 import org.apache.activemq.broker.BrokerService;
 import org.apache.activemq.broker.jmx.BrokerViewMBean;
 import org.apache.activemq.broker.jmx.NetworkConnectorViewMBean;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Before;
 import org.junit.Test;
 
 import javax.management.ObjectName;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.fail;
 
 /**
  * This test shows that when we create a network connector via JMX,
@@ -36,35 +40,69 @@ public class JmxCreateNCTest {
 
     private static final String BROKER_NAME = "jmx-broker";
 
-    @Test
-    public void testBridgeRegistration() throws Exception {
+    private BrokerService broker;
+    private BrokerViewMBean proxy;
 
-        System.setProperty("org.apache.activemq.audit", "all");
-
-        BrokerService broker = new BrokerService();
+    @Before
+    public void setUp() throws Exception {
+        broker = new BrokerService();
         broker.setBrokerName(BROKER_NAME);
         broker.setUseJmx(true); // explicitly set this so no funny issues
         broker.start();
         broker.waitUntilStarted();
 
-        // now create network connector over JMX
         ObjectName brokerObjectName = new ObjectName("org.apache.activemq:type=Broker,brokerName=" + BROKER_NAME);
-        BrokerViewMBean proxy = (BrokerViewMBean) broker.getManagementContext().newProxyInstance(brokerObjectName,
+        proxy = (BrokerViewMBean) broker.getManagementContext().newProxyInstance(brokerObjectName,
                 BrokerViewMBean.class, true);
+    }
 
+    @After
+    public void tearDown() throws Exception {
+        broker.stop();
+        broker.waitUntilStopped();
+    }
+
+    @Test
+    public void testBridgeRegistration() throws Exception {
         assertNotNull("We could not retrieve the broker from JMX", proxy);
 
         // let's add the NC
-        String connectoName = proxy.addNetworkConnector("static:(tcp://localhost:61617)");
-        assertEquals("NC", connectoName);
+        String connectorName = proxy.addNetworkConnector("static:(tcp://localhost:61617)");
+        assertEquals("NC", connectorName);
 
         // Make sure we can retrieve the NC through JMX
         ObjectName networkConnectorObjectName = new ObjectName("org.apache.activemq:type=Broker,brokerName=" + BROKER_NAME +
-                ",connector=networkConnectors,networkConnectorName=" + connectoName);
+                ",connector=networkConnectors,networkConnectorName=" + connectorName);
         NetworkConnectorViewMBean nc  = (NetworkConnectorViewMBean) broker.getManagementContext().newProxyInstance(networkConnectorObjectName,
                 NetworkConnectorViewMBean.class, true);
 
         assertNotNull(nc);
         assertEquals("NC", nc.getName());
     }
+
+    @Test
+    public void testVmBridgeBlocked() throws Exception {
+        // Test composite network connector uri
+        try {
+            proxy.addNetworkConnector("static:(vm://localhost)");
+            fail("Should have failed trying to add vm connector bridge");
+        } catch (IllegalArgumentException e) {
+            assertEquals("VM scheme is not allowed", e.getMessage());
+        }
+
+        try {
+            proxy.addNetworkConnector("multicast:(vm://localhost)");
+            fail("Should have failed trying to add vm connector bridge");
+        } catch (IllegalArgumentException e) {
+            assertEquals("VM scheme is not allowed", e.getMessage());
+        }
+
+        // verify direct vm as well
+        try {
+            proxy.addNetworkConnector("vm://localhost");
+            fail("Should have failed trying to add vm connector bridge");
+        } catch (IllegalArgumentException e) {
+            assertEquals("VM scheme is not allowed", e.getMessage());
+        }
+    }
 }