[bitnami/openldap] using `LDAP_REQUIRE_TLS=yes` does not actually require clients to use TLS

cfryanr commented 1 year ago

Name and Version

bitnami/openldap:2.6.5

What architecture are you using?

amd64

What steps will reproduce the bug?

When deploying bitnami/openldap while using LDAP_REQUIRE_TLS=yes, I am still able to connect to the server from a client which is using ldap:// without TLS.

For example, from within another container running in the same Kubernetes cluster, I can use a command like ldapsearch -x -H 'ldap://ldap.tools.svc.cluster.local' -D 'cn=admin,dc=pinniped,dc=dev' -w password -b 'dc=pinniped,dc=dev' and it returns a successful query result showing all my users and groups.

What is the expected behavior?

I would expect to get an error like this, because the server is supposed to be requiring the use of TLS.

# ldapsearch -x -H 'ldap://ldap.tools.svc.cluster.local' -D 'cn=admin,dc=pinniped,dc=dev' -w password -b 'dc=pinniped,dc=dev'
ldap_bind: Confidentiality required (13)
    additional info: TLS confidentiality required

What do you see instead?

Instead, I see a successful query result showing all my users and groups.

Additional information

As an experiment, I tried changing this line to instead say dn: olcDatabase={2}mdb,cn=config and it seems to fix the problem. After building the container image again and redeploying, then the ldapsearch clients get the expected TLS confidentiality required error when they are not using either TLS or StartTLS. I am not enough of an LDAP expert to know if this is the most appropriate solution, but it does seem to work.

Note that I would have tried this with bitnami/openldap:2.6.6 but I ran into this other bug https://github.com/bitnami/containers/issues/49575 which prevented me from being able to try it. Since 2.6.5 did not contain the bug described in https://github.com/bitnami/containers/issues/49575, I used 2.6.5. However, I suspect 2.6.6 would suffer from the same problem that I am reporting here.

cfryanr commented 1 year ago

In case it helps, I don't think need multiple containers to reproduce this. You can exec into the openldap container itself, which includes bash and ldapsearch, and run the command there to reproduce the problem and test the fix.

juan131 commented 1 year ago

Hi @cfryanr

Thanks so much for reporting this bug! I just wanted to reach out to you to let you know that we are monitoring it. One of my colleagues who has a lot experience with LDAP is on PTO and I'd like to double-check with him the best approach we can follow to address it before taking any decision.

I'll keep you updated and contact you as soon as we have a resolution. Thanks for your patience.

cfryanr commented 1 year ago

Thanks for the update. When they return, hopefully they can also look at the related https://github.com/bitnami/containers/issues/49575 bug too.

rafariossaa commented 1 year ago

Hi @cfryanr, Thanks for spotting this out, the change you indicated I think it is the right one. As you proposed a fix for this issue, would you like to send a PR ? We will be glad to review and merge it.

cfryanr commented 1 year ago

@rafariossaa I'd be happy to attempt to put together a PR.

Is there a test suite that I could enhance as part of the PR? It would be nice to prevent this from regressing again in the future.

I don't think I can submit a PR that will fix this until the related bug is fixed by https://github.com/bitnami/containers/pull/51911. Before that fix, using LDAP_REQUIRE_TLS=yes currently just causes the container to fail to start due to an error in creating the LDAP tree, whether you are using custom LDIFs or not.

cfryanr commented 1 year ago

Actually, looking at this in more depth, I think I misunderstood when the LDAP_REQUIRE_TLS env var feature was added.

❯ docker run --rm -it bitnami/openldap:2.6.6 cat /opt/bitnami/scripts/libopenldap.sh | grep LDAP_REQUIRE_TLS
export LDAP_REQUIRE_TLS="${LDAP_REQUIRE_TLS:-no}"
            if is_boolean_yes "$LDAP_REQUIRE_TLS"; then
❯ docker run --rm -it bitnami/openldap:2.6.5 cat /opt/bitnami/scripts/libopenldap.sh | grep LDAP_REQUIRE_TLS

As you can see from the output above, the script never mentioned LDAP_REQUIRE_TLS until 2.6.6. It does not work in 2.6.5 because it was not a feature yet.

It seems to have been added very recently in this PR: https://github.com/bitnami/containers/pull/44732.

Sorry for the confusion. I will close this issue for now. I'll try again after https://github.com/bitnami/containers/pull/51911 is merged and released.

bitnami / containers