Uploaded image for project: 'camunda BPM'
  1. camunda BPM
  2. CAM-2309

LDAP Plug-in for groups doesn't work when user DN's have special characters

    • Icon: Bug Report Bug Report
    • Resolution: Fixed
    • Icon: L3 - Default L3 - Default
    • 7.2.0, 7.0.6, 7.1.3, 7.2.0-alpha3
    • 7.1.0
    • engine
    • None

      We are using Active Directory as our authentication backend. All of our directory entries have a "\" character in the DN like:

      CN=Howe\, David,OU=XXXXX,OU=XXXXX,cn=XXXXX,dc=XXXXX,dc=com,dc=internal

      The backslash is not an escape character, it's actually in the directory and on my site I am not in a position to change this.

      When we use the Camunda LDAP plug-in, authorizations based on groups don't work - no groups are ever returned by the LdapIdentityProviderSession.findGroupByQueryCriteria method. I have debugged this and found that the group filter that is used doesn't match anything as it isn't escaping the "\" character contained in the DN. The filter that is generated is:

      (&(objectclass=groupOfUniqueNames)(uniqueMember=CN=Howe\, David,OU=XXXXX,OU=XXXXX,cn=XXXXX,dc=XXXXX,dc=com,dc=internal))

      when it should be:

      (&(objectclass=groupOfUniqueNames)(uniqueMember=CN=Howe\5c, David,OU=XXXXX,OU=XXXXX,cn=XXXXX,dc=XXXXX,dc=com,dc=internal))

      The list of escaped characters for filters can be found here https://www.owasp.org/index.php/Preventing_LDAP_Injection_in_Java.

      I have attempted fixing this myself, but have run into a couple of problems:

      1. The version of Apache DS being used in the test cases doesn't properly support special characters (see https://issues.apache.org/jira/browse/DIRSERVER-1442) making it difficult to write a unit test that proves the escaping is working
      2. The LdapGroupQuery and LdapUserQueryImpl objects contain properties such as "firstName" and "firstNameLike". You can pass wildcards in both attributes and the query will work as expected. If you introduce blanket escaping of the filters in the LdapIdentityProviderSession class, then the wildcards get escaped as well and no longer function as wildcards. There are test cases (e.g. LdapUserQueryTest.testFilterByGroupIdAndEmailLike) that don't use the "userEmailLike" property to do a wildcard search.

      My initial thought was to escape the properties that don't contain like (e.g. "firstName") and not escape the properties that do contain like (e.g. "firstNameLike") but this didn't seem like a full solution, and changes existing functionality.

      To get around my immediate problem, escaping the user DN in LdapIdentityProviderSession.getGroupSearchFilter would suffice:

      if (query.getUserId() != null) {
      addFilter(ldapConfiguration.getGroupMemberAttribute(),
      escapeLDAPSearchFilter(getDnForUser(query.getUserId())), search);
      }

      Where escapeLDAPSearchFilter is taken from https://www.owasp.org/index.php/Preventing_LDAP_Injection_in_Java.

        This is the controller panel for Smart Panels app

            [CAM-2309] LDAP Plug-in for groups doesn't work when user DN's have special characters

            Daniel Meyer added a comment -

            Thanks David.

            This seems to be very well researched and I think we can move forward with this.

            Is there any License Information on the OWASP code? If it can be used under the terms of the Apache License we could probably include it along with the unit tests they propose.

            Do you think you could submit a pull request based on this?

            Daniel

            Daniel Meyer added a comment - Thanks David. This seems to be very well researched and I think we can move forward with this. Is there any License Information on the OWASP code? If it can be used under the terms of the Apache License we could probably include it along with the unit tests they propose. Do you think you could submit a pull request based on this? Daniel

            David Howe added a comment -

            Hi Daniel,

            There is no license information with the OWASP code - it's just a sample on their wiki page. I will create a pull request with just the fix for the group membership query. I started down the road of escaping some of the other queries, but it breaks some of the tests that have wildcards in them.

            Thanks for the quick response.

            David

            David Howe added a comment - Hi Daniel, There is no license information with the OWASP code - it's just a sample on their wiki page. I will create a pull request with just the fix for the group membership query. I started down the road of escaping some of the other queries, but it breaks some of the tests that have wildcards in them. Thanks for the quick response. David

            Daniel Meyer added a comment -

            Hi David,

            does it make sense to make this configurable? Meaning: should we add a boolean flag in the configuration allowing to turn on / off the escaping?

            Last week I had another behavior that I could not reproduce / test with the apache DS. I ended up writing unit test which ensure that the expected query is being generated:
            https://github.com/camunda/camunda-bpm-platform/commit/fd0c2fa4901d2d1383f5521314e9d44dce8ec1e4

            Daniel

            Daniel Meyer added a comment - Hi David, does it make sense to make this configurable? Meaning: should we add a boolean flag in the configuration allowing to turn on / off the escaping? Last week I had another behavior that I could not reproduce / test with the apache DS. I ended up writing unit test which ensure that the expected query is being generated: https://github.com/camunda/camunda-bpm-platform/commit/fd0c2fa4901d2d1383f5521314e9d44dce8ec1e4 Daniel

            David Howe added a comment -

            In theory, the fix that I have put in for the group query by user id should always escape special characters as you wouldn't be expecting wildcards in this query. I have put in a test case that uses brackets instead of backslashes and this works in Apache DS, but unfortunately it also works if you don't escape the brackets If you want to preserve backward compatibility then a switch might be safer.

            I will submit the pull request for the basic change now, and you can assess whether you want to put a feature toggle around it. It's a holiday long weekend here in Melbourne and I'm just about to head out.

            David Howe added a comment - In theory, the fix that I have put in for the group query by user id should always escape special characters as you wouldn't be expecting wildcards in this query. I have put in a test case that uses brackets instead of backslashes and this works in Apache DS, but unfortunately it also works if you don't escape the brackets If you want to preserve backward compatibility then a switch might be safer. I will submit the pull request for the basic change now, and you can assess whether you want to put a feature toggle around it. It's a holiday long weekend here in Melbourne and I'm just about to head out.

            Daniel Meyer added a comment -

            Awesome, thanks!

            Daniel

            Daniel Meyer added a comment - Awesome, thanks! Daniel

              smirnov Roman Smirnov
              david.howe@auspost.com.au David Howe
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Created:
                Updated:
                Resolved: