[apparmor] [PATCH 10/11] Add the safe xtransition key word

John Johansen john.johansen at canonical.com
Tue Dec 14 08:58:47 GMT 2010


Currently apparmor provides the unsafe keyword to indicate an xtransition
is not scrubbing its environment variables.  This can be used to be
explicit about which transition are unsafe instead of relying on people
remembering which of px Px is safe or unsafe.

Add the orthogonal keyword safe to allow specifying a transition is
safe.

Signed-off-by: John Johansen <john.johansen at canonical.com>
---
 parser/parser_misc.c                               |    1 +
 parser/parser_yacc.y                               |   46 ++++++++++-------
 parser/tst/Makefile                                |    1 +
 parser/tst/gen-xtrans.pl                           |   53 ++++++++++++++++++++
 .../tst/simple_tests/generated_perms_safe/readme   |    3 +
 5 files changed, 86 insertions(+), 18 deletions(-)
 create mode 100644 parser/tst/simple_tests/generated_perms_safe/readme

diff --git a/parser/parser_misc.c b/parser/parser_misc.c
index e544128..9cc0752 100644
--- a/parser/parser_misc.c
+++ b/parser/parser_misc.c
@@ -63,6 +63,7 @@ static struct keyword_table keyword_table[] = {
 	{"defined",		TOK_DEFINED},
 	{"change_profile",	TOK_CHANGE_PROFILE},
 	{"unsafe",		TOK_UNSAFE},
+	{"safe",		TOK_SAFE},
 	{"link",		TOK_LINK},
 	{"owner",		TOK_OWNER},
 	{"user",		TOK_OWNER},
diff --git a/parser/parser_yacc.y b/parser/parser_yacc.y
index 80f2378..4fc791c 100644
--- a/parser/parser_yacc.y
+++ b/parser/parser_yacc.y
@@ -98,6 +98,7 @@ void add_local_entry(struct codomain *cod);
 %token TOK_NETWORK
 %token TOK_HAT
 %token TOK_UNSAFE
+%token TOK_SAFE
 %token TOK_COLON
 %token TOK_LINK
 %token TOK_OWNER
@@ -171,6 +172,7 @@ void add_local_entry(struct codomain *cod);
 %type <cod>	cond_rule
 %type <network_entry> network_rule
 %type <user_entry> rule
+%type <user_entry> frule
 %type <flags>	flags
 %type <flags>	flagvals
 %type <flags>	flagval
@@ -869,40 +871,48 @@ opt_named_transition:
 		$$.name = $5;
 	};
 
-rule:	id_or_var file_mode opt_named_transition TOK_END_OF_RULE
-	{
-		$$ = do_file_rule($3.namespace, $1, $2, NULL, $3.name);
-	};
-
 opt_unsafe: { /* nothing */ $$ = 0; }
 	| TOK_UNSAFE { $$ = 1; };
+	| TOK_SAFE { $$ = 2; };
 
-rule:   opt_unsafe file_mode opt_subset_flag id_or_var opt_named_transition TOK_END_OF_RULE
+rule:	opt_unsafe frule
 	{
-		int mode = $2;
 		if ($1) {
-			if (!($2 & AA_EXEC_BITS))
+			if (!($2->mode & AA_EXEC_BITS))
 				yyerror(_("unsafe rule missing exec permissions"));
-			mode = ($2 & ~ALL_AA_EXEC_UNSAFE) |
-				((($2 & AA_EXEC_BITS) << 8) & ALL_AA_EXEC_UNSAFE);
+			if ($1 == 1) {
+				$2->mode |= (($2->mode & AA_EXEC_BITS) << 8) &
+					 ALL_AA_EXEC_UNSAFE;
+			}
+			else if ($1 == 2)
+				$2->mode &= ~ALL_AA_EXEC_UNSAFE;
 		}
+		$$ = $2;
+	};
+
+frule:	id_or_var file_mode opt_named_transition TOK_END_OF_RULE
+	{
+		$$ = do_file_rule($3.namespace, $1, $2, NULL, $3.name);
+	};
 
-		if ($3 && ($2 & ~AA_LINK_BITS))
+frule:	file_mode opt_subset_flag id_or_var opt_named_transition TOK_END_OF_RULE
+	{
+		if ($2 && ($1 & ~AA_LINK_BITS))
 			yyerror(_("subset can only be used with link rules."));
-		if ($5.present && ($2 & AA_LINK_BITS) && ($2 & AA_EXEC_BITS))
+		if ($4.present && ($1 & AA_LINK_BITS) && ($1 & AA_EXEC_BITS))
 			yyerror(_("link and exec perms conflict on a file rule using ->"));
-		if ($5.present && $5.namespace && ($2 & AA_LINK_BITS))
+		if ($4.present && $4.namespace && ($1 & AA_LINK_BITS))
 			yyerror(_("link perms are not allowed on a named profile transition.\n"));
-		if (($2 & AA_LINK_BITS)) {
-			$$ = do_file_rule(NULL, $4, mode, $5.name, NULL);
-			$$->subset = $3;
+		if (($1 & AA_LINK_BITS)) {
+			$$ = do_file_rule(NULL, $3, $1, $4.name, NULL);
+			$$->subset = $2;
 
 		} else {
-			$$ = do_file_rule($5.namespace, $4, mode, NULL, $5.name);
+			$$ = do_file_rule($4.namespace, $3, $1, NULL, $4.name);
 		}
  	};
 
-rule:  id_or_var file_mode id_or_var
+rule:  opt_unsafe id_or_var file_mode id_or_var
 	{
 		/* Oopsie, we appear to be missing an EOL marker. If we
 		 * were *smart*, we could work around it. Since we're
diff --git a/parser/tst/Makefile b/parser/tst/Makefile
index b30ec6e..a6668b2 100644
--- a/parser/tst/Makefile
+++ b/parser/tst/Makefile
@@ -41,3 +41,4 @@ $(PARSER):
 clean:
 	rm -f simple_tests/generated_x/*
 	rm -f simple_tests/generated_perms_leading/*
+	rm -f simple_tests/generated_perms_safe/*
diff --git a/parser/tst/gen-xtrans.pl b/parser/tst/gen-xtrans.pl
index a1fe4a7..1285063 100755
--- a/parser/tst/gen-xtrans.pl
+++ b/parser/tst/gen-xtrans.pl
@@ -8,6 +8,7 @@ setlocale(LC_MESSAGES, "");
 
 my $prefix="simple_tests/generated_x";
 my $prefix_leading="simple_tests/generated_perms_leading";
+my $prefix_safe="simple_tests/generated_perms_safe";
 
 my @trans_types = ("p", "P", "c", "C", "u", "i");
 my @modifiers = ("i", "u");
@@ -30,6 +31,20 @@ my %named_trans = (
     "i" => \@null_target,
     );
 
+my %safe_map = (
+    "p" => "unsafe",
+    "P" => "safe",
+    "c" => "unsafe",
+    "C" => "safe",
+    "u" => "",
+    "i" => "",
+    );
+
+my %invert_safe = (
+    "safe" => "unsafe",
+    "unsafe" => "safe",
+    );
+
 # audit qualifier disabled for now it really shouldn't affect the conflict
 # test but it may be worth checking every once in awhile
 #my @qualifiers = ("", "owner", "audit", "audit owner");
@@ -46,6 +61,16 @@ gen_leading_perms("exact-re", "/bin/*", "/bin/*");
 gen_leading_perms("overlap", "/*", "/bin/cat");
 gen_leading_perms("dominate", "/**", "/*");
 gen_leading_perms("ambiguous", "/a*", "/*b");
+gen_safe_perms("exact", "PASS", "", "/bin/cat", "/bin/cat");
+gen_safe_perms("exact-re", "PASS", "", "/bin/*", "/bin/*");
+gen_safe_perms("overlap", "PASS", "", "/*", "/bin/cat");
+gen_safe_perms("dominate", "PASS", "", "/**", "/*");
+gen_safe_perms("ambiguous", "PASS", "", "/a*", "/*b");
+gen_safe_perms("exact", "FAIL", "inv", "/bin/cat", "/bin/cat");
+gen_safe_perms("exact-re", "FAIL", "inv", "/bin/*", "/bin/*");
+gen_safe_perms("overlap", "PASS", "inv", "/*", "/bin/cat");
+gen_safe_perms("dominate", "FAIL", "inv", "/**", "/*");
+gen_safe_perms("ambiguous", "FAIL", "inv", "/a*", "/*b");
 
 print "Generated $count xtransition interaction tests\n";
 
@@ -180,3 +205,31 @@ sub gen_leading_perms($$$) {
 	}
     }
 }
+
+# test for rules with leading safe or unsafe keywords.
+# check they are equivalent to their counter part,
+# or if $invert that they properly conflict with their counterpart
+sub gen_safe_perms($$$$$) {
+    my ($name, $xres, $invert, $rule1, $rule2) = @_;
+
+    my @perms = gen_list();
+
+    foreach my $i (@perms) {
+	foreach my $t (@{$named_trans{substr($i, 0, 1)}}) {
+	    foreach my $q (@qualifiers) {
+		my $qual = $safe_map{substr($i, 0, 1)};
+		if ($invert) {
+		    $qual = $invert_safe{$qual};
+		}
+		if (! $invert || $qual) {
+		    my $file="${prefix_safe}/${name}-$invert-$q${qual}-rule-$i$t.sd";
+#		    print "$file\n";
+		    gen_file($file, $xres, 0, "$q $qual", $rule1, $i, $t, 1, $q, $rule2, $i, $t);
+		    $file="${prefix_safe}/${name}-$invert-$q$qual${i}-rule-$t.sd";
+		    gen_file($file, $xres, 0, $q, $rule1, $i, $t, 1, "$q $qual", $rule2, $i, $t);
+		}
+
+	    }
+	}
+    }
+}
diff --git a/parser/tst/simple_tests/generated_perms_safe/readme b/parser/tst/simple_tests/generated_perms_safe/readme
new file mode 100644
index 0000000..3a29bb6
--- /dev/null
+++ b/parser/tst/simple_tests/generated_perms_safe/readme
@@ -0,0 +1,3 @@
+Generated tests for safe and unsafe xtransition interaction in their various
+forms.
+
-- 
1.7.1




More information about the AppArmor mailing list