Hi! I decided to send an email to the security-dev email list as ldap is involved. Please redirect me to other list if it is not the proper audience.
The last last days I have faced the same issue that is commented in JDK-8176553 [1]. Although it is cataloged as fixed in 12, the issue is not solved in the openjdk master branch yet. You can test with this simple project [2]. The project is using apache-ds and creating 12 branches with redirects from one to the other. The search should be limited to 5 hops but you will see that all of them are followed (12). Therefore, If there are loops, the search hangs indefinitely. So JDK-8176553 is not fixed completely. You just need `mvn clean test` to reproduce the problem in that project. I have investigated and I think the attached little patch fixes the issue. Mainly the `LdapReferralException` is not stopping the referral loop in some situations. I have added a test in the jndi jtreg test-suite to check everything works OK; `make test TEST=jtreg:jdk/com/sun/jndi/ldap/ReferralLimitSearchTest.java` WDYT? Is the PR worthy? Thanks in advance! [1] https://bugs.openjdk.org/browse/JDK-8176553 [2] https://urldefense.com/v3/__https://github.com/rmartinc/apache-ds-referrals__;!!ACWV5N9M2RV99hQ!IZkp5q_gOAeIP8Y9Gvr8aniLloG51lZJwlG1yN6BRDyHHLpyr0W64TDMUPAzoPu7dOBOyJrNcKYmwaOF9REM3oA$
diff --git a/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java b/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java index 5c7644212f9..89e7427cea6 100644 --- a/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java +++ b/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1999, 2018, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1999, 2022, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -310,10 +310,8 @@ abstract class AbstractLdapNamingEnumeration<T extends NameClassPair> */ protected final boolean hasMoreReferrals() throws NamingException { - if ((refEx != null) && - (refEx.hasMoreReferrals() || - refEx.hasMoreReferralExceptions() - && !(errEx instanceof LimitExceededException))) { + if ((refEx != null) && !(errEx instanceof LimitExceededException) && + (refEx.hasMoreReferrals() || refEx.hasMoreReferralExceptions())) { if (homeCtx.handleReferrals == LdapClient.LDAP_REF_THROW) { throw (NamingException)(refEx.fillInStackTrace()); @@ -333,8 +331,11 @@ abstract class AbstractLdapNamingEnumeration<T extends NameClassPair> } catch (LdapReferralException re) { - // record a previous exception - if (errEx == null) { + // record a previous exception and quit if any limit is reached + if (re.getNamingException() instanceof LimitExceededException) { + errEx = re.getNamingException(); + break; + } else if (errEx == null) { errEx = re.getNamingException(); } refEx = re; @@ -381,6 +382,10 @@ abstract class AbstractLdapNamingEnumeration<T extends NameClassPair> entries = ne.entries; refEx = ne.refEx; listArg = ne.listArg; + // record a previous exception and quit if any limit is reached + if (errEx == null || ne.errEx instanceof LimitExceededException) { + errEx = ne.errEx; + } } @SuppressWarnings("removal") diff --git a/test/jdk/com/sun/jndi/ldap/ReferralLimitSearchTest.java b/test/jdk/com/sun/jndi/ldap/ReferralLimitSearchTest.java new file mode 100644 index 00000000000..76991e907a1 --- /dev/null +++ b/test/jdk/com/sun/jndi/ldap/ReferralLimitSearchTest.java @@ -0,0 +1,164 @@ +/* + * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/** + * @test + * @bug 8176553 + * @summary LdapContext follows referrals infinitely ignoring set limit + * @library lib/ /test/lib + */ + +import java.io.IOException; +import java.io.OutputStream; +import java.net.Socket; +import javax.naming.NamingEnumeration; +import javax.naming.directory.DirContext; +import javax.naming.directory.SearchControls; +import java.nio.charset.StandardCharsets; +import java.net.InetAddress; +import java.util.Properties; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import javax.naming.Context; +import javax.naming.LimitExceededException; +import javax.naming.directory.InitialDirContext; +import javax.naming.directory.SearchResult; +import jdk.test.lib.net.URIBuilder; + +/** + * <p>Little test for referral limit. The ldap server is configured to + * always return a referral so LimitExceededException is expected when + * <em>java.naming.ldap.referral.limit</em> is reached.</p> + * + * @author rmartinc + */ +public class ReferralLimitSearchTest { + + // number of refarral hops to test + private static final int MAX_REFERRAL_LIMIT = 4; + + // success bind response (byte 4 is the message id) + private static final byte[] bindResponse = { + 0x30, 0x0C, 0x02, 0x01, 0x00, 0x61, 0x07, 0x0A, + 0x01, 0x00, 0x04, 0x00, 0x04, 0x00 + }; + + // search res done (byte 4 is the message id) + private static final byte[] searchResponse = { + 0x30, 0x0C, 0x02, 0x01, 0x00, 0x65, 0x07, 0x0A, + 0x01, 0x00, 0x04, 0x00, 0x04, 0x00 + }; + + public static void main(String[] args) throws Exception { + + final CountDownLatch latch = new CountDownLatch(1); + + // Start the LDAP server + BaseLdapServer ldapServer = new BaseLdapServer() { + AtomicInteger hops = new AtomicInteger(0); + + @Override + protected void handleRequest(Socket socket, LdapMessage request, + OutputStream out) throws IOException { + switch (request.getOperation()) { + case BIND_REQUEST: + bindResponse[4] = (byte) request.getMessageID(); + out.write(bindResponse); + break; + case SEARCH_REQUEST: + if (hops.incrementAndGet() > MAX_REFERRAL_LIMIT + 1) { + throw new IOException("Referral limit not enforced. Number of hops=" + hops); + } + byte[] referral = new StringBuilder("ldap://") + .append(InetAddress.getLoopbackAddress().getHostAddress()) + .append(":") + .append(getPort()) + .append("/ou=People??sub") + .toString() + .getBytes(StandardCharsets.UTF_8); + out.write(0x30); + out.write(referral.length + 7); + out.write(new byte[] {0x02, 0x01}); + out.write(request.getMessageID()); + out.write(0x73); + out.write(referral.length + 2); + out.write(0x04); + out.write(referral.length); + out.write(referral); + + searchResponse[4] = (byte) request.getMessageID(); + out.write(searchResponse); + break; + default: + break; + } + } + + protected void beforeAcceptingConnections() { + latch.countDown(); + } + }.start(); + + try (ldapServer) { + + if (!latch.await(5, TimeUnit.SECONDS)) { + throw new RuntimeException("LdapServer not started in time"); + } + + // Setup JNDI parameters + Properties env = new Properties(); + env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory"); + env.put(Context.REFERRAL, "follow"); + env.put("java.naming.ldap.referral.limit", Integer.toString(MAX_REFERRAL_LIMIT)); + env.put(Context.PROVIDER_URL, URIBuilder.newBuilder() + .scheme("ldap") + .loopback() + .port(ldapServer.getPort()) + .build().toString()); + + System.out.println("Client: connecting..."); + DirContext ctx = new InitialDirContext(env); + SearchControls sc = new SearchControls(); + sc.setSearchScope(SearchControls.SUBTREE_SCOPE); + sc.setReturningAttributes(new String[]{"uid"}); + System.out.println("Client: performing search..."); + NamingEnumeration<SearchResult> ne = ctx.search("ou=People", "(uid=*)", sc); + while (ne.hasMore()) { + SearchResult sr = ne.next(); + System.out.println("Client: Search result " + sr); + } + System.out.println("Client: search done..."); + ctx.close(); + + // LimitExceededException expected throw error + throw new RuntimeException("LimitExceededException expected"); + + } catch (LimitExceededException e) { + System.out.println("Passed: caught the expected Exception - " + e); + } catch (Exception e) { + System.err.println("Failed: caught an unexpected Exception - " + e); + throw e; + } + } +}