Details
-
Bug
-
Status: Closed (View Workflow)
-
Major
-
Resolution: Fixed
-
None
-
Platform: All, OS: All
Description
In: ActiveDirectoryUnixAuthenticationProvider.java
Line 85:
context.search(toDC(domainName),"(&
(userPrincipalName="principalName")(objectClass=user))", controls);
Line 89:
renum = context.search(toDC(domainName),"(&
(sAMAccountName="username")(objectClass=user))", controls);
String appends with no escaping leave the code vulnerable to an (unlikely) LDAP
injection attack.
The Acegi code already has protection against this, you can see how that's
implemented here:
https://src.springframework.org/svn/spring-security/tags/release_1_0_6/core/src/main/java/org/acegisecurity/ldap/search/FilterBasedLdapUserSearch.java
(this eventually ends up using the spring code for encoding, in
org/springframework/ldap/support/LdapEncoder.java)
If you use the LdapTemplate code to do this, as acegi did, see the notes about
referrals at:
http://static.springframework.org/spring-ldap/docs/1.1/api/org/springframework/ldap/LdapTemplate.html
Incidentally, this referrals stuff should be irrelevant for authentication; if
you connect to the Global Catalog Server (port 3268) for your forest, not the AD
server (port 389), you should not be seeing referrals. Hudson is querying
_ldap._tcp.<domain name>, but should probably be querying
_gc._tcp.<DnsForestName>. (equivalent, I think, to COM4J.getObject(IADs.class,
"GC:", null); in the windows version, instead of COM4J.getObject(IADs.class,
"LDAP://RootDSE", null)
Attachments
Activity
bazzargh
added a comment - The actual escaping doesn't happen in FilterBasedLdapUserSearch, but it's where
acegi calls spring-ldap rather than use raw JNDI, delegating the construction of
the search string:
LdapUserDetailsImpl.Essence user = (LdapUserDetailsImpl.Essence)
template.searchForSingleEntry(searchBase,
searchFilter, new String[]
, userDetailsMapper);
user.setUsername(username);
return user.createUserDetails();
In the above, 'searchFilter' looks like:
"(&(userPrincipalName=
)(objectClass=user))". This doesn't get combined with
the escaped username until several layers of indirection later.
The LdapTemplate code also takes care of things like ensuring only one result is
returned by the search. however, if you just want to escape the string and
ignore the rest of the spring stuff, this should do what you want:
"(&(userPrincipalName=" + LdapEncoder.filterEncode(principalName) +
")(objectClass=user))"
As for the referrals: I'll raise a separate bug later today with a patch.
bazzargh
added a comment - The actual escaping doesn't happen in FilterBasedLdapUserSearch, but it's where
acegi calls spring-ldap rather than use raw JNDI, delegating the construction of
the search string:
LdapUserDetailsImpl.Essence user = (LdapUserDetailsImpl.Essence)
template.searchForSingleEntry(searchBase,
searchFilter, new String[]
{username}
, userDetailsMapper);
user.setUsername(username);
return user.createUserDetails();
In the above, 'searchFilter' looks like:
"(&(userPrincipalName=
{0}
)(objectClass=user))". This doesn't get combined with
the escaped username until several layers of indirection later.
The LdapTemplate code also takes care of things like ensuring only one result is
returned by the search. however, if you just want to escape the string and
ignore the rest of the spring stuff, this should do what you want:
"(&(userPrincipalName=" + LdapEncoder.filterEncode(principalName) +
")(objectClass=user))"
As for the referrals: I'll raise a separate bug later today with a patch. Code changed in hudson
User: : kohsuke
Path:
trunk/hudson/plugins/active-directory/src/main/java/hudson/plugins/active_directory/ActiveDirectoryUnixAuthenticationProvider.java
http://jenkins-ci.org/commit/31442
Log:
[FIXED JENKINS-3118] Fixed a possible LDAP injection problem.
Code changed in hudson
User: : kohsuke
Path:
trunk/hudson/plugins/active-directory/src/main/java/hudson/plugins/active_directory/ActiveDirectoryUnixAuthenticationProvider.java
http://jenkins-ci.org/commit/31443
Log:
[FIXED JENKINS-3118] Fixed a possible LDAP injection problem.
I looked at the FilterBasedLdapUserSearch.java source code you cited, but I
didn't find anything in there that does the escaping. Can you help me find where
it is?
And I'm afraid I know nothing about AD forest. If you know what you are talking
about (which I assume you are), please send in the patch and I'd be happy to
apply it.
I thought the recent version of the AD plugin handles referrals correctly.