On 02/11/2016 01:57 PM, Tyler Hicks wrote: > https://launchpad.net/bugs/1544387 > > Don't split namespaces from profile names using YACC grammar. Instead, > treat the entire string as a label in the grammer. The label can then be > split into a namespace and a profile name using the new parse_label() > function. > > This fixes a bug that caused the profile keyword to not be used with a > label containing a namespace in the profile declaration. > > Fixing this bug uncovered a bad parser test case at > simple_tests/profile/profile_ns_ok1.sd. The test case mistakenly > included two definitions of the :foo:unattached profile despite being > marked as expected to pass. I've adjusted the name of one of the > profiles to :foo:unattached2. > > Signed-off-by: Tyler Hicks <[email protected]>
Acked-by: John Johansen <[email protected]> for 2.10 as well > --- > parser/parser.h | 1 + > parser/parser_lex.l | 8 +-- > parser/parser_misc.c | 46 +++++++++++++ > parser/parser_yacc.y | 40 ++++++------ > parser/tst/simple_tests/profile/profile_ns_bad5.sd | 9 +++ > parser/tst/simple_tests/profile/profile_ns_bad6.sd | 13 ++++ > parser/tst/simple_tests/profile/profile_ns_bad7.sd | 13 ++++ > parser/tst/simple_tests/profile/profile_ns_bad8.sd | 9 +++ > parser/tst/simple_tests/profile/profile_ns_ok1.sd | 2 +- > tests/regression/apparmor/Makefile | 1 + > tests/regression/apparmor/namespaces.sh | 76 > ++++++++++++++++++++++ > tests/regression/apparmor/prologue.inc | 9 +++ > 12 files changed, 202 insertions(+), 25 deletions(-) > create mode 100644 parser/tst/simple_tests/profile/profile_ns_bad5.sd > create mode 100644 parser/tst/simple_tests/profile/profile_ns_bad6.sd > create mode 100644 parser/tst/simple_tests/profile/profile_ns_bad7.sd > create mode 100644 parser/tst/simple_tests/profile/profile_ns_bad8.sd > create mode 100755 tests/regression/apparmor/namespaces.sh > > diff --git a/parser/parser.h b/parser/parser.h > index 58bd00a..b72c601 100644 > --- a/parser/parser.h > +++ b/parser/parser.h > @@ -393,6 +393,7 @@ 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); > +void parse_label(char **ns, char **name, const char *label); > extern struct cod_entry *new_entry(char *ns, char *id, int mode, char > *link_id); > > /* returns -1 if value != true or false, otherwise 0 == false, 1 == true */ > diff --git a/parser/parser_lex.l b/parser/parser_lex.l > index af5dec4..e26b2b9 100644 > --- a/parser/parser_lex.l > +++ b/parser/parser_lex.l > @@ -225,8 +225,8 @@ SET_VAR_PREFIX @ > SET_VARIABLE {SET_VAR_PREFIX}(\{{VARIABLE_NAME}\}|{VARIABLE_NAME}) > BOOL_VARIABLE $(\{{VARIABLE_NAME}\}|{VARIABLE_NAME}) > > -PATHNAME (\/|{SET_VARIABLE}{POST_VAR_ID}){ID}* > -QPATHNAME \"(\/|{SET_VAR_PREFIX})([^\0"]|\\\")*\" > +LABEL (\/|{SET_VARIABLE}{POST_VAR_ID}|{COLON}){ID}* > +QUOTED_LABEL \"(\/|{SET_VAR_PREFIX}|{COLON})([^\0"]|\\\")*\" > > OPEN_PAREN \( > CLOSE_PAREN \) > @@ -510,7 +510,7 @@ LT_EQUAL <= > } > > <MOUNT_MODE,DBUS_MODE,SIGNAL_MODE,PTRACE_MODE,UNIX_MODE>{ > - ({IDS_NOEQ}|{PATHNAME}|{QUOTED_ID}) { > + ({IDS_NOEQ}|{LABEL}|{QUOTED_ID}) { > yylval.id = processid(yytext, yyleng); > RETURN_TOKEN(TOK_ID); > } > @@ -557,7 +557,7 @@ include/{WS} { > > {CLOSE_BRACE} { RETURN_TOKEN(TOK_CLOSE); } > > -({PATHNAME}|{QPATHNAME}) { > +({LABEL}|{QUOTED_LABEL}) { > yylval.id = processid(yytext, yyleng); > RETURN_TOKEN(TOK_ID); > } > diff --git a/parser/parser_misc.c b/parser/parser_misc.c > index 3d06b79..a0e62ea 100644 > --- a/parser/parser_misc.c > +++ b/parser/parser_misc.c > @@ -569,6 +569,52 @@ int parse_X_mode(const char *X, int valid, const char > *str_mode, int *mode, int > return 1; > } > > +void parse_label(char **ns, char **name, const char *label) > +{ > + const char *name_start = NULL; > + char *_ns = NULL; > + char *_name = NULL; > + > + if (label[0] != ':') { > + /* There is no namespace specified in the label */ > + name_start = label; > + } else { > + /* A leading ':' indicates that a namespace is specified */ > + const char *ns_start = label + 1; > + const char *ns_end = strstr(ns_start, ":"); > + > + if (!ns_end) > + yyerror(_("Namespace not terminated: %s\n"), label); > + else if (ns_end - ns_start == 0) > + yyerror(_("Empty namespace: %s\n"), label); > + > + /** > + * Handle either of the two namespace formats: > + * 1) :ns:name > + * 2) :ns://name > + */ > + name_start = ns_end + 1; > + if (!strncmp(name_start, "//", 2)) > + name_start += 2; > + > + _ns = strndup(ns_start, ns_end - ns_start); > + if (!_ns) > + yyerror(_("Memory allocation error.")); > + } > + > + if (!strlen(name_start)) > + yyerror(_("Empty named transition profile name: %s\n"), label); > + > + _name = strdup(name_start); > + if (!_name) { > + free(_ns); > + yyerror(_("Memory allocation error.")); > + } > + > + *ns = _ns; > + *name = _name; > +} > + > struct cod_entry *new_entry(char *ns, char *id, int mode, char *link_id) > { > struct cod_entry *entry = NULL; > diff --git a/parser/parser_yacc.y b/parser/parser_yacc.y > index ad54035..c116e61 100644 > --- a/parser/parser_yacc.y > +++ b/parser/parser_yacc.y > @@ -318,14 +318,26 @@ profile_base: TOK_ID opt_id_or_var flags TOK_OPEN rules > TOK_CLOSE > yyerror(_("Memory allocation error.")); > } > > + parse_label(&prof->ns, &prof->name, $1); > + free($1); > + > /* Honor the --namespace-string command line option */ > if (profile_ns) { > + /** > + * Print warning if the profile specified a namespace > + * different than the one specified with the > + * --namespace-string command line option > + */ > + if (prof->ns && strcmp(prof->ns, profile_ns)) > + pwarn("%s: -n %s overriding policy specified > namespace :%s:\n", > + progname, profile_ns, prof->ns); > + > + free(prof->ns); > prof->ns = strdup(profile_ns); > if (!prof->ns) > yyerror(_("Memory allocation error.")); > } > > - prof->name = $1; > prof->attachment = $2; > if ($2 && !($2[0] == '/' || strncmp($2, "@{", 2) == 0)) > yyerror(_("Profile attachment must begin with a '/' or > variable.")); > @@ -347,30 +359,18 @@ profile_base: TOK_ID opt_id_or_var flags TOK_OPEN rules > TOK_CLOSE > > }; > > -profile: opt_profile_flag opt_ns profile_base > +profile: opt_profile_flag profile_base > { > - Profile *prof = $3; > - if ($2) > - PDEBUG("Matched: %s://%s { ... }\n", $2, $3->name); > + Profile *prof = $2; > + > + if ($2->ns) > + PDEBUG("Matched: :%s://%s { ... }\n", $2->ns, $2->name); > else > - PDEBUG("Matched: %s { ... }\n", $3->name); > + PDEBUG("Matched: %s { ... }\n", $2->name); > > - if ($3->name[0] != '/' && !($1 || $2)) > + if ($2->name[0] != '/' && !($1 || $2->ns)) > yyerror(_("Profile names must begin with a '/', > namespace or keyword 'profile' or 'hat'.")); > > - if (prof->ns) { > - /** > - * Print warning if the profile specified a namespace > - * different than the one specified with the > - * --namespace-string command line option > - */ > - if ($2 && strcmp(prof->ns, $2)) { > - pwarn("%s: -n %s overriding policy specified > namespace :%s:\n", > - progname, prof->ns, $2); > - } > - free($2); > - } else > - prof->ns = $2; > if ($1 == 2) > prof->flags.hat = 1; > $$ = prof; > diff --git a/parser/tst/simple_tests/profile/profile_ns_bad5.sd > b/parser/tst/simple_tests/profile/profile_ns_bad5.sd > new file mode 100644 > index 0000000..be4c4f2 > --- /dev/null > +++ b/parser/tst/simple_tests/profile/profile_ns_bad5.sd > @@ -0,0 +1,9 @@ > +# > +#=DESCRIPTION namespace with no profile name > +#=EXRESULT FAIL > +# vim:syntax=apparmor > +# Last Modified: Thu Feb 11 00:14:20 2016 > +# > +:namespace: { > + /does/not/exist r, > +} > diff --git a/parser/tst/simple_tests/profile/profile_ns_bad6.sd > b/parser/tst/simple_tests/profile/profile_ns_bad6.sd > new file mode 100644 > index 0000000..ea326b9 > --- /dev/null > +++ b/parser/tst/simple_tests/profile/profile_ns_bad6.sd > @@ -0,0 +1,13 @@ > +# > +#=DESCRIPTION collision same profile, same namespace with profile keyword > +#=EXRESULT FAIL > +# vim:syntax=apparmor > +# Last Modified: Thu Feb 11 00:14:20 2016 > +# > +profile :ns:/t { > + /does/not/exist r, > +} > + > +profile :ns:/t { > + /does/not/exist r, > +} > diff --git a/parser/tst/simple_tests/profile/profile_ns_bad7.sd > b/parser/tst/simple_tests/profile/profile_ns_bad7.sd > new file mode 100644 > index 0000000..f9231fe > --- /dev/null > +++ b/parser/tst/simple_tests/profile/profile_ns_bad7.sd > @@ -0,0 +1,13 @@ > +# > +#=DESCRIPTION collision same profile, same namespace w/ and w/o profile > keyword > +#=EXRESULT FAIL > +# vim:syntax=apparmor > +# Last Modified: Thu Feb 11 00:14:20 2016 > +# > +:ns:/t { > + /does/not/exist r, > +} > + > +profile :ns:/t { > + /does/not/exist r, > +} > diff --git a/parser/tst/simple_tests/profile/profile_ns_bad8.sd > b/parser/tst/simple_tests/profile/profile_ns_bad8.sd > new file mode 100644 > index 0000000..8bec87a > --- /dev/null > +++ b/parser/tst/simple_tests/profile/profile_ns_bad8.sd > @@ -0,0 +1,9 @@ > +# > +#=DESCRIPTION no terminating ':' for ns namespace (w/ profile keyword) > +#=EXRESULT FAIL > +# vim:syntax=apparmor > +# Last Modified: Thu Feb 11 00:14:20 2016 > +# > +profile :ns/t { > + /does/not/exist r, > +} > diff --git a/parser/tst/simple_tests/profile/profile_ns_ok1.sd > b/parser/tst/simple_tests/profile/profile_ns_ok1.sd > index 03213c3..ef22434 100644 > --- a/parser/tst/simple_tests/profile/profile_ns_ok1.sd > +++ b/parser/tst/simple_tests/profile/profile_ns_ok1.sd > @@ -40,7 +40,7 @@ profile :foo:/does/not/exist2 { > /bin/echo uxuxuxuxux, > } > > -profile :foo:unattached { > +profile :foo:unattached2 { > #include <includes/base> > > /usr/X11R6/lib/lib*so* rrr, > diff --git a/tests/regression/apparmor/Makefile > b/tests/regression/apparmor/Makefile > index 892f1c5..a6581e0 100644 > --- a/tests/regression/apparmor/Makefile > +++ b/tests/regression/apparmor/Makefile > @@ -200,6 +200,7 @@ TESTS=aa_exec \ > mount \ > mult_mount \ > named_pipe \ > + namespaces \ > net_raw \ > open \ > openat \ > diff --git a/tests/regression/apparmor/namespaces.sh > b/tests/regression/apparmor/namespaces.sh > new file mode 100755 > index 0000000..a82ae01 > --- /dev/null > +++ b/tests/regression/apparmor/namespaces.sh > @@ -0,0 +1,76 @@ > +#! /bin/bash > +# Copyright (C) 2016 Canonical, Ltd. > +# > +# This program is free software; you can redistribute it and/or > +# modify it under the terms of the GNU General Public License as > +# published by the Free Software Foundation, version 2 of the > +# License. > + > +#=NAME namespaces > +#=DESCRIPTION > +# Verifies basic namespace functionality > +#=END > + > +pwd=`dirname $0` > +pwd=`cd $pwd ; /bin/pwd` > + > +bin=$pwd > + > +. $bin/prologue.inc > +requires_namespace_interface > + > +# unique_ns - Print a randomly generated, unused namespace identifier to > stdout > +unique_ns() { > + # racy way of generating a namespace name that is likely to be unique > + local ns=$(mktemp --dry-run -dp > /sys/kernel/security/apparmor/policy/namespaces -t test_namespaces_XXXXXX) > + basename "$ns" > +} > + > +# genprofile_ns - Generate and load a profile using a randomly generated > namespace > +# $1: The profile name to use (without a namespace) > +# $2: Non-zero if the 'profile' keyword should be prefixed to the declaration > +# > +# Returns the randomly generated namespace that the profile was loaded into > +genprofile_ns() { > + local prefix="" > + local ns=$(unique_ns) > + local prof=$1 > + > + if [ $2 -ne 0 ]; then > + prefix="profile " > + fi > + > + # override the sys_profiles variable with a bad path so that genprofile > + # doesn't perform profile load checking in the wrong policy namespace > + echo "${prefix}:${ns}:${prof} {}" | sys_profiles="${sys_profiles}XXX" > genprofile --stdin > + echo "$ns" > +} > + > +# genprofile_ns_and_verify - Generate and load a profile into a namespace and > +# verify the creation of the profile and namespace > +# $1: A description of this test > +# $2: Non-zero if the 'profile' keyword should be prefixed to the declaration > +genprofile_ns_and_verify() { > + local desc=$1 > + local prof="p" > + local ns=$(genprofile_ns "$prof" $2) > + > + > + [ -d /sys/kernel/security/apparmor/policy/namespaces/${ns} ] > + local dir_created=$? > + [ -d > /sys/kernel/security/apparmor/policy/namespaces/${ns}/profiles/${prof}* ] > + local prof_created=$? > + removeprofile > + if [ $dir_created -ne 0 ]; then > + echo "Error: ${testname} failed. Test '${desc}' did not create > the expected namespace directory in apparmorfs: policy/namespaces/${ns}" > + testfailed > + elif [ $prof_created -ne 0 ]; then > + echo "Error: ${testname} failed. Test '${desc}' did not create > the expected namespaced profile directory in apparmorfs: > policy/namespaces/${ns}/profiles/${prof}" > + testfailed > + elif [ -n "$VERBOSE" ]; then > + echo "ok: ${desc}" > + fi > +} > + > +genprofile_ns_and_verify "NAMESPACES create unique ns (w/o profile keyword > prefix)" 0 > +genprofile_ns_and_verify "NAMESPACES create unique ns (w/ profile keyword > prefix)" 1 > diff --git a/tests/regression/apparmor/prologue.inc > b/tests/regression/apparmor/prologue.inc > index f6707ab..67e1aae 100755 > --- a/tests/regression/apparmor/prologue.inc > +++ b/tests/regression/apparmor/prologue.inc > @@ -49,6 +49,15 @@ requires_kernel_features() > fi > } > > +requires_namespace_interface() > +{ > + if [ ! -e "/sys/kernel/security/apparmor/policy/namespaces" ] > + then > + echo "Namespaces in apparmorfs policy interface not supported. > Skipping tests ..." > + exit 0 > + fi > +} > + > requires_query_interface() > { > if [ ! -e "/sys/kernel/security/apparmor/.access" ] > -- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
