[apparmor] [PATCH 1/2] parser: Allow the profile keyword to be used with namespaces
John Johansen
john.johansen at canonical.com
Thu Feb 18 06:43:05 UTC 2016
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 <tyhicks at canonical.com>
Acked-by: John Johansen <john.johansen at canonical.com>
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" ]
>
More information about the AppArmor
mailing list