[apparmor] [PATCH 1/2] Fix compilation failure of deny link rules

[Tyler Hicks]

On 2015-03-19 03:52:13, John Johansen wrote:
> BugLink: http://bugs.launchpad.net/bugs/1433829
> 
> The apparmor_parser fails to compile deny rules with only link permissions.
> 
>   Eg.
>        deny /f l,
>        deny l /f,
>        deny link /f -> /d,
> 
> Will all fail to compile with the following assert
> 
>   apparmor_parser: aare_rules.cc:99: Node* convert_file_perms(int, uint32_t, uint32_t, bool): Assertion `perms != 0' failed.
> 
> Signed-off-by: John Johansen <john.johansen at canonical.com>

I don't understand the purpose link pair rules and can't find a
description of them in the source and/or commit messages. However, if
the intended purpose of this patch is to prevent the first pair rule
from being added when there is no second pair rule, then this patch gets
my ack (pending the changes that cboltz requested).

As a side note, when adding the first pair rule, the conditionals for
checking the LINK_BITS and CHANGE_PROFILE bits are set up in an if-else
style. When adding the second pair rule, the conditionals are set up in
an if-if style. That seems wrong but I can't say if that's an issue in
practice (would a mode ever have both of those sets of bits set?).

Tyler


> ---
>  parser/parser_regex.c                                | 3 ++-
>  parser/tst/simple_tests/file/okay_audit_deny_link.sd | 9 +++++++++
>  parser/tst/simple_tests/file/okay_deny_link.sd       | 9 +++++++++
>  3 files changed, 20 insertions(+), 1 deletion(-)
>  create mode 100644 parser/tst/simple_tests/file/okay_audit_deny_link.sd
>  create mode 100644 parser/tst/simple_tests/file/okay_deny_link.sd
> 
> diff --git a/parser/parser_regex.c b/parser/parser_regex.c
> index 45f7f3e..699afac 100644
> --- a/parser/parser_regex.c
> +++ b/parser/parser_regex.c
> @@ -523,7 +523,8 @@ static int process_dfa_entry(aare_rules *dfarules, struct cod_entry *entry)
>  	 * match.  audit info for the link is carried on the second
>  	 * entry of the pair
>  	 */
> -	if (entry->deny && (entry->mode & AA_LINK_BITS)) {
> +	if (entry->deny && (entry->mode & AA_LINK_BITS) &&
> +	    (entry->mode & ~AA_LINK_BITS)) {
>  		if (!dfarules->add_rule(tbuf.c_str(), entry->deny,
>  					entry->mode & ~AA_LINK_BITS,
>  					entry->audit & ~AA_LINK_BITS, dfaflags))
> diff --git a/parser/tst/simple_tests/file/okay_audit_deny_link.sd b/parser/tst/simple_tests/file/okay_audit_deny_link.sd
> new file mode 100644
> index 0000000..393f906
> --- /dev/null
> +++ b/parser/tst/simple_tests/file/okay_audit_deny_link.sd
> @@ -0,0 +1,9 @@
> +#
> +#=DESCRIPTION simple link access test
> +#=EXRESULT PASS
> +#
> +
> +profile test {
> +  audit deny link /alpha/beta -> /tmp/**,
> +}
> +
> diff --git a/parser/tst/simple_tests/file/okay_deny_link.sd b/parser/tst/simple_tests/file/okay_deny_link.sd
> new file mode 100644
> index 0000000..fe0684c
> --- /dev/null
> +++ b/parser/tst/simple_tests/file/okay_deny_link.sd
> @@ -0,0 +1,9 @@
> +#
> +#=DESCRIPTION simple link access test
> +#=EXRESULT PASS
> +#
> +
> +profile test {
> +  deny link /alpha/beta -> /tmp/**,
> +}
> +
> -- 
> 2.1.4
> 
> 
> -- 
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150319/02d6774f/attachment-0001.pgp>