[reposting to core-libs-dev as this is in the JNDI/LDAP component]
-------- Forwarded Message --------
Subject: Bug JDK-8176553
Date: Fri, 17 Jun 2022 14:23:11 +0200
From: Ricardo Martin Camarero <rmart...@redhat.com>
To: security-...@openjdk.org
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;
+ }
+ }
+}