Details

    • Similar Issues:

      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

          Hide
          kohsuke Kohsuke Kawaguchi added a comment -

          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.

          Show
          kohsuke Kohsuke Kawaguchi added a comment - 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.
          Hide
          bazzargh 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.

          Show
          bazzargh 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.
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          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.

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - 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.
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          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.

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - 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.

            People

            • Assignee:
              Unassigned
              Reporter:
              bazzargh bazzargh
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: