[SSSD] [PATCH] IPA: Do not save intermediate data to sysdb

Jakub Hrozek jhrozek at redhat.com
Tue Mar 4 15:15:37 UTC 2014


On Tue, Mar 04, 2014 at 03:50:21PM +0100, Jakub Hrozek wrote:
> Hi,
> 
> the attached patch fixes
> https://bugzilla.redhat.com/show_bug.cgi?id=1071578
> 
> The problem was that the intermediate computing we did added an
> originalMemberUser attribute to sysdb_attrs that was never intended to
> be stored in sysdb. But because we stored the attributes after the
> computing, we also stored originalMemberUser.
> 
> When sssd was online, this wasn't a problem as the cache got purged and
> recreated again, but when offline, adding another originalMemberUser
> added a duplicate attribute so storing the rules failed with EEXIST and
> the user was assigned a default rule.
> 
> Another improvement might be to only delete and store the rules when
> online -- right now the logic is a bit strange when offline as we read
> rules from the cache, delete them and then store the same rules again.

Attached is also a version for 1.11 that differs in DEBUG messages only.
-------------- next part --------------
>From ec2454c2a55669304e0f11c83775746b19ff1646 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Tue, 4 Mar 2014 13:48:36 +0100
Subject: [PATCH] IPA: Do not save intermediate data to sysdb

---
 src/providers/ipa/ipa_selinux.c | 68 ++++++++++++++++++++---------------------
 1 file changed, 34 insertions(+), 34 deletions(-)

diff --git a/src/providers/ipa/ipa_selinux.c b/src/providers/ipa/ipa_selinux.c
index c227db937a84228c0f3945dbe11ba904c7ad9744..2209ca188654d8c79ee402ba71beeadab2904093 100644
--- a/src/providers/ipa/ipa_selinux.c
+++ b/src/providers/ipa/ipa_selinux.c
@@ -251,6 +251,40 @@ static void ipa_selinux_handler_done(struct tevent_req *req)
         goto fail;
     }
 
+    ret = sysdb_transaction_start(sysdb);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
+        goto fail;
+    }
+    in_transaction = true;
+
+    ret = sysdb_delete_usermaps(op_ctx->domain->sysdb, op_ctx->domain);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              ("Cannot delete existing maps from sysdb\n"));
+        goto fail;
+    }
+
+    ret = sysdb_store_selinux_config(sysdb, op_ctx->domain,
+                                     default_user, map_order);
+    if (ret != EOK) {
+        goto fail;
+    }
+
+    if (map_count > 0 && maps != NULL) {
+        ret = ipa_save_user_maps(sysdb, op_ctx->domain, map_count, maps);
+        if (ret != EOK) {
+            goto fail;
+        }
+    }
+
+    ret = sysdb_transaction_commit(sysdb);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Could not commit transaction\n"));
+        goto fail;
+    }
+    in_transaction = false;
+
     /* Process the maps and return list of best matches (maps with
      * highest priority). The input maps are also parent memory
      * context for the output list of best matches. The best match
@@ -279,40 +313,6 @@ static void ipa_selinux_handler_done(struct tevent_req *req)
         goto fail;
     }
 
-    ret = sysdb_transaction_start(sysdb);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
-        goto fail;
-    }
-    in_transaction = true;
-
-    ret = sysdb_delete_usermaps(op_ctx->domain->sysdb, op_ctx->domain);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_CRIT_FAILURE,
-              ("Cannot delete existing maps from sysdb\n"));
-        goto fail;
-    }
-
-    ret = sysdb_store_selinux_config(sysdb, op_ctx->domain,
-                                     default_user, map_order);
-    if (ret != EOK) {
-        goto fail;
-    }
-
-    if (map_count > 0 && maps != NULL) {
-        ret = ipa_save_user_maps(sysdb, op_ctx->domain, map_count, maps);
-        if (ret != EOK) {
-            goto fail;
-        }
-    }
-
-    ret = sysdb_transaction_commit(sysdb);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_OP_FAILURE, ("Could not commit transaction\n"));
-        goto fail;
-    }
-    in_transaction = false;
-
     /* If we got here in online mode, set last_update to current time */
     if (!be_is_offline(be_ctx)) {
         op_ctx->selinux_ctx->last_update = time(NULL);
-- 
1.8.5.3



More information about the sssd-devel mailing list