On Tue, Sep 15, 2020 at 10:33 AM Chris PeBenito
<chpebeni@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On 9/15/20 9:34 AM, Stephen Smalley wrote:
> > On Tue, Sep 15, 2020 at 9:11 AM Chris PeBenito
> > <chpebeni@xxxxxxxxxxxxxxxxxxx> wrote:
> >> diff --git a/libselinux/src/avc_internal.c b/libselinux/src/avc_internal.c
> >> index 572b2159..35ea59b6 100644
> >> --- a/libselinux/src/avc_internal.c
> >> +++ b/libselinux/src/avc_internal.c
> >> @@ -59,14 +59,14 @@ int avc_process_setenforce(int enforcing)
> >> int rc = 0;
> >>
> >> avc_log(SELINUX_SETENFORCE,
> >> - "%s: received setenforce notice (enforcing=%d)\n",
> >> + "%s: op=setenforce lsm=selinux_uavc enforcing=%d res=1",
> >> avc_prefix, enforcing);
> >> if (avc_setenforce)
> >> goto out;
> >> avc_enforcing = enforcing;
> >> if (avc_enforcing && (rc = avc_ss_reset(0)) < 0) {
> >> avc_log(SELINUX_ERROR,
> >> - "%s: cache reset returned %d (errno %d)\n",
> >> + "%s: op=cache_reset lsm=selinux_uavc rc=%d errno=%d res=0",
> >> avc_prefix, rc, errno);
> >
> > If we do this at all, I would think the op= would still be setenforce
> > and this would just be an error for it.
>
> At this point we already audited success for the setenforce operation. Wouldn't
> it be confusing to have a op=setenforce res=1 and then immediately op=setenforce
> res=0?
Yes. On second thought, I don't think any of the SELINUX_ERROR
messages are intended for audit and since that is already a different
type value, the callbacks can already redirect those to stderr or
syslog as appropriate instead of audit.
> > Looking at the kernel, we aren't even checking avc_ss_reset() for
> > failure and none of the kernel avc callbacks ever return an error.
>
> I'm not deeply familiar with the AVC intricacies. If the cache fails to clear,
> then there could be wrong cache entries, no? If so, I would argue that it's
> worthy of an audit. If not, then I agree that it's not auditable.
The AVC reset itself can't fail, only the callbacks registered by the
application. Original design/concept was e.g. if the object manager
could not flush all migrated permissions for some reason. Don't think
there are any such callbacks that can fail today.
