From 9a9bfd2d94f261fd468ae20a93d862ddaf80ae88 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: May 05 2018 19:59:47 +0000 Subject: [PATCH 1/2] selinux_child: Allow to query sssd The function getpwnam_r is indirectly used ins selinux_child on few places. (in libselinux and libsemanage) There is not any reason why we should block nss calls with sssd. It is a child process and loop cannot be created. (BTW it is also allowed in krb_child and proxy_child) #0 _nss_sss_getpwnam_r (name=0x55c0e6471a50 "user4_2", result=0x7ffe9ab0d05, buffer=0x55c0e64741a0 "\200é\256\177\177", buflen=1024, errnop=0x7f7fafbcdb08) at src/sss_client/nss_passwd.c:132 #1 0x00007f7fae7ad48f in __getpwnam_r (name=name@entry=0x55c0e6471a50 "user4_2", resbuf=resbuf@entry=0x7ffe9ab0d050, buffer=buffer@entry=0x55c0e64741a0 "\200é\256\177\177", buflen=buflen@entry=1024, result=result@entry=0x7ffe9ab0d048) at ../nss/getXXbyYY_r.c:316 #2 0x00007f7faeabc9e2 in get_default_gid (name=0x55c0e6471a50 "user4_2") at seusers.c:105 #3 getseuserbyname (name=0x55c0e6471a50 "user4_2", r_seuser=0x7ffe9ab0d0f0, r_level=0x7ffe9ab0d0f8) at seusers.c:186 #4 0x000055c0e5126d02 in seuser_needs_update (ibuf=0x55c0e64718e0) at src/providers/ipa/selinux_child.c:175 #5 main (argc=, argv=) at src/providers/ipa/selinux_child.c:332 #0 _nss_sss_getpwnam_r (name=0x55c0e647dda0 "user3_1", result=0x7ffe9ab0cce0, buffer=0x55c0e6482180 "\240AG\346\300U", buflen=1024, errnop=0x7f7fafbcdb08) at src/sss_client/nss_passwd.c:132 #1 0x00007f7fae7ad48f in __getpwnam_r (name=name@entry=0x55c0e647dda0 "user3_1", resbuf=resbuf@entry=0x7ffe9ab0cce0, buffer=buffer@entry=0x55c0e6482180 "\240AG\346\300U", buflen=buflen@entry=1024, result=result@entry=0x7ffe9ab0ccd8) at ../nss/getXXbyYY_r.c:316 #2 0x00007f7faece29b3 in add_user (head=head@entry=0x7ffe9ab0ce28, user=user@entry=0x55c0e64b5930, name=name@entry=0x55c0e647dda0 "user3_1", sename=sename@entry=0x55c0e647bdc0 "staff_u", selogin=selogin@entry=0x55c0e647dda0 "user3_1", s=) at genhomedircon.c:999 #3 0x00007f7faece334c in get_users (errors=, s=0x7ffe9ab0ce70) at genhomedircon.c:1167 #4 write_gen_home_dir_context (homedir_context_tpl=0x55c0e647d3d0, user_context_tpl=0x55c0e647a870, username_context_tpl=0x0, out=0x55c0e646fa80, s=0x7ffe9ab0ce70) at genhomedircon.c:1205 #5 write_context_file (out=, s=0x7ffe9ab0ce70) at genhomedircon.c:1317 #6 semanage_genhomedircon (sh=sh@entry=0x55c0e6476380, policydb=, usepasswd=, ignoredirs=) at genhomedircon.c:1382 #7 0x00007f7faecdfb95 in semanage_direct_commit (sh=0x55c0e6476380) at direct_api.c:1575 #8 0x00007f7faece4d6d in semanage_commit (sh=0x55c0e6476380) at handle.c:426 #9 0x000055c0e5127cf8 in sss_set_seuser (login_name=0x55c0e6471a5 "user4_2", seuser_name=0x55c0e6471960 "staff_u", mls=) at src/util/sss_semanage.c:335 #10 0x000055c0e5126eea in sc_set_seuser (mls=0x55c0e64719d0 "s0-s0:c0.c1023", seuser_name=0x55c0e6471960 "staff_u", login_name=0x55c0e6471a50 "user4_2") at src/providers/ipa/selinux_child.c:162 #11 main (argc=, argv=) at src/providers/ipa/selinux_child.c:334 Merges: https://pagure.io/SSSD/sssd/pull-request/3732 --- diff --git a/src/providers/ipa/selinux_child.c b/src/providers/ipa/selinux_child.c index a601b28..8177c65 100644 --- a/src/providers/ipa/selinux_child.c +++ b/src/providers/ipa/selinux_child.c @@ -263,6 +263,22 @@ int main(int argc, const char *argv[]) "Running with effective IDs: [%"SPRIuid"][%"SPRIgid"].\n", geteuid(), getegid()); + /* The functions semanage_genhomedircon and getseuserbyname use gepwnam_r + * and they might fail to return values if they are not in memory cache. + * [main] (0x0400): performing selinux operations + * [seuser_needs_update] (0x2000): getseuserbyname: ret: 0 + * seuser: unconfined_u mls: s0-s0:c0.c15 + * [libsemanage] (0x0020): semanage_genhomedircon returned error code -1. + * [sss_set_seuser] (0x0020): Cannot commit SELinux transaction + * [main] (0x0020): Cannot set SELinux login context. + * [main] (0x0020): selinux_child failed! + */ + if (unsetenv("_SSS_LOOPS") != 0) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Failed to unset _SSS_LOOPS, some libsemanage functions might " + "fail.\n"); + } + /* libsemanage calls access(2) which works with real IDs, not effective. * We need to switch also the real ID to 0. */ From 066332ef0ea05e9d73c0009b26d539ca51b55b99 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: May 05 2018 19:59:47 +0000 Subject: [PATCH 2/2] selinux_child: Fix crash with initialized key The semanage_seuser_key_t can be NULL in done section in case of issues with initializing semanage handle or failure with creating transaction. The semanage_seuser_key_free is not NULL tolerant therefore its better to prevent dereference of NULL pointer. #0 semanage_seuser_key_free_internal (key=0x0) at seuser_record.c:83 #1 0x000055c1f8a687ed in sss_set_seuser (login_name=0x55c1f9ff8a50 "user4_1", seuser_name=0x55c1f9ff8960 "staff_u", mls=) at src/util/sss_semanage.c:344 #2 0x000055c1f8a67eea in sc_set_seuser (mls=0x55c1f9ff89d0 "s0-s0:c0.c1023", seuser_name=0x55c1f9ff8960 "staff_u", login_name=0x55c1f9ff8a50 "user4_1") at src/providers/ipa/selinux_child.c:162 #3 main (argc=, argv=) at src/providers/ipa/selinux_child.c:334 Merges: https://pagure.io/SSSD/sssd/pull-request/3732 --- diff --git a/src/util/sss_semanage.c b/src/util/sss_semanage.c index 1150b62..7125205 100644 --- a/src/util/sss_semanage.c +++ b/src/util/sss_semanage.c @@ -341,7 +341,9 @@ int sss_set_seuser(const char *login_name, const char *seuser_name, ret = EOK; done: - semanage_seuser_key_free(key); + if (key != NULL { + semanage_seuser_key_free(key); + } sss_semanage_close(handle); return ret; }