On (06/10/14 15:46), Jakub Hrozek wrote:
>Hi,
>
>attached are more patches that are related to running sssd as a
>non-root. I split the functions to become a different user into a
>separate module and provided unit tests using nss_wrapper and
>uid_wrapper.
>
>There is a separate Makefile.am for the cwrap-based tests, because it's
>not easy (read: possible without a wrapper script) to run tests with a
>custom environment. Some build systems, like cmake, allow that, but
>autotools does not.
>From 565aca30c666cb696ccb5c6a9426144276c7dce9 Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek(a)redhat.com>
>Date: Sat, 26 Jul 2014 12:46:26 +0200
>Subject: [PATCH 1/3] UTIL: Move become_user outside krb5 tree
>
>In order for several other SSSD processes to run as a non-root user, we
>need to move the functions to become another user to a shared space in
>our source tree.
>---
> Makefile.am | 20 ++++++++++++--------
> src/providers/krb5/krb5_utils.h | 8 --------
> .../krb5/krb5_become_user.c => util/become_user.c} | 1 -
> src/util/util.h | 9 +++++++++
> 4 files changed, 21 insertions(+), 17 deletions(-)
> rename src/{providers/krb5/krb5_become_user.c => util/become_user.c} (99%)
>
>diff --git a/Makefile.am b/Makefile.am
>index
eb0e649437f465d3354a0c063a29c65203824506..ac3f26cad6e1d0a5ac8979ce13286bc896c62b62 100644
>--- a/Makefile.am
>+++ b/Makefile.am
>@@ -1286,13 +1286,14 @@ strtonum_tests_LDADD = \
> krb5_utils_tests_SOURCES = \
> src/tests/krb5_utils-tests.c \
> src/providers/krb5/krb5_utils.c \
>- src/providers/krb5/krb5_become_user.c \
> src/providers/krb5/krb5_common.c \
> src/util/sss_krb5.c \
> src/providers/data_provider_fo.c \
> src/providers/data_provider_opts.c \
> src/providers/data_provider_callbacks.c \
>- $(SSSD_FAILOVER_OBJ)
>+ src/util/become_user.c \
>+ $(SSSD_FAILOVER_OBJ) \
>+ $(NULL)
Does it make sense to add NULL at the end?
You put new file before SSSD_FAILOVER_OBJ.
If you think that it make sense I will not insist on change.
> krb5_utils_tests_CFLAGS = \
> $(AM_CFLAGS) \
> $(KRB5_CFLAGS) \
>@@ -1567,13 +1568,14 @@ krb5_child_test_SOURCES = \
> src/tests/krb5_child-test.c \
> src/providers/krb5/krb5_utils.c \
> src/providers/krb5/krb5_child_handler.c \
>- src/providers/krb5/krb5_become_user.c \
> src/providers/krb5/krb5_common.c \
> src/util/sss_krb5.c \
> src/providers/data_provider_fo.c \
> src/providers/data_provider_opts.c \
> src/providers/data_provider_callbacks.c \
>- $(SSSD_FAILOVER_OBJ)
>+ src/util/become_user.c \
>+ $(SSSD_FAILOVER_OBJ) \
>+ $(NULL)
The same as previous one.
> krb5_child_test_CFLAGS = \
> $(AM_CFLAGS) \
> -DKRB5_CHILD_DIR=\"$(builddir)\" \
>@@ -2243,7 +2245,6 @@ libsss_ad_common_la_LIBADD = \
>
> libsss_krb5_common_la_SOURCES = \
> src/providers/krb5/krb5_utils.c \
>- src/providers/krb5/krb5_become_user.c \
> src/providers/krb5/krb5_delayed_online_authentication.c \
> src/providers/krb5/krb5_renew_tgt.c \
> src/providers/krb5/krb5_wait_queue.c \
>@@ -2252,7 +2253,9 @@ libsss_krb5_common_la_SOURCES = \
> src/providers/krb5/krb5_access.c \
> src/providers/krb5/krb5_child_handler.c \
> src/providers/krb5/krb5_init_shared.c \
>- src/util/sss_krb5.c
>+ src/util/sss_krb5.c \
>+ src/util/become_user.c \
>+ $(NULL)
> libsss_krb5_common_la_CFLAGS = \
> $(KRB5_CFLAGS)
> libsss_krb5_common_la_LIBADD = \
>@@ -2432,7 +2435,6 @@ libsss_ad_la_LDFLAGS = \
> -module
>
> krb5_child_SOURCES = \
>- src/providers/krb5/krb5_become_user.c \
> src/providers/krb5/krb5_child.c \
> src/providers/dp_pam_data_util.c \
> src/util/user_info_msg.c \
>@@ -2441,7 +2443,9 @@ krb5_child_SOURCES = \
> src/util/authtok.c \
> src/util/util.c \
> src/util/signal.c \
>- src/sss_client/common.c
>+ src/util/become_user.c \
>+ src/sss_client/common.c \
>+ $(NULL)
> krb5_child_CFLAGS = \
> $(AM_CFLAGS) \
> $(POPT_CFLAGS) \
>diff --git a/src/providers/krb5/krb5_utils.h b/src/providers/krb5/krb5_utils.h
//snip
ack to other changes.
>From afc346f2c8735656256cd8136e38f636923cd8ed Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek(a)redhat.com>
>Date: Sat, 26 Jul 2014 14:58:58 +0200
>Subject: [PATCH 2/3] BUILD: Detect nss_wrapper and uid_wrapper during
> configure
>
>Unit testing the utilities to become another user requires the use of
>the cwrap libraries. This patch augments our build system with macros to
>detect the nss_wrapper and and uid_wrapper libraries.
ACK
>From b962ad8b9fb4aa120e12a37c874f907a68abfc8e Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek(a)redhat.com>
>Date: Sun, 27 Jul 2014 14:44:24 +0200
>Subject: [PATCH 3/3] TESTS: Add a test to change user IDs
>
>---
> Makefile.am | 11 +++
> configure.ac | 2 +-
> contrib/sssd.spec.in | 2 +
> src/man/Makefile.am | 2 +
> src/tests/cwrap/Makefile.am | 45 +++++++++++
> src/tests/cwrap/cwrap_test_setup.sh | 17 ++++
> src/tests/cwrap/group | 1 +
> src/tests/cwrap/passwd | 1 +
> src/tests/cwrap/test_become_user.c | 156 ++++++++++++++++++++++++++++++++++++
> 9 files changed, 236 insertions(+), 1 deletion(-)
> create mode 100644 src/tests/cwrap/Makefile.am
> create mode 100755 src/tests/cwrap/cwrap_test_setup.sh
> create mode 100644 src/tests/cwrap/group
> create mode 100644 src/tests/cwrap/passwd
> create mode 100644 src/tests/cwrap/test_become_user.c
>
>diff --git a/Makefile.am b/Makefile.am
>index
ac3f26cad6e1d0a5ac8979ce13286bc896c62b62..91cda02ca57209d33a378f302a150f897974ff25 100644
>--- a/Makefile.am
>+++ b/Makefile.am
>@@ -19,6 +19,16 @@ if HAVE_MANPAGES
> SUBDIRS += src/man
> endif
>
>+# The cwrap tests require the wrapper libraries and are written
>+# using cmocka
>+if HAVE_CMOCKA
>+if HAVE_NSS_WRAPPER
>+if HAVE_UID_WRAPPER
>+SUBDIRS += src/tests/cwrap
>+endif # HAVE_UID_WRAPPER
>+endif # HAVE_NSS_WRAPPER
>+endif # HAVE_CMOCKA
>+
If wrappers are not installed you can change directory to src/tests/cwrap
and make check will fail. This "if statements" should be also in cwrap Makefile
for ckeck_PROGRAMS.
> # Some old versions of automake don't define builddir
> builddir ?= .
>
>@@ -2829,6 +2839,7 @@ endif
> CLEANFILES = *.X */*.X */*/*.X
>
> tests: all $(check_PROGRAMS)
>+ (cd src/tests/cwrap && $(MAKE) $(AM_MAKEFLAGS) $@) || exit 1;
>
>
> # RPM-related tasks
>diff --git a/configure.ac b/configure.ac
>index
1edf4cb19e390ced2043353341d321425652da71..660ea8d373272917d2d6ceeafdccf5dddbe493bc 100644
>--- a/configure.ac
>+++ b/configure.ac
>@@ -335,7 +335,7 @@ AC_SUBST([abs_builddir], $abs_build_dir)
>
> AC_CONFIG_FILES([Makefile contrib/sssd.spec src/examples/rwtab src/doxy.config
> src/sysv/sssd src/sysv/gentoo/sssd src/sysv/SUSE/sssd
>- po/Makefile.in src/man/Makefile
>+ po/Makefile.in src/man/Makefile src/tests/cwrap/Makefile
> src/providers/ipa/ipa_hbac.pc src/providers/ipa/ipa_hbac.doxy
> src/lib/idmap/sss_idmap.pc src/lib/idmap/sss_idmap.doxy
> src/sss_client/sudo/sss_sudo.doxy
>diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
>index
611730c1928b5bbfaa2aee50967226c5824bbca0..4c11bab24eb9eb0c94ee865cc1fc975180aa2ee2 100644
>--- a/contrib/sssd.spec.in
>+++ b/contrib/sssd.spec.in
>@@ -113,6 +113,8 @@ BuildRequires: glib2-devel
> BuildRequires: selinux-policy-targeted
> %if 0%{?fedora}
> BuildRequires: libcmocka-devel
>+BuildRequires: uid_wrapper
>+BuildRequires: nss_wrapper
{nss,uid}_wrapper are not in fedora 19. On the other hand, new ding-libs isn't
either. Do we want to be correct and add proper condition for wrappers.
BTW you can also add wrappers into additionals dependencies for rhel distros in
CI (contrib/ci/deps.sh)
The wrappers are in epel6 and epel7
https://admin.fedoraproject.org/pkgdb/package/uid_wrapper/
https://admin.fedoraproject.org/pkgdb/package/nss_wrapper/
> %endif
> %if (0%{?fedora} || 0%{?rhel} >= 7)
> BuildRequires: libnl3-devel
>diff --git a/src/man/Makefile.am b/src/man/Makefile.am
>index
a92ac0d1e62c4ef96e347d47ccf427f94a60fea2..f8e8f81af964fa7084300745ead80635739c3a47 100644
>--- a/src/man/Makefile.am
>+++ b/src/man/Makefile.am
>@@ -218,3 +218,5 @@ uninstall-local-yes:
> man_MANS="$$manpages"; \
> fi \
> done
>+
>+tests:
This change is not necessary with current version of recursive target "tests".
>diff --git a/src/tests/cwrap/Makefile.am b/src/tests/cwrap/Makefile.am
>new file mode 100644
>index
0000000000000000000000000000000000000000..3bf2ff9f89ea49870f3676fd9b255a8094d47770
>--- /dev/null
>+++ b/src/tests/cwrap/Makefile.am
>@@ -0,0 +1,45 @@
>+
>+AM_CPPFLAGS = \
>+ -Wall \
>+ -I$(top_srcdir)/src \
>+ -I. \
>+ $(DBUS_CFLAGS) \
>+ $(NULL)
>+
>+AM_TESTS_ENVIRONMENT = \
>+ SSSD_SRCDIR=$(top_srcdir) \
>+ . $(top_srcdir)/src/tests/cwrap/cwrap_test_setup.sh; \
>+ $(NULL)
>+
>+check_PROGRAMS = \
>+ become_user-tests \
>+ $(NULL)
>+
>+check_LTLIBRARIES = \
>+ libsss_test_common.la
>+
>+TESTS = $(check_PROGRAMS)
>+
>+libsss_test_common_la_SOURCES = \
>+ ../common_tev.c \
>+ ../common_dom.c \
>+ ../leak_check.c \
>+ ../common.c
>+libsss_test_common_la_LIBADD = \
>+ $(TALLOC_LIBS) \
>+ $(TEVENT_LIBS)
>+
>+become_user_tests_SOURCES = \
>+ test_become_user.c \
>+ $(NULL)
>+become_user_tests_CFLAGS = \
>+ $(AM_CFLAGS) \
>+ $(NULL)
>+become_user_tests_LDADD = \
>+ $(POPT_LIBS) \
>+ $(CMOCKA_LIBS) \
>+ $(abs_top_builddir)/libsss_debug.la \
>+ libsss_test_common.la \
>+ $(NULL)
>+
>+tests: $(check_PROGRAMS)
>diff --git a/src/tests/cwrap/cwrap_test_setup.sh
b/src/tests/cwrap/cwrap_test_setup.sh
>new file mode 100755
>index
0000000000000000000000000000000000000000..d5ddffb8ee81509e7b43183ed6e0671258366fe3
This script is not added in any makefile, therefore make distcheck failed.
The same with src/tests/cwrap/group src/tests/cwrap/passwd
make distcheck failed few times for me.
* I had to change order of executing makefile
(add "." to SUBDIRS before directory "src/tests/cwrap")
* add missing script to tarball (makefile)
* add missing tests files (passwd,group) to tarball (makefile)
* modify paths in cwrap_test_setup to absolute paths.
Attaching diff which fixes make distcheck failures.
LS