Skip to content

Commit c2fc7a1

Browse files
cshannoncshannon
authored
Block the XBeanBrokerFactory by default inside VMTransportFactory (#2003) (#2010)
By default the VMTransportFactory will not be allowed to use XBeanBrokerFactory to create new brokers. Only the default and properties factories will be enabled. To enable the XBeanBrokerFactory, the property org.apache.activemq.transport.VM_TRANSPORT_FACTORY_SCHEMES_ENABLED can be configured. (cherry picked from commit 103962e)
1 parent 4d395cc commit c2fc7a1

5 files changed

Lines changed: 248 additions & 5 deletions

File tree

‎activemq-broker/src/main/java/org/apache/activemq/transport/vm/VMTransportFactory.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,14 @@
1919
import java.io.IOException;
2020
import java.net.URI;
2121
import java.net.URISyntaxException;
22+
import java.util.Arrays;
2223
import java.util.HashMap;
2324
import java.util.Map;
25+
import java.util.Set;
2426
import java.util.concurrent.ConcurrentHashMap;
2527
import java.util.concurrent.ConcurrentMap;
2628

29+
import java.util.stream.Collectors;
2730
import org.apache.activemq.broker.BrokerFactory;
2831
import org.apache.activemq.broker.BrokerFactoryHandler;
2932
import org.apache.activemq.broker.BrokerRegistry;
@@ -44,12 +47,29 @@
4447

4548
public class VMTransportFactory extends TransportFactory {
4649

50+
public static final String VM_TRANSPORT_FACTORY_SCHEMES_ENABLED_PROP =
51+
"org.apache.activemq.transport.VM_TRANSPORT_FACTORY_SCHEMES_ENABLED";
52+
public static final String DEFAULT_ALLOWED_SCHEMES = "broker,properties";
53+
4754
public static final ConcurrentMap<String, BrokerService> BROKERS = new ConcurrentHashMap<String, BrokerService>();
4855
public static final ConcurrentMap<String, TransportConnector> CONNECTORS = new ConcurrentHashMap<String, TransportConnector>();
4956
public static final ConcurrentMap<String, VMTransportServer> SERVERS = new ConcurrentHashMap<String, VMTransportServer>();
5057
private static final Logger LOG = LoggerFactory.getLogger(VMTransportFactory.class);
5158

5259
BrokerFactoryHandler brokerFactoryHandler;
60+
private final Set<String> allowedSchemes;
61+
62+
public VMTransportFactory() {
63+
final String allowedSchemes = System.getProperty(VM_TRANSPORT_FACTORY_SCHEMES_ENABLED_PROP,
64+
DEFAULT_ALLOWED_SCHEMES);
65+
66+
// Asterisk will map to null which will allow all and skip checking
67+
// Empty string will map to an empty set and will deny all
68+
this.allowedSchemes = !allowedSchemes.equals("*") ?
69+
Arrays.stream(allowedSchemes.split("\\s*,\\s*"))
70+
.filter(s -> !s.isBlank())
71+
.collect(Collectors.toUnmodifiableSet()) : null;
72+
}
5373

5474
@Override
5575
public Transport doConnect(URI location) throws Exception {
@@ -119,6 +139,7 @@ public Transport doCompositeConnect(URI location) throws Exception {
119139
throw new IOException("Broker named '" + host + "' does not exist.");
120140
}
121141
try {
142+
validateBrokerCreationSchema(host, brokerURI);
122143
if (brokerFactoryHandler != null) {
123144
broker = brokerFactoryHandler.createBroker(brokerURI);
124145
} else {
@@ -162,6 +183,20 @@ public Transport doCompositeConnect(URI location) throws Exception {
162183
return transport;
163184
}
164185

186+
private void validateBrokerCreationSchema(String host, URI brokerURI) {
187+
if (allowedSchemes != null) {
188+
final String detectedScheme = brokerURI.getScheme();
189+
if (detectedScheme == null) {
190+
throw new IllegalArgumentException("Could not detect scheme in given URI [" + brokerURI + "]");
191+
}
192+
if (!allowedSchemes.contains(detectedScheme)){
193+
throw new IllegalArgumentException("Broker named '" + host + "' does not exist and "
194+
+ "broker creation using the scheme '" + detectedScheme + "' is not enabled via the VMTransportFactory. "
195+
+ "To allow creation, configure the system property " + VM_TRANSPORT_FACTORY_SCHEMES_ENABLED_PROP);
196+
}
197+
}
198+
}
199+
165200
private static String extractHost(URI location) {
166201
String host = location.getHost();
167202
if (host == null || host.length() == 0) {

‎activemq-unit-tests/src/test/java/org/apache/activemq/config/BrokerXmlConfigFromJNDITest.java‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,34 @@
1616
*/
1717
package org.apache.activemq.config;
1818

19-
import java.io.File;
20-
import java.util.Hashtable;
21-
22-
import javax.naming.Context;
23-
import javax.naming.InitialContext;
19+
import static org.apache.activemq.util.VmTransportTestUtils.resetVmTransportFactory;
2420

2521
import org.apache.activemq.ActiveMQConnectionFactory;
2622
import org.apache.activemq.test.JmsTopicSendReceiveWithTwoConnectionsTest;
2723

24+
import javax.naming.Context;
25+
import javax.naming.InitialContext;
26+
import java.io.File;
27+
import java.util.Hashtable;
28+
2829
/**
2930
*
3031
*/
3132
public class BrokerXmlConfigFromJNDITest extends JmsTopicSendReceiveWithTwoConnectionsTest {
33+
34+
@Override
35+
protected void setUp() throws Exception {
36+
// reset before each test
37+
resetVmTransportFactory("xbean");
38+
super.setUp();
39+
}
40+
41+
@Override
42+
protected void tearDown() throws Exception {
43+
super.tearDown();
44+
resetVmTransportFactory();
45+
}
46+
3247
@Override
3348
protected ActiveMQConnectionFactory createConnectionFactory() throws Exception {
3449
assertBaseDirectoryContainsSpaces();

‎activemq-unit-tests/src/test/java/org/apache/activemq/spring/ActiveMQConnectionFactoryXBeanTest.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
*/
1717
package org.apache.activemq.spring;
1818

19+
import static org.apache.activemq.util.VmTransportTestUtils.resetVmTransportFactory;
1920
import static org.apache.activemq.xbean.XBeanBrokerFactory.DEFAULT_ALLOWED_PROTOCOLS;
2021
import static org.apache.activemq.xbean.XBeanBrokerFactory.XBEAN_BROKER_FACTORY_PROTOCOLS_PROP;
2122
import static org.junit.Assert.assertNotNull;
@@ -28,14 +29,22 @@
2829
import java.net.UnknownHostException;
2930
import java.nio.file.NoSuchFileException;
3031
import org.apache.activemq.ActiveMQConnectionFactory;
32+
import org.apache.activemq.transport.vm.VMTransportFactory;
3133
import org.apache.activemq.xbean.XBeanBrokerFactory;
3234
import org.junit.AfterClass;
3335
import org.junit.Before;
36+
import org.junit.BeforeClass;
3437
import org.junit.Test;
3538
import org.springframework.beans.FatalBeanException;
3639

3740
public class ActiveMQConnectionFactoryXBeanTest {
3841

42+
@BeforeClass
43+
public static void beforeClass() throws Exception {
44+
// enable xbean for the vm transport factory
45+
resetVmTransportFactory("xbean");
46+
}
47+
3948
@Before
4049
public void setUp() throws Exception {
4150
// reset before each test
@@ -45,6 +54,8 @@ public void setUp() throws Exception {
4554
@AfterClass
4655
public static void tearDown() throws Exception {
4756
System.setProperty(XBEAN_BROKER_FACTORY_PROTOCOLS_PROP, DEFAULT_ALLOWED_PROTOCOLS);
57+
// reset defaults
58+
resetVmTransportFactory();
4859
}
4960

5061
// File and classpath are allowed by default

‎activemq-unit-tests/src/test/java/org/apache/activemq/transport/vm/VMTransportFactoryTest.java‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.activemq.transport.vm;
18+
19+
import static org.apache.activemq.util.VmTransportTestUtils.resetVmTransportFactory;
20+
import static org.junit.Assert.assertNotNull;
21+
import static org.junit.Assert.assertTrue;
22+
import static org.junit.Assert.fail;
23+
24+
import jakarta.jms.Connection;
25+
import jakarta.jms.JMSException;
26+
import org.apache.activemq.ActiveMQConnectionFactory;
27+
import org.junit.AfterClass;
28+
import org.junit.Before;
29+
import org.junit.Test;
30+
31+
public class VMTransportFactoryTest {
32+
33+
@Before
34+
public void setUp() throws Exception {
35+
// reset before each test
36+
resetVmTransportFactory();
37+
}
38+
39+
@AfterClass
40+
public static void tearDown() throws Exception {
41+
resetVmTransportFactory();
42+
}
43+
44+
@Test
45+
public void testDefaults() throws Exception {
46+
// broker and properties allowed by default
47+
assertBrokerCreated("vm:localhost?persistent=false");
48+
assertBrokerCreated("vm:(broker:(tcp://localhost:0)?persistent=false)");
49+
assertBrokerCreated("vm://localhost?brokerConfig=properties:org/apache/activemq/config/broker.properties");
50+
51+
// xbean not allowed by default
52+
assertBrokerStartError("vm://localhost?brokerConfig=xbean:activemq.xml");
53+
}
54+
55+
@Test
56+
public void testAllowAll() throws Exception {
57+
resetVmTransportFactory("broker,properties,xbean");
58+
59+
// broker and properties allowed by default
60+
assertBrokerCreated("vm:localhost?persistent=false");
61+
assertBrokerCreated("vm:(broker:(tcp://localhost:0)?persistent=false)");
62+
assertBrokerCreated("vm://localhost?brokerConfig=properties:org/apache/activemq/config/broker.properties");
63+
64+
// xbean now allowed
65+
assertBrokerCreated("vm://localhost?brokerConfig=xbean:activemq.xml");
66+
}
67+
68+
@Test
69+
public void testAllowAllWildcard() throws Exception {
70+
resetVmTransportFactory("*");
71+
72+
// all allowed
73+
assertBrokerCreated("vm:localhost?persistent=false");
74+
assertBrokerCreated("vm:(broker:(tcp://localhost:0)?persistent=false)");
75+
assertBrokerCreated("vm://localhost?brokerConfig=properties:org/apache/activemq/config/broker.properties");
76+
assertBrokerCreated("vm://localhost?brokerConfig=xbean:activemq.xml");
77+
}
78+
79+
@Test
80+
public void testNullSchemes() throws Exception {
81+
// should set to defaults
82+
resetVmTransportFactory(null);
83+
84+
// broker and properties allowed by default
85+
assertBrokerCreated("vm:localhost?persistent=false");
86+
assertBrokerCreated("vm:(broker:(tcp://localhost:0)?persistent=false)");
87+
assertBrokerCreated("vm://localhost?brokerConfig=properties:org/apache/activemq/config/broker.properties");
88+
89+
// xbean not allowed by default
90+
assertBrokerStartError("vm://localhost?brokerConfig=xbean:activemq.xml");
91+
}
92+
93+
@Test
94+
public void testNoneAllowed() throws Exception {
95+
// deny all
96+
resetVmTransportFactory("");
97+
98+
// nothing allowed
99+
assertBrokerStartError("vm:localhost?persistent=false");
100+
assertBrokerStartError("vm:(broker:(tcp://localhost:0)?persistent=false)");
101+
assertBrokerStartError("vm://localhost?brokerConfig=properties:org/apache/activemq/config/broker.properties");
102+
assertBrokerStartError("vm://localhost?brokerConfig=xbean:activemq.xml");
103+
}
104+
105+
@Test
106+
public void testOneAllowed() throws Exception {
107+
// deny all
108+
resetVmTransportFactory("properties");
109+
110+
// only properties allowed
111+
assertBrokerStartError("vm:localhost?persistent=false");
112+
assertBrokerStartError("vm:(broker:(tcp://localhost:0)?persistent=false)");
113+
assertBrokerCreated("vm://localhost?brokerConfig=properties:org/apache/activemq/config/broker.properties");
114+
assertBrokerStartError("vm://localhost?brokerConfig=xbean:activemq.xml");
115+
}
116+
117+
private void assertBrokerCreated(String url) throws JMSException {
118+
final ActiveMQConnectionFactory factory = new ActiveMQConnectionFactory(url);
119+
try (Connection connection = factory.createConnection()) {
120+
assertNotNull(connection);
121+
}
122+
}
123+
124+
private void assertBrokerStartError(String url) {
125+
final ActiveMQConnectionFactory factory = new ActiveMQConnectionFactory(url);
126+
try (Connection ignored = factory.createConnection()) {
127+
fail("Should have failed with an exception");
128+
} catch (JMSException e) {
129+
Throwable cause = e.getCause() != null ? e.getCause() : e;
130+
assertTrue(cause instanceof IllegalArgumentException);
131+
}
132+
}
133+
134+
}

‎activemq-unit-tests/src/test/java/org/apache/activemq/util/VmTransportTestUtils.java‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.activemq.util;
18+
19+
import java.lang.reflect.Field;
20+
import java.util.concurrent.ConcurrentMap;
21+
import org.apache.activemq.transport.TransportFactory;
22+
import org.apache.activemq.transport.vm.VMTransportFactory;
23+
24+
public class VmTransportTestUtils {
25+
26+
public static void resetVmTransportFactory() throws Exception {
27+
resetVmTransportFactory(VMTransportFactory.DEFAULT_ALLOWED_SCHEMES);
28+
}
29+
30+
@SuppressWarnings("unchecked")
31+
public static void resetVmTransportFactory(String allowedSchemes) throws Exception {
32+
if (allowedSchemes == null) {
33+
System.clearProperty(VMTransportFactory.VM_TRANSPORT_FACTORY_SCHEMES_ENABLED_PROP);
34+
} else {
35+
// set property to allowed schemes
36+
System.setProperty(VMTransportFactory.VM_TRANSPORT_FACTORY_SCHEMES_ENABLED_PROP,
37+
allowedSchemes);
38+
}
39+
40+
// clear any cached factory so the next call will create a new transport and use
41+
// the correct property setting
42+
Field factoriesField = TransportFactory.class.getDeclaredField("TRANSPORT_FACTORYS");
43+
factoriesField.setAccessible(true);
44+
ConcurrentMap<String, TransportFactory> factories =
45+
(ConcurrentMap<String, TransportFactory>) factoriesField.get(TransportFactory.class);
46+
factories.remove("vm");
47+
}
48+
}

0 commit comments

Comments
 (0)