On Fri, Aug 16, 2019 at 10:57 AM Stephen Smalley [off-list ref] wrote:

On 8/15/19 6:32 PM, James Morris wrote:

quoted

On Thu, 15 Aug 2019, Aaron Goidel wrote:

quoted

In SELinux this new information is leveraged here to perform an
additional inode based check for capabilities relevant to inodes. Since
the inode provided to capable_wrt_inode_uidgid() is a const argument,
this also required propagating const down to dump_common_audit_data() and
dropping the use of d_find_alias() to find an alias for the inode. This
was sketchy to begin with and should be obsoleted by a separate change
that will allow LSMs to trigger audit collection for all file-related
information.

Will the audit logs look the same once the 2nd patch is applied? We need
to be careful about breaking existing userland.

It was already the case that the name= field in the AVC audit record was
not guaranteed to be emitted in this case, since d_find_alias could
return NULL.  And it was only a hint, since that name might have nothing
to do with the name used to look up the inode in the first place. So I
don't believe userland could have ever relied upon it being present
here.  Removing it also fixes a problem with AVC audit generation under
RCU walk; we should be able to drop the code that skips audit generation
in that case with this d_find_alias call gone IIUC.

With the ability for an LSM to enable collection and generation of
AUDIT_PATH and other AUDIT_* records (which is made possible via the
other patch), we will get more complete and relevant information in the
audit log.  It won't look exactly the same (there will be separate AVC,
PATH, ... records that can be correlated based on timestamp/serial and
ausearch does this automatically for you).

Regardless of if it is The Right Thing, changes like this should
probably be put into a separate, unrelated patch.

I think there are a few things in dump_common_audit_data() that should
have been done differently, but unfortunately the audit records (and
IMHO the many stupid design decisions that went into them) are
effectively part of the kernel API and need to be treated with care.

-- 
paul moore
www.paul-moore.com