[SSSD] [PATCH] SELinux: Always use the default if it exists on the server

Jakub Hrozek jhrozek at redhat.com
Thu Sep 13 10:24:06 UTC 2012


In order to test these patches properly, you need to be running the
latest patches that are currently on review on freeipa-devel. Ping me
off-line for access to such IPA client.

[PATCH 1/2] DB: Always write the SELinux object to sysdb
This is a fallout of the transaction processing refactoring.

There's no point in checking if the object already exists because we always
wipe the whole sysdb subtree. We were also immediatelly cancelling the
transaction because we'd jump to goto, even though it was with EOK.

[PATCH 2/2] SELinux: Always use the default if it exists on the server

https://fedorahosted.org/sssd/ticket/1513

This is a counterpart of the FreeIPA ticket https://fedorahosted.org/freeipa/ticket/3045

During an e-mail discussion, it was decided that

    * if the default is set in the IPA config object, the SSSD would use
      that default no matter what
    * if the default is not set (aka empty or missing), the SSSD would just
      use the system default and skip creating the login file altogether
-------------- next part --------------
>From 5198b6563fe1d7600d2ee3eeb1d4a80ad675ff1f Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Thu, 13 Sep 2012 05:44:12 -0400
Subject: [PATCH 1/2] DB: Always write the SELinux object to sysdb

There's no point in checking if the object already exists because we always
wipe the whole sysdb subtree. We were also immediatelly cancelling the
transaction because we'd jump to goto, even though it was with EOK.
---
 src/db/sysdb_selinux.c | 78 ++++++--------------------------------------------
 1 file changed, 8 insertions(+), 70 deletions(-)

diff --git a/src/db/sysdb_selinux.c b/src/db/sysdb_selinux.c
index 565b7612fe1618cbe951317227b9001af549ca0e..bc067225b7ea2f921f0c21c2482285da62f600c0 100644
--- a/src/db/sysdb_selinux.c
+++ b/src/db/sysdb_selinux.c
@@ -24,48 +24,6 @@
 #include "db/sysdb_private.h"
 
 /* Some generic routines */
-static errno_t get_rm_msg(TALLOC_CTX *mem_ctx,
-                          struct ldb_message *old_msg,
-                          struct sysdb_attrs *new_attrs,
-                          struct ldb_message **_msg)
-{
-    struct ldb_message *rm_msg;
-    const char *tmp_str;
-    errno_t ret;
-    int i;
-
-    rm_msg = ldb_msg_new(mem_ctx);
-    if (rm_msg == NULL) {
-        ret = ENOMEM;
-        goto done;
-    }
-
-    rm_msg->dn = old_msg->dn;
-    rm_msg->elements = talloc_zero_array(rm_msg, struct ldb_message_element,
-                                         old_msg->num_elements);
-    rm_msg->num_elements = 0;
-
-    for (i = 0; i < old_msg->num_elements; i++) {
-        ret = sysdb_attrs_get_string(new_attrs, old_msg->elements[i].name, &tmp_str);
-        if (ret != ENOENT) {
-            continue;
-        }
-
-        rm_msg->elements[rm_msg->num_elements] = old_msg->elements[i];
-        rm_msg->elements[rm_msg->num_elements].flags = LDB_FLAG_MOD_DELETE;
-        rm_msg->num_elements++;
-    }
-
-    ret = EOK;
-done:
-    if (ret != EOK) {
-        talloc_free(rm_msg);
-    } else {
-        *_msg = rm_msg;
-    }
-
-    return ret;
-}
 
 static errno_t
 sysdb_add_selinux_entity(struct sysdb_ctx *sysdb,
@@ -123,8 +81,6 @@ static errno_t sysdb_store_selinux_entity(struct sysdb_ctx *sysdb,
                                           enum selinux_entity_type type)
 {
     TALLOC_CTX *tmp_ctx;
-    struct ldb_message *msg;
-    struct ldb_message *rm_msg;
     bool in_transaction = false;
     const char *objectclass = NULL;
     const char *name;
@@ -185,18 +141,8 @@ static errno_t sysdb_store_selinux_entity(struct sysdb_ctx *sysdb,
     ret = sysdb_attrs_add_time_t(attrs, SYSDB_LAST_UPDATE, now);
     if (ret) goto done;
 
-    if (type == SELINUX_CONFIG) {
-        ret = sysdb_search_selinux_config(tmp_ctx, sysdb, NULL, &msg);
-    } else if (type == SELINUX_USER_MAP) {
-        ret = sysdb_search_selinux_usermap_by_mapname(tmp_ctx, sysdb, name,
-                                                      NULL, &msg);
-    }
-
-    if (ret && ret != ENOENT) {
-        goto done;
-    }
-    if (ret == ENOENT) {
-        ret = sysdb_add_selinux_entity(sysdb, dn, objectclass, attrs, now);
+    ret = sysdb_add_selinux_entity(sysdb, dn, objectclass, attrs, now);
+    if (ret != EOK) {
         goto done;
     }
 
@@ -205,16 +151,6 @@ static errno_t sysdb_store_selinux_entity(struct sysdb_ctx *sysdb,
         goto done;
     }
 
-    /* Now delete attributes which are no longer present */
-    ret = get_rm_msg(tmp_ctx, msg, attrs, &rm_msg);
-    if (ret != EOK) {
-        goto done;
-    }
-
-    if (rm_msg->num_elements > 0) {
-        ret = ldb_modify(sysdb->ldb, rm_msg);
-    }
-
     ret = sysdb_transaction_commit(sysdb);
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction\n"));
@@ -255,10 +191,12 @@ errno_t sysdb_store_selinux_config(struct sysdb_ctx *sysdb,
         return ENOMEM;
     }
 
-    ret = sysdb_attrs_add_string(attrs, SYSDB_SELINUX_DEFAULT_USER,
-                                 default_user);
-    if (ret != EOK) {
-        goto done;
+    if (default_user) {
+        ret = sysdb_attrs_add_string(attrs, SYSDB_SELINUX_DEFAULT_USER,
+                                    default_user);
+        if (ret != EOK) {
+            goto done;
+        }
     }
 
     ret = sysdb_attrs_add_string(attrs, SYSDB_SELINUX_DEFAULT_ORDER,
-- 
1.7.11.4

-------------- next part --------------
>From 93eacefe9ff8be035a2e603e155331f3319bc076 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Thu, 13 Sep 2012 10:07:29 +0200
Subject: [PATCH 2/2] SELinux: Always use the default if it exists on the
 server

https://fedorahosted.org/sssd/ticket/1513

This is a counterpart of the FreeIPA ticket https://fedorahosted.org/freeipa/ticket/3045

During an e-mail discussion, it was decided that

    * if the default is set in the IPA config object, the SSSD would use
      that default no matter what
    * if the default is not set (aka empty or missing), the SSSD
      would just use the system default and skip creating the login
      file altogether
---
 src/db/sysdb_selinux.c          |  6 ++++--
 src/providers/ipa/ipa_selinux.c | 18 ++++++++---------
 src/responder/pam/pamsrv_cmd.c  | 43 ++++++++++++++++++++---------------------
 3 files changed, 34 insertions(+), 33 deletions(-)

diff --git a/src/db/sysdb_selinux.c b/src/db/sysdb_selinux.c
index bc067225b7ea2f921f0c21c2482285da62f600c0..ce884e211180bbf6d0bd7286173427a527589c1a 100644
--- a/src/db/sysdb_selinux.c
+++ b/src/db/sysdb_selinux.c
@@ -205,7 +205,7 @@ errno_t sysdb_store_selinux_config(struct sysdb_ctx *sysdb,
         goto done;
     }
 
-    ret =  sysdb_store_selinux_entity(sysdb, attrs, SELINUX_CONFIG);
+    ret = sysdb_store_selinux_entity(sysdb, attrs, SELINUX_CONFIG);
 done:
     talloc_free(attrs);
     return ret;
@@ -344,7 +344,9 @@ errno_t sysdb_search_selinux_usermap_by_username(TALLOC_CTX *mem_ctx,
 
     ret = sysdb_search_entry(tmp_ctx, sysdb, basedn, LDB_SCOPE_SUBTREE, filter,
                              attrs, &msgs_count, &msgs);
-    if (ret) {
+    if (ret == ENOENT) {
+        msgs_count = 0;
+    } else if (ret) {
         goto done;
     }
 
diff --git a/src/providers/ipa/ipa_selinux.c b/src/providers/ipa/ipa_selinux.c
index 36a2bfb4a43a03afc17da5faa5f564dcba5b8367..0adc0fd02e110a4d4bb3168e34c99c217ead6f74 100644
--- a/src/providers/ipa/ipa_selinux.c
+++ b/src/providers/ipa/ipa_selinux.c
@@ -136,11 +136,9 @@ static void ipa_selinux_handler_done(struct tevent_req *req)
         goto fail;
     }
 
-    if (default_user != NULL && map_order != NULL) {
-        ret = sysdb_store_selinux_config(sysdb, default_user, map_order);
-        if (ret != EOK) {
-            goto fail;
-        }
+    ret = sysdb_store_selinux_config(sysdb, default_user, map_order);
+    if (ret != EOK) {
+        goto fail;
     }
 
     if (map_count > 0 && maps != NULL) {
@@ -668,13 +666,15 @@ ipa_get_selinux_recv(struct tevent_req *req,
     if (state->defaults != NULL) {
         ret = sysdb_attrs_get_string(state->defaults, IPA_CONFIG_SELINUX_DEFAULT_MAP,
                                      &tmp_str);
-        if (ret != EOK) {
+        if (ret != EOK && ret != ENOENT) {
             return ret;
         }
 
-        *default_user = talloc_strdup(mem_ctx, tmp_str);
-        if (*default_user == NULL) {
-            return ENOMEM;
+        if (ret == EOK) {
+            *default_user = talloc_strdup(mem_ctx, tmp_str);
+            if (*default_user == NULL) {
+                return ENOMEM;
+            }
         }
 
         ret = sysdb_attrs_get_string(state->defaults, IPA_CONFIG_SELINUX_MAP_ORDER,
diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c
index 4c0356832df29b849229f424e1f393d6b3ffa687..07fa96ab83a46e82a4f501e2d73cf0ef167c3cba 100644
--- a/src/responder/pam/pamsrv_cmd.c
+++ b/src/responder/pam/pamsrv_cmd.c
@@ -519,30 +519,33 @@ static errno_t process_selinux_mappings(struct pam_auth_req *preq)
         goto done;
     }
 
-    /* We need two values from the config object:
-     * - default SELinux user in case no other is available
-     * - the order for fetched usermaps
-     */
-    for (i = 0; i < config->num_elements; i++) {
-        if (strcasecmp(config->elements[i].name, SYSDB_SELINUX_DEFAULT_USER) == 0) {
-            default_user = (const char *)config->elements[i].values[0].data;
-        } else if (strcasecmp(config->elements[i].name, SYSDB_SELINUX_DEFAULT_ORDER) == 0) {
-            tmp_str = (char *)config->elements[i].values[0].data;
-            len = config->elements[i].values[0].length;
-            order = talloc_strdup(tmp_ctx, tmp_str);
-            if (order == NULL) {
-                goto done;
-            }
-        }
+    default_user = ldb_msg_find_attr_as_string(config,
+                                               SYSDB_SELINUX_DEFAULT_USER,
+                                               NULL);
+    if (!default_user || default_user[0] == '\0') {
+        /* Skip creating the maps altogether if there is no default
+         * or empty default
+         */
+        ret = EOK;
+        goto done;
     }
 
-    if (default_user == NULL || order == NULL) {
-        DEBUG(SSSDBG_OP_FAILURE, ("No default SELinux user "
-                                  "or map order given!\n"));
+    tmp_str = ldb_msg_find_attr_as_string(config,
+                                          SYSDB_SELINUX_DEFAULT_ORDER,
+                                          NULL);
+    if (tmp_str == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, ("No map order given!\n"));
         ret = EINVAL;
         goto done;
     }
 
+    order = talloc_strdup(tmp_ctx, tmp_str);
+    if (order == NULL) {
+        ret = ENOMEM;
+        goto done;
+    }
+    len = strlen(order);
+
     /* The "order" string contains one or more SELinux user records
      * separated by $. Now we need to create an array of string from
      * this one string. First find out how many elements in the array
@@ -577,10 +580,6 @@ static errno_t process_selinux_mappings(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 no maps match, we'll use the default SELinux user from the
-- 
1.7.11.4



More information about the sssd-devel mailing list