Re: [PATCH v4 3/3] audit: add OPENAT2 record to list how

[Richard Guy Briggs <rgb@redhat.com>]

On 2021-05-24 19:08, Paul Moore wrote:
> On Thu, May 20, 2021 at 4:03 AM Christian Brauner
> <christian.brauner@ubuntu.com> wrote:
> > On Wed, May 19, 2021 at 04:00:22PM -0400, Richard Guy Briggs wrote:
> > > Since the openat2(2) syscall uses a struct open_how pointer to communicate
> > > its parameters they are not usefully recorded by the audit SYSCALL record's
> > > four existing arguments.
> > >
> > > Add a new audit record type OPENAT2 that reports the parameters in its
> > > third argument, struct open_how with fields oflag, mode and resolve.
> > >
> > > The new record in the context of an event would look like:
> > > time->Wed Mar 17 16:28:53 2021
> > > type=PROCTITLE msg=audit(1616012933.531:184): proctitle=73797363616C6C735F66696C652F6F70656E617432002F746D702F61756469742D7465737473756974652D737641440066696C652D6F70656E617432
> > > type=PATH msg=audit(1616012933.531:184): item=1 name="file-openat2" inode=29 dev=00:1f mode=0100600 ouid=0 ogid=0 rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0 nametype=CREATE cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0
> > > type=PATH msg=audit(1616012933.531:184): item=0 name="/root/rgb/git/audit-testsuite/tests" inode=25 dev=00:1f mode=040700 ouid=0 ogid=0 rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0 nametype=PARENT cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0
> > > type=CWD msg=audit(1616012933.531:184): cwd="/root/rgb/git/audit-testsuite/tests"
> > > type=OPENAT2 msg=audit(1616012933.531:184): oflag=0100302 mode=0600 resolve=0xa
> > > type=SYSCALL msg=audit(1616012933.531:184): arch=c000003e syscall=437 success=yes exit=4 a0=3 a1=7ffe315f1c53 a2=7ffe315f1550 a3=18 items=2 ppid=528 pid=540 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=ttyS0 ses=1 comm="openat2" exe="/root/rgb/git/audit-testsuite/tests/syscalls_file/openat2" subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key="testsuite-1616012933-bjAUcEPO"
> > >
> > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > Link: https://lore.kernel.org/r/d23fbb89186754487850367224b060e26f9b7181.1621363275.git.rgb@redhat.com
> > > ---
> > >  fs/open.c                  |  2 ++
> > >  include/linux/audit.h      | 10 ++++++++++
> > >  include/uapi/linux/audit.h |  1 +
> > >  kernel/audit.h             |  2 ++
> > >  kernel/auditsc.c           | 18 +++++++++++++++++-
> > >  5 files changed, 32 insertions(+), 1 deletion(-)
> 
> ...
> 
> > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > > index 3f59ab209dfd..faf2485323a9 100644
> > > --- a/kernel/auditsc.c
> > > +++ b/kernel/auditsc.c
> > > @@ -76,7 +76,7 @@
> > >  #include <linux/fsnotify_backend.h>
> > >  #include <uapi/linux/limits.h>
> > >  #include <uapi/linux/netfilter/nf_tables.h>
> > > -#include <uapi/linux/openat2.h>
> > > +#include <uapi/linux/openat2.h> // struct open_how
> > >
> > >  #include "audit.h"
> > >
> > > @@ -1319,6 +1319,12 @@ static void show_special(struct audit_context *context, int *call_panic)
> > >               audit_log_format(ab, "fd=%d flags=0x%x", context->mmap.fd,
> > >                                context->mmap.flags);
> > >               break;
> > > +     case AUDIT_OPENAT2:
> > > +             audit_log_format(ab, "oflag=0%llo mode=0%llo resolve=0x%llx",
> >
> > Hm, should we maybe follow the struct member names for all entries, i.e.
> > replace s/oflag/flags?
> 
> There is some precedence for using "oflags" to refer to "open" flags,
> my guess is Richard is trying to be consistent here.  I agree it's a
> little odd, but it looks like the right thing to me from an audit
> perspective; the audit perspective is a little odd after all :)

Thanks Paul.

I could have sworn I had a conversation with someone about this but I
can't find any of that evidence otherwise I'd paste it here.

With the help of our audit field dictionary we have some guidance of
what these new field names should be:
	https://github.com/linux-audit/audit-documentation/blob/main/specs/fields/field-dictionary.csv

The "flags" field is used for the mmap record (coincidentally in the
context diff), so should not be used here because it will cause issues
in the userspace parser.  The open syscall flags are listed with
"oflag".  Other flag fields are named after their domain.

The value field has a precedence of "val" that is not associated with
any particular domain and is alphanumeric.  Other value fields take the
name of their domain, so that was a possibility.

"resolve" would be a new field for which I have a note to add it to this
document if the patch is accepted.

> paul moore

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit