From: Ondrej Mosnacek <omosnace@redhat.com>
To: Paul Moore <paul@paul-moore.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 10:53:08 +0100 [thread overview]
Message-ID: <CAFqZXNtkUC88X9sfr4Kaq=QDN-H9G=xHKEntyE2mfvC5S0aVXQ@mail.gmail.com> (raw)
In-Reply-To: <CAHC9VhRDSMeojFEFz+8rshiP4TmtoGuEuOK0AwZecyJ0SWN3Ug@mail.gmail.com>
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 :)
>
> Also, under what cases would we ever see a srawcon field? This is
> only going to happen if we have a running process whose domain is
> removed during a policy reload, correct? I'm find with including this
> for the sake of completeness, but I would mention this in the patch
> description for the next revision.
I tried to find a similar reproducer as for trawcon, but it doesn't
seem to be possible to do that without reloading the policy. I will
just add a note.
>
> > Cc: Daniel Walsh <dwalsh@redhat.com>
> > Link: https://bugzilla.redhat.com/show_bug.cgi?id=1135683
> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> > ---
> >
> > v2: Rename fields to "(s|t)rawcon".
> >
> > security/selinux/avc.c | 49 +++++++++++++++++++++++++-----------------
> > 1 file changed, 29 insertions(+), 20 deletions(-)
> >
> > diff --git a/security/selinux/avc.c b/security/selinux/avc.c
> > index 9b63d8ee1687..df5490db575b 100644
> > --- a/security/selinux/avc.c
> > +++ b/security/selinux/avc.c
> > @@ -165,6 +165,32 @@ static void avc_dump_av(struct audit_buffer *ab, u16 tclass, u32 av)
> > audit_log_format(ab, " }");
> > }
> >
> > +static void avc_dump_sid(struct audit_buffer *ab, struct selinux_state *state,
> > + u32 sid, char type)
> > +{
> > + int rc;
> > + char *context, *rcontext;
> > + u32 context_len, rcontext_len;
> > +
> > + rc = security_sid_to_context(state, sid, &context, &context_len);
> > + if (rc) {
> > + audit_log_format(ab, "%csid=%d ", type, sid);
> > + return;
> > + }
> > +
> > + audit_log_format(ab, "%ccontext=%s ", type, context);
> > +
> > + /* in case of invalid context report also the actual context string */
> > + rc = security_sid_to_context_force(state, sid, &rcontext,
> > + &rcontext_len);
> > + if (!rc) {
> > + if (strcmp(context, rcontext))
> > + audit_log_format(ab, "%crawcon=%s ", type, rcontext);
> > + kfree(rcontext);
> > + }
> > + kfree(context);
> > +}
> > +
> > /**
> > * avc_dump_query - Display a SID pair and a class in human-readable form.
> > * @ssid: source security identifier
> > @@ -174,28 +200,11 @@ static void avc_dump_av(struct audit_buffer *ab, u16 tclass, u32 av)
> > static void avc_dump_query(struct audit_buffer *ab, struct selinux_state *state,
> > u32 ssid, u32 tsid, u16 tclass)
> > {
> > - int rc;
> > - char *scontext;
> > - u32 scontext_len;
> > -
> > - rc = security_sid_to_context(state, ssid, &scontext, &scontext_len);
> > - if (rc)
> > - audit_log_format(ab, "ssid=%d", ssid);
> > - else {
> > - audit_log_format(ab, "scontext=%s", scontext);
> > - kfree(scontext);
> > - }
> > -
> > - rc = security_sid_to_context(state, tsid, &scontext, &scontext_len);
> > - if (rc)
> > - audit_log_format(ab, " tsid=%d", tsid);
> > - else {
> > - audit_log_format(ab, " tcontext=%s", scontext);
> > - kfree(scontext);
> > - }
> > + avc_dump_sid(ab, state, ssid, 's');
> > + avc_dump_sid(ab, state, tsid, 't');
> >
> > BUG_ON(!tclass || tclass >= ARRAY_SIZE(secclass_map));
> > - audit_log_format(ab, " tclass=%s", secclass_map[tclass-1].name);
> > + audit_log_format(ab, "tclass=%s", secclass_map[tclass-1].name);
> > }
> >
> > /**
> > --
> > 2.20.1
>
> --
> paul moore
> www.paul-moore.com
--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.
next prev parent reply other threads:[~2019-01-25 9:53 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-21 15:36 [PATCH v2] selinux: log invalid contexts in AVCs Ondrej Mosnacek
2019-01-22 19:42 ` Paul Moore
2019-01-22 20:00 ` Stephen Smalley
2019-01-25 9:53 ` Ondrej Mosnacek [this message]
2019-01-25 17:31 ` Paul Moore
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAFqZXNtkUC88X9sfr4Kaq=QDN-H9G=xHKEntyE2mfvC5S0aVXQ@mail.gmail.com' \
--to=omosnace@redhat.com \
--cc=dwalsh@redhat.com \
--cc=linux-audit@redhat.com \
--cc=paul@paul-moore.com \
--cc=sds@tycho.nsa.gov \
--cc=selinux@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).