URL: https://github.com/SSSD/sssd/pull/558
Author: jhrozek
 Title: #558: WIP: Add a test for sss_nss_getgrouplist_timeout and fix 
invalidating the initgroups cache
Action: opened

PR body:
"""
This is a WIP on adding tests for the sss_nss_ex interface. I covered only the 
sss_nss_getgrouplist_timeout function so far.

I'm submitting the PR already in this state to get some feedback if this
coverage is enough and the other functions can be covered similarly or
if there is some issue with this approach.

Also, I found a bug in invalidating the initgroups memory cache, that's
the first of the two patches. Here I'm really not sure if the fix is even
how the issue should be fixed, so I just hacked something up, even without
allocation checks etc.
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/558/head:pr558
git checkout pr558
From 5024bdb2b237ddaf46d04a568672e7b8413393bf Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Tue, 24 Apr 2018 16:31:38 +0200
Subject: [PATCH 1/2] NSS: Fix deleting named entries from the initgroup memory
 cache

---
 src/responder/nss/nss_cmd.c        |  8 ++++++--
 src/responder/nss/nss_get_object.c | 17 +++++++++++------
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/src/responder/nss/nss_cmd.c b/src/responder/nss/nss_cmd.c
index 9f8479b7b..2f4850fd9 100644
--- a/src/responder/nss/nss_cmd.c
+++ b/src/responder/nss/nss_cmd.c
@@ -493,12 +493,16 @@ static errno_t invalidate_cache(struct nss_cmd_ctx *cmd_ctx,
         return ret;
     }
 
-    memcache_delete_entry(cmd_ctx->nss_ctx, cmd_ctx->nss_ctx->rctx, NULL,
-                          output_name, 0, memcache_type);
     if (memcache_type == SSS_MC_INITGROUPS) {
+        memcache_delete_entry(cmd_ctx->nss_ctx, cmd_ctx->nss_ctx->rctx, NULL,
+                              result->lookup_name, 0, memcache_type);
+
         /* Invalidate the passwd data as well */
         memcache_delete_entry(cmd_ctx->nss_ctx, cmd_ctx->nss_ctx->rctx,
                               result->domain, output_name, 0, SSS_MC_PASSWD);
+    } else {
+        memcache_delete_entry(cmd_ctx->nss_ctx, cmd_ctx->nss_ctx->rctx, NULL,
+                              output_name, 0, memcache_type);
     }
     talloc_free(output_name);
 
diff --git a/src/responder/nss/nss_get_object.c b/src/responder/nss/nss_get_object.c
index 15faced00..bab817ab4 100644
--- a/src/responder/nss/nss_get_object.c
+++ b/src/responder/nss/nss_get_object.c
@@ -109,12 +109,17 @@ memcache_delete_entry(struct nss_ctx *nss_ctx,
         }
 
         if (name != NULL) {
-            ret = sized_output_name(NULL, rctx, name, dom, &sized_name);
-            if (ret != EOK) {
-                DEBUG(SSSDBG_OP_FAILURE,
-                      "Unable to create sized name [%d]: %s\n",
-                      ret, sss_strerror(ret));
-                return ret;
+            if (type == SSS_MC_INITGROUPS) {
+                sized_name = talloc_zero(NULL, struct sized_string);
+                to_sized_string(sized_name, name);
+            } else {
+                ret = sized_output_name(NULL, rctx, name, dom, &sized_name);
+                if (ret != EOK) {
+                    DEBUG(SSSDBG_OP_FAILURE,
+                        "Unable to create sized name [%d]: %s\n",
+                        ret, sss_strerror(ret));
+                    return ret;
+                }
             }
 
             ret = memcache_delete_entry_by_name(nss_ctx, sized_name, type);

From 094b17954ecef072ca2e619c0f6f5f67f6d33187 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Mon, 23 Apr 2018 21:33:49 +0200
Subject: [PATCH 2/2] TESTS: Add tests for the sss_nss_getgrouplist_timeout
 function

---
 src/tests/intg/Makefile.am    |   2 +
 src/tests/intg/sssd_nss_ex.py |  86 ++++++++++++++
 src/tests/intg/test_nss_ex.py | 261 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 349 insertions(+)
 create mode 100644 src/tests/intg/sssd_nss_ex.py
 create mode 100644 src/tests/intg/test_nss_ex.py

diff --git a/src/tests/intg/Makefile.am b/src/tests/intg/Makefile.am
index 9c5338261..028fe8ed3 100644
--- a/src/tests/intg/Makefile.am
+++ b/src/tests/intg/Makefile.am
@@ -3,6 +3,7 @@ dist_noinst_DATA = \
     config.py.m4 \
     util.py \
     sssd_nss.py \
+    sssd_nss_ex.py \
     sssd_id.py \
     sssd_ldb.py \
     sssd_netgroup.py \
@@ -36,6 +37,7 @@ dist_noinst_DATA = \
     data/ad_schema.ldif \
     test_pysss_nss_idmap.py \
     test_infopipe.py \
+    test_nss_ex.py \
     $(NULL)
 
 EXTRA_DIST = data/cwrap-dbus-system.conf.in
diff --git a/src/tests/intg/sssd_nss_ex.py b/src/tests/intg/sssd_nss_ex.py
new file mode 100644
index 000000000..381f3cae3
--- /dev/null
+++ b/src/tests/intg/sssd_nss_ex.py
@@ -0,0 +1,86 @@
+#
+# Shared module for integration tests that need to access the sssd_nss_ex
+# interface directly
+#
+# Copyright (c) 2018 Red Hat, Inc.
+#
+# This is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; version 2 only
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import config
+import errno
+from ctypes import (cdll, c_int, c_char_p, c_char,
+                    c_uint32, c_uint, POINTER, pointer)
+
+
+def nss_sss_ex_ctypes_loader(func_name):
+    libnss_idmap_path = config.NSS_MODULE_DIR + "/libsss_nss_idmap.so"
+    libnss_idmap = cdll.LoadLibrary(libnss_idmap_path)
+    func = getattr(libnss_idmap, func_name)
+    return func
+
+
+class NssExFlags(object):
+    """ 'enum' class for name the flags the sssd_nss_ex calls accept """
+    NONE = 0,
+    SSS_NSS_EX_FLAG_NO_CACHE = 1,
+    SSS_NSS_EX_FLAG_INVALIDATE_CACHE = 2,
+
+
+class SssNssGetgrouplistResult:
+    def __init__(self, errno, ngroups, groups):
+        self.errno = errno
+        self.ngroups = ngroups
+        self.groups = groups
+
+
+def sss_nss_getgrouplist_timeout(name,
+                                 gid,
+                                 num_groups,
+                                 flags=NssExFlags.NONE,
+                                 timeout=5000):
+    """
+    A python wrapper for:
+
+    int sss_nss_getgrouplist_timeout(const char *name, gid_t group,
+                                    gid_t *groups, int *ngroups,
+                                    uint32_t flags, unsigned int timeout)
+    """
+    func = nss_sss_ex_ctypes_loader("sss_nss_getgrouplist_timeout")
+
+    func.restype = c_int
+    func.argtypes = [POINTER(c_char), c_uint32, POINTER(c_uint32),
+                     POINTER(c_int), c_uint32, c_uint]
+
+    group_array = (c_uint32 * num_groups)()
+    p_num_groups = pointer(c_int(num_groups))
+
+    res = func(c_char_p(name.encode('utf-8')),
+               c_uint32(gid),
+               group_array,
+               p_num_groups,
+               c_uint32(int(flags[0])),
+               c_uint(timeout))
+
+    groups = []
+    group_num = 0
+
+    if res == 0:
+        group_num = p_num_groups[0]
+    elif res == errno.ERANGE:
+        group_num = num_groups
+    # else group_num == 0 and the loop will fall through
+
+    for i in range(0, group_num):
+        groups.append(int(group_array[i]))
+
+    return SssNssGetgrouplistResult(res, group_num, groups)
diff --git a/src/tests/intg/test_nss_ex.py b/src/tests/intg/test_nss_ex.py
new file mode 100644
index 000000000..23ed9e183
--- /dev/null
+++ b/src/tests/intg/test_nss_ex.py
@@ -0,0 +1,261 @@
+#
+# Integration test for the nss_ex interface
+#
+# Copyright (c) 2016 Red Hat, Inc.
+#
+# This is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; version 2 only
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+import os
+import ent
+import errno
+import stat
+import signal
+import subprocess
+import time
+import ldap
+import pytest
+import ds_openldap
+import ldap_ent
+import config
+from util import unindent
+from sssd_nss_ex import sss_nss_getgrouplist_timeout, NssExFlags
+
+
+LDAP_BASE_DN = "dc=example,dc=com"
+SSSD_DOMAIN = "LDAP"
+SCHEMA_RFC2307_BIS = "rfc2307bis"
+
+
+@pytest.fixture(scope="module")
+def ds_inst(request):
+    """LDAP server instance fixture"""
+    ds_inst = ds_openldap.DSOpenLDAP(
+        config.PREFIX, 10389, LDAP_BASE_DN,
+        "cn=admin", "Secret123")
+    try:
+        ds_inst.setup()
+    except:
+        ds_inst.teardown()
+        raise
+    request.addfinalizer(lambda: ds_inst.teardown())
+    return ds_inst
+
+
+@pytest.fixture(scope="module")
+def ldap_conn(request, ds_inst):
+    """LDAP server connection fixture"""
+    ldap_conn = ds_inst.bind()
+    ldap_conn.ds_inst = ds_inst
+    request.addfinalizer(lambda: ldap_conn.unbind_s())
+    return ldap_conn
+
+
+def create_ldap_fixture(request, ldap_conn, ent_list):
+    """Add LDAP entries and add teardown for removing them"""
+    for entry in ent_list:
+        ldap_conn.add_s(entry[0], entry[1])
+
+    def teardown():
+        for entry in ent_list:
+            try:
+                ldap_conn.delete_s(entry[0])
+            except ldap.NO_SUCH_OBJECT:
+                # if the test already removed an object, it's fine
+                # to not care in the teardown
+                pass
+    request.addfinalizer(teardown)
+
+
+def create_conf_fixture(request, contents):
+    """Generate sssd.conf and add teardown for removing it"""
+    conf = open(config.CONF_PATH, "w")
+    conf.write(contents)
+    conf.close()
+    os.chmod(config.CONF_PATH, stat.S_IRUSR | stat.S_IWUSR)
+    request.addfinalizer(lambda: os.unlink(config.CONF_PATH))
+
+
+def stop_sssd():
+    pid_file = open(config.PIDFILE_PATH, "r")
+    pid = int(pid_file.read())
+    os.kill(pid, signal.SIGTERM)
+    while True:
+        try:
+            os.kill(pid, signal.SIGCONT)
+        except:
+            break
+        time.sleep(1)
+
+
+def create_sssd_fixture(request):
+    """Start sssd and add teardown for stopping it and removing state"""
+    if subprocess.call(["sssd", "-D", "-f"]) != 0:
+        raise Exception("sssd start failed")
+
+    def teardown():
+        try:
+            stop_sssd()
+        except:
+            pass
+        for path in os.listdir(config.DB_PATH):
+            os.unlink(config.DB_PATH + "/" + path)
+        for path in os.listdir(config.MCACHE_PATH):
+            os.unlink(config.MCACHE_PATH + "/" + path)
+    request.addfinalizer(teardown)
+
+
+def load_data_to_ldap(request, ldap_conn, schema):
+    ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
+    ent_list.add_user("user1", 1001, 2001)
+
+    for gid in range(20000, 20010):
+        groupname = "group%d" % gid
+        ent_list.add_group_bis(groupname, gid, ("user1",))
+
+    ent_list.add_user("user2", 1002, 2002)
+    ent_list.add_group_bis("user2_group", 3001, member_uids=("user2",))
+    create_ldap_fixture(request, ldap_conn, ent_list)
+
+
+def load_2307bis_data_to_ldap(request, ldap_conn):
+    return load_data_to_ldap(request, ldap_conn, SCHEMA_RFC2307_BIS)
+
+
+@pytest.fixture
+def setup_rfc2307bis(request, ldap_conn):
+    load_2307bis_data_to_ldap(request, ldap_conn)
+
+    conf = unindent("""\
+        [sssd]
+        domains             = LDAP
+        services            = nss
+
+        [nss]
+        debug_level = 10
+
+        [domain/LDAP]
+        ldap_schema             = rfc2307bis
+        id_provider             = ldap
+        auth_provider           = ldap
+        sudo_provider           = ldap
+        ldap_group_object_class = groupOfNames
+        ldap_uri                = {ldap_conn.ds_inst.ldap_url}
+        ldap_search_base        = {ldap_conn.ds_inst.base_dn}
+    """).format(**locals())
+    create_conf_fixture(request, conf)
+    create_sssd_fixture(request)
+    return None
+
+
+def run_getgrouplist_timeout(username,
+                             pgid,
+                             exp_groups,
+                             flags=NssExFlags.NONE):
+    res = sss_nss_getgrouplist_timeout(username, pgid, 100, flags=flags)
+    assert res.errno == 0
+    assert sorted(res.groups) == sorted(exp_groups)
+    assert res.ngroups == len(exp_groups)
+
+
+def user1_grouplist_ok_and_erange():
+    pgid = 2001
+    exp_groups = [g for g in range(20000, 20010)]
+    exp_groups.append(pgid)
+
+    # Positive test -- a large enough array
+    run_getgrouplist_timeout("user1", pgid, exp_groups)
+
+    # Pass in an array too small, just make sure we don't crash
+    res = sss_nss_getgrouplist_timeout("user1", pgid, 5)
+    assert res.errno == errno.ERANGE
+    assert res.ngroups == 5
+    # It is not reliable between successive runs /which/ groups
+    # will be returned, so we don't check a slice of the exp_groups
+
+
+def test_sss_nss_getgrouplist_timeout(ldap_conn,
+                                      setup_rfc2307bis):
+    """
+    Test calling the sss_nss_getgrouplist_timeout function
+    """
+    user1_grouplist_ok_and_erange()
+
+    # Test that the same case works fine just replying from the
+    # memory cache
+    stop_sssd()
+    user1_grouplist_ok_and_erange()
+
+
+def test_sss_nss_getgrouplist_timeout_etime(ldap_conn,
+                                            setup_rfc2307bis):
+    """
+    Test that the function does time out with a ridiculously low timeout
+    """
+    res = sss_nss_getgrouplist_timeout("user1", 2001, 100, timeout=1)
+    assert res.errno == errno.ETIME
+
+
+def test_sss_nss_getgrouplist_timeout_no_cache(ldap_conn,
+                                               setup_rfc2307bis):
+    """
+    Test that the NssExFlags.SSS_NSS_EX_FLAG_NO_CACHE flag works well
+    with the getgrouplist_timeout function
+    """
+    pgid = 2002
+    exp_groups = [3001, pgid]
+
+    # Cache the user first
+    run_getgrouplist_timeout("user2", pgid, exp_groups)
+
+    # Modify the user group memberships
+    ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
+
+    ent_list.add_group_bis("addtl_group", 3002, member_uids=("user2",))
+    for entry in ent_list:
+        ldap_conn.add_s(entry[0], entry[1])
+
+    # Still the same results since normally we get results from the cache
+    run_getgrouplist_timeout("user2", pgid, exp_groups)
+
+    # Bypassing the cache should now return the extra group
+    exp_groups.append(3002)
+    run_getgrouplist_timeout("user2", pgid, exp_groups,
+                             NssExFlags.SSS_NSS_EX_FLAG_NO_CACHE)
+
+
+def test_sss_nss_getgrouplist_timeout_invalidate_cache(ldap_conn,
+                                                       setup_rfc2307bis):
+    """
+    Test that the NssExFlags.SSS_NSS_EX_FLAG_INVALIDATE_CACHE flag works well
+    with the getgrouplist_timeout function
+    """
+    pgid = 2002
+    exp_groups = [3001, pgid]
+
+    # Cache the user first
+    run_getgrouplist_timeout("user2", pgid, exp_groups)
+
+    # Modify the user group memberships
+    ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
+
+    ent_list.add_group_bis("addtl_group", 3002, member_uids=("user2",))
+    for entry in ent_list:
+        ldap_conn.add_s(entry[0], entry[1])
+
+    # Still the same results since normally we get results from the cache
+    run_getgrouplist_timeout("user2", pgid, exp_groups,
+                             NssExFlags.SSS_NSS_EX_FLAG_INVALIDATE_CACHE)
+
+    # Bypassing the cache should now return the extra group
+    exp_groups.append(3002)
+    run_getgrouplist_timeout("user2", pgid, exp_groups)
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

Reply via email to