wangzhigang1999 commented on code in PR #7761:
URL: https://github.com/apache/kyuubi/pull/7761#discussion_r4120726204
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapSearch.scala:
##########
@@ -59,7 +59,7 @@ class LdapSearch(conf: KyuubiConf, ctx: DirContext) extends
DirSearch with Loggi
var allLdapNames: Array[String] = null
if (LdapUtils.isDn(user)) {
val userBaseDn: String = LdapUtils.extractBaseDn(user)
- val userRdn: String = LdapUtils.extractFirstRdn(user)
+ val userRdn: String =
LdapUtils.escapeLDAPSearchFilter(LdapUtils.extractFirstRdn(user))
Review Comment:
The escaping looks right overall — I confirmed the reported bypass is fixed
(the same auth test fails on base and passes here), and `replaceAll` →
`replace` is a good catch.
One regression in `findUserDn`:
```scala
val userRdn: String =
LdapUtils.escapeLDAPSearchFilter(LdapUtils.extractFirstRdn(user))
```
This escapes the *serialized* RDN. For a user named `Alice+Ops` the DN is
`cn=Alice\+Ops,...`, where the backslash is DN syntax rather than part of the
value, so this produces `(cn=Alice\5c+Ops)` and searches for a value containing
a literal backslash. With a bind user configured the user can no longer be
found; direct bind is unaffected. Reproduced against the embedded server:
```
AssertionError: Authentication failed for user
'cn=Alice\+Ops,ou=People,dc=example,dc=com'
```
Could we decode the RDN before escaping the value?
```scala
def firstRdnAsFilterFragment(dn: String): String =
try {
val name = new LdapName(dn)
val rdn = name.getRdn(name.size() - 1)
s"${rdn.getType}=${escapeLDAPSearchFilter(String.valueOf(rdn.getValue))}"
} catch {
case _: InvalidNameException => null
}
```
With that change the LDAP suites pass 119/119 and
`FindUserDnWhenUserDnEscapesSpecialCharacters` still passes, so a
client-supplied `CN=User1*` is still escaped to `(CN=User1\2a)`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]