Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] ldap config setting custom_attr_allowlist does not do anything. #2032

Closed
Martin-Kemp opened this issue Aug 17, 2022 · 13 comments
Closed
Labels
bug Something isn't working

Comments

@Martin-Kemp
Copy link
Contributor

Martin-Kemp commented Aug 17, 2022

What is the bug?
Ldap config setting custom_attr_allowlist which is supposed to limit the attributes searched for does not appear to do anything.

How can one reproduce the bug?

  1. Download the example docker-compose files here: https://opensearch.org/docs/2.2/security-plugin/configuration/ldap/
  2. Add some custom attributes to the config.yml file.
  3. docker-compose up
  4. Run a query against the api. The example in the docs will do.
  5. Check logs of the ldap container.
  6. You'll notice the search does not limit the attributes:
62fcbc4b conn=1013 op=1 SRCH base="ou=People,dc=example,dc=org" scope=2 deref=3 filter="(cn=psantos)"
62fcbc4b conn=1013 op=1 SRCH attr=* +

What is the expected behavior?
Attributes listed in custom_attr_allowlist should be filtered out, this should be evident in the ldap logs. Expected output:

62fcbc4b conn=1013 op=1 SRCH base="ou=People,dc=example,dc=org" scope=2 deref=3 filter="(cn=psantos)"
62fcbc4b conn=1013 op=1 SRCH attr=attribute1

What is your host/environment?

  • OS: Ubuntu WSL2. Also tested on other distros
@Martin-Kemp Martin-Kemp added bug Something isn't working untriaged Require the attention of the repository maintainers and may need to be prioritized labels Aug 17, 2022
@peternied
Copy link
Member

@Martin-Kemp Thanks for filing this issue

@peternied peternied removed the untriaged Require the attention of the repository maintainers and may need to be prioritized label Aug 17, 2022
@Martin-Kemp
Copy link
Contributor Author

@peternied no problem. Let me know if it's not clear enough or you can't reproduce it.

@Martin-Kemp
Copy link
Contributor Author

It looks like the custom_attr_allowlist doesn't end up in the user object's attributes.

I ran the test testCustomAttributes:

settings = Settings.builder()
        .putList(ConfigConstants.LDAP_HOSTS, "127.0.0.1:4", "localhost:" + ldapPort)
        .put(ConfigConstants.LDAP_AUTHC_USERSEARCH, "(uid={0})")
        .putList(ConfigConstants.LDAP_CUSTOM_ATTR_WHITELIST, "*objectclass*","entryParentId")
        .build();

user = (LdapUser) new LDAPAuthenticationBackend(settings, null).authenticate(new AuthCredentials("jacksonm", "secret"
        .getBytes(StandardCharsets.UTF_8)));

Assert.assertEquals(user.getCustomAttributesMap().toString(), 2, user.getCustomAttributesMap().size());

Then forcing it to fail with:

Assert.assertEquals(user.getCustomAttributesMap().toString(), 1, user.getCustomAttributesMap().size());

We can then see the custom attributes *objectclass* and entryParentId don't appear in the users object's attributes:

java.lang.AssertionError: {ldap.original.username=jacksonm, ldap.dn=cn=Michael Jackson,ou=people,o=TEST} expected:<1> but was:<2>

@peternied
Copy link
Member

@Martin-Kemp I think I found the source of the issue while doing code inspection, the value is custom_attr_whitelist in the codebase- can you adjust the setting and see if this resolves your issue?

Assuming this lets you work around it, we need to update the key to the inclusive terminology allowlist and we need to fix the documentation until this variant is avaliable.

@Martin-Kemp
Copy link
Contributor Author

@peternied Thanks for the suggestion, I have tried using custom_attr_whitelist too but still the same issue:

62ff304d conn=1003 op=1 SRCH base="ou=People,dc=example,dc=org" scope=2 deref=3 filter="(cn=psantos)"
62ff304d conn=1003 op=1 SRCH attr=* +

The problem is that SRCH attr=* + shows that we're still requesting all attributes from the ldap server.

Could you try to reproduce the issue on your side? It's quite quick to do with the zip file supplied in the documentation. Just add:

custom_attr_whitelist:
              - attribute1

to config.dynamic.authz.ldap_roles.authorization_backend.config in the config.yml file. Like this:

---
_meta:
  type: "config"
  config_version: 2

config:
  dynamic:
    http:
      anonymous_auth_enabled: false
    authc:
      internal_auth:
        order: 0
        description: "HTTP basic authentication using the internal user database"
        http_enabled: true
        transport_enabled: true
        http_authenticator:
          type: basic
          challenge: false
        authentication_backend:
          type: internal
      ldap_auth:
        order: 1
        description: "Authenticate using LDAP"
        http_enabled: true
        transport_enabled: true
        http_authenticator:
          type: basic
          challenge: false
        authentication_backend:
          type: ldap
          config:
            enable_ssl: false
            enable_start_tls: false
            enable_ssl_client_auth: false
            verify_hostnames: true
            hosts:
            - openldap:389
            bind_dn: cn=readonly,dc=example,dc=org
            password: changethistoo
            userbase: ou=People,dc=example,dc=org
            usersearch: (cn={0})
            username_attribute: cn

    authz:
      ldap_roles:
        description: "Authorize using LDAP"
        http_enabled: true
        transport_enabled: true
        authorization_backend:
          type: ldap
          config:
            enable_ssl: false
            enable_start_tls: false
            enable_ssl_client_auth: false
            verify_hostnames: true
            hosts:
            - openldap:389
            bind_dn: cn=readonly,dc=example,dc=org
            password: changethistoo
            userbase: ou=People,dc=example,dc=org
            usersearch: (cn={0})
            username_attribute: cn
            skip_users:
              - admin
              - kibanaserver
            rolebase: ou=Groups,dc=example,dc=org
            rolesearch: (uniqueMember={0})
            userroleattribute: null
            userrolename: disabled
            rolename: cn
            resolve_nested_roles: false
            custom_attr_whitelist:
              - attribute1

Also change the version of opensearch in the docker-compose file, it's outdated (I opened an issue about that too: opensearch-project/documentation-website#905)

Then just check the logs of the ldap container when you run the query supplied in the documentation.

@Martin-Kemp
Copy link
Contributor Author

I also haven't been able to get this working with Opendistro either. So I'm guessing the issue predates the change to inclusive terminology.

@cwperks
Copy link
Member

cwperks commented Aug 23, 2022

@Martin-Kemp From what I've been able to put together of tracing through the codepath it looks like searching all attributes is expected. See LdapHelper.search. This is the method used to locate the user and return an LdapEntry.

The custom_attr_whitelist and custom_attr_maxval_len are used to filter attributes after they are retrieved. See LdapUser.extractLdapAttributes.

This in turn is used to enrich the user object by adding these custom attributes to User. See LdapAuthenticationBackend.exists

I also had to add custom_attr_whitelist under authc not under authz.

See the output below of a curl command to /_plugins/_security/api/account before have custom_attr_whitelist and then adding:

custom_attr_whitelist:
    - givenName
    - cn
    - mail

Screen Shot 2022-08-23 at 1 48 39 PM

Notice that custom_attribute_names is filtered with the values from the allow list.

@Martin-Kemp
Copy link
Contributor Author

Thanks for the clarification @cwperks. So then I misunderstood the documentation.

Our need is to limit the attributes requested by Opensearch because some attributes may contain sensitive information.

We can do this with ldapsearch for example:

$ ldapsearch -x -H "ldap://localhost:389" -D "cn=readonly,dc=example,dc=org" -w changethistoo "cn=psantos" testAttr
# extended LDIF
#
# LDAPv3
# base <> (default) with scope subtree
# filter: cn=psantos
# requesting: testAttr
#

ldap logs:

6305282c conn=1023 op=1 SRCH base="" scope=2 deref=0 filter="(cn=psantos)"
6305282c conn=1023 op=1 SRCH attr=testAttr

So if filtering on the client side (Opensearch) is expected behaviour then I suppose this feature is not supported.

@Martin-Kemp
Copy link
Contributor Author

I also had to add custom_attr_whitelist under authc not under authz.

In the documentation it's under authz, maybe that should be changed then.

@Martin-Kemp
Copy link
Contributor Author

See LdapHelper.search. This is the method used to locate the user and return an LdapEntry.

Yeah, you're right. If I hard code an attribute there it works as I expected:

request.setReturnAttributes("testAttr");

Then rebuilding results in ldap logs:

6305d15a conn=1000 op=1 SRCH base="ou=People,dc=example,dc=org" scope=2 deref=3 filter="(cn=psantos)"
6305d15a conn=1000 op=1 SRCH attr=testAttr

I wonder if supporting custom return attributes would be helpful to anyone else. Maybe I could do this and open a PR?

@cwperks
Copy link
Member

cwperks commented Aug 24, 2022

@Martin-Kemp That would be welcomed. We would gladly accept a PR with your contribution.

@Martin-Kemp
Copy link
Contributor Author

@cwperks, I submitted a PR, would you mind having a look? #2093

@Martin-Kemp
Copy link
Contributor Author

#2093 has been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants