Re: [PATCH v2] selinux: log invalid contexts in AVCs

From: Paul Moore <paul@paul-moore.com>
To: Ondrej Mosnacek <omosnace@redhat.com>
Cc: selinux@vger.kernel.org, Stephen Smalley <sds@tycho.nsa.gov>,
	Linux-Audit Mailing List <linux-audit@redhat.com>,
	Daniel Walsh <dwalsh@redhat.com>
Subject: Re: [PATCH v2] selinux: log invalid contexts in AVCs
Date: Fri, 25 Jan 2019 12:31:21 -0500	[thread overview]
Message-ID: <CAHC9VhT-4mujhEJz3XcmOJBGc7dhOcoxkkoJ_ZU=+TgKSxv2Aw@mail.gmail.com> (raw)
In-Reply-To: <CAFqZXNtkUC88X9sfr4Kaq=QDN-H9G=xHKEntyE2mfvC5S0aVXQ@mail.gmail.com>

On Fri, Jan 25, 2019 at 4:53 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> On Tue, Jan 22, 2019 at 8:42 PM Paul Moore <paul@paul-moore.com> wrote:
> > On Mon, Jan 21, 2019 at 10:36 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > > In case a file has an invalid context set, in an AVC record generated
> > > upon access to such file, the target context is always reported as
> > > unlabeled. This patch adds new optional fields to the AVC record
> > > (srawcon and trawcon) that report the actual context string if it
> > > differs from the one reported in scontext/tcontext. This is useful for
> > > diagnosing SELinux denials involving invalid contexts.
> > >
> > > To trigger an AVC that illustrates this situation:
> > >
> > >     # setenforce 0
> > >     # touch /tmp/testfile
> > >     # setfattr -n security.selinux -v system_u:object_r:banana_t:s0 /tmp/testfile
> > >     # runcon system_u:system_r:sshd_t:s0 cat /tmp/testfile
> > >
> > > AVC before:
> > >
> > > type=AVC msg=audit(1547801083.248:11): avc:  denied  { open } for  pid=1149 comm="cat" path="/tmp/testfile" dev="tmpfs" ino=6608 scontext=system_u:system_r:sshd_t:s0 tcontext=system_u:object_r:unlabeled_t:s15:c0.c1023 tclass=file permissive=1
> > >
> > > AVC after:
> > >
> > > type=AVC msg=audit(1547801083.248:11): avc:  denied  { open } for  pid=1149 comm="cat" path="/tmp/testfile" dev="tmpfs" ino=6608 scontext=system_u:system_r:sshd_t:s0 tcontext=system_u:object_r:unlabeled_t:s15:c0.c1023 trawcon=system_u:object_r:banana_t:s0 tclass=file permissive=1
> >
> > I would like us to add new fields at the end of existing records; the
> > recent audit config changes are a bit of a special case as discussed
> > previously.
>
> Okay, I happened to find a way to do this a little differently (taking
> a suggestion from Stephen about avoiding the need to do strcmp()) so
> now it is actually easy to move them at the end. But I didn't expect
> to get a more liberal reply from Steve (who is usually more strict
> about this) than you :)

Yeah, the audit record format is a delicate subject with lots of
disagreement between Steve and I.  I think you've seen some of that
since you've been involved in audit, but it goes back years.

The general rule that I've been sticking to is that new fields get
added to the end of the record.  There are exceptions, e.g. the config
records, but those exceptions are typically only given in the case of
a record format that is so irregular it really doesn't matter.

-- 
paul moore
www.paul-moore.com