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@ubuntu.com>

parser/apparmor.d.pod

@@ -83,7 +83,7 @@ B<FILEGLOB> = (must start with '/' (after variable expansion), B<?*[]{}^> have s
 
 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)
 

parser/parser_variable.c

@@ -36,8 +36,14 @@ static inline char *get_var_end(char *var)
 	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)

parser/tst/simple_tests/vars/vars_alternation_1.sd

@@ -0,0 +1,9 @@
+#=DESCRIPTION reference variables in rules that also have alternations
+#=EXRESULT
+
+@{FOO}=bar baz
+@{BAR}=${FOO} blort
+
+/does/not/exist {
+  /does/not/{exist,notexist}/blah/@{BAR} r,
+}

parser/tst/simple_tests/vars/vars_alternation_2.sd

@@ -0,0 +1,9 @@
+#=DESCRIPTION reference variables in rules that also have alternations
+#=EXRESULT
+
+@{FOO}=bar baz
+@{BAR}=${FOO} blort
+
+/does/not/exist {
+  /does/not/@{BAR}/blah/{exist,notexist} r,
+}

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,
+}

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,
+}

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,
+}

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,
+}

utils/apparmor.vim

@@ -113,7 +113,7 @@ syn match sdError /^.*$/ contains=sdComment "highlight all non-valid lines as er
 " 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