Security plugin: adding checking of return code for dbus policy verification

2 files changed, 37 insertions, 35 deletions

diff --git a/plugins/msm-plugin.c b/plugins/msm-plugin.c
index bc95cf447..569f9a2ce 100644
--- a/plugins/msm-plugin.c
+++ b/plugins/msm-plugin.c
@@ -500,6 +500,8 @@ rpmRC PLUGINHOOK_PSM_PRE_FUNC(rpmte te)
     rpmRC rc = RPMRC_OK;
     int ret = 0;
 
+    package_created = 0;
+
     if (!root && !rootSWSource) {
         /* no sw source config, just exit */
         goto exit;
@@ -535,6 +537,7 @@ rpmRC PLUGINHOOK_PSM_PRE_FUNC(rpmte te)
             }
             headerFree(h);
         }
+        package_created = 1;
         /* if (!current) {
             rpmlog(RPMLOG_INFO, "no sw source for removing %s\n", rpmteN(ctx->te));
             goto exit;
@@ -624,6 +627,7 @@ rpmRC PLUGINHOOK_PSM_PRE_FUNC(rpmte te)
             goto fail;
         }
 
+        package_created = 1;
         if (rootSWSource) {
             /* current is root */
             root = ctx->mfx;
@@ -639,7 +643,6 @@ rpmRC PLUGINHOOK_PSM_PRE_FUNC(rpmte te)
                 if (ret) {
                     rpmlog(RPMLOG_ERR, "SW source setup failed for %s\n",
 			    rpmteN(ctx->te));
-                    msmCancelPackage(ctx->mfx->name);
                     goto fail;
                 }
             }           
@@ -648,7 +651,6 @@ rpmRC PLUGINHOOK_PSM_PRE_FUNC(rpmte te)
                 if (ret) {
                     rpmlog(RPMLOG_ERR, "AC domain setup failed for %s\n",
                            rpmteN(ctx->te));
-                    msmCancelPackage(ctx->mfx->name);
                     goto fail;
                 } else {
 		    smackLabel = 1;
@@ -661,7 +663,14 @@ rpmRC PLUGINHOOK_PSM_PRE_FUNC(rpmte te)
                 if (ret) {
                     rpmlog(RPMLOG_ERR, "Request setup failed for %s\n",
                            rpmteN(ctx->te));
-                    msmCancelPackage(ctx->mfx->name);
+                    goto fail;
+                }
+            }
+            if (package->provides) {
+                ret = msmSetupDBusPolicies(package, ctx->mfx);
+                if (ret) {
+                    rpmlog(RPMLOG_ERR, "Setting up dbus policies for %s failed\n",
+                           rpmteN(ctx->te));
                     goto fail;
                 }
             }
@@ -672,30 +681,20 @@ rpmRC PLUGINHOOK_PSM_PRE_FUNC(rpmte te)
                 if (ret) {
                     rpmlog(RPMLOG_ERR, "Setting up smack rules for %s failed\n",
                            rpmteN(ctx->te));
-                    msmCancelPackage(ctx->mfx->name);
                     goto fail; 
                 }
             }
-            if (package->provides) {
-                ret = msmSetupDBusPolicies(package, ctx->mfx);
-                if (ret) {
-                    rpmlog(RPMLOG_ERR, "Setting up dbus policies for %s failed\n",
-                           rpmteN(ctx->te));
-                    msmCancelPackage(ctx->mfx->name);
-                    goto fail;
-                }
-            }
 
             /* last check is needed in order to catch in advance 
                the situation when no ac domain defined or requested */
             if (smackLabel == 0) {
                 rpmlog(RPMLOG_ERR, "No ac domain defined or requested for package %s. Abort.\n",   rpmteN(ctx->te));
-                msmCancelPackage(ctx->mfx->name);
                 goto fail;
             }
         }
 
     } else if (rpmteDependsOn(ctx->te)) { /* TR_REMOVED */
+        package_created = 1;
         rpmlog(RPMLOG_DEBUG, "upgrading package %s by %s\n",
                rpmteNEVR(ctx->te), rpmteNEVR(rpmteDependsOn(ctx->te)));
     } else if (mfx->sw_sources) {
@@ -705,7 +704,6 @@ rpmRC PLUGINHOOK_PSM_PRE_FUNC(rpmte te)
     }
 
     rpmlog(RPMLOG_DEBUG, "Finished with pre psm hook \n");
-    package_created = 1;
 
     goto exit;
 
@@ -799,7 +797,6 @@ rpmRC PLUGINHOOK_FSM_COMMIT_FUNC(const char* path, mode_t mode, int type)
 rpmRC PLUGINHOOK_PSM_POST_FUNC(rpmte te, int rpmrc)
 {
 
-    int ret = 0;
     packagecontext *ctx = context;
     if (!ctx) return RPMRC_FAIL;
 
@@ -812,13 +809,13 @@ rpmRC PLUGINHOOK_PSM_POST_FUNC(rpmte te, int rpmrc)
         /* failure in rpm psm, rollback */
         if (rpmteType(ctx->te) == TR_ADDED)
             msmCancelPackage(ctx->mfx->name);
-        goto exit;
+        return RPMRC_FAIL;
     }
 
     if (!ctx->mfx){
         rpmlog(RPMLOG_ERR, "Manifest is missing while it should be present for the package %s\n",
                rpmteN(ctx->te));
-        goto exit;
+        return RPMRC_FAIL;
     }
 
     if (rootSWSource) {
@@ -842,12 +839,6 @@ rpmRC PLUGINHOOK_PSM_POST_FUNC(rpmte te, int rpmrc)
         }
     }
 
- exit:
-    current = NULL;
-
-    if (ret) {
-        return RPMRC_FAIL;
-    }
     return rpmrc;
 }
 
diff --git a/plugins/msmxattr.c b/plugins/msmxattr.c
index a5c5c81a3..eaf064dd1 100644
--- a/plugins/msmxattr.c
+++ b/plugins/msmxattr.c
@@ -548,11 +548,12 @@ static int msmSetupDBusConfig(package_x *package, dbus_x *dbus, int phase, manif
     node_x *node;
     interface_x *interface;
     member_x *member;
-    int ret = -1;
+    int ret = 0;
 
     char *sysconfdir = rpmExpand("%{?_sysconfdir}", NULL);
     if (!sysconfdir || !strcmp(sysconfdir, "")) {
 	rpmlog(RPMLOG_ERR, "Failed to expand %%_sysconfdir macro\n");
+        ret = -1;
 	goto exit;
     }
     snprintf(path, sizeof(path), "%s/dbus-1/%s.d/manifest.%s.conf", 
@@ -561,6 +562,7 @@ static int msmSetupDBusConfig(package_x *package, dbus_x *dbus, int phase, manif
     file = fopen(path, phase ? "a" : "w");
     if (!file) {
 	rpmlog(RPMLOG_ERR, "Cannot open %s: %s\n", path, strerror(errno));
+        ret = -1;
 	goto exit;
     }
 
@@ -573,6 +575,7 @@ static int msmSetupDBusConfig(package_x *package, dbus_x *dbus, int phase, manif
 	if (fputs(data, file) == EOF) {
 	    rpmlog(RPMLOG_ERR, "Failed to write %s: %s\n", 
 		   path, strerror(errno));
+            ret = -1;
 	    goto exit;
 	}
     }
@@ -590,6 +593,7 @@ static int msmSetupDBusConfig(package_x *package, dbus_x *dbus, int phase, manif
             if (fputs(data, file) == EOF) {
                 rpmlog(RPMLOG_ERR, "Failed to write %s: %s\n", 
                        path, strerror(errno));
+                ret = -1;
                 goto exit;
             }
 	}
@@ -599,26 +603,30 @@ static int msmSetupDBusConfig(package_x *package, dbus_x *dbus, int phase, manif
 	}
 	for (node = dbus->nodes; node; node = node->prev) {
 	    if (node->annotation) {
-                msmSetupDBusRule(file, node->annotation->value, DBUS_PATH,
+                ret = msmSetupDBusRule(file, node->annotation->value, DBUS_PATH,
                                  dbus->name, node->name, NULL, NULL, mfx);
+                if (ret < 0) goto exit;
 	    }
 	    for (member = node->members; member; member = member->prev) {
 		if (member->annotation) {
-                    msmSetupDBusRule(file, member->annotation->value, member->type, 
+                    ret = msmSetupDBusRule(file, member->annotation->value, member->type, 
                                      dbus->name, member->name, 
                                      "path", node->name, mfx);
+                    if (ret < 0) goto exit;
 		}
 	    }
 	    for (interface = node->interfaces; interface; interface = interface->prev) {
 		if (interface->annotation) {
-                    msmSetupDBusRule(file, interface->annotation->value, DBUS_INTERFACE, 
+                    ret = msmSetupDBusRule(file, interface->annotation->value, DBUS_INTERFACE, 
                                      dbus->name, interface->name, NULL, NULL, mfx);
+                    if (ret < 0) goto exit;
 		}
 		for (member = interface->members; member; member = member->prev) {
 		    if (member->annotation) {
-                        msmSetupDBusRule(file, member->annotation->value, member->type, 
+                        ret = msmSetupDBusRule(file, member->annotation->value, member->type, 
                                          dbus->name, member->name,
                                          "interface", interface->name, mfx);
+                        if (ret < 0) goto exit;
 		    }
 		}
 	    }
@@ -630,6 +638,7 @@ static int msmSetupDBusConfig(package_x *package, dbus_x *dbus, int phase, manif
 	if (fputs(data, file) == EOF) {
 	    rpmlog(RPMLOG_ERR, "Failed to write %s: %s\n", 
 		   path, strerror(errno));
+            ret = -1;
 	    goto exit;
 	}
 	rpmlog(RPMLOG_DEBUG, "wrote dbus config %s\n", path);	
@@ -638,7 +647,7 @@ static int msmSetupDBusConfig(package_x *package, dbus_x *dbus, int phase, manif
 
  exit:
     if (file) fclose(file);
-    if (ret) unlink(path);
+    if (ret < 0) unlink(path);
     msmFreePointer((void**)&sysconfdir);
     return ret;
 }
@@ -794,22 +803,24 @@ int msmSetupDBusPolicies(package_x *package, manifest_x *mfx)
     dbus_x *system = NULL;
     provide_x *provide;
     dbus_x *dbus;
+    int ret = 0;
 
     for (provide = package->provides; provide; provide = provide->prev) {
         for (dbus = provide->dbuss; dbus; dbus = dbus->prev) {
             if (!strcmp(dbus->bus, "session")) {
-                msmSetupDBusConfig(package, dbus, session ? 1 : 0, mfx);
+                ret = msmSetupDBusConfig(package, dbus, session ? 1 : 0, mfx);
                 session = dbus;
             } else if (!strcmp(dbus->bus, "system")) {
-                msmSetupDBusConfig(package, dbus, system ? 1 : 0, mfx);
+                ret = msmSetupDBusConfig(package, dbus, system ? 1 : 0, mfx);
                 system = dbus;
             } else return -1;
+            if (ret < 0) return ret;
         }
-       if (session) msmSetupDBusConfig(package, session, -1, mfx);
-       if (system) msmSetupDBusConfig(package, system, -1, mfx);
+       if (session) ret = msmSetupDBusConfig(package, session, -1, mfx);
+       if (system) ret = msmSetupDBusConfig(package, system, -1, mfx);
        session = system = NULL;
     }
-    return 0;
+    return ret;
 }
 
 static int msmCheckDomainRequestOrPermit(manifest_x *mfx, const char* domain)