Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<String> ALLOWED_PROTOCOLS;

/**
* JNDI environment properties that must not be supplied from untrusted input.
* <p>
* 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.
* </p>
* <ul>
* <li>{@code java.naming.factory.object} — object factories reconstruct objects returned
* by a lookup and can deserialize remote payloads.</li>
* <li>{@code java.naming.factory.state} — state factories run during bind/rebind and
* can trigger arbitrary serialization logic.</li>
* <li>{@code java.naming.factory.url.pkgs} — injects packages for URL context factory
* resolution and could register a handler for an otherwise-allowed scheme.</li>
* </ul>
*/
private static final Set<String> BLOCKED_ENV_KEYS = Set.of(
"java.naming.factory.object",
"java.naming.factory.state",
"java.naming.factory.url.pkgs"
);

private Properties environment;

static {
Expand All @@ -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()
Expand All @@ -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> T lookup(final String name, Class<T> requiredType) throws NamingException {
validateJndiName(name);
Context ctx = createInitialContext();
try {
Object located = ctx.lookup(name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
}