From 0b0147cdaad0f1fc54451c23b6e5d70da178736f Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Wed, 11 Nov 2020 08:59:18 -0500 Subject: [PATCH 7/8] 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