This cleans things up a bit and fixes a bug where not all rules are
getting properly counted so that the addition of policy_mediation
rules fails to generate the policy dfa in some cases.
Because the policy dfa is being generated correctly now we need to
fix some tests to use the new -M flag to specify the expected features
set of the test.
Signed-off-by: John Johansen <john.johan...@canonical.com>
---
parser/dbus.c | 24 ++++------
parser/libapparmor_re/aare_rules.cc | 80 +++++++++++-------------------------
parser/libapparmor_re/aare_rules.h | 34 +++++++--------
parser/mount.c | 47 +++++++++------------
parser/parser_regex.c | 79 +++++++++++++++--------------------
parser/profile.cc | 4 -
parser/profile.h | 6 +-
7 files changed, 114 insertions(+), 160 deletions(-)
--- 2.9-test.orig/parser/dbus.c
+++ 2.9-test/parser/dbus.c
@@ -284,28 +284,26 @@
}
if (mode & AA_DBUS_BIND) {
- if (!aare_add_rule_vec(prof.policy.rules, deny,
- mode & AA_DBUS_BIND,
- audit & AA_DBUS_BIND,
- 2, vec, dfaflags))
+ if (!prof.policy.rules->add_rule_vec(deny, mode & AA_DBUS_BIND,
+ audit & AA_DBUS_BIND,
+ 2, vec, dfaflags))
goto fail;
}
if (mode & (AA_DBUS_SEND | AA_DBUS_RECEIVE)) {
- if (!aare_add_rule_vec(prof.policy.rules, deny,
- mode & (AA_DBUS_SEND | AA_DBUS_RECEIVE),
- audit & (AA_DBUS_SEND | AA_DBUS_RECEIVE),
- 6, vec, dfaflags))
+ if (!prof.policy.rules->add_rule_vec(deny,
+ mode & (AA_DBUS_SEND | AA_DBUS_RECEIVE),
+ audit & (AA_DBUS_SEND | AA_DBUS_RECEIVE),
+ 6, vec, dfaflags))
goto fail;
}
if (mode & AA_DBUS_EAVESDROP) {
- if (!aare_add_rule_vec(prof.policy.rules, deny,
- mode & AA_DBUS_EAVESDROP,
- audit & AA_DBUS_EAVESDROP,
- 1, vec, dfaflags))
+ if (!prof.policy.rules->add_rule_vec(deny,
+ mode & AA_DBUS_EAVESDROP,
+ audit & AA_DBUS_EAVESDROP,
+ 1, vec, dfaflags))
goto fail;
}
- prof.policy.count++;
return RULE_OK;
fail:
--- 2.9-test.orig/parser/libapparmor_re/aare_rules.cc
+++ 2.9-test/parser/libapparmor_re/aare_rules.cc
@@ -34,38 +34,20 @@
#include "chfa.h"
#include "../immunix.h"
-struct aare_ruleset {
- int reverse;
- Node *root;
-};
-aare_ruleset_t *aare_new_ruleset(int reverse)
-{
- aare_ruleset_t *container = (aare_ruleset_t *)
malloc(sizeof(aare_ruleset_t));
- if (!container)
- return NULL;
-
- container->root = NULL;
- container->reverse = reverse;
-
- return container;
-}
-void aare_delete_ruleset(aare_ruleset_t *rules)
+aare_rules::~aare_rules(void)
{
- if (rules) {
- if (rules->root)
- rules->root->release();
- free(rules);
- }
+ if (root)
+ root->release();
aare_reset_matchflags();
}
-int aare_add_rule(aare_ruleset_t *rules, const char *rule, int deny,
- uint32_t perms, uint32_t audit, dfaflags_t flags)
+bool aare_rules::add_rule(const char *rule, int deny, uint32_t perms,
+ uint32_t audit, dfaflags_t flags)
{
- return aare_add_rule_vec(rules, deny, perms, audit, 1, &rule, flags);
+ return add_rule_vec(deny, perms, audit, 1, &rule, flags);
}
#define FLAGS_WIDTH 2
@@ -94,9 +76,8 @@
#undef RESET_FLAGS
}
-int aare_add_rule_vec(aare_ruleset_t *rules, int deny,
- uint32_t perms, uint32_t audit,
- int count, const char **rulev, dfaflags_t
flags)
+bool aare_rules::add_rule_vec(int deny, uint32_t perms, uint32_t audit,
+ int count, const char **rulev, dfaflags_t flags)
{
Node *tree = NULL, *accept;
int exact_match;
@@ -105,15 +86,15 @@
assert(perms != 0);
if (regex_parse(&tree, rulev[0]))
- return 0;
+ return false;
for (int i = 1; i < count; i++) {
Node *subtree = NULL;
Node *node = new CharNode(0);
if (!node)
- return 0;
+ return false;
tree = new CatNode(tree, node);
if (regex_parse(&subtree, rulev[i]))
- return 0;
+ return false;
tree = new CatNode(tree, subtree);
}
@@ -132,26 +113,15 @@
exact_match = 0;
}
- if (rules->reverse)
+ if (reverse)
flip_tree(tree);
/* 0x7f == 4 bits x mods + 1 bit unsafe mask + 1 bit ix, + 1 pux after shift */
#define EXTRACT_X_INDEX(perm, shift) (((perm) >> (shift + 7)) & 0x7f)
-//if (perms & ALL_AA_EXEC_TYPE && (!perms & AA_EXEC_BITS))
-// fprintf(stderr, "adding X rule without MAY_EXEC: 0x%x %s\n", perms,
rulev[0]);
-
-//if (perms & ALL_EXEC_TYPE)
-// fprintf(stderr, "adding X rule %s 0x%x\n", rulev[0], perms);
-
-//if (audit)
-//fprintf(stderr, "adding rule with audit bits set: 0x%x %s\n", audit,
rulev[0]);
-
-//if (perms & AA_CHANGE_HAT)
-// fprintf(stderr, "adding change_hat rule %s\n", rulev[0]);
-/* the permissions set is assumed to be non-empty if any audit
- * bits are specified */
+ /* the permissions set is assumed to be non-empty if any audit
+ * bits are specified */
accept = NULL;
for (unsigned int n = 0; perms && n < (sizeof(perms) * 8); n++) {
uint32_t mask = 1 << n;
@@ -230,44 +200,44 @@
cerr << "\n\n";
}
- if (rules->root)
- rules->root = new AltNode(rules->root, new CatNode(tree,
accept));
+ if (root)
+ root = new AltNode(root, new CatNode(tree, accept));
else
- rules->root = new CatNode(tree, accept);
+ root = new CatNode(tree, accept);
- return 1;
+ rule_count++;
+ return true;
}
/* create a dfa from the ruleset
* returns: buffer contain dfa tables, @size set to the size of the tables
* else NULL on failure
*/
-void *aare_create_dfa(aare_ruleset_t *rules, size_t *size,
- dfaflags_t flags)
+void *aare_rules::create_dfa(size_t *size, dfaflags_t flags)
{
char *buffer = NULL;
- label_nodes(rules->root);
+ label_nodes(root);
if (flags & DFA_DUMP_TREE) {
cerr << "\nDFA: Expression Tree\n";
- rules->root->dump(cerr);
+ root->dump(cerr);
cerr << "\n\n";
}
if (flags & DFA_CONTROL_TREE_SIMPLE) {
- rules->root = simplify_tree(rules->root, flags);
+ root = simplify_tree(root, flags);
if (flags & DFA_DUMP_SIMPLE_TREE) {
cerr << "\nDFA: Simplified Expression Tree\n";
- rules->root->dump(cerr);
+ root->dump(cerr);
cerr << "\n\n";
}
}
stringstream stream;
try {
- DFA dfa(rules->root, flags);
+ DFA dfa(root, flags);
if (flags & DFA_DUMP_UNIQ_PERMS)
dfa.dump_uniq_perms("dfa");
--- 2.9-test.orig/parser/libapparmor_re/aare_rules.h
+++ 2.9-test/parser/libapparmor_re/aare_rules.h
@@ -24,26 +24,24 @@
#include <stdint.h>
#include "apparmor_re.h"
+#include "expr-tree.h"
-#ifdef __cplusplus
-extern "C" {
-#endif
+class aare_rules {
+ Node *root;
+public:
+ int reverse;
+ int rule_count;
+ aare_rules(): root(NULL), reverse(0), rule_count(0) { };
+ aare_rules(int reverse): root(NULL), reverse(reverse), rule_count(0) {
};
+ ~aare_rules();
+
+ bool add_rule(const char *rule, int deny, uint32_t perms,
+ uint32_t audit, dfaflags_t flags);
+ bool add_rule_vec(int deny, uint32_t perms, uint32_t audit, int count,
+ const char **rulev, dfaflags_t flags);
+ void *create_dfa(size_t *size, dfaflags_t flags);
+};
-struct aare_ruleset;
-
-typedef struct aare_ruleset aare_ruleset_t;
-
-aare_ruleset_t *aare_new_ruleset(int reverse);
-void aare_delete_ruleset(aare_ruleset_t *rules);
-int aare_add_rule(aare_ruleset_t *rules, const char *rule, int deny, uint32_t
perms,
- uint32_t audit, dfaflags_t flags);
-int aare_add_rule_vec(aare_ruleset_t *rules, int deny, uint32_t perms,
- uint32_t audit, int count, const char **rulev,
- dfaflags_t flags);
-void *aare_create_dfa(aare_ruleset_t *rules, size_t *size, dfaflags_t flags);
void aare_reset_matchflags(void);
-#ifdef __cplusplus
-}
-#endif
#endif /* __LIBAA_RE_RULES_H */
--- 2.9-test.orig/parser/mount.c
+++ 2.9-test/parser/mount.c
@@ -614,9 +614,9 @@
tmpallow = allow;
/* rule for match without required data || data MATCH_CONT */
- if (!aare_add_rule_vec(prof.policy.rules, deny, tmpallow,
- audit | AA_AUDIT_MNT_DATA, 4,
- vec, dfaflags))
+ if (!prof.policy.rules->add_rule_vec(deny, tmpallow,
+ audit | AA_AUDIT_MNT_DATA, 4,
+ vec, dfaflags))
goto fail;
count++;
@@ -626,10 +626,9 @@
if (!build_mnt_opts(optsbuf, opts))
goto fail;
vec[4] = optsbuf.c_str();
- if (!aare_add_rule_vec(prof.policy.rules, deny,
- allow,
- audit | AA_AUDIT_MNT_DATA,
- 5, vec, dfaflags))
+ if (!prof.policy.rules->add_rule_vec(deny, allow,
+ audit | AA_AUDIT_MNT_DATA,
+ 5, vec, dfaflags))
goto fail;
count++;
}
@@ -657,8 +656,8 @@
if (!build_mnt_flags(flagsbuf, PATH_MAX, tmpflags,
tmpinv_flags))
goto fail;
vec[3] = flagsbuf;
- if (!aare_add_rule_vec(prof.policy.rules, deny, allow,
- audit, 4, vec, dfaflags))
+ if (!prof.policy.rules->add_rule_vec(deny, allow, audit, 4, vec,
+ dfaflags))
goto fail;
count++;
}
@@ -686,8 +685,8 @@
if (!build_mnt_flags(flagsbuf, PATH_MAX, tmpflags,
tmpinv_flags))
goto fail;
vec[3] = flagsbuf;
- if (!aare_add_rule_vec(prof.policy.rules, deny, allow,
- audit, 4, vec, dfaflags))
+ if (!prof.policy.rules->add_rule_vec(deny, allow, audit, 4, vec,
+ dfaflags))
goto fail;
count++;
}
@@ -716,8 +715,8 @@
if (!build_mnt_flags(flagsbuf, PATH_MAX, tmpflags,
tmpinv_flags))
goto fail;
vec[3] = flagsbuf;
- if (!aare_add_rule_vec(prof.policy.rules, deny, allow,
- audit, 4, vec, dfaflags))
+ if (!prof.policy.rules->add_rule_vec(deny, allow, audit, 4, vec,
+ dfaflags))
goto fail;
count++;
}
@@ -756,9 +755,9 @@
tmpallow = allow;
/* rule for match without required data || data MATCH_CONT */
- if (!aare_add_rule_vec(prof.policy.rules, deny, tmpallow,
- audit | AA_AUDIT_MNT_DATA, 4,
- vec, dfaflags))
+ if (!prof.policy.rules->add_rule_vec(deny, tmpallow,
+ audit | AA_AUDIT_MNT_DATA, 4,
+ vec, dfaflags))
goto fail;
count++;
@@ -768,10 +767,9 @@
if (!build_mnt_opts(optsbuf, opts))
goto fail;
vec[4] = optsbuf.c_str();
- if (!aare_add_rule_vec(prof.policy.rules, deny,
- allow,
- audit | AA_AUDIT_MNT_DATA,
- 5, vec, dfaflags))
+ if (!prof.policy.rules->add_rule_vec(deny, allow,
+ audit | AA_AUDIT_MNT_DATA,
+ 5, vec, dfaflags))
goto fail;
count++;
}
@@ -782,8 +780,8 @@
if (!convert_entry(mntbuf, mnt_point))
goto fail;
vec[0] = mntbuf.c_str();
- if (!aare_add_rule_vec(prof.policy.rules, deny, allow,
- audit, 1, vec, dfaflags))
+ if (!prof.policy.rules->add_rule_vec(deny, allow, audit, 1, vec,
+ dfaflags))
goto fail;
count++;
}
@@ -796,8 +794,8 @@
if (!clear_and_convert_entry(devbuf, device))
goto fail;
vec[1] = devbuf.c_str();
- if (!aare_add_rule_vec(prof.policy.rules, deny, allow,
- audit, 2, vec, dfaflags))
+ if (!prof.policy.rules->add_rule_vec(deny, allow, audit, 2, vec,
+ dfaflags))
goto fail;
count++;
}
@@ -806,7 +804,6 @@
/* didn't actually encode anything */
goto fail;
- prof.policy.count++;
return RULE_OK;
fail:
--- 2.9-test.orig/parser/parser_regex.c
+++ 2.9-test/parser/parser_regex.c
@@ -431,11 +431,11 @@
prof->xmatch_size = 0;
} else {
/* build a dfa */
- aare_ruleset_t *rule = aare_new_ruleset(0);
- if (!rule)
+ aare_rules *rules = new aare_rules();
+ if (!rules)
return FALSE;
- if (!aare_add_rule(rule, tbuf.c_str(), 0, AA_MAY_EXEC, 0,
dfaflags)) {
- aare_delete_ruleset(rule);
+ if (!rules->add_rule(tbuf.c_str(), 0, AA_MAY_EXEC, 0,
dfaflags)) {
+ delete rules;
return FALSE;
}
if (prof->altnames) {
@@ -450,15 +450,14 @@
len = strlen(alt->name);
if (len < prof->xmatch_len)
prof->xmatch_len = len;
- if (!aare_add_rule(rule, tbuf.c_str(), 0,
AA_MAY_EXEC, 0, dfaflags)) {
- aare_delete_ruleset(rule);
+ if (!rules->add_rule(tbuf.c_str(), 0,
AA_MAY_EXEC, 0, dfaflags)) {
+ delete rules;
return FALSE;
}
}
}
- prof->xmatch = aare_create_dfa(rule, &prof->xmatch_size,
- dfaflags);
- aare_delete_ruleset(rule);
+ prof->xmatch = rules->create_dfa(&prof->xmatch_size, dfaflags);
+ delete rules;
if (!prof->xmatch)
return FALSE;
}
@@ -466,7 +465,7 @@
return TRUE;
}
-static int process_dfa_entry(aare_ruleset_t *dfarules, struct cod_entry *entry)
+static int process_dfa_entry(aare_rules *dfarules, struct cod_entry *entry)
{
std::string tbuf;
pattern_t ptype;
@@ -498,13 +497,13 @@
* entry of the pair
*/
if (entry->deny && (entry->mode & AA_LINK_BITS)) {
- if (!aare_add_rule(dfarules, tbuf.c_str(), entry->deny,
- entry->mode & ~AA_LINK_BITS,
- entry->audit & ~AA_LINK_BITS, dfaflags))
+ if (!dfarules->add_rule(tbuf.c_str(), entry->deny,
+ entry->mode & ~AA_LINK_BITS,
+ entry->audit & ~AA_LINK_BITS, dfaflags))
return FALSE;
} else if (entry->mode & ~AA_CHANGE_PROFILE) {
- if (!aare_add_rule(dfarules, tbuf.c_str(), entry->deny,
entry->mode,
- entry->audit, dfaflags))
+ if (!dfarules->add_rule(tbuf.c_str(), entry->deny, entry->mode,
+ entry->audit, dfaflags))
return FALSE;
}
@@ -526,7 +525,7 @@
perms |= LINK_TO_LINK_SUBSET(perms);
vec[1] = "/[^/].*";
}
- if (!aare_add_rule_vec(dfarules, entry->deny, perms,
entry->audit & AA_LINK_BITS, 2, vec, dfaflags))
+ if (!dfarules->add_rule_vec(entry->deny, perms, entry->audit &
AA_LINK_BITS, 2, vec, dfaflags))
return FALSE;
}
if (entry->mode & AA_CHANGE_PROFILE) {
@@ -545,12 +544,12 @@
vec[index++] = tbuf.c_str();
/* regular change_profile rule */
- if (!aare_add_rule_vec(dfarules, 0, AA_CHANGE_PROFILE |
AA_ONEXEC, 0, index - 1, &vec[1], dfaflags))
+ if (!dfarules->add_rule_vec(0, AA_CHANGE_PROFILE | AA_ONEXEC,
0, index - 1, &vec[1], dfaflags))
return FALSE;
/* onexec rules - both rules are needed for onexec */
- if (!aare_add_rule_vec(dfarules, 0, AA_ONEXEC, 0, 1, vec,
dfaflags))
+ if (!dfarules->add_rule_vec(0, AA_ONEXEC, 0, 1, vec, dfaflags))
return FALSE;
- if (!aare_add_rule_vec(dfarules, 0, AA_ONEXEC, 0, index, vec,
dfaflags))
+ if (!dfarules->add_rule_vec(0, AA_ONEXEC, 0, index, vec,
dfaflags))
return FALSE;
}
return TRUE;
@@ -560,15 +559,12 @@
{
int ret = TRUE;
struct cod_entry *entry;
- int count = 0;
list_for_each(prof->entries, entry) {
if (!process_dfa_entry(prof->dfa.rules, entry))
ret = FALSE;
- count++;
}
- prof->dfa.count = count;
return ret;
}
@@ -579,17 +575,17 @@
if (!process_profile_name_xmatch(prof))
goto out;
- prof->dfa.rules = aare_new_ruleset(0);
+ prof->dfa.rules = new aare_rules();
if (!prof->dfa.rules)
goto out;
if (!post_process_entries(prof))
goto out;
- if (prof->dfa.count > 0) {
- prof->dfa.dfa = aare_create_dfa(prof->dfa.rules,
&prof->dfa.size,
- dfaflags);
- aare_delete_ruleset(prof->dfa.rules);
+ if (prof->dfa.rules->rule_count > 0) {
+ prof->dfa.dfa = prof->dfa.rules->create_dfa(&prof->dfa.size,
+ dfaflags);
+ delete prof->dfa.rules;
prof->dfa.rules = NULL;
if (!prof->dfa.dfa)
goto out;
@@ -683,7 +679,7 @@
{
int error = -1;
- prof->policy.rules = aare_new_ruleset(0);
+ prof->policy.rules = new aare_rules();
if (!prof->policy.rules)
goto out;
@@ -694,26 +690,21 @@
* to be supported
*/
- if (kernel_supports_mount) {
- if (!aare_add_rule(prof->policy.rules, mediates_mount, 0,
AA_MAY_READ, 0, dfaflags))
+ if (kernel_supports_mount &&
+ !prof->policy.rules->add_rule(mediates_mount, 0, AA_MAY_READ, 0,
dfaflags))
goto out;
- prof->policy.count++;
- }
- if (kernel_supports_dbus) {
- if (!aare_add_rule(prof->policy.rules, mediates_dbus, 0,
AA_MAY_READ, 0, dfaflags))
- goto out;
- prof->policy.count++;
- }
- if (prof->policy.count > 0) {
- prof->policy.dfa = aare_create_dfa(prof->policy.rules,
- &prof->policy.size,
- dfaflags);
- aare_delete_ruleset(prof->policy.rules);
+ if (kernel_supports_dbus &&
+ !prof->policy.rules->add_rule(mediates_dbus, 0, AA_MAY_READ, 0,
dfaflags))
+ goto out;
+ if (prof->policy.rules->rule_count > 0) {
+ prof->policy.dfa =
prof->policy.rules->create_dfa(&prof->policy.size, dfaflags);
+ delete prof->policy.rules;
+
prof->policy.rules = NULL;
if (!prof->policy.dfa)
goto out;
} else {
- aare_delete_ruleset(prof->policy.rules);
+ delete prof->policy.rules;
prof->policy.rules = NULL;
}
@@ -722,7 +713,7 @@
error = 0;
out:
- aare_delete_ruleset(prof->policy.rules);
+ delete prof->policy.rules;
prof->policy.rules = NULL;
return error;
--- 2.9-test.orig/parser/profile.cc
+++ 2.9-test/parser/profile.cc
@@ -67,11 +67,11 @@
for (RuleList::iterator i = rule_ents.begin(); i != rule_ents.end();
i++)
delete *i;
if (dfa.rules)
- aare_delete_ruleset(dfa.rules);
+ delete dfa.rules;
if (dfa.dfa)
free(dfa.dfa);
if (policy.rules)
- aare_delete_ruleset(policy.rules);
+ delete policy.rules;
if (policy.dfa)
free(policy.dfa);
if (xmatch)
--- 2.9-test.orig/parser/profile.h
+++ 2.9-test/parser/profile.h
@@ -18,6 +18,7 @@
#include "parser.h"
#include "rule.h"
+#include "libapparmor_re/aare_rules.h"
class Profile;
@@ -120,12 +121,11 @@
};
struct dfa_stuff {
- aare_ruleset_t *rules;
- int count;
+ aare_rules *rules;
void *dfa;
size_t size;
- dfa_stuff(void): rules(NULL), count(0), dfa(NULL), size(0) { }
+ dfa_stuff(void): rules(NULL), dfa(NULL), size(0) { }
};
class Profile {
--
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
