From c1a786981cf5a7d4d3e37049ea65bbb632e44521 Mon Sep 17 00:00:00 2001 From: kimyenac Date: Wed, 5 Aug 2026 11:23:37 +0900 Subject: [PATCH 1/3] [ZEPPELIN-6599] Escape user-supplied search text in LDAP user search filters --- .../service/ShiroAuthenticationService.java | 34 +++- ...henticationServiceFilterInjectionTest.java | 148 ++++++++++++++++++ 2 files changed, 174 insertions(+), 8 deletions(-) create mode 100644 zeppelin-server/src/test/java/org/apache/zeppelin/service/ShiroAuthenticationServiceFilterInjectionTest.java diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/service/ShiroAuthenticationService.java b/zeppelin-server/src/main/java/org/apache/zeppelin/service/ShiroAuthenticationService.java index 21219c6e2e5..3797624a01a 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/service/ShiroAuthenticationService.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/service/ShiroAuthenticationService.java @@ -53,6 +53,7 @@ import org.apache.shiro.util.ThreadContext; import org.apache.zeppelin.conf.ZeppelinConfiguration; import org.apache.zeppelin.realm.ActiveDirectoryGroupRealm; +import org.apache.zeppelin.realm.LdapFilterEncoder; import org.apache.zeppelin.realm.LdapRealm; import org.apache.zeppelin.realm.jwt.KnoxJwtRealm; import org.slf4j.Logger; @@ -315,7 +316,7 @@ private List getUserList(DefaultLdapRealm r, String searchText, int numU String[] attrIDs = {userDnPrefix}; constraints.setReturningAttributes(attrIDs); NamingEnumeration result = - ctx.search(userDnSuffix, "(" + userDnPrefix + "=*" + searchText + "*)", constraints); + ctx.search(userDnSuffix, buildUserSearchFilter(userDnPrefix, searchText), constraints); while (result.hasMore()) { Attributes attrs = result.next().getAttributes(); if (attrs.get(userDnPrefix) != null) { @@ -330,6 +331,17 @@ private List getUserList(DefaultLdapRealm r, String searchText, int numU return userList; } + /** + * Builds the user search filter for {@link DefaultLdapRealm}. The attribute name and the search + * text are escaped per RFC 4515; the wildcards Zeppelin adds around the search text stay outside + * the escaped value so that substring matching keeps working. + */ + static String buildUserSearchFilter(String userDnPrefix, String searchText) { + return String.format("(%s=*%s*)", + LdapFilterEncoder.escapeFilterValue(userDnPrefix), + LdapFilterEncoder.escapeFilterValue(searchText)); + } + /** Function to extract users from Zeppelin LdapRealm. */ private List getUserList(LdapRealm r, String searchText, int numUsersToFetch) { List userList = new ArrayList<>(); @@ -348,13 +360,7 @@ private List getUserList(LdapRealm r, String searchText, int numUsersToF NamingEnumeration result = ctx.search( userSearchRealm, - "(&(objectclass=" - + userObjectClass - + ")(" - + userAttribute - + "=*" - + searchText - + "*))", + buildUserSearchFilterWithObjectClass(userObjectClass, userAttribute, searchText), constraints); while (result.hasMore()) { Attributes attrs = result.next().getAttributes(); @@ -377,6 +383,18 @@ private List getUserList(LdapRealm r, String searchText, int numUsersToF return userList; } + /** + * Builds the user search filter for Zeppelin {@link LdapRealm}. Follows the same escaping rules + * as {@link #buildUserSearchFilter(String, String)}, with the user object class escaped as well. + */ + static String buildUserSearchFilterWithObjectClass(String userObjectClass, String userAttribute, + String searchText) { + return String.format("(&(objectclass=%s)(%s=*%s*))", + LdapFilterEncoder.escapeFilterValue(userObjectClass), + LdapFilterEncoder.escapeFilterValue(userAttribute), + LdapFilterEncoder.escapeFilterValue(searchText)); + } + /** * * Get user roles from shiro.ini for Zeppelin LdapRealm. * diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/service/ShiroAuthenticationServiceFilterInjectionTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/service/ShiroAuthenticationServiceFilterInjectionTest.java new file mode 100644 index 00000000000..59268f33613 --- /dev/null +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/service/ShiroAuthenticationServiceFilterInjectionTest.java @@ -0,0 +1,148 @@ +/* + * 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.zeppelin.service; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.stream.Stream; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +/** + * Tests verifying that the search text supplied to + * {@code GET /api/security/userlist/{searchText}} cannot inject LDAP filter metacharacters into + * the filters that {@link ShiroAuthenticationService} builds for the LDAP realms. The rendered + * filter must never contain unescaped {@code (}, {@code )} or {@code *} characters that + * originated in the search text. + */ +class ShiroAuthenticationServiceFilterInjectionTest { + + private static final String USER_ATTRIBUTE = "uid"; + private static final String USER_OBJECT_CLASS = "person"; + + // "(uid=*%s*)" contributes 1 '(', 1 ')' and the 2 wildcards Zeppelin adds itself. + private static final int DEFAULT_LDAP_OPEN_PARENS = 1; + private static final int DEFAULT_LDAP_CLOSE_PARENS = 1; + private static final int DEFAULT_LDAP_ASTERISKS = 2; + + // "(&(objectclass=person)(uid=*%s*))" contributes 3 '(', 3 ')' and the same 2 wildcards. + private static final int LDAP_REALM_OPEN_PARENS = 3; + private static final int LDAP_REALM_CLOSE_PARENS = 3; + private static final int LDAP_REALM_ASTERISKS = 2; + + static Stream injectionPayloads() { + return Stream.of( + ")(uid=*", + "admin)(|(uid=*", + "*", + "admin)(cn=a*", + ")(mail=*@corp.com", + "alice)(userPassword=*"); + } + + @ParameterizedTest + @MethodSource("injectionPayloads") + void defaultLdapRealmFilterNeutralizesPayload(String payload) { + String rendered = ShiroAuthenticationService.buildUserSearchFilter(USER_ATTRIBUTE, payload); + + assertMetacharacterCounts(rendered, payload, + DEFAULT_LDAP_OPEN_PARENS, DEFAULT_LDAP_CLOSE_PARENS, DEFAULT_LDAP_ASTERISKS); + } + + @ParameterizedTest + @MethodSource("injectionPayloads") + void ldapRealmFilterNeutralizesPayload(String payload) { + String rendered = ShiroAuthenticationService.buildUserSearchFilterWithObjectClass( + USER_OBJECT_CLASS, USER_ATTRIBUTE, payload); + + assertMetacharacterCounts(rendered, payload, + LDAP_REALM_OPEN_PARENS, LDAP_REALM_CLOSE_PARENS, LDAP_REALM_ASTERISKS); + } + + @Test + void normalSearchTextKeepsSubstringMatching() { + assertEquals("(uid=*alice*)", + ShiroAuthenticationService.buildUserSearchFilter(USER_ATTRIBUTE, "alice")); + assertEquals("(&(objectclass=person)(uid=*alice*))", + ShiroAuthenticationService.buildUserSearchFilterWithObjectClass( + USER_OBJECT_CLASS, USER_ATTRIBUTE, "alice")); + } + + @Test + void asteriskInSearchTextBecomesLiteral() { + // The wildcards Zeppelin adds stay wildcards, the one typed by the user does not. + assertEquals("(uid=*a\\2ab*)", + ShiroAuthenticationService.buildUserSearchFilter(USER_ATTRIBUTE, "a*b")); + assertEquals("(&(objectclass=person)(uid=*a\\2ab*))", + ShiroAuthenticationService.buildUserSearchFilterWithObjectClass( + USER_OBJECT_CLASS, USER_ATTRIBUTE, "a*b")); + } + + @Test + void emptySearchTextKeepsExistingBehaviour() { + assertEquals("(uid=**)", + ShiroAuthenticationService.buildUserSearchFilter(USER_ATTRIBUTE, "")); + assertEquals("(&(objectclass=person)(uid=**))", + ShiroAuthenticationService.buildUserSearchFilterWithObjectClass( + USER_OBJECT_CLASS, USER_ATTRIBUTE, "")); + } + + @Test + void configuredAttributeNamesAreEscapedAsWell() { + assertEquals("(&(objectclass=per\\29son)(u\\28id=*alice*))", + ShiroAuthenticationService.buildUserSearchFilterWithObjectClass("per)son", "u(id", + "alice")); + } + + private static void assertMetacharacterCounts(String rendered, String payload, + int expectedOpenParens, int expectedCloseParens, int expectedAsterisks) { + assertEquals(expectedOpenParens, count(rendered, '('), + "extra unescaped '(' from payload: " + rendered); + assertEquals(expectedCloseParens, count(rendered, ')'), + "extra unescaped ')' from payload: " + rendered); + assertEquals(expectedAsterisks, count(rendered, '*'), + "extra unescaped '*' from payload: " + rendered); + + if (payload.indexOf('(') >= 0) { + assertTrue(rendered.contains("\\28"), "missing \\28 in: " + rendered); + } + if (payload.indexOf(')') >= 0) { + assertTrue(rendered.contains("\\29"), "missing \\29 in: " + rendered); + } + if (payload.indexOf('*') >= 0) { + assertTrue(rendered.contains("\\2a"), "missing \\2a in: " + rendered); + } + } + + /** + * Counts occurrences of {@code ch} in the rendered filter. {@code LdapFilterEncoder} replaces + * every metacharacter with a hex escape such as {@code \2a}, so a metacharacter that is still + * present as itself is by definition an unescaped one. + */ + private static int count(String s, char ch) { + int count = 0; + for (int i = 0; i < s.length(); i++) { + if (s.charAt(i) == ch) { + count++; + } + } + return count; + } +} From 4a20a5bc08323f6d90fcad888f9f0cc0a36f6852 Mon Sep 17 00:00:00 2001 From: kimyenac Date: Wed, 5 Aug 2026 11:53:21 +0900 Subject: [PATCH 2/3] [ZEPPELIN-6599] Stop compiling the user list search text as a regular expression --- .../apache/zeppelin/rest/SecurityRestApi.java | 15 +++++---------- .../zeppelin/rest/SecurityRestApiTest.java | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/SecurityRestApi.java b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/SecurityRestApi.java index 89bc317734d..5eec6e7713f 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/SecurityRestApi.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/SecurityRestApi.java @@ -18,6 +18,7 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -102,16 +103,10 @@ public Response getUserList(@PathParam("searchText") final String searchText) { List autoSuggestRoleList = new ArrayList<>(); Collections.sort(usersList); Collections.sort(rolesList); - Collections.sort( - usersList, - (o1, o2) -> { - if (o1.matches(searchText + "(.*)") && o2.matches(searchText + "(.*)")) { - return 0; - } else if (o1.matches(searchText + "(.*)")) { - return -1; - } - return 0; - }); + // List the users whose name starts with the search text first, keeping the alphabetical order + // within each group. The search text comes from the client, so it must not be compiled as a + // regular expression here. + usersList.sort(Comparator.comparing((String user) -> !user.startsWith(searchText))); int maxLength = 0; for (String user : usersList) { if (StringUtils.containsIgnoreCase(user, searchText)) { diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/SecurityRestApiTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/SecurityRestApiTest.java index b7e01b1fec7..25912b063f3 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/SecurityRestApiTest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/SecurityRestApiTest.java @@ -91,6 +91,23 @@ void testGetUserList() throws IOException { notUser.close(); } + @Test + void testGetUserListWithRegexMetacharacters() throws IOException { + // The search text is not a regular expression. Metacharacters must not break the endpoint. + for (String searchText : new String[] {"%2A", "%28", "%2B"}) { + CloseableHttpResponse get = httpGet("/security/userlist/" + searchText, "admin", "password1"); + assertThat("Status code for search text " + searchText, + get.getStatusLine().getStatusCode(), CoreMatchers.equalTo(200)); + Map resp = gson.fromJson( + EntityUtils.toString(get.getEntity(), StandardCharsets.UTF_8), + new TypeToken>(){}.getType()); + List userList = (List) ((Map) resp.get("body")).get("users"); + assertThat("Search result size for search text " + searchText, userList.size(), + CoreMatchers.equalTo(0)); + get.close(); + } + } + @Test void testRolesEscaped() throws IOException { CloseableHttpResponse get = httpGet("/security/ticket", "admin", "password1"); From c7310e099968798a8494cd7e7b7d765815eecca5 Mon Sep 17 00:00:00 2001 From: kimyenac Date: Wed, 5 Aug 2026 12:57:46 +0900 Subject: [PATCH 3/3] [ZEPPELIN-6599] Cover backslash and NUL in the LDAP filter injection tests --- ...henticationServiceFilterInjectionTest.java | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/service/ShiroAuthenticationServiceFilterInjectionTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/service/ShiroAuthenticationServiceFilterInjectionTest.java index 59268f33613..14dca43b90a 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/service/ShiroAuthenticationServiceFilterInjectionTest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/service/ShiroAuthenticationServiceFilterInjectionTest.java @@ -54,7 +54,12 @@ static Stream injectionPayloads() { "*", "admin)(cn=a*", ")(mail=*@corp.com", - "alice)(userPassword=*"); + "alice)(userPassword=*", + "alice\\", + "alice\\2a", + "alice\\29\\28uid=\\2a", + "\\", + "\0"); } @ParameterizedTest @@ -111,6 +116,17 @@ void configuredAttributeNamesAreEscapedAsWell() { "alice")); } + @Test + void backslashAndNulInSearchTextAreEscaped() { + assertEquals("(uid=*alice\\5c*)", + ShiroAuthenticationService.buildUserSearchFilter(USER_ATTRIBUTE, "alice\\")); + assertEquals("(uid=*alice\\00*)", + ShiroAuthenticationService.buildUserSearchFilter(USER_ATTRIBUTE, "alice\0")); + assertEquals("(&(objectclass=person)(uid=*alice\\5c\\00*))", + ShiroAuthenticationService.buildUserSearchFilterWithObjectClass( + USER_OBJECT_CLASS, USER_ATTRIBUTE, "alice\\\0")); + } + private static void assertMetacharacterCounts(String rendered, String payload, int expectedOpenParens, int expectedCloseParens, int expectedAsterisks) { assertEquals(expectedOpenParens, count(rendered, '('), @@ -129,6 +145,12 @@ private static void assertMetacharacterCounts(String rendered, String payload, if (payload.indexOf('*') >= 0) { assertTrue(rendered.contains("\\2a"), "missing \\2a in: " + rendered); } + if (payload.indexOf('\\') >= 0) { + assertTrue(rendered.contains("\\5c"), "missing \\5c in: " + rendered); + } + if (payload.indexOf('\0') >= 0) { + assertTrue(rendered.contains("\\00"), "missing \\00 in: " + rendered); + } } /**