For context: I am developing a web application where users need to authenticate to view internal documents. I neither need any detailed info on users nor special permission management, two states are sufficient: Either a session belongs to an authenticated user (→ documents can be accessed) or it does not (→ documents cannot be accessed). A user authenticates by providing a username and a password, which I want to check against an LDAP server.
I am using Python 3.10 and the ldap3 Python library.
I am currently using the following code to authenticate a user:
#!/usr/bin/env python3
import ssl
from ldap3 import Tls, Server, Connection
from ldap3.core.exceptions import LDAPBindError, LDAPPasswordIsMandatoryError
def is_valid(username: str, password: str) -> bool:
tls_configuration = Tls(validate=ssl.CERT_REQUIRED)
server = Server("ldaps://ldap.example.com", tls=tls_configuration)
user_dn = f"cn={username},ou=ops,dc=ldap,dc=example,dc=com"
try:
with Connection(server, user=user_dn, password=password):
return True
except (LDAPBindError, LDAPPasswordIsMandatoryError):
return False
If you want to run this code, you could try using the FreeIPA's project demo LDAP server.
CERT_REQUIRED with CERT_NONE because the server only provides a self-signed cert (this obviously is a security flaw, but required to use this particular demo – the server I want to use uses a Let's Encrypt certificate)."ldaps://ldap.example.com" with ldaps://ipa.demo1.freeipa.orguser_dn with f"uid={username},cn=users,cn=accounts,dc=demo1,dc=freeipa,dc=org"After doing so, you could try running the following commands:
>>> is_valid("admin", "Secret123")
True
>>> is_valid("admin", "Secret1234")
False
>>> is_valid("admin", "")
False
>>> is_valid("admin", None)
False
>>> is_valid("nonexistent", "Secret123")
False
Does the code above safely determine if a user has provided valid credentials?
Notably, I am concerned about the following particular aspects:
with statement should only be executed if binding was successful and therefore returns True without further ado. Is this safe? Or could it be possible that binding succeeds but the password provided would still be considered wrong and not sufficient to authenticate the user against the web app.user_dn = f"cn={username},ou=ops,dc=ldap,dc=example,dc=com" uses the untrusted username (that came directly from the web form) to build a string. That basically screams LDAP injection.Also, of course, if there is anything else unsafe about my code, I'd be happy to know what it is.
I've already searched for answers to the particular aspects. Sadly, I have found nothing definite (i.e. no one definitely saying something I do here is bad or good), but I wanted to provide them as a starting point for a potential answer:
ldap3 documentation of the Connection object does mention one should use escape_rdn when binding with an untrusted username. This is at odds with my suppositions, who's right?Is TLS properly configured?
Yes, ldap3/core/tls.py looks good to me – it uses ssl.create_default_context() when supported, which loads the system default CA certificates and sets up sensible TLS defaults, so you shouldn't need any manual configuration in your program.
(Although it is a little weird that ldap3 does implement custom TLS hostname checking instead of relying on the ssl module's check_hostname, but perhaps the LDAP-over-TLS spec defines wildcard matching rules that are slightly incompatible with the usual HTTP-over-TLS ones...)
Is attempting to bind to the LDAP server enough to verify credentials?
Yes, as 'bind' is literally the "login" operation for an LDAP server.
Strictly speaking, it is not meant for credential verification – rather, it's for logging in to the LDAP directory itself (much as you would log in to a MySQL database server) – but it nevertheless works for that purpose, and many applications already use it in exactly that way (not only webapps, but also e.g. pam_ldap for Linux OS-level authentication).
That is, many applications rely on this behavior, and I've never heard of any server where the bind result would be deferred until another operation.
I'd be a little more worried about the 'ldap3' module, as in my experience initializing a Connection did not attempt to connect – much less bind – to the server until I explicitly called .bind() (or unless I specified auto_bind=True), but if your example works then I assume that using a with block does this correctly.
My own old code holds a persistent connection anyway, so I've never attempted to use 'with' – I've used this, but it may be outdated:
conn = ldap3.Connection(server, raise_exceptions=True)
conn.bind()
For some simple apps, I use Apache as a reverse proxy and its mod_auth_ldap handles LDAP authentication for me (especially when the app doesn't care about which user and only needs a general "is authenticated" check).
Am I opening myself up to injection attacks? If so, how to properly mitigate them?
Injection attacks in the 'bind' operation are not easily exploitable, as the bind DN isn't a free-form query – it is literally the "user name" field and it must exactly match an existing entry; it doesn't take wildcards or anything like that.
So even if someone tries to play injection tricks with the 'bind' DN – e.g. if someone provides Foo,OU=Admins as their username, resulting in UID=[Foo,OU=Admins],OU=Users, as the DN – that should not lead to any security issue because they would still have to provide the correct password for that account.
Still, it's easy to mitigate this; any good LDAP library will have a function to escape DN component values:
ldap3.utils.dn.escape_rdn(string)ldap.dn.escape_dn_chars(string)See also the bottom of this post for an alternative approach to constructing DNs without using string templates (the build_dn() function that I've used in some projects).
I actually dislike the "DN template" approach for a completely different reason: It only works with LDAP directories that use a rigid, flat hierarchy where all of your accounts are under the same OU, and only when they're named after their username attribute.
While your directory might have such a flat hierarchy, that's not guaranteed to be the case. In fact, with something like Microsoft Active Directory it will never be the case, as the user account entries will be named after CN=Full Name Jr. and will almost always be scattered across many OUs.
AD has a (non-standard) shortcut where it accepts faux-DNs in the shape of "user@domain", but more generally a two-step approach is used for LDAP credential checking:
Bind using your app's service credentials, then search the directory for any "user" entries that have the username in their uid attribute, or similar, and verify that you found exactly one entry;
Unbind (optional?), then bind again with the user's found DN and the provided password.
This is much more flexible, although you do have to worry about LDAP filter injection attacks when performing a search – a username like foo)(uid=* will give undesirable results. But your LDAP module also has a function to escape values for use in search filters:
ldap3.utils.conv.escape_filter_chars(string)ldap.filter.escape_filter_chars(string)(Note that escaping rules for filter values are different from those for DN values, so you need to use the correct one for the specific context.)
python-ldap also has a convenient wrapper ldap.filter.filter_format around this, but it's basically just the_filter % tuple(map(escape_filter_chars, args)).
Aside from escape_filter_chars(), it is also a good idea for the program to verify that the search results match exactly 1 entry (not "at least 1 entry").
An alternative approach instead of manually escaping DN templates:
dn = build_dn({"CN": f"{last}, {first} ({username})"},
{"OU": "Faculty of Foo and Bar (XYZF)"},
{"OU": "Staff"},
ad.BASE_DN)
Implemented as:
def build_dn(*args):
components = []
for rdn in args:
if isinstance(rdn, dict):
# Replace with ldap3.utils.dn.escape_rdn(v) if needed
rdn = [(a, ldap.dn.escape_dn_chars(v))
for a, v in rdn.items()]
rdn.sort()
rdn = "+".join(["%s=%s" % av for av in rdn])
components.append(rdn)
elif isinstance(rdn, str):
components.append(rdn)
else:
raise ValueError("Unacceptable RDN type for %r" % (rdn,))
return ",".join(components)