[SSSD] [PATCH] Don't call semanage transaction unless the context has changed

Jakub Hrozek jhrozek at redhat.com
Fri Apr 10 13:24:45 UTC 2015


Hi,

the attached patches fix ticket
https://fedorahosted.org/sssd/ticket/2624

It turns out that calling libsemanage transactions is a fairly intensive
operation that involves copying multiple files under the /etc/selinux
hierarchy to a temporary subtree and then back. With the patches, the
context from IPA is checked against the local database first and only
applied if it differs.
-------------- next part --------------
>From bde77adee7d81aaf2f840f64db0e48cce29bbbff Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 10 Apr 2015 10:55:22 +0200
Subject: [PATCH 1/3] selinux: Disconnect before closing the handle

libsemanage documentation says:
~~~~
be sure that a semanage_disconnect() was previously called if the handle
was connected.
~~~~

Otherwise we get a memory leak.
---
 src/util/sss_semanage.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/util/sss_semanage.c b/src/util/sss_semanage.c
index b85831c3d3f262f49b19082e96aa62ccf3afeaa8..d141de1c671e6d62a731e56b10ee14069f27ae87 100644
--- a/src/util/sss_semanage.c
+++ b/src/util/sss_semanage.c
@@ -68,6 +68,13 @@ static void sss_semanage_error_callback(void *varg,
     free(message);
 }
 
+static void sss_semanage_close(semanage_handle_t *handle)
+{
+    /* Calling disconnect on a disconnected handle is safe */
+    semanage_disconnect(handle);
+    semanage_handle_destroy(handle);
+}
+
 static semanage_handle_t *sss_semanage_init(void)
 {
     int ret;
@@ -110,7 +117,7 @@ static semanage_handle_t *sss_semanage_init(void)
 
     return handle;
 fail:
-    semanage_handle_destroy(handle);
+    sss_semanage_close(handle);
     return NULL;
 }
 
@@ -278,7 +285,7 @@ int set_seuser(const char *login_name, const char *seuser_name,
     ret = EOK;
 done:
     semanage_seuser_key_free(key);
-    semanage_handle_destroy(handle);
+    sss_semanage_close(handle);
     return ret;
 }
 
@@ -350,7 +357,7 @@ int del_seuser(const char *login_name)
 
     ret = EOK;
 done:
-    semanage_handle_destroy(handle);
+    sss_semanage_close(handle);
     return ret;
 }
 
-- 
2.1.0

-------------- next part --------------
>From 90cb16a69330303eed8fef2975962e0bdfcebd90 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 10 Apr 2015 11:06:44 +0200
Subject: [PATCH 2/3] selinux: Begin and end the transaction on the same
 nesting level

Transaction should be started and commited on the same code nesting or
abstraction level. Also, transactions are really costly with libselinux
and splitting them from initialization will make init function reusable
by read-only libsemanage functions.
---
 src/util/sss_semanage.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/src/util/sss_semanage.c b/src/util/sss_semanage.c
index d141de1c671e6d62a731e56b10ee14069f27ae87..c0342498cbd0495733a0bf701a06a02cfb705fc7 100644
--- a/src/util/sss_semanage.c
+++ b/src/util/sss_semanage.c
@@ -109,12 +109,6 @@ static semanage_handle_t *sss_semanage_init(void)
         goto fail;
     }
 
-    ret = semanage_begin_transaction(handle);
-    if (ret != 0) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "Cannot begin SELinux transaction\n");
-        goto fail;
-    }
-
     return handle;
 fail:
     sss_semanage_close(handle);
@@ -243,6 +237,13 @@ int set_seuser(const char *login_name, const char *seuser_name,
         goto done;
     }
 
+    ret = semanage_begin_transaction(handle);
+    if (ret != 0) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Cannot begin SELinux transaction\n");
+        ret = EIO;
+        goto done;
+    }
+
     ret = semanage_seuser_key_create(handle, login_name, &key);
     if (ret != 0) {
         DEBUG(SSSDBG_CRIT_FAILURE, "Cannot create SELinux user key\n");
@@ -303,6 +304,13 @@ int del_seuser(const char *login_name)
         goto done;
     }
 
+    ret = semanage_begin_transaction(handle);
+    if (ret != 0) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Cannot begin SELinux transaction\n");
+        ret = EIO;
+        goto done;
+    }
+
     ret = semanage_seuser_key_create(handle, login_name, &key);
     if (ret != 0) {
         DEBUG(SSSDBG_CRIT_FAILURE, "Cannot create SELinux user key\n");
-- 
2.1.0

-------------- next part --------------
>From e4b5b605f4b1582fad37bbea95b4716e44c44dd5 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Thu, 9 Apr 2015 22:18:35 +0200
Subject: [PATCH 3/3] selinux: Only call semanage if the context actually
 changes

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

Add a function to query the libsemanage database for a user context and
only update the database if the context differes from the one set on the
server.
---
 src/providers/ipa/selinux_child.c | 35 ++++++++++++++++---
 src/util/sss_semanage.c           | 71 +++++++++++++++++++++++++++++++++++++++
 src/util/util.h                   |  2 ++
 3 files changed, 104 insertions(+), 4 deletions(-)

diff --git a/src/providers/ipa/selinux_child.c b/src/providers/ipa/selinux_child.c
index 81c1de877ef08a299d07837fefcd195d465849fa..7c5731d66b7d0ed17b7be18c4adaa65394002fc4 100644
--- a/src/providers/ipa/selinux_child.c
+++ b/src/providers/ipa/selinux_child.c
@@ -165,6 +165,29 @@ static int sc_set_seuser(const char *login_name, const char *seuser_name,
     return ret;
 }
 
+static bool seuser_needs_update(struct input_buffer *ibuf)
+{
+    bool needs_update = true;
+    char *db_seuser = NULL;
+    char *db_mls_range = NULL;
+    errno_t ret;
+
+    ret = get_seuser(ibuf, ibuf->username, &db_seuser, &db_mls_range);
+    DEBUG(SSSDBG_TRACE_INTERNAL,
+          "get_seuser: ret: %d seuser: %s mls: %s\n",
+          ret, db_seuser ? db_seuser : "unknown",
+          db_mls_range ? db_mls_range : "unknown");
+    if (ret == EOK && db_seuser && db_mls_range &&
+            strcmp(db_seuser, ibuf->seuser) == 0 &&
+            strcmp(db_mls_range, ibuf->mls_range) == 0) {
+        needs_update = false;
+    }
+
+    talloc_free(db_seuser);
+    talloc_free(db_mls_range);
+    return needs_update;
+}
+
 int main(int argc, const char *argv[])
 {
     int opt;
@@ -177,6 +200,7 @@ int main(int argc, const char *argv[])
     struct input_buffer *ibuf = NULL;
     struct response *resp = NULL;
     ssize_t written;
+    bool needs_update;
 
     struct poptOption long_options[] = {
         POPT_AUTOHELP
@@ -296,10 +320,13 @@ int main(int argc, const char *argv[])
 
     DEBUG(SSSDBG_TRACE_FUNC, "performing selinux operations\n");
 
-    ret = sc_set_seuser(ibuf->username, ibuf->seuser, ibuf->mls_range);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "Cannot set SELinux login context.\n");
-        goto fail;
+    needs_update = seuser_needs_update(ibuf);
+    if (needs_update == true) {
+        ret = sc_set_seuser(ibuf->username, ibuf->seuser, ibuf->mls_range);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, "Cannot set SELinux login context.\n");
+            goto fail;
+        }
     }
 
     ret = prepare_response(main_ctx, ret, &resp);
diff --git a/src/util/sss_semanage.c b/src/util/sss_semanage.c
index c0342498cbd0495733a0bf701a06a02cfb705fc7..01a2f41d8752e127f2aa1b72faa61c23f315edd7 100644
--- a/src/util/sss_semanage.c
+++ b/src/util/sss_semanage.c
@@ -369,6 +369,71 @@ done:
     return ret;
 }
 
+int get_seuser(TALLOC_CTX *mem_ctx, const char *login_name,
+               char **_seuser, char **_mls_range)
+{
+    errno_t ret;
+    const char *seuser;
+    const char *mls_range;
+    semanage_handle_t *sm_handle = NULL;
+    semanage_seuser_t *sm_user = NULL;
+    semanage_seuser_key_t *sm_key = NULL;
+
+    sm_handle = sss_semanage_init();
+    if (sm_handle == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Cannot create SELinux handle\n");
+        ret = EIO;
+        goto done;
+    }
+
+    ret = semanage_seuser_key_create(sm_handle, login_name, &sm_key);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Cannot create key for %s\n", login_name);
+        ret = EIO;
+        goto done;
+    }
+
+    ret = semanage_seuser_query(sm_handle, sm_key, &sm_user);
+    if (ret < 0) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Cannot query for %s\n", login_name);
+        ret = EIO;
+        goto done;
+    }
+
+    seuser = semanage_seuser_get_sename(sm_user);
+    if (seuser != NULL) {
+        *_seuser = talloc_strdup(mem_ctx, seuser);
+        if (*_seuser == NULL) {
+            ret = ENOMEM;
+            goto done;
+        }
+        DEBUG(SSSDBG_OP_FAILURE,
+              "SELinux user for %s: %s\n", login_name, *_seuser);
+    } else {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Cannot get sename for %s\n", login_name);
+    }
+
+    mls_range = semanage_seuser_get_mlsrange(sm_user);
+    if (mls_range != NULL) {
+        *_mls_range = talloc_strdup(mem_ctx, mls_range);
+        if (*_mls_range == NULL) {
+            ret = ENOMEM;
+            goto done;
+        }
+        DEBUG(SSSDBG_OP_FAILURE,
+              "SELinux range for %s: %s\n", login_name, *_mls_range);
+    } else {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Cannot get mlsrange for %s\n", login_name);
+    }
+
+    ret = EOK;
+done:
+    semanage_seuser_key_free(sm_key);
+    semanage_seuser_free(sm_user);
+    sss_semanage_close(sm_handle);
+    return ret;
+}
+
 #else /* HAVE_SEMANAGE */
 int set_seuser(const char *login_name, const char *seuser_name,
                const char *mls)
@@ -380,4 +445,10 @@ int del_seuser(const char *login_name)
 {
     return EOK;
 }
+
+int get_seuser(TALLOC_CTX *mem_ctx, const char *login_name,
+               char **_seuser, char **_mls_range)
+{
+    return EOK;
+}
 #endif  /* HAVE_SEMANAGE */
diff --git a/src/util/util.h b/src/util/util.h
index 704e10dc657b3a9e7b15f24a907ac385fa501431..06ffb22b0ecb877f1f0a9c39f96ced0a3d2b092e 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -654,6 +654,8 @@ errno_t restore_creds(struct sss_creds *saved_creds);
 int set_seuser(const char *login_name, const char *seuser_name,
                const char *mlsrange);
 int del_seuser(const char *login_name);
+int get_seuser(TALLOC_CTX *mem_ctx, const char *login_name,
+               char **_seuser, char **_mls_range);
 
 /* convert time from generalized form to unix time */
 errno_t sss_utc_to_time_t(const char *str, const char *format, time_t *unix_time);
-- 
2.1.0



More information about the sssd-devel mailing list