[apparmor] [PATCH 4/6] parser: stop splitting the namespace from the named transition targets
John Johansen
john.johansen at canonical.com
Fri Mar 4 16:28:45 UTC 2016
On 03/04/2016 12:16 AM, Tyler Hicks wrote:
> 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.
>
not quite right, the kernel never did for pivotroot transitions but
it actually does for x transitions. With that said this is a better
way to handle it and the current kernel handles either.
We can add the null terminated ns as a conditional in the final
encode step.
ie. don't change this patch we can add a new one on top, for the
file x transitions.
> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
Acked-by: John Johansen <john.johansen 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)
>
More information about the AppArmor
mailing list