Blob Blame History Raw
From 51ea1d34b861dfffb12fbe6be4e23d9342fd0fe2 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Fri, 2 Aug 2019 14:36:24 -0400
Subject: [PATCH] Issue 50530 - Directory Server not RFC 4511 compliant with
 requested attr "1.1"

Bug Description:  A regression was introduced some time back that changed the
                  behavior of how the server handled the "1.1" requested attribute
                  in a search request.  If "1.1" was requested along with other
                  attributes then no attibutes were returned, but in this case "1.1"
                  is expected to be ignroed.

Fix Description:  Only comply with "1.1" if it is the only requested attribute

relates: https://pagure.io/389-ds-base/issue/50530

Reviewed by: firstyear(Thanks!)
---
 dirsrvtests/tests/suites/basic/basic_test.py | 57 +++++++++++++++++---
 ldap/servers/slapd/result.c                  |  7 ++-
 2 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/dirsrvtests/tests/suites/basic/basic_test.py b/dirsrvtests/tests/suites/basic/basic_test.py
index 0f7536b63..cea4f6bfe 100644
--- a/dirsrvtests/tests/suites/basic/basic_test.py
+++ b/dirsrvtests/tests/suites/basic/basic_test.py
@@ -28,6 +28,7 @@ log = logging.getLogger(__name__)
 USER1_DN = 'uid=user1,' + DEFAULT_SUFFIX
 USER2_DN = 'uid=user2,' + DEFAULT_SUFFIX
 USER3_DN = 'uid=user3,' + DEFAULT_SUFFIX
+USER4_DN = 'uid=user4,' + DEFAULT_SUFFIX
 
 ROOTDSE_DEF_ATTR_LIST = ('namingContexts',
                          'supportedLDAPVersion',
@@ -409,8 +410,8 @@ def test_basic_acl(topology_st, import_example_ldif):
                                              'uid': 'user1',
                                              'userpassword': PASSWORD})))
     except ldap.LDAPError as e:
-        log.fatal('test_basic_acl: Failed to add test user ' + USER1_DN
-                  + ': error ' + e.message['desc'])
+        log.fatal('test_basic_acl: Failed to add test user ' + USER1_DN +
+                  ': error ' + e.message['desc'])
         assert False
 
     try:
@@ -421,8 +422,8 @@ def test_basic_acl(topology_st, import_example_ldif):
                                              'uid': 'user2',
                                              'userpassword': PASSWORD})))
     except ldap.LDAPError as e:
-        log.fatal('test_basic_acl: Failed to add test user ' + USER1_DN
-                  + ': error ' + e.message['desc'])
+        log.fatal('test_basic_acl: Failed to add test user ' + USER1_DN +
+                  ': error ' + e.message['desc'])
         assert False
 
     #
@@ -572,6 +573,50 @@ def test_basic_searches(topology_st, import_example_ldif):
     log.info('test_basic_searches: PASSED')
 
 
+@pytest.fixture(scope="module")
+def add_test_entry(topology_st, request):
+    # Add test entry
+    topology_st.standalone.add_s(Entry((USER4_DN,
+                                        {'objectclass': "top extensibleObject".split(),
+                                         'cn': 'user1', 'uid': 'user1'})))
+
+
+search_params = [(['1.1'], 'cn', False),
+                 (['1.1', 'cn'], 'cn', True),
+                 (['+'], 'nsUniqueId', True),
+                 (['*'], 'cn', True),
+                 (['cn'], 'cn', True)]
+@pytest.mark.parametrize("attrs, attr, present", search_params)
+def test_search_req_attrs(topology_st, add_test_entry, attrs, attr, present):
+    """Test requested attributes in search operations.
+    :id: 426a59ff-49b8-4a70-b377-0c0634a29b6e
+    :setup: Standalone instance
+    :steps:
+         1. Test "1.1" does not return any attributes.
+         2. Test "1.1" is ignored if there are other requested attributes
+         3. Test "+" returns all operational attributes
+         4. Test "*" returns all attributes
+         5. Test requested attributes
+
+    :expectedresults:
+         1. Success
+         2. Success
+         3. Success
+         4. Success
+         5. Success
+    """
+
+    log.info("Testing attrs: {} attr: {} present: {}".format(attrs, attr, present))
+    entry = topology_st.standalone.search_s(USER4_DN,
+                                            ldap.SCOPE_BASE,
+                                            'objectclass=top',
+                                            attrs)
+    if present:
+        assert entry[0].hasAttr(attr)
+    else:
+        assert not entry[0].hasAttr(attr)
+
+
 def test_basic_referrals(topology_st, import_example_ldif):
     """Test LDAP server in referral mode.
 
@@ -716,8 +761,8 @@ def test_basic_systemctl(topology_st, import_example_ldif):
     log.info('Attempting to start the server with broken dse.ldif...')
     try:
         topology_st.standalone.start()
-    except:
-        log.info('Server failed to start as expected')
+    except Exception as e:
+        log.info('Server failed to start as expected: ' + str(e))
     log.info('Check the status...')
     assert (not topology_st.standalone.status())
     log.info('Server failed to start as expected')
diff --git a/ldap/servers/slapd/result.c b/ldap/servers/slapd/result.c
index d9f431cc5..34ddd8566 100644
--- a/ldap/servers/slapd/result.c
+++ b/ldap/servers/slapd/result.c
@@ -1546,6 +1546,8 @@ send_ldap_search_entry_ext(
      * "+" means all operational attributes (rfc3673)
      * operational attributes are only retrieved if they are named
      * specifically or when "+" is specified.
+     * In the case of "1.1", if there are other requested attributes
+     * then "1.1" should be ignored.
      */
 
     /* figure out if we want all user attributes or no attributes at all */
@@ -1560,7 +1562,10 @@ send_ldap_search_entry_ext(
             if (strcmp(LDAP_ALL_USER_ATTRS, attrs[i]) == 0) {
                 alluserattrs = 1;
             } else if (strcmp(LDAP_NO_ATTRS, attrs[i]) == 0) {
-                noattrs = 1;
+                /* "1.1" is only valid if it's the only requested attribute */
+                if (i == 0 && attrs[1] == NULL) {
+                    noattrs = 1;
+                }
             } else if (strcmp(LDAP_ALL_OPERATIONAL_ATTRS, attrs[i]) == 0) {
                 alloperationalattrs = 1;
             } else {
-- 
2.21.0