[apparmor] [PATCH v1.3 05/11] parser: Allow change_profile rules to accept an exec mode modifier
Tyler Hicks
tyhicks at canonical.com
Tue May 31 20:17:20 UTC 2016
https://launchpad.net/bugs/1584069
This patch allows policy authors to specify how exec transitions should
be handled with respect to setting AT_SECURE in the new process'
auxiliary vector and, ultimately, having libc scrub (or not scrub) the
environment.
An exec mode of 'safe' means that the environment will be scrubbed and
this is the default in kernels that support AppArmor profile stacking.
An exec mode of 'unsafe' means that the environment will not be scrubbed
and this is the default and only supported change_profile exec mode in
kernels that do not support AppArmor profile stacking.
Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
---
* Changes from v1.2:
- Create a SUB_ID_WS mode that eats whitespace and have
CHANGE_PROFILE_MODE push state their whenever it encounters an ARROW
- Drop the optional trailing {WS} and \n match following an ARROW in
CHANGE_PROFILE_MODE
parser/parser_lex.l | 25 ++++++++++++++++++++++---
parser/parser_regex.c | 44 +++++++++++++++++++++++++++++++++++++-------
parser/parser_yacc.y | 28 ++++++++++++++++++++++++----
3 files changed, 83 insertions(+), 14 deletions(-)
diff --git a/parser/parser_lex.l b/parser/parser_lex.l
index 49b1f22..a59daa6 100644
--- a/parser/parser_lex.l
+++ b/parser/parser_lex.l
@@ -239,6 +239,7 @@ LT_EQUAL <=
/* IF adding new state please update state_names table at eof */
%x SUB_ID
+%x SUB_ID_WS
%x SUB_VALUE
%x EXTCOND_MODE
%x EXTCONDLIST_MODE
@@ -268,7 +269,7 @@ LT_EQUAL <=
}
%}
-<INITIAL,INCLUDE,LIST_VAL_MODE,EXTCOND_MODE,LIST_COND_VAL,LIST_COND_PAREN_VAL,LIST_COND_MODE,EXTCONDLIST_MODE,ASSIGN_MODE,NETWORK_MODE,CHANGE_PROFILE_MODE,RLIMIT_MODE,MOUNT_MODE,DBUS_MODE,SIGNAL_MODE,PTRACE_MODE,UNIX_MODE>{
+<INITIAL,SUB_ID_WS,INCLUDE,LIST_VAL_MODE,EXTCOND_MODE,LIST_COND_VAL,LIST_COND_PAREN_VAL,LIST_COND_MODE,EXTCONDLIST_MODE,ASSIGN_MODE,NETWORK_MODE,CHANGE_PROFILE_MODE,RLIMIT_MODE,MOUNT_MODE,DBUS_MODE,SIGNAL_MODE,PTRACE_MODE,UNIX_MODE>{
{WS}+ { DUMP_PREPROCESS; /* Ignoring whitespace */ }
}
@@ -329,6 +330,14 @@ LT_EQUAL <=
}
}
+<SUB_ID_WS>{
+ ({IDS}|{QUOTED_ID}) {
+ /* Go into separate state to match generic ID strings */
+ yylval.id = processid(yytext, yyleng);
+ POP_AND_RETURN(TOK_ID);
+ }
+}
+
<SUB_VALUE>{
({IDS}|{QUOTED_ID}) {
/* Go into separate state to match generic VALUE strings */
@@ -439,7 +448,16 @@ LT_EQUAL <=
}
<CHANGE_PROFILE_MODE>{
- {ARROW} { RETURN_TOKEN(TOK_ARROW); }
+ safe { RETURN_TOKEN(TOK_SAFE); }
+ unsafe { RETURN_TOKEN(TOK_UNSAFE); }
+
+ {ARROW} {
+ /**
+ * Push state so that we can return TOK_ID even when the
+ * change_profile target is 'safe' or 'unsafe'.
+ */
+ PUSH_AND_RETURN(SUB_ID_WS, TOK_ARROW);
+ }
({IDS}|{QUOTED_ID}) {
yylval.id = processid(yytext, yyleng);
@@ -632,7 +650,7 @@ include/{WS} {
}
}
-<INITIAL,SUB_ID,SUB_VALUE,LIST_VAL_MODE,EXTCOND_MODE,LIST_COND_VAL,LIST_COND_PAREN_VAL,LIST_COND_MODE,EXTCONDLIST_MODE,ASSIGN_MODE,NETWORK_MODE,CHANGE_PROFILE_MODE,MOUNT_MODE,DBUS_MODE,SIGNAL_MODE,PTRACE_MODE,UNIX_MODE>{
+<INITIAL,SUB_ID,SUB_ID_WS,SUB_VALUE,LIST_VAL_MODE,EXTCOND_MODE,LIST_COND_VAL,LIST_COND_PAREN_VAL,LIST_COND_MODE,EXTCONDLIST_MODE,ASSIGN_MODE,NETWORK_MODE,CHANGE_PROFILE_MODE,MOUNT_MODE,DBUS_MODE,SIGNAL_MODE,PTRACE_MODE,UNIX_MODE>{
[^\n] {
DUMP_PREPROCESS;
/* Something we didn't expect */
@@ -647,6 +665,7 @@ include/{WS} {
unordered_map<int, string> state_names = {
STATE_TABLE_ENT(INITIAL),
STATE_TABLE_ENT(SUB_ID),
+ STATE_TABLE_ENT(SUB_ID_WS),
STATE_TABLE_ENT(SUB_VALUE),
STATE_TABLE_ENT(EXTCOND_MODE),
STATE_TABLE_ENT(EXTCONDLIST_MODE),
diff --git a/parser/parser_regex.c b/parser/parser_regex.c
index 8cc08c6..bdc3a58 100644
--- a/parser/parser_regex.c
+++ b/parser/parser_regex.c
@@ -494,6 +494,23 @@ static int process_profile_name_xmatch(Profile *prof)
static int warn_change_profile = 1;
+static bool is_change_profile_mode(int mode)
+{
+ /**
+ * A change_profile entry will have the AA_CHANGE_PROFILE bit set.
+ * It could also have the (AA_EXEC_BITS | ALL_AA_EXEC_UNSAFE) bits
+ * set by the frontend parser. That means that it is incorrect to
+ * identify change_profile modes using a test like this:
+ *
+ * (mode & ~AA_CHANGE_PROFILE)
+ *
+ * The above test would incorrectly return true on a
+ * change_profile mode that has the
+ * (AA_EXEC_BITS | ALL_AA_EXEC_UNSAFE) bits set.
+ */
+ return mode & AA_CHANGE_PROFILE;
+}
+
static int process_dfa_entry(aare_rules *dfarules, struct cod_entry *entry)
{
std::string tbuf;
@@ -504,7 +521,7 @@ static int process_dfa_entry(aare_rules *dfarules, struct cod_entry *entry)
return TRUE;
- if (entry->mode & ~AA_CHANGE_PROFILE)
+ if (!is_change_profile_mode(entry->mode))
filter_slashes(entry->name);
ptype = convert_aaregex_to_pcre(entry->name, 0, glob_default, tbuf, &pos);
if (ptype == ePatternInvalid)
@@ -530,13 +547,14 @@ static int process_dfa_entry(aare_rules *dfarules, struct cod_entry *entry)
* TODO: split link and change_profile entries earlier
*/
if (entry->deny) {
- if ((entry->mode & ~(AA_LINK_BITS | AA_CHANGE_PROFILE)) &&
+ if ((entry->mode & ~AA_LINK_BITS) &&
+ !is_change_profile_mode(entry->mode) &&
!dfarules->add_rule(tbuf.c_str(), entry->deny,
entry->mode & ~(AA_LINK_BITS | AA_CHANGE_PROFILE),
entry->audit & ~(AA_LINK_BITS | AA_CHANGE_PROFILE),
dfaflags))
return FALSE;
- } else if (entry->mode & ~AA_CHANGE_PROFILE) {
+ } else if (!is_change_profile_mode(entry->mode)) {
if (!dfarules->add_rule(tbuf.c_str(), entry->deny, entry->mode,
entry->audit, dfaflags))
return FALSE;
@@ -563,12 +581,13 @@ static int process_dfa_entry(aare_rules *dfarules, struct cod_entry *entry)
if (!dfarules->add_rule_vec(entry->deny, perms, entry->audit & AA_LINK_BITS, 2, vec, dfaflags))
return FALSE;
}
- if (entry->mode & AA_CHANGE_PROFILE) {
+ if (is_change_profile_mode(entry->mode)) {
const char *vec[3];
std::string lbuf, xbuf;
autofree char *ns = NULL;
autofree char *name = NULL;
int index = 1;
+ uint32_t onexec_perms = AA_ONEXEC;
if ((warnflags & WARN_RULE_DOWNGRADED) && entry->audit && warn_change_profile) {
/* don't have profile name here, so until this code
@@ -610,12 +629,23 @@ static int process_dfa_entry(aare_rules *dfarules, struct cod_entry *entry)
}
/* regular change_profile rule */
- if (!dfarules->add_rule_vec(entry->deny, AA_CHANGE_PROFILE | AA_ONEXEC, 0, index - 1, &vec[1], dfaflags))
+ if (!dfarules->add_rule_vec(entry->deny,
+ AA_CHANGE_PROFILE | onexec_perms,
+ 0, index - 1, &vec[1], dfaflags))
return FALSE;
+
/* onexec rules - both rules are needed for onexec */
- if (!dfarules->add_rule_vec(entry->deny, AA_ONEXEC, 0, 1, vec, dfaflags))
+ if (!dfarules->add_rule_vec(entry->deny, onexec_perms,
+ 0, 1, vec, dfaflags))
return FALSE;
- if (!dfarules->add_rule_vec(entry->deny, AA_ONEXEC, 0, index, vec, dfaflags))
+
+ /**
+ * pick up any exec bits, from the frontend parser, related to
+ * unsafe exec transitions
+ */
+ onexec_perms |= (entry->mode & (AA_EXEC_BITS | ALL_AA_EXEC_UNSAFE));
+ if (!dfarules->add_rule_vec(entry->deny, onexec_perms,
+ 0, index, vec, dfaflags))
return FALSE;
}
return TRUE;
diff --git a/parser/parser_yacc.y b/parser/parser_yacc.y
index 91c6d68..bb40f09 100644
--- a/parser/parser_yacc.y
+++ b/parser/parser_yacc.y
@@ -1474,11 +1474,31 @@ file_mode: TOK_MODE
free($1);
}
-change_profile: TOK_CHANGE_PROFILE opt_id opt_named_transition TOK_END_OF_RULE
+change_profile: TOK_CHANGE_PROFILE opt_unsafe opt_id opt_named_transition TOK_END_OF_RULE
{
struct cod_entry *entry;
- char *exec = $2;
- char *target = $3;
+ int mode = AA_CHANGE_PROFILE;
+ int exec_mode = $2;
+ char *exec = $3;
+ char *target = $4;
+
+ if (exec_mode) {
+ if (!exec)
+ yyerror(_("Exec condition is required when unsafe or safe keywords are present"));
+
+ if (exec_mode == 1) {
+ mode |= (AA_EXEC_BITS | ALL_AA_EXEC_UNSAFE);
+ } else if (exec_mode == 2 &&
+ !kernel_supports_stacking &&
+ warnflags & WARN_RULE_DOWNGRADED) {
+ pwarn("downgrading change_profile safe rule to unsafe due to lack of necessary kernel support\n");
+ /**
+ * No need to do anything because the 'unsafe'
+ * variant is the only supported type of
+ * change_profile rules in non-stacking kernels
+ */
+ }
+ }
if (exec && !(exec[0] == '/' || strncmp(exec, "@{", 2) == 0))
yyerror(_("Exec condition must begin with '/'."));
@@ -1492,7 +1512,7 @@ change_profile: TOK_CHANGE_PROFILE opt_id opt_named_transition TOK_END_OF_RULE
yyerror(_("Memory allocation error."));
}
- entry = new_entry(target, AA_CHANGE_PROFILE, exec);
+ entry = new_entry(target, mode, exec);
if (!entry)
yyerror(_("Memory allocation error."));
--
2.7.4
More information about the AppArmor
mailing list