Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-3118

Potential LDAP injection bug

    XMLWordPrintable

Details

    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 bazzargh created issue -

          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.

          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.
          kohsuke Kohsuke Kawaguchi made changes -
          Field Original Value New Value
          Status Open [ 1 ] In Progress [ 3 ]
          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.

          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.

          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.

          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.
          scm_issue_link SCM/JIRA link daemon made changes -
          Resolution Fixed [ 1 ]
          Status In Progress [ 3 ] Resolved [ 5 ]

          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.

          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.
          abayer Andrew Bayer made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          rtyler R. Tyler Croy made changes -
          Workflow JNJira [ 133191 ] JNJira + In-Review [ 201980 ]

          People

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

            Dates

              Created:
              Updated:
              Resolved: