From 27147915fbea307fce63e2d3bb3404ca80b8e52c Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Wed, 08 Jul 2026 16:17:55 +0000
Subject: [PATCH] Fix ACI grouped bind rule wrongly rejected when a value contains parentheses (#716)
---
opendj-server-legacy/src/test/java/org/opends/server/authorization/dseecompat/BindRuleParenTest.java | 99 +++++++++++++++++++++++++++++++++
opendj-server-legacy/src/main/java/org/opends/server/authorization/dseecompat/BindRule.java | 26 +++++---
2 files changed, 116 insertions(+), 9 deletions(-)
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/authorization/dseecompat/BindRule.java b/opendj-server-legacy/src/main/java/org/opends/server/authorization/dseecompat/BindRule.java
index 52f7b09..8952ea6 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/authorization/dseecompat/BindRule.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/authorization/dseecompat/BindRule.java
@@ -13,6 +13,7 @@
*
* Copyright 2008 Sun Microsystems, Inc.
* Portions Copyright 2013-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC
*/
package org.opends.server.authorization.dseecompat;
@@ -133,11 +134,10 @@
}
/*
- * TODO Verify this method handles escaped parentheses by writing
- * a unit test.
- *
- * It doesn't look like the decode() method handles the possibility of
- * escaped parentheses in a bind rule.
+ * Parentheses embedded in a quoted bind rule expression (for example a DN
+ * value such as userdn="ldap:///cn=a(b),dc=example,dc=com") are treated as
+ * literal data and are not mistaken for grouping parentheses. See the
+ * quote-aware scan below and BindRuleParenTest for the covering cases.
*/
/**
* Decode an ACI bind rule string representation.
@@ -160,19 +160,27 @@
int currentPos;
int numOpen = 0;
int numClose = 0;
+ boolean inQuotes = false;
- // Find the associated closed parenthesis
+ // Find the associated closed parenthesis. Parentheses that appear
+ // inside a quoted bind rule expression (e.g. a DN value containing
+ // '(' or ')') are literal data and must not be counted as grouping.
for (currentPos = 0; currentPos < bindruleArray.length; currentPos++)
{
- if (bindruleArray[currentPos] == '(')
+ char currentChar = bindruleArray[currentPos];
+ if (currentChar == '"')
+ {
+ inQuotes = !inQuotes;
+ }
+ else if (!inQuotes && currentChar == '(')
{
numOpen++;
}
- else if (bindruleArray[currentPos] == ')')
+ else if (!inQuotes && currentChar == ')')
{
numClose++;
}
- if (numClose == numOpen)
+ if (!inQuotes && numClose == numOpen)
{
// We found the associated closed parenthesis the parenthesis are removed
String bindruleStr1 = bindruleStr.substring(1, currentPos);
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/authorization/dseecompat/BindRuleParenTest.java b/opendj-server-legacy/src/test/java/org/opends/server/authorization/dseecompat/BindRuleParenTest.java
new file mode 100644
index 0000000..7174f08
--- /dev/null
+++ b/opendj-server-legacy/src/test/java/org/opends/server/authorization/dseecompat/BindRuleParenTest.java
@@ -0,0 +1,99 @@
+/*
+ * The contents of this file are subject to the terms of the Common Development and
+ * Distribution License (the License). You may not use this file except in compliance with the
+ * License.
+ *
+ * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the
+ * specific language governing permission and limitations under the License.
+ *
+ * When distributing Covered Software, include this CDDL Header Notice in each file and include
+ * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL
+ * Header, with the fields enclosed by brackets [] replaced by your own identifying
+ * information: "Portions Copyright [year] [name of copyright owner]".
+ *
+ * Portions Copyright 2026 3A Systems, LLC
+ */
+package org.opends.server.authorization.dseecompat;
+
+import static org.assertj.core.api.Assertions.*;
+
+import org.opends.server.DirectoryServerTestCase;
+import org.opends.server.TestCaseUtils;
+import org.opends.server.types.DirectoryException;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+/**
+ * Verifies that {@link BindRule#decode(String)} treats parentheses embedded in a
+ * quoted bind rule expression (e.g. a DN value containing '(' or ')') as literal
+ * data instead of grouping parentheses. Before the quote-aware scan, a grouped
+ * bind rule whose value contained a parenthesis was wrongly rejected because the
+ * embedded parenthesis was counted when locating the matching close parenthesis.
+ * Also verifies the scan stays fail-closed: genuinely unbalanced grouping
+ * parentheses (outside quotes) are still rejected with an {@link AciException}.
+ */
+@SuppressWarnings("javadoc")
+public class BindRuleParenTest extends DirectoryServerTestCase
+{
+ @BeforeClass
+ public void setUp() throws Exception
+ {
+ TestCaseUtils.startFakeServer();
+ }
+
+ @AfterClass
+ public void tearDown() throws DirectoryException
+ {
+ TestCaseUtils.shutdownFakeServer();
+ }
+
+ @DataProvider(name = "acisWithParenInValue")
+ public Object[][] acisWithParenInValue()
+ {
+ return new Object[][] {
+ // simple (ungrouped) bind rule with a ')' / '(' in the DN value
+ { "(version 3.0; acl \"t\"; allow (search) userdn=\"ldap:///cn=a)b,dc=x\";)" },
+ { "(version 3.0; acl \"t\"; allow (search) userdn=\"ldap:///cn=a(b,dc=x\";)" },
+ // grouped bind rule with a ')' embedded in a value
+ { "(version 3.0; acl \"t\"; allow (search) "
+ + "(userdn=\"ldap:///cn=a)b,dc=x\" or userdn=\"ldap:///self\");)" },
+ // grouped bind rule with a '(' embedded in a value
+ { "(version 3.0; acl \"t\"; allow (search) "
+ + "(userdn=\"ldap:///cn=a(b,dc=x\" or userdn=\"ldap:///self\");)" },
+ // grouped bind rule with matched parentheses embedded in a value
+ { "(version 3.0; acl \"t\"; allow (search) "
+ + "(userdn=\"ldap:///cn=a(b),dc=x\" or userdn=\"ldap:///self\");)" },
+ };
+ }
+
+ @Test(dataProvider = "acisWithParenInValue")
+ public void decodesParenInsideQuotedValue(String aci) throws Exception
+ {
+ AciBody body = AciBody.decode(aci);
+ assertThat(body.getPermBindRulePairs()).hasSize(1);
+ assertThat(body.getPermBindRulePairs().get(0).getBindRule()).isNotNull();
+ }
+
+ @DataProvider(name = "malformedBindRules")
+ public Object[][] malformedBindRules()
+ {
+ return new Object[][] {
+ // grouped bind rule with no close parenthesis at all
+ { "(userdn=\"ldap:///self\"" },
+ // grouped bind rule with one extra (unmatched) open parenthesis
+ { "((userdn=\"ldap:///self\")" },
+ // grouped bind rule whose only ')' is embedded in a quoted value, so the
+ // group is genuinely unterminated: the quote-aware scan must not treat the
+ // embedded ')' as the closing parenthesis and silently accept the input.
+ { "(userdn=\"ldap:///cn=a)b,dc=x\" or userdn=\"ldap:///self\"" },
+ };
+ }
+
+ @Test(dataProvider = "malformedBindRules")
+ public void rejectsUnbalancedParensOutsideQuotes(String bindRule)
+ {
+ assertThatThrownBy(() -> BindRule.decode(bindRule)).isInstanceOf(AciException.class);
+ }
+}
--
Gitblit v1.10.0