[sssd/f18] Only create the SELinux login file if there are SELinux mappings on the IPA server

Jakub Hrozek jhrozek at fedoraproject.org
Fri Aug 17 13:09:55 UTC 2012


commit a3d7c670abc8f5535d41801a4df3aa2495ad0f5b
Author: Jakub Hrozek <jhrozek at redhat.com>
Date:   Fri Aug 17 14:59:01 2012 +0200

    Only create the SELinux login file if there are SELinux mappings on the IPA server

 ...to-remove-the-temp-login-file-if-already-.patch |   44 ++++
 ...-the-SELinux-login-file-if-there-are-mapp.patch |  223 ++++++++++++++++++++
 sssd.spec                                          |    8 +-
 3 files changed, 274 insertions(+), 1 deletions(-)
---
diff --git a/0002-Do-not-try-to-remove-the-temp-login-file-if-already-.patch b/0002-Do-not-try-to-remove-the-temp-login-file-if-already-.patch
new file mode 100644
index 0000000..354d491
--- /dev/null
+++ b/0002-Do-not-try-to-remove-the-temp-login-file-if-already-.patch
@@ -0,0 +1,44 @@
+From 79402313dc0d7f854b4334dd427e03b7baf0b9db Mon Sep 17 00:00:00 2001
+From: Jakub Hrozek <jhrozek at redhat.com>
+Date: Sun, 5 Aug 2012 22:03:11 +0200
+Subject: [PATCH 1/2] Do not try to remove the temp login file if already
+ renamed
+
+write_selinux_string() would try to unlink the temporary file even after
+it was renamed. Failure to unlink the file would not be fatal, but would
+produce a confusing error message.
+
+Also don't use "0" for the default fd number, that's reserved for stdin.
+Using -1 is safer.
+---
+ src/responder/pam/pamsrv_cmd.c | 5 +++--
+ 1 file changed, 3 insertions(+), 2 deletions(-)
+
+diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c
+index 8c9dd9b557982e04989bf9e63fd93ea294979252..944845a86dfa8166367029e6f7bddc478e5ebd03 100644
+--- a/src/responder/pam/pamsrv_cmd.c
++++ b/src/responder/pam/pamsrv_cmd.c
+@@ -366,7 +366,7 @@ static errno_t write_selinux_string(const char *username, char *string)
+     char *tmp_path = NULL;
+     ssize_t written;
+     int len;
+-    int fd = 0;
++    int fd = -1;
+     mode_t oldmask;
+     TALLOC_CTX *tmp_ctx;
+     char *full_string = NULL;
+@@ -437,9 +437,10 @@ static errno_t write_selinux_string(const char *username, char *string)
+     } else {
+         ret = EOK;
+     }
++    fd = -1;
+ 
+ done:
+-    if (fd > 0) {
++    if (fd != -1) {
+         close(fd);
+         if (unlink(tmp_path) < 0) {
+             DEBUG(SSSDBG_MINOR_FAILURE, ("Could not remove file [%s]",
+-- 
+1.7.11.2
+
diff --git a/0003-Only-create-the-SELinux-login-file-if-there-are-mapp.patch b/0003-Only-create-the-SELinux-login-file-if-there-are-mapp.patch
new file mode 100644
index 0000000..d1290e7
--- /dev/null
+++ b/0003-Only-create-the-SELinux-login-file-if-there-are-mapp.patch
@@ -0,0 +1,223 @@
+From f004e23af14fe020d81b8f97f30b448105b79606 Mon Sep 17 00:00:00 2001
+From: Jakub Hrozek <jhrozek at redhat.com>
+Date: Sun, 5 Aug 2012 22:37:09 +0200
+Subject: [PATCH 2/2] Only create the SELinux login file if there are mappings
+ on the server
+
+https://fedorahosted.org/sssd/ticket/1455
+
+In case there are no rules on the IPA server, we must simply avoid generating
+the login file. That would make us fall back to the system-wide default
+defined in /etc/selinux/targeted/seusers.
+
+The IPA default must be only used if there *are* rules on the server,
+but none matches.
+---
+ src/db/sysdb_selinux.c         |   7 +--
+ src/responder/pam/pamsrv_cmd.c | 120 ++++++++++++++++++++++++++---------------
+ 2 files changed, 77 insertions(+), 50 deletions(-)
+
+diff --git a/src/db/sysdb_selinux.c b/src/db/sysdb_selinux.c
+index eaf07b50a4435b11f937473e37c708edc6e2e0eb..976489503e0995b7e025146c23a7f4e7874c1643 100644
+--- a/src/db/sysdb_selinux.c
++++ b/src/db/sysdb_selinux.c
+@@ -364,7 +364,7 @@ errno_t sysdb_search_selinux_usermap_by_username(TALLOC_CTX *mem_ctx,
+     struct ldb_message **msgs = NULL;
+     struct sysdb_attrs *user;
+     struct sysdb_attrs *tmp_attrs;
+-    struct ldb_message **usermaps;
++    struct ldb_message **usermaps = NULL;
+     struct sss_domain_info *domain;
+     struct ldb_dn *basedn;
+     size_t msgs_count = 0;
+@@ -462,11 +462,6 @@ errno_t sysdb_search_selinux_usermap_by_username(TALLOC_CTX *mem_ctx,
+         }
+     }
+ 
+-    if (usermaps[0] == NULL) {
+-        ret = ENOENT;
+-        goto done;
+-    }
+-
+     *_usermaps = talloc_steal(mem_ctx, usermaps);
+ 
+     ret = EOK;
+diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c
+index 944845a86dfa8166367029e6f7bddc478e5ebd03..238b4fa7feee666e3419eae55c4952e45fd7184c 100644
+--- a/src/responder/pam/pamsrv_cmd.c
++++ b/src/responder/pam/pamsrv_cmd.c
+@@ -359,8 +359,10 @@ fail:
+ #ifdef HAVE_SELINUX
+ 
+ #define ALL_SERVICES "*"
++#define selogin_path(mem_ctx, username) \
++    talloc_asprintf(mem_ctx, "%s/logins/%s", selinux_policy_root(), username)
+ 
+-static errno_t write_selinux_string(const char *username, char *string)
++static errno_t write_selinux_login_file(const char *username, char *string)
+ {
+     char *path = NULL;
+     char *tmp_path = NULL;
+@@ -383,8 +385,7 @@ static errno_t write_selinux_string(const char *username, char *string)
+         return ENOMEM;
+     }
+ 
+-    path = talloc_asprintf(tmp_ctx, "%s/logins/%s", selinux_policy_root(),
+-                           username);
++    path = selogin_path(tmp_ctx, username);
+     if (path == NULL) {
+         ret = ENOMEM;
+         goto done;
+@@ -452,7 +453,33 @@ done:
+     return ret;
+ }
+ 
+-static errno_t get_selinux_string(struct pam_auth_req *preq)
++static errno_t remove_selinux_login_file(const char *username)
++{
++    char *path;
++    errno_t ret;
++
++    path = selogin_path(NULL, username);
++    if (!path) return ENOMEM;
++
++    errno = 0;
++    ret = unlink(path);
++    if (ret < 0) {
++        ret = errno;
++        if (ret == ENOENT) {
++            /* Just return success if the file was not there */
++            ret = EOK;
++        } else {
++            DEBUG(SSSDBG_OP_FAILURE,
++                  ("Could not remove login file %s [%d]: %s\n",
++                   path, ret, strerror(ret)));
++        }
++    }
++
++    talloc_free(path);
++    return ret;
++}
++
++static errno_t process_selinux_mappings(struct pam_auth_req *preq)
+ {
+     struct sysdb_ctx *sysdb;
+     TALLOC_CTX *tmp_ctx;
+@@ -464,7 +491,7 @@ static errno_t get_selinux_string(struct pam_auth_req *preq)
+     const char *tmp_str;
+     char *order = NULL;
+     char **order_array;
+-    errno_t ret;
++    errno_t ret, err;
+     int i, j;
+     size_t order_count;
+     size_t len = 0;
+@@ -550,53 +577,58 @@ static errno_t get_selinux_string(struct pam_auth_req *preq)
+                                                    &usermaps);
+     if (ret != EOK && ret != ENOENT) {
+         goto done;
++    } else if (ret == ENOENT) {
++        DEBUG(SSSDBG_TRACE_FUNC, ("No maps defined on the server\n"));
++        ret = EOK;
++        goto done;
+     }
+ 
+-    if (ret == ENOENT) {
+-        DEBUG(SSSDBG_TRACE_FUNC, ("No user maps found, using default!"));
+-        file_content = talloc_strdup(tmp_ctx, default_user);
+-        if (file_content == NULL) {
+-            ret = ENOMEM;
+-            goto done;
+-        }
+-    } else {
+-        /* Iterate through the order array and try to find SELinux users
+-         * in fetched maps. The order array contains all SELinux users
+-         * allowed in the domain in the same order they should appear
+-         * in the SELinux config file. If any user from the order array
+-         * is not in fetched user maps, it means it should not be allowed
+-         * for the user who is just logging in.
+-         *
+-         * Right now we have empty content of the SELinux config file,
+-         * we shall add only those SELinux users that are present both in
+-         * the order array and user maps applicable to the user who is
+-         * logging in.
+-         */
+-        for (i = 0; i < order_count; i++) {
+-            for (j = 0; usermaps[j] != NULL; j++) {
+-                tmp_str = sss_selinux_map_get_seuser(usermaps[j]);
++    /* If no maps match, we'll use the default SELinux user from the
++     * config */
++    file_content = talloc_strdup(tmp_ctx, default_user);
++    if (file_content == NULL) {
++        ret = ENOMEM;
++        goto done;
++    }
++
++    /* Iterate through the order array and try to find SELinux users
++     * in fetched maps. The order array contains all SELinux users
++     * allowed in the domain in the same order they should appear
++     * in the SELinux config file. If any user from the order array
++     * is not in fetched user maps, it means it should not be allowed
++     * for the user who is just logging in.
++     *
++     * Right now we have empty content of the SELinux config file,
++     * we shall add only those SELinux users that are present both in
++     * the order array and user maps applicable to the user who is
++     * logging in.
++     */
++    for (i = 0; i < order_count; i++) {
++        for (j = 0; usermaps[j] != NULL; j++) {
++            tmp_str = sss_selinux_map_get_seuser(usermaps[j]);
+ 
+-                if (tmp_str && !strcasecmp(tmp_str, order_array[i])) {
+-                    /* If file_content contained something, overwrite it.
+-                     * This record has higher priority.
+-                     */
+-                    talloc_zfree(file_content);
+-                    file_content = talloc_strdup(tmp_ctx, tmp_str);
+-                    if (file_content == NULL) {
+-                        ret = ENOMEM;
+-                        goto done;
+-                    }
+-                    break;
++            if (tmp_str && !strcasecmp(tmp_str, order_array[i])) {
++                /* If file_content contained something, overwrite it.
++                 * This record has higher priority.
++                 */
++                talloc_zfree(file_content);
++                file_content = talloc_strdup(tmp_ctx, tmp_str);
++                if (file_content == NULL) {
++                    ret = ENOMEM;
++                    goto done;
+                 }
++                break;
+             }
+         }
+     }
+ 
+-    if (file_content) {
+-        ret = write_selinux_string(pd->user, file_content);
+-    }
+-
++    ret = write_selinux_login_file(pd->user, file_content);
+ done:
++    if (!file_content) {
++        err = remove_selinux_login_file(pd->user);
++        /* Don't overwrite original error condition if there was one */
++        if (ret == EOK) ret = err;
++    }
+     talloc_free(tmp_ctx);
+     return ret;
+ }
+@@ -802,7 +834,7 @@ static void pam_reply(struct pam_auth_req *preq)
+         pd->pam_status == PAM_SUCCESS) {
+         /* Try to fetch data from sysdb
+          * (auth already passed -> we should have them) */
+-        ret = get_selinux_string(preq);
++        ret = process_selinux_mappings(preq);
+         if (ret != EOK) {
+             pd->pam_status = PAM_SYSTEM_ERR;
+         }
+-- 
+1.7.11.2
+
diff --git a/sssd.spec b/sssd.spec
index 0f53155..f6f85c2 100644
--- a/sssd.spec
+++ b/sssd.spec
@@ -16,7 +16,7 @@
 
 Name: sssd
 Version: 1.9.0
-Release: 15%{?dist}.beta6
+Release: 16%{?dist}.beta6
 Group: Applications/System
 Summary: System Security Services Daemon
 License: GPLv3+
@@ -26,6 +26,8 @@ BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
 
 ### Patches ###
 Patch0001:  0001-Abort-PAM-access-phase-if-HBAC-does-not-return-PAM_S.patch
+Patch0002:  0002-Do-not-try-to-remove-the-temp-login-file-if-already-.patch
+Patch0003:  0003-Only-create-the-SELinux-login-file-if-there-are-mapp.patch
 
 
 ### Dependencies ###
@@ -515,6 +517,10 @@ fi
 %postun -n libsss_sudo -p /sbin/ldconfig
 
 %changelog
+* Fri Aug 17 2012 Jakub Hrozek <jhrozek at redhat.com> - 1.9.0-16.beta6
+- Only create the SELinux login file if there are SELinux mappings on
+  the IPA server
+
 * Fri Aug 10 2012 Jakub Hrozek <jhrozek at redhat.com> - 1.9.0-14.beta6
 - Don't discard HBAC rule processing result if SELinux is on
   Resolves: rhbz#846792 (CVE-2012-3462)


More information about the scm-commits mailing list