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 <[email protected]>

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
> [email protected]
> Modify settings or unsubscribe at: 
> https://lists.ubuntu.com/mailman/listinfo/apparmor

Attachment: signature.asc
Description: Digital signature

-- 
AppArmor mailing list
[email protected]
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to