On Mon, Sep 23, 2024 at 2:17 PM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> On Mon, Sep 23, 2024 at 12:14:29PM -0400, Paul Moore wrote:
>
> > > And having everything that passed through getname()/getname_kernel()
> > > shoved into ->names_list leads to very odd behaviour, especially with
> > > audit_names conversions in audit_inode()/audit_inode_child().
> > >
> > > Look at the handling of AUDIT_DEV{MAJOR,MINOR} or AUDIT_OBJ_{UID,GID}
> > > or AUDIT_COMPARE_..._TO_OBJ; should they really apply to audit_names
> > > resulting from copying the symlink body into the kernel? And if they
> > > should be applied to audit_names instance that had never been associated
> > > with any inode, should that depend upon the string in those being
> > > equal to another argument of the same syscall?
> > >
> > > I'm going through the kernel/auditsc.c right now, but it's more of
> > > a "document what it does" - I don't have the specs and I certainly
> > > don't remember such details.
> >
> > My approach to audit is "do what makes sense for a normal person", if
> > somebody needs silly behavior to satisfy some security cert then they
> > can get involved in upstream development and send me patches that
> > don't suck.
>
> root@kvm1:/tmp# auditctl -l
> -a always,exit -S all -F dir=/tmp/blah -F perm=rwxa -F obj_uid=0
> root@kvm1:/tmp# su - al
> al@kvm1:~$ cd /tmp/blah
> al@kvm1:/tmp/blah$ ln -s a a
> al@kvm1:/tmp/blah$ ln -s c b
> al@kvm1:/tmp/blah$ ls -l
> total 0
> lrwxrwxrwx 1 al al 1 Sep 23 13:44 a -> a
> lrwxrwxrwx 1 al al 1 Sep 23 13:44 b -> c
> al@kvm1:/tmp/blah$ ls -ld
> drwxr-xr-x 2 al al 4096 Sep 23 13:44 .
...
> Note that
> * none of the filesystem objects involved have UID 0
> * of two calls of symlinkat(), only the second one triggers the rule
> ... and removing the -F obj_uid=0 from the rule catches both (as expected),
> with the first one getting *two* items instead of 3 - there's PARENT (identical
> to the second call), there's CREATE (for "a" instead of "b", obviously) and
> there's no UNKNOWN with the symlink body.
>
> Does the behaviour above make sense for a normal person, by your definition?
I never said that there aren't plenty of bugs in the current
implementation. Really. Audit is a train wreck for so many reasons,
most of my time spent with audit is spent making sure it doesn't
panic, or some other subsystem hasn't wrecked it even further.
If you're looking for someone to justify audit's behavior here, you'll
need to keep looking. Personally I've got plans to burn the whole
thing down if I ever get enough time to work on it, but in the
meantime I want to try and make it as usable as possible. Patches are
always welcome.
--
paul-moore.com
