Blob Blame History Raw
From 81dcaf1c37c2de24c46672df8d4f968c2fb40a6e Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Wed, 11 Nov 2020 08:59:18 -0500
Subject: [PATCH 1/3] Issue 4383 - Do not normalize escaped spaces in a DN

Bug Description:  Adding an entry with an escaped leading space leads to many
                  problems.  Mainly id2entry can get corrupted during an
                  import of such an entry, and the entryrdn index is not
                  updated correctly

Fix Description:  In slapi_dn_normalize_ext() leave an escaped space intact.

Relates: https://github.com/389ds/389-ds-base/issues/4383

Reviewed by: firstyear, progier, and tbordaz (Thanks!!!)
---
 .../tests/suites/syntax/acceptance_test.py    | 75 ++++++++++++++++++-
 ldap/servers/slapd/dn.c                       |  8 +-
 2 files changed, 77 insertions(+), 6 deletions(-)

diff --git a/dirsrvtests/tests/suites/syntax/acceptance_test.py b/dirsrvtests/tests/suites/syntax/acceptance_test.py
index 543718689..7939a99a7 100644
--- a/dirsrvtests/tests/suites/syntax/acceptance_test.py
+++ b/dirsrvtests/tests/suites/syntax/acceptance_test.py
@@ -1,5 +1,5 @@
 # --- BEGIN COPYRIGHT BLOCK ---
-# Copyright (C) 2019 Red Hat, Inc.
+# Copyright (C) 2020 Red Hat, Inc.
 # All rights reserved.
 #
 # License: GPL (version 3 or any later version).
@@ -7,13 +7,12 @@
 # --- END COPYRIGHT BLOCK ---
 
 import ldap
-import logging
 import pytest
 import os
 from lib389.schema import Schema
 from lib389.config import Config
 from lib389.idm.user import UserAccounts
-from lib389.idm.group import Groups
+from lib389.idm.group import Group, Groups
 from lib389._constants import DEFAULT_SUFFIX
 from lib389.topologies import log, topology_st as topo
 
@@ -127,7 +126,7 @@ def test_invalid_dn_syntax_crash(topo):
         4. Success
     """
 
-    # Create group
+        # Create group
     groups = Groups(topo.standalone, DEFAULT_SUFFIX)
     group = groups.create(properties={'cn': ' test'})
 
@@ -145,6 +144,74 @@ def test_invalid_dn_syntax_crash(topo):
     groups.list()
 
 
+@pytest.mark.parametrize("props, rawdn", [
+                         ({'cn': ' leadingSpace'}, "cn=\\20leadingSpace,ou=Groups,dc=example,dc=com"),
+                         ({'cn': 'trailingSpace '}, "cn=trailingSpace\\20,ou=Groups,dc=example,dc=com")])
+def test_dn_syntax_spaces_delete(topo,  props,  rawdn):
+    """Test that an entry with a space as the first character in the DN can be
+    deleted without error.  We also want to make sure the indexes are properly
+    updated by repeatedly adding and deleting the entry, and that the entry cache
+    is properly maintained.
+
+    :id: b993f37c-c2b0-4312-992c-a9048ff98965
+    :parametrized: yes
+    :setup: Standalone Instance
+    :steps:
+        1. Create a group with a DN that has a space as the first/last
+           character.
+        2. Delete group
+        3. Add group
+        4. Modify group
+        5. Restart server and modify entry
+        6. Delete group
+        7. Add group back
+        8. Delete group using specific DN
+    :expectedresults:
+        1. Success
+        2. Success
+        3. Success
+        4. Success
+        5. Success
+        6. Success
+        7. Success
+        8. Success
+    """
+
+    # Create group
+    groups = Groups(topo.standalone, DEFAULT_SUFFIX)
+    group = groups.create(properties=props.copy())
+
+    # Delete group (verifies DN/RDN parsing works and cache is correct)
+    group.delete()
+
+    # Add group again (verifies entryrdn index was properly updated)
+    groups = Groups(topo.standalone, DEFAULT_SUFFIX)
+    group = groups.create(properties=props.copy())
+
+    # Modify the group (verifies dn/rdn parsing is correct)
+    group.replace('description', 'escaped space group')
+
+    # Restart the server.  This will pull the entry from the database and
+    # convert it into a cache entry, which is different than how a client
+    # first adds an entry and is put into the cache before being written to
+    # disk.
+    topo.standalone.restart()
+
+    # Make sure we can modify the entry (verifies cache entry was created
+    # correctly)
+    group.replace('description', 'escaped space group after restart')
+
+    # Make sure it can still be deleted (verifies cache again).
+    group.delete()
+
+    # Add it back so we can delete it using a specific DN (sanity test to verify
+    # another DN/RDN parsing variation).
+    groups = Groups(topo.standalone, DEFAULT_SUFFIX)
+    group = groups.create(properties=props.copy())
+    group = Group(topo.standalone, dn=rawdn)
+    group.delete()
+
+
 if __name__ == '__main__':
     # Run isolated
     # -s for DEBUG mode
diff --git a/ldap/servers/slapd/dn.c b/ldap/servers/slapd/dn.c
index 2af3f38fc..3980b897f 100644
--- a/ldap/servers/slapd/dn.c
+++ b/ldap/servers/slapd/dn.c
@@ -894,8 +894,7 @@ slapi_dn_normalize_ext(char *src, size_t src_len, char **dest, size_t *dest_len)
                             s++;
                         }
                     }
-                } else if (s + 2 < ends &&
-                           isxdigit(*(s + 1)) && isxdigit(*(s + 2))) {
+                } else if (s + 2 < ends && isxdigit(*(s + 1)) && isxdigit(*(s + 2))) {
                     /* esc hexpair ==> real character */
                     int n = slapi_hexchar2int(*(s + 1));
                     int n2 = slapi_hexchar2int(*(s + 2));
@@ -903,6 +902,11 @@ slapi_dn_normalize_ext(char *src, size_t src_len, char **dest, size_t *dest_len)
                     if (n == 0) { /* don't change \00 */
                         *d++ = *++s;
                         *d++ = *++s;
+                    } else if (n == 32) { /* leave \20 (space) intact */
+                        *d++ = *s;
+                        *d++ = *++s;
+                        *d++ = *++s;
+                        s++;
                     } else {
                         *d++ = n;
                         s += 3;
-- 
2.26.2