URL: https://github.com/freeipa/freeipa/pull/4985
Author: abbra
 Title: #4985: [Backport][ipa-4-8] extdom-extop: refactor tests to use 
unshare+chroot to override nss_fi…
Action: opened

PR body:
"""
This PR was opened automatically because PR #4972 was pushed to master and 
backport to ipa-4-8 is required.
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/4985/head:pr4985
git checkout pr4985
From d5fffa48c69b87ed39dceb93b880861862f755bc Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <[email protected]>
Date: Sat, 1 Aug 2020 11:49:30 +0300
Subject: [PATCH] extdom-extop: refactor tests to use unshare+chroot to
 override nss_files configuration

Unit tests for ipa-extdom-extop plugin use nss_files.so.2 module to test the
functionality instead of relying on SSSD API or nss_sss.so.2 module. The latter
two cannot be used in build environment.

nss_files.so.2 always tries to open /etc/passwd and /etc/group. In past, we
overloaded 'fopen()' to change the path to opened file but this stops working
after glibc consolidate file opening in nss_files with the code starting at
https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=299210c1fa67e2dfb564475986fce11cd33db9ad,
this method is not usable anymore and builds against glibc 2.31.9000+ fail in
cmocka unit test execution in Rawhide.

Apply an alternative approach that uses a new user namespace to unshare the
test from its parent and chroot to the test data where expected /etc/passwd and
/etc/group are provided. This method works only on Linux, thus only run the
unit test on Linux.

In case unshare() or chroot() fail, we have to skip tests that use
nss_files.so.2.

Fixes: https://pagure.io/freeipa/issue/8437
Signed-off-by: Alexander Bokovoy <[email protected]>
---
 configure.ac                                  |  2 +
 .../ipa-extdom-extop/Makefile.am              |  2 +
 .../ipa_extdom_cmocka_tests.c                 | 60 ++++++++-----------
 .../test_data/{ => etc}/group                 |  0
 .../test_data/{ => etc}/passwd                |  0
 server.m4                                     |  9 +++
 6 files changed, 38 insertions(+), 35 deletions(-)
 rename daemons/ipa-slapi-plugins/ipa-extdom-extop/test_data/{ => etc}/group (100%)
 rename daemons/ipa-slapi-plugins/ipa-extdom-extop/test_data/{ => etc}/passwd (100%)

diff --git a/configure.ac b/configure.ac
index 586b2532af..d066405354 100644
--- a/configure.ac
+++ b/configure.ac
@@ -546,6 +546,8 @@ AS_CASE([$JSLINT],
 AC_SUBST([JSLINT])
 AM_CONDITIONAL([WITH_JSLINT], [test "x${JSLINT}" != "xno"])
 
+AM_CONDITIONAL([HAVE_UNSHARE],
+    [test "x${ac_cv_func_unshare}" = "xyes" -a "x${ac_cv_func_chroot}" = "xyes"])
 
 # Flags
 
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/Makefile.am b/daemons/ipa-slapi-plugins/ipa-extdom-extop/Makefile.am
index cbdd570eab..1dd1cca5fa 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/Makefile.am
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/Makefile.am
@@ -50,9 +50,11 @@ TESTS =
 check_PROGRAMS =
 
 if HAVE_CMOCKA
+if HAVE_UNSHARE
 TESTS += extdom_cmocka_tests
 check_PROGRAMS += extdom_cmocka_tests
 endif
+endif
 
 extdom_cmocka_tests_SOURCES = 		\
 	ipa_extdom_cmocka_tests.c	\
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c
index 1fa4c6af82..04fb0b63ca 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c
@@ -21,6 +21,7 @@
 */
 #define _GNU_SOURCE
 
+#include <sched.h>
 #include <errno.h>
 #include <stdarg.h>
 #include <stddef.h>
@@ -36,10 +37,13 @@
 #include <stdio.h>
 #include <dlfcn.h>
 
+static bool skip_tests = false;
+
 #define MAX_BUF (1024*1024*1024)
 struct test_data {
     struct extdom_req *req;
     struct ipa_extdom_ctx *ctx;
+    bool skip_test;
 };
 
 /*
@@ -138,40 +142,6 @@ int cmocka_extdom_init_context(struct nss_ops_ctx **nss_context)
     return -1;
 }
 
-struct {
-    const char *o, *n;
-} path_table[] = {
-    { .o = "/etc/passwd", .n = "./test_data/passwd"},
-    { .o = "/etc/group",  .n = "./test_data/group"},
-    { .o = NULL, .n = NULL}};
-
-FILE *(*original_fopen)(const char*, const char*) = NULL;
-
-FILE *fopen(const char *path, const char *mode) {
-    const char *_path = NULL;
-
-    /* Do not handle before-main() cases */
-    if (original_fopen == NULL) {
-        return NULL;
-    }
-    for(int i=0; path_table[i].o != NULL; i++) {
-        if (strcmp(path, path_table[i].o) == 0) {
-                _path = path_table[i].n;
-                break;
-        }
-    }
-    return (*original_fopen)(_path ? _path : path, mode);
-}
-
-/* Attempt to initialize original_fopen before main()
- * There is no explicit order when all initializers are called,
- * so we might still be late here compared to a code in a shared
- * library initializer, like libselinux */
-void redefined_fopen_ctor (void) __attribute__ ((constructor));
-void redefined_fopen_ctor(void) {
-    original_fopen = dlsym(RTLD_NEXT, "fopen");
-}
-
 void test_getpwnam_r_wrapper(void **state)
 {
     int ret;
@@ -181,6 +151,9 @@ void test_getpwnam_r_wrapper(void **state)
     struct test_data *test_data;
 
     test_data = (struct test_data *) *state;
+    if (test_data->skip_test) {
+        skip();
+    }
 
     ret = get_buffer(&buf_len, &buf);
     assert_int_equal(ret, 0);
@@ -238,6 +211,9 @@ void test_getpwuid_r_wrapper(void **state)
     struct test_data *test_data;
 
     test_data = (struct test_data *) *state;
+    if (test_data->skip_test) {
+        skip();
+    }
 
     ret = get_buffer(&buf_len, &buf);
     assert_int_equal(ret, 0);
@@ -290,6 +266,9 @@ void test_getgrnam_r_wrapper(void **state)
     struct test_data *test_data;
 
     test_data = (struct test_data *) *state;
+    if (test_data->skip_test) {
+        skip();
+    }
 
     ret = get_buffer(&buf_len, &buf);
     assert_int_equal(ret, 0);
@@ -340,6 +319,9 @@ void test_getgrgid_r_wrapper(void **state)
     struct test_data *test_data;
 
     test_data = (struct test_data *) *state;
+    if (test_data->skip_test) {
+        skip();
+    }
 
     ret = get_buffer(&buf_len, &buf);
     assert_int_equal(ret, 0);
@@ -389,6 +371,9 @@ void test_get_user_grouplist(void **state)
     struct test_data *test_data;
 
     test_data = (struct test_data *) *state;
+    if (test_data->skip_test) {
+        skip();
+    }
 
     /* This is a bit odd behaviour of getgrouplist() it does not check if the
      * user exists, only if memberships of the user can be found. */
@@ -446,6 +431,11 @@ static int  extdom_req_setup(void **state)
     assert_non_null(test_data->ctx->nss_ctx);
 
     back_extdom_set_timeout(test_data->ctx->nss_ctx, 10000);
+
+    test_data->skip_test = skip_tests;
+    if (chroot("test_data") != 0) {
+        test_data->skip_test = true;
+    }
     *state = test_data;
 
     return 0;
@@ -655,6 +645,6 @@ int main(int argc, const char *argv[])
         cmocka_unit_test(test_decode),
     };
 
-    assert_non_null(original_fopen);
+    skip_tests = (unshare(CLONE_NEWUSER) == -1);
     return cmocka_run_group_tests(tests, extdom_req_setup, extdom_req_teardown);
 }
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/test_data/group b/daemons/ipa-slapi-plugins/ipa-extdom-extop/test_data/etc/group
similarity index 100%
rename from daemons/ipa-slapi-plugins/ipa-extdom-extop/test_data/group
rename to daemons/ipa-slapi-plugins/ipa-extdom-extop/test_data/etc/group
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/test_data/passwd b/daemons/ipa-slapi-plugins/ipa-extdom-extop/test_data/etc/passwd
similarity index 100%
rename from daemons/ipa-slapi-plugins/ipa-extdom-extop/test_data/passwd
rename to daemons/ipa-slapi-plugins/ipa-extdom-extop/test_data/etc/passwd
diff --git a/server.m4 b/server.m4
index 842d599d22..3a2c3ae336 100644
--- a/server.m4
+++ b/server.m4
@@ -153,3 +153,12 @@ dnl Check for libverto
 dnl ---------------------------------------------------------------------------
 
 PKG_CHECK_MODULES([LIBVERTO], [libverto])
+
+dnl ---------------------------------------------------------------------------
+dnl Check for unshare(2) - Linux-only. We also check for chroot(2) as we use both
+dnl ---------------------------------------------------------------------------
+
+AC_CHECK_HEADER(sched.h, [
+    AC_CHECK_FUNC(unshare, [], [AC_MSG_WARN([unshare not found, no extdom unit tests to be run])])
+    AC_CHECK_FUNC(chroot, [], [AC_MSG_WARN([chroot not found, no extdom unit tests to be run])])
+], [AC_MSG_WARN([sched.h not found, unshare is not available])])
_______________________________________________
FreeIPA-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/[email protected]

Reply via email to