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 <[email protected]> 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:[email protected]";>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:[email protected]";>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 = *"); }); } }
