Blob Blame History Raw
From 4c8a875082f0da0ea78977e97696b22d622728a6 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <ab@vda.li>
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.

Fixes: https://pagure.io/freeipa/issue/8437
Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
---
 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                                     |  8 +++
 6 files changed, 37 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 5ec529088..3dfa9ac44 100644
--- a/configure.ac
+++ b/configure.ac
@@ -527,6 +527,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 cbdd570ea..1dd1cca5f 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 1fa4c6af8..04fb0b63c 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 @@ fail:
     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 d35823e80..a8d4930fc 100644
--- a/server.m4
+++ b/server.m4
@@ -172,3 +172,11 @@ AC_ARG_WITH([systemdtmpfilesdir],
             [systemdtmpfilesdir=$with_systemdtmpfilesdir],
         [systemdtmpfilesdir=$($PKG_CONFIG --define-variable=prefix='${prefix}' --variable=tmpfilesdir systemd)])
 AC_SUBST([systemdtmpfilesdir])
+
+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])])
-- 
2.26.2