Merge changes from topic 'checkseapp-fixups'

* changes:
  checkseapp: remove .data = NULL assignments
  checkseapp: remove data types form static map
  checkseapp: generalize input validation
  checkseapp: update error message output
  checkseapp: declare internal function as static

1 files changed, 85 insertions, 72 deletions

diff --git a/tools/check_seapp.c b/tools/check_seapp.c
index 5a03b7f..69db388 100644
--- a/tools/check_seapp.c
+++ b/tools/check_seapp.c
@@ -80,14 +80,6 @@ enum key_dir {
 	dir_in, dir_out
 };
 
-/**
- * The expected "type" of data the value in the key
- * value pair should be.
- */
-enum data_type {
-	dt_bool, dt_string
-};
-
 struct list_element {
 	list_element *next;
 };
@@ -110,9 +102,9 @@ struct key_map_regex {
 struct key_map {
 	char *name;
 	key_dir dir;
-	data_type type;
 	char *data;
 	key_map_regex regex;
+	bool (*fn_validate)(char *value, char **errmsg);
 };
 
 /**
@@ -197,25 +189,31 @@ static list line_order_list = list_init(line_order_list_freefn);
  */
 static list nallow_list = list_init(line_order_list_freefn);
 
+/* validation call backs */
+static bool validate_bool(char *value, char **errmsg);
+static bool validate_levelFrom(char *value, char **errmsg);
+static bool validate_selinux_type(char *value, char **errmsg);
+static bool validate_selinux_level(char *value, char **errmsg);
+
 /**
  * The heart of the mapping process, this must be updated if a new key value pair is added
  * to a rule.
  */
 key_map rules[] = {
                 /*Inputs*/
-                { .name = "isSystemServer", .type = dt_bool,   .dir = dir_in,  .data = NULL },
-                { .name = "isOwner",        .type = dt_bool,   .dir = dir_in,  .data = NULL },
-                { .name = "user",           .type = dt_string, .dir = dir_in,  .data = NULL },
-                { .name = "seinfo",         .type = dt_string, .dir = dir_in,  .data = NULL },
-                { .name = "name",           .type = dt_string, .dir = dir_in,  .data = NULL },
-                { .name = "path",           .type = dt_string, .dir = dir_in,  .data = NULL },
-                { .name = "isPrivApp",      .type = dt_bool,   .dir = dir_in,  .data = NULL },
+                { .name = "isSystemServer", .dir = dir_in, .fn_validate = validate_bool },
+                { .name = "isOwner",        .dir = dir_in, .fn_validate = validate_bool },
+                { .name = "user",           .dir = dir_in,                              },
+                { .name = "seinfo",         .dir = dir_in,                              },
+                { .name = "name",           .dir = dir_in,                              },
+                { .name = "path",           .dir = dir_in,                              },
+                { .name = "isPrivApp",      .dir = dir_in, .fn_validate = validate_bool },
                 /*Outputs*/
-                { .name = "domain",         .type = dt_string, .dir = dir_out, .data = NULL },
-                { .name = "type",           .type = dt_string, .dir = dir_out, .data = NULL },
-                { .name = "levelFromUid",   .type = dt_bool,   .dir = dir_out, .data = NULL },
-                { .name = "levelFrom",      .type = dt_string,   .dir = dir_out, .data = NULL },
-                { .name = "level",          .type = dt_string, .dir = dir_out, .data = NULL },
+                { .name = "domain",         .dir = dir_out, .fn_validate = validate_selinux_type  },
+                { .name = "type",           .dir = dir_out, .fn_validate = validate_selinux_type  },
+                { .name = "levelFromUid",   .dir = dir_out, .fn_validate = validate_bool          },
+                { .name = "levelFrom",      .dir = dir_out, .fn_validate = validate_levelFrom     },
+                { .name = "level",          .dir = dir_out, .fn_validate = validate_selinux_level },
 };
 
 /**
@@ -305,7 +303,7 @@ log_msg(FILE *out, const char *prefix, const char *fmt, ...) {
  * 	statically to this executable and LINK_SEPOL_STATIC is not
  * 	defined.
  */
-int check_type(sepol_policydb_t *db, char *type) {
+static int check_type(sepol_policydb_t *db, char *type) {
 
 	int rc = 1;
 #if defined(LINK_SEPOL_STATIC)
@@ -352,6 +350,63 @@ static bool compile_regex(key_map *km, const char **errbuf, int *erroff) {
 	return true;
 }
 
+static bool validate_bool(char *value, char **errmsg) {
+
+	if (!strcmp("true", value) || !strcmp("false", value)) {
+		return true;
+	}
+
+	*errmsg = "Expecting \"true\" or \"false\"";
+	return false;
+}
+
+static bool validate_levelFrom(char *value, char **errmsg) {
+
+	if(strcasecmp(value, "none") && strcasecmp(value, "all") &&
+		strcasecmp(value, "app") && strcasecmp(value, "user")) {
+		*errmsg = "Expecting one of: \"none\", \"all\", \"app\" or \"user\"";
+		return false;
+	}
+	return true;
+}
+
+static bool validate_selinux_type(char *value, char **errmsg) {
+
+	/*
+	 * No policy file present means we cannot check
+	 * SE Linux types
+	 */
+	if (!pol.policy_file) {
+		return true;
+	}
+
+	if(!check_type(pol.db, value)) {
+		*errmsg = "Expecting a valid SELinux type";
+		return false;
+	}
+
+	return true;
+}
+
+static bool validate_selinux_level(char *value, char **errmsg) {
+
+	/*
+	 * No policy file present means we cannot check
+	 * SE Linux MLS
+	 */
+	if (!pol.policy_file) {
+		return true;
+	}
+
+	int ret = sepol_mls_check(pol.handle, pol.db, value);
+	if (ret < 0) {
+		*errmsg = "Expecting a valid SELinux MLS value";
+		return false;
+	}
+
+	return true;
+}
+
 /**
  * Validates a key_map against a set of enforcement rules, this
  * function exits the application on a type that cannot be properly
@@ -370,10 +425,9 @@ static bool key_map_validate(key_map *m, const char *filename, int lineno,
 	int erroff;
 	const char *errbuf;
 	bool rc = true;
-	int ret = 1;
 	char *key = m->name;
 	char *value = m->data;
-	data_type type = m->type;
+	char *errmsg = NULL;
 
 	log_info("Validating %s=%s\n", key, value);
 
@@ -392,51 +446,13 @@ static bool key_map_validate(key_map *m, const char *filename, int lineno,
 		goto out;
 	}
 
-	/* Booleans can always be checked for sanity */
-	if (type == dt_bool && (!strcmp("true", value) || !strcmp("false", value))) {
-		goto out;
-	}
-	else if (type == dt_bool) {
-		log_error("Expected boolean value got: %s=%s on line: %d in file: %s\n",
-				key, value, lineno, filename);
-		rc = false;
-		goto out;
-	}
+	/* If the key has a validation routine, call it */
+	if (m->fn_validate) {
+		rc = m->fn_validate(value, &errmsg);
 
-	if (!strcasecmp(key, "levelFrom") &&
-	    (strcasecmp(value, "none") && strcasecmp(value, "all") &&
-	     strcasecmp(value, "app") && strcasecmp(value, "user"))) {
-		log_error("Unknown levelFrom=%s on line: %d in file: %s\n",
-			  value, lineno, filename);
-		rc = false;
-		goto out;
-	}
-
-	/*
-	 * If there is no policy file present,
-	 * then it is not going to enforce the types against the policy so just return.
-	 * User and name cannot really be checked.
-	 */
-	if (!pol.policy_file) {
-		goto out;
-	}
-	else if (!strcasecmp(key, "type") || !strcasecmp(key, "domain")) {
-
-		if(!check_type(pol.db, value)) {
-			log_error("Could not find selinux type \"%s\" on line: %d in file: %s\n", value,
-			lineno, filename);
-			rc = false;
-		}
-		goto out;
-	}
-	else if (!strcasecmp(key, "level")) {
-
-		ret = sepol_mls_check(pol.handle, pol.db, value);
-		if (ret < 0) {
-			log_error("Could not find selinux level \"%s\", on line: %d in file: %s\n", value,
-			lineno, filename);
-			rc = false;
-			goto out;
+		if (!rc) {
+			log_error("Could not validate key \"%s\" for value \"%s\" on line: %d in file: \"%s\": %s\n", key, value,
+			lineno, filename, errmsg);
 		}
 	}
 
@@ -493,9 +509,6 @@ static map_match rule_map_cmp(rule_map *rmA, rule_map *rmB) {
 			mB = &(rmB->m[j]);
 			input_mode = 0;
 
-			if (mA->type != mB->type)
-				continue;
-
 			if (strcmp(mA->name, mB->name))
 				continue;
 
@@ -1095,7 +1108,7 @@ static void parse_file(file_info *in_file) {
 	return;
 
 err:
-	log_error("reading %s, line %zu, name %s, value %s\n",
+	log_error("Reading file: \"%s\" line: %zu name: \"%s\" value: \"%s\"\n",
 		in_file->name, lineno, name, value);
 	if(found_whitespace && name && !strcasecmp(name, "neverallow")) {
 		log_error("perhaps whitespace before neverallow\n");