Dne pondělí 30 července 2012 19:04:50, Jakub Hrozek napsal(a):
> On Mon, Jul 30, 2012 at 09:34:43AM +0200, Jan Zelený wrote:
> > These three patches provide changes that reduce the amount of data
> > retrieved from IPA server in case this data is previously retrieved by
> > HBAC access provider.
> > 
> > #168: modify hbac_get_cached_rules() so it can be used out of the HBAC
> > code
> > #169: use cache for HBAC rules
> > #170: use cache for host record
> 
> Nack, the patches break HBAC-linked SELinux mappings completely.
> 
> hbac_get_cached_rules() doesn't return originalDN, yet
> ipa_get_selinux_hbac_process depends on them.

Thanks for catching this. As it turned out, I had disabled HBAC-linked rules 
on my IPA server at some point, that's why I didn't catch this myself.

> Code style issues are inline.

All fixed, new patches attached.

Thanks
Jan
>From 82092dd091240d2f8034a7f9ee0d100fa1ebbbf7 Mon Sep 17 00:00:00 2001
From: Jan Zeleny <jzel...@redhat.com>
Date: Thu, 26 Jul 2012 11:47:42 -0400
Subject: [PATCH 1/3] Modify hbac_get_cached_rules() so it can be used outside
 of HBAC code

---
 src/providers/ipa/ipa_access.c |   31 +++++++++++++++++--------------
 src/providers/ipa/ipa_access.h |    5 +++++
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/src/providers/ipa/ipa_access.c b/src/providers/ipa/ipa_access.c
index 4cb904ff6ec93aacd47ac30ed67e70f1f12f5d57..571085e5a524d40a90e512dbb0ade0001eed8903 100644
--- a/src/providers/ipa/ipa_access.c
+++ b/src/providers/ipa/ipa_access.c
@@ -600,9 +600,6 @@ fail:
     ipa_access_reply(hbac_ctx, PAM_SYSTEM_ERR);
 }
 
-static errno_t hbac_get_cached_rules(TALLOC_CTX *mem_ctx,
-                                     struct hbac_ctx *hbac_ctx);
-
 void ipa_hbac_evaluate_rules(struct hbac_ctx *hbac_ctx)
 {
     errno_t ret;
@@ -612,7 +609,8 @@ void ipa_hbac_evaluate_rules(struct hbac_ctx *hbac_ctx)
     struct hbac_info *info;
 
     /* Get HBAC rules from the sysdb */
-    ret = hbac_get_cached_rules(hbac_ctx, hbac_ctx);
+    ret = hbac_get_cached_rules(hbac_ctx, hbac_ctx_sysdb(hbac_ctx),
+                                &hbac_ctx->rule_count, &hbac_ctx->rules);
     if (ret != EOK) {
         DEBUG(1, ("Could not retrieve rules from the cache\n"));
         ipa_access_reply(hbac_ctx, PAM_SYSTEM_ERR);
@@ -655,17 +653,20 @@ void ipa_hbac_evaluate_rules(struct hbac_ctx *hbac_ctx)
     ipa_access_reply(hbac_ctx, PAM_PERM_DENIED);
 }
 
-static errno_t hbac_get_cached_rules(TALLOC_CTX *mem_ctx,
-                                     struct hbac_ctx *hbac_ctx)
+errno_t hbac_get_cached_rules(TALLOC_CTX *mem_ctx,
+                              struct sysdb_ctx *sysdb,
+                              size_t *_rule_count,
+                              struct sysdb_attrs ***_rules)
 {
     errno_t ret;
-    struct sysdb_ctx *sysdb = hbac_ctx_sysdb(hbac_ctx);
-    size_t count;
     struct ldb_message **msgs;
+    struct sysdb_attrs **rules;
+    size_t rule_count;
     TALLOC_CTX *tmp_ctx;
     char *filter;
     const char *attrs[] = { OBJECTCLASS,
                             IPA_CN,
+                            SYSDB_ORIG_DN,
                             IPA_UNIQUE_ID,
                             IPA_ENABLED_FLAG,
                             IPA_ACCESS_RULE_TYPE,
@@ -680,7 +681,7 @@ static errno_t hbac_get_cached_rules(TALLOC_CTX *mem_ctx,
                             IPA_HOST_CATEGORY,
                             NULL };
 
-    tmp_ctx = talloc_new(hbac_ctx);
+    tmp_ctx = talloc_new(NULL);
     if (tmp_ctx == NULL) return ENOMEM;
 
     filter = talloc_asprintf(tmp_ctx, "(objectClass=%s)", IPA_HBAC_RULE);
@@ -689,22 +690,24 @@ static errno_t hbac_get_cached_rules(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    ret = sysdb_search_custom(mem_ctx, sysdb, filter,
+    ret = sysdb_search_custom(tmp_ctx, sysdb, filter,
                               HBAC_RULES_SUBDIR, attrs,
-                              &count, &msgs);
+                              &rule_count, &msgs);
     if (ret != EOK && ret != ENOENT) {
         DEBUG(1, ("Error looking up HBAC rules"));
         goto done;
     } if (ret == ENOENT) {
-       count = 0;
+       rule_count = 0;
     }
 
-    ret = sysdb_msg2attrs(mem_ctx, count, msgs, &hbac_ctx->rules);
+    ret = sysdb_msg2attrs(tmp_ctx, rule_count, msgs, &rules);
     if (ret != EOK) {
         DEBUG(1, ("Could not convert ldb message to sysdb_attrs\n"));
         goto done;
     }
-    hbac_ctx->rule_count = count;
+
+    if (_rules) *_rules = talloc_steal(mem_ctx, rules);
+    if (_rule_count) *_rule_count = rule_count;
 
     ret = EOK;
 done:
diff --git a/src/providers/ipa/ipa_access.h b/src/providers/ipa/ipa_access.h
index 6cd425490ade17c52f0ff1f88f5e2892e4c389cc..3c389dec32985bcf0f0f38769eb6ba61babc06e7 100644
--- a/src/providers/ipa/ipa_access.h
+++ b/src/providers/ipa/ipa_access.h
@@ -117,4 +117,9 @@ static inline bool hbac_ctx_is_offline(struct hbac_ctx *ctx)
 
 void ipa_access_handler(struct be_req *be_req);
 
+errno_t hbac_get_cached_rules(TALLOC_CTX *mem_ctx,
+                              struct sysdb_ctx *sysdb,
+                              size_t *_rule_count,
+                              struct sysdb_attrs ***_rules);
+
 #endif /* _IPA_ACCESS_H_ */
-- 
1.7.7.6

>From 64be40dd21e6e078f51235d5b02242f8c321c114 Mon Sep 17 00:00:00 2001
From: Jan Zeleny <jzel...@redhat.com>
Date: Thu, 26 Jul 2012 03:38:22 -0400
Subject: [PATCH 2/3] Support fetching of HBAC rules from sysdb in SELinux
 code

If HBAC is active, SELinux code will reuse them instead of downloading
them from the server again.
---
 src/providers/ipa/ipa_selinux.c |   61 ++++++++++++++++++++++++++++++---------
 1 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/src/providers/ipa/ipa_selinux.c b/src/providers/ipa/ipa_selinux.c
index 03b7eb459a9838f331d515fd99394001565e1c1b..0a2dd0830a891db4421982e106ed14fcfa8e29d1 100644
--- a/src/providers/ipa/ipa_selinux.c
+++ b/src/providers/ipa/ipa_selinux.c
@@ -33,6 +33,7 @@
 #include "providers/ipa/ipa_hosts.h"
 #include "providers/ipa/ipa_hbac_rules.h"
 #include "providers/ipa/ipa_hbac_private.h"
+#include "providers/ipa/ipa_access.h"
 #include "providers/ipa/ipa_selinux_common.h"
 #include "providers/ipa/ipa_selinux_maps.h"
 
@@ -72,6 +73,10 @@ 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 int
+ipa_get_selinux_hbac_process(struct ipa_get_selinux_state *state,
+                             struct sysdb_attrs **rules,
+                             size_t rule_count);
 
 void ipa_selinux_handler(struct be_req *be_req)
 {
@@ -379,6 +384,11 @@ static void ipa_get_selinux_maps_done(struct tevent_req *subreq)
     struct be_ctx *bctx;
     struct ipa_id_ctx *id_ctx;
 
+    char *selinux_name;
+    char *access_name;
+    struct sysdb_attrs **rules;
+    size_t rule_count;
+
     const char *tmp_str;
     uint32_t priority = 0;
     errno_t ret;
@@ -437,9 +447,19 @@ static void ipa_get_selinux_maps_done(struct tevent_req *subreq)
     }
 
     if (state->possible_matches) {
-        /* FIXME: detect if HBAC is configured
-         * - if yes, we can skip HBAC retrieval and get it directly from sysdb
-         */
+        access_name = state->be_req->be_ctx->bet_info[BET_ACCESS].mod_name;
+        selinux_name = state->be_req->be_ctx->bet_info[BET_SELINUX].mod_name;
+        if (strcasecmp(access_name, selinux_name) == 0) {
+            ret = hbac_get_cached_rules(state, state->be_req->be_ctx->sysdb,
+                                        &rule_count, &rules);
+            if (ret != EOK) {
+                goto done;
+            }
+
+            ret = ipa_get_selinux_hbac_process(state, rules, rule_count);
+            goto done;
+        }
+
         DEBUG(SSSDBG_TRACE_FUNC, ("%d SELinux maps referenced an HBAC rule. "
               "Need to refresh HBAC rules\n", state->possible_matches));
         subreq = ipa_hbac_rule_info_send(state, false, bctx->ev,
@@ -472,14 +492,8 @@ static void ipa_get_selinux_hbac_done(struct tevent_req *subreq)
     struct ipa_get_selinux_state *state = tevent_req_data(req,
                                                   struct ipa_get_selinux_state);
     struct sysdb_attrs **rules;
-    struct sysdb_attrs *usermap;
-    struct ldb_message_element *el;
-    const char *hbac_dn;
-    const char *seealso_dn;
     size_t rule_count;
-    uint32_t priority = 0;
     errno_t ret;
-    int i, j;
 
     ret = ipa_hbac_rule_info_recv(subreq, state, &rule_count,
                                   &rules);
@@ -490,6 +504,29 @@ static void ipa_get_selinux_hbac_done(struct tevent_req *subreq)
         goto done;
     }
 
+    ret = ipa_get_selinux_hbac_process(state, rules, rule_count);
+
+done:
+    if (ret != EOK) {
+        tevent_req_error(req, ret);
+    } else {
+        tevent_req_done(req);
+    }
+}
+
+static int
+ipa_get_selinux_hbac_process(struct ipa_get_selinux_state *state,
+                             struct sysdb_attrs **rules,
+                             size_t rule_count)
+{
+    int i, j;
+    errno_t ret;
+    uint32_t priority = 0;
+    const char *hbac_dn;
+    const char *seealso_dn;
+    struct sysdb_attrs *usermap;
+    struct ldb_message_element *el;
+
     for (i = 0; i < rule_count; i++) {
         ret = sysdb_attrs_get_string(rules[i], SYSDB_ORIG_DN, &hbac_dn);
         if (ret != EOK) {
@@ -563,11 +600,7 @@ static void ipa_get_selinux_hbac_done(struct tevent_req *subreq)
 
     ret = EOK;
 done:
-    if (ret != EOK) {
-        tevent_req_error(req, ret);
-    } else {
-        tevent_req_done(req);
-    }
+    return ret;
 }
 
 static errno_t
-- 
1.7.7.6

>From 5e2d4edd6f36a4bc3bf39acf851ac7a5b71adc71 Mon Sep 17 00:00:00 2001
From: Jan Zeleny <jzel...@redhat.com>
Date: Fri, 27 Jul 2012 04:28:24 -0400
Subject: [PATCH 3/3] Support fetching of host from sysdb in SELinux code

The host record will be fetched if HBAC is used as access provider since
the record is already downloaded and it can be trusted to be valid.
---
 src/providers/ipa/ipa_selinux.c |   66 ++++++++++++++++++++++++++++++++------
 1 files changed, 55 insertions(+), 11 deletions(-)

diff --git a/src/providers/ipa/ipa_selinux.c b/src/providers/ipa/ipa_selinux.c
index 0a2dd0830a891db4421982e106ed14fcfa8e29d1..b5a84269e498774db08a9f140b16150758ff6349 100644
--- a/src/providers/ipa/ipa_selinux.c
+++ b/src/providers/ipa/ipa_selinux.c
@@ -245,6 +245,17 @@ static void ipa_get_selinux_connect_done(struct tevent_req *subreq)
     struct ipa_id_ctx *id_ctx = state->selinux_ctx->id_ctx;
     struct be_ctx *bctx = state->be_req->be_ctx;
 
+    const char *access_name;
+    const char *selinux_name;
+    struct ldb_dn *host_dn;
+    const char *attrs[] = { SYSDB_ORIG_DN,
+                            SYSDB_ORIG_MEMBEROF,
+                            NULL };
+    size_t count;
+    struct ldb_message **msgs;
+    struct sysdb_attrs **hosts;
+    struct sss_domain_info *domain;
+
     ret = sdap_id_op_connect_recv(subreq, &dp_error);
     talloc_zfree(subreq);
 
@@ -260,10 +271,42 @@ static void ipa_get_selinux_connect_done(struct tevent_req *subreq)
     state->hostname = dp_opt_get_string(state->selinux_ctx->id_ctx->ipa_options->basic,
                                         IPA_HOSTNAME);
 
-    /* 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()
-     */
+    access_name = state->be_req->be_ctx->bet_info[BET_ACCESS].mod_name;
+    selinux_name = state->be_req->be_ctx->bet_info[BET_SELINUX].mod_name;
+    if (strcasecmp(access_name, selinux_name) == 0) {
+        domain = sysdb_ctx_get_domain(bctx->sysdb);
+        host_dn = sysdb_custom_dn(bctx->sysdb, state, domain->name,
+                                  state->hostname, HBAC_HOSTS_SUBDIR);
+        if (host_dn == NULL) {
+            ret = ENOMEM;
+            goto fail;
+        }
+
+        /* Look up the host to get its originalMemberOf entries */
+        ret = sysdb_search_entry(state, bctx->sysdb, host_dn,
+                                 LDB_SCOPE_BASE, NULL,
+                                 attrs, &count, &msgs);
+        if (ret == ENOENT || count == 0) {
+            /* We need to query the server */
+            goto server;
+        } else if (ret != EOK) {
+            goto fail;
+        } else if (count > 1) {
+            DEBUG(SSSDBG_OP_FAILURE, ("More than one result for a BASE search!\n"));
+            ret = EIO;
+            goto fail;
+        }
+
+        ret = sysdb_msg2attrs(state, count, msgs, &hosts);
+        if (ret != EOK) {
+            goto fail;
+        }
+
+        state->host = hosts[0];
+        return ipa_get_config_step(req);
+    }
+
+server:
     subreq = ipa_host_info_send(state, bctx->ev, bctx->sysdb,
                                 sdap_id_op_handle(state->op),
                                 id_ctx->sdap_id_ctx->opts,
@@ -291,7 +334,6 @@ static void ipa_get_selinux_hosts_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;
     size_t host_count, hostgroup_count;
     struct sysdb_attrs **hostgroups;
     struct sysdb_attrs **host;
@@ -304,12 +346,6 @@ static void ipa_get_selinux_hosts_done(struct tevent_req *subreq)
     }
     state->host = host[0];
 
-    ret = sss_selinux_extract_user(state, bctx->sysdb,
-                                   state->pd->user, &state->user);
-    if (ret != EOK) {
-        goto done;
-    }
-
     return ipa_get_config_step(req);
 
 done:
@@ -320,6 +356,7 @@ done:
 
 static void ipa_get_config_step(struct tevent_req *req)
 {
+    errno_t ret;
     const char *domain;
     struct tevent_req *subreq;
     struct ipa_get_selinux_state *state = tevent_req_data(req,
@@ -327,6 +364,13 @@ static void ipa_get_config_step(struct tevent_req *req)
     struct be_ctx *bctx = state->be_req->be_ctx;
     struct ipa_id_ctx *id_ctx = state->selinux_ctx->id_ctx;
 
+    ret = sss_selinux_extract_user(state, bctx->sysdb,
+                                   state->pd->user, &state->user);
+    if (ret != EOK) {
+        tevent_req_error(req, ret);
+        return;
+    }
+
     domain = dp_opt_get_string(state->selinux_ctx->id_ctx->ipa_options->basic,
                                IPA_KRB5_REALM);
     subreq = ipa_get_config_send(state, bctx->ev,
-- 
1.7.7.6

Attachment: signature.asc
Description: This is a digitally signed message part.

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to