Escape user input in LDAP filters and DNs

The uid was formatted straight into search filters, so a password
reset for user '*' matched and changed the first entry in the tree
(staff accounts included). Escape filter values with
escape_filter_chars and DN values with escape_rdn.

Also fix the username pattern: [a-z|0-9|\-|_] allowed '|'.

dal/tests/test_flows.py

@@ -73,6 +73,7 @@ class RegisterTests(LdapTestCase):
     def test_validation_errors(self):
         self.assert_error(self.form(username=""), "Please supply a username")
         self.assert_error(self.form(username="Bad Name"), "lowercase letters")
+        self.assert_error(self.form(username="pipe|name"), "lowercase letters")
         self.assert_error(self.form(password2="different"), "match")
         self.assert_error(self.form(email="not-an-email"), "email address is invalid")
         self.assert_error(self.form(lastname=""), "firstname and lastname")
@@ -251,9 +252,8 @@ class ResetPasswordTests(LdapTestCase):
         self.assertFalse(self.can_bind(victim, "attacker-1234", ou="users"),
                          "account takeover without any token")
 
-    @expectedFailure
-    def test_bug_ldap_filter_injection_in_reset(self):
-        """CRITICAL: user='*' matches the first entry in the whole tree."""
+    def test_ldap_filter_injection_in_reset(self):
+        """user='*' must not match the first entry in the whole tree."""
         self.make_ldap_user(password="victim-pass-1234")
         self.client.post("/reset/", {
             "user": "*", "password1": "attacker-1234", "password2": "attacker-1234"})

dal/ungleich_ldap.py

@@ -4,6 +4,8 @@ import random
 
 import ldap3
 from django.conf import settings
+from ldap3.utils.conv import escape_filter_chars
+from ldap3.utils.dn import escape_rdn
 import logging
 
 logger = logging.getLogger(__name__)
@@ -93,7 +95,7 @@ class LdapManager:
         try:
             uid = user  # user.encode("utf-8")
             conn.add("uid={uid},{customer_dn}".format(
-                    uid=uid, customer_dn=settings.LDAP_CUSTOMER_DN
+                    uid=escape_rdn(uid), customer_dn=settings.LDAP_CUSTOMER_DN
                 ),
                 ["inetOrgPerson", "posixAccount", "ldapPublickey"],
                 {
@@ -216,7 +218,7 @@ class LdapManager:
             result = conn.search(
                 search_base=search_base,
                 search_filter=search_filter if len(search_filter)> 0 else
-                '(uid={uid})'.format(uid=uid),
+                '(uid={uid})'.format(uid=escape_filter_chars(uid)),
                 attributes=attributes
             )
             entries = conn.entries
@@ -234,7 +236,7 @@ class LdapManager:
         conn = self.get_admin_conn()
         try:
             return_val = conn.delete(
-                ("uid={uid}," + settings.LDAP_CUSTOMER_DN).format(uid=uid),
+                ("uid={uid}," + settings.LDAP_CUSTOMER_DN).format(uid=escape_rdn(uid)),
             )
             msg = "success"
         except Exception as ex:

dal/views.py

@@ -37,7 +37,7 @@ from django.contrib.auth.mixins import LoginRequiredMixin
 
 # Small helper used for registration.
 def is_username_valid(username):
-    return re.fullmatch(r"^[a-z|0-9|\-|_]+$", username)
+    return re.fullmatch(r"[a-z0-9_-]+", username)
 
 admin_seed = config('ADMIN_SEED')
 admin_name = config('ADMIN_NAME')