RFC: Expand checks about requirements to userspace classes

.gitignore

@@ -32,6 +32,7 @@ tests/check_startup
 tests/check_te_checks
 tests/check_ordering
 tests/check_perm_macro
+tests/check_name_list
 tests/functional/policies/parse_errors/test3_tmp.if
 tests/functional/policies/parse_errors/test5_tmp.te
 tests/functional/policies/parse_errors/test6_tmp.if

README

@@ -175,9 +175,9 @@ CHECK IDS
 	S-009: Permission macro suffix does not match class name
 	S-010: Permission macro usage suggested
 
-	W-001: Type or attribute referenced without explicit declaration
-	W-002: Type, attribute or role used but not listed in require block in interface
-	W-003: Unused type, attribute or role listed in require block
+	W-001: Type, attribute or userspace class referenced without explicit declaration
+	W-002: Type, attribute, role or userspace class used but not listed in require block in interface
+	W-003: Unused type, attribute, role or userspace class listed in require block
 	W-004: Potentially unescaped regex character in file contexts paths
 	W-005: Interface call from module not in optional_policy block
 	W-006: Interface call with empty argument
@@ -201,3 +201,22 @@ CHECK IDS
 
 	F-001: Policy syntax error prevents further processing
 	F-002: Internal error in SELint
+
+REFERENCE POLICY CONVENTIONS
+
+	To improve the accuracy and avoid false-positives SELint makes some assumptions about
+	naming conventions and formatting of the policy:
+
+	* Type identifiers should end with the suffix '_t'.
+	* Role identifiers should end with the suffix '_r'.
+	* Names of noop interfaces for availability checks should end with the suffix '_stub'.
+	* Permission macros should end with the suffix '_perms'.
+	* Class set macros should end with the suffix '_class_set'.
+	* Security class declarations of userspace classes in the security_classes file should be
+	  declared with a comment including the word 'userspace'.
+	* Interfaces that wrap a file based type-transition should end with the suffix '_filetrans'.
+	* Interfaces that transforms their arguments, e.g. associate an attribute with them,
+	  and thus should be handled like a declaration should have one of the following common
+	  suffixes: '_type', '_file', '_domain', '_node', '_agent', '_delivery', '_sender',
+	  '_boolean', '_content', '_constrained', '_executable', '_exemption', '_object'
+	  or '_mountpoint'.

src/Makefile.am

@@ -13,7 +13,7 @@
 # limitations under the License.
 
 bin_PROGRAMS = selint
-selint_SOURCES = main.c lex.l parse.y tree.c tree.h selint_error.h parse_functions.c parse_functions.h maps.c maps.h runner.c runner.h parse_fc.c parse_fc.h template.c template.h file_list.c file_list.h check_hooks.c check_hooks.h fc_checks.c fc_checks.h util.c util.h if_checks.c if_checks.h selint_config.c selint_config.h string_list.c string_list.h startup.c startup.h te_checks.c te_checks.h ordering.c ordering.h color.c color.h perm_macro.c perm_macro.h xalloc.h
+selint_SOURCES = main.c lex.l parse.y tree.c tree.h selint_error.h parse_functions.c parse_functions.h maps.c maps.h runner.c runner.h parse_fc.c parse_fc.h template.c template.h file_list.c file_list.h check_hooks.c check_hooks.h fc_checks.c fc_checks.h util.c util.h if_checks.c if_checks.h selint_config.c selint_config.h string_list.c string_list.h startup.c startup.h te_checks.c te_checks.h ordering.c ordering.h color.c color.h perm_macro.c perm_macro.h xalloc.h name_list.c name_list.h
 BUILT_SOURCES = parse.h
 AM_YFLAGS = -d -Wno-yacc -Werror=conflicts-rr -Werror=conflicts-sr
 

src/if_checks.c

@@ -262,7 +262,7 @@ struct check_result *check_name_used_but_not_required_in_if(const struct
 
 	const struct policy_node *cur = node;
 
-	struct string_list *names_in_current_node = get_names_in_node(node);
+	struct name_list *names_in_current_node = get_names_in_node(node);
 
 	if (!names_in_current_node) {
 		return NULL;
@@ -276,15 +276,15 @@ struct check_result *check_name_used_but_not_required_in_if(const struct
 	}
 
 	if (!cur) {
-		free_string_list(names_in_current_node);
+		free_name_list(names_in_current_node);
 		return NULL;
 	}
 	// In a template or interface, and cur is a pointer to the definition node
 
 	cur = cur->first_child;
 
-	struct string_list *names_required = NULL;
-	struct string_list *names_required_tail = NULL;
+	struct name_list *names_required = NULL;
+	struct name_list *names_required_tail = NULL;
 
 	while (cur && cur != node) {
 	       if (cur->flavor == NODE_GEN_REQ
@@ -305,36 +305,34 @@ struct check_result *check_name_used_but_not_required_in_if(const struct
 		                     // for example in an ifdef
 	}
 
-	const struct string_list *name_node = names_in_current_node;
+	const struct name_list *name_node = names_in_current_node;
 	/* In declarations skip the first name, which is the new declared type */
 	if (node->flavor == NODE_DECL) {
 		name_node = name_node->next;
 	}
 	const char *flavor = NULL;
 
 	while (name_node) {
-		if (!str_in_sl(name_node->string, names_required)) {
-			if (0 == strcmp(name_node->string, "system_r")) {
+		const struct name_data *ndata = name_node->data;
+		if (!name_list_contains_name(names_required, ndata)) {
+			if (name_is_role(ndata) && 0 == strcmp(ndata->name, "system_r")) {
 				// system_r is required by default in all modules
 				// so that is an exception that shouldn't be warned
 				// about.
 				name_node = name_node->next;
 				continue;
 			}
-			if (look_up_in_decl_map(name_node->string, DECL_TYPE)) {
+			if (name_is_type(ndata) && look_up_in_decl_map(ndata->name, DECL_TYPE)) {
 				flavor = "Type";
-			} else
-			if (look_up_in_decl_map
-			            (name_node->string, DECL_ATTRIBUTE)) {
+			} else if (name_is_typeattr(ndata) && look_up_in_decl_map(ndata->name, DECL_ATTRIBUTE)) {
 				flavor = "Attribute";
-			} else
-			if (look_up_in_decl_map
-			            (name_node->string, DECL_ATTRIBUTE_ROLE)) {
+			} else if (name_is_roleattr(ndata) && look_up_in_decl_map(ndata->name, DECL_ATTRIBUTE_ROLE)) {
 				flavor = "Role Attribute";
-			} else
-			if (look_up_in_decl_map
-			            (name_node->string, DECL_ROLE)) {
+			} else if (name_is_role(ndata) && look_up_in_decl_map(ndata->name, DECL_ROLE)) {
 				flavor = "Role";
+			} else if (name_is_class(ndata) && look_up_in_decl_map(ndata->name, DECL_CLASS) &&
+			           userspace_class_support && is_userspace_class(ndata->name, ndata->traits)) {
+				flavor = "Class";
 			} else {
 				// This is a string we don't recognize.  Other checks and/or
 				// the compiler catch invalid bare words
@@ -344,16 +342,16 @@ struct check_result *check_name_used_but_not_required_in_if(const struct
 
 			struct check_result *res =
 				make_check_result('W', W_ID_NO_REQ, NOT_REQ_MESSAGE,
-				                  flavor, name_node->string);
-			free_string_list(names_in_current_node);
-			free_string_list(names_required);
+				                  flavor, ndata->name);
+			free_name_list(names_in_current_node);
+			free_name_list(names_required);
 			return res;
 		}
 		name_node = name_node->next;
 	}
 
-	free_string_list(names_in_current_node);
-	free_string_list(names_required);
+	free_name_list(names_in_current_node);
+	free_name_list(names_required);
 
 	return NULL;
 }
@@ -379,6 +377,14 @@ struct check_result *check_name_required_but_not_used_in_if(const struct
 		flavor = "Role Attribute";
 	} else if (dd->flavor == DECL_ROLE) {
 		flavor = "Role";
+	} else if (dd->flavor == DECL_CLASS && userspace_class_support) {
+		flavor = "Class";
+		if (!is_userspace_class(dd->name, dd->attrs)) {
+			return make_check_result('W',
+			                         W_ID_UNUSED_REQ,
+			                         "Class %s is listed in require block but is not a userspace class",
+			                         dd->name);
+		}
 	} else {
 		return NULL;
 	}
@@ -405,7 +411,7 @@ struct check_result *check_name_required_but_not_used_in_if(const struct
 		return NULL;
 	}
 
-	struct string_list *names_to_check = get_names_in_node(node);
+	struct name_list *names_to_check = names_to_check = get_names_in_node(node);
 	if (!names_to_check) {
 		// This should never happen
 		return alloc_internal_error(
@@ -416,22 +422,22 @@ struct check_result *check_name_required_but_not_used_in_if(const struct
 
 	cur = cur->next;
 
-	struct string_list *sl_end = NULL;
-	struct string_list *sl_head = NULL;
+	struct name_list *nl_end = NULL;
+	struct name_list *nl_head = NULL;
 
 	int depth = 0;
 
 	while (cur) {
-		struct string_list *names_used = get_names_in_node(cur);
+		struct name_list *names_used = get_names_in_node(cur);
 		if (names_used) {
-			if (!sl_head) {
-				sl_head = sl_end = names_used;
+			if (!nl_head) {
+				nl_head = nl_end = names_used;
 			} else {
-				sl_end->next = names_used;
+				nl_end->next = names_used;
 			}
 
-			while (sl_end->next) {
-				sl_end = sl_end->next;
+			while (nl_end->next) {
+				nl_end = nl_end->next;
 			}
 		}
 
@@ -452,24 +458,21 @@ struct check_result *check_name_required_but_not_used_in_if(const struct
 		}
 	}
 
-	const struct string_list *name_node = names_to_check;
-
 	struct check_result *res = NULL;
 
-	while (name_node) {
-		if (!str_in_sl(name_node->string, sl_head)) {
+	for (const struct name_list *name_node = names_to_check; name_node; name_node = name_node->next) {
+		if (!name_list_contains_name(nl_head, name_node->data)) {
 			res = make_check_result('W',
 			                        W_ID_UNUSED_REQ,
 			                        "%s %s is listed in require block but not used in interface",
 			                        flavor,
-			                        name_node->string);
+			                        name_node->data->name);
 			break;
 		}
-		name_node = name_node->next;
 	}
 
-	free_string_list(sl_head);
-	free_string_list(names_to_check);
+	free_name_list(nl_head);
+	free_name_list(names_to_check);
 	return res;
 }
 

src/main.c

@@ -411,6 +411,7 @@ int main(int argc, char **argv)
 	char *obj_perm_sets_path = NULL;
 	char *access_vector_path = NULL;
 	struct string_list *global_cond_files = NULL;
+	char *security_classes_path = NULL;
 
 	while (file) {
 		const char *suffix = (file->fts_pathlen > 3) ? (file->fts_path + file->fts_pathlen - 3) : NULL;
@@ -462,6 +463,10 @@ int main(int argc, char **argv)
 		           && (!strcmp(file->fts_name, "global_booleans") || !strcmp(file->fts_name, "global_tunables"))) {
 			// TODO: Make names configurable
 			global_cond_files = concat_string_lists(global_cond_files, sl_from_str(file->fts_path));
+		} else if (source_flag
+			   && !strcmp(file->fts_name, "security_classes")) {
+			// TODO: Make security_classes name configurable
+			security_classes_path = strdup(file->fts_path);
 		} else {
 			// Directories might get traversed twice: preorder and final visit.
 			// Print only the final visit
@@ -533,6 +538,10 @@ int main(int argc, char **argv)
 			           && !str_in_sl(file->fts_path, global_cond_files)
 			           && (0 == strcmp(file->fts_name, "global_booleans") || 0 == strcmp(file->fts_name, "global_tunables"))) {
 				global_cond_files = concat_string_lists(global_cond_files, sl_from_str(file->fts_path));
+			} else if (source_flag
+			           && !security_classes_path
+			           && 0 == strcmp(file->fts_name, "security_classes")) {
+				security_classes_path = strdup(file->fts_path);
 			}
 			file = fts_read(ftsp);
 		}
@@ -557,6 +566,18 @@ int main(int argc, char **argv)
 			printf("%sWarning%s: Failed to locate access_vectors file.\n", color_warning(), color_reset());
 		}
 
+		if (security_classes_path) {
+			enum selint_error res = load_security_classes_source(security_classes_path);
+			if (res != SELINT_SUCCESS) {
+				printf("%sWarning%s: Failed to parse security_classes from %s: %d\n", color_warning(), color_reset(), security_classes_path, res);
+			} else {
+				print_if_verbose("Loaded security classes from %s\n", security_classes_path);
+				userspace_class_support = 1;
+			}
+		} else {
+			printf("%sWarning%s: Failed to locate security_classes file.\n", color_warning(), color_reset());
+		}
+
 		if (modules_conf_path) {
 			enum selint_error res =
 				load_modules_source(modules_conf_path);
@@ -630,13 +651,15 @@ int main(int argc, char **argv)
 		free_file_list(context_if_files);
 		free(obj_perm_sets_path);
 		free(access_vector_path);
+		free(security_classes_path);
 		free(modules_conf_path);
 		free_string_list(global_cond_files);
 		return EX_CONFIG;
 	}
 
 	free(obj_perm_sets_path);
 	free(access_vector_path);
+	free(security_classes_path);
 	free(modules_conf_path);
 	free_string_list(global_cond_files);
 

src/maps.c

@@ -27,6 +27,8 @@
 #define no_sanitize_unsigned_integer_
 #endif
 
+int userspace_class_support = 0;
+
 static struct hash_elem *type_map = NULL;
 static struct hash_elem *role_map = NULL;
 static struct hash_elem *user_map = NULL;
@@ -38,6 +40,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 if_hash_elem *interfaces_map = NULL;
+static struct bool_hash_elem *userspace_class_map = NULL;
 static struct sl_hash_elem *permmacros_map = NULL;
 static struct template_hash_elem *template_map = NULL;
 
@@ -273,6 +276,56 @@ unsigned int decl_map_count(enum decl_flavor flavor)
 	}
 }
 
+no_sanitize_unsigned_integer_
+void mark_userspace_class(const char *class_name)
+{
+	struct bool_hash_elem *userspace_class;
+
+	HASH_FIND(hh_userspace_class, userspace_class_map, class_name, strlen(class_name), userspace_class);
+
+	if (!userspace_class) {
+		userspace_class = malloc(sizeof(struct bool_hash_elem));
+		userspace_class->key = strdup(class_name);
+		userspace_class->val = 1;
+		HASH_ADD_KEYPTR(hh_userspace_class, userspace_class_map, userspace_class->key,
+				strlen(userspace_class->key), userspace_class);
+	} else {
+		userspace_class->val = 1;
+	}
+}
+
+no_sanitize_unsigned_integer_
+int is_userspace_class(const char *class_name, const struct string_list *permissions)
+{
+	struct bool_hash_elem *userspace_class;
+	HASH_FIND(hh_userspace_class, userspace_class_map, class_name, strlen(class_name), userspace_class);
+	if (userspace_class && userspace_class->val == 1) {
+		return 1;
+	}
+
+	// the system class might be used by systemd depending on the permission
+	if (0 != strcmp(class_name, "system")) {
+		return 0;
+	}
+
+	for (const struct string_list *p = permissions; p; p = p->next) {
+		// if permission is not one of the kernel ones
+		// treat system as userspace class
+		if (0 != strcmp(p->string, "ipc_info") &&
+		    0 != strcmp(p->string, "syslog_read") &&
+		    0 != strcmp(p->string, "syslog_mod") &&
+		    0 != strcmp(p->string, "syslog_console") &&
+		    0 != strcmp(p->string, "module_request") &&
+		    0 != strcmp(p->string, "module_load") &&
+		    0 != strcmp(p->string, "*") &&
+		    0 != strcmp(p->string, "~")) {
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
 no_sanitize_unsigned_integer_
 void mark_transform_if(const char *if_name)
 {
@@ -583,6 +636,12 @@ unsigned int permmacros_map_count(void)
 		free(cur_decl); \
 } \
 
+#define FREE_BOOL_MAP(mn) HASH_ITER(hh_ ## mn, mn ## _map, cur_bool, tmp_bool) { \
+		HASH_DELETE(hh_ ## mn, mn ## _map, cur_bool); \
+		free(cur_bool->key); \
+		free(cur_bool); \
+} \
+
 #define FREE_IF_MAP(mn) HASH_ITER(hh_ ## mn, mn ## _map, cur_if, tmp_if) { \
 		HASH_DELETE(hh_ ## mn, mn ## _map, cur_if); \
 		free(cur_if->name); \
@@ -619,6 +678,10 @@ void free_all_maps(void)
 
 	FREE_IF_MAP(interfaces);
 
+	struct bool_hash_elem *cur_bool, *tmp_bool;
+
+	FREE_BOOL_MAP(userspace_class);
+
 	struct sl_hash_elem *cur_sl, *tmp_sl;
 
 	HASH_ITER(hh_permmacros, permmacros_map, cur_sl, tmp_sl) {