[apparmor] [PATCH 4/6] parser: stop splitting the namespace from the named transition targets

Tyler Hicks tyhicks at canonical.com
Fri Mar 4 08:16:39 UTC 2016


The parser was splitting up the namespace and profile name from named
transition targets only to rejoin it later when creating the binary
policy. This complicated the changes needed to support the stacking
identifier '&' in named transition targets.

To keep the stacking support simple, this patch keeps the entire named
transition target string intact from initial profile parsing to writing
out the binary.

All of these changes are straightforward except the hunk that removes
the namespace string addition to the vector in the process_dfa_entry()
function. After speaking with John, the kernel has never expected the
namespace to be separated from the profile name.

Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
---
 parser/parser.h        | 13 ++-----------
 parser/parser_merge.c  | 12 ------------
 parser/parser_misc.c   | 32 +++++++++++---------------------
 parser/parser_policy.c | 17 +++--------------
 parser/parser_regex.c  |  5 -----
 parser/parser_yacc.y   | 45 +++++++++++++++++++--------------------------
 6 files changed, 35 insertions(+), 89 deletions(-)

diff --git a/parser/parser.h b/parser/parser.h
index 581a87a..ab55a74 100644
--- a/parser/parser.h
+++ b/parser/parser.h
@@ -66,13 +66,6 @@ struct prefixes {
 	int owner;
 };
 
-
-struct named_transition {
-	int present;
-	char *ns;
-	char *name;
-};
-
 struct cod_pattern {
 	char *regex;		// posix regex
 };
@@ -98,7 +91,6 @@ struct cond_entry_list {
 };
 
 struct cod_entry {
-	char *ns;
 	char *name;
 	union {
 		char *link_name;
@@ -393,10 +385,9 @@ extern int get_rlimit(const char *name);
 extern char *process_var(const char *var);
 extern int parse_mode(const char *mode);
 extern int parse_X_mode(const char *X, int valid, const char *str_mode, int *mode, int fail);
+bool label_contains_ns(const char *label);
 void parse_label(char **ns, char **name, const char *label);
-void parse_named_transition_target(struct named_transition *nt,
-				   const char *target);
-extern struct cod_entry *new_entry(char *ns, char *id, int mode, char *link_id);
+extern struct cod_entry *new_entry(char *id, int mode, char *link_id);
 
 /* returns -1 if value != true or false, otherwise 0 == false, 1 == true */
 extern int str_to_boolean(const char* str);
diff --git a/parser/parser_merge.c b/parser/parser_merge.c
index 06d0b0e..4898218 100644
--- a/parser/parser_merge.c
+++ b/parser/parser_merge.c
@@ -32,18 +32,6 @@ static int file_comp(const void *c1, const void *c2)
 	e2 = (struct cod_entry **)c2;
 	int res = 0;
 
-	//PERROR("strcmp %s %s\n", (*e1)->name, (*e2)->name);
-	if ((*e1)->ns) {
-		if ((*e2)->ns)
-			res = strcmp((*e1)->ns, (*e2)->ns);
-		else
-			return 1;
-	} else if ((*e2)->ns) {
-		return -1;
-	}
-	if (res)
-		return res;
-
 	if ((*e1)->link_name) {
 		if ((*e2)->link_name)
 			res = strcmp((*e1)->link_name, (*e2)->link_name);
diff --git a/parser/parser_misc.c b/parser/parser_misc.c
index d0d8115..aa29675 100644
--- a/parser/parser_misc.c
+++ b/parser/parser_misc.c
@@ -637,6 +637,16 @@ static int _parse_label(char **ns, size_t *ns_len,
 	return 0;
 }
 
+bool label_contains_ns(const char *label)
+{
+	char *ns = NULL;
+	char *name = NULL;
+	size_t ns_len = 0;
+	size_t name_len = 0;
+
+	return _parse_label(&ns, &ns_len, &name, &name_len, label) == 0 && ns;
+}
+
 void parse_label(char **_ns, char **_name, const char *label)
 {
 	char *ns = NULL;
@@ -671,20 +681,7 @@ void parse_label(char **_ns, char **_name, const char *label)
 	}
 }
 
-void parse_named_transition_target(struct named_transition *nt,
-				   const char *target)
-{
-	memset(nt, 0, sizeof(*nt));
-	if (!target) {
-		/* Return with nt->present set to 0 (thanks to the memset) */
-		return;
-	}
-
-	parse_label(&nt->ns, &nt->name, target);
-	nt->present = 1;
-}
-
-struct cod_entry *new_entry(char *ns, char *id, int mode, char *link_id)
+struct cod_entry *new_entry(char *id, int mode, char *link_id)
 {
 	struct cod_entry *entry = NULL;
 
@@ -692,7 +689,6 @@ struct cod_entry *new_entry(char *ns, char *id, int mode, char *link_id)
 	if (!entry)
 		return NULL;
 
-	entry->ns = ns;
 	entry->name = id;
 	entry->link_name = link_id;
 	entry->mode = mode;
@@ -716,7 +712,6 @@ struct cod_entry *copy_cod_entry(struct cod_entry *orig)
 	if (!entry)
 		return NULL;
 
-	DUP_STRING(orig, entry, ns, err);
 	DUP_STRING(orig, entry, name, err);
 	DUP_STRING(orig, entry, link_name, err);
 	DUP_STRING(orig, entry, nt_name, err);
@@ -743,8 +738,6 @@ void free_cod_entries(struct cod_entry *list)
 		return;
 	if (list->next)
 		free_cod_entries(list->next);
-	if (list->ns)
-		free(list->ns);
 	if (list->name)
 		free(list->name);
 	if (list->link_name)
@@ -797,9 +790,6 @@ void debug_cod_entries(struct cod_entry *list)
 		else
 			printf("\tName:\tNULL\n");
 
-		if (item->ns)
-			printf("\tNs:\t(%s)\n", item->ns);
-
 		if (AA_LINK_BITS & item->mode)
 			printf("\tlink:\t(%s)\n", item->link_name ? item->link_name : "/**");
 
diff --git a/parser/parser_policy.c b/parser/parser_policy.c
index 292abff..545f613 100644
--- a/parser/parser_policy.c
+++ b/parser/parser_policy.c
@@ -91,7 +91,7 @@ static int add_named_transition(Profile *prof, struct cod_entry *entry)
 	char *name = NULL;
 
 	/* check to see if it is a local transition */
-	if (!entry->ns) {
+	if (!label_contains_ns(entry->nt_name)) {
 		char *sub = strstr(entry->nt_name, "//");
 		/* does the subprofile name match the rule */
 
@@ -127,17 +127,6 @@ static int add_named_transition(Profile *prof, struct cod_entry *entry)
 			name = entry->nt_name;
 			entry->nt_name = NULL;
 		}
-	} else {
-	  name = (char *) malloc(strlen(entry->ns) + strlen(entry->nt_name) + 3);
-		if (!name) {
-			PERROR("Memory allocation error\n");
-			exit(1);
-		}
-		sprintf(name, ":%s:%s", entry->ns, entry->nt_name);
-		free(entry->ns);
-		free(entry->nt_name);
-		entry->ns = NULL;
-		entry->nt_name = NULL;
 	}
 
 	return add_entry_to_x_table(prof, name);
@@ -184,7 +173,7 @@ void post_process_file_entries(Profile *prof)
 			PERROR("Memory allocation error\n");
 			exit(1);
 		}
-		new_ent = new_entry(NULL, buffer, AA_MAY_WRITE, NULL);
+		new_ent = new_entry(buffer, AA_MAY_WRITE, NULL);
 		if (!new_ent) {
 			PERROR("Memory allocation error\n");
 			exit(1);
@@ -214,7 +203,7 @@ static int profile_add_hat_rules(Profile *prof)
 		return 0;
 
 	/* add entry to hat */
-	entry = new_entry(NULL, strdup(CHANGEHAT_PATH), AA_MAY_WRITE, NULL);
+	entry = new_entry(strdup(CHANGEHAT_PATH), AA_MAY_WRITE, NULL);
 	if (!entry)
 		return ENOMEM;
 
diff --git a/parser/parser_regex.c b/parser/parser_regex.c
index ca781d4..9270076 100644
--- a/parser/parser_regex.c
+++ b/parser/parser_regex.c
@@ -585,11 +585,6 @@ static int process_dfa_entry(aare_rules *dfarules, struct cod_entry *entry)
 			/* allow change_profile for all execs */
 			vec[0] = "/[^/\\x00][^\\x00]*";
 
-		if (entry->ns) {
-			int pos;
-			ptype = convert_aaregex_to_pcre(entry->ns, 0, glob_default, lbuf, &pos);
-			vec[index++] = lbuf.c_str();
-		}
 		vec[index++] = tbuf.c_str();
 
 		/* regular change_profile rule */
diff --git a/parser/parser_yacc.y b/parser/parser_yacc.y
index 7af78ce..69fa4ff 100644
--- a/parser/parser_yacc.y
+++ b/parser/parser_yacc.y
@@ -71,8 +71,7 @@
 
 int parser_token = 0;
 
-struct cod_entry *do_file_rule(char *ns, char *id, int mode,
-			       char *link_id, char *nt);
+struct cod_entry *do_file_rule(char *id, int mode, char *link_id, char *nt);
 mnt_rule *do_mnt_rule(struct cond_entry *src_conds, char *src,
 		      struct cond_entry *dst_conds, char *dst,
 		      int mode);
@@ -212,7 +211,6 @@ void add_local_entry(Profile *prof);
 	struct cond_entry *cond_entry;
 	struct cond_entry_list cond_entry_list;
 	int boolean;
-	struct named_transition transition;
 	struct prefixes prefix;
 }
 
@@ -277,7 +275,7 @@ void add_local_entry(Profile *prof);
 %type <fmode>	opt_net_perm
 %type <unix_entry>	unix_rule
 %type <id>	opt_target
-%type <transition> opt_named_transition
+%type <id>	opt_named_transition
 %type <boolean> opt_unsafe
 %type <boolean> opt_file
 %%
@@ -1048,14 +1046,10 @@ id_or_var: TOK_SET_VAR { $$ = $1; };
 opt_target: /* nothing */ { $$ = NULL; }
 opt_target: TOK_ARROW id_or_var { $$ = $2; };
 
-opt_named_transition:
-	{ /* nothing */
-		parse_named_transition_target(&$$, NULL);
-	}
+opt_named_transition: { /* nothing */ $$ = NULL; }
 	| TOK_ARROW id_or_var
 	{
-		parse_named_transition_target(&$$, $2);
-		free($2);
+		$$ = $2;
 	};
 
 rule: file_rule { $$ = $1; }
@@ -1070,23 +1064,24 @@ opt_file: { /* nothing */ $$ = 0; }
 
 frule:	id_or_var file_mode opt_named_transition TOK_END_OF_RULE
 	{
-		$$ = do_file_rule($3.ns, $1, $2, NULL, $3.name);
+		$$ = do_file_rule($1, $2, NULL, $3);
 	};
 
 frule:	file_mode opt_subset_flag id_or_var opt_named_transition TOK_END_OF_RULE
 	{
 		if ($2 && ($1 & ~AA_LINK_BITS))
 			yyerror(_("subset can only be used with link rules."));
-		if ($4.present && ($1 & AA_LINK_BITS) && ($1 & AA_EXEC_BITS))
+		if ($4 && ($1 & AA_LINK_BITS) && ($1 & AA_EXEC_BITS))
 			yyerror(_("link and exec perms conflict on a file rule using ->"));
-		if ($4.present && $4.ns && ($1 & AA_LINK_BITS))
+		if ($4 && label_contains_ns($4) && ($1 & AA_LINK_BITS))
 			yyerror(_("link perms are not allowed on a named profile transition.\n"));
+
 		if (($1 & AA_LINK_BITS)) {
-			$$ = do_file_rule(NULL, $3, $1, $4.name, NULL);
+			$$ = do_file_rule($3, $1, $4, NULL);
 			$$->subset = $2;
 
 		} else {
-			$$ = do_file_rule($4.ns, $3, $1, NULL, $4.name);
+			$$ = do_file_rule($3, $1, NULL, $4);
 		}
  	};
 
@@ -1099,7 +1094,7 @@ file_rule: TOK_FILE TOK_END_OF_RULE
 		perms |= perms << AA_OTHER_SHIFT;
 		if (!path)
 			yyerror(_("Memory allocation error."));
-		$$ = do_file_rule(NULL, path, perms, NULL, NULL);
+		$$ = do_file_rule(path, perms, NULL, NULL);
 	}
 	| opt_file file_rule_tail { $$ = $2; }
 
@@ -1132,7 +1127,7 @@ link_rule: TOK_LINK opt_subset_flag id_or_var TOK_ARROW id_or_var TOK_END_OF_RUL
 	{
 		struct cod_entry *entry;
 		PDEBUG("Matched: link tok_id (%s) -> (%s)\n", $3, $5);
-		entry = new_entry(NULL, $3, AA_LINK_BITS, $5);
+		entry = new_entry($3, AA_LINK_BITS, $5);
 		entry->subset = $2;
 		PDEBUG("rule.entry: link (%s)\n", entry->name);
 		$$ = entry;
@@ -1486,17 +1481,16 @@ change_profile: change_profile_head opt_named_transition TOK_END_OF_RULE
 	{
 		struct cod_entry *entry;
 
-		if ($2.present) {
-			PDEBUG("Matched change_profile: tok_id (:%s://%s)\n",
-			       $2.ns ? $2.ns : "", $2.name);
-			entry = new_entry($2.ns, $2.name, AA_CHANGE_PROFILE, $1);
+		if ($2) {
+			PDEBUG("Matched change_profile: tok_id (%s)\n", $2);
+			entry = new_entry($2, AA_CHANGE_PROFILE, $1);
 		} else {
 			char *rule = strdup("**");
 			if (!rule)
 				yyerror(_("Memory allocation error."));
 
 			PDEBUG("Matched change_profile,\n");
-			entry = new_entry(NULL, rule, AA_CHANGE_PROFILE, $1);
+			entry = new_entry(rule, AA_CHANGE_PROFILE, $1);
 		}
 		if (!entry)
 			yyerror(_("Memory allocation error."));
@@ -1567,12 +1561,11 @@ void yyerror(const char *msg, ...)
 	exit(1);
 }
 
-struct cod_entry *do_file_rule(char *ns, char *id, int mode,
-			       char *link_id, char *nt)
+struct cod_entry *do_file_rule(char *id, int mode, char *link_id, char *nt)
 {
 		struct cod_entry *entry;
 		PDEBUG("Matched: tok_id (%s) tok_mode (0x%x)\n", id, mode);
-		entry = new_entry(ns, id, mode, link_id);
+		entry = new_entry(id, mode, link_id);
 		if (!entry)
 			yyerror(_("Memory allocation error."));
 		entry->nt_name = nt;
@@ -1595,7 +1588,7 @@ void add_local_entry(Profile *prof)
 			yyerror(_("Memory allocation error."));
 		sprintf(name, "%s//%s", prof->parent->name, prof->name);
 
-		entry = new_entry(NULL, name, prof->local_mode, NULL);
+		entry = new_entry(name, prof->local_mode, NULL);
 		entry->audit = prof->local_audit;
 		entry->nt_name = trans;
 		if (!entry)
-- 
2.7.0




More information about the AppArmor mailing list