mirrors/apparmor
1
0
Fork 0
mirror of https://gitlab.com/apparmor/apparmor.git synced 2025-03-04 08:24:42 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

parser: Allow change_profile rules to accept an exec mode modifier

Browse source 
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@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
This commit is contained in:
Tyler Hicks 2016-05-31 15:32:08 -05:00
parent d5322575f5
commit 0c4c975509
3 changed files with 83 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

25
parser/parser_lex.l
View file

@ -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),

44
parser/parser_regex.c
View file

@ -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;

28
parser/parser_yacc.y
View file

@ -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."));