Skip to content

Commit fe1a3a4

Browse files
jgallimorejgallimore
committed
Add Http discovery transport to denied list for JMX (apache#1918) (apache#1926)
Cherry-picked from 5.19.x: c84899b
1 parent 316aa67 commit fe1a3a4

3 files changed

Lines changed: 59 additions & 32 deletions

File tree

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ public class BrokerView implements BrokerViewMBean {
4444

4545
private static final Logger LOG = LoggerFactory.getLogger(BrokerView.class);
4646

47+
private static final Set<String> DENIED_TRANSPORT_SCHEMES = Set.of("vm", "http");
48+
4749
ManagedRegionBroker broker;
4850

4951
private final BrokerService brokerService;
@@ -372,7 +374,7 @@ public ObjectName[] getDynamicDestinationProducers() {
372374

373375
@Override
374376
public String addConnector(String discoveryAddress) throws Exception {
375-
// Verify VM transport is not used
377+
// Verify a denied transport scheme is not used
376378
validateAllowedUrl(discoveryAddress);
377379
TransportConnector connector = brokerService.addConnector(discoveryAddress);
378380
if (connector == null) {
@@ -384,7 +386,7 @@ public String addConnector(String discoveryAddress) throws Exception {
384386

385387
@Override
386388
public String addNetworkConnector(String discoveryAddress) throws Exception {
387-
// Verify VM transport is not used
389+
// Verify a denied transport scheme is not used
388390
validateAllowedUrl(discoveryAddress);
389391
NetworkConnector connector = brokerService.addNetworkConnector(discoveryAddress);
390392
if (connector == null) {
@@ -552,7 +554,7 @@ private static void validateAllowedUrl(String uriString) throws URISyntaxExcepti
552554
validateAllowedUri(new URI(uriString), 0);
553555
}
554556

555-
// Validate the URI does not contain VM transport
557+
// Validate the URI does not contain a denied transport scheme
556558
private static void validateAllowedUri(URI uri, int depth) throws URISyntaxException {
557559
// Don't allow more than 5 nested URIs to prevent blowing the stack
558560
// If we are greater than 4 then this is the 5th level of composite
@@ -580,10 +582,13 @@ private static void validateAllowedUri(URI uri, int depth) throws URISyntaxExcep
580582
}
581583
}
582584

583-
// We don't allow VM transport scheme to be used
585+
// Check all denied schemes
584586
private static void validateAllowedScheme(String scheme) {
585-
if (scheme.equals("vm")) {
586-
throw new IllegalArgumentException("VM scheme is not allowed");
587+
for (String denied : DENIED_TRANSPORT_SCHEMES) {
588+
// The schemes should be case-insensitive but ignore case as a precaution
589+
if (scheme.equalsIgnoreCase(denied)) {
590+
throw new IllegalArgumentException("Transport scheme '" + scheme + "' is not allowed");
591+
}
587592
}
588593
}
589594
}

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

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2033,34 +2033,50 @@ public void testSubscriptionViewProperties() throws Exception {
20332033
assertTrue(subscription.isExclusive());
20342034
}
20352035

2036-
// Test to verify VM transport is not allowed to be added as a connector
2036+
// Test to verify http transport is not allowed to be added as a connector
2037+
// through the Broker MBean
2038+
public void testAddHttpConnectorBlockedBrokerView() throws Exception {
2039+
testAddTransportConnectorBlockedBrokerView("http");
2040+
}
2041+
2042+
// Test to verify vm transport is not allowed to be added as a connector
20372043
// through the Broker MBean
20382044
public void testAddVmConnectorBlockedBrokerView() throws Exception {
2045+
testAddTransportConnectorBlockedBrokerView("vm");
2046+
}
2047+
2048+
protected void testAddTransportConnectorBlockedBrokerView(String scheme) throws Exception {
20392049
ObjectName brokerName = assertRegisteredObjectName(domain + ":type=Broker,brokerName=localhost");
20402050
BrokerViewMBean brokerView = MBeanServerInvocationHandler.newProxyInstance(mbeanServer, brokerName, BrokerViewMBean.class, true);
20412051

20422052
try {
2043-
brokerView.addConnector("vm://localhost");
2044-
fail("Should have failed trying to add vm connector");
2053+
brokerView.addConnector(scheme + "://localhost");
2054+
fail("Should have failed trying to add connector");
20452055
} catch (IllegalArgumentException e) {
2046-
assertEquals("VM scheme is not allowed", e.getMessage());
2056+
assertEquals("Transport scheme '" + scheme + "' is not allowed", e.getMessage());
20472057
}
20482058

20492059
try {
20502060
// verify any composite URI is blocked as well
2051-
brokerView.addConnector("failover:(tcp://0.0.0.0:0,vm://" + brokerName + ")");
2052-
fail("Should have failed trying to add vm connector");
2061+
brokerView.addConnector("failover:(tcp://0.0.0.0:0," + scheme + "://" + brokerName + ")");
2062+
fail("Should have failed trying to add connector");
20532063
} catch (IllegalArgumentException e) {
2054-
assertEquals("VM scheme is not allowed", e.getMessage());
2064+
assertEquals("Transport scheme '" + scheme + "' is not allowed", e.getMessage());
20552065
}
20562066

20572067
try {
20582068
// verify nested composite URI is blocked
2059-
brokerView.addConnector("failover:(failover:(failover:(vm://localhost)))");
2060-
fail("Should have failed trying to add vm connector");
2069+
brokerView.addConnector("failover:(failover:(failover:(" + scheme + "://localhost)))");
2070+
fail("Should have failed trying to add connector");
20612071
} catch (IllegalArgumentException e) {
2062-
assertEquals("VM scheme is not allowed", e.getMessage());
2072+
assertEquals("Transport scheme '" + scheme + "' is not allowed", e.getMessage());
20632073
}
2074+
}
2075+
2076+
// Test too many nested URIs
2077+
public void testNestedAddTransportConnector() throws Exception {
2078+
ObjectName brokerName = assertRegisteredObjectName(domain + ":type=Broker,brokerName=localhost");
2079+
BrokerViewMBean brokerView = MBeanServerInvocationHandler.newProxyInstance(mbeanServer, brokerName, BrokerViewMBean.class, true);
20642080

20652081
try {
20662082
// verify nested composite URI with more than 5 levels is blocked
@@ -2070,7 +2086,6 @@ public void testAddVmConnectorBlockedBrokerView() throws Exception {
20702086
} catch (IllegalArgumentException e) {
20712087
assertEquals("URI can't contain more than 5 nested composite URIs", e.getMessage());
20722088
}
2073-
20742089
}
20752090

20762091
}

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

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,6 @@
3333
/**
3434
* This test shows that when we create a network connector via JMX,
3535
* the NC/bridge shows up in the MBean Server
36-
*
37-
* @author <a href="http://www.christianposta.com/blog">Christian Posta</a>
3836
*/
3937
public class JmxCreateNCTest {
4038

@@ -82,35 +80,44 @@ public void testBridgeRegistration() throws Exception {
8280

8381
@Test
8482
public void testVmBridgeBlocked() throws Exception {
83+
testDeniedBridgeBlocked("vm");
84+
}
85+
86+
@Test
87+
public void testHttpBridgeBlocked() throws Exception {
88+
testDeniedBridgeBlocked("http");
89+
}
90+
91+
protected void testDeniedBridgeBlocked(String scheme) throws Exception {
8592
// Test composite network connector uri
8693
try {
87-
proxy.addNetworkConnector("static:(vm://localhost)");
88-
fail("Should have failed trying to add vm connector bridge");
94+
proxy.addNetworkConnector("static:(" + scheme + "://localhost)");
95+
fail("Should have failed trying to add connector bridge");
8996
} catch (IllegalArgumentException e) {
90-
assertEquals("VM scheme is not allowed", e.getMessage());
97+
assertEquals("Transport scheme '" + scheme + "' is not allowed", e.getMessage());
9198
}
9299

93100
try {
94-
proxy.addNetworkConnector("multicast:(vm://localhost)");
95-
fail("Should have failed trying to add vm connector bridge");
101+
proxy.addNetworkConnector("multicast:(" + scheme + "://localhost)");
102+
fail("Should have failed trying to add connector bridge");
96103
} catch (IllegalArgumentException e) {
97-
assertEquals("VM scheme is not allowed", e.getMessage());
104+
assertEquals("Transport scheme '" + scheme + "' is not allowed", e.getMessage());
98105
}
99106

100-
// verify direct vm as well
107+
// verify direct connector as well
101108
try {
102-
proxy.addNetworkConnector("vm://localhost");
103-
fail("Should have failed trying to add vm connector bridge");
109+
proxy.addNetworkConnector(scheme + "://localhost");
110+
fail("Should have failed trying to add connector bridge");
104111
} catch (IllegalArgumentException e) {
105-
assertEquals("VM scheme is not allowed", e.getMessage());
112+
assertEquals("Transport scheme '" + scheme + "' is not allowed", e.getMessage());
106113
}
107114

108115
try {
109116
// verify nested composite URI is blocked
110-
proxy.addNetworkConnector("static:(failover:(failover:(tcp://localhost:0,vm://localhost)))");
111-
fail("Should have failed trying to add vm connector bridge");
117+
proxy.addNetworkConnector("static:(failover:(failover:(tcp://localhost:0," + scheme + "://localhost)))");
118+
fail("Should have failed trying to add connector bridge");
112119
} catch (IllegalArgumentException e) {
113-
assertEquals("VM scheme is not allowed", e.getMessage());
120+
assertEquals("Transport scheme '" + scheme + "' is not allowed", e.getMessage());
114121
}
115122
}
116123

0 commit comments

Comments
 (0)