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: change priority so that it accumulates based on permissions

Browse source 
The current behavior of priority rules can be non-intuitive with
higher priority rules completely overriding lower priority rules even in
permissions not held in common. This behavior does have use cases but
its can be very confusing, and does not normal policy behavior

Eg.
  priority=0 allow r /**,
  priority=1 deny  w /**,

will result in no allowed permissions even though the deny rule is
only removing the w permission, beause the higher priority rule
completely over ride lower priority permissions sets (including
none shared permissions).

Instead move to tracking the priority at a per permission level. This
allows the w permission to still override at priority 1, while the
read permission is allowed at priority 0.

The final constructed state will still drop priority for the final
permission set on the state.

Note: this patch updates the equality tests for the cases where
the complete override behavior was being tested for.

The complete override behavior will be reintroduced in a future
patch with a keyword extension, enabling that behavior to be used
for ordered blocks etc.

Signed-off-by: John Johansen <john.johansen@canonical.com>
(cherry picked from commit 1ebd991155)
Signed-off-by: John Johansen <john.johansen@canonical.com>
This commit is contained in:
John Johansen 2024-12-24 01:27:10 -08:00
parent 6e37bc0067
commit 2ab1941d9d
6 changed files with 200 additions and 76 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
parser/common_optarg.c
View file

@ -49,6 +49,7 @@ optflag_table_t dumpflag_table[] = {
{ 1, "dfa-states-post-filter", "Dump dfa state immediately after filtering deny", DUMP_DFA_STATES_POST_FILTER },
{ 1, "dfa-states-post-minimize", "Dump dfa state immediately after initial build", DUMP_DFA_STATES_POST_MINIMIZE },
{ 1, "dfa-states-post-unreachable", "Dump dfa state immediately after filtering deny", DUMP_DFA_STATES_POST_UNREACHABLE },
{ 1, "dfa-perms-build", "Dump permission being built from accept node", DUMP_DFA_PERMS },
{ 1, "dfa-graph", "Dump dfa dot (graphviz) graph", DUMP_DFA_GRAPH },
{ 1, "dfa-minimize", "Dump dfa minimization", DUMP_DFA_MINIMIZE },
{ 1, "dfa-unreachable", "Dump dfa unreachable states",

6
parser/immunix.h
View file

@ -95,6 +95,12 @@
#define ALL_USER_EXEC (AA_USER_EXEC | AA_USER_EXEC_TYPE)
#define ALL_OTHER_EXEC (AA_OTHER_EXEC | AA_OTHER_EXEC_TYPE)
#define AA_USER_EXEC_INHERIT (AA_EXEC_INHERIT << AA_USER_SHIFT)
#define AA_OTHER_EXEC_INHERIT (AA_EXEC_INHERIT << AA_OTHER_SHIFT)
#define AA_USER_EXEC_MMAP (AA_OLD_EXEC_MMAP << AA_USER_SHIFT)
#define AA_OTHER_EXEC_MMAP (AA_OLD_EXEC_MMAP << AA_OTHER_SHIFT)
#define AA_LINK_BITS ((AA_OLD_MAY_LINK << AA_USER_SHIFT) | \
(AA_OLD_MAY_LINK << AA_OTHER_SHIFT))

1
parser/libapparmor_re/apparmor_re.h
View file

@ -65,5 +65,6 @@
#define DUMP_DFA_STATES_POST_MINIMIZE (1 << 27)
#define DUMP_DFA_STATES_POST_UNREACHABLE (1 << 28)
#define DUMP_DFA_COMPTRESSED_STATES (1 << 29)
#define DUMP_DFA_PERMS (1 << 30)
#endif /* APPARMOR_RE_H */

237
parser/libapparmor_re/hfa.cc
View file

@ -317,7 +317,8 @@ static void split_node_types(NodeSet *nodes, NodeSet **anodes, NodeSet **nnodes
*nnodes = nodes;
}
State *DFA::add_new_state(NodeSet *anodes, NodeSet *nnodes, State *other)
State *DFA::add_new_state(optflags const &opts, NodeSet *anodes,
NodeSet *nnodes, State *other)
{
NodeVec *nnodev, *anodev;
nnodev = nnodes_cache.insert(nnodes);
@ -325,7 +326,7 @@ State *DFA::add_new_state(NodeSet *anodes, NodeSet *nnodes, State *other)
ProtoState proto;
proto.init(nnodev, anodev);
State *state = new State(node_map.size(), proto, other, filedfa);
State *state = new State(opts, node_map.size(), proto, other, filedfa);
pair<NodeMap::iterator,bool> x = node_map.insert(proto, state);
if (x.second == false) {
delete state;
@ -337,7 +338,7 @@ State *DFA::add_new_state(NodeSet *anodes, NodeSet *nnodes, State *other)
return x.first->second;
}
State *DFA::add_new_state(NodeSet *nodes, State *other)
State *DFA::add_new_state(optflags const &opts, NodeSet *nodes, State *other)
{
/* The splitting of nodes should probably get pushed down into
* follow(), ie. put in separate lists from the start
@ -345,12 +346,12 @@ State *DFA::add_new_state(NodeSet *nodes, State *other)
NodeSet *anodes, *nnodes;
split_node_types(nodes, &anodes, &nnodes);
State *state = add_new_state(anodes, nnodes, other);
State *state = add_new_state(opts, anodes, nnodes, other);
return state;
}
void DFA::update_state_transitions(State *state)
void DFA::update_state_transitions(optflags const &opts, State *state)
{
/* Compute possible transitions for state->nodes. This is done by
* iterating over all the nodes in state->nodes and combining the
@ -373,7 +374,8 @@ void DFA::update_state_transitions(State *state)
/* check the default transition first */
if (cases.otherwise)
state->otherwise = add_new_state(cases.otherwise, nonmatching);
state->otherwise = add_new_state(opts, cases.otherwise,
nonmatching);
else
state->otherwise = nonmatching;
@ -382,7 +384,7 @@ void DFA::update_state_transitions(State *state)
*/
for (Cases::iterator j = cases.begin(); j != cases.end(); j++) {
State *target;
target = add_new_state(j->second, nonmatching);
target = add_new_state(opts, j->second, nonmatching);
/* Don't insert transition that the otherwise transition
* already covers
@ -429,7 +431,7 @@ void DFA::process_work_queue(const char *header, optflags const &opts)
/* Update 'from's transitions, and if it transitions to any
* unknown State create it and add it to the work_queue
*/
update_state_transitions(from);
update_state_transitions(opts, from);
} /* while (!work_queue.empty()) */
}
@ -459,8 +461,8 @@ DFA::DFA(Node *root, optflags const &opts, bool buildfiledfa): root(root), filed
(*i)->compute_followpos();
}
nonmatching = add_new_state(new NodeSet, NULL);
start = add_new_state(new NodeSet(root->firstpos), nonmatching);
nonmatching = add_new_state(opts, new NodeSet, NULL);
start = add_new_state(opts, new NodeSet(root->firstpos), nonmatching);
/* the work_queue contains the states that need to have their
* transitions computed. This could be done with a recursive
@ -1384,85 +1386,184 @@ static inline int diff_qualifiers(perm32_t perm1, perm32_t perm2)
(perm1 & AA_EXEC_TYPE) != (perm2 & AA_EXEC_TYPE));
}
/* update a single permission based on priority - only called if match->perm | match-> audit bit set */
static int pri_update_perm(optflags const &opts, vector<int> &priority, int i,
MatchFlag *match, perms_t &perms, perms_t &exact,
bool filedfa)
{
if (priority[i] > match->priority) {
if (opts.dump & DUMP_DFA_PERMS)
cerr << " " << match << "[" << i << "]=" << priority[i] << " > " << match->priority << " SKIPPING " << hex << (match->perms) << "/" << (match->audit) << dec << "\n";
return 0;
}
perm32_t xmask = 0;
perm32_t mask = 1 << i;
perm32_t amask = mask;
// drop once we move the xindex out of the perms in the front end
if (filedfa) {
if (mask & AA_USER_EXEC) {
xmask = AA_USER_EXEC_TYPE;
// ix implies EXEC_MMAP
if (match->perms & AA_EXEC_INHERIT) {
xmask |= AA_USER_EXEC_MMAP;
//USER_EXEC_MAP = 6
if (priority[6] < match->priority)
priority[6] = match->priority;
}
amask = mask | xmask;
} else if (mask & AA_OTHER_EXEC) {
xmask = AA_OTHER_EXEC_TYPE;
// ix implies EXEC_MMAP
if (match->perms & AA_OTHER_EXEC_INHERIT) {
xmask |= AA_OTHER_EXEC_MMAP;
//OTHER_EXEC_MAP = 20
if (priority[20] < match->priority)
priority[20] = match->priority;
}
amask = mask | xmask;
} else if (((mask & AA_USER_EXEC_MMAP) &&
(match->perms & AA_USER_EXEC_INHERIT)) ||
((mask & AA_OTHER_EXEC_MMAP) &&
(match->perms & AA_OTHER_EXEC_INHERIT))) {
// if exec && ix we handled mmp above
if (opts.dump & DUMP_DFA_PERMS)
cerr << " " << match << "[" << i << "]=" << priority[i] << " <= " << match->priority << " SKIPPING mmap unmasked " << hex << match->perms << "/" << match->audit << " masked " << (match->perms & amask) << "/" << (match->audit & amask) << " data " << (perms.allow & mask) << "/" << (perms.audit & mask) << " exact " << (exact.allow & mask) << "/" << (exact.audit & mask) << dec << "\n";
return 0;
}
}
if (opts.dump & DUMP_DFA_PERMS)
cerr << " " << match << "[" << i << "]=" << priority[i] << " vs. " << match->priority << " mask: " << hex << mask << " xmask: " << xmask << " amask: " << amask << dec << "\n";
if (priority[i] < match->priority) {
if (opts.dump & DUMP_DFA_PERMS)
cerr << " " << match << "[" << i << "]=" << priority[i] << " < " << match->priority << " clearing " << hex << (perms.allow & amask) << "/" << (perms.audit & amask) << " -> " << dec;
priority[i] = match->priority;
perms.clear_bits(amask);
exact.clear_bits(amask);
if (opts.dump & DUMP_DFA_PERMS)
cerr << hex << (perms.allow & amask) << "/" << (perms.audit & amask) << dec << "\n";
}
// the if conditions in order of permission priority
if (match->is_type(NODE_TYPE_DENYMATCHFLAG)) {
if (opts.dump & DUMP_DFA_PERMS)
cerr << " " << match << "[" << i << "]=" << priority[i] << " <= " << match->priority << " deny " << hex << (match->perms & amask) << "/" << (match->audit & amask) << dec << "\n";
perms.deny |= match->perms & amask;
perms.quiet |= match->audit & amask;
perms.allow &= ~amask;
perms.audit &= ~amask;
perms.prompt &= ~amask;
} else if (match->is_type(NODE_TYPE_EXACTMATCHFLAG)) {
/* exact match only asserts dominance on the XTYPE */
if (opts.dump & DUMP_DFA_PERMS)
cerr << " " << match << "[" << i << "]=" << priority[i] << " <= " << match->priority << " exact " << hex << (match->perms & amask) << "/" << (match->audit & amask) << dec << "\n";
if (filedfa &&
!is_merged_x_consistent(exact.allow, match->perms & amask)) {
if (opts.dump & DUMP_DFA_PERMS)
cerr << " " << match << "[" << i << "]=" << priority[i] << " <= " << match->priority << " exact match conflict" << "\n";
return 1;
}
exact.allow |= match->perms & amask;
exact.audit |= match->audit & amask;
// dominance is only done for XTYPE so only clear that
// note xmask only set if setting x perm bit, so this
// won't clear for other bit types
perms.allow &= ~xmask;
perms.audit &= ~xmask;
perms.prompt &= ~xmask;
perms.allow |= match->perms & amask;
perms.audit |= match->audit & amask;
// can't specify exact prompt atm
} else if (!match->is_type(NODE_TYPE_PROMPTMATCHFLAG)) {
// allow perms, if exact has been encountered will already be set
// if overlaps x here, don't conflict, because exact will override
if (opts.dump & DUMP_DFA_PERMS)
cerr << " " << match << "[" << i << "]=" << priority[i] << " <= " << match->priority << " allow " << hex << (match->perms & amask) << "/" << (match->audit & amask) << dec << "\n";
if (filedfa && !(exact.allow & mask) &&
!is_merged_x_consistent(perms.allow, match->perms & amask)) {
if (opts.dump & DUMP_DFA_PERMS)
cerr << " " << match << "[" << i << "]=" << priority[i] << " <= " << match->priority << " allow match conflict" << "\n";
return 1;
}
// mask off if XTYPE in xmatch
if ((exact.allow | exact.audit) & mask) {
// mask == amask & ~xmask
perms.allow |= match->perms & mask;
perms.audit |= match->audit & mask;
} else {
perms.allow |= match->perms & amask;
perms.audit |= match->audit & amask;
}
} else { // if (match->is_type(NODE_TYPE_PROMPTMATCHFLAG)) {
if (opts.dump & DUMP_DFA_PERMS)
cerr << " " << match << "[" << i << "]=" << priority[i] << " <= " << match->priority << " promt " << hex << (match->perms & amask) << "/" << (match->audit & amask) << dec << "\n";
if (filedfa && !((exact.allow | perms.allow) & mask) &&
!is_merged_x_consistent(perms.allow, match->perms & amask)) {
if (opts.dump & DUMP_DFA_PERMS)
cerr << " " << match << "[" << i << "]=" << priority[i] << " <= " << match->priority << " prompt match conflict" << "\n";
return 1;
}
if ((exact.allow | exact.audit | perms.allow | perms.audit) & mask) {
// mask == amask & ~xmask
perms.prompt |= match->perms & mask;
perms.audit |= match->audit & mask;
} else {
perms.prompt |= match->perms & amask;
perms.audit |= match->audit & amask;
}
}
return 0;
}
/**
* Compute the permission flags that this state corresponds to. If we
* have any exact matches, then they override the execute and safe
* execute flags.
*/
int accept_perms(NodeVec *state, perms_t &perms, bool filedfa)
int accept_perms(optflags const &opts, NodeVec *state, perms_t &perms,
bool filedfa)
{
int error = 0;
perms_t exact;
int priority = MIN_INTERNAL_PRIORITY;
std::vector<int> priority(sizeof(perm32_t)*8, MIN_INTERNAL_PRIORITY); // 32 but wan't tied to perm32_t
perms.clear();
if (!state)
return error;
if (opts.dump & DUMP_DFA_PERMS)
cerr << "Building\n";
for (NodeVec::iterator i = state->begin(); i != state->end(); i++) {
if (!(*i)->is_type(NODE_TYPE_MATCHFLAG))
continue;
MatchFlag *match = static_cast<MatchFlag *>(*i);
if (priority > match->priority)
continue;
if (priority < match->priority) {
priority = match->priority;
perms.clear();
exact.clear();
}
if (match->is_type(NODE_TYPE_EXACTMATCHFLAG)) {
/* exact match only ever happens with x */
if (filedfa &&
!is_merged_x_consistent(exact.allow, match->perms))
error = 1;
exact.allow |= match->perms;
exact.audit |= match->audit;
} else if (match->is_type(NODE_TYPE_DENYMATCHFLAG)) {
perms.deny |= match->perms;
perms.quiet |= match->audit;
} else if (match->is_type(NODE_TYPE_PROMPTMATCHFLAG)) {
perms.prompt |= match->perms;
perms.audit |= match->audit;
} else {
if (filedfa &&
!is_merged_x_consistent(perms.allow, match->perms))
error = 1;
perms.allow |= match->perms;
perms.audit |= match->audit;
perm32_t bit = 1;
perm32_t check = match->perms | match->audit;
if (filedfa)
check &= ~ALL_AA_EXEC_TYPE;
for (int i = 0; check; i++) {
if (check & bit) {
error = pri_update_perm(opts, priority, i, match, perms, exact, filedfa);
if (error)
goto out;
}
check &= ~bit;
bit <<= 1;
}
}
if (filedfa) {
perms.allow |= exact.allow & ~(ALL_AA_EXEC_TYPE);
perms.prompt |= exact.prompt & ~(ALL_AA_EXEC_TYPE);
perms.audit |= exact.audit & ~(ALL_AA_EXEC_TYPE);
} else {
perms.allow |= exact.allow;
perms.prompt |= exact.prompt;
perms.audit |= exact.audit;
if (opts.dump & DUMP_DFA_PERMS) {
cerr << " computed: "; perms.dump(cerr); cerr << "\n";
}
if (exact.allow & AA_USER_EXEC) {
perms.allow = (exact.allow & AA_USER_EXEC_TYPE) |
(perms.allow & ~AA_USER_EXEC_TYPE);
perms.exact = AA_USER_EXEC_TYPE;
}
if (exact.allow & AA_OTHER_EXEC) {
perms.allow = (exact.allow & AA_OTHER_EXEC_TYPE) |
(perms.allow & ~AA_OTHER_EXEC_TYPE);
perms.exact |= AA_OTHER_EXEC_TYPE;
}
if (filedfa && (AA_USER_EXEC & perms.deny))
perms.deny |= AA_USER_EXEC_TYPE;
if (filedfa && (AA_OTHER_EXEC & perms.deny))
perms.deny |= AA_OTHER_EXEC_TYPE;
perms.allow &= ~perms.deny;
perms.quiet &= perms.deny;
perms.prompt &= ~perms.deny;
perms.prompt &= ~perms.allow;
out:
if (error)
fprintf(stderr, "profile has merged rule with conflicting x modifiers\n");

27
parser/libapparmor_re/hfa.h
View file

@ -70,6 +70,17 @@ public:
void clear(void) {
allow = deny = prompt = audit = quiet = exact = 0;
}
void clear_bits(perm32_t bits)
{
allow &= ~bits;
deny &= ~bits;
prompt &= ~bits;
audit &= ~bits;
quiet &= ~bits;
exact &= ~bits;
}
void add(perms_t &rhs, bool filedfa)
{
deny |= rhs.deny;
@ -159,7 +170,8 @@ public:
perm32_t allow, deny, prompt, audit, quiet, exact;
};
int accept_perms(NodeVec *state, perms_t &perms, bool filedfa);
int accept_perms(optflags const &opts, NodeVec *state, perms_t &perms,
bool filedfa);
/*
* ProtoState - NodeSet and ancillery information used to create a state
@ -223,7 +235,8 @@ struct DiffDag {
*/
class State {
public:
State(int l, ProtoState &n, State *other, bool filedfa):
State(optflags const &opts, int l, ProtoState &n, State *other,
bool filedfa):
label(l), flags(0), idx(0), perms(), trans()
{
int error;
@ -236,7 +249,7 @@ public:
proto = n;
/* Compute permissions associated with the State. */
error = accept_perms(n.anodes, perms, filedfa);
error = accept_perms(opts, n.anodes, perms, filedfa);
if (error) {
//cerr << "Failing on accept perms " << error << "\n";
throw error;
@ -340,9 +353,11 @@ typedef map<const State *, size_t> Renumber_Map;
/* Transitions in the DFA. */
class DFA {
void dump_node_to_dfa(void);
State *add_new_state(NodeSet *nodes, State *other);
State *add_new_state(NodeSet *anodes, NodeSet *nnodes, State *other);
void update_state_transitions(State *state);
State *add_new_state(optflags const &opts, NodeSet *nodes,
State *other);
State *add_new_state(optflags const &opts,NodeSet *anodes,
NodeSet *nnodes, State *other);
void update_state_transitions(optflags const &opts, State *state);
void process_work_queue(const char *header, optflags const &);
void dump_diff_chain(ostream &os, map<State *, Partition> &relmap,
Partition &chain, State *state,

4
parser/tst/equality.sh
View file

@ -725,7 +725,7 @@ else
#this one may not be true in the future depending on if the compiled profile
#is explicitly including deny permissions for dynamic composition
verify_binary_inequality "'$p1'x'$p2' Deny of ungranted perm" \
verify_binary_equality "'$p1'x'$p2' Deny of ungranted perm" \
"/t { $p1 /foo/[abc] r, audit deny /foo/b w, }" \
"/t { $p2 /foo/[abc] r, }"
fi
@ -823,7 +823,7 @@ if { priority_lt "$p1" "" && priority_lt "$p2" "" ; } ||
"/t { $p1 owner /proc/[0-9]*/attr/{apparmor/,}current a, ^test { $p1 owner /proc/[0-9]*/attr/{apparmor/,}current a, /f r, }}" \
"/t { $p2 owner /proc/[0-9]*/attr/{apparmor/,}current w, ^test { $p2 owner /proc/[0-9]*/attr/{apparmor/,}current w, /f r, }}"
else
verify_binary_inequality "'$p1'x'$p2' change_hat rules automatically inserted"\
verify_binary_equality "'$p1'x'$p2' change_hat rules automatically inserted"\
"/t { $p1 owner /proc/[0-9]*/attr/{apparmor/,}current a, ^test { $p1 owner /proc/[0-9]*/attr/{apparmor/,}current a, /f r, }}" \
"/t { $p2 owner /proc/[0-9]*/attr/{apparmor/,}current w, ^test { $p2 owner /proc/[0-9]*/attr/{apparmor/,}current w, /f r, }}"
fi