[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