[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;
+        }
+    }
+}

Reply via email to