Feature/file context duplicate entry check

src/check_hooks.c

@@ -294,7 +294,9 @@ void display_check_issue_counts(const struct checks *ck)
 
 void free_check_result(struct check_result *res)
 {
-	free(res->message);
+	if (res) {
+		free(res->message);
+	}
 	free(res);
 }
 

src/check_hooks.h

@@ -69,6 +69,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. The term both     *
+	 *   here does not mean to literally be defined under the same conditional           *
+	 *   although it would also return true in this case, instead it is 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_fc_entries_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/maps.c

@@ -27,6 +27,7 @@ static struct hash_elem *perm_map = NULL;
 static struct hash_elem *mods_map = NULL;
 static struct hash_elem *mod_layers_map = NULL;
 static struct hash_elem *ifs_map = NULL;
+static struct fc_entry_hash *fc_entries_map = NULL;
 static struct bool_hash_elem *transform_map = NULL;
 static struct bool_hash_elem *filetrans_map = NULL;
 static struct bool_hash_elem *role_if_map = NULL;
@@ -77,6 +78,22 @@ static struct hash_elem *look_up_hash_elem(const char *name, enum decl_flavor fl
 	return decl;
 }
 
+#if defined(__clang__) && defined(__clang_major__) && (__clang_major__ >= 4)
+__attribute__((no_sanitize("unsigned-integer-overflow")))
+#endif
+static struct fc_entry_hash *look_up_fcs_entry_hash(const char *path)
+{
+
+	if (!path) {
+		return NULL;
+	}
+
+	struct fc_entry_hash *out;
+	HASH_FIND(hh_fc_entry, fc_entries_map, path, strlen(path), out);
+
+	return out;
+}
+
 #if defined(__clang__) && defined(__clang_major__) && (__clang_major__ >= 4)
 __attribute__((no_sanitize("unsigned-integer-overflow")))
 #endif
@@ -146,6 +163,21 @@ const char *look_up_in_decl_map(const char *name, enum decl_flavor flavor)
 	}
 }
 
+#if defined(__clang__) && defined(__clang_major__) && (__clang_major__ >= 4)
+__attribute__((no_sanitize("unsigned-integer-overflow")))
+#endif
+struct fc_entry_map_info *look_up_in_fc_entries_map(const char *path)
+{
+
+	struct fc_entry_hash *out = look_up_fcs_entry_hash(path);
+
+	if (!out) {
+		return NULL;
+	} else {
+			return out->val;
+	}
+}
+
 #if defined(__clang__) && defined(__clang_major__) && (__clang_major__ >= 4)
 __attribute__((no_sanitize("unsigned-integer-overflow")))
 #endif
@@ -437,6 +469,21 @@ static void insert_into_template_map(const char *name, void *new_node,
 	insertion_func(template, new_node);
 }
 
+#if defined(__clang__) && defined(__clang_major__) && (__clang_major__ >= 4)
+__attribute__((no_sanitize("unsigned-integer-overflow")))
+#endif
+void insert_into_fc_entries_map(struct fc_entry_map_info *info)
+{
+	struct fc_entry_hash *entry = look_up_fcs_entry_hash(info->entry->path);
+
+	if (!entry) {// Item not in hash table already
+		entry = malloc(sizeof(struct fc_entry_hash));
+		entry->key = strdup(info->entry->path);
+		entry->val = info;
+		HASH_ADD_KEYPTR(hh_fc_entry, fc_entries_map, entry->key, strlen(entry->key), entry);
+	}
+}
+
 void insert_template_into_template_map(const char *name)
 {
 	insert_into_template_map(name, NULL, insert_noop);
@@ -569,6 +616,19 @@ unsigned int permmacros_map_count()
 		free(cur_bool); \
 } \
 
+void free_fc_entry_map_info(struct fc_entry_map_info *to_free)
+{
+	//we do not call free_fc_entry(to_free->entry) here
+	//because fc_entry_map_info shares the memory address
+	// of the fc_entry structs which the free_fc_entry
+	//itself frees.
+
+	if (to_free) {
+		free(to_free->file_name);
+	}
+	free(to_free);
+}
+
 void free_all_maps()
 {
 
@@ -622,4 +682,13 @@ void free_all_maps()
 		free_if_call_list(cur_template->calls);
 		free(cur_template);
 	}
+
+	struct fc_entry_hash *cur_elm, *tmp_elm;
+
+	HASH_ITER(hh_fc_entry, fc_entries_map, cur_elm, tmp_elm) {
+		HASH_DELETE(hh_fc_entry, fc_entries_map, cur_elm);
+		free(cur_elm->key);
+		free_fc_entry_map_info(cur_elm->val);
+		free(cur_elm);
+	}
 }

src/maps.h

@@ -48,6 +48,20 @@ struct template_hash_elem {
 	UT_hash_handle hh;
 };
 
+struct fc_entry_map_info {
+	struct fc_entry *entry;
+	unsigned int lineno;
+	char *file_name;
+};
+
+struct fc_entry_hash {
+	char *key;
+	struct fc_entry_map_info *val;
+	UT_hash_handle hh_fc_entry;
+};
+
+
+
 void insert_into_decl_map(const char *name, const char *module_name,
                           enum decl_flavor flavor);
 
@@ -65,6 +79,10 @@ void insert_into_ifs_map(const char *if_name, const char *mod_name);
 
 const char *look_up_in_ifs_map(const char *if_name);
 
+void insert_into_fc_entries_map(struct fc_entry_map_info *info);
+
+struct fc_entry_map_info *look_up_in_fc_entries_map(const char *path);
+
 void mark_transform_if(const char *if_name);
 
 int is_transform_if(const char *if_name);
@@ -103,6 +121,8 @@ unsigned int permmacros_map_count(void);
 
 unsigned int decl_map_count(enum decl_flavor flavor);
 
+void free_fc_entry_map_info(struct fc_entry_map_info *to_free);
+
 void free_all_maps(void);
 
 #endif