[apparmor] [PATCH] Fix the mount flags set generated by the parser
Steve Beattie
steve at nxnw.org
Mon Mar 12 19:46:38 UTC 2012
On Mon, Mar 12, 2012 at 09:17:32AM -0700, John Johansen wrote:
> When generating the flag set the parser was not generating the complete
> set when flags where not consecutive. This is because the len value
> was not being reset for each flag considered, so once it was set for
> a flag, then the next flag would have to be set to reset it else the
> output string was still incremented by the old len value.
>
> Eg.
> echo "/t { mount options=rbind, }" | apparmor_parser -QT -D rule-exprs
>
> results in
> rule: \x07[^\000]*\x00[^\000]*\x00[^\000]*\x00\x0d ->
>
> however \x0d only covers the bind and not the recursive flag
>
> This is fixed by adding a continue to the flags generation loop for the
> else case.
>
> resulting the dump from above generating
>
> rule: \x07[^\000]*\x00[^\000]*\x00[^\000]*\x00\x0d\x0f ->
>
> \x0d\x0f covers both of the required flags
>
> Also fix the flags output to allow for the allow any flags case. This
> was being screened out. By masking the flags even when no flags where
> specified.
>
> this results in a difference of
>
> echo "/t { mount, }" | apparmor_parser -QT -D rule-exprs
>
> rule: \x07[^\000]*\x00[^\000]*\x00[^\000]*\x00(\x01|)(\x02|)(\x03|)(\x04|)(\x05|)\x00[^\000]*
>
> becoming
> \x07[^\000]*\x00[^\000]*\x00[^\000]*\x00[^\000]*\x00[^\000]*
>
> which is simplified and covers all permissions vs. the first rule output
>
> Signed-off-by: John Johansen <john.johansen at canonical.com>
Acked-By: Steve Beattie <sbeattie at ubuntu.com>
> ---
> parser/mount.c | 4 +-
> parser/parser_regex.c | 62 ++++++++++++++++++++++++++++++++++--------------
> 2 files changed, 46 insertions(+), 20 deletions(-)
>
> diff --git a/parser/mount.c b/parser/mount.c
> index f3a7ca7..d4a5845 100644
> --- a/parser/mount.c
> +++ b/parser/mount.c
> @@ -416,8 +416,8 @@ struct mnt_entry *new_mnt_entry(struct cond_entry *src_conds, char *device,
> ent->inv_flags = 0;
> } else if (!(ent->flags | ent->inv_flags)) {
> /* no flag options, and not remount, allow everything */
> - ent->flags = 0xffffffff;
> - ent->inv_flags = 0xffffffff;
> + ent->flags = MS_ALL_FLAGS;
> + ent->inv_flags = MS_ALL_FLAGS;
> }
>
> ent->allow = allow;
> diff --git a/parser/parser_regex.c b/parser/parser_regex.c
> index 236c80c..7a1218c 100644
> --- a/parser/parser_regex.c
> +++ b/parser/parser_regex.c
> @@ -692,7 +692,7 @@ static int build_mnt_flags(char *buffer, int size, unsigned int flags,
> char *p = buffer;
> int i, len = 0;
>
> - if (flags == 0xffffffff) {
> + if (flags == MS_ALL_FLAGS) {
> /* all flags are optional */
> len = snprintf(p, size, "[^\\000]*");
> if (len < 0 || len >= size)
> @@ -704,14 +704,14 @@ static int build_mnt_flags(char *buffer, int size, unsigned int flags,
> len = snprintf(p, size, "(\\x%02x|)", i + 1);
> else if (flags & (1 << i))
> len = snprintf(p, size, "\\x%02x", i + 1);
> - /* else no entry = not set */
> + else /* no entry = not set */
> + continue;
>
> if (len < 0 || len >= size)
> return FALSE;
> p += len;
> size -= len;
> }
> -
> if (buffer == p) {
> /* match nothing - use impossible 254 as regex parser doesn't
> * like the empty string
> @@ -774,6 +774,7 @@ static int process_mnt_entry(aare_ruleset_t *dfarules, struct mnt_entry *entry)
> char optsbuf[PATH_MAX + 3];
> char *p, *vec[5];
> int count = 0;
> + unsigned int flags, inv_flags;
>
> /* a single mount rule may result in multiple matching rules being
> * created in the backend to cover all the possible choices
> @@ -801,9 +802,14 @@ static int process_mnt_entry(aare_ruleset_t *dfarules, struct mnt_entry *entry)
> vec[1] = devbuf;
> /* skip type */
> vec[2] = devbuf;
> - if (!build_mnt_flags(flagsbuf, PATH_MAX,
> - entry->flags & MS_REMOUNT_FLAGS,
> - entry->inv_flags & MS_REMOUNT_FLAGS))
> +
> + flags = entry->flags;
> + inv_flags = entry->inv_flags;
> + if (flags != MS_ALL_FLAGS)
> + flags &= MS_REMOUNT_FLAGS;
> + if (inv_flags != MS_ALL_FLAGS)
> + flags &= MS_REMOUNT_FLAGS;
> + if (!build_mnt_flags(flagsbuf, PATH_MAX, flags, inv_flags))
> goto fail;
> vec[3] = flagsbuf;
> if (!build_mnt_opts(optsbuf, PATH_MAX, entry->opts))
> @@ -829,9 +835,14 @@ static int process_mnt_entry(aare_ruleset_t *dfarules, struct mnt_entry *entry)
> if (!convert_entry(typebuf, PATH_MAX +3, NULL))
> goto fail;
> vec[2] = typebuf;
> - if (!build_mnt_flags(flagsbuf, PATH_MAX,
> - entry->flags & MS_BIND_FLAGS,
> - entry->inv_flags & MS_BIND_FLAGS))
> +
> + flags = entry->flags;
> + inv_flags = entry->inv_flags;
> + if (flags != MS_ALL_FLAGS)
> + flags &= MS_BIND_FLAGS;
> + if (inv_flags != MS_ALL_FLAGS)
> + flags &= MS_BIND_FLAGS;
> + if (!build_mnt_flags(flagsbuf, PATH_MAX, flags, inv_flags))
> goto fail;
> vec[3] = flagsbuf;
> if (!aare_add_rule_vec(dfarules, entry->deny, entry->allow,
> @@ -856,9 +867,14 @@ static int process_mnt_entry(aare_ruleset_t *dfarules, struct mnt_entry *entry)
> goto fail;
> vec[1] = devbuf;
> vec[2] = devbuf;
> - if (!build_mnt_flags(flagsbuf, PATH_MAX,
> - entry->flags & MS_MAKE_FLAGS,
> - entry->inv_flags & MS_MAKE_FLAGS))
> +
> + flags = entry->flags;
> + inv_flags = entry->inv_flags;
> + if (flags != MS_ALL_FLAGS)
> + flags &= MS_MAKE_FLAGS;
> + if (inv_flags != MS_ALL_FLAGS)
> + flags &= MS_MAKE_FLAGS;
> + if (!build_mnt_flags(flagsbuf, PATH_MAX, flags, inv_flags))
> goto fail;
> vec[3] = flagsbuf;
> if (!aare_add_rule_vec(dfarules, entry->deny, entry->allow,
> @@ -884,9 +900,14 @@ static int process_mnt_entry(aare_ruleset_t *dfarules, struct mnt_entry *entry)
> if (!convert_entry(typebuf, PATH_MAX +3, NULL))
> goto fail;
> vec[2] = typebuf;
> - if (!build_mnt_flags(flagsbuf, PATH_MAX,
> - entry->flags & MS_MOVE_FLAGS,
> - entry->inv_flags & MS_MOVE_FLAGS))
> +
> + flags = entry->flags;
> + inv_flags = entry->inv_flags;
> + if (flags != MS_ALL_FLAGS)
> + flags &= MS_MOVE_FLAGS;
> + if (inv_flags != MS_ALL_FLAGS)
> + flags &= MS_MOVE_FLAGS;
> + if (!build_mnt_flags(flagsbuf, PATH_MAX, flags, inv_flags))
> goto fail;
> vec[3] = flagsbuf;
> if (!aare_add_rule_vec(dfarules, entry->deny, entry->allow,
> @@ -911,9 +932,14 @@ static int process_mnt_entry(aare_ruleset_t *dfarules, struct mnt_entry *entry)
> if (!build_list_val_expr(typebuf, PATH_MAX+2, entry->dev_type))
> goto fail;
> vec[2] = typebuf;
> - if (!build_mnt_flags(flagsbuf, PATH_MAX,
> - entry->flags & ~MS_CMDS,
> - entry->inv_flags & ~MS_CMDS))
> +
> + flags = entry->flags;
> + inv_flags = entry->inv_flags;
> + if (flags != MS_ALL_FLAGS)
> + flags &= ~MS_CMDS;
> + if (inv_flags != MS_ALL_FLAGS)
> + flags &= ~MS_CMDS;
> + if (!build_mnt_flags(flagsbuf, PATH_MAX, flags, inv_flags))
> goto fail;
> vec[3] = flagsbuf;
> if (!build_mnt_opts(optsbuf, PATH_MAX, entry->opts))
> --
> 1.7.9.1
>
>
> --
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
--
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20120312/50730887/attachment.pgp>
More information about the AppArmor
mailing list