From 9255ad463666d45223d93361cca016b68ab45685 Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy Date: Aug 03 2020 09:36:19 +0000 Subject: Make use of unshare+chroot in ipa-extdom-extop unittests to work against glibc 2.32 --- diff --git a/freeipa-fix-unittests-glibc-2.31.9000.patch b/freeipa-fix-unittests-glibc-2.31.9000.patch new file mode 100644 index 0000000..da3b14d --- /dev/null +++ b/freeipa-fix-unittests-glibc-2.31.9000.patch @@ -0,0 +1,228 @@ +From 4c8a875082f0da0ea78977e97696b22d622728a6 Mon Sep 17 00:00:00 2001 +From: Alexander Bokovoy +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 +--- + 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 + #include + #include + #include +@@ -36,10 +37,13 @@ + #include + #include + ++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 + diff --git a/freeipa.spec b/freeipa.spec index 3509bac..f142a41 100644 --- a/freeipa.spec +++ b/freeipa.spec @@ -156,14 +156,14 @@ Name: %{package_name} Version: %{IPA_VERSION} -Release: 4%{?dist} +Release: 5%{?dist} Summary: The Identity, Policy and Audit system License: GPLv3+ URL: http://www.freeipa.org/ Source0: https://releases.pagure.org/freeipa/freeipa-%{version}.tar.gz Source1: https://releases.pagure.org/freeipa/freeipa-%{version}.tar.gz.asc - +Patch0: freeipa-fix-unittests-glibc-2.31.9000.patch # For the timestamp trick in patch application BuildRequires: diffstat @@ -1492,6 +1492,9 @@ fi %endif %changelog +* Mon Aug 03 2020 Alexander Bokovoy - 4.8.7-5 +- Make use of unshare+chroot in ipa-extdom-extop unittests to work against glibc 2.32 + * Sat Aug 01 2020 Fedora Release Engineering - 4.8.7-4 - Second attempt - Rebuilt for https://fedoraproject.org/wiki/Fedora_33_Mass_Rebuild