From b22038c7467698347146c6ecdcf22d592d88041d Mon Sep 17 00:00:00 2001 From: Peter Marschall Date: Sun, 29 Dec 2024 17:02:39 +0100 Subject: [PATCH 1/6] LDAP auth: a little bit of cleanup - correct grammar in some cases - we're doing authentication here, not authorization - uppercase LDAP in messages & comments - rename variable _ldap_version to _ldap_module_version to avoid misunderstanding it as LDAP's protocol version --- radicale/auth/ldap.py | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/radicale/auth/ldap.py b/radicale/auth/ldap.py index cb3858dc..80ceb448 100644 --- a/radicale/auth/ldap.py +++ b/radicale/auth/ldap.py @@ -15,11 +15,11 @@ # You should have received a copy of the GNU General Public License # along with Radicale. If not, see . """ -Authentication backend that checks credentials with a ldap server. +Authentication backend that checks credentials with a LDAP server. Following parameters are needed in the configuration: - ldap_uri The ldap url to the server like ldap://localhost - ldap_base The baseDN of the ldap server - ldap_reader_dn The DN of a ldap user with read access to get the user accounts + ldap_uri The LDAP URL to the server like ldap://localhost + ldap_base The baseDN of the LDAP server + ldap_reader_dn The DN of a LDAP user with read access to get the user accounts ldap_secret The password of the ldap_reader_dn ldap_secret_file The path of the file containing the password of the ldap_reader_dn ldap_filter The search filter to find the user to authenticate by the username @@ -43,7 +43,7 @@ class Auth(auth.BaseAuth): _ldap_secret: str _ldap_filter: str _ldap_load_groups: bool - _ldap_version: int = 3 + _ldap_module_version: int = 3 _ldap_use_ssl: bool = False _ldap_ssl_verify_mode: int = ssl.CERT_REQUIRED _ldap_ssl_ca_file: str = "" @@ -56,7 +56,7 @@ class Auth(auth.BaseAuth): except ImportError: try: import ldap - self._ldap_version = 2 + self._ldap_module_version = 2 self.ldap = ldap except ImportError as e: raise RuntimeError("LDAP authentication requires the ldap3 module") from e @@ -70,7 +70,7 @@ class Auth(auth.BaseAuth): if ldap_secret_file_path: with open(ldap_secret_file_path, 'r') as file: self._ldap_secret = file.read().rstrip('\n') - if self._ldap_version == 3: + if self._ldap_module_version == 3: self._ldap_use_ssl = configuration.get("auth", "ldap_use_ssl") if self._ldap_use_ssl: self._ldap_ssl_ca_file = configuration.get("auth", "ldap_ssl_ca_file") @@ -94,7 +94,7 @@ class Auth(auth.BaseAuth): logger.info("auth.ldap_secret : (from config)") if self._ldap_reader_dn and not self._ldap_secret: logger.error("auth.ldap_secret : (not provided)") - raise RuntimeError("LDAP authentication requires ldap_secret for reader_dn") + raise RuntimeError("LDAP authentication requires ldap_secret for ldap_reader_dn") logger.info("auth.ldap_use_ssl : %s" % self._ldap_use_ssl) if self._ldap_use_ssl is True: logger.info("auth.ldap_ssl_verify_mode : %s" % self._ldap_ssl_verify_mode) @@ -114,14 +114,14 @@ class Auth(auth.BaseAuth): """Search for the dn of user to authenticate""" res = conn.search_s(self._ldap_base, self.ldap.SCOPE_SUBTREE, filterstr=self._ldap_filter.format(login), attrlist=['memberOf']) if len(res) == 0: - """User could not be find""" + """User could not be found""" return "" user_dn = res[0][0] logger.debug("LDAP Auth user: %s", user_dn) - """Close ldap connection""" + """Close LDAP connection""" conn.unbind() except Exception as e: - raise RuntimeError(f"Invalid ldap configuration:{e}") + raise RuntimeError(f"Invalid LDAP configuration:{e}") try: """Bind as user to authenticate""" @@ -157,14 +157,14 @@ class Auth(auth.BaseAuth): server = self.ldap3.Server(self._ldap_uri) conn = self.ldap3.Connection(server, self._ldap_reader_dn, password=self._ldap_secret) except self.ldap3.core.exceptions.LDAPSocketOpenError: - raise RuntimeError("Unable to reach ldap server") + raise RuntimeError("Unable to reach LDAP server") except Exception as e: logger.debug(f"_login3 error 1 {e}") pass if not conn.bind(): - logger.debug("_login3 can not bind") - raise RuntimeError("Unable to read from ldap server") + logger.debug("_login3 cannot bind") + raise RuntimeError("Unable to read from LDAP server") logger.debug(f"_login3 bind as {self._ldap_reader_dn}") """Search the user dn""" @@ -175,8 +175,8 @@ class Auth(auth.BaseAuth): attributes=['memberOf'] ) if len(conn.entries) == 0: - logger.debug(f"_login3 user '{login}' can not be find") - """User could not be find""" + """User could not be found""" + logger.debug(f"_login3 user '{login}' cannot be found") return "" user_entry = conn.response[0] @@ -187,7 +187,7 @@ class Auth(auth.BaseAuth): """Try to bind as the user itself""" conn = self.ldap3.Connection(server, user_dn, password=password) if not conn.bind(): - logger.debug(f"_login3 user '{login}' can not be find") + logger.debug(f"_login3 user '{login}' cannot be found") return "" if self._ldap_load_groups: tmp = [] @@ -195,7 +195,7 @@ class Auth(auth.BaseAuth): tmp.append(g.split(',')[0][3:]) self._ldap_groups = set(tmp) conn.unbind() - logger.debug(f"_login3 {login} successfully authorized") + logger.debug(f"_login3 {login} successfully authenticated") return login except Exception as e: logger.debug(f"_login3 error 2 {e}") @@ -204,10 +204,10 @@ class Auth(auth.BaseAuth): def _login(self, login: str, password: str) -> str: """Validate credentials. - In first step we make a connection to the ldap server with the ldap_reader_dn credential. + In first step we make a connection to the LDAP server with the ldap_reader_dn credential. In next step the DN of the user to authenticate will be searched. In the last step the authentication of the user will be proceeded. """ - if self._ldap_version == 2: + if self._ldap_module_version == 2: return self._login2(login, password) return self._login3(login, password) From 6f82333ff7ab69d5ed8c7d6d11bbdb77f535f7cf Mon Sep 17 00:00:00 2001 From: Peter Marschall Date: Sun, 29 Dec 2024 17:18:00 +0100 Subject: [PATCH 2/6] LDAP auth: harmonize _login2() and _login3() methods --- radicale/auth/ldap.py | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/radicale/auth/ldap.py b/radicale/auth/ldap.py index 80ceb448..4833d18d 100644 --- a/radicale/auth/ldap.py +++ b/radicale/auth/ldap.py @@ -112,12 +112,18 @@ class Auth(auth.BaseAuth): conn.set_option(self.ldap.OPT_REFERRALS, 0) conn.simple_bind_s(self._ldap_reader_dn, self._ldap_secret) """Search for the dn of user to authenticate""" - res = conn.search_s(self._ldap_base, self.ldap.SCOPE_SUBTREE, filterstr=self._ldap_filter.format(login), attrlist=['memberOf']) + res = conn.search_s( + self._ldap_base, + self.ldap.SCOPE_SUBTREE, + filterstr=self._ldap_filter.format(login), + attrlist=['memberOf'] + ) if len(res) == 0: """User could not be found""" return "" - user_dn = res[0][0] - logger.debug("LDAP Auth user: %s", user_dn) + user_entry = res[0] + user_dn = user_entry[0] + logger.debug(f"_login2 found LDAP user DN {user_dn}") """Close LDAP connection""" conn.unbind() except Exception as e: @@ -132,11 +138,12 @@ class Auth(auth.BaseAuth): tmp: list[str] = [] if self._ldap_load_groups: tmp = [] - for t in res[0][1]['memberOf']: - tmp.append(t.decode('utf-8').split(',')[0][3:]) + for g in user_entry[1]['memberOf']: + tmp.append(g.decode('utf-8').split(',')[0][3:]) self._ldap_groups = set(tmp) - logger.debug("LDAP Auth groups of user: %s", ",".join(self._ldap_groups)) + logger.debug("_login2 LDAP groups of user: %s", ",".join(self._ldap_groups)) conn.unbind() + logger.debug(f"_login2 {login} successfully authenticated") return login except self.ldap.INVALID_CREDENTIALS: return "" @@ -182,18 +189,20 @@ class Auth(auth.BaseAuth): user_entry = conn.response[0] conn.unbind() user_dn = user_entry['dn'] - logger.debug(f"_login3 found user_dn {user_dn}") + logger.debug(f"_login3 found LDAP user DN {user_dn}") try: """Try to bind as the user itself""" conn = self.ldap3.Connection(server, user_dn, password=password) if not conn.bind(): logger.debug(f"_login3 user '{login}' cannot be found") return "" + tmp: list[str] = [] if self._ldap_load_groups: tmp = [] for g in user_entry['attributes']['memberOf']: tmp.append(g.split(',')[0][3:]) self._ldap_groups = set(tmp) + logger.debug("_login3 LDAP groups of user: %s", ",".join(self._ldap_groups)) conn.unbind() logger.debug(f"_login3 {login} successfully authenticated") return login From c243ae4ebf52a833ebe04d71001d8bad4fa93f72 Mon Sep 17 00:00:00 2001 From: Peter Marschall Date: Sun, 29 Dec 2024 07:16:27 +0100 Subject: [PATCH 3/6] LDAP auth: require exactly one result when searching for the LDAP user DN This makes sure not fail securely when the query returns multiple entries - correct grammar in some cases - we're doing _authentication here, not authorization - uppercase LDAP in messages & comments - rename variable _ldap_version to _ldap_module_version to avoid misunderstanding it as LDAP's protocol version - align formatting & messages better between _login2() and _login3() --- radicale/auth/ldap.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/radicale/auth/ldap.py b/radicale/auth/ldap.py index 4833d18d..4f80a362 100644 --- a/radicale/auth/ldap.py +++ b/radicale/auth/ldap.py @@ -118,8 +118,9 @@ class Auth(auth.BaseAuth): filterstr=self._ldap_filter.format(login), attrlist=['memberOf'] ) - if len(res) == 0: - """User could not be found""" + if len(res) != 1: + """User could not be found unambiguously""" + logger.debug(f"_login2 no unique DN found for '{login}'") return "" user_entry = res[0] user_dn = user_entry[0] @@ -181,9 +182,9 @@ class Auth(auth.BaseAuth): search_scope=self.ldap3.SUBTREE, attributes=['memberOf'] ) - if len(conn.entries) == 0: - """User could not be found""" - logger.debug(f"_login3 user '{login}' cannot be found") + if len(conn.entries) != 1: + """User could not be found unambiguously""" + logger.debug(f"_login3 no unique DN found for '{login}'") return "" user_entry = conn.response[0] From 8c2feb4726857746d1afbf10f77cb43f3a3d1aad Mon Sep 17 00:00:00 2001 From: Peter Marschall Date: Sun, 29 Dec 2024 08:29:27 +0100 Subject: [PATCH 4/6] LDAP auth: escape values used in LDAP filters to avoid possible injection of malicious code. --- radicale/auth/ldap.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/radicale/auth/ldap.py b/radicale/auth/ldap.py index 4f80a362..25da242c 100644 --- a/radicale/auth/ldap.py +++ b/radicale/auth/ldap.py @@ -112,10 +112,12 @@ class Auth(auth.BaseAuth): conn.set_option(self.ldap.OPT_REFERRALS, 0) conn.simple_bind_s(self._ldap_reader_dn, self._ldap_secret) """Search for the dn of user to authenticate""" + escaped_login = self.ldap.filter.escape_filter_chars(login) + logger.debug(f"_login2 login escaped for LDAP filters: {escaped_login}") res = conn.search_s( self._ldap_base, self.ldap.SCOPE_SUBTREE, - filterstr=self._ldap_filter.format(login), + filterstr=self._ldap_filter.format(escaped_login), attrlist=['memberOf'] ) if len(res) != 1: @@ -176,9 +178,11 @@ class Auth(auth.BaseAuth): logger.debug(f"_login3 bind as {self._ldap_reader_dn}") """Search the user dn""" + escaped_login = self.ldap3.utils.conv.escape_filter_chars(login) + logger.debug(f"_login3 login escaped for LDAP filters: {escaped_login}") conn.search( search_base=self._ldap_base, - search_filter=self._ldap_filter.format(login), + search_filter=self._ldap_filter.format(escaped_login), search_scope=self.ldap3.SUBTREE, attributes=['memberOf'] ) From 0253682c0049011ed267887d1bff8b5b02e49050 Mon Sep 17 00:00:00 2001 From: Peter Marschall Date: Sun, 29 Dec 2024 13:18:39 +0100 Subject: [PATCH 5/6] LDAP auth: do not blindly assume groups have a 2-letter naming attribute Instead, strip away everything before (and including) the '=' sign of ther RDN. --- radicale/auth/ldap.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/radicale/auth/ldap.py b/radicale/auth/ldap.py index 25da242c..40f0ef09 100644 --- a/radicale/auth/ldap.py +++ b/radicale/auth/ldap.py @@ -142,7 +142,9 @@ class Auth(auth.BaseAuth): if self._ldap_load_groups: tmp = [] for g in user_entry[1]['memberOf']: - tmp.append(g.decode('utf-8').split(',')[0][3:]) + """Get group g's RDN's attribute value""" + g = g.decode('utf-8').split(',')[0] + tmp.append(g.partition('=')[2]) self._ldap_groups = set(tmp) logger.debug("_login2 LDAP groups of user: %s", ",".join(self._ldap_groups)) conn.unbind() @@ -205,7 +207,9 @@ class Auth(auth.BaseAuth): if self._ldap_load_groups: tmp = [] for g in user_entry['attributes']['memberOf']: - tmp.append(g.split(',')[0][3:]) + """Get group g's RDN's attribute value""" + g = g.split(',')[0] + tmp.append(g.partition('=')[2]) self._ldap_groups = set(tmp) logger.debug("_login3 LDAP groups of user: %s", ",".join(self._ldap_groups)) conn.unbind() From 99f5ec389d3f4ca01d3c50f97c629977168497f4 Mon Sep 17 00:00:00 2001 From: Peter Marschall Date: Sun, 29 Dec 2024 08:05:42 +0100 Subject: [PATCH 6/6] LDAP auth: indroduce config option 'ldap_user_attribute' This option gives us - flexible authentication options where the name used for logging on does not have to be the account name e.g. use ldap_filter = (&(obhjectclass=inetOrgperson)(|(cn={0]})(mail={0}))) to allow loginng on using the cn or the mail address - automatically consistent / canonicalized username values (i.e. exactly the way the LDAP server returns them) --- DOCUMENTATION.md | 6 ++++++ config | 3 +++ radicale/auth/ldap.py | 42 +++++++++++++++++++++++++++++++++--------- radicale/config.py | 4 ++++ 4 files changed, 46 insertions(+), 9 deletions(-) diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index f590a294..e0dd6e39 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -901,6 +901,12 @@ The search filter to find the user DN to authenticate by the username. User '{0} Default: `(cn={0})` +#### ldap_user_attribute + +The LDAP attribute whose value shall be used as the user name after successful authentication + +Default: not set, i.e. the login name given is used directly. + ##### ldap_load_groups Load the ldap groups of the authenticated user. These groups can be used later on to define rights. This also gives you access to the group calendars, if they exist. diff --git a/config b/config index c34b9d28..38b845c3 100644 --- a/config +++ b/config @@ -83,6 +83,9 @@ # The filter to find the DN of the user. This filter must contain a python-style placeholder for the login #ldap_filter = (&(objectClass=person)(uid={0})) +# the attribute holding the value to be used as username after authentication +#ldap_user_attribute = cn + # Use ssl on the ldap connection #ldap_use_ssl = False diff --git a/radicale/auth/ldap.py b/radicale/auth/ldap.py index 40f0ef09..ee256fed 100644 --- a/radicale/auth/ldap.py +++ b/radicale/auth/ldap.py @@ -17,13 +17,14 @@ """ Authentication backend that checks credentials with a LDAP server. Following parameters are needed in the configuration: - ldap_uri The LDAP URL to the server like ldap://localhost - ldap_base The baseDN of the LDAP server - ldap_reader_dn The DN of a LDAP user with read access to get the user accounts - ldap_secret The password of the ldap_reader_dn - ldap_secret_file The path of the file containing the password of the ldap_reader_dn - ldap_filter The search filter to find the user to authenticate by the username - ldap_load_groups If the groups of the authenticated users need to be loaded + ldap_uri The LDAP URL to the server like ldap://localhost + ldap_base The baseDN of the LDAP server + ldap_reader_dn The DN of a LDAP user with read access to get the user accounts + ldap_secret The password of the ldap_reader_dn + ldap_secret_file The path of the file containing the password of the ldap_reader_dn + ldap_filter The search filter to find the user to authenticate by the username + ldap_user_attribute The attribute to be used as username after authentication + ldap_load_groups If the groups of the authenticated users need to be loaded Following parameters controls SSL connections: ldap_use_ssl If the connection ldap_ssl_verify_mode The certificate verification mode. NONE, OPTIONAL, default is REQUIRED @@ -42,6 +43,7 @@ class Auth(auth.BaseAuth): _ldap_reader_dn: str _ldap_secret: str _ldap_filter: str + _ldap_user_attr: str _ldap_load_groups: bool _ldap_module_version: int = 3 _ldap_use_ssl: bool = False @@ -66,6 +68,7 @@ class Auth(auth.BaseAuth): self._ldap_load_groups = configuration.get("auth", "ldap_load_groups") self._ldap_secret = configuration.get("auth", "ldap_secret") self._ldap_filter = configuration.get("auth", "ldap_filter") + self._ldap_user_attr = configuration.get("auth", "ldap_user_attribute") ldap_secret_file_path = configuration.get("auth", "ldap_secret_file") if ldap_secret_file_path: with open(ldap_secret_file_path, 'r') as file: @@ -84,6 +87,10 @@ class Auth(auth.BaseAuth): logger.info("auth.ldap_reader_dn : %r" % self._ldap_reader_dn) logger.info("auth.ldap_load_groups : %s" % self._ldap_load_groups) logger.info("auth.ldap_filter : %r" % self._ldap_filter) + if self._ldap_user_attr: + logger.info("auth.ldap_user_attribute : %r" % self._ldap_user_attr) + else: + logger.info("auth.ldap_user_attribute : (not provided)") if ldap_secret_file_path: logger.info("auth.ldap_secret_file_path: %r" % ldap_secret_file_path) if self._ldap_secret: @@ -114,11 +121,15 @@ class Auth(auth.BaseAuth): """Search for the dn of user to authenticate""" escaped_login = self.ldap.filter.escape_filter_chars(login) logger.debug(f"_login2 login escaped for LDAP filters: {escaped_login}") + attrs = ['memberof'] + if self._ldap_user_attr: + attrs = ['memberOf', self._ldap_user_attr] + logger.debug(f"_login2 attrs: {attrs}") res = conn.search_s( self._ldap_base, self.ldap.SCOPE_SUBTREE, filterstr=self._ldap_filter.format(escaped_login), - attrlist=['memberOf'] + attrlist=attrs ) if len(res) != 1: """User could not be found unambiguously""" @@ -147,6 +158,11 @@ class Auth(auth.BaseAuth): tmp.append(g.partition('=')[2]) self._ldap_groups = set(tmp) logger.debug("_login2 LDAP groups of user: %s", ",".join(self._ldap_groups)) + if self._ldap_user_attr: + if user_entry[1][self._ldap_user_attr]: + tmplogin = user_entry[1][self._ldap_user_attr][0] + login = tmplogin.decode('utf-8') + logger.debug(f"_login2 user set to: '{login}'") conn.unbind() logger.debug(f"_login2 {login} successfully authenticated") return login @@ -182,11 +198,15 @@ class Auth(auth.BaseAuth): """Search the user dn""" escaped_login = self.ldap3.utils.conv.escape_filter_chars(login) logger.debug(f"_login3 login escaped for LDAP filters: {escaped_login}") + attrs = ['memberof'] + if self._ldap_user_attr: + attrs = ['memberOf', self._ldap_user_attr] + logger.debug(f"_login3 attrs: {attrs}") conn.search( search_base=self._ldap_base, search_filter=self._ldap_filter.format(escaped_login), search_scope=self.ldap3.SUBTREE, - attributes=['memberOf'] + attributes=attrs ) if len(conn.entries) != 1: """User could not be found unambiguously""" @@ -212,6 +232,10 @@ class Auth(auth.BaseAuth): tmp.append(g.partition('=')[2]) self._ldap_groups = set(tmp) logger.debug("_login3 LDAP groups of user: %s", ",".join(self._ldap_groups)) + if self._ldap_user_attr: + if user_entry['attributes'][self._ldap_user_attr]: + login = user_entry['attributes'][self._ldap_user_attr][0] + logger.debug(f"_login3 user set to: '{login}'") conn.unbind() logger.debug(f"_login3 {login} successfully authenticated") return login diff --git a/radicale/config.py b/radicale/config.py index 0ac5970c..7a085f71 100644 --- a/radicale/config.py +++ b/radicale/config.py @@ -227,6 +227,10 @@ DEFAULT_CONFIG_SCHEMA: types.CONFIG_SCHEMA = OrderedDict([ "value": "(cn={0})", "help": "the search filter to find the user DN to authenticate by the username", "type": str}), + ("ldap_user_attribute", { + "value": "", + "help": "the attribute to be used as username after authentication", + "type": str}), ("ldap_load_groups", { "value": "False", "help": "load the ldap groups of the authenticated user",