This is an automated email from the ASF dual-hosted git repository.
cshannon pushed a commit to branch activemq-5.19.x
in repository https://gitbox.apache.org/repos/asf/activemq.git
The following commit(s) were added to refs/heads/activemq-5.19.x by this push:
new 0927bcb99e Add validation for LDAP network connector URIs (#2077)
(#2099)
0927bcb99e is described below
commit 0927bcb99e18c517728898be9f125589653ee899
Author: Christopher L. Shannon <[email protected]>
AuthorDate: Mon Jun 8 17:48:34 2026 -0400
Add validation for LDAP network connector URIs (#2077) (#2099)
Validate the URIs provided by LDAP searches do not contain any protocols
from the denied list.
(cherry picked from commit ac3d06454bdf8b21db7ef419f42d17ab124a6008)
---
.../org/apache/activemq/broker/jmx/BrokerView.java | 62 +-------------
.../activemq/network/LdapNetworkConnector.java | 5 ++
.../activemq/util/TransportValidationUtils.java | 99 ++++++++++++++++++++++
.../activemq/network/LdapNetworkConnectorTest.java | 72 ++++++++++++++++
.../org/apache/activemq/broker/jmx/MBeanTest.java | 3 +-
.../org/apache/activemq/jmx/JmxCreateNCTest.java | 2 +-
6 files changed, 181 insertions(+), 62 deletions(-)
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 e4151eaeca..4972f26326 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
@@ -16,10 +16,11 @@
*/
package org.apache.activemq.broker.jmx;
+import static
org.apache.activemq.util.TransportValidationUtils.validateAllowedUrl;
+
import java.io.File;
import java.io.IOException;
import java.net.URI;
-import java.net.URISyntaxException;
import java.util.*;
import java.util.concurrent.atomic.AtomicInteger;
@@ -36,7 +37,6 @@ import org.apache.activemq.broker.region.Subscription;
import org.apache.activemq.command.*;
import org.apache.activemq.network.NetworkConnector;
import org.apache.activemq.util.BrokerSupport;
-import org.apache.activemq.util.URISupport;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -44,10 +44,6 @@ public class BrokerView implements BrokerViewMBean {
private static final Logger LOG =
LoggerFactory.getLogger(BrokerView.class);
- public static final Set<String> DENIED_TRANSPORT_SCHEMES = Set.of("vm",
"http", "https",
- "multicast", "zeroconf", "discovery", "fanout", "mock", "peer",
"failover",
- "proxy", "reliable", "simple", "udp", "masterslave");
-
ManagedRegionBroker broker;
private final BrokerService brokerService;
@@ -567,58 +563,4 @@ public class BrokerView implements BrokerViewMBean {
return
safeGetBroker().getDestinationStatistics().getMaxUncommittedExceededCount().getCount();
}
- private static void validateAllowedUrl(String uriString) throws
URISyntaxException {
- validateAllowedUri(new URI(uriString), 0);
- }
-
- // 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) {
- throw new IllegalArgumentException("URI can't contain more than 5
nested composite URIs");
- }
-
- // First check the main URI scheme
- validateAllowedScheme(uri.getScheme());
-
- // We need to check if the URI is composite and/or contains nested URIs
- // The utility method URISupport#isCompositeURI is not good enough here
- // because it misses if there are no parentheses and also is primarily
meant
- // for checking comma separated URIs and not nested URIs.
- //
- // The best way to handle all cases is to use the same logic that the
transports
- // use to process the URIs and that is to simply attempt to parse it
and check each
- // of the parsed components. This wll correctly handle the case when
there
- // are parentheses and also when the parentheses are skipped.
- final URISupport.CompositeData data;
- try {
- data = URISupport.parseComposite(uri);
- } catch (URISyntaxException e) {
- // If this is not a valid URI then we can stop checking
- // This can happen when parsing a nested URI and at the last
portion
- return;
- }
-
- if (data.getComponents() != null) {
- depth++;
- for (URI component : data.getComponents()) {
- // Each URI could be a nested and/or composite URI so call
validateAllowedUri()
- // to validate it. If the scheme is null then the original URI
is not composite
- // or nested so we can skip the check, and we are finished.
- if (component.getScheme() != null) {
- validateAllowedUri(component, depth);
- }
- }
- }
- }
-
- // Check all denied schemes
- private static void validateAllowedScheme(String scheme) {
- 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/network/LdapNetworkConnector.java
b/activemq-broker/src/main/java/org/apache/activemq/network/LdapNetworkConnector.java
index 5efd82b750..293091f293 100644
---
a/activemq-broker/src/main/java/org/apache/activemq/network/LdapNetworkConnector.java
+++
b/activemq-broker/src/main/java/org/apache/activemq/network/LdapNetworkConnector.java
@@ -35,6 +35,7 @@ import javax.naming.event.NamingEvent;
import javax.naming.event.NamingExceptionEvent;
import javax.naming.event.ObjectChangeListener;
+import org.apache.activemq.util.TransportValidationUtils;
import org.apache.activemq.util.URISupport;
import org.apache.activemq.util.URISupport.CompositeData;
import org.slf4j.Logger;
@@ -307,6 +308,10 @@ public class LdapNetworkConnector extends NetworkConnector
implements NamespaceC
// want to map/manage these differently in the future
// boolean useJMX = getBrokerService().isUseJmx();
// getBrokerService().setUseJmx(false);
+
+ // Validate LDAP provided URI
+ TransportValidationUtils.validateAllowedUri(connectorURI);
+
NetworkConnector connector =
getBrokerService().addNetworkConnector(connectorURI);
// getBrokerService().setUseJmx(useJMX);
diff --git
a/activemq-broker/src/main/java/org/apache/activemq/util/TransportValidationUtils.java
b/activemq-broker/src/main/java/org/apache/activemq/util/TransportValidationUtils.java
new file mode 100644
index 0000000000..b0fed2e7cb
--- /dev/null
+++
b/activemq-broker/src/main/java/org/apache/activemq/util/TransportValidationUtils.java
@@ -0,0 +1,99 @@
+/*
+ * 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.util;
+
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.Set;
+
+public class TransportValidationUtils {
+
+ public static final Set<String> DENIED_TRANSPORT_SCHEMES = Set.of("vm",
"http", "https",
+ "multicast", "zeroconf", "discovery", "fanout", "mock", "peer",
"failover",
+ "proxy", "reliable", "simple", "udp", "masterslave");
+
+ /**
+ * Used to validate externally provided url is not using a denied scheme
+ *
+ * @param uriString
+ * @throws URISyntaxException
+ */
+ public static void validateAllowedUrl(String uriString) throws
URISyntaxException {
+ validateAllowedUri(new URI(uriString));
+ }
+
+ /**
+ * Used to validate externally provided URI is not using a denied scheme
+ *
+ * @param uri
+ * @throws URISyntaxException
+ */
+ public static void validateAllowedUri(URI uri) throws URISyntaxException {
+ validateAllowedUri(uri, 0);
+ }
+
+ // 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) {
+ throw new IllegalArgumentException("URI can't contain more than 5
nested composite URIs");
+ }
+
+ // First check the main URI scheme
+ validateAllowedScheme(uri.getScheme());
+
+ // We need to check if the URI is composite and/or contains nested URIs
+ // The utility method URISupport#isCompositeURI is not good enough here
+ // because it misses if there are no parentheses and also is primarily
meant
+ // for checking comma separated URIs and not nested URIs.
+ //
+ // The best way to handle all cases is to use the same logic that the
transports
+ // use to process the URIs and that is to simply attempt to parse it
and check each
+ // of the parsed components. This wll correctly handle the case when
there
+ // are parentheses and also when the parentheses are skipped.
+ final URISupport.CompositeData data;
+ try {
+ data = URISupport.parseComposite(uri);
+ } catch (URISyntaxException e) {
+ // If this is not a valid URI then we can stop checking
+ // This can happen when parsing a nested URI and at the last
portion
+ return;
+ }
+
+ if (data.getComponents() != null) {
+ depth++;
+ for (URI component : data.getComponents()) {
+ // Each URI could be a nested and/or composite URI so call
validateAllowedUri()
+ // to validate it. If the scheme is null then the original URI
is not composite
+ // or nested so we can skip the check, and we are finished.
+ if (component.getScheme() != null) {
+ validateAllowedUri(component, depth);
+ }
+ }
+ }
+ }
+
+ // Check all denied schemes
+ private static void validateAllowedScheme(String scheme) {
+ 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/test/java/org/apache/activemq/network/LdapNetworkConnectorTest.java
b/activemq-broker/src/test/java/org/apache/activemq/network/LdapNetworkConnectorTest.java
new file mode 100644
index 0000000000..660860d493
--- /dev/null
+++
b/activemq-broker/src/test/java/org/apache/activemq/network/LdapNetworkConnectorTest.java
@@ -0,0 +1,72 @@
+/*
+ * 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.network;
+
+
+import static
org.apache.activemq.util.TransportValidationUtils.DENIED_TRANSPORT_SCHEMES;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+import javax.naming.directory.BasicAttributes;
+import javax.naming.directory.SearchResult;
+import org.apache.activemq.broker.BrokerService;
+import org.junit.Test;
+
+public class LdapNetworkConnectorTest {
+
+ @Test
+ public void testValidation() throws Exception {
+ LdapNetworkConnector connector = new LdapNetworkConnector();
+ connector.setBrokerService(new BrokerService());
+
+ for (String deniedScheme : DENIED_TRANSPORT_SCHEMES) {
+ try {
+ connector.addConnector(newSearchResult(deniedScheme,
"localhost"));
+ fail("Should have failed trying to add connector with scheme:
" + deniedScheme);
+ } catch (IllegalArgumentException e) {
+ assertEquals("Transport scheme '" + deniedScheme + "' is not
allowed", e.getMessage());
+ }
+ }
+ }
+
+ @Test
+ public void testCompositeValidation() throws Exception {
+ LdapNetworkConnector connector = new LdapNetworkConnector();
+ connector.setBrokerService(new BrokerService());
+
+ for (String deniedScheme : DENIED_TRANSPORT_SCHEMES) {
+ try {
+ connector.addConnector(newSearchResult("tcp",
"static:(tcp://localhost," +
+ deniedScheme + "://localhost)"));
+ fail("Should have failed trying to add connector with scheme:
" + deniedScheme);
+ } catch (IllegalArgumentException e) {
+ assertEquals("Transport scheme '" + deniedScheme + "' is not
allowed", e.getMessage());
+ }
+ }
+ }
+
+ private SearchResult newSearchResult(String protocol, String host) {
+ BasicAttributes attrs = new BasicAttributes();
+ attrs.put("ipserviceprotocol", protocol);
+ attrs.put("iphostnumber", host);
+ attrs.put("ipserviceport", "0");
+ SearchResult result = new SearchResult("name", null, attrs);
+ result.setNameInNamespace("fullname");
+ return result;
+ }
+
+}
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 b5f31eede6..0cb9a55683 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,7 +16,8 @@
*/
package org.apache.activemq.broker.jmx;
-import static
org.apache.activemq.broker.jmx.BrokerView.DENIED_TRANSPORT_SCHEMES;
+
+import static
org.apache.activemq.util.TransportValidationUtils.DENIED_TRANSPORT_SCHEMES;
import java.io.BufferedReader;
import java.io.InputStreamReader;
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 b20518e884..e3ef738c98 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
@@ -27,7 +27,7 @@ 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.apache.activemq.util.TransportValidationUtils.DENIED_TRANSPORT_SCHEMES;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.fail;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
