diff --git a/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConfigFactory.java b/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConfigFactory.java index c3c9266513f..5a95b40d406 100644 --- a/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConfigFactory.java +++ b/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConfigFactory.java @@ -190,6 +190,7 @@ private static TransactionManager getTransactionManagerFromJndi(String transacti if (transactionManagerJndiName == null) { return null; } + JndiHelper.validateJndiName(transactionManagerJndiName); try { InitialContext ictx = new InitialContext(); return (TransactionManager)ictx.lookup(transactionManagerJndiName); diff --git a/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/util/JndiHelper.java b/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/util/JndiHelper.java index 0f86ead1474..63721cae82a 100644 --- a/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/util/JndiHelper.java +++ b/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/util/JndiHelper.java @@ -23,6 +23,7 @@ import java.util.Collections; import java.util.List; import java.util.Properties; +import java.util.Set; import java.util.function.Predicate; import javax.naming.Context; @@ -44,6 +45,32 @@ public class JndiHelper { */ private static final String DEFAULT_JMS_PROTOCOLS = "vm,tcp,nio,ssl,http,https,ws,wss"; private static final List ALLOWED_PROTOCOLS; + + /** + * JNDI environment properties that must not be supplied from untrusted input. + *

+ * Note: {@code java.naming.factory.initial} (INITIAL_CONTEXT_FACTORY) is intentionally + * absent. It is a first-class, documented configuration parameter — every JNDI-based JMS + * deployment sets it to the broker's context factory (e.g. ActiveMQ, Artemis, WebLogic). + * It is safe to allow because any attempt to redirect lookups to a remote host via a + * substitute factory still requires setting PROVIDER_URL to a non-JMS scheme, which the + * protocol allowlist check below independently rejects. + *

+ * + */ + private static final Set BLOCKED_ENV_KEYS = Set.of( + "java.naming.factory.object", + "java.naming.factory.state", + "java.naming.factory.url.pkgs" + ); + private Properties environment; static { @@ -68,6 +95,13 @@ public class JndiHelper { public JndiHelper(Properties environment) { this.environment = environment; + // Reject properties that could be used to redirect or hijack the JNDI lookup + for (String blocked : BLOCKED_ENV_KEYS) { + if (environment.containsKey(blocked)) { + throw new IllegalArgumentException("Disallowed JNDI environment property: " + blocked); + } + } + // Avoid unsafe protocols if they are somehow misconfigured String providerUrl = environment.getProperty(Context.PROVIDER_URL); if (providerUrl != null && !providerUrl.isEmpty() @@ -76,8 +110,19 @@ public JndiHelper(Properties environment) { } } + /** + * Rejects lookup names that contain a URL scheme (e.g. ldap://, rmi://), which would + * redirect the lookup to a remote server and enable JNDI injection. + */ + public static void validateJndiName(String name) { + if (name != null && name.contains("://")) { + throw new IllegalArgumentException("JNDI name must not contain a URL: " + name); + } + } + @SuppressWarnings("unchecked") public T lookup(final String name, Class requiredType) throws NamingException { + validateJndiName(name); Context ctx = createInitialContext(); try { Object located = ctx.lookup(name); diff --git a/rt/transports/jms/src/test/java/org/apache/cxf/transport/jms/JMSConfigFactoryTest.java b/rt/transports/jms/src/test/java/org/apache/cxf/transport/jms/JMSConfigFactoryTest.java index 8ac1f79d263..0c4af03be86 100644 --- a/rt/transports/jms/src/test/java/org/apache/cxf/transport/jms/JMSConfigFactoryTest.java +++ b/rt/transports/jms/src/test/java/org/apache/cxf/transport/jms/JMSConfigFactoryTest.java @@ -45,11 +45,11 @@ public void testJndiForbiddenProtocol() throws Exception { env.put(Context.PROVIDER_URL, "ldap://127.0.0.1:12345"); // Allow following referrals (important for LDAP injection) env.put(Context.REFERRAL, "follow"); - + JMSConfiguration jmsConfig = new JMSConfiguration(); jmsConfig.setJndiEnvironment(env); jmsConfig.setConnectionFactoryName("objectName"); - + try { jmsConfig.getConnectionFactory(); Assert.fail("JNDI lookup should have failed"); @@ -103,6 +103,25 @@ public void testTransactionManagerFromJndi() throws XAException, NamingException // TODO Check JNDI lookup } + @Test + public void testTransactionManagerJndiInjectionRejected() { + // Ensure URL-style JNDI names (e.g. ldap://, rmi://) are rejected to prevent + // JNDI injection via getTransactionManagerFromJndi. + for (String malicious : new String[]{"ldap://attacker.com/exploit", + "rmi://attacker.com/exploit", + "corba://attacker.com/exploit"}) { + Bus testBus = BusFactory.newInstance().createBus(); + JMSEndpoint endpoint = new JMSEndpoint("jms:queue:Foo.Bar?jndiTransactionManagerName=" + + malicious); + try { + JMSConfigFactory.createFromEndpoint(testBus, endpoint); + Assert.fail("Expected IllegalArgumentException for JNDI name: " + malicious); + } catch (IllegalArgumentException e) { + Assert.assertTrue(e.getMessage().contains("JNDI name must not contain a URL")); + } + } + } + @Test public void testConcurrentConsumers() { JMSEndpoint endpoint = new JMSEndpoint("jms:queue:Foo.Bar?concurrentConsumers=4"); diff --git a/rt/transports/jms/src/test/java/org/apache/cxf/transport/jms/util/JndiHelperTest.java b/rt/transports/jms/src/test/java/org/apache/cxf/transport/jms/util/JndiHelperTest.java new file mode 100644 index 00000000000..22996d8ac84 --- /dev/null +++ b/rt/transports/jms/src/test/java/org/apache/cxf/transport/jms/util/JndiHelperTest.java @@ -0,0 +1,138 @@ +/** + * 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.cxf.transport.jms.util; + +import java.util.Properties; + +import javax.naming.Context; + +import org.junit.Assert; +import org.junit.Test; + +public class JndiHelperTest { + + // --- validateJndiName --- + + @Test + public void testValidateJndiNamePlainNamesAllowed() { + // Ordinary local JNDI names must not be rejected + JndiHelper.validateJndiName("TransactionManager"); + JndiHelper.validateJndiName("java:comp/env/jms/MyQueue"); + JndiHelper.validateJndiName("java:/TransactionManager"); + JndiHelper.validateJndiName(null); + } + + @Test + public void testValidateJndiNameRemoteUrlRejected() { + for (String malicious : new String[]{ + "ldap://attacker.com/exploit", + "ldaps://attacker.com/exploit", + "rmi://attacker.com/exploit", + "iiop://attacker.com/exploit", + "corba://attacker.com/exploit", + "dns://attacker.com/exploit"}) { + try { + JndiHelper.validateJndiName(malicious); + Assert.fail("Expected IllegalArgumentException for: " + malicious); + } catch (IllegalArgumentException e) { + Assert.assertTrue(e.getMessage().contains("JNDI name must not contain a URL")); + } + } + } + + // --- constructor: blocked environment keys --- + + @Test + public void testInitialContextFactoryAllowed() { + // INITIAL_CONTEXT_FACTORY is a first-class config parameter set by every JNDI-based + // JMS deployment (e.g. ActiveMQ, Artemis) and must not be blocked. + Properties env = new Properties(); + env.put(Context.PROVIDER_URL, "vm://localhost"); + env.put(Context.INITIAL_CONTEXT_FACTORY, + "org.apache.activemq.artemis.jndi.ActiveMQInitialContextFactory"); + new JndiHelper(env); // must not throw + } + + @Test + public void testBlockedObjectFactoryRejected() { + Properties env = new Properties(); + env.put("java.naming.factory.object", "com.attacker.EvilObjectFactory"); + try { + new JndiHelper(env); + Assert.fail("Expected IllegalArgumentException for java.naming.factory.object"); + } catch (IllegalArgumentException e) { + Assert.assertTrue(e.getMessage().contains("Disallowed JNDI environment property")); + Assert.assertTrue(e.getMessage().contains("java.naming.factory.object")); + } + } + + @Test + public void testBlockedStateFactoryRejected() { + Properties env = new Properties(); + env.put("java.naming.factory.state", "com.attacker.EvilStateFactory"); + try { + new JndiHelper(env); + Assert.fail("Expected IllegalArgumentException for java.naming.factory.state"); + } catch (IllegalArgumentException e) { + Assert.assertTrue(e.getMessage().contains("Disallowed JNDI environment property")); + Assert.assertTrue(e.getMessage().contains("java.naming.factory.state")); + } + } + + @Test + public void testBlockedUrlPkgsRejected() { + Properties env = new Properties(); + env.put("java.naming.factory.url.pkgs", "com.attacker"); + try { + new JndiHelper(env); + Assert.fail("Expected IllegalArgumentException for java.naming.factory.url.pkgs"); + } catch (IllegalArgumentException e) { + Assert.assertTrue(e.getMessage().contains("Disallowed JNDI environment property")); + Assert.assertTrue(e.getMessage().contains("java.naming.factory.url.pkgs")); + } + } + + @Test + public void testUnsafeProviderUrlRejected() { + Properties env = new Properties(); + env.put(Context.PROVIDER_URL, "ldap://attacker.com:389"); + try { + new JndiHelper(env); + Assert.fail("Expected IllegalArgumentException for unsafe PROVIDER_URL"); + } catch (IllegalArgumentException e) { + Assert.assertTrue(e.getMessage().contains("Unsafe protocol in JNDI URL")); + } + } + + @Test + public void testSafeProviderUrlAllowed() { + // Allowed JMS broker protocols must not be rejected + for (String safe : new String[]{"vm://localhost", "tcp://localhost:61616", + "ssl://localhost:61617", "nio://localhost:61618"}) { + Properties env = new Properties(); + env.put(Context.PROVIDER_URL, safe); + new JndiHelper(env); // must not throw + } + } + + @Test + public void testEmptyPropertiesAllowed() { + new JndiHelper(new Properties()); // must not throw + } +}