You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by tw...@apache.org on 2026/09/26 14:46:40 UTC
(mina-sshd) 11/18: Fix LDAP authentication
This is an automated email from the ASF dual-hosted git repository.
tomaswolf pushed a commit to branch rc-2.20.0
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
commit acc0f1ed436529c24f6677da83e7ec4e3537c4c9
Author: Thomas Wolf <tw...@apache.org>
AuthorDate: Sat Sep 19 23:09:00 2026 +0200
Fix LDAP authentication
Properly encode filter parameters following RFC 4515, and encode
parameters for the distinguished name following RFC 4514. Also harden
the password authenticator by requiring a non-empty result from LDAP.
---
.../org/apache/sshd/ldap/LdapNetworkConnector.java | 75 +++++++++++++++++++++-
.../sshd/ldap/LdapPasswordAuthenticator.java | 6 +-
.../org/apache/sshd/ldap/LdapEncodingTest.java | 47 ++++++++++++++
.../sshd/ldap/LdapPasswordAuthenticatorTest.java | 10 ++-
.../sshd/ldap/LdapPublickeyAuthenticatorTest.java | 14 ++--
5 files changed, 137 insertions(+), 15 deletions(-)
diff --git a/sshd-ldap/src/main/java/org/apache/sshd/ldap/LdapNetworkConnector.java b/sshd-ldap/src/main/java/org/apache/sshd/ldap/LdapNetworkConnector.java
index 5be56e073..d6ce5c522 100644
--- a/sshd-ldap/src/main/java/org/apache/sshd/ldap/LdapNetworkConnector.java
+++ b/sshd-ldap/src/main/java/org/apache/sshd/ldap/LdapNetworkConnector.java
@@ -19,6 +19,7 @@
package org.apache.sshd.ldap;
+import java.nio.charset.StandardCharsets;
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.Arrays;
@@ -88,6 +89,9 @@ public class LdapNetworkConnector<C> extends NetworkConnector {
+ ",userPassword,userCertificate,cACertificate,authorityRevocationList,certificateRevocationList"
+ ",crossCertificatePair,x500UniqueIdentifier";
+ private static final char[] HEX = new char[] {
+ '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F' };
+
protected final SearchControls searchControls = new SearchControls();
protected final Map<String, Object> ldapEnv = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
protected MessageFormat bindDNPattern = new MessageFormat(DEFAULT_LDAP_BIND_DN_PATTERN);
@@ -435,13 +439,14 @@ public class LdapNetworkConnector<C> extends NetworkConnector {
String mode = Objects.toString(env.get(Context.SECURITY_AUTHENTICATION), null);
boolean anonymous = GenericUtils.isEmpty(mode) || PropertyResolverUtils.isNoneValue(mode);
if (!anonymous) {
- Object[] bindParams = { username, password };
if (!env.containsKey(Context.SECURITY_PRINCIPAL)) {
+ Object[] bindParams = { encodeDistinguishedName(username), encodeDistinguishedName(password) };
String bindDN = Objects.requireNonNull(bindDNPattern, "No bind DN pattern").format(bindParams);
env.put(Context.SECURITY_PRINCIPAL, ValidateUtils.checkNotNullAndNotEmpty(bindDN, "No bind DN"));
}
if (!env.containsKey(Context.SECURITY_CREDENTIALS)) {
+ Object[] bindParams = { username, password };
String bindPassword
= Objects.requireNonNull(bindPasswordPattern, "No bind password pattern").format(bindParams);
env.put(Context.SECURITY_CREDENTIALS, ValidateUtils.checkNotNullAndNotEmpty(bindPassword, "No bind password"));
@@ -453,13 +458,13 @@ public class LdapNetworkConnector<C> extends NetworkConnector {
protected String resolveBaseDN(C queryContext, Map<?, ?> ldapConfig, String username, String password)
throws NamingException {
- Object[] bindParams = { username, password };
+ Object[] bindParams = { encodeDistinguishedName(username), encodeDistinguishedName(password) };
return Objects.requireNonNull(baseDNPattern, "No base DN pattern").format(bindParams);
}
protected String resolveSearchFilter(C queryContext, Map<?, ?> ldapConfig, String username, String password)
throws NamingException {
- Object[] bindParams = { username, password };
+ Object[] bindParams = { encodeFilter(username), encodeFilter(password) };
return Objects.requireNonNull(searchFilterPattern, "No search filter pattern").format(bindParams);
}
@@ -552,4 +557,68 @@ public class LdapNetworkConnector<C> extends NetworkConnector {
return attrVal.toString();
}
+
+ /**
+ * Encodes a value following RFC 4515 for use in an LDAP filter template.
+ *
+ * @param value to encode
+ * @return the encoded value
+ * @see <a href="https://www.rfc-editor.org/info/rfc4515/">RFC 4515</a>
+ */
+ public static String encodeFilter(String value) {
+ if (value == null) {
+ return "";
+ }
+ byte[] utf8 = value.getBytes(StandardCharsets.UTF_8);
+ StringBuilder sb = new StringBuilder(utf8.length);
+ for (byte b : utf8) {
+ int ch = b & 0xFF;
+ if (ch < ' ' || ch == '*' || ch == '(' || ch == ')' || ch == '\\' || ch >= 0x7F) {
+ sb.append('\\').append(HEX[ch >>> 4]).append(HEX[ch & 0x0F]);
+ } else {
+ sb.append((char) ch);
+ }
+ }
+ return sb.toString();
+ }
+
+ /**
+ * Encodes a value following RFC 4514 for use in an LDAP Distinguished Name.
+ *
+ * @param value to encode
+ * @return the encoded value
+ * @see <a href="https://www.rfc-editor.org/info/rfc4514/">RFC 4514</a>
+ */
+ public static String encodeDistinguishedName(String value) {
+ if (value == null) {
+ return "";
+ }
+ // Note: javax.naming.ldap.Rdn.escapeValue() does not escape NUL nor other control characters. It escapes _all_
+ // leading and trailing spaces (and \r), and _all_ hashes. It implements RFC 2553, which didn't mention NUL and
+ // which was obsoleted by RFC 4514.
+ byte[] utf8 = value.getBytes(StandardCharsets.UTF_8);
+ StringBuilder sb = new StringBuilder(utf8.length);
+ for (int i = 0; i < utf8.length; i++) {
+ int ch = utf8[i] & 0xFF;
+ if (ch == ' ') {
+ if (i == 0 || i == utf8.length - 1) {
+ sb.append('\\');
+ }
+ sb.append((char) ch);
+ } else if (ch == '#') {
+ if (i == 0) {
+ sb.append('\\');
+ }
+ sb.append((char) ch);
+ } else if (ch == '"' || ch == '+' || ch == ',' || ch == ';' || ch == '<' || ch == '>' || ch == '\\') {
+ sb.append('\\').append((char) ch);
+ } else if (ch < ' ' || ch >= 0x7F) {
+ sb.append('\\').append(HEX[ch >>> 4]).append(HEX[ch & 0x0F]);
+ } else {
+ sb.append((char) ch);
+ }
+ }
+ return sb.toString();
+ }
+
}
diff --git a/sshd-ldap/src/main/java/org/apache/sshd/ldap/LdapPasswordAuthenticator.java b/sshd-ldap/src/main/java/org/apache/sshd/ldap/LdapPasswordAuthenticator.java
index 3b3945ca2..bb7ba36bb 100644
--- a/sshd-ldap/src/main/java/org/apache/sshd/ldap/LdapPasswordAuthenticator.java
+++ b/sshd-ldap/src/main/java/org/apache/sshd/ldap/LdapPasswordAuthenticator.java
@@ -75,10 +75,6 @@ public class LdapPasswordAuthenticator extends LdapAuthenticator implements Pass
protected boolean authenticate(
String username, String password, ServerSession session, Map<String, ?> attrs) {
- /*
- * By default we assume that the user + password are the same for accessing the LDAP as the user's account, so
- * the very LDAP query success is enough
- */
- return true;
+ return attrs != null && !attrs.isEmpty();
}
}
diff --git a/sshd-ldap/src/test/java/org/apache/sshd/ldap/LdapEncodingTest.java b/sshd-ldap/src/test/java/org/apache/sshd/ldap/LdapEncodingTest.java
new file mode 100644
index 000000000..afff7e964
--- /dev/null
+++ b/sshd-ldap/src/test/java/org/apache/sshd/ldap/LdapEncodingTest.java
@@ -0,0 +1,47 @@
+/*
+ * 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.sshd.ldap;
+
+import org.apache.sshd.util.test.JUnitTestSupport;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+@Tag("NoIoTestCase")
+public class LdapEncodingTest extends JUnitTestSupport {
+
+ @Test
+ void testEncodeFilter() {
+ assertEquals("\\00\\7F\\C3\\BF\\2A\\28\\29\\5Ca0B\\C3\\A4\\C3\\A9",
+ LdapNetworkConnector.encodeFilter("\000\177\377*()\\a0Bäé"));
+ assertEquals("Lu\\C4\\8Di\\C4\\87", LdapNetworkConnector.encodeFilter("Lučić")); // From RFC 4515
+ }
+
+ @Test
+ void testEncodeDN() {
+ assertEquals("J. Smith", LdapNetworkConnector.encodeDistinguishedName("J. Smith"));
+ assertEquals("James \\\"Jim\\\" Smith", LdapNetworkConnector.encodeDistinguishedName("James \"Jim\" Smith"));
+ assertEquals("Before\\0DAfter", LdapNetworkConnector.encodeDistinguishedName("Before\rAfter"));
+ assertEquals("Lu\\C4\\8Di\\C4\\87", LdapNetworkConnector.encodeDistinguishedName("Lučić"));
+ assertEquals("J\\C3\\B6rg M\\C3\\BCller", LdapNetworkConnector.encodeDistinguishedName("Jörg Müller"));
+ assertEquals("\\#foo", LdapNetworkConnector.encodeDistinguishedName("#foo"));
+ assertEquals("\\ foo \\ ", LdapNetworkConnector.encodeDistinguishedName(" foo "));
+ assertEquals("\\ foo\\00\\+\\,\\;\\<\\>\\\\bar\\ ",
+ LdapNetworkConnector.encodeDistinguishedName(" foo\000+,;<>\\bar "));
+ }
+}
diff --git a/sshd-ldap/src/test/java/org/apache/sshd/ldap/LdapPasswordAuthenticatorTest.java b/sshd-ldap/src/test/java/org/apache/sshd/ldap/LdapPasswordAuthenticatorTest.java
index 37ea3cbad..7414bc3a7 100644
--- a/sshd-ldap/src/test/java/org/apache/sshd/ldap/LdapPasswordAuthenticatorTest.java
+++ b/sshd-ldap/src/test/java/org/apache/sshd/ldap/LdapPasswordAuthenticatorTest.java
@@ -35,9 +35,6 @@ import org.junit.jupiter.api.TestMethodOrder;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mockito;
-import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertTrue;
-
/**
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
@@ -73,6 +70,13 @@ public class LdapPasswordAuthenticatorTest extends BaseAuthenticatorTest {
usersMap.forEach((username, password) -> {
outputDebugMessage("Authenticate: user=%s, password=%s", username, password);
assertTrue(auth.authenticate(username, password, session), "Failed to authenticate " + username);
+ String otherPassword = password + "bar";
+ outputDebugMessage("Authenticate: user=%s, password=%s", username, otherPassword);
+ assertFalse(auth.authenticate(username, otherPassword, session), "Should not have authenticated " + username);
});
+ String otherUser = "other";
+ assertFalse(usersMap.containsKey(otherUser));
+ assertFalse(auth.authenticate(otherUser, otherUser, session), "Should not have authenticated other");
+ assertFalse(auth.authenticate("*", "*", session), "Should not have authenticated *");
}
}
diff --git a/sshd-ldap/src/test/java/org/apache/sshd/ldap/LdapPublickeyAuthenticatorTest.java b/sshd-ldap/src/test/java/org/apache/sshd/ldap/LdapPublickeyAuthenticatorTest.java
index 827e2692f..07c788158 100644
--- a/sshd-ldap/src/test/java/org/apache/sshd/ldap/LdapPublickeyAuthenticatorTest.java
+++ b/sshd-ldap/src/test/java/org/apache/sshd/ldap/LdapPublickeyAuthenticatorTest.java
@@ -19,6 +19,8 @@
package org.apache.sshd.ldap;
+import java.security.KeyPair;
+import java.security.KeyPairGenerator;
import java.security.PublicKey;
import java.util.Collections;
import java.util.Comparator;
@@ -43,9 +45,6 @@ import org.junit.jupiter.api.TestMethodOrder;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mockito;
-import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertTrue;
-
/**
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
@@ -88,12 +87,19 @@ public class LdapPublickeyAuthenticatorTest extends BaseAuthenticatorTest {
auth.setKeyAttributeName(TEST_ATTR_NAME);
auth.setRetrievedAttributes(TEST_ATTR_NAME);
+ KeyPairGenerator gen = KeyPairGenerator.getInstance("RSA");
+ gen.initialize(1024);
+ KeyPair bogus = gen.generateKeyPair();
ServerSession session = Mockito.mock(ServerSession.class);
outputDebugMessage("%s: %s", getCurrentTestName(), auth);
KEYS_MAP.forEach((username, key) -> {
outputDebugMessage("Authenticate: user=%s, key-type=%s, fingerprint=%s",
username, KeyUtils.getKeyType(key), KeyUtils.getFingerPrint(key));
- assertTrue(auth.authenticate(username, key, session), "Failed to authenticate user=" + username);
+ assertTrue(auth.authenticate(username, key, session), "Failed to authenticate user = " + username);
+ assertFalse(auth.authenticate(username, bogus.getPublic(), session),
+ "Should not have authenticated user = " + username);
+ assertFalse(auth.authenticate("other", key, session), "Should not have authenticated user = other");
+ assertFalse(auth.authenticate("*", key, session), "Should not have authenticated user = *");
});
}
}