Re: [RFC] struct filename, io_uring and audit troubles

[Al Viro <viro@zeniv.linux.org.uk>]

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 .

resulting in
type=USER_START msg=audit(1727113434.684:497): pid=3433 uid=0 auid=0 ses=1 subj=unconfined msg='op=PAM:session_open grantors=pam_key
init,pam_env,pam_env,pam_mail,pam_limits,pam_permit,pam_unix,pam_ecryptfs acct="al" exe="/usr/bin/su" hostname=? addr=? terminal=/de
v/pts/0 res=success'^]UID="root" AUID="root"
type=SYSCALL msg=audit(1727113466.464:498): arch=c000003e syscall=266 success=yes exit=0 a0=7ffc441ad843 a1=ffffff9c a2=7ffc441ad845 a3=7f10e4816a90 items=3 ppid=3434 pid=3449 auid=0 uid=1000 gid=1000 euid=1000 suid=1000 fsuid=1000 egid=1000 sgid=1000 fsgid=1000 tty=pts0 ses=1 comm="ln" exe="/usr/bin/ln" subj=unconfined key=(null)^]ARCH=x86_64 SYSCALL=symlinkat AUID="root" UID="al" GID="al" EUID="al" SUID="al" FSUID="al" EGID="al" SGID="al" FSGID="al"
type=CWD msg=audit(1727113466.464:498): cwd="/tmp/blah"
type=PATH msg=audit(1727113466.464:498): item=0 name="/tmp/blah" inode=135460 dev=08:14 mode=041777 ouid=1000 ogid=1000 rdev=00:00 nametype=PARENT cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0^]OUID="al" OGID="al"
type=PATH msg=audit(1727113466.464:498): item=1 name="c" nametype=UNKNOWN cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0
type=PATH msg=audit(1727113466.464:498): item=2 name="b" inode=135497 dev=08:14 mode=0120777 ouid=1000 ogid=1000 rdev=00:00 nametype=CREATE cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0^]OUID="al" OGID="al"
type=PROCTITLE msg=audit(1727113466.464:498): proctitle=6C6E002D7300630062
type=USER_END msg=audit(1727113560.852:499): pid=3433 uid=0 auid=0 ses=1 subj=unconfined msg='op=PAM:session_close grantors=pam_keyinit,pam_env,pam_env,pam_mail,pam_limits,pam_permit,pam_unix,pam_ecryptfs acct="al" exe="/usr/bin/su" hostname=? addr=? terminal=/dev/pts/0 res=success'^]UID="root" AUID="root"

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?

What happens is that we start with two UNKNOWN (for oldname and newname arguments
of symlinkat(); incidentally, for the order is undefined - it's up to compiler
which argument of do_symlinkat(getname(oldname), newdfd, getname(newname)) gets
evaluated first).  When we get through the call of __filename_parentat(), we get
the newname's audit_names instance hit by audit_inode(...., AUDIT_INODE_PARENT).
That converts the one for newname from UNKNOWN to PARENT.  Then, in may_create(),
we hit it with audit_inode_child() for AUDIT_TYPE_CHILD_CREATING.  Which finds
the parent (newname's audit_names, converted to PARENT by that point) and goes
looking for child.

That's where the execution paths diverge - for symlink("a", "a") the oldname's
audit_names instance looks like a valid candidate and gets converted to CHILD_CREATING.
For symlink("c", "b") such candidate is there, so we get a new item allocated -
CHILD_CREATING for "b".  Name reference gets cloned from PARENT.

Now, ->symlink() is done, it succeeds and fsnotify_create(dir, dentry) gets
called.  We get an identical audit_inode_child() call, except that now dentry
is positive.  It finds PARENT instance, finds CHILD_CREATING one, and slaps
the inode metadata into it, setting ->uid to non-zero.

As the result, in one case you are left with PARENT, UNKNOWN, CHILD_CREATING and
in another - PARENT, CHILD_CREATING.  Modulo reordering first two items if
compiler decides to evaluate the first argument of do_symlinkat() call before
the third one...  In any case, AUDIT_OBJ_UID(0) gives a match on UNKNOWN -
if it's there.  Since both PARENT and CHILD_CREATING refer to non-root-owned
inodes, AUDIT_OBJ_UID(0) does not match either...