Re: [PATCH v4] selinux: support deferred mapping of contexts

Re: [PATCH v4] selinux: support deferred mapping of contexts

[Eric Paris]

On Wed, May 7, 2008 at 11:23 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
>
>
>  On Wed, 2008-05-07 at 11:17 -0400, Eric Paris wrote:
>  > >  I assume we do NOT want to use this variant interface when getting
>  > >  contexts to display in audit messages, as we want the audit messages to
>  > >  correspond to the actual denial and to yield proper policy if turned
>  > >  into an allow rule.
>  >
>  > Is there any way we could get them both displayed if there is a
>  > denial?  Might be interesting to know both that the denial was
>  > actually unlabeled_t object but also what the 'incorrect' label
>  > was.....
>
>  Easy to do kernel-side, but requires a new avc audit field that won't
>  cause any complaints by audit userland or tools like audit2allow.

Well, I'm not concerned about audit userland, if they can't handle
arbitrary users or the audit subsystem that's an audit failure.  As to
audit2allow I'm clueless but I guess i could take a look if others
think it is an interesting piece of knowledge...

-Eric

Re: [PATCH v4] selinux: support deferred mapping of contexts

[Steve Grubb]

On Wednesday 07 May 2008 11:29:36 Eric Paris wrote:
> On Wed, May 7, 2008 at 11:23 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> >  On Wed, 2008-05-07 at 11:17 -0400, Eric Paris wrote:
> >  > >  I assume we do NOT want to use this variant interface when getting
> >  > >  contexts to display in audit messages, as we want the audit
> >  > > messages to correspond to the actual denial and to yield proper
> >  > > policy if turned into an allow rule.
> >  >
> >  > Is there any way we could get them both displayed if there is a
> >  > denial?  Might be interesting to know both that the denial was
> >  > actually unlabeled_t object but also what the 'incorrect' label
> >  > was.....
> >
> >  Easy to do kernel-side, but requires a new avc audit field that won't
> >  cause any complaints by audit userland or tools like audit2allow.

What would be the proposed name of this new field? Would it hold just a 
context string? FWIW, audit user land doesn't really care except that we 
don't have name collisions on fields.

-Steve

Re: [PATCH v4] selinux: support deferred mapping of contexts

[Stephen Smalley]

On Wed, 2008-05-07 at 12:48 -0400, Steve Grubb wrote:
> On Wednesday 07 May 2008 11:29:36 Eric Paris wrote:
> > On Wed, May 7, 2008 at 11:23 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> > >  On Wed, 2008-05-07 at 11:17 -0400, Eric Paris wrote:
> > >  > >  I assume we do NOT want to use this variant interface when getting
> > >  > >  contexts to display in audit messages, as we want the audit
> > >  > > messages to correspond to the actual denial and to yield proper
> > >  > > policy if turned into an allow rule.
> > >  >
> > >  > Is there any way we could get them both displayed if there is a
> > >  > denial?  Might be interesting to know both that the denial was
> > >  > actually unlabeled_t object but also what the 'incorrect' label
> > >  > was.....
> > >
> > >  Easy to do kernel-side, but requires a new avc audit field that won't
> > >  cause any complaints by audit userland or tools like audit2allow.
> 
> What would be the proposed name of this new field? Would it hold just a 
> context string? FWIW, audit user land doesn't really care except that we 
> don't have name collisions on fields.

If we did this (not part of my current patch, but can be done as a
follow-on), then we'd need to define two new fields, one to correspond
to the real/raw context string corresponding to the scontext and one to
correspond to the real/raw context string corresponding to the tcontext.
And they would only be present if the scontext and/or tcontext happened
to be invalid under current policy.  Maybe "rscontext" and "rtcontext"
if we don't think that will confuse existing userspace
(audit2allow/sepolgen, setroubleshoot, seaudit, ...).
   
-- 
Stephen Smalley
National Security Agency

Re: [PATCH v4] selinux: support deferred mapping of contexts

[Steve Grubb]

On Wednesday 07 May 2008 13:20:42 Stephen Smalley wrote:
> then we'd need to define two new fields, one to correspond
> to the real/raw context string corresponding to the scontext and one to
> correspond to the real/raw context string corresponding to the tcontext.
> And they would only be present if the scontext and/or tcontext happened
> to be invalid under current policy.  Maybe "rscontext" and "rtcontext"
> if we don't think that will confuse existing userspace

Sounds good to me. I don't think either names you mentioned are taken.

-Steve

Re: [PATCH v4] selinux: support deferred mapping of contexts

[Stephen Smalley]

On Wed, 2008-05-07 at 14:45 -0400, Steve Grubb wrote:
> On Wednesday 07 May 2008 13:20:42 Stephen Smalley wrote:
> > then we'd need to define two new fields, one to correspond
> > to the real/raw context string corresponding to the scontext and one to
> > correspond to the real/raw context string corresponding to the tcontext.
> > And they would only be present if the scontext and/or tcontext happened
> > to be invalid under current policy.  Maybe "rscontext" and "rtcontext"
> > if we don't think that will confuse existing userspace
> 
> Sounds good to me. I don't think either names you mentioned are taken.

I created a trivial patch to do this, not the way I would do it for
real, just to see what impact if any it has on existing userland.  This
generated audit messages like this:
# scontext is not defined by current policy, show rscontext=
type=AVC msg=audit(1210258514.347:48): avc:  denied  { associate } for  pid=3352 comm="chcon" name="bar" dev=dm-1 ino=7210044 scontext=system_u:object_r:unlabeled_t:s0 rscontext=unconfined_u:object_r:foo_exec_t:s0 tcontext=system_u:object_r:fs_t:s0 tclass=filesystem
# tcontext is not defined by current policy, show rtcontext=
type=AVC msg=audit(1210258720.269:56): avc:  denied  { read } for  pid=3415 comm="cat" name="bar" dev=dm-1 ino=7210044 scontext=user_u:user_r:user_t:s0 tcontext=system_u:object_r:unlabeled_t:s0 rtcontext=unconfined_u:object_r:foo_exec_t:s0 tclass=file

audit2allow seemed to handle this fine by ignoring the extra fields.
setroubleshoot appeared to ignore/reject the messages altogether, as it
didn't report them.
seaudit complained about malformed audit.log.

The patch is below, but note that I am not asking for this patch to be
merged - it was just the quick and easy way to experiment with adding
this information.  To do it for real, I would create an extended form of
security_sid_to_context_force() that gives back both context strings in
a single call, with the rcontext left NULL if the context was valid
under policy.

diff --git a/security/selinux/avc.c b/security/selinux/avc.c
index 114b4b4..995d42f 100644
--- a/security/selinux/avc.c
+++ b/security/selinux/avc.c
@@ -199,23 +199,35 @@ static void avc_dump_av(struct audit_buffer *ab, u16 tclass, u32 av)
 static void avc_dump_query(struct audit_buffer *ab, u32 ssid, u32 tsid, u16 tclass)
 {
 	int rc;
-	char *scontext;
-	u32 scontext_len;
+	char *context, *rcontext;
+	u32 context_len;
 
-	rc = security_sid_to_context(ssid, &scontext, &scontext_len);
+	rc = security_sid_to_context(ssid, &context, &context_len);
 	if (rc)
 		audit_log_format(ab, "ssid=%d", ssid);
 	else {
-		audit_log_format(ab, "scontext=%s", scontext);
-		kfree(scontext);
+		audit_log_format(ab, "scontext=%s", context);
+		rc = security_sid_to_context_force(ssid, &rcontext, &context_len);
+		if (!rc) {
+			if (strcmp(context, rcontext))
+				audit_log_format(ab, " rscontext=%s", rcontext);
+			kfree(rcontext);
+		}
+		kfree(context);
 	}
 
-	rc = security_sid_to_context(tsid, &scontext, &scontext_len);
+	rc = security_sid_to_context(tsid, &context, &context_len);
 	if (rc)
 		audit_log_format(ab, " tsid=%d", tsid);
 	else {
-		audit_log_format(ab, " tcontext=%s", scontext);
-		kfree(scontext);
+		audit_log_format(ab, " tcontext=%s", context);
+		rc = security_sid_to_context_force(tsid, &rcontext, &context_len);
+		if (!rc) {
+			if (strcmp(context, rcontext))
+				audit_log_format(ab, " rtcontext=%s", rcontext);
+			kfree(rcontext);
+		}
+		kfree(context);
 	}
 
 	BUG_ON(tclass >= ARRAY_SIZE(class_to_string) || !class_to_string[tclass]);


-- 
Stephen Smalley
National Security Agency