Adding check “E-011” to check for multiple specification of the same …

…entry which setfiles

would report as an error during the policy build. Even if those two entries were identical,
setfiles would still report that as an error during the policy build. For instance,
'/foo – foo_context' and '/foo – foo_context' would be considered multiple specification
for the same entry just like '/foo – foo_context' and '/foo – bar_context' would be
considered multiple specification. This check helps detecting that kind of problem by
parsing all the fc entries and looking for multiple specifications across all other file
contexts instead of just comparing entries within each file since it is possible to have
multiple specifications across file contexts and that would also cause issues.

All findings are returned as selint errors with the file context names and line numbers
involve in the result of the violation.

src/check_hooks.h

@@ -68,6 +68,7 @@ enum error_ids {
 	E_ID_UNKNOWN_CLASS     = 8,
 	E_ID_EMPTY_BLOCK       = 9,
 	E_ID_STRAY_WORD        = 10,
+	E_ID_FC_DUPLICATE_ENTRY = 11,
 	E_END
 };
 

src/fc_checks.c

@@ -232,3 +232,178 @@ struct check_result *check_file_context_types_exist(__attribute__((unused)) cons
 
 	return NULL;
 }
+
+/**********************************
+ * Return true if the two fc_entry nodes are the same
+ * and false otherwise
+ **********************************/
+static bool is_same_fc_entry(const struct fc_entry *entry_one,
+                             const struct fc_entry *entry_two)
+{
+	return !strcmp(entry_one->path, entry_two->path)
+		&& ((!entry_one->context && !entry_two->context)	//when <<none>>
+			|| (entry_one->obj == entry_two->obj
+				&& !strcmp(entry_one->path, entry_two->path)
+				&& !strcmp(entry_one->context->range,
+				           entry_two->context->range)
+				&& !strcmp(entry_one->context->role,
+				           entry_two->context->role)
+				&& !strcmp(entry_one->context->type,
+				           entry_two->context->type)
+				&& !strcmp(entry_one->context->user,
+				           entry_two->context->user)));
+}
+
+/**********************************
+ * Return true if an entry has multiple specification
+ * and false otherwise
+ **********************************/
+static bool is_multiple_fc_entry_spec(const struct fc_entry *entry_one,
+                                      const struct fc_entry *entry_two)
+{
+
+	if (!(entry_two->context && entry_one->context)) {
+		return false;
+	} else {
+		return !strcmp(entry_one->path, entry_two->path)
+			&& entry_one->obj == entry_two->obj
+			&& !strcmp(entry_one->path, entry_two->path)
+			&& (strcmp(entry_one->context->range, entry_two->context->range)
+				|| strcmp(entry_one->context->role,
+				          entry_two->context->role)
+				|| strcmp(entry_one->context->type,
+				          entry_two->context->type)
+				|| strcmp(entry_one->context->user,
+				          entry_two->context->user));
+	}
+}
+
+/**********************************
+ * Return true if the two duplicates or multiple
+ * specification would cause problems and false otherwise
+ **********************************/
+static bool is_problematic(const struct fc_entry *entry_one,
+                           const struct fc_entry *entry_two)
+{
+
+	/************************************************************************************
+	 * The ideal solution here would be to evaluate ifdef/ifndef parameters, but that   *
+	 * seems to be a bit more complicated. We are using this function as an alternate   *
+	 * solution to help us determine when multiple specifications of an entry would     *
+	 * break the build. To start with we make the assumption that ifdef parameters are  *
+	 * mutually exclusive, even though in theory they are not, in practice you should   *
+	 * never have distro_redhat and distro_gentoo, for instance both defined. contrary  *
+	 * to that, ifndef is not mutually exclusive both in theory and practice so that    *
+	 * assumption is made here as well. Those are the condition under which this        *
+	 * function will return true:                                                       *
+	 *                                                                                  *
+	 * 1)Two duplicates whereby none of them is within any ifdef/ifndef.                *
+	 *                                                                                  *
+	 * 2)Two duplicates whereby one of them is within an ifdef/ifndef and the other     *
+	 *   one is not within any.                                                         *
+	 *                                                                                  *
+	 * 3)Two duplicates whereby both are within the same ifdef/ifndef. By both          *
+	 *   here I'm not referring to literally be defined under the same conditional      *
+	 *   although it would also return true in this case, rather I'm referring to the   *
+	 *   parameters or conditions being the same. So if you had the same specification  *
+	 *   under distro_redhat in two different files, it would treat them as being under *
+	 *   the same conditional and return true.                                          *
+	 *                                                                                  *
+	 * 4)Two duplicates whereby one is within an ifdef, the other within an ifndef      *
+	 *   and the conditions differ i.e ifdef(`distro_gentoo',` and                      *
+	 *   ifndef(`distro_redhat',`                                                       *
+	 *                                                                                  *
+	 * 5)Two duplicates whereby both are within ifndef and the conditions differ i.e    *
+	 *   ifndef(`distro_gentoo',` and ifndef(`distro_redhat',`                          *
+	 *                                                                                  *
+	 * For any other conditions, it returns false                                       *
+	 *                                                                                  *
+	 * For Future improvement we can consider evaluating ifdef/ifndef instead.          *
+	 * The conditional_data which is a field from fc_entry contains a boolean field     *
+	 * called 'state' specifically to handle that for future improvement. Currently     *
+	 * this field is not used it's just a place holder.                                 *
+	 ************************************************************************************/
+
+	if (!entry_one->conditional || !entry_two->conditional)
+	{
+		return true;
+	}
+	else if (entry_one->conditional && entry_two->conditional)
+	{
+		if (entry_one->conditional->flavor == entry_two->conditional->flavor
+			&& (!strcmp(entry_one->conditional->condition,
+				entry_two->conditional->condition)
+				|| (entry_one->conditional->flavor == CONDITION_IFNDEF
+					&& strcmp(entry_one->conditional->condition,
+					          entry_two->conditional->condition)))) {
+			return true;
+		} else if (entry_one->conditional->flavor
+			!= entry_two->conditional->flavor
+			&& strcmp(entry_one->conditional->condition,
+			          entry_two->conditional->condition)) {
+			return true;
+		}
+	}
+	return false;
+}
+
+/**********************************
+ * Return true if the node we are currently processing
+ * is the one that is already registered in the fc_entry_map
+ * else false
+ **********************************/
+static bool is_the_registered_entry(const struct fc_entry_map_info *registered,
+                                    unsigned int lineno, char *filename)
+{
+
+	return !strcmp(registered->file_name, filename)
+			&& registered->lineno == lineno;
+}
+
+struct check_result* check_file_contexts_duplicate_entry(const struct check_data *data,
+                                                         const struct policy_node *node)
+{
+	if (node->flavor == NODE_FC_ENTRY) {
+		struct fc_entry_map_info *out = look_up_in_fcs_entry_map(
+				node->data.fc_data->path);
+		struct check_result *ret = NULL;
+
+		if(!out){
+			return(alloc_internal_error("Error while parsing file context entry"));
+		}
+
+		if (!is_the_registered_entry(out, node->lineno, data->filename)) {
+			if (is_same_fc_entry(out->entry, node->data.fc_data)
+					&& is_problematic(out->entry, node->data.fc_data)) {
+
+				if (!strcmp(out->file_name, data->filename)) {
+					ret = make_check_result('E', E_ID_FC_DUPLICATE_ENTRY,
+					                        "Duplicate entry at line (%u) and (%u) for entry \"%s\"",
+					                        out->lineno, node->lineno,
+					                        node->data.fc_data->path);
+				} else {
+					ret = make_check_result('E', E_ID_FC_DUPLICATE_ENTRY,
+					                        "Duplicate entry at line (%u) and line (%u) from (%s) for entry \"%s\"",
+					                        node->lineno, out->lineno, out->file_name,
+					                        node->data.fc_data->path);
+				}
+				return ret;
+			} else if (is_multiple_fc_entry_spec(out->entry, node->data.fc_data)
+					&& is_problematic(out->entry, node->data.fc_data)) {
+				if (!strcmp(out->file_name, data->filename)) {
+					ret = make_check_result('E', E_ID_FC_DUPLICATE_ENTRY,
+					                        "Multiple specification at line (%u) and (%u) for entry \"%s\"",
+					                        out->lineno, node->lineno,
+					                        node->data.fc_data->path);
+				} else {
+					ret = make_check_result('E', E_ID_FC_DUPLICATE_ENTRY,
+					                        "Multiple Specification at line (%u) and line (%u) from (%s) for entry \"%s\"",
+					                        node->lineno, out->lineno, out->file_name,
+					                        node->data.fc_data->path);
+				}
+				return ret;
+			}
+		}
+	}
+	return NULL;
+}

src/fc_checks.h

@@ -91,4 +91,16 @@ struct check_result *check_file_context_types_exist(const struct check_data
                                                     const struct policy_node
                                                     *node);
 
+/*********************************************
+* Check for duplicate entry or multiple specification
+* across file contexts.
+* Called on NODE_FC_ENTRY nodes.
+* node - the node to check
+* returns NULL if passed or check_result for issue E-011
+*********************************************/
+struct check_result *check_file_contexts_duplicate_entry(const struct check_data
+                                                    *data,
+                                                    const struct policy_node
+                                                    *node);
+
 #endif

src/runner.c

@@ -281,7 +281,10 @@ struct checks *register_checks(char level,
 		}
 		if (CHECK_ENABLED("E-010")) {
 			add_check(NODE_M4_SIMPLE_MACRO, ck, "E-010",
-				  check_stray_word);
+			          check_stray_word);
+		}
+		if (CHECK_ENABLED("E-011")) {
+			add_check(NODE_FC_ENTRY, ck, "E-011", check_file_contexts_duplicate_entry);
 		}
 	case 'F':
 		break;