diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java b/activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java index b642cbf32cc..e69f248c60d 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java @@ -1076,19 +1076,22 @@ public String getBrokerName() { return brokerName; } + + // Matches a single character that is invalid in a broker name + private static final String INVALID_BROKER_NAME_CHAR_REG_EXP = "[^a-zA-Z0-9._\\-:]"; + /** * Sets the name of this broker; which must be unique in the network * * @param brokerName */ - private static final String brokerNameReplacedCharsRegExp = "[^a-zA-Z0-9\\.\\_\\-\\:]"; public void setBrokerName(String brokerName) { if (brokerName == null) { throw new NullPointerException("The broker name cannot be null"); } - String str = brokerName.replaceAll(brokerNameReplacedCharsRegExp, "_"); + String str = brokerName.replaceAll(INVALID_BROKER_NAME_CHAR_REG_EXP, "_"); if (!str.equals(brokerName)) { - LOG.error("Broker Name: {} contained illegal characters matching regExp: {} - replaced with {}", brokerName, brokerNameReplacedCharsRegExp, str); + LOG.error("Broker Name: {} contained illegal characters matching regExp: {} - replaced with {}", brokerName, INVALID_BROKER_NAME_CHAR_REG_EXP, str); } this.brokerName = str.trim(); } @@ -2431,7 +2434,6 @@ protected Broker createRegionBroker(DestinationInterceptor destinationIntercepto } destinationFactory.setRegionBroker(regionBroker); regionBroker.setKeepDurableSubsActive(keepDurableSubsActive); - regionBroker.setBrokerName(getBrokerName()); regionBroker.getDestinationStatistics().setEnabled(enableStatistics); regionBroker.setAllowTempAutoCreationOnSend(isAllowTempAutoCreationOnSend()); if (brokerId != null) { diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/BrokerView.java b/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/BrokerView.java index 6f1f2123d85..466ee30fa1e 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/BrokerView.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/BrokerView.java @@ -44,6 +44,11 @@ public class BrokerView implements BrokerViewMBean { private static final Logger LOG = LoggerFactory.getLogger(BrokerView.class); + public static final Set DENIED_TRANSPORT_SCHEMES = Collections.unmodifiableSet( + new HashSet<>(Arrays.asList("vm", "http", + "multicast", "zeroconf", "discovery", "fanout", "mock", "peer", "failover", + "proxy", "reliable", "simple", "udp"))); + ManagedRegionBroker broker; private final BrokerService brokerService; @@ -372,7 +377,7 @@ public ObjectName[] getDynamicDestinationProducers() { @Override public String addConnector(String discoveryAddress) throws Exception { - // Verify VM transport is not used + // Verify a denied transport scheme is not used validateAllowedUrl(discoveryAddress); TransportConnector connector = brokerService.addConnector(discoveryAddress); if (connector == null) { @@ -384,7 +389,7 @@ public String addConnector(String discoveryAddress) throws Exception { @Override public String addNetworkConnector(String discoveryAddress) throws Exception { - // Verify VM transport is not used + // Verify a denied transport scheme is not used validateAllowedUrl(discoveryAddress); NetworkConnector connector = brokerService.addNetworkConnector(discoveryAddress); if (connector == null) { @@ -552,10 +557,11 @@ private static void validateAllowedUrl(String uriString) throws URISyntaxExcepti validateAllowedUri(new URI(uriString), 0); } - // Validate the URI does not contain VM transport + // Validate the URI does not contain a denied transport scheme private static void validateAllowedUri(URI uri, int depth) throws URISyntaxException { // Don't allow more than 5 nested URIs to prevent blowing the stack - if (depth > 5) { + // If we are greater than 4 then this is the 5th level of composite + if (depth > 4) { throw new IllegalArgumentException("URI can't contain more than 5 nested composite URIs"); } @@ -570,19 +576,22 @@ private static void validateAllowedUri(URI uri, int depth) throws URISyntaxExcep // Each URI could be a nested composite URI so call validateAllowedUri() // to validate it. This check if composite first so we don't add to // the recursive stack depth if there's a lot of URIs that are not composite - if (URISupport.isCompositeURI(uri)) { + if (URISupport.isCompositeURI(component)) { validateAllowedUri(component, depth); } else { - validateAllowedScheme(uri.getScheme()); + validateAllowedScheme(component.getScheme()); } } } } - // We don't allow VM transport scheme to be used + // Check all denied schemes private static void validateAllowedScheme(String scheme) { - if (scheme.equals("vm")) { - throw new IllegalArgumentException("VM scheme is not allowed"); + for (String denied : DENIED_TRANSPORT_SCHEMES) { + // The schemes should be case-insensitive but ignore case as a precaution + if (scheme.equalsIgnoreCase(denied)) { + throw new IllegalArgumentException("Transport scheme '" + scheme + "' is not allowed"); + } } } } diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/DestinationView.java b/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/DestinationView.java index c695fb4ce44..d1db12e8b88 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/DestinationView.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/DestinationView.java @@ -17,6 +17,7 @@ package org.apache.activemq.broker.jmx; import java.io.IOException; +import java.net.URI; import java.net.URISyntaxException; import java.util.ArrayList; import java.util.Collections; @@ -385,7 +386,7 @@ public String sendTextMessage(String body, String user, @Sensitive String passwo @Override public String sendTextMessage(Map headers, String body, String userName, @Sensitive String password) throws Exception { - String brokerUrl = "vm://" + broker.getBrokerName(); + URI brokerUrl = broker.getVmConnectorURI(); ActiveMQDestination dest = destination.getActiveMQDestination(); ActiveMQConnectionFactory cf = new ActiveMQConnectionFactory(brokerUrl); diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/region/RegionBroker.java b/activemq-broker/src/main/java/org/apache/activemq/broker/region/RegionBroker.java index fc0afb205eb..56608aedd8e 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/region/RegionBroker.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/region/RegionBroker.java @@ -24,6 +24,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; @@ -104,7 +105,7 @@ public class RegionBroker extends EmptyBroker { private final LongSequenceGenerator sequenceGenerator = new LongSequenceGenerator(); private BrokerId brokerId; - private String brokerName; + private final String brokerName; private final Map clientIdSet = new HashMap(); private final DestinationInterceptor destinationInterceptor; private ConnectionContext adminConnectionContext; @@ -138,7 +139,8 @@ public void run() { public RegionBroker(BrokerService brokerService, TaskRunnerFactory taskRunnerFactory, SystemUsage memoryManager, DestinationFactory destinationFactory, DestinationInterceptor destinationInterceptor, Scheduler scheduler, ThreadPoolExecutor executor) throws IOException { - this.brokerService = brokerService; + this.brokerService = Objects.requireNonNull(brokerService); + this.brokerName = Objects.requireNonNull(brokerService.getBrokerName(), "The broker name cannot be null"); this.executor = executor; this.scheduler = scheduler; if (destinationFactory == null) { @@ -564,20 +566,9 @@ public void setBrokerId(BrokerId brokerId) { @Override public String getBrokerName() { - if (brokerName == null) { - try { - brokerName = InetAddressUtil.getLocalHostName().toLowerCase(Locale.ENGLISH); - } catch (Exception e) { - brokerName = "localhost"; - } - } return brokerName; } - public void setBrokerName(String brokerName) { - this.brokerName = brokerName; - } - public DestinationStatistics getDestinationStatistics() { return destinationStatistics; } diff --git a/activemq-client/src/main/java/org/apache/activemq/transport/WriteTimeoutFilter.java b/activemq-client/src/main/java/org/apache/activemq/transport/WriteTimeoutFilter.java index cfdf8597c1d..ac4afe09f2c 100644 --- a/activemq-client/src/main/java/org/apache/activemq/transport/WriteTimeoutFilter.java +++ b/activemq-client/src/main/java/org/apache/activemq/transport/WriteTimeoutFilter.java @@ -129,7 +129,12 @@ protected static boolean deRegisterWrite(WriteTimeoutFilter filter, boolean fail @Override public void start() throws Exception { - super.start(); + try { + registerWrite(this); + super.start(); + } finally { + deRegisterWrite(this, false, null); + } } @Override @@ -157,8 +162,10 @@ public void run() { while (run && filters.hasNext()) { WriteTimeoutFilter filter = filters.next(); if (filter.getWriteTimeout()<=0) continue; //no timeout set - long writeStart = filter.getWriter().getWriteTimestamp(); - long delta = (filter.getWriter().isWriting() && writeStart>0)?System.currentTimeMillis() - writeStart:-1; + TimeStampStream writer = filter.getWriter(); + if (writer == null) continue; //stream not yet initialized + long writeStart = writer.getWriteTimestamp(); + long delta = (writer.isWriting() && writeStart>0)?System.currentTimeMillis() - writeStart:-1; if (delta>filter.getWriteTimeout()) { WriteTimeoutFilter.deRegisterWrite(filter, true,null); }//if timeout diff --git a/activemq-client/src/main/java/org/apache/activemq/util/IntrospectionSupport.java b/activemq-client/src/main/java/org/apache/activemq/util/IntrospectionSupport.java index c43b3a2bdb9..d69fd5b6b26 100644 --- a/activemq-client/src/main/java/org/apache/activemq/util/IntrospectionSupport.java +++ b/activemq-client/src/main/java/org/apache/activemq/util/IntrospectionSupport.java @@ -169,6 +169,9 @@ public static boolean setProperty(Object target, String name, Object value) { if (target instanceof SSLServerSocket) { // overcome illegal access issues with internal implementation class clazz = SSLServerSocket.class; + } else if (target instanceof javax.net.ssl.SSLSocket) { + // overcome illegal access issues with internal implementation class + clazz = javax.net.ssl.SSLSocket.class; } Method setter = findSetterMethod(clazz, name); if (setter == null) { diff --git a/activemq-jms-pool/src/main/java/org/apache/activemq/jms/pool/IntrospectionSupport.java b/activemq-jms-pool/src/main/java/org/apache/activemq/jms/pool/IntrospectionSupport.java index c0b223d1376..ac45f7ffd6a 100644 --- a/activemq-jms-pool/src/main/java/org/apache/activemq/jms/pool/IntrospectionSupport.java +++ b/activemq-jms-pool/src/main/java/org/apache/activemq/jms/pool/IntrospectionSupport.java @@ -22,6 +22,7 @@ import java.util.Map.Entry; import javax.net.ssl.SSLServerSocket; +import javax.net.ssl.SSLSocket; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -60,6 +61,9 @@ public static boolean setProperty(Object target, String name, Object value) { if (target instanceof SSLServerSocket) { // overcome illegal access issues with internal implementation class clazz = SSLServerSocket.class; + } else if (target instanceof javax.net.ssl.SSLSocket) { + // overcome illegal access issues with internal implementation class + clazz = javax.net.ssl.SSLSocket.class; } Method setter = findSetterMethod(clazz, name); if (setter == null) { diff --git a/activemq-spring/src/main/java/org/apache/activemq/spring/Utils.java b/activemq-spring/src/main/java/org/apache/activemq/spring/Utils.java index ba776e605e4..c63d378afc1 100644 --- a/activemq-spring/src/main/java/org/apache/activemq/spring/Utils.java +++ b/activemq-spring/src/main/java/org/apache/activemq/spring/Utils.java @@ -19,6 +19,9 @@ import java.io.File; import java.io.FileNotFoundException; import java.net.MalformedURLException; +import java.net.URI; +import java.net.URISyntaxException; +import java.util.Set; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.FileSystemResource; import org.springframework.core.io.Resource; @@ -27,22 +30,101 @@ public class Utils { + public static final String FILE_PROTOCOL = "file"; + public static final String CLASSPATH_PROTOCOL = "classpath"; + // Special marker to indicate we want to allow remote files such as + // Windows UNC, etc + public static final String REMOTE_FILE_PROTOCOL = "remote-" + FILE_PROTOCOL; + public static Resource resourceFromString(String uri) throws MalformedURLException { - Resource resource; - File file = new File(uri); - if (file.exists()) { + // default allows all + return resourceFromString(uri, null); + } + + public static Resource resourceFromString(String uri, Set allowedProtocols) throws MalformedURLException { + // Empty set means nothing is allowed + if (allowedProtocols != null && allowedProtocols.isEmpty()) { + throw new IllegalArgumentException("No protocols are allowed for loading resources."); + } + + final Resource resource; + + // First, just try and load a local file (if it exists) and if "file" + // as part of the allow list. This preserves previous behavior of + // always optimistically trying a local file first. + if (isAllowFile(allowedProtocols, uri) && new File(uri).exists()) { resource = new FileSystemResource(uri); + // If file isn't allowed, or if the file can't be found then check + // if the string is a valid URL. If it's valid, then we need + // to validate if it's allowed before loading the URL. + // isUrl() uses URI internally so it won't actually load anything } else if (ResourceUtils.isUrl(uri)) { try { + validateUrlAllowed(uri, allowedProtocols); resource = new UrlResource(ResourceUtils.getURL(uri)); - } catch (FileNotFoundException e) { + } catch (FileNotFoundException | URISyntaxException e) { MalformedURLException malformedURLException = new MalformedURLException(uri); malformedURLException.initCause(e); - throw malformedURLException; + throw malformedURLException; } - } else { + // Fallback to trying on the classpath if not a valid Url, and we allow it which + // also preserves the previous behavior (if classpath is allowed) + } else if (isAllowClasspath(allowedProtocols)){ resource = new ClassPathResource(uri); + // Catch all fail-safe if nothing else matches. This could happen if file is allowed + // but not classpath but the file doesn't exist + } else { + throw new IllegalArgumentException("URL [" + uri + "] can't be found or the protocol" + + " is not allowed for loading resources"); } return resource; } + + // These method treats local files and remote files (that are pre-fixed + // with two forward/backward slashes) differently + static boolean isAllowFile(Set allowedProtocols, String uri) { + if (allowedProtocols == null) { + return true; + } + return isUnqualifiedRemoteFile(uri) ? allowedProtocols.contains(REMOTE_FILE_PROTOCOL) : + allowedProtocols.contains(FILE_PROTOCOL); + } + + static boolean isAllowClasspath(Set allowedProtocols) { + return allowedProtocols == null || allowedProtocols.contains(CLASSPATH_PROTOCOL); + } + + static void validateUrlAllowed(String uriString, Set allowedProtocols) + throws URISyntaxException { + // Use new URI() to get the scheme + // This is important because ResourceUtils.getURL() actually searches + // the classpath which we don't want to do if not allowed + if (allowedProtocols != null) { + final String detectedProtocol = getProtocolFromScheme(uriString); + if (detectedProtocol == null) { + throw new IllegalArgumentException("Could not detect protocol in given URI [" + uriString + "]"); + } + if (!allowedProtocols.contains(detectedProtocol)){ + throw new IllegalArgumentException("URL [" + uriString + + "] uses protocol '" + detectedProtocol + "' which is not allowed " + + "for loading URL resources"); + } + } + } + + // If this is a qualified remote file then we return a special marker + // so that we know this is trying to access a remote resource as that will be + // validated differently + private static String getProtocolFromScheme(String uriString) throws URISyntaxException { + return isQualifiedRemoteFile(uriString) ? REMOTE_FILE_PROTOCOL : + new URI(uriString).getScheme(); + } + + private static boolean isUnqualifiedRemoteFile(String uri) { + return uri.startsWith("//") || uri.startsWith("\\\\"); + } + + private static boolean isQualifiedRemoteFile(String uri) { + return uri.startsWith(FILE_PROTOCOL + "://") || uri.startsWith(FILE_PROTOCOL + ":\\\\"); + } } diff --git a/activemq-spring/src/main/java/org/apache/activemq/xbean/XBeanBrokerFactory.java b/activemq-spring/src/main/java/org/apache/activemq/xbean/XBeanBrokerFactory.java index 7cee25a9b1d..f023d53064f 100644 --- a/activemq-spring/src/main/java/org/apache/activemq/xbean/XBeanBrokerFactory.java +++ b/activemq-spring/src/main/java/org/apache/activemq/xbean/XBeanBrokerFactory.java @@ -20,7 +20,10 @@ import java.net.MalformedURLException; import java.net.URI; -import org.apache.activemq.broker.BrokerContextAware; +import java.util.Arrays; +import java.util.Collections; +import java.util.Set; +import java.util.stream.Collectors; import org.apache.activemq.broker.BrokerFactoryHandler; import org.apache.activemq.broker.BrokerService; import org.apache.activemq.spring.SpringBrokerContext; @@ -35,20 +38,38 @@ import org.springframework.beans.FatalBeanException; import org.springframework.beans.factory.xml.XmlBeanDefinitionReader; import org.springframework.context.ApplicationContext; -import org.springframework.context.ApplicationContextAware; import org.springframework.core.io.Resource; /** * */ public class XBeanBrokerFactory implements BrokerFactoryHandler { - private static final transient Logger LOG = LoggerFactory.getLogger(XBeanBrokerFactory.class); + private static final Logger LOG = LoggerFactory.getLogger(XBeanBrokerFactory.class); + + public static final String XBEAN_BROKER_FACTORY_PROTOCOLS_PROP = + "org.apache.activemq.xbean.XBEAN_BROKER_FACTORY_PROTOCOLS"; + public static final String DEFAULT_ALLOWED_PROTOCOLS = Utils.FILE_PROTOCOL + "," + Utils.CLASSPATH_PROTOCOL; + + private final Set allowedProtocols; static { PropertyEditorManager.registerEditor(URI.class, URIEditor.class); } + public XBeanBrokerFactory() { + final String allowedProtocols = System.getProperty(XBEAN_BROKER_FACTORY_PROTOCOLS_PROP, + DEFAULT_ALLOWED_PROTOCOLS); + + // Asterisk will map to null which will allow all and skip checking + // Empty string will map to an empty set and will deny all + this.allowedProtocols = !allowedProtocols.equals("*") ? + Arrays.stream(allowedProtocols.split("\\s*,\\s*")) + .filter(s -> !s.trim().isEmpty()) + .collect(Collectors.collectingAndThen(Collectors.toSet(), Collections::unmodifiableSet)) : null; + } + private boolean validate = true; + public boolean isValidate() { return validate; } @@ -75,12 +96,10 @@ public BrokerService createBroker(URI config) throws Exception { if (broker == null) { // lets try find by type String[] names = context.getBeanNamesForType(BrokerService.class); - for (int i = 0; i < names.length; i++) { - String name = names[i]; - broker = (BrokerService)context.getBean(name); - if (broker != null) { - break; - } + for (String name : names) { + // No need to check for null, this will throw an exception if not found + broker = (BrokerService) context.getBean(name); + break; } } if (broker == null) { @@ -98,8 +117,8 @@ public BrokerService createBroker(URI config) throws Exception { } protected ApplicationContext createApplicationContext(String uri) throws MalformedURLException { - Resource resource = Utils.resourceFromString(uri); - LOG.debug("Using " + resource + " from " + uri); + Resource resource = Utils.resourceFromString(uri, allowedProtocols); + LOG.debug("Using {} from {}", resource, uri); try { return new ResourceXmlApplicationContext(resource) { @Override @@ -108,9 +127,14 @@ protected void initBeanDefinitionReader(XmlBeanDefinitionReader reader) { } }; } catch (FatalBeanException errorToLog) { - LOG.error("Failed to load: " + resource + ", reason: " + errorToLog.getLocalizedMessage(), errorToLog); + LOG.error("Failed to load: {}, reason: {}", resource, errorToLog.getLocalizedMessage(), + errorToLog); throw errorToLog; } } + // Package scope for testing + Set getAllowedProtocols() { + return allowedProtocols; + } } diff --git a/activemq-spring/src/test/java/org/apache/activemq/spring/UtilsTest.java b/activemq-spring/src/test/java/org/apache/activemq/spring/UtilsTest.java new file mode 100644 index 00000000000..2a52444f1b7 --- /dev/null +++ b/activemq-spring/src/test/java/org/apache/activemq/spring/UtilsTest.java @@ -0,0 +1,381 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.activemq.spring; + +import static org.apache.activemq.spring.Utils.CLASSPATH_PROTOCOL; +import static org.apache.activemq.spring.Utils.FILE_PROTOCOL; +import static org.apache.activemq.spring.Utils.REMOTE_FILE_PROTOCOL; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.net.MalformedURLException; +import java.net.URISyntaxException; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.Callable; +import org.junit.Test; +import org.springframework.core.io.ClassPathResource; +import org.springframework.core.io.FileSystemResource; +import org.springframework.core.io.Resource; +import org.springframework.core.io.UrlResource; + +public class UtilsTest { + + @Test + public void testIsAllowLocalFile() { + Set localFileExamples = setOf("/some/absolute/file", "relative/file", "file.txt"); + for (String localFile : localFileExamples) { + assertTrue(Utils.isAllowFile(null, localFile)); + assertTrue(Utils.isAllowFile(setOf(FILE_PROTOCOL), localFile)); + assertTrue(Utils.isAllowFile(setOf(FILE_PROTOCOL, "ftp", "ssl"), localFile)); + + assertFalse(Utils.isAllowFile(setOf(CLASSPATH_PROTOCOL, "ftp", "ssl"), localFile)); + assertFalse(Utils.isAllowFile(setOf(), localFile)); + assertFalse(Utils.isAllowFile(setOf(""), localFile)); + } + + // Test a remote file isn't allowed with only local + // Check windows backward slashes as well + Set remoteFileExamples = setOf("//some/remote/file", "\\\\remote\\file"); + for (String remoteFileName : remoteFileExamples) { + assertTrue(Utils.isAllowFile(null, remoteFileName)); + // None of these should be allowed as remote-file isn't included with FILE + assertFalse(Utils.isAllowFile(setOf(FILE_PROTOCOL), remoteFileName)); + assertFalse(Utils.isAllowFile(setOf(FILE_PROTOCOL, "ftp", "ssl"), remoteFileName)); + } + } + + @Test + public void testIsAllowRemoteFile() { + // Test a remote file + Set remoteFileExamples = setOf("//some/remote/file", "\\\\remote\\file"); + for (String remoteFileName : remoteFileExamples) { + assertTrue(Utils.isAllowFile(null, remoteFileName)); + assertTrue(Utils.isAllowFile(setOf(REMOTE_FILE_PROTOCOL), remoteFileName)); + assertTrue(Utils.isAllowFile(setOf(REMOTE_FILE_PROTOCOL, "ftp", "ssl"), remoteFileName)); + assertFalse(Utils.isAllowFile(setOf(CLASSPATH_PROTOCOL, "ftp", "ssl"), remoteFileName)); + assertFalse(Utils.isAllowFile(setOf(), remoteFileName)); + assertFalse(Utils.isAllowFile(setOf(""), remoteFileName)); + } + } + + @Test + public void testIsAllowClasspath() { + assertTrue(Utils.isAllowClasspath(null)); + assertTrue(Utils.isAllowClasspath(setOf(CLASSPATH_PROTOCOL))); + assertTrue(Utils.isAllowClasspath(setOf(CLASSPATH_PROTOCOL, "ftp", "ssl"))); + assertFalse(Utils.isAllowClasspath(setOf(FILE_PROTOCOL, "ftp", "ssl"))); + assertFalse(Utils.isAllowClasspath(setOf())); + assertFalse(Utils.isAllowClasspath(setOf(""))); + } + + @Test + public void testValidateUrlAllowed() throws URISyntaxException { + // not a qualified url so this throws an exception + assertValidateUrlAllowedThrows("somefile.txt", setOf(FILE_PROTOCOL)); + + // Test File - Allowed + Utils.validateUrlAllowed("file:/somefile.txt", setOf(FILE_PROTOCOL)); + Utils.validateUrlAllowed("file:/somefile.txt", setOf(FILE_PROTOCOL, CLASSPATH_PROTOCOL)); + Utils.validateUrlAllowed("file:some/other/file.txt", null); + + // Test File - Blocked + assertValidateUrlAllowedThrows("file:somefile.txt", setOf()); + assertValidateUrlAllowedThrows("file:some/other/file.txt", setOf("")); + assertValidateUrlAllowedThrows("file:some/other/file.txt", setOf(CLASSPATH_PROTOCOL)); + + // Test Remote File - Allowed + Utils.validateUrlAllowed("file://somefile.txt", setOf(REMOTE_FILE_PROTOCOL)); + Utils.validateUrlAllowed("file://somefile.txt", setOf(FILE_PROTOCOL, REMOTE_FILE_PROTOCOL)); + Utils.validateUrlAllowed("file://some/other/file.txt", null); + + // Test File - Blocked + assertValidateUrlAllowedThrows("file://somefile.txt", setOf()); + assertValidateUrlAllowedThrows("file://some/other/file.txt", setOf("")); + assertValidateUrlAllowedThrows("file://some/other/file.txt", setOf(FILE_PROTOCOL)); + // Test extra slash, we consider everything with more than one slash to be remote to simplify + assertValidateUrlAllowedThrows("file:///some/other/file.txt", setOf(FILE_PROTOCOL)); + + // Test http - Allowed + Utils.validateUrlAllowed("http://somefile.txt", setOf("http")); + Utils.validateUrlAllowed("http://somefile.txt", setOf(FILE_PROTOCOL, "http")); + Utils.validateUrlAllowed("http://some/other/file.txt", null); + + // Test http - Blocked + assertValidateUrlAllowedThrows("http://somefile.txt", setOf()); + assertValidateUrlAllowedThrows("http://some/other/file.txt", setOf("")); + assertValidateUrlAllowedThrows("http://some/other/file.txt", setOf("ftp")); + } + + private void assertValidateUrlAllowedThrows(String uriString, Set allowedProtocols) + throws URISyntaxException { + try { + Utils.validateUrlAllowed(uriString, allowedProtocols); + fail("Should have failed with an exception"); + } catch (IllegalArgumentException ignored) { + // expected + } + } + + // Test 1: Check file and remote file is NOT allowed to load, and does NOT exist + @Test + public void testResourceFromStringFile1() throws Exception { + testResourceFromStringFile1(FILE_PROTOCOL, "doesNotExist", "file:doesNotExist"); + } + + @Test + public void testResourceFromStringRemoteFile1() throws Exception { + testResourceFromStringFile1(REMOTE_FILE_PROTOCOL, "//doesNotExist", "file://doesNotExist"); + } + + protected void testResourceFromStringFile1(String protocol, String url, String fqUrl) throws Exception { + Resource resource; + + // file not allowed, only jar + assertNotAllowed("URL [" + url + "] can't be found or the protocol is not allowed for loading resources", + () -> Utils.resourceFromString(url, setOf("jar"))); + // if classpath is allowed and not fully qualified, it will not find the file and fallback + resource = Utils.resourceFromString(url, setOf(CLASSPATH_PROTOCOL)); + assertTrue(resource instanceof ClassPathResource); + assertFalse(resource.exists()); + // fully qualified fails regardless + assertNotAllowed("URL [" + fqUrl + "] uses protocol '" + protocol + "' which is not allowed for loading URL resources", + () -> Utils.resourceFromString(fqUrl, setOf(CLASSPATH_PROTOCOL))); + // Test Uri format, empty set not allowed + assertNotAllowed("No protocols are allowed for loading resources.", + () -> Utils.resourceFromString(fqUrl, setOf())); + + } + + // Test 2: Check file and remote file is allowed to load, but it does not exist + // This will throw an exception if classpath is not allowed or fully qualified uri, + // otherwise it will fallback to try classpath if classpath is allowed + @Test + public void testResourceFromStringFile2() throws Exception { + testResourceFromStringFile2(FILE_PROTOCOL, "doesNotExist", "file:doesNotExist"); + } + + @Test + public void testResourceFromStringRemoteFile2() throws Exception { + testResourceFromStringFile2(REMOTE_FILE_PROTOCOL, "//doesNotExist", "file://doesNotExist"); + } + + protected void testResourceFromStringFile2(String protocol, String url, String fqUrl) throws Exception { + Resource resource; + + //classpath allowed so it will fallback + resource = Utils.resourceFromString(url, setOf(protocol, CLASSPATH_PROTOCOL)); + assertTrue(resource instanceof ClassPathResource); + assertFalse(resource.exists()); + resource = Utils.resourceFromString(url, null); + assertTrue(resource instanceof ClassPathResource); + assertFalse(resource.exists()); + // single argument - default is null for allowed protocols so all are allowed + resource = Utils.resourceFromString(url); + assertTrue(resource instanceof ClassPathResource); + assertFalse(resource.exists()); + // classpath not allowed so we get an exception as we can't find it + assertNotAllowed("URL [" + url + "] can't be found or the protocol is not allowed for loading resources", + () -> Utils.resourceFromString(url, setOf(protocol))); + resource = Utils.resourceFromString(fqUrl, setOf(protocol)); + assertNotNull(resource); + assertFalse(resource.exists()); + } + + // Test 3: Check file is NOT allowed to load, but it does exist + @Test + public void tetsResourceFromStringFile3() throws Exception { + Resource resource; + + // Test using "jar" as allowed so we don't fallback + assertNotAllowed("URL [src/test/resources/activemq.xml] can't be found or the protocol is not allowed for loading resources", + () -> Utils.resourceFromString("src/test/resources/activemq.xml", setOf("jar"))); + // classpath is allowed so it won't find the file and fallback to classpath + resource = Utils.resourceFromString("src/test/resources/activemq.xml", setOf(CLASSPATH_PROTOCOL)); + assertTrue(resource instanceof ClassPathResource); + assertFalse(resource.exists()); + // empty allow list + assertNotAllowed("No protocols are allowed for loading resources.", + () -> Utils.resourceFromString("src/test/resources/activemq.xml", setOf())); + // Test Uri format - only classpath allowed + assertNotAllowed("URL [file:src/test/resources/activemq.xml] uses protocol 'file' which is not allowed for loading URL resources", + () -> Utils.resourceFromString("file:src/test/resources/activemq.xml", setOf(CLASSPATH_PROTOCOL))); + } + + // Test 4: Check file is both allowed and exists + @Test + public void testResourceFromStringFile4() throws Exception { + Resource resource; + + resource = Utils.resourceFromString("src/test/resources/activemq.xml", setOf(FILE_PROTOCOL)); + assertTrue(resource instanceof FileSystemResource); + assertTrue(resource.exists()); + + // Retry with file allowed using uri format + resource = Utils.resourceFromString("file:src/test/resources/activemq.xml", setOf(FILE_PROTOCOL)); + assertTrue(resource instanceof UrlResource); + assertTrue(resource.exists()); + } + + // Test 1: Check classpath is NOT allowed to load, and does NOT exist + @Test + public void tetsResourceFromStringClasspath1() { + // Check classpath is NOT allowed to load, and does NOT exist + assertNotAllowed("URL [doesNotExist] can't be found or the protocol is not allowed for loading resources", + () -> Utils.resourceFromString("doesNotExist", setOf(FILE_PROTOCOL))); + assertNotAllowed("URL [classpath:doesNotExist] uses protocol 'classpath' which is not allowed for loading URL resources", + () -> Utils.resourceFromString("classpath:doesNotExist", setOf(FILE_PROTOCOL))); + // Test Uri format, empty set not allowed + assertNotAllowed("No protocols are allowed for loading resources.", + () -> Utils.resourceFromString("classpath:doesNotExist", setOf())); + } + + // Test 2: Check classpath is allowed to load, but it does not exist + // This will return a classpath resource because file is tried first + // but won't be found so it eventually returns a classpath resource + @Test + public void testResourceFromStringClasspath2() throws Exception { + Resource resource; + + resource = Utils.resourceFromString("doesNotExist", setOf(FILE_PROTOCOL, CLASSPATH_PROTOCOL)); + assertTrue(resource instanceof ClassPathResource); + assertFalse(resource.exists()); + resource = Utils.resourceFromString("doesNotExist", setOf(CLASSPATH_PROTOCOL)); + assertTrue(resource instanceof ClassPathResource); + assertFalse(resource.exists()); + resource = Utils.resourceFromString("doesNotExist", null); + assertTrue(resource instanceof ClassPathResource); + assertFalse(resource.exists()); + // test single argument + resource = Utils.resourceFromString("doesNotExist"); + assertTrue(resource instanceof ClassPathResource); + assertFalse(resource.exists()); + } + + // Test 3: Check classpath is NOT allowed to load, but it does exist + @Test + public void testResourceFromStringClasspath3() { + // This exists on the classpath but not allowed, only file is allowed + assertNotAllowed("URL [activemq.xml] can't be found or the protocol is not allowed for loading resources", + () -> Utils.resourceFromString("activemq.xml", setOf(FILE_PROTOCOL))); + // empty allow list + assertNotAllowed("No protocols are allowed for loading resources.", + () -> Utils.resourceFromString("activemq.xml", setOf())); + // Test Uri format - only file allowed + assertNotAllowed("URL [classpath:activemq.xml] uses protocol 'classpath' which is not allowed for loading URL resources", + () -> Utils.resourceFromString("classpath:activemq.xml", setOf(FILE_PROTOCOL))); + } + + @Test + public void testResourceFromStringClasspath4() throws Exception { + Resource resource; + + // Test 4: Check classpath is both allowed and exists + resource = Utils.resourceFromString("activemq.xml", setOf(CLASSPATH_PROTOCOL)); + assertTrue(resource instanceof ClassPathResource); + assertTrue(resource.exists()); + // Retry with classpath allowed using uri format + resource = Utils.resourceFromString("classpath:activemq.xml", setOf(CLASSPATH_PROTOCOL)); + assertTrue(resource instanceof UrlResource); + assertTrue(resource.exists()); + } + + // Test URIs not allowed + @Test + public void testResourceFromStringUri1() throws Exception { + Resource resource; + + // none of these protocols are allowed + assertNotAllowed("URL [file://invalid] uses protocol 'remote-file' which is not allowed for loading URL resources", + () -> Utils.resourceFromString("file://invalid", setOf(FILE_PROTOCOL,CLASSPATH_PROTOCOL))); + assertNotAllowed("URL [file:\\\\invalid] uses protocol 'remote-file' which is not allowed for loading URL resources", + () -> Utils.resourceFromString("file:\\\\invalid", setOf(FILE_PROTOCOL,CLASSPATH_PROTOCOL))); + assertNotAllowed("URL [http://invalid] uses protocol 'http' which is not allowed for loading URL resources", + () -> Utils.resourceFromString("http://invalid", setOf(FILE_PROTOCOL,CLASSPATH_PROTOCOL))); + assertNotAllowed("URL [ftp://invalid] uses protocol 'ftp' which is not allowed for loading URL resources", + () -> Utils.resourceFromString("ftp://invalid", setOf(FILE_PROTOCOL,CLASSPATH_PROTOCOL))); + assertNotAllowed("URL [jar:file:invalid.jar!/] uses protocol 'jar' which is not allowed for loading URL resources", + () -> Utils.resourceFromString("jar:file:invalid.jar!/", setOf(FILE_PROTOCOL,CLASSPATH_PROTOCOL))); + assertNotAllowed("No protocols are allowed for loading resources.", + () -> Utils.resourceFromString("http://invalid", setOf())); + // malformed + try { + // not allowed but should have malformed error before it even checks + Utils.resourceFromString("http:", setOf("http")); + fail("should have exception"); + } catch (MalformedURLException e) { + assertTrue(e.getCause() instanceof URISyntaxException); + } + + // special edge case - "bad" is not a valid protocol so it skips the URI loading + // and falls back to a classpath search, which is allowed. That will of course fail because + // it's not a valid classpath entry + resource = Utils.resourceFromString("bad://doesNotExist", setOf(CLASSPATH_PROTOCOL)); + assertTrue(resource instanceof ClassPathResource); + assertFalse(resource.exists()); + resource = Utils.resourceFromString("bad:doesNotExist", setOf(CLASSPATH_PROTOCOL)); + assertTrue(resource instanceof ClassPathResource); + assertFalse(resource.exists()); + + // classpath is now not allowed either so it fails + assertNotAllowed("URL [bad://invalid] can't be found or the protocol is not allowed for loading resources", + () -> Utils.resourceFromString("bad://invalid", setOf(FILE_PROTOCOL))); + } + + // check urls that are allowed + @Test + public void testResourceFromStringUri2() throws Exception { + Resource resource; + + // we should be able to build the resources now that they allowed even if they don't exist + resource = Utils.resourceFromString("http://doesNotExist", setOf("http")); + assertTrue(resource instanceof UrlResource); + assertFalse(resource.exists()); + + resource = Utils.resourceFromString("jar:file:invalid.jar!/", setOf("jar")); + assertTrue(resource instanceof UrlResource); + assertFalse(resource.exists()); + + try { + // allowed but should have malformed error + Utils.resourceFromString("http:", setOf("http")); + fail("should have exception"); + } catch (MalformedURLException e) { + assertTrue(e.getCause() instanceof URISyntaxException); + } + } + + private static void assertNotAllowed(String expected, Callable callable) { + try { + callable.call(); + fail("Should have failed with Exception"); + } catch (Exception e) { + assertTrue(e instanceof IllegalArgumentException); + assertEquals(expected, e.getMessage()); + } + } + + @SafeVarargs + private static Set setOf(T... elements) { + return Collections.unmodifiableSet(new HashSet<>(Arrays.asList(elements))); + } +} diff --git a/activemq-spring/src/test/java/org/apache/activemq/xbean/XBeanBrokerFactoryTest.java b/activemq-spring/src/test/java/org/apache/activemq/xbean/XBeanBrokerFactoryTest.java new file mode 100644 index 00000000000..2154e03b2ee --- /dev/null +++ b/activemq-spring/src/test/java/org/apache/activemq/xbean/XBeanBrokerFactoryTest.java @@ -0,0 +1,254 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.activemq.xbean; + +import static org.apache.activemq.xbean.XBeanBrokerFactory.DEFAULT_ALLOWED_PROTOCOLS; +import static org.apache.activemq.xbean.XBeanBrokerFactory.XBEAN_BROKER_FACTORY_PROTOCOLS_PROP; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.io.FileNotFoundException; +import java.net.UnknownHostException; +import java.nio.file.NoSuchFileException; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; +import org.apache.activemq.broker.BrokerFactory; +import org.apache.activemq.broker.BrokerService; +import org.apache.activemq.spring.Utils; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.Test; +import org.springframework.beans.FatalBeanException; + +public class XBeanBrokerFactoryTest { + + @Before + public void setUp() throws Exception { + // reset before each test + System.setProperty(XBEAN_BROKER_FACTORY_PROTOCOLS_PROP, DEFAULT_ALLOWED_PROTOCOLS); + } + + @AfterClass + public static void tearDown() throws Exception { + System.setProperty(XBEAN_BROKER_FACTORY_PROTOCOLS_PROP, DEFAULT_ALLOWED_PROTOCOLS); + } + + @Test + public void testXBeanAllowedProtocolParsing() throws Exception { + // new instance will read the current property protocol prop and build set + XBeanBrokerFactory factory = new XBeanBrokerFactory(); + assertEquals(setOf(Utils.FILE_PROTOCOL, Utils.CLASSPATH_PROTOCOL), factory.getAllowedProtocols()); + + // set property + System.setProperty(XBEAN_BROKER_FACTORY_PROTOCOLS_PROP, "file,jar"); + factory = new XBeanBrokerFactory(); + assertEquals(setOf("jar","file"), factory.getAllowedProtocols()); + System.setProperty(XBEAN_BROKER_FACTORY_PROTOCOLS_PROP, "http"); + factory = new XBeanBrokerFactory(); + assertEquals(setOf("http"), factory.getAllowedProtocols()); + + // check allow all + System.setProperty(XBEAN_BROKER_FACTORY_PROTOCOLS_PROP, "*"); + factory = new XBeanBrokerFactory(); + assertNull(factory.getAllowedProtocols()); + + // check allow none + System.setProperty(XBEAN_BROKER_FACTORY_PROTOCOLS_PROP, ""); + factory = new XBeanBrokerFactory(); + assertTrue(factory.getAllowedProtocols().isEmpty()); + + // test empty and white space only + System.setProperty(XBEAN_BROKER_FACTORY_PROTOCOLS_PROP, "jar , ftp, http"); + factory = new XBeanBrokerFactory(); + assertEquals(setOf("jar","ftp", "http"), factory.getAllowedProtocols()); + System.setProperty(XBEAN_BROKER_FACTORY_PROTOCOLS_PROP, " "); + factory = new XBeanBrokerFactory(); + assertTrue(factory.getAllowedProtocols().isEmpty()); + } + + + // Test default protocols + @Test + public void testDefaultXBeanProtocols() throws Exception { + // file works + startBroker("xbean:src/test/resources/spring/xbean-test.xml"); + startBroker("xbean:file:src/test/resources/spring/xbean-test.xml"); + + // classpath works + startBroker("xbean:spring/xbean-test.xml"); + startBroker("xbean:classpath:spring/xbean-test.xml"); + + // http/fttp blocked by default + startBrokerNotAllowedError("xbean:http://bad/xbean-test.xml"); + startBrokerNotAllowedError("xbean:ftp:bad/xbean-test.xml"); + // should get illegal state exception, we are not allowed to use the jar protocol + startBrokerNotAllowedError("xbean:jar:file:invalid.jar!/"); + // remote file is blocked + startBrokerNotAllowedError("xbean:file://remote/xbean-test.xml"); + + // custom is not a known protocol so this is detected as invalid protocol + // and not a URI, so it fallsback and tries classpath which is allowed + // but won't be found + startBrokerBeanError("xbean:custom://invalid", FileNotFoundException.class); + } + + @Test + public void testXBeanAllowNone() throws Exception { + // Allow nothing + System.setProperty(XBEAN_BROKER_FACTORY_PROTOCOLS_PROP, ""); + + // everything is not allowed + startBrokerNotAllowedError("xbean:http://bad/xbean-test.xml", + "No protocols are allowed for loading resources"); + startBrokerNotAllowedError("xbean:ftp:bad/xbean-test.xml", + "No protocols are allowed for loading resources"); + startBrokerNotAllowedError("xbean:jar:file:invalid.jar!/", + "No protocols are allowed for loading resources"); + startBrokerNotAllowedError("xbean:src/test/resources/spring/xbean-test.xml", + "No protocols are allowed for loading resources"); + startBrokerNotAllowedError("xbean://src/test/resources/spring/xbean-test.xml", + "No protocols are allowed for loading resources"); + startBrokerNotAllowedError("xbean:file:src/test/resources/spring/xbean-test.xml", + "No protocols are allowed for loading resources"); + startBrokerNotAllowedError("xbean:file://src/test/resources/spring/xbean-test.xml", + "No protocols are allowed for loading resources"); + startBrokerNotAllowedError("xbean:classpath:src/test/resources/spring/xbean-test.xml", + "No protocols are allowed for loading resources"); + } + + @Test + public void testXBeanAllowAll() throws Exception { + // set to asterisk to allow all + System.setProperty(XBEAN_BROKER_FACTORY_PROTOCOLS_PROP, "*"); + + // file works + startBroker("xbean:src/test/resources/spring/xbean-test.xml"); + startBroker("xbean:file:src/test/resources/spring/xbean-test.xml"); + // classpath works + startBroker("xbean:spring/xbean-test.xml"); + startBroker("xbean:classpath:spring/xbean-test.xml"); + // jar, http, ftp are allowed so they just get bean errors as can't be found + startBrokerBeanError("xbean:jar:file:invalid.jar!/", NoSuchFileException.class); + startBrokerBeanError("xbean:ftp://invalid", UnknownHostException.class); + startBrokerBeanError("xbean:http://invalid", UnknownHostException.class); + // check remote file too + startBrokerBeanError("xbean:file://invalid", UnknownHostException.class); + } + + @Test + public void testDefaultXBeanProtocolsCustom() throws Exception { + // update allowed list + System.setProperty(XBEAN_BROKER_FACTORY_PROTOCOLS_PROP, "jar,ftp,http"); + + // jar is allowed, but file is invalid so we get a different error + startBrokerBeanError("xbean:jar:file:invalid.jar!/", NoSuchFileException.class); + + // ftp/http is allowed but url is invalid we get a different error + // because the host is unknown + startBrokerBeanError("xbean:ftp://invalid", UnknownHostException.class); + startBrokerBeanError("xbean:http://invalid", UnknownHostException.class); + + // file, remote file and classpath are all blocked + startBrokerNotAllowedError("xbean:src/test/resources/spring/xbean-test.xml", + "can't be found or the protocol is not allowed"); + startBrokerNotAllowedError("xbean://remote/spring/xbean-test.xml", + "can't be found or the protocol is not allowed"); + startBrokerNotAllowedError("xbean:file:src/test/resources/spring/xbean-test.xml"); + startBrokerNotAllowedError("xbean:file://remote/spring/xbean-test.xml"); + startBrokerNotAllowedError("xbean:spring/xbean-test.xml", + "can't be found or the protocol is not allowed"); + startBrokerNotAllowedError("xbean:classpath:spring/xbean-test.xml"); + } + + @Test + public void testXBeanProtocolsOnlyFileOrClasspath() throws Exception { + // block classpath + System.setProperty(XBEAN_BROKER_FACTORY_PROTOCOLS_PROP, Utils.FILE_PROTOCOL); + + // Files should work fine + startBroker("xbean:src/test/resources/spring/xbean-test.xml"); + startBroker("xbean:file:src/test/resources/spring/xbean-test.xml"); + + // Classpath entries won't work + // not a URI and classpath isn't allowed so errors out + startBrokerNotAllowedError("xbean:spring/xbean-test.xml", "can't be found or the protocol is not allowed"); + startBrokerNotAllowedError("xbean:classpath:spring/xbean-test.xml"); + + // block files, allow classpath + System.setProperty(XBEAN_BROKER_FACTORY_PROTOCOLS_PROP, Utils.CLASSPATH_PROTOCOL); + + // Files should now break + // This will fallback to trying classpath because file isn't allowed and it isn't + // a qualified URI so it will just get a bean exception as won't be on the classpath + startBrokerBeanError("xbean:src/test/resources/spring/xbean-test.xml", + FileNotFoundException.class); + // qualified URI will try file no matter won't and will be blocked as file is not allowed + // so it won't fall back + startBrokerNotAllowedError("xbean:file:src/test/resources/spring/xbean-test.xml"); + + // classpath should work + startBroker("xbean:spring/xbean-test.xml"); + startBroker("xbean:classpath:spring/xbean-test.xml"); + } + + private void startBrokerBeanError(String url, Class expected) throws Exception { + try { + startBroker(url); + fail("Should have failed with an exception"); + } catch (FatalBeanException e) { + Throwable cause = e.getCause() != null ? e.getCause() : e; + assertTrue(expected.isAssignableFrom(cause.getClass())); + } + } + + private void startBrokerNotAllowedError(String url) throws Exception { + startBrokerNotAllowedError(url, "which is not allowed for loading URL resources"); + } + + private void startBrokerNotAllowedError(String url, String expected) throws Exception { + try { + startBroker(url); + fail("Should have failed with an exception"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains(expected)); + } + } + + private void startBroker(String url) throws Exception { + BrokerService broker = null; + try { + broker = BrokerFactory.createBroker(url); + assertNotNull(broker); + broker.stop(); + } finally { + if (broker != null) { + broker.stop(); + broker.waitUntilStopped(); + } + } + } + + @SafeVarargs + private static Set setOf(T... elements) { + return Collections.unmodifiableSet(new HashSet<>(Arrays.asList(elements))); + } +} diff --git a/activemq-spring/src/test/resources/spring/xbean-test.xml b/activemq-spring/src/test/resources/spring/xbean-test.xml new file mode 100644 index 00000000000..36c1a2b1e76 --- /dev/null +++ b/activemq-spring/src/test/resources/spring/xbean-test.xml @@ -0,0 +1,32 @@ + + + + + + + + + + + + + diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/BrokerServiceTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/BrokerServiceTest.java index bbf1c9f84ac..aab6c1b197c 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/BrokerServiceTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/BrokerServiceTest.java @@ -91,6 +91,44 @@ public void testSystemUsage() { assertEquals( 1024L * 1024 * 1024 * 100, service.getSystemUsage().getStoreUsage().getLimit() ); } + public void testSetBrokerNameInvalidChars() { + final BrokerService brokerService = new BrokerService(); + + // All valid + brokerService.setBrokerName("valid"); + assertEquals("valid", brokerService.getBrokerName()); + brokerService.setBrokerName("valid123"); + assertEquals("valid123", brokerService.getBrokerName()); + brokerService.setBrokerName("this_is_valid"); + assertEquals("this_is_valid", brokerService.getBrokerName()); + brokerService.setBrokerName("this_123_valid"); + assertEquals("this_123_valid", brokerService.getBrokerName()); + brokerService.setBrokerName("valid-name123"); + assertEquals("valid-name123", brokerService.getBrokerName()); + brokerService.setBrokerName("1235.6789"); + assertEquals("1235.6789", brokerService.getBrokerName()); + brokerService.setBrokerName("valid:123"); + assertEquals("valid:123", brokerService.getBrokerName()); + + // Test invalid names + brokerService.setBrokerName("abc?bad"); + assertEquals("abc_bad", brokerService.getBrokerName()); + brokerService.setBrokerName("#"); + assertEquals("_", brokerService.getBrokerName()); + brokerService.setBrokerName("?"); + assertEquals("_", brokerService.getBrokerName()); + brokerService.setBrokerName("invalid%"); + assertEquals("invalid_", brokerService.getBrokerName()); + brokerService.setBrokerName("\\"); + assertEquals("_", brokerService.getBrokerName()); + brokerService.setBrokerName("<>"); + assertEquals("__", brokerService.getBrokerName()); + brokerService.setBrokerName("abc="); + assertEquals("abc_", brokerService.getBrokerName()); + brokerService.setBrokerName("name:abc=?bad"); + assertEquals("name:abc__bad", brokerService.getBrokerName()); + } + @Test public void testLargeFileSystem() throws Exception { BrokerService service = new BrokerService(); diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/jmx/MBeanTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/jmx/MBeanTest.java index 3f544028ba4..f8dd452ce91 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/jmx/MBeanTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/jmx/MBeanTest.java @@ -16,8 +16,7 @@ */ package org.apache.activemq.broker.jmx; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.fail; +import static org.apache.activemq.broker.jmx.BrokerView.DENIED_TRANSPORT_SCHEMES; import java.io.BufferedReader; import java.io.InputStreamReader; @@ -67,7 +66,6 @@ import org.apache.activemq.util.JMXSupport; import org.apache.activemq.util.URISupport; import org.apache.activemq.util.Wait; -import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -2033,44 +2031,56 @@ public void testSubscriptionViewProperties() throws Exception { assertTrue(subscription.isExclusive()); } - // Test to verify VM transport is not allowed to be added as a connector + // Test to verify blocked transport schemes are not allowed to be added as a connector // through the Broker MBean - public void testAddVmConnectorBlockedBrokerView() throws Exception { + public void testAddConnectorBlockedBrokerView() throws Exception { + for (String deniedScheme : DENIED_TRANSPORT_SCHEMES) { + LOG.info("verify testAddConnectorBlockedBrokerView scheme: {}", deniedScheme); + testAddTransportConnectorBlockedBrokerView(deniedScheme); + } + } + + protected void testAddTransportConnectorBlockedBrokerView(String scheme) 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"); + brokerView.addConnector(scheme + "://localhost"); + fail("Should have failed trying to add connector with scheme: " + scheme); } catch (IllegalArgumentException e) { - assertEquals("VM scheme is not allowed", e.getMessage()); + assertEquals("Transport scheme '" + 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"); + brokerView.addConnector("static:(tcp://0.0.0.0:0," + scheme + "://" + brokerName + ")"); + fail("Should have failed trying to add connector with scheme: " + scheme); } catch (IllegalArgumentException e) { - assertEquals("VM scheme is not allowed", e.getMessage()); + assertEquals("Transport scheme '" + scheme + "' is not allowed", e.getMessage()); } try { // verify nested composite URI is blocked - brokerView.addConnector("failover:(failover:(failover:(vm://localhost)))"); - fail("Should have failed trying to add vm connector"); + brokerView.addConnector("static:(static:(static:(" + scheme + "://localhost)))"); + fail("Should have failed trying to add connector with scheme: " + scheme); } catch (IllegalArgumentException e) { - assertEquals("VM scheme is not allowed", e.getMessage()); + assertEquals("Transport scheme '" + scheme + "' is not allowed", e.getMessage()); } + } + + // Test too many nested URIs + public void testNestedAddTransportConnector() throws Exception { + ObjectName brokerName = assertRegisteredObjectName(domain + ":type=Broker,brokerName=localhost"); + BrokerViewMBean brokerView = MBeanServerInvocationHandler.newProxyInstance(mbeanServer, brokerName, BrokerViewMBean.class, true); try { // verify nested composite URI with more than 5 levels is blocked brokerView.addConnector( - "static:(failover:(failover:(failover:(failover:(failover:(tcp://localhost:0))))))"); + "static:(static:(static:(static:(static:(static:(tcp://localhost:0))))))"); fail("Should have failed trying to add vm connector bridge"); } catch (IllegalArgumentException e) { assertEquals("URI can't contain more than 5 nested composite URIs", e.getMessage()); } - } } diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/jmx/JmxCreateNCTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/jmx/JmxCreateNCTest.java index 141e5c0ee82..3d3b8c1cdc5 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/jmx/JmxCreateNCTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/jmx/JmxCreateNCTest.java @@ -20,12 +20,14 @@ 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 org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import static org.apache.activemq.broker.jmx.BrokerView.DENIED_TRANSPORT_SCHEMES; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; @@ -33,11 +35,11 @@ /** * This test shows that when we create a network connector via JMX, * the NC/bridge shows up in the MBean Server - * - * @author Christian Posta */ public class JmxCreateNCTest { + private static final Logger LOG = LoggerFactory.getLogger(JmxCreateNCTest.class); + private static final String BROKER_NAME = "jmx-broker"; private BrokerService broker; @@ -81,46 +83,51 @@ public void testBridgeRegistration() throws Exception { } @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()); + public void testTransportSchemeBridgeBlocked() throws Exception { + for (String deniedScheme : DENIED_TRANSPORT_SCHEMES) { + LOG.info("verify testTransportSchemeBridgeBlocked scheme: {}", deniedScheme); + testTransportSchemeBridgeBlocked(deniedScheme); } + } + protected void testTransportSchemeBridgeBlocked(String scheme) throws Exception { + // Test composite network connector uri try { - proxy.addNetworkConnector("multicast:(vm://localhost)"); - fail("Should have failed trying to add vm connector bridge"); + proxy.addNetworkConnector("static:(" + scheme + "://localhost)"); + fail("Should have failed trying to add connector bridge with scheme: " + scheme); } catch (IllegalArgumentException e) { - assertEquals("VM scheme is not allowed", e.getMessage()); + assertEquals("Transport scheme '" + scheme + "' is not allowed", e.getMessage()); } - // verify direct vm as well + // verify direct connector as well try { - proxy.addNetworkConnector("vm://localhost"); - fail("Should have failed trying to add vm connector bridge"); + proxy.addNetworkConnector(scheme + "://localhost"); + fail("Should have failed trying to add connector bridge with scheme: " + scheme); } catch (IllegalArgumentException e) { - assertEquals("VM scheme is not allowed", e.getMessage()); + assertEquals("Transport scheme '" + scheme + "' is not allowed", e.getMessage()); } try { // verify nested composite URI is blocked - proxy.addNetworkConnector("static:(failover:(failover:(tcp://localhost:0,vm://localhost)))"); - fail("Should have failed trying to add vm connector bridge"); + proxy.addNetworkConnector("static:(static:(static:(tcp://localhost:0," + scheme + "://localhost)))"); + fail("Should have failed trying to add connector bridge with scheme: " + scheme); } catch (IllegalArgumentException e) { - assertEquals("VM scheme is not allowed", e.getMessage()); + assertEquals("Transport scheme '" + scheme + "' is not allowed", e.getMessage()); } } @Test public void testAddNetworkConnectorMaxComposite() throws Exception { + // Should allow 5 nested (excludes first wrapper) so no exception thrown + assertNotNull(proxy.addNetworkConnector( + "static:(static:(static:(static:(static:(bad://localhost)))))")); + try { - // verify nested composite URI with more than 5 levels is blocked + // verify nested composite URI with more than 5 levels is blocked. This has 6 nested + // (not including first wrapper url proxy.addNetworkConnector( - "static:(failover:(failover:(failover:(failover:(failover:(tcp://localhost:0))))))"); - fail("Should have failed trying to add vm connector bridge"); + "static:(static:(static:(static:(static:(static:(tcp://localhost:0))))))"); + fail("Should have failed trying to add more than 5 connector bridges"); } catch (IllegalArgumentException e) { assertEquals("URI can't contain more than 5 nested composite URIs", e.getMessage()); } diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/spring/ActiveMQConnectionFactoryXBeanTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/spring/ActiveMQConnectionFactoryXBeanTest.java new file mode 100644 index 00000000000..93055c6a59c --- /dev/null +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/spring/ActiveMQConnectionFactoryXBeanTest.java @@ -0,0 +1,222 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.activemq.spring; + +import static org.apache.activemq.xbean.XBeanBrokerFactory.DEFAULT_ALLOWED_PROTOCOLS; +import static org.apache.activemq.xbean.XBeanBrokerFactory.XBEAN_BROKER_FACTORY_PROTOCOLS_PROP; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import javax.jms.Connection; +import javax.jms.JMSException; +import java.io.FileNotFoundException; +import java.net.UnknownHostException; +import java.nio.file.NoSuchFileException; +import org.apache.activemq.ActiveMQConnectionFactory; +import org.apache.activemq.xbean.XBeanBrokerFactory; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.Test; +import org.springframework.beans.FatalBeanException; + +public class ActiveMQConnectionFactoryXBeanTest { + + @Before + public void setUp() throws Exception { + // reset before each test + System.setProperty(XBEAN_BROKER_FACTORY_PROTOCOLS_PROP, DEFAULT_ALLOWED_PROTOCOLS); + } + + @AfterClass + public static void tearDown() throws Exception { + System.setProperty(XBEAN_BROKER_FACTORY_PROTOCOLS_PROP, DEFAULT_ALLOWED_PROTOCOLS); + } + + // File and classpath are allowed by default + @Test + public void testCreateBrokerDefaults() throws Exception { + // File resources + assertBrokerCreated("vm://localhost?brokerConfig=xbean:src/test/resources/activemq.xml"); + assertBrokerCreated("vm://localhost?brokerConfig=xbean:file:src/test/resources/activemq.xml"); + + // Classpath resources + assertBrokerCreated("vm://localhost?brokerConfig=xbean:activemq.xml"); + assertBrokerCreated("vm://localhost?brokerConfig=xbean:classpath:activemq.xml"); + + // Remote file not allowed by default, falls back to classpath as it isn't fully qualified + assertBrokerStartError("vm://localhost?brokerConfig=xbean://activemq.xml", + FileNotFoundException.class); + // Remote file not allowed by default + assertUrlNotAllowed("vm://localhost?brokerConfig=xbean:file://activemq.xml"); + + // other URL types blocked + assertUrlNotAllowed("vm://localhost?brokerConfig=xbean:http://activemq.xml"); + assertUrlNotAllowed("vm://localhost?brokerConfig=xbean:ftp://activemq.xml"); + assertUrlNotAllowed("vm://localhost?brokerConfig=xbean:jar:file:invalid.jar!/"); + + // Custom is not a valid protocol so this does not get processed as a URI + // so classpath is tried (as it is allowed and gets tried last with no uri + // prefix). Spring won't find the file on the classpath. + assertBrokerStartError("vm://localhost?brokerConfig=xbean:custom:activemq.xml", + FileNotFoundException.class); + } + + @Test + public void testCreateBrokerAllowNone() { + // empty allows none + System.setProperty(XBeanBrokerFactory.XBEAN_BROKER_FACTORY_PROTOCOLS_PROP, ""); + + // File resources blocked + assertUrlNotAllowed("vm://localhost?brokerConfig=xbean:src/test/resources/activemq.xml", + "No protocols are allowed for loading resources"); + assertUrlNotAllowed("vm://localhost?brokerConfig=xbean:file:src/test/resources/activemq.xml", + "No protocols are allowed for loading resources"); + + // Remote file resources blocked + assertUrlNotAllowed("vm://localhost?brokerConfig=xbean://remote/resources/activemq.xml", + "No protocols are allowed for loading resources"); + assertUrlNotAllowed("vm://localhost?brokerConfig=xbean:file://remote/resources/activemq.xml", + "No protocols are allowed for loading resources"); + + // Classpath resources blocked + assertUrlNotAllowed("vm://localhost?brokerConfig=xbean:activemq.xml", + "No protocols are allowed for loading resources"); + assertUrlNotAllowed("vm://localhost?brokerConfig=xbean:classpath:activemq.xml", + "No protocols are allowed for loading resources"); + + //others blocked, we get IllegalArgumentException without trying to load + assertUrlNotAllowed("vm://localhost?brokerConfig=xbean:http://invalid", + "No protocols are allowed for loading resources"); + assertUrlNotAllowed("vm://localhost?brokerConfig=xbean:ftp://invalid", + "No protocols are allowed for loading resources"); + assertUrlNotAllowed("vm://localhost?brokerConfig=xbean:jar:file:invalid.jar!/", + "No protocols are allowed for loading resources"); + } + + // File and classpath are allowed by default + @Test + public void testCreateBrokerAllowAll() throws Exception { + // allow all with asterisk + System.setProperty(XBeanBrokerFactory.XBEAN_BROKER_FACTORY_PROTOCOLS_PROP, "*"); + + // File resources + assertBrokerCreated("vm://localhost?brokerConfig=xbean:src/test/resources/activemq.xml"); + assertBrokerCreated("vm://localhost?brokerConfig=xbean:file:src/test/resources/activemq.xml"); + + // Classpath resources + assertBrokerCreated("vm://localhost?brokerConfig=xbean:activemq.xml"); + assertBrokerCreated("vm://localhost?brokerConfig=xbean:classpath:activemq.xml"); + + // http/ftp allowed but unknown host + assertBrokerStartError("vm://localhost?brokerConfig=xbean:http://invalid", + UnknownHostException.class); + assertBrokerStartError("vm://localhost?brokerConfig=xbean:ftp://invalid", + UnknownHostException.class); + } + + @Test + public void testCreateBrokerPropConfigured() throws Exception { + // allow only file and jar + System.setProperty(XBeanBrokerFactory.XBEAN_BROKER_FACTORY_PROTOCOLS_PROP, "file,jar,ftp"); + + // File resources - allowed + assertBrokerCreated("vm://localhost?brokerConfig=xbean:src/test/resources/activemq.xml"); + assertBrokerCreated("vm://localhost?brokerConfig=xbean:file:src/test/resources/activemq.xml"); + + // Remote file resources - not allowed + assertUrlNotAllowed("vm://localhost?brokerConfig=xbean://activemq.xml", + "can't be found or the protocol is not allowed"); + assertUrlNotAllowed("vm://localhost?brokerConfig=xbean:file://activemq.xml"); + + // Classpath resources - not allowed + assertUrlNotAllowed("vm://localhost?brokerConfig=xbean:activemq.xml", + "can't be found or the protocol is not allowed"); + assertUrlNotAllowed("vm://localhost?brokerConfig=xbean:classpath:activemq.xml"); + + // http not allowed + assertUrlNotAllowed("vm://localhost?brokerConfig=xbean:http://invalid"); + + // ftp is allowed, but can't be found with bad host + assertBrokerStartError("vm://localhost?brokerConfig=xbean:ftp://invalid", + UnknownHostException.class); + + // jar is now allowed but file doesn't exist + assertBrokerStartError("vm://localhost?brokerConfig=xbean:jar:file:invalid.jar!/", NoSuchFileException.class); + + System.setProperty(XBeanBrokerFactory.XBEAN_BROKER_FACTORY_PROTOCOLS_PROP, "remote-file,jar,ftp"); + // remote file is now allowed, but can't be found with bad host + assertBrokerStartError("vm://localhost?brokerConfig=xbean:file://invalid", + UnknownHostException.class); + } + + private void assertBrokerCreated(final String url) throws JMSException { + final ActiveMQConnectionFactory factory = new ActiveMQConnectionFactory(url); + final Connection connection = factory.createConnection(); + try { + assertNotNull(connection); + } finally { + connection.close(); + } + } + + private void assertUrlNotAllowed(final String url) { + assertUrlNotAllowed(url, " which is not allowed for loading URL resources"); + } + + private void assertUrlNotAllowed(final String url, final String error) { + final ActiveMQConnectionFactory factory = new ActiveMQConnectionFactory(url); + Connection connection = null; + try { + connection = factory.createConnection(); + fail("Should of thrown exception"); + } catch (JMSException e) { + assertTrue(e.getCause() instanceof IllegalArgumentException); + assertTrue(e.getMessage().contains(error)); + } finally { + if (connection != null) { + try { + connection.close(); + } catch (JMSException e) { + // ignore + } + } + } + } + + private void assertBrokerStartError(final String url, final Class expected) { + final ActiveMQConnectionFactory factory = new ActiveMQConnectionFactory(url); + Connection connection = null; + try { + connection = factory.createConnection(); + fail("Should have failed with an exception"); + } catch (JMSException e) { + Throwable cause = e.getCause() != null ? e.getCause() : e; + assertTrue(cause instanceof FatalBeanException); + cause = cause.getCause() != null ? cause.getCause() : cause; + assertTrue(expected.isAssignableFrom(cause.getClass())); + } finally { + if (connection != null) { + try { + connection.close(); + } catch (JMSException e) { + // ignore + } + } + } + } +} diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/transport/SoWriteTimeoutSslHandshakeTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/transport/SoWriteTimeoutSslHandshakeTest.java new file mode 100644 index 00000000000..08ee9a55a4f --- /dev/null +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/transport/SoWriteTimeoutSslHandshakeTest.java @@ -0,0 +1,194 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.activemq.transport; + +import java.io.IOException; +import java.net.ServerSocket; +import java.net.Socket; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; + +import javax.jms.Connection; +import javax.jms.JMSException; + +import org.apache.activemq.ActiveMQConnectionFactory; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.fail; + + +/** + * Test for AMQ-9569: WriteTimeoutFilter does not timeout SSL write (handshake). + * + * This test demonstrates that when a client connects via SSL to a server that + * accepts the TCP connection but never responds to the SSL handshake, the + * WriteTimeoutFilter does NOT enforce the soWriteTimeout during transport start(). + * + * The SSL handshake is triggered during WireFormatNegotiator.start() -> + * sendWireFormat() -> TcpTransport.oneway() -> TcpBufferedOutputStream.flush(), + * which calls SSLSocketImpl.startHandshake() implicitly on the first write. + * Since WriteTimeoutFilter.start() does not call registerWrite(), the + * TimeoutThread has nothing to monitor, and the connection blocks indefinitely. + */ +public class SoWriteTimeoutSslHandshakeTest { + + private static final Logger LOG = LoggerFactory.getLogger(SoWriteTimeoutSslHandshakeTest.class); + + private static final String KEYSTORE_TYPE = "jks"; + private static final String PASSWORD = "password"; + private static final String SERVER_KEYSTORE = "src/test/resources/org/apache/activemq/security/broker1.ks"; + private static final String TRUST_KEYSTORE = "src/test/resources/org/apache/activemq/security/broker1.ks"; + + /** A plain TCP ServerSocket that accepts connections but never responds (simulates unresponsive SSL peer) */ + private ServerSocket silentServer; + private ExecutorService executor; + private final AtomicBoolean serverRunning = new AtomicBoolean(true); + + @Before + public void setUp() throws Exception { + // Configure SSL system properties for the client side + System.setProperty("javax.net.ssl.trustStore", TRUST_KEYSTORE); + System.setProperty("javax.net.ssl.trustStorePassword", PASSWORD); + System.setProperty("javax.net.ssl.trustStoreType", KEYSTORE_TYPE); + System.setProperty("javax.net.ssl.keyStore", SERVER_KEYSTORE); + System.setProperty("javax.net.ssl.keyStoreType", KEYSTORE_TYPE); + System.setProperty("javax.net.ssl.keyStorePassword", PASSWORD); + + // Start a plain TCP server that accepts connections but never reads/writes + // This simulates a peer that is unreachable at the SSL layer + silentServer = new ServerSocket(0); + executor = Executors.newCachedThreadPool(); + executor.execute(() -> { + while (serverRunning.get()) { + try { + Socket accepted = silentServer.accept(); + LOG.info("Silent server accepted connection from: {}", accepted.getRemoteSocketAddress()); + // Intentionally do nothing - don't read, don't write, don't close + // This will cause the SSL handshake to block on the client side + } catch (IOException e) { + if (serverRunning.get()) { + LOG.debug("Silent server accept error: {}", e.getMessage()); + } + } + } + }); + LOG.info("Silent TCP server started on port: {}", silentServer.getLocalPort()); + } + + @After + public void tearDown() throws Exception { + serverRunning.set(false); + if (silentServer != null) { + silentServer.close(); + } + if (executor != null) { + executor.shutdownNow(); + executor.awaitTermination(5, TimeUnit.SECONDS); + } + } + + /** + * This test proves the bug: WriteTimeoutFilter.start() does NOT register + * the write timeout, so the SSL handshake blocks beyond the configured + * soWriteTimeout. + * + * Expected behavior (after fix): connection attempt should fail within + * roughly soWriteTimeout + TimeoutThread polling interval (~2s + ~5s = ~7s). + * + * Current behavior (bug): connection attempt blocks for much longer than + * soWriteTimeout because WriteTimeoutFilter.start() never calls registerWrite(). + * + * We use a generous upper bound of 15 seconds. If the write timeout worked + * during start(), the connection should fail within ~7-8 seconds (2s timeout + * + 5s polling interval + margin). If it takes more than 15 seconds, the + * timeout is NOT being enforced during start(). + */ + @Test + public void testSslHandshakeWriteTimeoutNotEnforcedDuringStart() throws Exception { + final int soWriteTimeout = 2000; // 2 second write timeout + // Upper bound: soWriteTimeout + TimeoutThread sleep (5s) + margin + final int expectedMaxSeconds = 15; + + // Use ssl:// with soWriteTimeout pointing to our silent TCP server. + // The failover transport ensures the connection attempt doesn't just throw + // immediately but actually tries to establish the SSL connection. + // maxReconnectAttempts=1 to avoid infinite reconnects. + String uri = "failover:(ssl://localhost:" + silentServer.getLocalPort() + + "?soWriteTimeout=" + soWriteTimeout + + "&socket.verifyHostName=false" + + ")?maxReconnectAttempts=2" + + "&startupMaxReconnectAttempts=1" + + "&initialReconnectDelay=500"; + + LOG.info("Connecting with URI: {}", uri); + + final CountDownLatch connectFinished = new CountDownLatch(1); + final AtomicReference connectException = new AtomicReference<>(); + + // Run connection attempt in a separate thread since it may block + executor.execute(() -> { + Connection connection = null; + try { + ActiveMQConnectionFactory factory = new ActiveMQConnectionFactory(uri); + connection = factory.createConnection(); + connection.start(); + LOG.info("Connection unexpectedly succeeded"); + } catch (JMSException e) { + LOG.info("Connection failed as expected: {}", e.getMessage()); + connectException.set(e); + } finally { + connectFinished.countDown(); + if (connection != null) { + try { + connection.close(); + } catch (JMSException ignored) { + } + } + } + }); + + // Wait for the connection attempt to complete or timeout + boolean finished = connectFinished.await(expectedMaxSeconds, TimeUnit.SECONDS); + + if (finished) { + // The connection attempt completed within the time limit. + // This means the timeout WAS enforced during start() (fix is working). + assertNotNull("Connection should have failed with an exception", connectException.get()); + LOG.info("PASS: SSL handshake was timed out correctly within {} seconds", expectedMaxSeconds); + } else { + // The connection attempt is still blocking after expectedMaxSeconds. + // This proves the bug: WriteTimeoutFilter.start() does NOT enforce + // the write timeout during SSL handshake. + LOG.warn("BUG CONFIRMED: SSL handshake blocked for more than {} seconds. " + + "WriteTimeoutFilter.start() does not register the write timeout. " + + "See AMQ-9569.", expectedMaxSeconds); + fail("AMQ-9569: WriteTimeoutFilter.start() did not enforce soWriteTimeout during SSL handshake. " + + "Connection blocked for more than " + expectedMaxSeconds + " seconds. " + + "Expected the write timeout (" + soWriteTimeout + "ms) to abort the blocked handshake."); + } + } + +} diff --git a/activemq-web-console/src/main/java/org/apache/activemq/web/filter/ApplicationContextFilter.java b/activemq-web-console/src/main/java/org/apache/activemq/web/filter/ApplicationContextFilter.java index ca6f27cb51c..5b0c1754289 100644 --- a/activemq-web-console/src/main/java/org/apache/activemq/web/filter/ApplicationContextFilter.java +++ b/activemq-web-console/src/main/java/org/apache/activemq/web/filter/ApplicationContextFilter.java @@ -66,6 +66,7 @@ public class ApplicationContextFilter implements Filter { private String applicationContextName = "applicationContext"; private String requestContextName = "requestContext"; private String requestName = "request"; + private String slavePage = "slave.jsp"; public void init(FilterConfig config) throws ServletException { this.servletContext = config.getServletContext(); @@ -84,19 +85,22 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha Map requestContextWrapper = createRequestContextWrapper(request); String path = ((HttpServletRequest)request).getRequestURI(); // handle slave brokers -// try { -// if ( !(path.endsWith("css") || path.endsWith("png") || path.endsWith("ico") || path.endsWith(slavePage)) -// && ((BrokerFacade)requestContextWrapper.get("brokerQuery")).isSlave()) { -// ((HttpServletResponse)response).sendRedirect(slavePage); -// return; -// } -// } catch (Exception e) { -// LOG.warn(path + ", failed to access BrokerFacade: reason: " + e.getLocalizedMessage()); -// if (LOG.isDebugEnabled()) { -// LOG.debug(request.toString(), e); -// } -// throw new IOException(e); -// } + try { + boolean isSlave = ((BrokerFacade) requestContextWrapper.get("brokerQuery")).getBrokerAdmin().isSlave(); + if (isSlave && !(path.endsWith("css") || path.endsWith("png") || path.endsWith("ico") || path.endsWith(slavePage))) { + ((HttpServletResponse) response).sendRedirect(slavePage); + return; + } else if (!isSlave && path.endsWith(slavePage)) { + ((HttpServletResponse) response).sendRedirect(((HttpServletRequest) request).getContextPath() + "/index.jsp"); + return; + } + } catch (Exception e) { + LOG.warn(path + ", failed to access BrokerFacade: reason: " + e.getLocalizedMessage()); + if (LOG.isDebugEnabled()) { + LOG.debug(request.toString(), e); + } + throw new IOException(e); + } request.setAttribute(requestContextName, requestContextWrapper); request.setAttribute(requestName, request); chain.doFilter(request, response); diff --git a/activemq-web/pom.xml b/activemq-web/pom.xml index 2b9ff1db546..dd7e88b7019 100644 --- a/activemq-web/pom.xml +++ b/activemq-web/pom.xml @@ -96,6 +96,13 @@ ${jetty-version} + + + org.apache.commons + commons-text + test + + com.rometools diff --git a/activemq-web/src/main/java/org/apache/activemq/web/QueueBrowseServlet.java b/activemq-web/src/main/java/org/apache/activemq/web/QueueBrowseServlet.java index 2b5cb2523e0..69c4b40370f 100644 --- a/activemq-web/src/main/java/org/apache/activemq/web/QueueBrowseServlet.java +++ b/activemq-web/src/main/java/org/apache/activemq/web/QueueBrowseServlet.java @@ -67,34 +67,49 @@ public QueueBrowseServlet() { // ------------------------------------------------------------------------- protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { try { - WebClient client = WebClient.getWebClient(request); - Session session = client.getSession(); - Queue queue = getQueue(request, session); - if (queue == null) { - throw new ServletException("No queue URI specified"); - } - - String msgId = request.getParameter("msgId"); - if (msgId == null) { - MessageRenderer renderer = getMessageRenderer(request); - configureRenderer(request, renderer); - - String selector = getSelector(request); - QueueBrowser browser = session.createBrowser(queue, selector); - renderer.renderMessages(request, response, browser); - } - else { - XmlMessageRenderer renderer = new XmlMessageRenderer(); - QueueBrowser browser = session.createBrowser(queue, "JMSMessageID='" + msgId + "'"); - if (!browser.getEnumeration().hasMoreElements()) { - response.sendError(HttpServletResponse.SC_NOT_FOUND); - return; + final WebClient client = WebClient.getWebClient(request); + try { + final Session session = client.getSession(); + try { + final Queue queue = getQueue(request, session); + if (queue == null) { + throw new ServletException("No queue URI specified"); + } + + final String msgId = request.getParameter("msgId"); + if (msgId == null) { + final MessageRenderer renderer = getMessageRenderer(request); + configureRenderer(request, renderer); + + final String selector = getSelector(request); + final QueueBrowser browser = session.createBrowser(queue, selector); + try { + renderer.renderMessages(request, response, browser); + } finally { + browser.close(); + } + } else { + final XmlMessageRenderer renderer = new XmlMessageRenderer(); + final QueueBrowser browser = session.createBrowser(queue, "JMSMessageID='" + msgId + "'"); + try { + if (!browser.getEnumeration().hasMoreElements()) { + response.sendError(HttpServletResponse.SC_NOT_FOUND); + return; + } + final Message message = (Message) browser.getEnumeration().nextElement(); + + final PrintWriter writer = response.getWriter(); + renderer.renderMessage(writer, request, response, browser, message); + writer.flush(); + } finally { + browser.close(); + } + } + } finally { + session.close(); } - Message message = (Message) browser.getEnumeration().nextElement(); - - PrintWriter writer = response.getWriter(); - renderer.renderMessage(writer, request, response, browser, message); - writer.flush(); + } finally { + client.close(); } } catch (JMSException e) { diff --git a/activemq-web/src/main/java/org/apache/activemq/web/WebClient.java b/activemq-web/src/main/java/org/apache/activemq/web/WebClient.java index c04df936c84..7184789c2a1 100644 --- a/activemq-web/src/main/java/org/apache/activemq/web/WebClient.java +++ b/activemq-web/src/main/java/org/apache/activemq/web/WebClient.java @@ -60,7 +60,7 @@ * * */ -public class WebClient implements HttpSessionActivationListener, HttpSessionBindingListener, Externalizable { +public class WebClient implements HttpSessionActivationListener, HttpSessionBindingListener, Externalizable, AutoCloseable { public static final String WEB_CLIENT_ATTRIBUTE = "org.apache.activemq.webclient"; public static final String CONNECTION_FACTORY_ATTRIBUTE = "org.apache.activemq.connectionFactory"; @@ -278,7 +278,7 @@ protected static synchronized void initConnectionFactory(ServletContext servletC if (broker == null) { throw new IllegalStateException("missing brokerURL (specified via " + BROKER_URL_INIT_PARAM + " init-Param) or embedded broker"); } else { - brokerURL = "vm://" + broker.getBrokerName(); + brokerURL = broker.getVmConnectorURI().toString(); } } diff --git a/activemq-web/src/main/java/org/apache/activemq/web/util/ViewUtils.java b/activemq-web/src/main/java/org/apache/activemq/web/util/ViewUtils.java new file mode 100644 index 00000000000..c091809ca1b --- /dev/null +++ b/activemq-web/src/main/java/org/apache/activemq/web/util/ViewUtils.java @@ -0,0 +1,66 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.activemq.web.util; + +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Map.Entry; + +public class ViewUtils { + + public static final String AMP = "&"; + public static final String QUOTE = "\""; + public static final String LT = "<"; + public static final String GT = ">"; + public static final String APOS = "'"; + + public static final String XML_ESCAPED_AMP = "&"; + public static final String XML_ESCAPED_QUOTE = """; + public static final String XML_ESCAPED_LT = "<"; + public static final String XML_ESCAPED_GT = ">"; + public static final String XML_ESCAPED_APOS = "'"; + + public static final Map XML_ESCAPE_MAPPINGS; + + static { + // order matters for processing so use a linked map + Map mappings = new LinkedHashMap<>(); + mappings.put(AMP, XML_ESCAPED_AMP); + mappings.put(LT, XML_ESCAPED_LT); + mappings.put(GT, XML_ESCAPED_GT); + mappings.put(QUOTE, XML_ESCAPED_QUOTE); + mappings.put(APOS, XML_ESCAPED_APOS); + + XML_ESCAPE_MAPPINGS = Collections.unmodifiableMap(mappings); + } + + + public static String escapeXml(String input) { + if (input == null) { + return null; + } + + String escaped = input; + for (Entry entry : XML_ESCAPE_MAPPINGS.entrySet()) { + escaped = escaped.replace(entry.getKey(), entry.getValue()); + } + + return escaped; + } + +} diff --git a/activemq-web/src/main/java/org/apache/activemq/web/view/RssMessageRenderer.java b/activemq-web/src/main/java/org/apache/activemq/web/view/RssMessageRenderer.java index 21b7a2e9f9f..b1afb2a8c4c 100644 --- a/activemq-web/src/main/java/org/apache/activemq/web/view/RssMessageRenderer.java +++ b/activemq-web/src/main/java/org/apache/activemq/web/view/RssMessageRenderer.java @@ -49,7 +49,7 @@ public class RssMessageRenderer extends SimpleMessageRenderer { private String feedType = "rss_2.0"; private SyndFeed feed; private String description = "This feed is auto-generated by Apache ActiveMQ"; - private String entryContentType = "text/plain"; + private static final String ENTRY_CONTENT_TYPE = "text/plain"; public void renderMessage(PrintWriter writer, HttpServletRequest request, HttpServletResponse response, QueueBrowser browser, Message message) throws JMSException { SyndFeed feed = getFeed(browser, request); @@ -79,11 +79,7 @@ public void setFeedType(String feedType) { } public String getEntryContentType() { - return entryContentType; - } - - public void setEntryContentType(String entryContentType) { - this.entryContentType = entryContentType; + return ENTRY_CONTENT_TYPE; } // Implementation methods @@ -122,7 +118,7 @@ protected SyndEntry createEntry(QueueBrowser browser, Message message, HttpServl protected SyndContent createEntryContent(QueueBrowser browser, Message message, HttpServletRequest request) throws JMSException { SyndContent description = new SyndContentImpl(); - description.setType(entryContentType); + description.setType(getEntryContentType()); if (message instanceof TextMessage) { String text = ((TextMessage)message).getText(); diff --git a/activemq-web/src/main/java/org/apache/activemq/web/view/SimpleMessageRenderer.java b/activemq-web/src/main/java/org/apache/activemq/web/view/SimpleMessageRenderer.java index 35983aedc42..ad1d45c1526 100644 --- a/activemq-web/src/main/java/org/apache/activemq/web/view/SimpleMessageRenderer.java +++ b/activemq-web/src/main/java/org/apache/activemq/web/view/SimpleMessageRenderer.java @@ -26,6 +26,7 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.activemq.web.util.ViewUtils; /** * A simple rendering of the contents of a queue appear as a list of message @@ -35,11 +36,12 @@ */ public class SimpleMessageRenderer implements MessageRenderer { - private String contentType = "text/xml"; + protected static final String DEFAULT_CONTENT_TYPE = "text/xml"; + private int maxMessages; public void renderMessages(HttpServletRequest request, HttpServletResponse response, QueueBrowser browser) throws IOException, JMSException, ServletException { - // lets use XML by default + // XML is used by default unless a child class overrides this method response.setContentType(getContentType()); PrintWriter writer = response.getWriter(); printHeader(writer, browser, request); @@ -53,10 +55,10 @@ public void renderMessages(HttpServletRequest request, HttpServletResponse respo printFooter(writer, browser, request); } - public void renderMessage(PrintWriter writer, HttpServletRequest request, HttpServletResponse response, QueueBrowser browser, Message message) throws JMSException, ServletException { + public void renderMessage(PrintWriter writer, HttpServletRequest request, HttpServletResponse response, QueueBrowser browser, Message message) throws JMSException { // lets just write the message IDs for now writer.print(""); } @@ -71,25 +73,21 @@ public void setMaxMessages(int maxMessages) { } public String getContentType() { - return contentType; - } - - public void setContentType(String contentType) { - this.contentType = contentType; + return DEFAULT_CONTENT_TYPE; } // Implementation methods // ------------------------------------------------------------------------- - protected void printHeader(PrintWriter writer, QueueBrowser browser, HttpServletRequest request) throws IOException, JMSException, ServletException { + protected void printHeader(PrintWriter writer, QueueBrowser browser, HttpServletRequest request) throws IOException, JMSException { writer.println(""); writer.print(""); diff --git a/activemq-web/src/test/java/org/apache/activemq/web/util/ViewUtilsTest.java b/activemq-web/src/test/java/org/apache/activemq/web/util/ViewUtilsTest.java new file mode 100644 index 00000000000..17b996f6f48 --- /dev/null +++ b/activemq-web/src/test/java/org/apache/activemq/web/util/ViewUtilsTest.java @@ -0,0 +1,40 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.activemq.web.util; + +import static org.junit.Assert.assertEquals; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Paths; +import org.apache.commons.text.StringEscapeUtils; +import org.junit.Test; + +public class ViewUtilsTest { + + @Test + public void testXmlEscape() throws IOException { + final String original = new String(Files.readAllBytes(Paths.get("src/test/resources/activemq.xml")), StandardCharsets.UTF_8); + final String escaped = ViewUtils.escapeXml(original); + + // Verify that our escape method matches StringEscapeUtils + assertEquals(StringEscapeUtils.escapeXml11(original), ViewUtils.escapeXml(original)); + // Verify if we unescape we get back the original + assertEquals(original, StringEscapeUtils.unescapeXml(escaped)); + } +} diff --git a/activemq-web/src/test/resources/activemq.xml b/activemq-web/src/test/resources/activemq.xml new file mode 100644 index 00000000000..9affdddf3e8 --- /dev/null +++ b/activemq-web/src/test/resources/activemq.xml @@ -0,0 +1,37 @@ + + + + + + + + + + + + + + + + + diff --git a/assembly/src/release/conf/activemq.xml b/assembly/src/release/conf/activemq.xml index 6c56f38bdb4..a00ff0357e2 100644 --- a/assembly/src/release/conf/activemq.xml +++ b/assembly/src/release/conf/activemq.xml @@ -37,7 +37,7 @@ - + diff --git a/assembly/src/release/conf/jetty.xml b/assembly/src/release/conf/jetty.xml index 8af3574f3f3..f94eeda19f8 100644 --- a/assembly/src/release/conf/jetty.xml +++ b/assembly/src/release/conf/jetty.xml @@ -175,10 +175,10 @@ - + - + diff --git a/assembly/src/release/examples/conf/activemq-demo.xml b/assembly/src/release/examples/conf/activemq-demo.xml index 13524dbb643..12fbc2ec2b4 100644 --- a/assembly/src/release/examples/conf/activemq-demo.xml +++ b/assembly/src/release/examples/conf/activemq-demo.xml @@ -47,7 +47,7 @@ - Change the brokerName attribute to something unique --> - + - + - + diff --git a/assembly/src/release/examples/conf/activemq-stomp.xml b/assembly/src/release/examples/conf/activemq-stomp.xml index 0fe5b11c2b7..d2f6fee4bf9 100644 --- a/assembly/src/release/examples/conf/activemq-stomp.xml +++ b/assembly/src/release/examples/conf/activemq-stomp.xml @@ -44,7 +44,7 @@ - + com.rometools @@ -1135,8 +1142,8 @@ true false - clean install - deploy + clean install -DskipTests + deploy -DskipTests false false activemq-@{project.version}