Re: [RFC PATCH ghak9 0/3] audit: Record the path of FDs passed to *at(2) syscalls

Ondrej Mosnacek
2018-08-03T09:19:39Z

On Fri, Aug 3, 2018 at 1:58 AM Paul Moore <paul@paul-moore.com> wrote:
> On Thu, Jul 26, 2018 at 5:13 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > On Thu, Jul 26, 2018 at 10:12 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > > I think it should be possible to collect that information by putting
> > > hooks in the right places of the filesystem code (and fixing the
> > > current ones).
> >
> > Hm, after closer look, it seems this won't be doable (at least not
> > easily). The PATH record always logs the original path string from
> > userspace (and I think we need to preserve this behavior in case
> > someone relies on it). In case of PARENT records, it truncates away
> > the last component (because it wants to log inode information also for
> > the parent directory). If this truncated string ends up empty (i.e.
> > the original string had just one component), it just smashes in the
> > absolute path of the CWD (which is known), because it pretends no
> > *at() syscalls exist and all relative paths are relative to current
> > CWD.
> >
> > So to fix this, we need to do one of the following:
> > 1. Add a new field to the PATH records that would specify the path of
> > the directory that the value of name=... is relative to. If this is
> > CWD, we can just use some special value
> > ("(null)"/"(none)"/"(cwd)"/...) or omit the field completely. I prefer
> > this approach, because it will best solve the case of renameat(),
> > where different PATH records can have different base directories.
>
> It is worth calling extra attention to the fact that we would now be
> effectively recording two paths in a single record. I'm not sure how
> much we care about that, but it does increase the chances we blow past
> the end of the netlink buffer; although it is worth noting that a
> single PATH_MAX entry would do that today.

Yes, but note that the two "paths" are actually just parts of a single
(full) path. The situation will be no different from when the user
would just specify a full path. But your comment just made me realize
that I will need to make sure that the new field is empty also when
dirfd != AT_FDCWD, but the path supplied to the syscall is absolute
(because then it would be just useless information).

> Also, would this new field remain empty for non-*at syscalls?

Yes, the emptiness (or some special value depending on the chosen
implementation) would indicate that dirfd is CWD (which can be looked
up in the CWD record).

> > 2. If adding fields is considered A Bad Thing, we could alternatively
> > provide this information in separate records (either PATH with special
> > nametype or a new record). However in such case we need to somehow
> > specify to which PATH records each base directory corresponds. For
> > PATH records this could be guessed from their order, but this is a
> > fragile thing (changes in filesystem code could change the order).
>
> While this may be a bit more difficult it seems like this is more in
> keeping with the current methodology, and would keep the overall audit
> logs smaller. In the case of the *at syscalls I presume you would
> create PARENT records for the base directory, omitting it if AT_FDCWD
> was used?

Right now, the parent/child records are only emitted when a file is
moved, created, or removed (actually I'm not sure right now about the
latter two, but definitely for the first case). The PARENT record
always contains the (relative or absolute) path as given to the
syscall minus the last element and the corresponding CREATE/DELETE
record contains only the last element. I don't know what is the goal
of this splitting (probably to report details (inode, ...) also about
the containing directory or maybe to deduplicate PATH records if src
and dst directories are the same?), but I'm not sure it is a good idea
to overload the semantics of nametype=PARENT for yet another purpose
(one of the PARENTs would be parent for the other PARENT...). I'm
almost certain this would also confuse the current userspace code
(especially considering that for rename() the kernel generates both
PARENT records before the child records...).

That said, it seems to me this would actually make the logs *bigger*,
because you would also log the usual PATH record details (inode,
fcaps, ...) for the base directory (dirfd).

> I imagine this would also help the traditional rename() syscall?

I'm not sure how, given the facts above.

--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.