parser: fix option flag processing for single conditional rules

The combined optional flag and exact match flag processing is problematic
separate out the optional flag processing so it is only combined during
match string generation.

While doing so we fix the flag output so that multiple rules are
not output when they shouldn't be.

In addition we temporarily break multiple options= and 'options in'
conditionals in a single rule, which we will fix in a separate patch.

Bug Link: https://bugs.launchpad.net/apparmor/+bug/1597017

Signed-off-by: John Johansen <john.johansen@canonical.com>
- rebased to bba1a023bf
- made tests happy by changing condition in gen_policy_re()
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
This commit is contained in:
John Johansen 2023-03-28 04:23:20 -07:00
parent ae1950b004
commit 300889c3a4
3 changed files with 103 additions and 55 deletions

View file

@ -369,11 +369,9 @@ static struct value_list *extract_fstype(struct cond_entry **conds)
return list;
}
static struct value_list *extract_options(struct cond_entry **conds, int eq)
static struct cond_entry *extract_options(struct cond_entry **conds, int eq)
{
struct value_list *list = NULL;
struct cond_entry *entry, *tmp, *prev = NULL;
struct cond_entry *list = NULL, *entry, *tmp, *prev = NULL;
list_for_each_safe(*conds, entry, tmp) {
if ((strcmp(entry->name, "options") == 0 ||
@ -381,10 +379,8 @@ static struct value_list *extract_options(struct cond_entry **conds, int eq)
entry->eq == eq) {
list_remove_at(*conds, prev, entry);
PDEBUG(" extracting option %s\n", entry->name);
list_append(entry->vals, list);
list = entry->vals;
entry->vals = NULL;
free_cond_entry(entry);
list_append(entry, list);
list = entry;
} else
prev = entry;
}
@ -392,60 +388,109 @@ static struct value_list *extract_options(struct cond_entry **conds, int eq)
return list;
}
static void perror_conds(const char *rule, struct cond_entry *conds)
{
struct cond_entry *entry;
list_for_each(conds, entry) {
PERROR( "unsupported %s condition '%s%s(...)'\n", rule, entry->name, entry->eq ? "=" : " in ");
}
}
static void perror_vals(const char *rule, struct value_list *vals)
{
struct value_list *entry;
list_for_each(vals, entry) {
PERROR( "unsupported %s value '%s'\n", rule, entry->value);
}
}
static void process_one_option(struct cond_entry *&opts, unsigned int &flags,
unsigned int &inv_flags)
{
struct cond_entry *entry;
struct value_list *vals;
entry = list_first(opts);
list_remove(opts, entry);
vals = entry->vals;
entry->vals = NULL;
/* fail if there are any unknown optional flags */
if (opts) {
PERROR(" unsupported multiple 'mount options %s(...)'\n", entry->eq ? "=" : " in ");
exit(1);
}
free_cond_entry(entry);
flags = extract_flags(&vals, &inv_flags);
if (vals) {
perror_vals("mount option", vals);
exit(1);
}
}
mnt_rule::mnt_rule(struct cond_entry *src_conds, char *device_p,
struct cond_entry *dst_conds unused, char *mnt_point_p,
int allow_p):
mnt_point(mnt_point_p), device(device_p), trans(NULL), opts(NULL),
flags(0), inv_flags(0), audit(0), deny(0)
flags(0), opt_flags(0), audit(0), deny(0)
{
/* FIXME: dst_conds are ignored atm */
dev_type = extract_fstype(&src_conds);
if (src_conds) {
struct value_list *list = extract_options(&src_conds, 0);
/* move options in () to local list */
struct cond_entry *opts_in = extract_options(&src_conds, 0);
opts = extract_options(&src_conds, 1);
if (opts)
flags = extract_flags(&opts, &inv_flags);
if (list) {
if (opts_in) {
unsigned int tmpflags, tmpinv_flags = 0;
tmpflags = extract_flags(&list, &tmpinv_flags);
/* these flags are optional so set both */
tmpflags |= tmpinv_flags;
tmpinv_flags |= tmpflags;
flags |= tmpflags;
inv_flags |= tmpinv_flags;
if (opts)
list_append(opts, list);
else if (list)
opts = list;
process_one_option(opts_in, tmpflags, tmpinv_flags);
/* optional flags if set/clear mean the same thing and can be
* represented by a single bitset
*/
opt_flags |= tmpflags;
opt_flags |= tmpinv_flags;
}
/* move options=() to opts list */
struct cond_entry *opts_eq = extract_options(&src_conds, 1);
if (opts_eq) {
unsigned int tmpflags, tmpinv_flags = 0;
process_one_option(opts_eq, tmpflags, tmpinv_flags);
if (allow_p & AA_DUMMY_REMOUNT)
tmpflags |= MS_REMOUNT;
flags = tmpflags;
}
if (src_conds) {
perror_conds("mount", src_conds);
exit(1);
}
}
if (!(flags | opt_flags)) {
/* no flag options, and not remount, allow everything */
if (allow_p & AA_DUMMY_REMOUNT) {
flags = MS_REMOUNT;
opt_flags = MS_REMOUNT_FLAGS & (~MS_REMOUNT);
} else {
flags = MS_ALL_FLAGS;
opt_flags = MS_ALL_FLAGS;
}
} else if (!flags) {
/* no flags but opts set */
if (allow_p & AA_DUMMY_REMOUNT)
flags = MS_REMOUNT;
}
if (allow_p & AA_DUMMY_REMOUNT) {
allow_p = AA_MAY_MOUNT;
flags |= MS_REMOUNT;
inv_flags = 0;
} else if (!(flags | inv_flags)) {
/* no flag options, and not remount, allow everything */
flags = MS_ALL_FLAGS;
inv_flags = MS_ALL_FLAGS;
}
allow = allow_p;
if (src_conds) {
PERROR(" unsupported mount conditions\n");
exit(1);
}
if (opts) {
PERROR(" unsupported mount options\n");
exit(1);
}
}
ostream &mnt_rule::dump(ostream &os)
@ -459,7 +504,7 @@ ostream &mnt_rule::dump(ostream &os)
else
os << "error: unknown mount perm";
os << " (0x" << hex << flags << " - 0x" << inv_flags << ") ";
os << " (0x" << hex << flags << " - 0x" << opt_flags << ") ";
if (dev_type) {
os << " type=";
print_value_list(dev_type);
@ -515,7 +560,7 @@ int mnt_rule::expand_variables(void)
}
static int build_mnt_flags(char *buffer, int size, unsigned int flags,
unsigned int inv_flags)
unsigned int opt_flags)
{
char *p = buffer;
int i, len = 0;
@ -528,7 +573,7 @@ static int build_mnt_flags(char *buffer, int size, unsigned int flags,
return TRUE;
}
for (i = 0; i <= 31; ++i) {
if ((flags & inv_flags) & (1 << i))
if ((opt_flags) & (1 << i))
len = snprintf(p, size, "(\\x%02x|)", i + 1);
else if (flags & (1 << i))
len = snprintf(p, size, "\\x%02x", i + 1);
@ -616,7 +661,7 @@ int mnt_rule::gen_policy_remount(Profile &prof, int &count)
vec[2] = default_match_pattern;
if (!build_mnt_flags(flagsbuf, PATH_MAX, flags & MS_REMOUNT_FLAGS,
inv_flags & MS_REMOUNT_FLAGS))
opt_flags & MS_REMOUNT_FLAGS))
goto fail;
vec[3] = flagsbuf;
@ -677,7 +722,7 @@ int mnt_rule::gen_policy_bind_mount(Profile &prof, int &count)
vec[2] = default_match_pattern;
if (!build_mnt_flags(flagsbuf, PATH_MAX, flags & MS_BIND_FLAGS,
inv_flags & MS_BIND_FLAGS))
opt_flags & MS_BIND_FLAGS))
goto fail;
vec[3] = flagsbuf;
if (!prof.policy.rules->add_rule_vec(deny, allow, audit, 4, vec,
@ -716,7 +761,7 @@ int mnt_rule::gen_policy_change_mount_type(Profile &prof, int &count)
vec[2] = default_match_pattern;
if (!build_mnt_flags(flagsbuf, PATH_MAX, flags & MS_MAKE_FLAGS,
inv_flags & MS_MAKE_FLAGS))
opt_flags & MS_MAKE_FLAGS))
goto fail;
vec[3] = flagsbuf;
if (!prof.policy.rules->add_rule_vec(deny, allow, audit, 4, vec,
@ -757,7 +802,7 @@ int mnt_rule::gen_policy_move_mount(Profile &prof, int &count)
vec[2] = default_match_pattern;
if (!build_mnt_flags(flagsbuf, PATH_MAX, flags & MS_MOVE_FLAGS,
inv_flags & MS_MOVE_FLAGS))
opt_flags & MS_MOVE_FLAGS))
goto fail;
vec[3] = flagsbuf;
if (!prof.policy.rules->add_rule_vec(deny, allow, audit, 4, vec,
@ -798,7 +843,7 @@ int mnt_rule::gen_policy_new_mount(Profile &prof, int &count)
vec[2] = typebuf.c_str();
if (!build_mnt_flags(flagsbuf, PATH_MAX, flags & MS_NEW_FLAGS,
inv_flags & MS_NEW_FLAGS))
opt_flags & MS_NEW_FLAGS))
goto fail;
vec[3] = flagsbuf;
@ -838,12 +883,10 @@ int mnt_rule::gen_policy_re(Profile &prof)
std::string mntbuf;
std::string devbuf;
std::string typebuf;
char flagsbuf[PATH_MAX + 3];
std::string optsbuf;
char class_mount_hdr[64];
const char *vec[5];
int count = 0;
unsigned int tmpflags, tmpinv_flags;
if (!features_supports_mount) {
warn_once(prof.name);
@ -856,7 +899,11 @@ int mnt_rule::gen_policy_re(Profile &prof)
* created in the backend to cover all the possible choices
*/
if ((allow & AA_MAY_MOUNT) && (flags == MS_ALL_FLAGS)) {
/*
* XXX: added !flags to cover cases like:
* mount options in (bind) /d -> /4,
*/
if ((allow & AA_MAY_MOUNT) && (!flags || flags == MS_ALL_FLAGS)) {
/* no mount flags specified, generate multiple rules */
if (!device && !dev_type &&
gen_policy_remount(prof, count) == RULE_ERROR)
@ -890,7 +937,7 @@ int mnt_rule::gen_policy_re(Profile &prof)
if (gen_policy_move_mount(prof, count) == RULE_ERROR)
goto fail;
} else if ((allow & AA_MAY_MOUNT) &&
(flags | inv_flags) & ~MS_CMDS) {
(flags | opt_flags) & ~MS_CMDS) {
/* generic mount if flags are set that are not covered by
* above commands
*/

View file

@ -132,7 +132,7 @@ public:
struct value_list *dev_type;
struct value_list *opts;
unsigned int flags, inv_flags;
unsigned int flags, opt_flags;
int allow, audit;
int deny;

View file

@ -232,6 +232,7 @@ do { \
#endif
#define list_first(LIST) (LIST)
#define list_for_each(LIST, ENTRY) \
for ((ENTRY) = (LIST); (ENTRY); (ENTRY) = (ENTRY)->next)
#define list_for_each_safe(LIST, ENTRY, TMP) \