[SSSD] Only create the SELinux login file if there are mappings on the server

Jakub Hrozek jhrozek at redhat.com
Wed Aug 15 16:27:28 UTC 2012


On Tue, Aug 14, 2012 at 11:11:01AM +0200, Pavel Březina wrote:
> Nack.
> 
> User cannot login when the loginfile does not exist because you
> return !EOK here:
> 
>     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;

Thank you. I think I should just ignore ENOENT in
remove_selinux_login_file()

New patches are attached.
-------------- next part --------------
>From 5eacf73f1d6f4b9c9257fe5b2c0e8fe05df0218a 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 92cd734079be76f6c40873c00278f75d5947d686..6df8213f7fea3eef3f05fa71b55ad8b54da93181 100644
--- a/src/responder/pam/pamsrv_cmd.c
+++ b/src/responder/pam/pamsrv_cmd.c
@@ -362,7 +362,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;
@@ -433,9 +433,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

-------------- next part --------------
>From c70e98a3cd0d0a8951210e826494b4b16d681b73 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 6df8213f7fea3eef3f05fa71b55ad8b54da93181..e0c52f4800a50ec6ce2ceb4f9fcc54d91c717831 100644
--- a/src/responder/pam/pamsrv_cmd.c
+++ b/src/responder/pam/pamsrv_cmd.c
@@ -355,8 +355,10 @@ fail:
 }
 
 #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;
@@ -379,8 +381,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;
@@ -448,7 +449,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;
@@ -460,7 +487,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;
@@ -546,53 +573,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;
 }
@@ -796,7 +828,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



More information about the sssd-devel mailing list