This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new b36915ae93b4 [SPARK][SQL] LDAP Thriftserver improvement
b36915ae93b4 is described below
commit b36915ae93b489d087d415b92bab9022a8b89528
Author: Claude <Holden's Claude Robot [email protected]>
AuthorDate: Thu Jul 9 13:08:01 2026 -0700
[SPARK][SQL] LDAP Thriftserver improvement
Improve DN construction in the Hive ThriftServer LDAP provider by escaping
user-supplied values to avoid misconfiguration. Uses built in Java LDAP RFC
impl.
Adds unit tests for the DN construction.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-Authored-By: Holden Karau <[email protected]>
Co-Authored-By: Holden Karau <[email protected]>
Minor CR feedback
Apply suggestions from code review add missing period
Co-authored-by: Hyukjin Kwon <[email protected]>
Add a comment explaining the condition under which there is no candidate
principals.
Improve wording
Backport babd0ee17b8c0794e41efeeedf1e4ae7738af860
---
.../auth/LdapAuthenticationProviderImpl.java | 58 ++++++++-----
.../auth/LdapAuthenticationProviderImplSuite.scala | 98 ++++++++++++++++++++++
2 files changed, 137 insertions(+), 19 deletions(-)
diff --git
a/sql/hive-thriftserver/src/main/java/org/apache/hive/service/auth/LdapAuthenticationProviderImpl.java
b/sql/hive-thriftserver/src/main/java/org/apache/hive/service/auth/LdapAuthenticationProviderImpl.java
index e6ae9dab8b3e..09d0101bfa79 100644
---
a/sql/hive-thriftserver/src/main/java/org/apache/hive/service/auth/LdapAuthenticationProviderImpl.java
+++
b/sql/hive-thriftserver/src/main/java/org/apache/hive/service/auth/LdapAuthenticationProviderImpl.java
@@ -24,6 +24,7 @@ import java.util.List;
import javax.naming.Context;
import javax.naming.NamingException;
import javax.naming.directory.InitialDirContext;
+import javax.naming.ldap.Rdn;
import javax.security.sasl.AuthenticationException;
import org.apache.hadoop.hive.conf.HiveConf;
@@ -39,7 +40,11 @@ public class LdapAuthenticationProviderImpl implements
PasswdAuthenticationProvi
private final String userDNPattern;
LdapAuthenticationProviderImpl() {
- HiveConf conf = new HiveConf();
+ this(new HiveConf());
+ }
+
+ // Visible for testing.
+ LdapAuthenticationProviderImpl(HiveConf conf) {
ldapURL = conf.getVar(HiveConf.ConfVars.HIVE_SERVER2_PLAIN_LDAP_URL);
baseDN = conf.getVar(HiveConf.ConfVars.HIVE_SERVER2_PLAIN_LDAP_BASEDN);
ldapDomain = conf.getVar(HiveConf.ConfVars.HIVE_SERVER2_PLAIN_LDAP_DOMAIN);
@@ -62,24 +67,7 @@ public class LdapAuthenticationProviderImpl implements
PasswdAuthenticationProvi
}
// setup the security principal
- List<String> candidatePrincipals = new ArrayList<>();
- if (Utils.isBlank(userDNPattern)) {
- if (Utils.isNotBlank(baseDN)) {
- String pattern = "uid=" + user + "," + baseDN;
- candidatePrincipals.add(pattern);
- }
- } else {
- String[] patterns = userDNPattern.split(":");
- for (String pattern : patterns) {
- if (pattern.contains(",") && pattern.contains("=")) {
- candidatePrincipals.add(pattern.replaceAll("%s", user));
- }
- }
- }
-
- if (candidatePrincipals.isEmpty()) {
- candidatePrincipals = Collections.singletonList(user);
- }
+ List<String> candidatePrincipals = createCandidatePrincipals(user);
for (Iterator<String> iterator = candidatePrincipals.iterator();
iterator.hasNext();) {
String principal = iterator.next();
@@ -105,6 +93,38 @@ public class LdapAuthenticationProviderImpl implements
PasswdAuthenticationProvi
}
}
+ // Builds the list of LDAP principals (Distinguished Names) to attempt
binding with for the
+ // given user. The user-supplied value is escaped with {@link
Rdn#escapeValue} before it is
+ // placed into a DN so that LDAP special characters including ',', '=', '+',
'<', '>', etc.
+ // cannot alter the DN structure. See RFC-2253.
+ // Visible for testing.
+ List<String> createCandidatePrincipals(String user) {
+ String escapedUser = Rdn.escapeValue(user);
+ List<String> candidatePrincipals = new ArrayList<>();
+ if (Utils.isBlank(userDNPattern)) {
+ if (Utils.isNotBlank(baseDN)) {
+ String pattern = "uid=" + escapedUser + "," + baseDN;
+ candidatePrincipals.add(pattern);
+ }
+ } else {
+ String[] patterns = userDNPattern.split(":");
+ for (String pattern : patterns) {
+ if (pattern.contains(",") && pattern.contains("=")) {
+ // Use literal replacement: the escaped user may contain '\\', which
would otherwise be
+ // treated as a regex escape by String.replaceAll.
+ candidatePrincipals.add(pattern.replace("%s", escapedUser));
+ }
+ }
+ }
+
+ // If there is no configured pattern or base we accept the raw user in
full because there is no
+ // pattern we're trying to safely inject into.
+ if (candidatePrincipals.isEmpty()) {
+ candidatePrincipals = Collections.singletonList(user);
+ }
+ return candidatePrincipals;
+ }
+
private boolean hasDomain(String userName) {
return (ServiceUtils.indexOfDomainMatch(userName) > 0);
}
diff --git
a/sql/hive-thriftserver/src/test/scala/org/apache/hive/service/auth/LdapAuthenticationProviderImplSuite.scala
b/sql/hive-thriftserver/src/test/scala/org/apache/hive/service/auth/LdapAuthenticationProviderImplSuite.scala
new file mode 100644
index 000000000000..3753f78fe55a
--- /dev/null
+++
b/sql/hive-thriftserver/src/test/scala/org/apache/hive/service/auth/LdapAuthenticationProviderImplSuite.scala
@@ -0,0 +1,98 @@
+/*
+ * 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.hive.service.auth
+
+import java.util.{Arrays => JArrays, List => JList}
+
+import org.apache.hadoop.hive.conf.HiveConf
+
+import org.apache.spark.SparkFunSuite
+
+class LdapAuthenticationProviderImplSuite extends SparkFunSuite {
+
+ private def newProvider(
+ baseDN: String = null,
+ userDNPattern: String = null,
+ ldapDomain: String = null): LdapAuthenticationProviderImpl = {
+ val conf = new HiveConf()
+ // Avoid picking up a real LDAP URL from any hive-site.xml on the test
classpath.
+ conf.setVar(HiveConf.ConfVars.HIVE_SERVER2_PLAIN_LDAP_URL,
"ldap://localhost:389";)
+ if (baseDN != null) {
+ conf.setVar(HiveConf.ConfVars.HIVE_SERVER2_PLAIN_LDAP_BASEDN, baseDN)
+ }
+ if (userDNPattern != null) {
+ conf.setVar(HiveConf.ConfVars.HIVE_SERVER2_PLAIN_LDAP_USERDNPATTERN,
userDNPattern)
+ }
+ if (ldapDomain != null) {
+ conf.setVar(HiveConf.ConfVars.HIVE_SERVER2_PLAIN_LDAP_DOMAIN, ldapDomain)
+ }
+ new LdapAuthenticationProviderImpl(conf)
+ }
+
+ private def principals(provider: LdapAuthenticationProviderImpl, user:
String): JList[String] =
+ provider.createCandidatePrincipals(user)
+
+ private def dns(values: String*): JList[String] = JArrays.asList(values: _*)
+
+ test("legitimate usernames are left untouched when building the DN") {
+ val provider = newProvider(baseDN = "ou=users,dc=example,dc=com")
+ assert(principals(provider, "alice") ===
+ dns("uid=alice,ou=users,dc=example,dc=com"))
+ }
+
+ test("LDAP special characters in the username are escaped for the baseDN
pattern") {
+ val provider = newProvider(baseDN = "ou=users,dc=example,dc=com")
+ // Without escaping this would be
"uid=admin,ou=Admins,ou=users,dc=example,dc=com", letting
+ // the attacker bind as a different DN (authentication bypass /
impersonation).
+ assert(principals(provider, "admin,ou=Admins") ===
+ dns("uid=admin\\,ou\\=Admins,ou=users,dc=example,dc=com"))
+ }
+
+ test("full DN structure manipulation is neutralized for the baseDN pattern")
{
+ val provider = newProvider(baseDN = "ou=users,dc=example,dc=com")
+ assert(principals(provider, "attacker,ou=Admins,dc=example,dc=com") ===
+
dns("uid=attacker\\,ou\\=Admins\\,dc\\=example\\,dc\\=com,ou=users,dc=example,dc=com"))
+ }
+
+ test("LDAP special characters in the username are escaped for
userDNPattern") {
+ val provider = newProvider(userDNPattern =
"uid=%s,ou=service,dc=example,dc=com")
+ assert(principals(provider, "admin,ou=Admins") ===
+ dns("uid=admin\\,ou\\=Admins,ou=service,dc=example,dc=com"))
+ }
+
+ test("multiple userDNPatterns are all escaped") {
+ val provider = newProvider(
+ userDNPattern =
"uid=%s,ou=service,dc=example,dc=com:cn=%s,ou=people,dc=example,dc=com")
+ assert(principals(provider, "a+b,c") === dns(
+ "uid=a\\+b\\,c,ou=service,dc=example,dc=com",
+ "cn=a\\+b\\,c,ou=people,dc=example,dc=com"))
+ }
+
+ test("escaped backslashes are inserted literally (no regex replacement)") {
+ // The escaped value contains a backslash; String.replaceAll would treat
it as a regex escape
+ // and either throw or corrupt the output. Verify literal insertion.
+ val provider = newProvider(userDNPattern =
"uid=%s,ou=service,dc=example,dc=com")
+ assert(principals(provider, "a\\b") ===
+ dns("uid=a\\\\b,ou=service,dc=example,dc=com"))
+ }
+
+ test("leading and trailing spaces in the username are escaped") {
+ val provider = newProvider(baseDN = "ou=users,dc=example,dc=com")
+ assert(principals(provider, " alice ") ===
+ dns("uid=\\ alice\\ ,ou=users,dc=example,dc=com"))
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
