[SSSD] [PATCH] IPA: Return and save all SELinux rules in the provider

Jakub Hrozek jhrozek at redhat.com
Wed Jul 18 11:24:15 UTC 2012


The IPA session code used to download all enabled SELinux rules, but
then filter out those that match to the current user and save only
those. This meant that if a rule was deleted or disabled on the server,
it remained in the cache and was still evaluated.

The attached patch changes that behaviour to save all downloaded rules --
if this proves to be slow, we can optimize, for instance in a similar
way HBAC rules are optimized, by falling back to sysdb rules if several
requests arrive within a specified interval. However, we don't use
member/memberof links when saving the SELinux mappings, so the sysdb
write should be reasonably fast.

[PATCH 1/3] IPA: Download defaults even if there are no SELinux mappings
We should always download the defaults because even if there are no
rules, we might want to use (or update) the defaults. Previously, the
defaults were only downloaded when there were some rules on the server.

[PATCH 2/3] SYSDB: Delete SELinux mappings
A sysdb function that can be used to delete SELinux mappings from the
sysdb

[PATCH 3/3] IPA: Return and save all SELinux rules in the provider
https://fedorahosted.org/sssd/ticket/1421
-------------- next part --------------
>From 5c81f51bf06e0b0dded3ff9d9f93ec07ddd63426 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 16 Jul 2012 19:44:53 +0200
Subject: [PATCH 1/3] IPA: Download defaults even if there are no SELinux
 mappings

We should always download the defaults because even if there are no
rules, we might want to use (or update) the defaults.
---
 src/providers/ipa/ipa_session.c |  119 +++++++++++++++++++--------------------
 1 file changed, 59 insertions(+), 60 deletions(-)

diff --git a/src/providers/ipa/ipa_session.c b/src/providers/ipa/ipa_session.c
index 12e80729770b9550dd04cf4b3c7cb39efd325672..385b0090836501fd65a8615389f3d60916e7e086 100644
--- a/src/providers/ipa/ipa_session.c
+++ b/src/providers/ipa/ipa_session.c
@@ -65,9 +65,10 @@ static errno_t ipa_get_selinux_recv(struct tevent_req *req,
 
 static void ipa_get_selinux_connect_done(struct tevent_req *subreq);
 static void ipa_get_selinux_hosts_done(struct tevent_req *subreq);
+static void ipa_get_config_step(struct tevent_req *req);
+static void ipa_get_selinux_config_done(struct tevent_req *subreq);
 static void ipa_get_selinux_maps_done(struct tevent_req *subreq);
 static void ipa_get_selinux_hbac_done(struct tevent_req *subreq);
-static void ipa_get_selinux_config_done(struct tevent_req *subreq);
 
 void ipa_session_handler(struct be_req *be_req)
 {
@@ -246,6 +247,7 @@ static void ipa_get_selinux_connect_done(struct tevent_req *subreq)
 
     /* FIXME: detect if HBAC is configured
      * - if yes, we can skip host retrieval and get it directly from sysdb
+     *   and shortcut to ipa_get_config_step()
      */
     subreq = ipa_host_info_send(state, bctx->ev, bctx->sysdb,
                                 sdap_id_op_handle(state->op),
@@ -275,7 +277,6 @@ static void ipa_get_selinux_hosts_done(struct tevent_req *subreq)
     struct ipa_get_selinux_state *state = tevent_req_data(req,
                                                   struct ipa_get_selinux_state);
     struct be_ctx *bctx = state->be_req->be_ctx;
-    struct sdap_id_ctx *id_ctx = state->session_ctx->id_ctx->sdap_id_ctx;
     size_t host_count, hostgroup_count;
     struct sysdb_attrs **hostgroups;
     struct sysdb_attrs **host;
@@ -294,21 +295,69 @@ static void ipa_get_selinux_hosts_done(struct tevent_req *subreq)
         goto done;
     }
 
+    return ipa_get_config_step(req);
+
+done:
+    if (ret != EOK) {
+        tevent_req_error(req, ret);
+    }
+}
+
+static void ipa_get_config_step(struct tevent_req *req)
+{
+    const char *domain;
+    struct tevent_req *subreq;
+    struct ipa_get_selinux_state *state = tevent_req_data(req,
+                                                  struct ipa_get_selinux_state);
+    struct be_ctx *bctx = state->be_req->be_ctx;
+    struct ipa_id_ctx *id_ctx = state->session_ctx->id_ctx;
+
+    domain = dp_opt_get_string(state->session_ctx->id_ctx->ipa_options->basic,
+                               IPA_KRB5_REALM);
+    subreq = ipa_get_config_send(state, bctx->ev,
+                                 sdap_id_op_handle(state->op),
+                                 id_ctx->sdap_id_ctx->opts,
+                                 domain, NULL);
+    if (subreq == NULL) {
+        tevent_req_error(req, ENOMEM);
+    }
+    tevent_req_set_callback(subreq, ipa_get_selinux_config_done, req);
+}
+
+static void ipa_get_selinux_config_done(struct tevent_req *subreq)
+{
+    struct tevent_req *req = tevent_req_callback_data(subreq,
+                                                  struct tevent_req);
+    struct ipa_get_selinux_state *state = tevent_req_data(req,
+                                                  struct ipa_get_selinux_state);
+    struct be_ctx *bctx = state->be_req->be_ctx;
+    struct sdap_id_ctx *id_ctx = state->session_ctx->id_ctx->sdap_id_ctx;
+    errno_t ret;
+
+    ret = ipa_get_config_recv(subreq, state, &state->defaults);
+    talloc_free(subreq);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Could not get IPA config\n"));
+        goto done;
+    }
+
     subreq = ipa_selinux_get_maps_send(state, bctx->ev, bctx->sysdb,
-                                       sdap_id_op_handle(state->op),
-                                       id_ctx->opts,
-                                       state->session_ctx->id_ctx->ipa_options,
-                                       state->session_ctx->selinux_search_bases);
-    if (subreq == NULL) {
+                                     sdap_id_op_handle(state->op),
+                                     id_ctx->opts,
+                                     state->session_ctx->id_ctx->ipa_options,
+                                     state->session_ctx->selinux_search_bases);
+    if (!subreq) {
         ret = ENOMEM;
         goto done;
     }
-
     tevent_req_set_callback(subreq, ipa_get_selinux_maps_done, req);
+    return;
 
 done:
     if (ret != EOK) {
         tevent_req_error(req, ret);
+    } else {
+        tevent_req_done(req);
     }
 }
 
@@ -322,7 +371,6 @@ static void ipa_get_selinux_maps_done(struct tevent_req *subreq)
 
     struct sysdb_attrs **results;
     size_t count;
-    const char *domain;
     const char *tmp_str;
     size_t conf_cnt = 0;
     size_t pos_cnt = 0;
@@ -417,21 +465,7 @@ static void ipa_get_selinux_maps_done(struct tevent_req *subreq)
         return;
     }
 
-    domain = dp_opt_get_string(state->session_ctx->id_ctx->ipa_options->basic,
-                               IPA_KRB5_REALM);
-    subreq = ipa_get_config_send(state, bctx->ev,
-                                 sdap_id_op_handle(state->op),
-                                 id_ctx->sdap_id_ctx->opts,
-                                 domain, NULL);
-    if (subreq == NULL) {
-        ret = ENOMEM;
-        goto done;
-    }
-
-    tevent_req_set_callback(subreq, ipa_get_selinux_config_done, req);
-
-    return;
-
+    ret = EOK;
 done:
     if (ret == EOK) {
         tevent_req_done(req);
@@ -446,13 +480,10 @@ static void ipa_get_selinux_hbac_done(struct tevent_req *subreq)
                                                       struct tevent_req);
     struct ipa_get_selinux_state *state = tevent_req_data(req,
                                                   struct ipa_get_selinux_state);
-    struct be_ctx *bctx = state->be_req->be_ctx;
-    struct ipa_id_ctx *id_ctx = state->session_ctx->id_ctx;
     struct sysdb_attrs **rules;
     struct sysdb_attrs *usermap;
     const char *hbac_dn;
     const char *seealso_dn;
-    const char *domain;
     size_t rule_count;
     size_t conf_cnt;
     size_t pos_cnt;
@@ -534,39 +565,7 @@ static void ipa_get_selinux_hbac_done(struct tevent_req *subreq)
     /* Now we can dispose all possible rules, since they aren't possible any more */
     talloc_zfree(state->possible_match);
 
-    domain = dp_opt_get_string(state->session_ctx->id_ctx->ipa_options->basic,
-                               IPA_KRB5_REALM);
-    subreq = ipa_get_config_send(state, bctx->ev,
-                                 sdap_id_op_handle(state->op),
-                                 id_ctx->sdap_id_ctx->opts,
-                                 domain, NULL);
-    if (subreq == NULL) {
-        ret = ENOMEM;
-        goto done;
-    }
-
-    tevent_req_set_callback(subreq, ipa_get_selinux_config_done, req);
-
-done:
-    if (ret != EOK) {
-        tevent_req_error(req, ret);
-    }
-}
-
-static void ipa_get_selinux_config_done(struct tevent_req *subreq)
-{
-    struct tevent_req *req = tevent_req_callback_data(subreq,
-                                                  struct tevent_req);
-    struct ipa_get_selinux_state *state = tevent_req_data(req,
-                                                  struct ipa_get_selinux_state);
-    errno_t ret;
-
-    ret = ipa_get_config_recv(subreq, state, &state->defaults);
-    talloc_free(subreq);
-    if (ret != EOK) {
-        goto done;
-    }
-
+    ret = EOK;
 done:
     if (ret != EOK) {
         tevent_req_error(req, ret);
-- 
1.7.10.4

-------------- next part --------------
>From b5b4877135b78292f248d4548ff603d35be8c38f Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 16 Jul 2012 19:51:18 +0200
Subject: [PATCH 2/3] SYSDB: Delete SELinux mappings

---
 src/db/sysdb_selinux.c |   17 +++++++++++++++++
 src/db/sysdb_selinux.h |    2 ++
 2 files changed, 19 insertions(+)

diff --git a/src/db/sysdb_selinux.c b/src/db/sysdb_selinux.c
index 8e69cd3e276cc92681410cf7f9fb4921dab17ea7..5311545da74a9ce033791e15111392068028c631 100644
--- a/src/db/sysdb_selinux.c
+++ b/src/db/sysdb_selinux.c
@@ -271,7 +271,24 @@ done:
     return ret;
 }
 
+errno_t sysdb_delete_usermaps(struct sysdb_ctx *sysdb)
+{
+    struct ldb_dn *dn = NULL;
+    errno_t ret;
 
+    dn = ldb_dn_new_fmt(sysdb, sysdb->ldb,
+                        SYSDB_TMPL_SELINUX_BASE, sysdb->domain->name);
+    if (!dn) return ENOMEM;
+
+    ret = sysdb_delete_recursive(sysdb, dn, true);
+    talloc_free(dn);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("sysdb_delete_recursive failed.\n"));
+        return ret;
+    }
+
+    return EOK;
+}
 
 /* --- SYSDB SELinux search routines --- */
 errno_t sysdb_search_selinux_usermap_by_mapname(TALLOC_CTX *mem_ctx,
diff --git a/src/db/sysdb_selinux.h b/src/db/sysdb_selinux.h
index 7a0229236585c45051b3ce2461f84f82d33bf359..39bb67a1921d8e22d49802b6dd09e51b102f18a5 100644
--- a/src/db/sysdb_selinux.h
+++ b/src/db/sysdb_selinux.h
@@ -63,4 +63,6 @@ errno_t sysdb_search_selinux_config(TALLOC_CTX *mem_ctx,
                                     const char **attrs,
                                     struct ldb_message **_config);
 
+errno_t sysdb_delete_usermaps(struct sysdb_ctx *sysdb);
+
 #endif
-- 
1.7.10.4

-------------- next part --------------
>From 0ac596791a05e7fb3132fefac0dcfb1eed5575ea Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Wed, 18 Jul 2012 11:32:57 +0200
Subject: [PATCH 3/3] IPA: Return and save all SELinux rules in the provider

https://fedorahosted.org/sssd/ticket/1421
---
 src/providers/ipa/ipa_session.c |   74 ++++++++++++++-------------------------
 1 file changed, 27 insertions(+), 47 deletions(-)

diff --git a/src/providers/ipa/ipa_session.c b/src/providers/ipa/ipa_session.c
index 385b0090836501fd65a8615389f3d60916e7e086..38f0acd74bf3381dbe54ab1b981f54163f4f08bc 100644
--- a/src/providers/ipa/ipa_session.c
+++ b/src/providers/ipa/ipa_session.c
@@ -46,9 +46,10 @@ struct ipa_get_selinux_state {
     struct sysdb_attrs *user;
 
     struct sysdb_attrs *defaults;
+    struct sysdb_attrs **selinuxmaps;
+    size_t nmaps;
 
     struct sysdb_attrs **possible_match;
-    struct sysdb_attrs **confirmed_match;
 };
 
 static struct
@@ -118,6 +119,13 @@ static void ipa_session_handler_done(struct tevent_req *req)
     if (ret != EOK) goto fail;
     in_transaction = true;
 
+    ret = sysdb_delete_usermaps(breq->sysdb);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              ("Cannot delete existing maps from sysdb\n"));
+        goto fail;
+    }
+
     if (default_user != NULL && map_order != NULL) {
         ret = sysdb_store_selinux_config(sysdb, default_user, map_order);
         if (ret != EOK) {
@@ -369,10 +377,7 @@ static void ipa_get_selinux_maps_done(struct tevent_req *subreq)
     struct be_ctx *bctx;
     struct ipa_id_ctx *id_ctx;
 
-    struct sysdb_attrs **results;
-    size_t count;
     const char *tmp_str;
-    size_t conf_cnt = 0;
     size_t pos_cnt = 0;
     uint32_t priority = 0;
     errno_t ret;
@@ -383,7 +388,8 @@ static void ipa_get_selinux_maps_done(struct tevent_req *subreq)
     bctx = state->be_req->be_ctx;
     id_ctx = state->session_ctx->id_ctx;
 
-    ret = ipa_selinux_get_maps_recv(subreq, state, &count, &results);
+    ret = ipa_selinux_get_maps_recv(subreq, state,
+                                    &state->nmaps, &state->selinuxmaps);
     talloc_free(subreq);
     if (ret != EOK) {
         if (ret == ENOENT) {
@@ -395,62 +401,46 @@ static void ipa_get_selinux_maps_done(struct tevent_req *subreq)
         goto done;
     }
 
-    DEBUG(SSSDBG_TRACE_FUNC, ("Found %d SELinux user maps in total, "
-                              "filtering ...\n", count));
-    state->confirmed_match = talloc_zero_array(state, struct sysdb_attrs *,
-                                               count + 1);
+    DEBUG(SSSDBG_TRACE_FUNC,
+         ("Found %d SELinux user maps\n", state->nmaps));
     state->possible_match = talloc_zero_array(state, struct sysdb_attrs *,
-                                              count + 1);
-    if (state->confirmed_match == NULL || state->possible_match == NULL) {
+                                              state->nmaps + 1);
+    if (state->possible_match == NULL) {
         ret = ENOMEM;
         goto done;
     }
 
-    for (i = 0; i < count; i++) {
-        if (sss_selinux_match(results[i], state->user, state->host, &priority)) {
+    for (i = 0; i < state->nmaps; i++) {
+        if (sss_selinux_match(state->selinuxmaps[i], state->user,
+                              state->host, &priority)) {
             priority &= ~(SELINUX_PRIORITY_USER_NAME |
                           SELINUX_PRIORITY_USER_GROUP |
                           SELINUX_PRIORITY_USER_CAT);
-            ret = sysdb_attrs_add_uint32(results[i],
+            ret = sysdb_attrs_add_uint32(state->selinuxmaps[i],
                                          SYSDB_SELINUX_HOST_PRIORITY,
                                          priority);
             if (ret != EOK) {
                 goto done;
             }
-
-            state->confirmed_match[conf_cnt] = talloc_steal(state->confirmed_match,
-                                                            results[i]);
-            conf_cnt++;
             continue;
         }
 
-        ret = sysdb_attrs_get_string(results[i], SYSDB_SELINUX_SEEALSO, &tmp_str);
+        ret = sysdb_attrs_get_string(state->selinuxmaps[i],
+                                     SYSDB_SELINUX_SEEALSO, &tmp_str);
         if (ret == ENOENT) {
             continue;
         }
 
-        state->possible_match[pos_cnt] = talloc_steal(state->possible_match,
-                                                      results[i]);
+        state->possible_match[pos_cnt] = state->selinuxmaps[i];
         pos_cnt++;
     }
-    DEBUG(SSSDBG_TRACE_FUNC, ("Filtering done. Results: %d confirmed and %d "
-                              "possible maps remained.\n", conf_cnt, pos_cnt));
-
-    /* Don't shrink the confirmed list, it could be later filled by HBAC rules */
-    state->possible_match = talloc_realloc(state, state->possible_match,
-                                           struct sysdb_attrs *, pos_cnt + 1);
-    if (state->possible_match == NULL) {
-        ret = ENOMEM;
-        goto done;
-    }
-
-    /* This will free all rules that are neither confirmed nor possible */
-    talloc_free(results);
 
     if (pos_cnt) {
         /* FIXME: detect if HBAC is configured
          * - if yes, we can skip HBAC retrieval and get it directly from sysdb
          */
+        DEBUG(SSSDBG_TRACE_FUNC, ("%d SELinux maps referenced an HBAC rule. "
+              "Need to refresh HBAC rules\n", pos_cnt));
         subreq = ipa_hbac_rule_info_send(state, false, bctx->ev,
                                          sdap_id_op_handle(state->op),
                                          id_ctx->sdap_id_ctx->opts,
@@ -485,8 +475,6 @@ static void ipa_get_selinux_hbac_done(struct tevent_req *subreq)
     const char *hbac_dn;
     const char *seealso_dn;
     size_t rule_count;
-    size_t conf_cnt;
-    size_t pos_cnt;
     uint32_t priority = 0;
     errno_t ret;
     int i, j;
@@ -497,8 +485,6 @@ static void ipa_get_selinux_hbac_done(struct tevent_req *subreq)
     if (ret != EOK) {
         goto done;
     }
-    for (conf_cnt = 0 ; state->confirmed_match[conf_cnt]; conf_cnt++) ;
-    for (pos_cnt = 0 ; state->possible_match[pos_cnt]; pos_cnt++) ;
 
     for (i = 0; i < rule_count; i++) {
         if (!sss_selinux_match(rules[i], state->user, state->host, &priority)) {
@@ -543,9 +529,6 @@ static void ipa_get_selinux_hbac_done(struct tevent_req *subreq)
                     goto done;
                 }
 
-                state->confirmed_match[conf_cnt++] = talloc_steal(
-                                                        state->confirmed_match,
-                                                        usermap);
                 /* Just to boost the following lookup */
                 state->possible_match[j] = NULL;
 
@@ -586,7 +569,6 @@ ipa_get_selinux_recv(struct tevent_req *req,
             tevent_req_data(req, struct ipa_get_selinux_state);
     const char *tmp_str;
     errno_t ret;
-    int i;
 
     TEVENT_REQ_RETURN_ON_ERROR(req);
 
@@ -618,11 +600,9 @@ ipa_get_selinux_recv(struct tevent_req *req,
         *default_user = NULL;
     }
 
-    if (state->confirmed_match != NULL) {
-        for (i = 0; state->confirmed_match[i]; i++) ;
-
-        *count = i;
-        *maps = talloc_steal(mem_ctx, state->confirmed_match);
+    if (state->selinuxmaps != NULL) {
+        *count = state->nmaps;
+        *maps = talloc_steal(mem_ctx, state->selinuxmaps);
     } else {
         *count = 0;
         *maps = NULL;
-- 
1.7.10.4



More information about the sssd-devel mailing list