[apparmor] [patch] make parser's definition of allowed var names consistent
John Johansen
john.johansen at canonical.com
Mon Mar 28 10:37:37 UTC 2011
On 03/28/2011 02:08 AM, Steve Beattie wrote:
> [This patch is nominated for trunk and 2.6.2.]
>
> The parser's lexer supports variables defined matching the regex
> '[[:alpha:]][[:alnum:]_]*' (i.e. a single alpha followed by any number
> of alphanumerics or underscores). Unfortunately, the code that expends
> variables inside a profile does not match this, it incorrectly matched
> '([[:alpha:]]|_)+' (one or more alphas or underscores). This patch
> corrects the behavior there as well as synchronizing the expected
> variable names in the apparmor.d manpage and apparmor.vim syntax file.
>
> It also adds unit tests and testcases to verify the behavior.
>
:)
> Signed-off-by: Steve Beattie <sbeattie at ubuntu.com>
Acked-by: John Johansen <john.johansen at canonical.com>
> === modified file 'parser/parser_variable.c'
> ---
> parser/apparmor.d.pod | 2
> parser/parser_variable.c | 40 +++++++++++++++-
> parser/tst/simple_tests/vars/vars_bad_4.sd | 7 ++
> parser/tst/simple_tests/vars/vars_bad_5.sd | 7 ++
> parser/tst/simple_tests/vars/vars_file_evaluation_15.sd | 9 +++
> parser/tst/simple_tests/vars/vars_file_evaluation_16.sd | 7 ++
> utils/apparmor.vim | 2
> 7 files changed, 70 insertions(+), 4 deletions(-)
>
> Index: b/parser/parser_variable.c
> ===================================================================
> --- a/parser/parser_variable.c
> +++ b/parser/parser_variable.c
> @@ -36,8 +36,14 @@ static inline char *get_var_end(char *va
> while (*eptr) {
> if (*eptr == '}')
> return eptr;
> - if (!(*eptr == '_' || isalpha(*eptr)))
> - return NULL; /* invalid char */
> + /* first character must be alpha */
> + if (eptr == var) {
> + if (!isalpha(*eptr))
> + return NULL; /* invalid char */
> + } else {
> + if (!(*eptr == '_' || isalnum(*eptr)))
> + return NULL; /* invalid char */
> + }
> eptr++;
> }
> return NULL; /* no terminating '}' */
> @@ -317,6 +323,8 @@ int test_split_out_var(void)
> struct var_string *ret_struct;
> char *prefix = "abcdefg";
> char *var = "boogie";
> + char *var2 = "V4rW1thNum5";
> + char *var3 = "boogie_board";
> char *suffix = "suffixication";
>
> /* simple case */
> @@ -394,6 +402,34 @@ int test_split_out_var(void)
> MY_TEST(strcmp(ret_struct->suffix, suffix) == 0, "split out var 7 suffix");
> free_var_string(ret_struct);
>
> + /* numeric char in var, should succeed */
> + asprintf(&tst_string, "%s@{%s}%s", prefix, var2, suffix);
> + ret_struct = split_out_var(tst_string);
> + MY_TEST(ret_struct && strcmp(ret_struct->prefix, prefix) == 0, "split out numeric var prefix");
> + MY_TEST(ret_struct && strcmp(ret_struct->var, var2) == 0, "split numeric var var");
> + MY_TEST(ret_struct && strcmp(ret_struct->suffix, suffix) == 0, "split out numeric var suffix");
> + free_var_string(ret_struct);
> +
> + /* numeric first char in var, should fail */
> + asprintf(&tst_string, "%s@{6%s}%s", prefix, var2, suffix);
> + ret_struct = split_out_var(tst_string);
> + MY_TEST(ret_struct == NULL, "split out var - numeric first char in var");
> + free_var_string(ret_struct);
> +
> + /* underscore char in var, should succeed */
> + asprintf(&tst_string, "%s@{%s}%s", prefix, var3, suffix);
> + ret_struct = split_out_var(tst_string);
> + MY_TEST(ret_struct && strcmp(ret_struct->prefix, prefix) == 0, "split out underscore var prefix");
> + MY_TEST(ret_struct && strcmp(ret_struct->var, var3) == 0, "split out underscore var");
> + MY_TEST(ret_struct && strcmp(ret_struct->suffix, suffix) == 0, "split out underscore var suffix");
> + free_var_string(ret_struct);
> +
> + /* underscore first char in var, should fail */
> + asprintf(&tst_string, "%s@{_%s%s}%s", prefix, var2, var3, suffix);
> + ret_struct = split_out_var(tst_string);
> + MY_TEST(ret_struct == NULL, "split out var - underscore first char in var");
> + free_var_string(ret_struct);
> +
> return rc;
> }
> int main(void)
> Index: b/parser/tst/simple_tests/vars/vars_bad_4.sd
> ===================================================================
> --- /dev/null
> +++ b/parser/tst/simple_tests/vars/vars_bad_4.sd
> @@ -0,0 +1,7 @@
> +#=DESCRIPTION don't accept variables with leading underscores
> +#=EXRESULT FAIL
> +@{_FOO} = /foo /bar /baz /biff
> +
> +/usr/bin/foo {
> + /@{_FOO}/.foo/* r,
> +}
> Index: b/parser/tst/simple_tests/vars/vars_file_evaluation_16.sd
> ===================================================================
> --- /dev/null
> +++ b/parser/tst/simple_tests/vars/vars_file_evaluation_16.sd
> @@ -0,0 +1,7 @@
> +#=DESCRIPTION simple expansions within file rules with underscore variable
> +#=EXRESULT PASS
> +@{F_OO} = /foo /bar /baz /biff
> +
> +/usr/bin/foo {
> + /@{F_OO}/.foo/* r,
> +}
> Index: b/parser/tst/simple_tests/vars/vars_bad_5.sd
> ===================================================================
> --- /dev/null
> +++ b/parser/tst/simple_tests/vars/vars_bad_5.sd
> @@ -0,0 +1,7 @@
> +#=DESCRIPTION don't accept variables with leading numeric
> +#=EXRESULT FAIL
> +@{4FOO} = /foo /bar /baz /biff
> +
> +/usr/bin/foo {
> + /@{4FOO}/.foo/* r,
> +}
> Index: b/parser/tst/simple_tests/vars/vars_file_evaluation_15.sd
> ===================================================================
> --- /dev/null
> +++ b/parser/tst/simple_tests/vars/vars_file_evaluation_15.sd
> @@ -0,0 +1,9 @@
> +#=DESCRIPTION simple expansions within file rules with numeric variable
> +#=EXRESULT PASS
> +@{FOO1} = /foo /bar /baz /biff
> +@{B1A1R1} = @{FOO1}
> +
> +/usr/bin/foo {
> + /@{FOO1}/.foo/* r,
> + /foo/@{B1A1R1}/.foo/* r,
> +}
> Index: b/parser/apparmor.d.pod
> ===================================================================
> --- a/parser/apparmor.d.pod
> +++ b/parser/apparmor.d.pod
> @@ -83,7 +83,7 @@ B<FILEGLOB> = (must start with '/' (afte
>
> B<ACCESS> = ( 'r' | 'w' | 'l' | 'ix' | 'ux' | 'Ux' | 'px' | 'Px' | 'cx -> ' I<PROGRAMCHILD> | 'Cx -> ' I<PROGRAMCHILD> | 'm' ) [ I<ACCESS> ... ] (not all combinations are allowed; see below.)
>
> -B<VARIABLE> = '@{' I<ALPHA> [ I<ALPHANUMERIC> ... ] '}'
> +B<VARIABLE> = '@{' I<ALPHA> [ ( I<ALPHANUMERIC> | '_' ) ... ] '}'
>
> B<VARIABLE ASSIGNMENT> = I<VARIABLE> ('=' | '+=') (space separated values)
>
> Index: b/utils/apparmor.vim
> ===================================================================
> --- a/utils/apparmor.vim
> +++ b/utils/apparmor.vim
> @@ -113,7 +113,7 @@ syn match sdError /^.*$/ contains=sdComm
> " This allows incorrect lines also and should be checked better.
> " This also (accidently ;-) includes variable definitions (@{FOO}=/bar)
> " TODO: make a separate pattern for variable definitions, then mark sdGlob as contained
> -syn match sdGlob /\v\?|\*|\{.*,.*\}|[[^\]]\+\]|\@\{[a-zA-Z_]*\}/
> +syn match sdGlob /\v\?|\*|\{.*,.*\}|[[^\]]\+\]|\@\{[a-zA-Z][a-zA-Z0-9_]*\}/
>
> syn match sdAlias /\v^alias\s+(\/|\@\{\S*\})\S*\s+-\>\s+(\/|\@\{\S*\})\S*\s*,(\s*$|(\s*#.*$)\@=)/ contains=sdGlob
>
>
>
More information about the AppArmor
mailing list