[SSSD] [sssd PR#614][comment] nss_protocol_fill_initgr: skip incomplete groups instead of bailing out
URL: https://github.com/SSSD/sssd/pull/614 Title: #614: nss_protocol_fill_initgr: skip incomplete groups instead of bailing out asheplyakov commented: """ @fidencio > would you have a cache dump, logs or even a machine that you could give us > access Unfortunately no. That was a client's machine, and we had no permission to copy any logs in first place. We couldn't reproduce the problem locally, perhaps it takes some tricky AD setup (and we had no permission examine their AD configuration). """ See the full comment at https://github.com/SSSD/sssd/pull/614#issuecomment-411689819 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/sssd-devel@lists.fedorahosted.org/message/UX6MHI62RTDA3EDMT6IMHYFGL5JNT2JC/
[SSSD] [sssd PR#616][edited] become_user_ex: add supplementary groups so ad provider can access keytab
URL: https://github.com/SSSD/sssd/pull/616 Author: asheplyakov Title: #616: become_user_ex: add supplementary groups so ad provider can access keytab Action: edited Changed field: title Original value: """ become_user: add supplementary groups so ad provider can access keytab """ ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/sssd-devel@lists.fedorahosted.org/message/JLXRSAR57XO4JCN5DF6EJUC6MPYSR6NO/
[SSSD] [sssd PR#616][synchronized] become_user: add supplementary groups so ad provider can access keytab
URL: https://github.com/SSSD/sssd/pull/616
Author: asheplyakov
Title: #616: become_user: add supplementary groups so ad provider can access
keytab
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/616/head:pr616
git checkout pr616
From 25ed58a1c020accf8955ba35b35026a95c1da28d Mon Sep 17 00:00:00 2001
From: Alexey Sheplyakov
Date: Tue, 10 Jul 2018 15:42:31 +
Subject: [PATCH] become_user_ex: add supplementary groups so ad provider can
access keytab
For security reasons one might want to run providers as a non-privileged
user (say, _sssd). However some providers (in particular ad) might need
an access to restricted (non world-readable) files (for instance,
/etc/krb5.keytab). One of the possible ways to solve the problem is to
- add a special group (for instance, _keytab)
- set the owner:group of the file in question to root:_keytab
- set the permissions of the file in question to 640
- make the _sssd user a member of the _keytab group
For this to work it's necessary to assign supplementary groups when switching
to a non-privileged user. However in some contexts finding out supplementary
groups might be too expensive/inappropriate (case in point: `krb5_child`
switching to the user being authenticated). To solve the problem add a new
function `become_user_ex`, and call it where appropriate, i.e. `server_setup`
and `ldap_child`.
Note: `ldap_child` can use the same approach to avoid being SUID (when sssd
runs as a non-privileged user). However that's a subject of a separate patch.
---
src/providers/ldap/ldap_child.c| 2 +-
src/tests/cwrap/group | 1 +
src/tests/cwrap/passwd | 1 +
src/tests/cwrap/test_become_user.c | 88 ++
src/util/become_user.c | 42 ++
src/util/server.c | 2 +-
src/util/util.h| 1 +
7 files changed, 127 insertions(+), 10 deletions(-)
diff --git a/src/providers/ldap/ldap_child.c b/src/providers/ldap/ldap_child.c
index 8c11d7896f..c65b8ed6a2 100644
--- a/src/providers/ldap/ldap_child.c
+++ b/src/providers/ldap/ldap_child.c
@@ -701,7 +701,7 @@ int main(int argc, const char *argv[])
}
DEBUG(SSSDBG_TRACE_INTERNAL, "Kerberos context initialized\n");
-kerr = become_user(ibuf->uid, ibuf->gid);
+kerr = become_user_ex(ibuf->uid, ibuf->gid, true);
if (kerr != 0) {
DEBUG(SSSDBG_CRIT_FAILURE, "become_user failed.\n");
goto fail;
diff --git a/src/tests/cwrap/group b/src/tests/cwrap/group
index d0cea659ea..e84e23ed49 100644
--- a/src/tests/cwrap/group
+++ b/src/tests/cwrap/group
@@ -1,2 +1,3 @@
sssd:x:123:
+_keytab:x:321:_sssd
foogroup:x:10001:
diff --git a/src/tests/cwrap/passwd b/src/tests/cwrap/passwd
index 862ccfe03e..4fe01c619b 100644
--- a/src/tests/cwrap/passwd
+++ b/src/tests/cwrap/passwd
@@ -1,2 +1,3 @@
sssd:x:123:456:sssd unprivileged user:/:/sbin/nologin
+_sssd:x:987:654:sssd unprivileged user:/:/sbin/nologin
foobar:x:10001:10001:User for SSSD testing:/home/foobar:/bin/bash
diff --git a/src/tests/cwrap/test_become_user.c b/src/tests/cwrap/test_become_user.c
index e63cde9d72..52a9123e5c 100644
--- a/src/tests/cwrap/test_become_user.c
+++ b/src/tests/cwrap/test_become_user.c
@@ -70,6 +70,93 @@ void test_become_user(void **state)
assert_int_equal(WEXITSTATUS(status), 0);
}
+void test_become_user_ex(void **state)
+{
+struct passwd *sssd;
+TALLOC_CTX *tmp_ctx;
+gid_t *groups, *actual_groups;
+errno_t ret;
+pid_t pid, wpid;
+int status;
+int group_count, actual_group_count;
+
+assert_true(leak_check_setup());
+
+tmp_ctx = talloc_new(global_talloc_context);
+assert_non_null(tmp_ctx);
+check_leaks_push(tmp_ctx);
+
+/* Must root as root, real or fake */
+assert_int_equal(geteuid(), 0);
+
+sssd = getpwnam("_sssd");
+assert_non_null(sssd);
+
+getgrouplist(sssd->pw_name, sssd->pw_gid, NULL, _count);
+assert_int_not_equal(group_count, 0);
+groups = talloc_array(tmp_ctx, gid_t, group_count);
+assert_non_null(groups);
+actual_groups = talloc_array(tmp_ctx, gid_t, group_count);
+assert_non_null(actual_groups);
+status = getgrouplist(sssd->pw_name, sssd->pw_gid, groups, _count);
+assert_int_equal(status, group_count);
+
+pid = fork();
+if (pid == 0) {
+/* Change the UID in a child */
+ret = become_user_ex(sssd->pw_uid, sssd->pw_gid, true);
+assert_int_equal(ret, EOK);
+
+/* Make sure we have the requested UID and GID now and there
+ * are no supplementary groups
+ */
+assert_int_equal(geteuid(), sssd->pw_uid);
+assert_int_equal(getegid(), sssd->pw_gid);
+assert_int_equal(getuid(), sssd->pw_uid);
+assert_int_equal(getgid(), sssd->pw_gid);
+
+/* Another become_
[SSSD] [sssd PR#614][comment] nss_protocol_fill_initgr: skip incomplete groups instead of bailing out
URL: https://github.com/SSSD/sssd/pull/614 Title: #614: nss_protocol_fill_initgr: skip incomplete groups instead of bailing out asheplyakov commented: """ @fidencio: I'm OK with the proposed commit message change """ See the full comment at https://github.com/SSSD/sssd/pull/614#issuecomment-405263666 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/sssd-devel@lists.fedorahosted.org/message/JA6INQYEPNAGFSRSJYN63QHLGXDA6FQJ/
[SSSD] [sssd PR#616][comment] become_user: add supplementary groups so ad provider can access keytab
URL: https://github.com/SSSD/sssd/pull/616 Title: #616: become_user: add supplementary groups so ad provider can access keytab asheplyakov commented: """ > become_user() is also used in krb5_child to switch to the user trying to log > in to create the ccache with the right permissions. Yep, calling initgroups in this context is indeed inappropriate. What about adding a new function `become_user_ext`, which adds supplementary groups, and using it where appropriate, for instance, in [server_setup](https://github.com/SSSD/sssd/blob/519354d079731e673244a8e3851e5c5522d1b45e/src/util/server.c#L487) and [data_provider_be.c:main](https://github.com/SSSD/sssd/blob/1038473e1c9775d1273809c46673fa1475e50937/src/providers/data_provider_be.c#L630)? """ See the full comment at https://github.com/SSSD/sssd/pull/616#issuecomment-405041095 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/sssd-devel@lists.fedorahosted.org/message/GQRNBOWKX2KRJHCK7BN223W3KESPZUYS/
[SSSD] [sssd PR#616][comment] become_user: add supplementary groups so ad provider can access keytab
URL: https://github.com/SSSD/sssd/pull/616 Title: #616: become_user: add supplementary groups so ad provider can access keytab asheplyakov commented: """ > I wonder if you wouldn't be able to achieve the same by setting the primary > group of the _sssd user to _keytab? This way other daemons which need access to keytab (apache, postgresql, you name it) might be able to read sssd caches and logs (which belong to _sssd:_keytab). It looks like sssd is careful enough to chmod 600 all those files, yet it's better to avoid possible bugs. > could the keytab file allow the sssd user to read the contents with a POSIX > ACL? - often keytab is managed automatically by `samba-tool join` or similar tools. Patching these tools to set proper ACLs *when sssd package is installed* doesn't look like a good idea. On the other hand, it's enough to patch libkrb5 to force correct group/permissions of /etc/krb5.keytab, and the patch is simple enough (see http://git.altlinux.org/people/sin/packages/?p=krb5.git;a=blob;f=krb5-1.16-alt-default_keytab_group.patch;h=3ea8c536d57045002b39e77992d7bf36cc94c3ac;hb=bd27c4dfd73611a0192691a2567101f4f5c89936#l100) - also not every filesystem/kernel support POSIX ACLs (think of those NAS devices), but virtually all sensible filesystems know what uid/gid are. """ See the full comment at https://github.com/SSSD/sssd/pull/616#issuecomment-405040134 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/sssd-devel@lists.fedorahosted.org/message/YAZ3PBQJNA323MBAFZ47TEPE6XE3TFTA/
[SSSD] [sssd PR#616][opened] become_user: add supplementary groups so ad provider can access keytab
URL: https://github.com/SSSD/sssd/pull/616
Author: asheplyakov
Title: #616: become_user: add supplementary groups so ad provider can access
keytab
Action: opened
PR body:
"""
For security reasons one might want to run providers as a non-privileged
user (say, _sssd). However some providers (in particular ad) might need
an access to restricted (non world-readable) files (for instance,
/etc/krb5.keytab). One of the possible ways to solve the problem is to
- add a special group (for instance, _keytab)
- set the owner:group of the file in question to root:_keytab
- set the permissions of the file in question to 640
- make the _sssd user a member of the _keytab group
For this to work become_user should assign supplementary groups, which
is what this patch does.
"""
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/616/head:pr616
git checkout pr616
From b27b33d4521e5b9b8c90b0abbbee753a8989b493 Mon Sep 17 00:00:00 2001
From: Alexey Sheplyakov
Date: Tue, 10 Jul 2018 15:42:31 +
Subject: [PATCH] become_user: add supplementary groups so ad provider can
access keytab
For security reasons one might want to run providers as a non-privileged
user (say, _sssd). However some providers (in particular ad) might need
an access to restricted (non world-readable) files (for instance,
/etc/krb5.keytab). One of the possible ways to solve the problem is to
- add a special group (for instance, _keytab)
- set the owner:group of the file in question to root:_keytab
- set the permissions of the file in question to 640
- make the _sssd user a member of the _keytab group
For this to work become_user should assign supplementary groups, which
is what this patch does.
---
src/tests/cwrap/test_become_user.c | 6 +-
src/util/become_user.c | 16 +---
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/src/tests/cwrap/test_become_user.c b/src/tests/cwrap/test_become_user.c
index e63cde9d7..88bffbd6b 100644
--- a/src/tests/cwrap/test_become_user.c
+++ b/src/tests/cwrap/test_become_user.c
@@ -30,9 +30,11 @@
void test_become_user(void **state)
{
struct passwd *sssd;
+gid_t gid;
errno_t ret;
pid_t pid, wpid;
int status;
+int group_count;
/* Must root as root, real or fake */
assert_int_equal(geteuid(), 0);
@@ -58,7 +60,9 @@ void test_become_user(void **state)
ret = become_user(sssd->pw_uid, sssd->pw_gid);
assert_int_equal(ret, EOK);
-assert_int_equal(getgroups(0, NULL), 0);
+group_count = getgroups(1, );
+assert_int_equal(1, group_count);
+assert_int_equal(gid, sssd->pw_gid);
exit(0);
}
diff --git a/src/util/become_user.c b/src/util/become_user.c
index c3f726d18..cc43ef588 100644
--- a/src/util/become_user.c
+++ b/src/util/become_user.c
@@ -24,11 +24,13 @@
#include "util/util.h"
#include
+#include
errno_t become_user(uid_t uid, gid_t gid)
{
uid_t cuid;
int ret;
+struct passwd *pwd;
DEBUG(SSSDBG_FUNC_DATA,
"Trying to become user [%"SPRIuid"][%"SPRIgid"].\n", uid, gid);
@@ -40,12 +42,20 @@ errno_t become_user(uid_t uid, gid_t gid)
return EOK;
}
-/* drop supplementary groups first */
-ret = setgroups(0, NULL);
+/* init supplmentary groups */
+errno = 0;
+pwd = getpwuid(uid);
+if (pwd == NULL || pwd->pw_name == NULL) {
+ret = errno ?: ENOENT;
+DEBUG(SSSDBG_CRIT_FAILURE,
+ "getpwuid failed [%d][%s].\n", ret, strerror(ret));
+return ret;
+}
+ret = initgroups(pwd->pw_name, gid);
if (ret == -1) {
ret = errno;
DEBUG(SSSDBG_CRIT_FAILURE,
- "setgroups failed [%d][%s].\n", ret, strerror(ret));
+ "initgroups failed [%d][%s].\n", ret, strerror(ret));
return ret;
}
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedoraproject.org/archives/list/sssd-devel@lists.fedorahosted.org/message/QA6FGDGJT7CFYYCEIVENMNZTCQNZOWJR/
[SSSD] [sssd PR#614][opened] nss_protocol_fill_initgr: skip incomplete groups instead of bailing out
URL: https://github.com/SSSD/sssd/pull/614
Author: asheplyakov
Title: #614: nss_protocol_fill_initgr: skip incomplete groups instead of
bailing out
Action: opened
PR body:
"""
Suppose the user U is a member of (AD) groups D1\A, D1\B, D2\X, and no
domain controllers in the domain D2 can be reached at the moment (and
there are no cached info). As of now initgroups won't assign any groups
at all. To improve the behavior skip the incomplete groups so initgroup
assigns at least some groups (D1\A, D1\B in the above example).
"""
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/614/head:pr614
git checkout pr614
From 4715b03dc7d5ad980cf0c3b8a7ae2823b30acbce Mon Sep 17 00:00:00 2001
From: Alexey Sheplyakov
Date: Tue, 10 Jul 2018 14:51:15 +
Subject: [PATCH] nss_protocol_fill_initgr: skip incomplete groups instead of
bailing out
Suppose the user U is a member of (AD) groups D1\A, D1\B, D2\X, and no
domain controllers in the domain D2 can be reached at the moment (and
there are no cached info). As of now initgroups won't assign any groups
at all. To improve the behavior skip the incomplete groups so initgroup
assigns at least some groups (D1\A, D1\B in the above example).
---
src/responder/nss/nss_protocol_grent.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/responder/nss/nss_protocol_grent.c b/src/responder/nss/nss_protocol_grent.c
index b89ce2bc5..a697e86ef 100644
--- a/src/responder/nss/nss_protocol_grent.c
+++ b/src/responder/nss/nss_protocol_grent.c
@@ -365,11 +365,10 @@ nss_protocol_fill_initgr(struct nss_ctx *nss_ctx,
if (posix != NULL && strcmp(posix, "FALSE") == 0) {
continue;
} else {
-DEBUG(SSSDBG_CRIT_FAILURE,
+DEBUG(SSSDBG_MINOR_FAILURE,
"Incomplete group object [%s] for initgroups! "
- "Aborting.\n", ldb_dn_get_linearized(msg->dn));
-ret = EINVAL;
-goto done;
+ "Skipping.\n", ldb_dn_get_linearized(msg->dn));
+continue;
}
}
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedoraproject.org/archives/list/sssd-devel@lists.fedorahosted.org/message/GM5UWIZSZCZ765JAWURE2YUDZUWB2BXE/
