Resolves: upstream#3621: FleetCommander integration must not require capability DAC_OVERRIDE

Together with the patches backported from upstream, we're changing
the deskprofilepath permissions from 755 to 751, reflecting the
upstream spec file changes.

Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
This commit is contained in:
Fabiano Fidêncio 2018-02-14 22:20:50 +01:00
parent 199a72e62a
commit 4b1fe8a0ab
7 changed files with 540 additions and 1 deletions

View File

@ -0,0 +1,38 @@
From 0fce902c563c3b54f2e67235668273ff7ff40752 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fidencio@redhat.com>
Date: Tue, 13 Feb 2018 22:02:45 +0100
Subject: [PATCH 83/88] DESKPROFILE: Harden the permission of deskprofilepath
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
After discussing the permissions with Simo, we have agreed on
having the deskprofile dir with the minimal set of permissions
needed
Related:
https://pagure.io/SSSD/sssd/issue/3621
Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Simo Sorce <simo@redhat.com>
---
contrib/sssd.spec.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
index f4430b424..37efcbff5 100644
--- a/contrib/sssd.spec.in
+++ b/contrib/sssd.spec.in
@@ -968,7 +968,7 @@ done
%if (0%{?with_secrets} == 1)
%attr(700,root,root) %dir %{secdbpath}
%endif
-%attr(755,sssd,sssd) %dir %{deskprofilepath}
+%attr(751,sssd,sssd) %dir %{deskprofilepath}
%ghost %attr(0644,sssd,sssd) %verify(not md5 size mtime) %{mcpath}/passwd
%ghost %attr(0644,sssd,sssd) %verify(not md5 size mtime) %{mcpath}/group
%ghost %attr(0644,sssd,sssd) %verify(not md5 size mtime) %{mcpath}/initgroups
--
2.14.3

View File

@ -0,0 +1,53 @@
From b576b290d3d7e165269edf36d6be27bc1441a688 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fidencio@redhat.com>
Date: Sat, 20 Jan 2018 15:06:37 +0100
Subject: [PATCH 84/88] DESKPROFILE: Soften umask for the domain's dir
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The default umask (0177) is way too strict, not allowing us to create
the domain's dir, which has to have its mode set as 751.
In order to solve this, let's soften the umask to 0026.
This issue was exposed due to CAP_DAC_OVERRIDE being removed from Fedora
package.
Resolves:
https://pagure.io/SSSD/sssd/issue/3621
Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Simo Sorce <simo@redhat.com>
---
src/providers/ipa/ipa_deskprofile_rules_util.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/providers/ipa/ipa_deskprofile_rules_util.c b/src/providers/ipa/ipa_deskprofile_rules_util.c
index 01b7d0527..989f3aadd 100644
--- a/src/providers/ipa/ipa_deskprofile_rules_util.c
+++ b/src/providers/ipa/ipa_deskprofile_rules_util.c
@@ -229,6 +229,7 @@ ipa_deskprofile_rules_create_user_dir(
char *domain;
char *domain_dir;
errno_t ret;
+ mode_t old_umask;
tmp_ctx = talloc_new(NULL);
if (tmp_ctx == NULL) {
@@ -243,8 +244,10 @@ ipa_deskprofile_rules_create_user_dir(
goto done;
}
- ret = sss_create_dir(IPA_DESKPROFILE_RULES_USER_DIR, domain, 0755,
+ old_umask = umask(0026);
+ ret = sss_create_dir(IPA_DESKPROFILE_RULES_USER_DIR, domain, 0751,
getuid(), getgid());
+ umask(old_umask);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE,
"Failed to create the directory \"%s/%s\" that would be used to "
--
2.14.3

View File

@ -0,0 +1,50 @@
From 2c5b03913c54234efdabcff83de368bae72dc799 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fidencio@redhat.com>
Date: Sat, 20 Jan 2018 23:58:14 +0100
Subject: [PATCH 85/88] DESKPROFILE: Fix the permissions and soften the umask
for user's dir
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The user dir has been created as 0600 and owned by the user. It doesn't
work anymore as CAP_DAC_OVERRIDE has been dropped from our systemd
service upstream.
In order to have it working again, let's change it to 0700 (as the
executable bit is needed for creating a file inside a folder) and soften
the default umask from (0177) to (0077) to be able to create this dir.
This issue was exposed due to CAP_DAC_OVERRIDE being removed from Fedora
package.
Resolves:
https://pagure.io/SSSD/sssd/issue/3621
Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Simo Sorce <simo@redhat.com>
---
src/providers/ipa/ipa_deskprofile_rules_util.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/providers/ipa/ipa_deskprofile_rules_util.c b/src/providers/ipa/ipa_deskprofile_rules_util.c
index 989f3aadd..0846b16f6 100644
--- a/src/providers/ipa/ipa_deskprofile_rules_util.c
+++ b/src/providers/ipa/ipa_deskprofile_rules_util.c
@@ -264,7 +264,11 @@ ipa_deskprofile_rules_create_user_dir(
goto done;
}
- ret = sss_create_dir(domain_dir, shortname, 0600, uid, gid);
+ /* In order to read, create and traverse the directory, we need to have its
+ * permissions set as 'rwx------' (700). */
+ old_umask = umask(0077);
+ ret = sss_create_dir(domain_dir, shortname, 0700, uid, gid);
+ umask(old_umask);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE,
"Failed to create the directory \"%s/%s/%s\" that would be used "
--
2.14.3

View File

@ -0,0 +1,144 @@
From 07ae0da06c0d94a3198e484d0de28c9282c4d6cd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fidencio@redhat.com>
Date: Mon, 22 Jan 2018 11:49:23 +0100
Subject: [PATCH 86/88] DESKPROFILE: Use seteuid()/setegid() to create the
profile
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
In order to create the file, having its owner properly, let's use
seteuid()/setegid() to create when creating the profile, as due to the
drop of the CAP_DAC_OVERRIDE "root" doesn't have access to the folder
where the profile will be created anymore.
By adopting the seteuid()/setegid() solution, calling fchown() in the
profile doesn't make sense, thus it was also removed.
This issue was exposed due to CAP_DAC_OVERRIDE being removed from Fedora
package.
Resolves:
https://pagure.io/SSSD/sssd/issue/3621
Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Simo Sorce <simo@redhat.com>
---
src/providers/ipa/ipa_deskprofile_rules_util.c | 70 ++++++++++++++++++++++++--
1 file changed, 66 insertions(+), 4 deletions(-)
diff --git a/src/providers/ipa/ipa_deskprofile_rules_util.c b/src/providers/ipa/ipa_deskprofile_rules_util.c
index 0846b16f6..eb04a69f8 100644
--- a/src/providers/ipa/ipa_deskprofile_rules_util.c
+++ b/src/providers/ipa/ipa_deskprofile_rules_util.c
@@ -706,6 +706,8 @@ ipa_deskprofile_rules_save_rule_to_disk(
const char *extension = "json";
uint32_t prio;
int fd = -1;
+ gid_t orig_gid;
+ uid_t orig_uid;
errno_t ret;
tmp_ctx = talloc_new(mem_ctx);
@@ -713,6 +715,9 @@ ipa_deskprofile_rules_save_rule_to_disk(
return ENOMEM;
}
+ orig_gid = getegid();
+ orig_uid = geteuid();
+
ret = sysdb_attrs_get_string(rule, IPA_CN, &rule_name);
if (ret != EOK) {
DEBUG(SSSDBG_TRACE_FUNC,
@@ -875,6 +880,26 @@ ipa_deskprofile_rules_save_rule_to_disk(
goto done;
}
+ ret = setegid(gid);
+ if (ret == -1) {
+ ret = errno;
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Unable to set effective group id (%"PRIu32") of the domain's "
+ "process [%d]: %s\n",
+ gid, ret, sss_strerror(ret));
+ goto done;
+ }
+
+ ret = seteuid(uid);
+ if (ret == -1) {
+ ret = errno;
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Unable to set effective user id (%"PRIu32") of the domain's "
+ "process [%d]: %s\n",
+ uid, ret, sss_strerror(ret));
+ goto done;
+ }
+
fd = open(filename_path, O_WRONLY | O_CREAT | O_TRUNC, 0600);
if (fd == -1) {
ret = errno;
@@ -895,12 +920,23 @@ ipa_deskprofile_rules_save_rule_to_disk(
goto done;
}
- ret = fchown(fd, uid, gid);
- if (ret != EOK) {
+ ret = seteuid(orig_uid);
+ if (ret == -1) {
ret = errno;
DEBUG(SSSDBG_CRIT_FAILURE,
- "Failed to own the Desktop Profile Rule file \"%s\" [%d]: %s\n",
- filename_path, ret, sss_strerror(ret));
+ "Failed to set the effect user id (%"PRIu32") of the domain's "
+ "process [%d]: %s\n",
+ orig_uid, ret, sss_strerror(ret));
+ goto done;
+ }
+
+ ret = setegid(orig_gid);
+ if (ret == -1) {
+ ret = errno;
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Failed to set the effect group id (%"PRIu32") of the domain's "
+ "process [%d]: %s\n",
+ orig_gid, ret, sss_strerror(ret));
goto done;
}
@@ -910,6 +946,32 @@ done:
if (fd != -1) {
close(fd);
}
+ if (geteuid() != orig_uid) {
+ ret = seteuid(orig_uid);
+ if (ret == -1) {
+ ret = errno;
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Unable to set effective user id (%"PRIu32") of the "
+ "domain's process [%d]: %s\n",
+ orig_uid, ret, sss_strerror(ret));
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Sending SIGUSR2 to the process: %d\n", getpid());
+ kill(getpid(), SIGUSR2);
+ }
+ }
+ if (getegid() != orig_gid) {
+ ret = setegid(orig_gid);
+ if (ret == -1) {
+ ret = errno;
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Unable to set effective group id (%"PRIu32") of the "
+ "domain's process. Let's have the process restartd!\n",
+ orig_gid);
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Sending SIGUSR2 to the process: %d\n", getpid());
+ kill(getpid(), SIGUSR2);
+ }
+ }
talloc_free(tmp_ctx);
return ret;
}
--
2.14.3

View File

@ -0,0 +1,207 @@
From 1a011c4f20e80f2bcb4d10a4d690b3a88c2fd70d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fidencio@redhat.com>
Date: Wed, 31 Jan 2018 16:50:38 +0100
Subject: [PATCH 87/88] DESKPROFILE: Use seteuid()/setegid() to delete the
profile/user's dir
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Let's use seteuid()/setegid() in order to properly delete the desktop
profiles related files.
Some malabarism has been introduced in order to proper delete those
dirs/files as:
/var/lib/sss/deskprofile/ipa.example/admin/profile
------------------------ ----------- ----- -------
| | | |
v | | |
Created by sssd package, | | |
not touching at all | | |
v | |
This one is owned by | |
root:root and has 751 | |
as permissions | |
v |
This one is owned by |
admin:admins and has |
0700 as permissions |
v
This one is owned by admin:admins
and has 0600 as permissions
So, when deleting we do:
- as admin:
- sss_remove_subtree("/var/lib/sss/deskprofile/ipa.example/admin/");
We can't remove the "admin" dir itself as it would require different
permissions in the domain's folder and that's something we don't
want to change
- as root:
- sss_remove_tree("/var/lib/sss/deskprofile/ipa.example/admin/");
Now we just removed the "admin" dir. The main reason behind not
being able to just delete it as root is because the permissions of
the file and dirs do not allow root to access then when not relying
in the CAP_DAC_OVERRIDE
This issue was exposed due to the CAP_DAC_OVERRIDE being removed from
Fedora package.
Resolves:
https://pagure.io/SSSD/sssd/issue/3621
Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Simo Sorce <simo@redhat.com>
---
src/providers/ipa/ipa_deskprofile_rules_util.c | 91 ++++++++++++++++++++++++--
src/providers/ipa/ipa_deskprofile_rules_util.h | 4 +-
src/providers/ipa/ipa_session.c | 4 +-
3 files changed, 93 insertions(+), 6 deletions(-)
diff --git a/src/providers/ipa/ipa_deskprofile_rules_util.c b/src/providers/ipa/ipa_deskprofile_rules_util.c
index eb04a69f8..2102713d6 100644
--- a/src/providers/ipa/ipa_deskprofile_rules_util.c
+++ b/src/providers/ipa/ipa_deskprofile_rules_util.c
@@ -977,21 +977,104 @@ done:
}
errno_t
-ipa_deskprofile_rules_remove_user_dir(const char *user_dir)
+ipa_deskprofile_rules_remove_user_dir(const char *user_dir,
+ uid_t uid,
+ gid_t gid)
{
+ gid_t orig_gid;
+ uid_t orig_uid;
errno_t ret;
+ orig_gid = getegid();
+ orig_uid = geteuid();
+
+ ret = setegid(gid);
+ if (ret == -1) {
+ ret = errno;
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Unable to set effective group id (%"PRIu32") of the domain's "
+ "process [%d]: %s\n",
+ gid, ret, sss_strerror(ret));
+ goto done;
+ }
+
+ ret = seteuid(uid);
+ if (ret == -1) {
+ ret = errno;
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Unable to set effective user id (%"PRIu32") of the domain's "
+ "process [%d]: %s\n",
+ uid, ret, sss_strerror(ret));
+ goto done;
+ }
+
+ ret = sss_remove_subtree(user_dir);
+ if (ret != EOK && ret != ENOENT) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Cannot remove \"%s\" directory [%d]: %s\n",
+ user_dir, ret, sss_strerror(ret));
+ goto done;
+ }
+
+ ret = seteuid(orig_uid);
+ if (ret == -1) {
+ ret = errno;
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Failed to set the effect user id (%"PRIu32") of the domain's "
+ "process [%d]: %s\n",
+ orig_uid, ret, sss_strerror(ret));
+ goto done;
+ }
+
+ ret = setegid(orig_gid);
+ if (ret == -1) {
+ ret = errno;
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Failed to set the effect group id (%"PRIu32") of the domain's "
+ "process [%d]: %s\n",
+ orig_gid, ret, sss_strerror(ret));
+ goto done;
+ }
+
ret = sss_remove_tree(user_dir);
if (ret == ENOENT) {
- return EOK;
+ ret = EOK;
} else if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE,
"Cannot remove \"%s\" directory [%d]: %s\n",
user_dir, ret, sss_strerror(ret));
- return ret;
+ goto done;
}
- return EOK;
+ ret = EOK;
+
+done:
+ if (geteuid() != orig_uid) {
+ ret = seteuid(orig_uid);
+ if (ret == -1) {
+ ret = errno;
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "unable to set effective user id (%"PRIu32") of the "
+ "domain's process [%d]: %s\n",
+ orig_uid, ret, sss_strerror(ret));
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Sending SIGUSR2 to the process: %d\n", getpid());
+ kill(getpid(), SIGUSR2);
+ }
+ }
+ if (getegid() != orig_gid) {
+ ret = setegid(orig_gid);
+ if (ret == -1) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Unable to set effective user id (%"PRIu32") of the "
+ "domain's process [%d]: %s\n",
+ orig_uid, ret, sss_strerror(ret));
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "Sending SIGUSR2 to the process: %d\n", getpid());
+ kill(getpid(), SIGUSR2);
+ }
+ }
+ return ret;
}
errno_t
diff --git a/src/providers/ipa/ipa_deskprofile_rules_util.h b/src/providers/ipa/ipa_deskprofile_rules_util.h
index 61f404df8..4016dbccf 100644
--- a/src/providers/ipa/ipa_deskprofile_rules_util.h
+++ b/src/providers/ipa/ipa_deskprofile_rules_util.h
@@ -45,7 +45,9 @@ ipa_deskprofile_rules_save_rule_to_disk(
uid_t uid,
gid_t gid);
errno_t
-ipa_deskprofile_rules_remove_user_dir(const char *user_dir);
+ipa_deskprofile_rules_remove_user_dir(const char *user_dir,
+ uid_t uid,
+ gid_t gid);
errno_t
deskprofile_get_cached_priority(struct sss_domain_info *domain,
diff --git a/src/providers/ipa/ipa_session.c b/src/providers/ipa/ipa_session.c
index 3c7dd33c3..25ad5ce51 100644
--- a/src/providers/ipa/ipa_session.c
+++ b/src/providers/ipa/ipa_session.c
@@ -524,7 +524,9 @@ ipa_pam_session_handler_send(TALLOC_CTX *mem_ctx,
/* As no proper merging mechanism has been implemented yet ...
* let's just remove the user directory stored in the disk as it's
* going to be created again in case there's any rule fetched. */
- ret = ipa_deskprofile_rules_remove_user_dir(state->user_dir);
+ ret = ipa_deskprofile_rules_remove_user_dir(state->user_dir,
+ state->uid,
+ state->gid);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE,
"ipa_deskprofile_rules_remove_user_dir() failed.\n");
--
2.14.3

View File

@ -0,0 +1,39 @@
From f0cbe890adf696d8318373203580d709f3d38d8c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fidencio@redhat.com>
Date: Mon, 5 Feb 2018 07:56:53 +0100
Subject: [PATCH 88/88] DESKPROFILE: Set the profile permissions to read-only
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Sumit suggested to have the profile permissions with the least possible
permissions and it does make sense.
So, let's change it from read-write to read-only.
Related:
https://pagure.io/SSSD/sssd/issue/362
Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Simo Sorce <simo@redhat.com>
---
src/providers/ipa/ipa_deskprofile_rules_util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/providers/ipa/ipa_deskprofile_rules_util.c b/src/providers/ipa/ipa_deskprofile_rules_util.c
index 2102713d6..e52587378 100644
--- a/src/providers/ipa/ipa_deskprofile_rules_util.c
+++ b/src/providers/ipa/ipa_deskprofile_rules_util.c
@@ -900,7 +900,7 @@ ipa_deskprofile_rules_save_rule_to_disk(
goto done;
}
- fd = open(filename_path, O_WRONLY | O_CREAT | O_TRUNC, 0600);
+ fd = open(filename_path, O_WRONLY | O_CREAT | O_TRUNC, 0400);
if (fd == -1) {
ret = errno;
DEBUG(SSSDBG_CRIT_FAILURE,
--
2.14.3

View File

@ -123,6 +123,12 @@ Patch0079: 0079-test_files_provider-Regression-test-for-implicit_fil.patch
Patch0080: 0080-BUILD-Add-missing-libs-found-by-Wl-z-defs.patch
Patch0081: 0081-SELINUX-Check-if-SELinux-is-managed-in-selinux_child.patch
Patch0082: 0082-DESKPROFILE-Add-checks-for-user-and-host-category.patch
Patch0083: 0083-DESKPROFILE-Harden-the-permission-of-deskprofilepath.patch
Patch0084: 0084-DESKPROFILE-Soften-umask-for-the-domain-s-dir.patch
Patch0085: 0085-DESKPROFILE-Fix-the-permissions-and-soften-the-umask.patch
Patch0086: 0086-DESKPROFILE-Use-seteuid-setegid-to-create-the-profil.patch
Patch0087: 0087-DESKPROFILE-Use-seteuid-setegid-to-delete-the-profil.patch
Patch0088: 0088-DESKPROFILE-Set-the-profile-permissions-to-read-only.patch
Patch0502: 0502-SYSTEMD-Use-capabilities.patch
Patch0503: 0503-Disable-stopping-idle-socket-activated-responders.patch
@ -914,7 +920,7 @@ done
%attr(700,root,root) %dir %{dbpath}
%attr(755,root,root) %dir %{mcpath}
%attr(700,root,root) %dir %{secdbpath}
%attr(755,root,root) %dir %{deskprofilepath}
%attr(751,root,root) %dir %{deskprofilepath}
%ghost %attr(0644,root,root) %verify(not md5 size mtime) %{mcpath}/passwd
%ghost %attr(0644,root,root) %verify(not md5 size mtime) %{mcpath}/group
%ghost %attr(0644,root,root) %verify(not md5 size mtime) %{mcpath}/initgroups
@ -1328,6 +1334,8 @@ fi
* Wed Feb 14 2018 Fabiano Fidêncio <fidencio@fedoraproject.org> - 1.16.0-12
- Resolves: rhbz#1538643 - SSSD crashes when retrieving a Desktop Profile
with no specific host/hostgroup set
- Resolves: upstream#3621 - FleetCommander integration must not require
capability DAC_OVERRIDE
* Wed Feb 07 2018 Lukas Slebodnik <lslebodn@fedoraproject.org> - 1.16.0-11
- Resolves: upstream#3618 - selinux_child segfaults in a docker container