[PATCH ghak114 V1] audit: enforce op for string fields

[Paul Moore]

On Wed, May 22, 2019 at 5:52 PM Richard Guy Briggs <rgb at redhat.com> wrote:
>
> The field operator is ignored on several string fields.  WATCH, DIR,
> PERM and FILETYPE field operators are completely ignored and meaningless
> since the op is not referenced in audit_filter_rules().  Range and
> bitwise operators are already addressed in ghak73.
>
> Honour the operator for WATCH, DIR, PERM, FILETYPE fields as is done in
> the EXE field.
>
> Please see github issue
> https://github.com/linux-audit/audit-kernel/issues/114
> ---
>  kernel/auditsc.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)

While the patch looks fine, it is missing your sign-off.  If you reply
to this thread with it, I'll go ahead and add to the patch when
merging.

I'm sure everyone is tired of hearing me complain about people not
checking their patches, but this is something that would have been
caught by running ./scripts/checkpatch.pl against your patch (the
entire patch, not just the code portion).  If you aren't running your
full patch through checkpatch already, it is easy to do (there are
likely other ways too, these are just the two that I use):

* using git
# git format-patch --stdout -1 <commit_id> | ./scripts/checkpatch.pl -

* using stgit (my favorite)
# stg export -s <patch> | ./scripts/checkpatch.pl -

> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 30aa07b0115f..087137d341a2 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -601,12 +601,20 @@ static int audit_filter_rules(struct task_struct *tsk,
>                         }
>                         break;
>                 case AUDIT_WATCH:
> -                       if (name)
> -                               result = audit_watch_compare(rule->watch, name->ino, name->dev);
> +                       if (name) {
> +                               result = audit_watch_compare(rule->watch,
> +                                                            name->ino,
> +                                                            name->dev);
> +                               if (f->op == Audit_not_equal)
> +                                       result = !result;
> +                       }
>                         break;
>                 case AUDIT_DIR:
> -                       if (ctx)
> +                       if (ctx) {
>                                 result = match_tree_refs(ctx, rule->tree);
> +                               if (f->op == Audit_not_equal)
> +                                       result = !result;
> +                       }
>                         break;
>                 case AUDIT_LOGINUID:
>                         result = audit_uid_comparator(audit_get_loginuid(tsk),
> @@ -684,9 +692,13 @@ static int audit_filter_rules(struct task_struct *tsk,
>                         break;
>                 case AUDIT_PERM:
>                         result = audit_match_perm(ctx, f->val);
> +                       if (f->op == Audit_not_equal)
> +                               result = !result;
>                         break;
>                 case AUDIT_FILETYPE:
>                         result = audit_match_filetype(ctx, f->val);
> +                       if (f->op == Audit_not_equal)
> +                               result = !result;
>                         break;
>                 case AUDIT_FIELD_COMPARE:
>                         result = audit_field_compare(tsk, cred, f, ctx, name);
> --
> 1.8.3.1

-- 
paul moore
www.paul-moore.com