Security plugin: allowing multiple domains definition
authorElena Reshetova <elena.reshetova@intel.com>
Fri, 5 Jul 2013 11:48:42 +0000 (14:48 +0300)
committerAnas Nashif <anas.nashif@intel.com>
Mon, 5 Aug 2013 16:52:44 +0000 (12:52 -0400)
- allowing multiple domains definition per manifest
- fixing indirect include of config.h
- restricting adding new sw source with the same key info

plugins/msm-plugin.c
plugins/msm.h
plugins/msmmanifest.c
plugins/msmxattr.c

index cc79bd0..f921329 100644 (file)
@@ -25,9 +25,6 @@
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
  * 02110-1301 USA
  */
-#include "plugin.h"
-#include "debug.h"
 
 #include <errno.h>
 #include <sys/types.h>
@@ -585,16 +582,16 @@ rpmRC PLUGINHOOK_PSM_PRE_FUNC(rpmte te)
                     goto fail;
                 }
             }           
-            if (ctx->mfx->define) {
-                if (ctx->mfx->define->name)
-                    smackLabel = 1;
-                ret = msmSetupDefine(ctx->smack_accesses, ctx->mfx);
+            if (ctx->mfx->defines) {
+                ret = msmSetupDefines(ctx->smack_accesses, ctx->mfx);
                 if (ret) {
                     rpmlog(RPMLOG_ERR, "AC domain setup failed for %s\n",
                            rpmteN(ctx->te));
                     msmCancelPackage(ctx->mfx->name);
                     goto fail;
-                }
+                } else {
+                   smackLabel = 1;
+               }
             }           
             if (ctx->mfx->request) {   
                 if (ctx->mfx->request->ac_domain)
@@ -776,7 +773,7 @@ rpmRC PLUGINHOOK_PSM_POST_FUNC(rpmte te, int rpmrc)
             } else {
                 rpmlog(RPMLOG_DEBUG, "removing %s manifest data\n", 
                        rpmteN(ctx->te));
-                if (ctx->mfx->define || ctx->mfx->provides || ctx->mfx->sw_sources) {
+                if (ctx->mfx->defines || ctx->mfx->provides || ctx->mfx->sw_sources) {
                     msmRemoveRules(ctx->smack_accesses, ctx->mfx, SmackEnabled);
                 }          
                 msmRemoveConfig(ctx->mfx);
index ffaddd3..4a0423d 100644 (file)
@@ -52,6 +52,9 @@
 #define DBUS_METHOD    4
 #define DBUS_SIGNAL    5
 
+#include "plugin.h"
+#include "debug.h"
+
 #include <uthash.h>
 #include <sys/capability.h>
 #include <sys/smack.h>
@@ -248,6 +251,8 @@ typedef struct define_x {
     struct d_request_x *d_requests;
     struct d_permit_x *d_permits;
     struct d_provide_x *d_provides;
+    struct define_x *prev;
+    struct define_x *next;
 } define_x;
 
 typedef struct package_x {
@@ -285,7 +290,7 @@ typedef struct manifest_x { /*package manifest */
     struct provide_x *provides; /* assign section */
     struct request_x *request; /* request section */
     struct sw_source_x *sw_sources; /*defined software sources(non-NULL only for configuration manifests)*/
-    struct define_x *define; /* define section */
+    struct define_x *defines; /* define section(s) */
     struct file_x *files; /* installed files */
 } manifest_x;
 
@@ -353,7 +358,7 @@ int msmSetupPackages(struct smack_accesses *smack_accesses, package_x *packages,
  * @param mfx                  package manifest
  * @return                     0 on success, else -1
  */
-int msmSetupDefine(struct smack_accesses *smack_accesses, manifest_x *mfx);
+int msmSetupDefines(struct smack_accesses *smack_accesses, manifest_x *mfx);
 
 /** \ingroup msm
  * Setup smack rules according to the manifest
index 7eae7c5..c713923 100644 (file)
@@ -822,7 +822,24 @@ static int msmProcessDefine(xmlTextReaderPtr reader, define_x *define, manifest_
     return ret;
 }
 
-static int msmProcessKeyinfo(xmlTextReaderPtr reader, origin_x *origin) 
+static int findSWSourceByKey(sw_source_x *sw_source, void *param)
+{
+    origin_x *origin;
+    keyinfo_x *keyinfo;
+    keyinfo_x *current_keyinfo = (keyinfo_x*)param;
+
+    for (origin = sw_source->origins; origin; origin = origin->prev) {
+           for (keyinfo = origin->keyinfos; keyinfo; keyinfo = keyinfo->prev) {
+               if (strncmp((const char*)current_keyinfo->keydata, (const char*)keyinfo->keydata, 
+                   strlen((const char*)current_keyinfo->keydata)) == 0
+                   && (current_keyinfo->keylen == keyinfo->keylen))
+                       return 0;
+           }
+    }
+    return 1;
+}
+
+static int msmProcessKeyinfo(xmlTextReaderPtr reader, origin_x *origin, sw_source_x *parent) 
 {
     const xmlChar *keydata;
     keyinfo_x *keyinfo;
@@ -839,6 +856,14 @@ static int msmProcessKeyinfo(xmlTextReaderPtr reader, origin_x *origin)
                rpmlog(RPMLOG_ERR, "Failed to decode keyinfo %s, %d\n", keydata, ret);
                ret = -1;
            }
+           if (!ret) {
+               // check that keys aren't matching
+               sw_source_x *old = msmSWSourceTreeTraversal(parent, findSWSourceByKey, (void *)keyinfo, NULL);
+               if (old) {
+                   rpmlog(RPMLOG_ERR, "SW source with this key has already been installed\n");
+                   return -1; 
+               }
+           }
            LISTADD(origin->keyinfos, keyinfo);
        } else return -1;
 
@@ -868,7 +893,7 @@ static access_x *msmProcessAccess(xmlTextReaderPtr reader, origin_x *origin)
     return NULL;
 }
 
-static int msmProcessOrigin(xmlTextReaderPtr reader, origin_x *origin) 
+static int msmProcessOrigin(xmlTextReaderPtr reader, origin_x *origin, sw_source_x *parent)  
 {
     const xmlChar *node, *type;
     int ret, depth;
@@ -882,7 +907,7 @@ static int msmProcessOrigin(xmlTextReaderPtr reader, origin_x *origin)
        node = xmlTextReaderConstName(reader);
        if (!node) return -1;
        if (!strcmp(ASCII(node), "keyinfo")) {
-           ret = msmProcessKeyinfo(reader, origin);
+           ret = msmProcessKeyinfo(reader, origin, parent);
        } else if (!strcmp(ASCII(node), "access")) {
            access_x *access = msmProcessAccess(reader, origin);
            if (access) {
@@ -1022,7 +1047,7 @@ static int msmProcessSWSource(xmlTextReaderPtr reader, sw_source_x *sw_source, c
            origin_x *origin = calloc(1, sizeof(origin_x));
            if (origin) {
                LISTADD(sw_source->origins, origin);
-               ret = msmProcessOrigin(reader, origin);
+               ret = msmProcessOrigin(reader, origin, sw_source->parent);
            } else return -1;
        } else if (!strcmp(ASCII(node), "package")) {
            /* config processing */
@@ -1093,7 +1118,7 @@ static int msmProcessMsm(xmlTextReaderPtr reader, manifest_x *mfx, sw_source_x *
 {
     const xmlChar *node;
     int ret, depth;
-    int assignPresent = 0, requestPresent = 0, definePresent = 0, attributesPresent = 0; /* there must be only one section per manifest */
+    int assignPresent = 0, requestPresent = 0, attributesPresent = 0; /* there must be only one section per manifest */
     mfx->sw_source = current;
 
     rpmlog(RPMLOG_DEBUG, "manifest\n");
@@ -1121,14 +1146,10 @@ static int msmProcessMsm(xmlTextReaderPtr reader, manifest_x *mfx, sw_source_x *
             attributesPresent = 1;
             ret = msmProcessAttributes(reader, mfx);
        } else if (!strcmp(ASCII(node), "define")) {
-            if (definePresent) {
-                rpmlog(RPMLOG_ERR, "A second request section in manifest isn't allowed. Abort installation.\n");
-                return -1; 
-            }
-            definePresent = 1;
-            mfx->define = calloc(1, sizeof(define_x));
-            if (mfx->define) {
-                ret = msmProcessDefine(reader, mfx->define, mfx, current);
+            define_x *define = calloc(1, sizeof(define_x));
+            if (define) {
+                LISTADD(mfx->defines, define);
+                ret = msmProcessDefine(reader, define, mfx, current);
             } else return -1;
        } else if (!strcmp(ASCII(node), "request")) {
             if (requestPresent) {
@@ -1416,14 +1437,42 @@ static d_provide_x *msmFreeDProvide(d_provide_x *d_provide)
     return next;
 }
 
+static define_x *msmFreeDefine(define_x *define)
+{
+    define_x *next = define->next;
+    d_request_x *d_request;
+    d_permit_x *d_permit;
+    d_provide_x *d_provide;
+
+    msmFreePointer((void**)&define->name);
+    msmFreePointer((void**)&define->policy);
+    msmFreePointer((void**)&define->plist);
+
+    if (define->d_requests) {
+       LISTHEAD(define->d_requests, d_request);        
+       for (; d_request; d_request = msmFreeDRequest(d_request));
+    }
+    rpmlog(RPMLOG_DEBUG, "after freeing define requests\n");
+    if (define->d_permits) {
+       LISTHEAD(define->d_permits, d_permit);  
+       for (; d_permit; d_permit = msmFreeDPermit(d_permit));
+    }
+    rpmlog(RPMLOG_DEBUG, "after freeing define permits\n");
+    if (define->d_provides) {
+       LISTHEAD(define->d_provides, d_provide);        
+       for (; d_provide; d_provide = msmFreeDProvide(d_provide));
+    }
+    rpmlog(RPMLOG_DEBUG, "after freeing provides\n");
+    msmFreePointer((void**)&define); 
+    return next;
+}
+
 manifest_x* msmFreeManifestXml(manifest_x* mfx)
 {
     provide_x *provide;
     file_x *file;
     sw_source_x *sw_source;
-    d_request_x *d_request;
-    d_permit_x *d_permit;
-    d_provide_x *d_provide;
+    define_x *define;
 
     rpmlog(RPMLOG_DEBUG, "in msmFreeManifestXml\n");
     if (mfx) {
@@ -1443,27 +1492,10 @@ manifest_x* msmFreeManifestXml(manifest_x* mfx)
        }
        msmFreePointer((void**)&mfx->name);
         rpmlog(RPMLOG_DEBUG, "after freeing name\n");
-       if (mfx->define) {
-            msmFreePointer((void**)&mfx->define->name);
-            msmFreePointer((void**)&mfx->define->policy);
-            msmFreePointer((void**)&mfx->define->plist);
-            if (mfx->define->d_requests) {
-                LISTHEAD(mfx->define->d_requests, d_request);  
-                for (; d_request; d_request = msmFreeDRequest(d_request));
-            }
-            rpmlog(RPMLOG_DEBUG, "after freeing define requests\n");
-            if (mfx->define->d_permits) {
-                LISTHEAD(mfx->define->d_permits, d_permit);    
-                for (; d_permit; d_permit = msmFreeDPermit(d_permit));
-            }
-            rpmlog(RPMLOG_DEBUG, "after freeing define permits\n");
-            if (mfx->define->d_provides) {
-                LISTHEAD(mfx->define->d_provides, d_provide);  
-                for (; d_provide; d_provide = msmFreeDProvide(d_provide));
-            }
-            rpmlog(RPMLOG_DEBUG, "after freeing provides\n");
-            msmFreePointer((void**)&mfx->define); 
-       }
+        if (mfx->defines) {
+            LISTHEAD(mfx->defines, define);
+            for (; define; define = msmFreeDefine(define));            
+        }
         rpmlog(RPMLOG_DEBUG, "after freeing defines \n");
         msmFreePointer((void**)&mfx);
     }
index fc8af6b..5af6ed0 100644 (file)
@@ -101,12 +101,15 @@ static int msmCheckLabelProvisioning(manifest_x *mfx, const char* label)
 {
 
     d_provide_x *provide = NULL;
+    define_x *define = NULL;
 
-    if ((mfx) && (label) && (mfx->define) && (mfx->define->d_provides)) {
-        for (provide = mfx->define->d_provides; provide; provide = provide->prev) {
-            if (strcmp(provide->label_name, label) == 0)
-                return 0;
-        }
+    if ((mfx) && (label) && (mfx->defines)) {
+        for (define = mfx->defines; define; define = define->prev) {
+           for (provide = define->d_provides; provide; provide = provide->prev) {
+               if (strcmp(provide->label_name, label) == 0)
+                    return 0;
+           }
+       }
     }
     rpmlog(RPMLOG_ERR, "Label %s hasn't been provided in the manifest\n", label);
     return -1;
@@ -713,6 +716,7 @@ static int msmCheckDomainJoinPossibility(manifest_x *mfx, ac_domain_x *defined_a
 int msmSetupRequests(manifest_x *mfx) 
 {
     ac_domain_x *defined_ac_domain = NULL; 
+    define_x *define = NULL;
 
     if ((!mfx) || (!mfx->request) || (!mfx->request->ac_domain))
         return -1;
@@ -727,11 +731,15 @@ int msmSetupRequests(manifest_x *mfx)
 #endif
     }
     //now check that the package can join the requested AC domain
-    if (mfx->define){
-        rpmlog(RPMLOG_DEBUG, "mfx->define->name %s mfx->request->ac_domain %s\n", mfx->define->name, mfx->request->ac_domain);
-        if (strcmp(mfx->define->name, mfx->request->ac_domain) == 0)
-            //ac domain is requested from the same package where it was define. This case is always allowed
-            return 0;          
+    if (mfx->defines){
+        LISTHEAD(mfx->defines, define);
+        while(define) {
+            rpmlog(RPMLOG_DEBUG, "define->name %s mfx->request->ac_domain %s\n", define->name, mfx->request->ac_domain);
+            if (strcmp(define->name, mfx->request->ac_domain) == 0)
+                //ac domain is requested from the same package where it was define. This case is always allowed
+                return 0;
+            define = define->next;
+        }              
     } 
     //need to check if developer allowed other packages to join this domain
     if (msmCheckDomainJoinPossibility(mfx, defined_ac_domain) < 0) {
@@ -819,6 +827,7 @@ int msmSetupDBusPolicies(package_x *package, manifest_x *mfx)
 static int msmCheckDomainRequestOrPermit(manifest_x *mfx, const char* domain) 
 {
     ac_domain_x *defined_ac_domain = NULL; 
+    define_x *define = NULL;
     char* name = NULL;
 
     if ((!mfx) || (!domain))
@@ -838,14 +847,18 @@ static int msmCheckDomainRequestOrPermit(manifest_x *mfx, const char* domain)
     }
 
     //now check that this ac_domain can be requested
-    if ((mfx->define) && (mfx->define->name)) {
-        rpmlog(RPMLOG_DEBUG, "mfx->define->name %s domain %s\n", mfx->define->name, name);
-        if (strcmp(mfx->define->name, name) == 0) {
-            // AC domain access is requested or permitted from the same package where it was defined. 
-            // This case is always allowed
-            msmFreePointer((void**)&name);
-            return 0;          
-         }
+    if (mfx->defines) {
+        LISTHEAD(mfx->defines, define);
+        while (define) {
+            rpmlog(RPMLOG_DEBUG, "define->name %s domain %s\n", define->name, name);
+            if (strcmp(define->name, name) == 0) {
+                // AC domain access is requested or permitted from the same package where it was defined. 
+                // This case is always allowed
+                msmFreePointer((void**)&name);
+                return 0;              
+            }
+            define = define->next;
+        }
     } 
 
     // no need to check if developer allowed other packages to request/permit this domain
@@ -862,68 +875,81 @@ static int msmCheckDomainRequestOrPermit(manifest_x *mfx, const char* domain)
     }
 }
 
-int msmSetupDefine(struct smack_accesses *smack_accesses, manifest_x *mfx)
+int msmSetupDefines(struct smack_accesses *smack_accesses, manifest_x *mfx)
 {
     d_request_x *d_request;
+    define_x *define;
     d_permit_x *d_permit;
     ac_domain_x * defined_ac_domain = NULL;
     int ret;
 
-    if ( (!mfx) || (!mfx->define) || (!mfx->define->name)) {
-       rpmlog(RPMLOG_ERR, "Failed to setup define with empty name\n");
-       return -1;
+    if ( (!mfx) || (!mfx->defines)) {
+        rpmlog(RPMLOG_ERR, "Failed to setup define\n");
+        return -1;
     }
 
-    /* need to check if domain hasn't been already defined by other package */
+    LISTHEAD(mfx->defines, define);
 
-    HASH_FIND(hh, all_ac_domains, mfx->define->name, strlen(mfx->define->name), defined_ac_domain);
-    if ((defined_ac_domain) && (defined_ac_domain->pkg_name)) { // this domain has been previously defined
-        if (strcmp(defined_ac_domain->pkg_name, mfx->name) != 0) {
-            rpmlog(RPMLOG_ERR, "Attempt to define a domain name %s that has been already defined by package %s\n",
-                   mfx->define->name, defined_ac_domain->pkg_name);
+    while (define) {
+        define_x *next = define->next;
+        if (!define->name) {
+            rpmlog(RPMLOG_ERR, "Attempt to define a domain with empty name. Abort\n");
             return -1;
         }
-    }
+        /* need to check if domain hasn't been already defined by other package */
 
-    if (mfx->define->d_requests) {
-        for (d_request = mfx->define->d_requests; d_request; d_request = d_request->prev) {
-            // first check if the current's package sw source can grant access to requested domain
-            if (msmCheckDomainRequestOrPermit(mfx, d_request->label_name) < 0) {
-#ifdef ENABLE_DCHECKS
+        HASH_FIND(hh, all_ac_domains, define->name, strlen(define->name), defined_ac_domain);
+        if ((defined_ac_domain) && (defined_ac_domain->pkg_name)) { // this domain has been previously defined
+            if (strcmp(defined_ac_domain->pkg_name, mfx->name) != 0) {
+                rpmlog(RPMLOG_ERR, "Attempt to define a domain name %s that has been already defined by package %s\n",
+                       define->name, defined_ac_domain->pkg_name);
                 return -1;
-#endif
             }
-            if (smack_accesses_add(smack_accesses, mfx->define->name, d_request->label_name, d_request->ac_type) < 0) {
-                rpmlog(RPMLOG_ERR, "Failed to set smack rules for domain requests\n");
-                return -1;
-            }  
         }
-    }
 
-    if (mfx->define->d_permits) {
-        for (d_permit = mfx->define->d_permits; d_permit; d_permit = d_permit->prev) {
-            // first check if the current's package sw source can grant access to permited domain
-            if (msmCheckDomainRequestOrPermit(mfx, d_permit->label_name) < 0) {
+        if (define->d_requests) {
+            for (d_request = define->d_requests; d_request; d_request = d_request->prev) {
+                // first check if the current's package sw source can grant access to requested domain
+                if (msmCheckDomainRequestOrPermit(mfx, d_request->label_name) < 0) {
 #ifdef ENABLE_DCHECKS
-                return -1;
+                    return -1;
 #endif
+                }
+                if (smack_accesses_add(smack_accesses, define->name, d_request->label_name, d_request->ac_type) < 0) {
+                    rpmlog(RPMLOG_ERR, "Failed to set smack rules for domain requests\n");
+                    return -1;
+                }    
             }
-            if (!d_permit->to_label_name)
-                ret = smack_accesses_add(smack_accesses, d_permit->label_name, mfx->define->name, d_permit->ac_type);
-            else {
-                if (msmCheckLabelProvisioning(mfx, d_permit->to_label_name) < 0) {
+        }
+
+        if (define->d_permits) {
+            for (d_permit = define->d_permits; d_permit; d_permit = d_permit->prev) {
+                // first check if the current's package sw source can grant access to permited domain
+                if (msmCheckDomainRequestOrPermit(mfx, d_permit->label_name) < 0) {
 #ifdef ENABLE_DCHECKS
                     return -1;
 #endif
                 }
-                ret = smack_accesses_add(smack_accesses, d_permit->label_name, d_permit->to_label_name, d_permit->ac_type);
+                if (!d_permit->to_label_name)
+                    ret = smack_accesses_add(smack_accesses, d_permit->label_name, define->name, d_permit->ac_type);
+                else {
+                    if (msmCheckLabelProvisioning(mfx, d_permit->to_label_name) < 0) {
+#ifdef ENABLE_DCHECKS
+                        return -1;
+#endif
+                    }
+                    ret = smack_accesses_add(smack_accesses, d_permit->label_name, d_permit->to_label_name, d_permit->ac_type);
+                }
+                if (ret < 0) {
+                    rpmlog(RPMLOG_ERR, "Failed to set smack rules for domain permits\n");
+                    return -1;
+                }    
             }
-            if (ret < 0) {
-                rpmlog(RPMLOG_ERR, "Failed to set smack rules for domain permits\n");
-                return -1;
-            }  
-        }
-    } 
+        } 
+
+        define = next;
+    }
+
     return 0;
 }
 
@@ -1019,9 +1045,8 @@ int msmSetupPackages(struct smack_accesses *smack_accesses, package_x *packages,
     char *p_rankkey, *c_rankkey; 
     for (package = packages; package; package = package->prev) {
        package_x *current_p;
-            rpmlog(RPMLOG_DEBUG, "before HASH_FIND, package->name %s\n", package->name);
+       rpmlog(RPMLOG_DEBUG, "before HASH_FIND, package->name %s\n", package->name);
        HASH_FIND(hh, allpackages, package->name, strlen(package->name), current_p);
-            rpmlog(RPMLOG_DEBUG, "after HASH_FIND\n");
        if (current_p) {
            if (!current_p->sw_source) {
                return -1;
@@ -1203,13 +1228,12 @@ int msmSetFileXAttributes(manifest_x *mfx, const char* filepath, magic_t cookie)
     if (exec_label) {
         execLabeldefined = 1;
         if ((strcmp(exec_label, "none") == 0) 
-            || ( (mfx->request) && (mfx->request->ac_domain) && (strcmp(exec_label, mfx->request->ac_domain) == 0))
-            || ( (mfx->define) &&  (mfx->define->name) && (strcmp(exec_label, mfx->define->name) == 0))) {
+            || ( (mfx->request) && (mfx->request->ac_domain) && (strcmp(exec_label, mfx->request->ac_domain) == 0))) {
             // these labels are allowed
         } else {
             // ignore all other exec labels, because they aren't allowed for security reasons
             exec_label = NULL;
-            rpmlog(RPMLOG_DEBUG, "It isn't allowed to label the file with smack64label other than ac domain or \"none\" value\n");
+            rpmlog(RPMLOG_DEBUG, "It isn't allowed to label the file with smack64label other than requested ac domain or \"none\" value\n");
             rpmlog(RPMLOG_DEBUG, "The default ac domain label will be used instead\n");
         }
     }  
@@ -1225,18 +1249,8 @@ int msmSetFileXAttributes(manifest_x *mfx, const char* filepath, magic_t cookie)
                 if (!label) label = isolatedLabel;
                 if (!exec_label) exec_label = isolatedLabel;
             }
-        } else if (mfx->define) { // AC domain defined in manifest
-            if (mfx->define->name) {
-                if (!label) label = mfx->define->name;
-                if (!exec_label) exec_label = mfx->define->name;
-            } else {
-                rpmlog(RPMLOG_DEBUG, "Define for AC domain is empty. Can't identify default file label\n");
-                rpmlog(RPMLOG_DEBUG, "File will be labelled with the label \"Isolated\"\n");
-                if (!label) label = isolatedLabel;
-                if (!exec_label) exec_label = isolatedLabel;
-            }           
-        } else { // no request or definition of domain
-            rpmlog(RPMLOG_DEBUG, "Both define and request sections are empty. Can't identify default file label\n");
+        } else { // no request of domain
+            rpmlog(RPMLOG_DEBUG, "The request section is missing. Can't identify default file label\n");
             rpmlog(RPMLOG_DEBUG, "File will be labelled with the label \"Isolated\"\n");
             if (!label) label = isolatedLabel;
             if (!exec_label) exec_label = isolatedLabel;
@@ -1287,7 +1301,7 @@ void msmRemoveRules(struct smack_accesses *smack_accesses, manifest_x *mfx, int
     if (!package)
        return;
 
-    if ((mfx->define) || (mfx->sw_sources)) {
+    if ((mfx->defines) || (mfx->sw_sources)) {
         /* remove smack rule file and rule set from kernel */
         rpmlog(RPMLOG_DEBUG, "removing smack rules for %s\n", mfx->name);
         msmSetupSmackRules(smack_accesses, mfx->name, SMACK_UNINSTALL, SmackEnabled);