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]> --- 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" ] -- 2.5.0 -- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
