New audit-perms patch [ Re: Audit perms check on recv ]

[Darrel Goeddel]

Serge E. Hallyn wrote:
> The attached patch addresses Stephen's comments about re-using
> dummy_capget code and properly checking capabilities in
> selinux_netlink_send.
> 
> To review, it 
> 
>    1.  adds two new capabilities, CAP_AUDIT_READ and CAP_AUDIT_WRITE
>    2.  changes dummy.c and selinux/hooks.c to set the
> netlink_audit message's eff_cap field to accurately reflect
> the capabilities of the sender,
>    3.  checks for the capabilities required for the requested audit
> action in the eff_cap field at audit_msg_recv
>    4.  adds checks to ensure that the message lengths are at least
> long enough to hold the structures they claim to hold.
> 
> thanks,
> -serge
> 

It would seem that separate CAP_AUDIT_ADMIN/CAP_AUDIT_WRITE capabilities are 
much more important than having a separate CAP_ADMIN_READ capability.  The 
CAP_AUDIT_WRITE capability should only allow a process to generate a userspace 
audit message.  I do not think we should impose a trust equivalence for programs 
that can generate an audit message and programs that can modify the audit 
subsystem configuration.

I think capability checks should map like this:

AUDIT_GET -> none (possibly CAP_AUDIT_ADMIN)
AUDIT_SET -> CAP_AUDIT_ADMIN
AUDIT_LIST -> none (possibly CAP_AUDIT_ADMIN)
AUDIT_ADD -> CAP_AUDIT_ADMIN
AUDIT_DEL -> CAP_AUDIT_ADMIN
AUDIT_USER -> CAP_AUDIT_WRITE
AUDIT_LOGIN -> CAP_AUDIT_ADMIN

The case of AUDIT_LOGIN has merit for a separate CAP_AUDIT_LOGIN capability 
because this carries much more importance than AUDIT_USER, but we really should 
not have the ability to mess with the rest of the configuration.  However, this 
action is as important to the reliability of the audit logs as the configuration 
of the audit subsystem.  I would prioritize this capability above CAP_AUDIT_READ 
as well.

-- 

Darrel