Re: type bounds audit messages

Re: type bounds audit messages

[KaiGai Kohei]

The attached patch allows to generate audit messages on access violations
related to bounds types.

1. When a multithread process gives an unbounded domain to setcon(3)
   to change its domain dynamically, the current kernel denies it
   without any notification or audit messages.
   This patch adds an audit_log() in the security_bounded_transition()
   to generate an audit message, when the dynamic type transition is
   failed due to the bounds violation.

   Example:
   type=SELINUX_ERR msg=audit(1245046106.725:65): SELinux: bounds violation: \
       domain transition from httpd_t to guest_webapp_t

2. When a set of permissions are masked due to the bounds violations,
   it shall be reported on the type_attribute_bounds_av() invoked from
   context_struct_context_av(), but it keeps silent on any AVC denials.
   It may make unclear what permissions were in violation of the boundary.
   This patch adds the "masked" field on av_decision, and it reports
   violated permissions on AVC denials.

   Example:
   type=AVC msg=audit(1245046439.315:72): avc:  denied  { create }     \
     for pid=3080 comm="httpd" name="hoge"                             \
     scontext=unconfined_u:system_r:user_webapp_t:s0                   \
     tcontext=unconfined_u:object_r:httpd_sys_content_t:s0 tclass=file \
     bounds: { create }
     ^^^^^^^^^^^^^^^^^^

Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com>
--
 security/selinux/avc.c              |    7 ++++-
 security/selinux/include/security.h |    1 +
 security/selinux/ss/services.c      |   47 ++++++++++++++--------------------
 3 files changed, 26 insertions(+), 29 deletions(-)

diff --git a/security/selinux/avc.c b/security/selinux/avc.c
index 7f9b5fa..c385e3b 100644
--- a/security/selinux/avc.c
+++ b/security/selinux/avc.c
@@ -536,7 +536,7 @@ void avc_audit(u32 ssid, u32 tsid,
 {
 	struct task_struct *tsk = current;
 	struct inode *inode = NULL;
-	u32 denied, audited;
+	u32 denied, audited, masked = 0;
 	struct audit_buffer *ab;

 	denied = requested & ~avd->allowed;
@@ -544,6 +544,7 @@ void avc_audit(u32 ssid, u32 tsid,
 		audited = denied;
 		if (!(audited & avd->auditdeny))
 			return;
+		masked = audited & avd->masked;
 	} else if (result) {
 		audited = denied = requested;
 	} else {
@@ -688,6 +689,10 @@ void avc_audit(u32 ssid, u32 tsid,
 	}
 	audit_log_format(ab, " ");
 	avc_dump_query(ab, ssid, tsid, tclass);
+	if (masked) {
+		audit_log_format(ab, " bounds:");
+		avc_dump_av(ab, tclass, masked);
+	}
 	audit_log_end(ab);
 }

diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 5c3434f..07e7a01 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -91,6 +91,7 @@ struct av_decision {
 	u32 auditallow;
 	u32 auditdeny;
 	u32 seqno;
+	u32 masked;	/* masked by bounds types */
 };

 int security_permissive_sid(u32 sid);
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index deeec6c..349bfb2 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -295,7 +295,6 @@ static void type_attribute_bounds_av(struct context *scontext,
 		= policydb.type_val_to_struct[scontext->type - 1];
 	struct type_datum *target
 		= policydb.type_val_to_struct[tcontext->type - 1];
-	u32 masked = 0;

 	if (source->bounds) {
 		memset(&lo_avd, 0, sizeof(lo_avd));
@@ -310,7 +309,7 @@ static void type_attribute_bounds_av(struct context *scontext,
 					  &lo_avd);
 		if ((lo_avd.allowed & avd->allowed) == avd->allowed)
 			return;		/* no masked permission */
-		masked = ~lo_avd.allowed & avd->allowed;
+		avd->masked = ~lo_avd.allowed & avd->allowed;
 	}

 	if (target->bounds) {
@@ -326,7 +325,7 @@ static void type_attribute_bounds_av(struct context *scontext,
 					  &lo_avd);
 		if ((lo_avd.allowed & avd->allowed) == avd->allowed)
 			return;		/* no masked permission */
-		masked = ~lo_avd.allowed & avd->allowed;
+		avd->masked = ~lo_avd.allowed & avd->allowed;
 	}

 	if (source->bounds && target->bounds) {
@@ -343,33 +342,12 @@ static void type_attribute_bounds_av(struct context *scontext,
 					  &lo_avd);
 		if ((lo_avd.allowed & avd->allowed) == avd->allowed)
 			return;		/* no masked permission */
-		masked = ~lo_avd.allowed & avd->allowed;
+		avd->masked = ~lo_avd.allowed & avd->allowed;
 	}

-	if (masked) {
-		struct audit_buffer *ab;
-		char *stype_name
-			= policydb.p_type_val_to_name[source->value - 1];
-		char *ttype_name
-			= policydb.p_type_val_to_name[target->value - 1];
-		char *tclass_name
-			= policydb.p_class_val_to_name[tclass - 1];
-
-		/* mask violated permissions */
-		avd->allowed &= ~masked;
-
-		/* notice to userspace via audit message */
-		ab = audit_log_start(current->audit_context,
-				     GFP_ATOMIC, AUDIT_SELINUX_ERR);
-		if (!ab)
-			return;
-
-		audit_log_format(ab, "av boundary violation: "
-				 "source=%s target=%s tclass=%s",
-				 stype_name, ttype_name, tclass_name);
-		avc_dump_av(ab, tclass, masked);
-		audit_log_end(ab);
-	}
+	/* mask violated permissions */
+	if (avd->masked)
+		avd->allowed &= ~avd->masked;
 }

 /*
@@ -410,6 +388,7 @@ static int context_struct_compute_av(struct context *scontext,
 	avd->auditallow = 0;
 	avd->auditdeny = 0xffffffff;
 	avd->seqno = latest_granting;
+	avd->masked = 0;

 	/*
 	 * Check for all the invalid cases.
@@ -711,6 +690,18 @@ int security_bounded_transition(u32 old_sid, u32 new_sid)
 		}
 		index = type->bounds;
 	}
+
+	if (rc) {
+		char *old_name
+			= policydb.p_type_val_to_name[old_context->type - 1];
+		char *new_name
+			= policydb.p_type_val_to_name[new_context->type - 1];
+
+		audit_log(current->audit_context,
+			  GFP_ATOMIC, AUDIT_SELINUX_ERR,
+			  "SELinux: bounds violation: domain transition "
+			  "from %s to %s", old_name, new_name);
+	}
 out:
 	read_unlock(&policy_rwlock);

-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: type bounds audit messages

[Eric Paris]

On Mon, 2009-06-15 at 15:56 +0900, KaiGai Kohei wrote:
> The attached patch allows to generate audit messages on access violations
> related to bounds types.
> 
> 1. When a multithread process gives an unbounded domain to setcon(3)
>    to change its domain dynamically, the current kernel denies it
>    without any notification or audit messages.
>    This patch adds an audit_log() in the security_bounded_transition()
>    to generate an audit message, when the dynamic type transition is
>    failed due to the bounds violation.
> 
>    Example:
>    type=SELINUX_ERR msg=audit(1245046106.725:65): SELinux: bounds violation: \
>        domain transition from httpd_t to guest_webapp_t

No, no 1000 times no.  We've finally got a new SELinux audit message
that tools don't understand.  I'm not about to suggest we continue to
print them in some new non-standard arbitrary format.  Everything that
includes audit_log_* from now on better be of the type key=value.

How would people on list feel about?

type=SELINUX_ERR msg=audit(1245046106.725:65): lsm="SELinux" \
    op="bounds violation on domain transition" type1="httpd_t" \
    type2="guest_webapp_t"

This would be the only place I can remember that we only output the type
instead of the complete context.  Why not ocontext= and ncontext=?  I
don't care what we call it, but I want it all to be key=value pairs and
I prefer that we (the audit system) starts making much heavier use of
audit_log_string() which includes the ""

> 2. When a set of permissions are masked due to the bounds violations,
>    it shall be reported on the type_attribute_bounds_av() invoked from
>    context_struct_context_av(), but it keeps silent on any AVC denials.
>    It may make unclear what permissions were in violation of the boundary.
>    This patch adds the "masked" field on av_decision, and it reports
>    violated permissions on AVC denials.
> 
>    Example:
>    type=AVC msg=audit(1245046439.315:72): avc:  denied  { create }     \
>      for pid=3080 comm="httpd" name="hoge"                             \
>      scontext=unconfined_u:system_r:user_webapp_t:s0                   \
>      tcontext=unconfined_u:object_r:httpd_sys_content_t:s0 tclass=file \
>      bounds: { create }
>      ^^^^^^^^^^^^^^^^^^

This one I'm still unhappy about but since it is continuing the
tradition of hard to parse audit rules I'm ok if it stays (assuming
tools like audit2allow don't get confused)

Now might be a perfect time to start emitting permissions in a better
format though, maybe someday the rest of selinux could convert to an
easier to parse format (ha ha, ok, I know it's funny)

bounds="create"

someday we might have perms="read write create open ioctl" instead of
{ ... } ???

???

Feedback strongly encouraged...

> Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com>
> --
>  security/selinux/avc.c              |    7 ++++-
>  security/selinux/include/security.h |    1 +
>  security/selinux/ss/services.c      |   47 ++++++++++++++--------------------
>  3 files changed, 26 insertions(+), 29 deletions(-)
> 
> diff --git a/security/selinux/avc.c b/security/selinux/avc.c
> index 7f9b5fa..c385e3b 100644
> --- a/security/selinux/avc.c
> +++ b/security/selinux/avc.c
> @@ -536,7 +536,7 @@ void avc_audit(u32 ssid, u32 tsid,
>  {
>  	struct task_struct *tsk = current;
>  	struct inode *inode = NULL;
> -	u32 denied, audited;
> +	u32 denied, audited, masked = 0;
>  	struct audit_buffer *ab;
> 
>  	denied = requested & ~avd->allowed;
> @@ -544,6 +544,7 @@ void avc_audit(u32 ssid, u32 tsid,
>  		audited = denied;
>  		if (!(audited & avd->auditdeny))
>  			return;
> +		masked = audited & avd->masked;
>  	} else if (result) {
>  		audited = denied = requested;
>  	} else {
> @@ -688,6 +689,10 @@ void avc_audit(u32 ssid, u32 tsid,
>  	}
>  	audit_log_format(ab, " ");
>  	avc_dump_query(ab, ssid, tsid, tclass);
> +	if (masked) {
> +		audit_log_format(ab, " bounds:");
> +		avc_dump_av(ab, tclass, masked);
> +	}
>  	audit_log_end(ab);
>  }
> 
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index 5c3434f..07e7a01 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -91,6 +91,7 @@ struct av_decision {
>  	u32 auditallow;
>  	u32 auditdeny;
>  	u32 seqno;
> +	u32 masked;	/* masked by bounds types */
>  };
> 
>  int security_permissive_sid(u32 sid);
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index deeec6c..349bfb2 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -295,7 +295,6 @@ static void type_attribute_bounds_av(struct context *scontext,
>  		= policydb.type_val_to_struct[scontext->type - 1];
>  	struct type_datum *target
>  		= policydb.type_val_to_struct[tcontext->type - 1];
> -	u32 masked = 0;
> 
>  	if (source->bounds) {
>  		memset(&lo_avd, 0, sizeof(lo_avd));
> @@ -310,7 +309,7 @@ static void type_attribute_bounds_av(struct context *scontext,
>  					  &lo_avd);
>  		if ((lo_avd.allowed & avd->allowed) == avd->allowed)
>  			return;		/* no masked permission */
> -		masked = ~lo_avd.allowed & avd->allowed;
> +		avd->masked = ~lo_avd.allowed & avd->allowed;
>  	}
> 
>  	if (target->bounds) {
> @@ -326,7 +325,7 @@ static void type_attribute_bounds_av(struct context *scontext,
>  					  &lo_avd);
>  		if ((lo_avd.allowed & avd->allowed) == avd->allowed)
>  			return;		/* no masked permission */
> -		masked = ~lo_avd.allowed & avd->allowed;
> +		avd->masked = ~lo_avd.allowed & avd->allowed;
>  	}
> 
>  	if (source->bounds && target->bounds) {
> @@ -343,33 +342,12 @@ static void type_attribute_bounds_av(struct context *scontext,
>  					  &lo_avd);
>  		if ((lo_avd.allowed & avd->allowed) == avd->allowed)
>  			return;		/* no masked permission */
> -		masked = ~lo_avd.allowed & avd->allowed;
> +		avd->masked = ~lo_avd.allowed & avd->allowed;
>  	}
> 
> -	if (masked) {
> -		struct audit_buffer *ab;
> -		char *stype_name
> -			= policydb.p_type_val_to_name[source->value - 1];
> -		char *ttype_name
> -			= policydb.p_type_val_to_name[target->value - 1];
> -		char *tclass_name
> -			= policydb.p_class_val_to_name[tclass - 1];
> -
> -		/* mask violated permissions */
> -		avd->allowed &= ~masked;
> -
> -		/* notice to userspace via audit message */
> -		ab = audit_log_start(current->audit_context,
> -				     GFP_ATOMIC, AUDIT_SELINUX_ERR);
> -		if (!ab)
> -			return;
> -
> -		audit_log_format(ab, "av boundary violation: "
> -				 "source=%s target=%s tclass=%s",
> -				 stype_name, ttype_name, tclass_name);
> -		avc_dump_av(ab, tclass, masked);
> -		audit_log_end(ab);
> -	}
> +	/* mask violated permissions */
> +	if (avd->masked)
> +		avd->allowed &= ~avd->masked;
>  }
> 
>  /*
> @@ -410,6 +388,7 @@ static int context_struct_compute_av(struct context *scontext,
>  	avd->auditallow = 0;
>  	avd->auditdeny = 0xffffffff;
>  	avd->seqno = latest_granting;
> +	avd->masked = 0;
> 
>  	/*
>  	 * Check for all the invalid cases.
> @@ -711,6 +690,18 @@ int security_bounded_transition(u32 old_sid, u32 new_sid)
>  		}
>  		index = type->bounds;
>  	}
> +
> +	if (rc) {
> +		char *old_name
> +			= policydb.p_type_val_to_name[old_context->type - 1];
> +		char *new_name
> +			= policydb.p_type_val_to_name[new_context->type - 1];
> +
> +		audit_log(current->audit_context,
> +			  GFP_ATOMIC, AUDIT_SELINUX_ERR,
> +			  "SELinux: bounds violation: domain transition "
> +			  "from %s to %s", old_name, new_name);
> +	}
>  out:
>  	read_unlock(&policy_rwlock);
> 


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: type bounds audit messages

[Stephen Smalley]

On Mon, 2009-06-15 at 09:17 -0400, Eric Paris wrote:
> On Mon, 2009-06-15 at 15:56 +0900, KaiGai Kohei wrote:
> > The attached patch allows to generate audit messages on access violations
> > related to bounds types.
> > 
> > 1. When a multithread process gives an unbounded domain to setcon(3)
> >    to change its domain dynamically, the current kernel denies it
> >    without any notification or audit messages.
> >    This patch adds an audit_log() in the security_bounded_transition()
> >    to generate an audit message, when the dynamic type transition is
> >    failed due to the bounds violation.
> > 
> >    Example:
> >    type=SELINUX_ERR msg=audit(1245046106.725:65): SELinux: bounds violation: \
> >        domain transition from httpd_t to guest_webapp_t
> 
> No, no 1000 times no.  We've finally got a new SELinux audit message
> that tools don't understand.  I'm not about to suggest we continue to
> print them in some new non-standard arbitrary format.  Everything that
> includes audit_log_* from now on better be of the type key=value.
> 
> How would people on list feel about?
> 
> type=SELINUX_ERR msg=audit(1245046106.725:65): lsm="SELinux" \
>     op="bounds violation on domain transition" type1="httpd_t" \
>     type2="guest_webapp_t"

Do we really need lsm="SELinux" given type=SELINUX_ERR?  Or is that for
the case where auditd isn't running and we lose the type= prefix
information?

> This would be the only place I can remember that we only output the type
> instead of the complete context.  Why not ocontext= and ncontext=?  I
> don't care what we call it, but I want it all to be key=value pairs and
> I prefer that we (the audit system) starts making much heavier use of
> audit_log_string() which includes the ""
> 
> > 2. When a set of permissions are masked due to the bounds violations,
> >    it shall be reported on the type_attribute_bounds_av() invoked from
> >    context_struct_context_av(), but it keeps silent on any AVC denials.
> >    It may make unclear what permissions were in violation of the boundary.
> >    This patch adds the "masked" field on av_decision, and it reports
> >    violated permissions on AVC denials.
> > 
> >    Example:
> >    type=AVC msg=audit(1245046439.315:72): avc:  denied  { create }     \
> >      for pid=3080 comm="httpd" name="hoge"                             \
> >      scontext=unconfined_u:system_r:user_webapp_t:s0                   \
> >      tcontext=unconfined_u:object_r:httpd_sys_content_t:s0 tclass=file \
> >      bounds: { create }
> >      ^^^^^^^^^^^^^^^^^^
> 
> This one I'm still unhappy about but since it is continuing the
> tradition of hard to parse audit rules I'm ok if it stays (assuming
> tools like audit2allow don't get confused)
> 
> Now might be a perfect time to start emitting permissions in a better
> format though, maybe someday the rest of selinux could convert to an
> easier to parse format (ha ha, ok, I know it's funny)
> 
> bounds="create"
> 
> someday we might have perms="read write create open ioctl" instead of
> { ... } ???
> 
> ???

This is interesting - I was expecting KaiGai to just add the masked
permissions to the existing av boundary violation message generated
during compute_av processing, not export the information up to the AVC.
On the one hand, this approach is nice in that it links the information
directly to the particular AVC it caused.  However, if the masked
permissions are never checked by the AVC, then we'll never get
notification of the boundary violation in the policy.  Also, this only
addresses kernel denials and would require an extension to
the /selinux/access interface to export the same information to the
userspace AVC, along with corresponding changes to the userspace AVC.

So I have to ask whether this is worth it, or if we should just add a
simple helper function to map the masked permissions to strings and call
it from the existing av boundary violation message generation?  See
sepol_av_to_string() in libsepol for example code of mapping an access
vector to string based on the policydb rather than using the kernel
definition tables (required if we do it from compute_av since they might
not be kernel permissions).

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: type bounds audit messages

[Steve Grubb]

On Monday 15 June 2009 09:17:15 am Eric Paris wrote:
> On Mon, 2009-06-15 at 15:56 +0900, KaiGai Kohei wrote:
> > The attached patch allows to generate audit messages on access violations
> > related to bounds types.
> >
> > 1. When a multithread process gives an unbounded domain to setcon(3)
> >    to change its domain dynamically, the current kernel denies it
> >    without any notification or audit messages.

Oops. This should be fixed. Eventually this will go through a CC evaluation and 
it will require access violations to be audited.


> >    This patch adds an audit_log() in the security_bounded_transition()
> >    to generate an audit message, when the dynamic type transition is
> >    failed due to the bounds violation.
> >
> >    Example:
> >    type=SELINUX_ERR msg=audit(1245046106.725:65): SELinux: bounds
> > violation: \ domain transition from httpd_t to guest_webapp_t

SELINUX_ERR audit type is for SE Linux error conditions and nothing else. This 
is an access violation so it should probably be an avc or a new type.

> Everything that includes audit_log_* from now on better be of the type
> key=value.

Agreed.


> How would people on list feel about?
>
> type=SELINUX_ERR msg=audit(1245046106.725:65): lsm="SELinux" \

The type should be changed. Also, the lsm field is not required because the 
audit type is in the SE Linux block.

>     op="bounds violation on domain transition" type1="httpd_t" \
>     type2="guest_webapp_t"
>
> This would be the only place I can remember that we only output the type
> instead of the complete context.  Why not ocontext= and ncontext=?  I
> don't care what we call it, but I want it all to be key=value pairs and
> I prefer that we (the audit system) starts making much heavier use of
> audit_log_string() which includes the ""

This is required if a non-admin can influence the field contents in any way. But 
we have to have information on the subject, object, what kind of access, and 
what the decision was. We also need to identify who did it and what session 
its related to. This can be done with a syscall audit record.


> > 2. When a set of permissions are masked due to the bounds violations,
> >    it shall be reported on the type_attribute_bounds_av() invoked from
> >    context_struct_context_av(), but it keeps silent on any AVC denials.
> >    It may make unclear what permissions were in violation of the
> > boundary. This patch adds the "masked" field on av_decision, and it
> > reports violated permissions on AVC denials.
> >
> >    Example:
> >    type=AVC msg=audit(1245046439.315:72): avc:  denied  { create }     \
> >      for pid=3080 comm="httpd" name="hoge"                             \
> >      scontext=unconfined_u:system_r:user_webapp_t:s0                   \
> >      tcontext=unconfined_u:object_r:httpd_sys_content_t:s0 tclass=file \
> >      bounds: { create }
> >      ^^^^^^^^^^^^^^^^^^
>
> This one I'm still unhappy about but since it is continuing the
> tradition of hard to parse audit rules I'm ok if it stays (assuming
> tools like audit2allow don't get confused)

That last field should be something like bounds=create. There is an audit 
parsing library that makes parsing audit events very easy. It has python 
bindings, too.


> Now might be a perfect time to start emitting permissions in a better
> format though, maybe someday the rest of selinux could convert to an
> easier to parse format (ha ha, ok, I know it's funny)
>
> bounds="create"
>
> someday we might have perms="read write create open ioctl" instead of
> { ... } ???

Make that comma separated instead of space separated and I'm happy.

Thanks,
-Steve

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: type bounds audit messages

[Eric Paris]

On Mon, 2009-06-15 at 10:01 -0400, Stephen Smalley wrote:
> On Mon, 2009-06-15 at 09:17 -0400, Eric Paris wrote:
> > On Mon, 2009-06-15 at 15:56 +0900, KaiGai Kohei wrote:
> > > The attached patch allows to generate audit messages on access violations
> > > related to bounds types.
> > > 
> > > 1. When a multithread process gives an unbounded domain to setcon(3)
> > >    to change its domain dynamically, the current kernel denies it
> > >    without any notification or audit messages.
> > >    This patch adds an audit_log() in the security_bounded_transition()
> > >    to generate an audit message, when the dynamic type transition is
> > >    failed due to the bounds violation.
> > > 
> > >    Example:
> > >    type=SELINUX_ERR msg=audit(1245046106.725:65): SELinux: bounds violation: \
> > >        domain transition from httpd_t to guest_webapp_t
> > 
> > No, no 1000 times no.  We've finally got a new SELinux audit message
> > that tools don't understand.  I'm not about to suggest we continue to
> > print them in some new non-standard arbitrary format.  Everything that
> > includes audit_log_* from now on better be of the type key=value.
> > 
> > How would people on list feel about?
> > 
> > type=SELINUX_ERR msg=audit(1245046106.725:65): lsm="SELinux" \
> >     op="bounds violation on domain transition" type1="httpd_t" \
> >     type2="guest_webapp_t"
> 
> Do we really need lsm="SELinux" given type=SELINUX_ERR?  Or is that for
> the case where auditd isn't running and we lose the type= prefix
> information?

no we don't 'need' it.  2.6.31 fixes the kernel to always emit type=XXXX
even if we are doing it through printk instead of through auditd.
(currently I believe the kernel emits type=XXXX if you stop auditd, but
it isn't emitted if you never start it, isn't that fun?)

Problem with the printk type= support is that it emits the numerical
value, not a human readable string translation.  Similar reason we left
lsm=SMACK in their record.

Maybe I'm getting a little too happy with the " though in my examples :)


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: type bounds audit messages

[KaiGai Kohei]

Stephen Smalley wrote:
>>> 2. When a set of permissions are masked due to the bounds violations,
>>>    it shall be reported on the type_attribute_bounds_av() invoked from
>>>    context_struct_context_av(), but it keeps silent on any AVC denials.
>>>    It may make unclear what permissions were in violation of the boundary.
>>>    This patch adds the "masked" field on av_decision, and it reports
>>>    violated permissions on AVC denials.
>>>
>>>    Example:
>>>    type=AVC msg=audit(1245046439.315:72): avc:  denied  { create }     \
>>>      for pid=3080 comm="httpd" name="hoge"                             \
>>>      scontext=unconfined_u:system_r:user_webapp_t:s0                   \
>>>      tcontext=unconfined_u:object_r:httpd_sys_content_t:s0 tclass=file \
>>>      bounds: { create }
>>>      ^^^^^^^^^^^^^^^^^^
>> This one I'm still unhappy about but since it is continuing the
>> tradition of hard to parse audit rules I'm ok if it stays (assuming
>> tools like audit2allow don't get confused)
>>
>> Now might be a perfect time to start emitting permissions in a better
>> format though, maybe someday the rest of selinux could convert to an
>> easier to parse format (ha ha, ok, I know it's funny)
>>
>> bounds="create"
>>
>> someday we might have perms="read write create open ioctl" instead of
>> { ... } ???
>>
>> ???
> 
> This is interesting - I was expecting KaiGai to just add the masked
> permissions to the existing av boundary violation message generated
> during compute_av processing, not export the information up to the AVC.

It might be discussed where/when/what we should generate the audit message
on bounds violation prior to the patch.

I don't have any strong idea about message format, so I'll follow the
conclusion of the suggestion. :)

> On the one hand, this approach is nice in that it links the information
> directly to the particular AVC it caused.  However, if the masked
> permissions are never checked by the AVC, then we'll never get
> notification of the boundary violation in the policy.  Also, this only
> addresses kernel denials and would require an extension to
> the /selinux/access interface to export the same information to the
> userspace AVC, along with corresponding changes to the userspace AVC.

Yes, indeed, it also requires an extension on security_compute_av()
for userspaces, although we already have a derivative version :(

>From viewpoint of the architecture, who should report the masked
permissions? I reconsidered it should be a role of security server,
not AVCs.

> So I have to ask whether this is worth it, or if we should just add a
> simple helper function to map the masked permissions to strings and call
> it from the existing av boundary violation message generation?  See
> sepol_av_to_string() in libsepol for example code of mapping an access
> vector to string based on the policydb rather than using the kernel
> definition tables (required if we do it from compute_av since they might
> not be kernel permissions).

In my preference, all the masked permissions (including ones for userspaces)
should be notified on security_compute_av() time, because it is always
generated prior to actual AVC denials. It means audit log analyzer can
find the reason why AVC denials using past logs.
In addition, it is also possible to notify the reason why the permissions
were masked more than type boundary, such as RBAC, MLS, Constratins.

For example, how do you feel the example on security_compute_av() time?

type=SELINUX_INFO msg=audit(1245046106.725:65):       \
  op=security_compute_av masked=bounds                \
  scontext=system_u:system_r:user_webapp_t:s0         \
  tcontext=system_u:object_r:httpd_sys_content_t:s0   \
  tclass=file { setattr write }

Thanks,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: type bounds audit messages

[Eric Paris]

On Tue, 2009-06-16 at 09:43 +0900, KaiGai Kohei wrote:
> Stephen Smalley wrote:

> For example, how do you feel the example on security_compute_av() time?
> 
> type=SELINUX_INFO msg=audit(1245046106.725:65):       \
>   op=security_compute_av masked=bounds                \
>   scontext=system_u:system_r:user_webapp_t:s0         \
>   tcontext=system_u:object_r:httpd_sys_content_t:s0   \
>   tclass=file { setattr write }

I feel good for all but the { setattr write }

It's a new message, we have no parsers which need the old format, how
would others feel about

perm="setattr,write"   ?

-Eric


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: type bounds audit messages

[Steve Grubb]

On Tuesday 16 June 2009 10:26:46 am Eric Paris wrote:
> On Tue, 2009-06-16 at 09:43 +0900, KaiGai Kohei wrote:
> > Stephen Smalley wrote:
> >
> > For example, how do you feel the example on security_compute_av() time?
> >
> > type=SELINUX_INFO msg=audit(1245046106.725:65):       \
> >   op=security_compute_av masked=bounds                \
> >   scontext=system_u:system_r:user_webapp_t:s0         \
> >   tcontext=system_u:object_r:httpd_sys_content_t:s0   \
> >   tclass=file { setattr write }
>
> I feel good for all but the { setattr write }
>
> It's a new message, we have no parsers which need the old format, how
> would others feel about
>
> perm="setattr,write"   ?

I'd recommend losing the quotes. I think you are doing this because of 
untrusted_string, but I doubt the user can influence this.

But I am also wondering if SELINUX_INFO is the most descriptive type name for 
what the record really means? Does this also result in a syscall record if 
audit is enabled?

-Steve

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: type bounds audit messages

[Eric Paris]

On Tue, 2009-06-16 at 10:40 -0400, Steve Grubb wrote:
> On Tuesday 16 June 2009 10:26:46 am Eric Paris wrote:
> > On Tue, 2009-06-16 at 09:43 +0900, KaiGai Kohei wrote:
> > > Stephen Smalley wrote:
> > >
> > > For example, how do you feel the example on security_compute_av() time?
> > >
> > > type=SELINUX_INFO msg=audit(1245046106.725:65):       \
> > >   op=security_compute_av masked=bounds                \
> > >   scontext=system_u:system_r:user_webapp_t:s0         \
> > >   tcontext=system_u:object_r:httpd_sys_content_t:s0   \
> > >   tclass=file { setattr write }
> >
> > I feel good for all but the { setattr write }
> >
> > It's a new message, we have no parsers which need the old format, how
> > would others feel about
> >
> > perm="setattr,write"   ?
> 
> I'd recommend losing the quotes. I think you are doing this because of 
> untrusted_string, but I doubt the user can influence this.

I'm starting to buy into the 'quotes makes it easy to know it's a
string' argument from jdennis.  Figure these are low volume and it
doesn't hurt.  (audit_log_string was actually what I was thinking, not
'untrustedstring')

> But I am also wondering if SELINUX_INFO is the most descriptive type name for 
> what the record really means? Does this also result in a syscall record if 
> audit is enabled?

Haven't seen the code   :)

-Eric


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: type bounds audit messages

[Daniel J Walsh]

On 06/16/2009 10:55 AM, Eric Paris wrote:
> On Tue, 2009-06-16 at 10:40 -0400, Steve Grubb wrote:
>> On Tuesday 16 June 2009 10:26:46 am Eric Paris wrote:
>>> On Tue, 2009-06-16 at 09:43 +0900, KaiGai Kohei wrote:
>>>> Stephen Smalley wrote:
>>>>
>>>> For example, how do you feel the example on security_compute_av() time?
>>>>
>>>> type=SELINUX_INFO msg=audit(1245046106.725:65):       \
>>>>    op=security_compute_av masked=bounds                \
>>>>    scontext=system_u:system_r:user_webapp_t:s0         \
>>>>    tcontext=system_u:object_r:httpd_sys_content_t:s0   \
>>>>    tclass=file { setattr write }
>>>
>>> I feel good for all but the { setattr write }
>>>
>>> It's a new message, we have no parsers which need the old format, how
>>> would others feel about
>>>
>>> perm="setattr,write"   ?
>>
>> I'd recommend losing the quotes. I think you are doing this because of
>> untrusted_string, but I doubt the user can influence this.
>
> I'm starting to buy into the 'quotes makes it easy to know it's a
> string' argument from jdennis.  Figure these are low volume and it
> doesn't hurt.  (audit_log_string was actually what I was thinking, not
> 'untrustedstring')
>
>> But I am also wondering if SELINUX_INFO is the most descriptive type name for
>> what the record really means? Does this also result in a syscall record if
>> audit is enabled?
>
> Haven't seen the code   :)
>
> -Eric
>
>
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.

I agree name value pairs is excellent, then we can cleanup the tools 
that analyze the avcs.  And what is an SELINUX_INFO, if this is a denial 
it should be a AVC.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: type bounds audit messages

[Steve Grubb]

On Tuesday 16 June 2009 10:55:33 am Eric Paris wrote:
> > > I feel good for all but the { setattr write }
> > >
> > > It's a new message, we have no parsers which need the old format, how
> > > would others feel about
> > >
> > > perm="setattr,write"   ?
> >
> > I'd recommend losing the quotes. I think you are doing this because of
> > untrusted_string, but I doubt the user can influence this.
>
> I'm starting to buy into the 'quotes makes it easy to know it's a
> string' argument from jdennis.

Any field that has a value starting and ending with quotes means that its 
encoded due to untrusted users having influence over it. That is the parsing 
rule.

-Steve

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: type bounds audit messages

[Daniel J Walsh]

On 06/16/2009 11:23 AM, Steve Grubb wrote:
> On Tuesday 16 June 2009 10:55:33 am Eric Paris wrote:
>>>> I feel good for all but the { setattr write }
>>>>
>>>> It's a new message, we have no parsers which need the old format, how
>>>> would others feel about
>>>>
>>>> perm="setattr,write"   ?
>>>
>>> I'd recommend losing the quotes. I think you are doing this because of
>>> untrusted_string, but I doubt the user can influence this.
>>
>> I'm starting to buy into the 'quotes makes it easy to know it's a
>> string' argument from jdennis.
>
> Any field that has a value starting and ending with quotes means that its
> encoded due to untrusted users having influence over it. That is the parsing
> rule.
>
> -Steve
>
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.
Well in that case you need the comma separated list.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: type bounds audit messages

[Eric Paris]

On Tue, 2009-06-16 at 11:23 -0400, Steve Grubb wrote:
> On Tuesday 16 June 2009 10:55:33 am Eric Paris wrote:
> > > > I feel good for all but the { setattr write }
> > > >
> > > > It's a new message, we have no parsers which need the old format, how
> > > > would others feel about
> > > >
> > > > perm="setattr,write"   ?
> > >
> > > I'd recommend losing the quotes. I think you are doing this because of
> > > untrusted_string, but I doubt the user can influence this.
> >
> > I'm starting to buy into the 'quotes makes it easy to know it's a
> > string' argument from jdennis.
> 
> Any field that has a value starting and ending with quotes means that its 
> encoded due to untrusted users having influence over it. That is the parsing 
> rule.

Maybe you meant to say that any field starting with a quote is a string
that COULD have been encoded but wasn't because it was found to contain
a safe valid string.  I don't see how this hurts those rules.

Like I said I'm ok with dropping the "" but I think it is a lot easier
on the parser to put " around strings to make them easier to
recognize....

-Eric


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: type bounds audit messages

[Stephen Smalley]

On Tue, 2009-06-16 at 11:19 -0400, Daniel J Walsh wrote:
> On 06/16/2009 10:55 AM, Eric Paris wrote:
> > On Tue, 2009-06-16 at 10:40 -0400, Steve Grubb wrote:
> >> On Tuesday 16 June 2009 10:26:46 am Eric Paris wrote:
> >>> On Tue, 2009-06-16 at 09:43 +0900, KaiGai Kohei wrote:
> >>>> Stephen Smalley wrote:
> >>>>
> >>>> For example, how do you feel the example on security_compute_av() time?
> >>>>
> >>>> type=SELINUX_INFO msg=audit(1245046106.725:65):       \
> >>>>    op=security_compute_av masked=bounds                \
> >>>>    scontext=system_u:system_r:user_webapp_t:s0         \
> >>>>    tcontext=system_u:object_r:httpd_sys_content_t:s0   \
> >>>>    tclass=file { setattr write }
> >>>
> >>> I feel good for all but the { setattr write }
> >>>
> >>> It's a new message, we have no parsers which need the old format, how
> >>> would others feel about
> >>>
> >>> perm="setattr,write"   ?
> >>
> >> I'd recommend losing the quotes. I think you are doing this because of
> >> untrusted_string, but I doubt the user can influence this.
> >
> > I'm starting to buy into the 'quotes makes it easy to know it's a
> > string' argument from jdennis.  Figure these are low volume and it
> > doesn't hurt.  (audit_log_string was actually what I was thinking, not
> > 'untrustedstring')
> >
> >> But I am also wondering if SELINUX_INFO is the most descriptive type name for
> >> what the record really means? Does this also result in a syscall record if
> >> audit is enabled?
> >
> > Haven't seen the code   :)
> >
> > -Eric
> >
> >
> > --
> > This message was distributed to subscribers of the selinux mailing list.
> > If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> > the words "unsubscribe selinux" without quotes as the message.
> 
> I agree name value pairs is excellent, then we can cleanup the tools 
> that analyze the avcs.  And what is an SELINUX_INFO, if this is a denial 
> it should be a AVC.

It isn't an AVC.  It is an internal inconsistency within the policy,
where an allow rule gave a child type more permissions than its parent.
It would be caught at policy link/expand time if expand-check=1 were
enabled in semanage.conf (same as neverallows), but will be caught at
runtime otherwise during compute_av processing.

It may later lead to an AVC if/when the particular permission is
checked.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: type bounds audit messages

[KaiGai Kohei]

Summary of opinions:
- SELINUX_INFO is good, because it is a new audit message type (Eric).
  But it is not unclear whether the most descriptive type name, or not (Steve).
- The { ... } style is not preferable, comma separated permissions list
  is better (Eric).
- The <name>=<value> style is more excellent (Dan).
- The quoted strung should be limited to describe untrusted strings (Steve).
- AVC denials also should use SELINUX_INFO with new style (Dan).
  - It describes an internal inconsistency within the policy, not AVC (Stephen).

Please tell me, if I missed your opinions.

I fixed the message format as follows:
("UNKNOWN[1418]" is AUDIT_SELINUX_INFO newly defined.)

  type=UNKNOWN[1418] msg=audit(1245207615.589:70):      \
      op=security_compute_av masked=bounds              \
      scontext=system_u:system_r:user_webapp_t:s0       \
      tcontext=system_u:object_r:shadow_t:s0:c0         \
      tclass=file perms=getattr,open

However, I think Stephen pointed out a significant thing.

Some of permissions are masked at runtime during security_compute_av()
due to RBAC, Constraint, MLS and Type bounds, although TE allows them.
It's not clear for me whether it should be considered as an error
(SELINUX_ERR) because of an internal inconsistency within the policy,
or should be considered as just an information (SELINUX_INFO) from
the kernel.

--------
[PATCH] Add audit messages for masked SELinux permissions

The following patch adds a few audit messages,
 1. when a multithread process switch its performing domain to unbounded
    one, and the hardwired rule prevent it.

  type=SELINUX_ERR msg=audit(1245207506.618:62):        \
      security_bounded_transition: denied for           \
      oldcontext=system_u:system_r:httpd_t:s0           \
      newcontext=system_u:system_r:guest_webapp_t:s0

 2. when RBAC, MLS/Constraints and Type bounds masks permissions allowed
    with TE rules on security_compute_av().

  * BRAC prevent domain transition
  type=UNKNOWN[1418] msg=audit(1245207539.227:67):      \
      op=security_compute_av masked=rbac                \
      scontext=unconfined_u:unconfined_r:unconfined_t:s0    \
      tcontext=staff_u:staff_r:staff_t:s0               \
      tclass=process perms=transition

  * MCS prevent accesses to *:s0:c0 by *:s0
  type=UNKNOWN[1418] msg=audit(1245212024.689:39):      \
      op=security_compute_av masked=constraint          \
      scontext=system_u:system_r:user_webapp_t:s0       \
      tcontext=system_u:object_r:shadow_t:s0:c0         \
      tclass=file perms=ioctl,read,write,create,setattr,lock,append,unlink,link,rename

  * Then, type bounds prevents accesses to shadow_t
  type=UNKNOWN[1418] msg=audit(1245212024.689:39):      \
      op=security_compute_av masked=bounds              \
      scontext=system_u:system_r:user_webapp_t:s0       \
      tcontext=system_u:object_r:shadow_t:s0:c0         \
      tclass=file perms=getattr,open

 Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com>
--
 include/linux/audit.h          |    1 +
 security/selinux/avc.c         |    2 +-
 security/selinux/include/avc.h |    3 -
 security/selinux/ss/services.c |  161 ++++++++++++++++++++++++++++++++++------
 4 files changed, 141 insertions(+), 26 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 4fa2810..6de6ef3 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -121,6 +121,7 @@
 #define AUDIT_MAC_IPSEC_EVENT	1415	/* Audit an IPSec event */
 #define AUDIT_MAC_UNLBL_STCADD	1416	/* NetLabel: add a static label */
 #define AUDIT_MAC_UNLBL_STCDEL	1417	/* NetLabel: del a static label */
+#define AUDIT_SELINUX_INFO	1418	/* Notifications from SE Linux */

 #define AUDIT_FIRST_KERN_ANOM_MSG   1700
 #define AUDIT_LAST_KERN_ANOM_MSG    1799
diff --git a/security/selinux/avc.c b/security/selinux/avc.c
index 7f9b5fa..4bf5d08 100644
--- a/security/selinux/avc.c
+++ b/security/selinux/avc.c
@@ -137,7 +137,7 @@ static inline int avc_hash(u32 ssid, u32 tsid, u16 tclass)
  * @tclass: target security class
  * @av: access vector
  */
-void avc_dump_av(struct audit_buffer *ab, u16 tclass, u32 av)
+static void avc_dump_av(struct audit_buffer *ab, u16 tclass, u32 av)
 {
 	const char **common_pts = NULL;
 	u32 common_base = 0;
diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h
index d12ff1a..46a940d 100644
--- a/security/selinux/include/avc.h
+++ b/security/selinux/include/avc.h
@@ -127,9 +127,6 @@ int avc_add_callback(int (*callback)(u32 event, u32 ssid, u32 tsid,
 		     u32 events, u32 ssid, u32 tsid,
 		     u16 tclass, u32 perms);

-/* Shows permission in human readable form */
-void avc_dump_av(struct audit_buffer *ab, u16 tclass, u32 av);
-
 /* Exported to selinuxfs */
 int avc_get_hash_stats(char *page);
 extern unsigned int avc_cache_threshold;
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index deeec6c..5b19fd3 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -22,6 +22,11 @@
  *
  *  Added validation of kernel classes and permissions
  *
+ * Updated: KaiGai Kohei <kaigai@ak.jp.nec.com>
+ *
+ *  Added support for bounds domain and audit messaged on masked permissions
+ *
+ * Copyright (C) 2008, 2009 NEC Corporation
  * Copyright (C) 2006, 2007 Hewlett-Packard Development Company, L.P.
  * Copyright (C) 2004-2006 Trusted Computer Solutions, Inc.
  * Copyright (C) 2003 - 2004, 2006 Tresys Technology, LLC
@@ -279,6 +284,98 @@ mls_ops:
 }

 /*
+ * security_dump_masked_av - dumps masked permissions during
+ * security_compute_av due to RBAC, MLS/Constraint and Type bounds.
+ */
+static int dump_masked_av_helper(void *k, void *d, void *args)
+{
+	struct perm_datum *pdatum = d;
+	char **permission_names = args;
+
+	BUG_ON(pdatum->value < 1 || pdatum->value > 32);
+
+	permission_names[pdatum->value - 1] = (char *)k;
+
+	return 0;
+}
+
+static void security_dump_masked_av(struct context *scontext,
+				    struct context *tcontext,
+				    u16 tclass,
+				    u32 permissions,
+				    const char *reason)
+{
+	struct common_datum *common_dat;
+	struct class_datum *tclass_dat;
+	struct audit_buffer *ab;
+	char *tclass_name;
+	char *scontext_name;
+	char *tcontext_name;
+	char *permission_names[32];
+	int index, length;
+	bool need_comma = false;
+
+	if (!permissions)
+		return;
+
+	tclass_name = policydb.p_class_val_to_name[tclass - 1];
+	tclass_dat = policydb.class_val_to_struct[tclass - 1];
+	common_dat = tclass_dat->comdatum;
+
+	/* init permission_names */
+	if (common_dat) {
+		if (hashtab_map(common_dat->permissions.table,
+				dump_masked_av_helper, permission_names) < 0)
+			goto out0;
+	}
+	if (hashtab_map(tclass_dat->permissions.table,
+			dump_masked_av_helper, permission_names) < 0)
+		goto out0;
+
+	/* get scontext/tcontext in text form */
+	if (context_struct_to_string(scontext,
+				     &scontext_name, &length) < 0)
+		goto out0;
+
+	if (context_struct_to_string(tcontext,
+				     &tcontext_name, &length) < 0)
+		goto out1;
+
+	/* audit a message */
+	ab = audit_log_start(current->audit_context,
+			     GFP_ATOMIC, AUDIT_SELINUX_INFO);
+	if (!ab)
+		goto out2;
+
+	audit_log_format(ab,
+			 "op=security_compute_av masked=%s "
+			 "scontext=%s tcontext=%s tclass=%s perms=",
+			 reason, scontext_name, tcontext_name, tclass_name);
+
+	for (index = 0; index < 32; index++) {
+		u32 mask = (1 << index);
+
+		if ((mask & permissions) == 0)
+			continue;
+
+		audit_log_format(ab, "%s%s",
+				 need_comma ? "," : "",
+				 permission_names[index]
+				 ? permission_names[index] : "????");
+		need_comma = true;
+	}
+	audit_log_end(ab);
+
+	/* release scontext/tcontext */
+out2:
+	kfree(tcontext_name);
+out1:
+	kfree(scontext_name);
+out0:
+	return;
+}
+
+/*
  * security_boundary_permission - drops violated permissions
  * on boundary constraint.
  */
@@ -347,28 +444,12 @@ static void type_attribute_bounds_av(struct context *scontext,
 	}

 	if (masked) {
-		struct audit_buffer *ab;
-		char *stype_name
-			= policydb.p_type_val_to_name[source->value - 1];
-		char *ttype_name
-			= policydb.p_type_val_to_name[target->value - 1];
-		char *tclass_name
-			= policydb.p_class_val_to_name[tclass - 1];
-
 		/* mask violated permissions */
 		avd->allowed &= ~masked;

-		/* notice to userspace via audit message */
-		ab = audit_log_start(current->audit_context,
-				     GFP_ATOMIC, AUDIT_SELINUX_ERR);
-		if (!ab)
-			return;
-
-		audit_log_format(ab, "av boundary violation: "
-				 "source=%s target=%s tclass=%s",
-				 stype_name, ttype_name, tclass_name);
-		avc_dump_av(ab, tclass, masked);
-		audit_log_end(ab);
+		/* audit masked permissions */
+		security_dump_masked_av(scontext, tcontext,
+					tclass, masked, "bounds");
 	}
 }

@@ -391,6 +472,7 @@ static int context_struct_compute_av(struct context *scontext,
 	struct ebitmap_node *snode, *tnode;
 	const struct selinux_class_perm *kdefs = &selinux_class_perm;
 	unsigned int i, j;
+	u32 masked;

 	/*
 	 * Remap extended Netlink classes for old policy versions.
@@ -475,15 +557,20 @@ static int context_struct_compute_av(struct context *scontext,
 	 * the MLS policy).
 	 */
 	constraint = tclass_datum->constraints;
+	masked = 0;
 	while (constraint) {
 		if ((constraint->permissions & (avd->allowed)) &&
 		    !constraint_expr_eval(scontext, tcontext, NULL,
 					  constraint->expr)) {
+			masked |= (avd->allowed & constraint->permissions);
 			avd->allowed = (avd->allowed) & ~(constraint->permissions);
 		}
 		constraint = constraint->next;
 	}

+	if (masked)
+		security_dump_masked_av(scontext, tcontext,
+					tclass, masked, "constraint");
 	/*
 	 * If checking process transition permission and the
 	 * role is changing, then check the (current_role, new_role)
@@ -497,9 +584,15 @@ static int context_struct_compute_av(struct context *scontext,
 			    tcontext->role == ra->new_role)
 				break;
 		}
-		if (!ra)
-			avd->allowed = (avd->allowed) & ~(PROCESS__TRANSITION |
-							PROCESS__DYNTRANSITION);
+		if (!ra) {
+			masked = avd->allowed & (PROCESS__TRANSITION |
+						 PROCESS__DYNTRANSITION);
+			avd->allowed &= ~(PROCESS__TRANSITION |
+					  PROCESS__DYNTRANSITION);
+			if (masked)
+				security_dump_masked_av(scontext, tcontext,
+							tclass, masked, "rbac");
+		}
 	}

 	/*
@@ -711,6 +804,30 @@ int security_bounded_transition(u32 old_sid, u32 new_sid)
 		}
 		index = type->bounds;
 	}
+
+	if (rc) {
+		char *old_name;
+		char *new_name;
+		int length;
+
+		if (context_struct_to_string(old_context,
+					     &old_name, &length) < 0)
+			goto out;
+		if (context_struct_to_string(new_context,
+					     &new_name, &length) < 0) {
+			kfree(old_name);
+			goto out;
+		}
+
+		audit_log(current->audit_context,
+			  GFP_ATOMIC, AUDIT_SELINUX_ERR,
+			  "security_bounded_transition: denied for "
+			  "oldcontext=%s newcontext=%s",
+			  old_name, new_name);
+
+		kfree(new_name);
+		kfree(old_name);
+	}
 out:
 	read_unlock(&policy_rwlock);


-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: type bounds audit messages

[Stephen Smalley]

On Wed, 2009-06-17 at 13:35 +0900, KaiGai Kohei wrote:
> Summary of opinions:
> - SELINUX_INFO is good, because it is a new audit message type (Eric).
>   But it is not unclear whether the most descriptive type name, or not (Steve).
> - The { ... } style is not preferable, comma separated permissions list
>   is better (Eric).
> - The <name>=<value> style is more excellent (Dan).
> - The quoted strung should be limited to describe untrusted strings (Steve).
> - AVC denials also should use SELINUX_INFO with new style (Dan).

No, we can't change the AVC denial format without breaking existing
userspace.

>   - It describes an internal inconsistency within the policy, not AVC (Stephen).
> 
> Please tell me, if I missed your opinions.
> 
> I fixed the message format as follows:
> ("UNKNOWN[1418]" is AUDIT_SELINUX_INFO newly defined.)
> 
>   type=UNKNOWN[1418] msg=audit(1245207615.589:70):      \
>       op=security_compute_av masked=bounds              \
>       scontext=system_u:system_r:user_webapp_t:s0       \
>       tcontext=system_u:object_r:shadow_t:s0:c0         \
>       tclass=file perms=getattr,open
> 
> However, I think Stephen pointed out a significant thing.
> 
> Some of permissions are masked at runtime during security_compute_av()
> due to RBAC, Constraint, MLS and Type bounds, although TE allows them.
> It's not clear for me whether it should be considered as an error
> (SELINUX_ERR) because of an internal inconsistency within the policy,
> or should be considered as just an information (SELINUX_INFO) from
> the kernel.

I assume the desire for a new message type is to make it clear that it
can be parsed using the new format.  But conceptually it is no different
than the SELINUX_ERR messages emitted by security_compute_sid or
security_validate_transition.

> --------
> [PATCH] Add audit messages for masked SELinux permissions
> 
> The following patch adds a few audit messages,
>  1. when a multithread process switch its performing domain to unbounded
>     one, and the hardwired rule prevent it.
> 
>   type=SELINUX_ERR msg=audit(1245207506.618:62):        \
>       security_bounded_transition: denied for           \
>       oldcontext=system_u:system_r:httpd_t:s0           \
>       newcontext=system_u:system_r:guest_webapp_t:s0

Might as well use the op=security_bounded_transition result=denied style
of syntax here as well for ease of parsing.

>  2. when RBAC, MLS/Constraints and Type bounds masks permissions allowed
>     with TE rules on security_compute_av().
> 
>   * BRAC prevent domain transition
>   type=UNKNOWN[1418] msg=audit(1245207539.227:67):      \
>       op=security_compute_av masked=rbac                \
>       scontext=unconfined_u:unconfined_r:unconfined_t:s0    \
>       tcontext=staff_u:staff_r:staff_t:s0               \
>       tclass=process perms=transition
> 
>   * MCS prevent accesses to *:s0:c0 by *:s0
>   type=UNKNOWN[1418] msg=audit(1245212024.689:39):      \
>       op=security_compute_av masked=constraint          \
>       scontext=system_u:system_r:user_webapp_t:s0       \
>       tcontext=system_u:object_r:shadow_t:s0:c0         \
>       tclass=file perms=ioctl,read,write,create,setattr,lock,append,unlink,link,rename

I'm not sure about adding such messages for RBAC or constraints - this
will potentially generate a fair number of messages for permissions that
will never be checked by the AVC, and they don't represent any
inconsistency within the policy, unlike the boundary violations.  Users
could potentially see many of these messages and be concerned that they
represent actual access attempts vs. just av computation.  If we were to
add such messages, I think we'd want to be able to easily disable them
(and do so by default), and then only enable them when doing policy
debugging.

>   * Then, type bounds prevents accesses to shadow_t
>   type=UNKNOWN[1418] msg=audit(1245212024.689:39):      \
>       op=security_compute_av masked=bounds              \
>       scontext=system_u:system_r:user_webapp_t:s0       \
>       tcontext=system_u:object_r:shadow_t:s0:c0         \
>       tclass=file perms=getattr,open

This looks ok, although I'm not sure masked= is the best name (vs. e.g.
reason=).

<snip>
> +static void security_dump_masked_av(struct context *scontext,
> +				    struct context *tcontext,
> +				    u16 tclass,
> +				    u32 permissions,
> +				    const char *reason)
> +{
> +	struct common_datum *common_dat;
> +	struct class_datum *tclass_dat;
> +	struct audit_buffer *ab;
> +	char *tclass_name;
> +	char *scontext_name;
> +	char *tcontext_name;

Initialize the *_name variables to NULL.
And then you don't need multiple out paths - you can just always kfree()
them on the way out.


> +	char *permission_names[32];
> +	int index, length;
> +	bool need_comma = false;
> +
> +	if (!permissions)
> +		return;
> +
> +	tclass_name = policydb.p_class_val_to_name[tclass - 1];
> +	tclass_dat = policydb.class_val_to_struct[tclass - 1];
> +	common_dat = tclass_dat->comdatum;
> +
> +	/* init permission_names */
> +	if (common_dat) {
> +		if (hashtab_map(common_dat->permissions.table,
> +				dump_masked_av_helper, permission_names) < 0)
> +			goto out0;
> +	}
> +	if (hashtab_map(tclass_dat->permissions.table,
> +			dump_masked_av_helper, permission_names) < 0)
> +		goto out0;
> +
> +	/* get scontext/tcontext in text form */
> +	if (context_struct_to_string(scontext,
> +				     &scontext_name, &length) < 0)
> +		goto out0;
> +
> +	if (context_struct_to_string(tcontext,
> +				     &tcontext_name, &length) < 0)
> +		goto out1;
> +
> +	/* audit a message */
> +	ab = audit_log_start(current->audit_context,
> +			     GFP_ATOMIC, AUDIT_SELINUX_INFO);
> +	if (!ab)
> +		goto out2;
> +
> +	audit_log_format(ab,
> +			 "op=security_compute_av masked=%s "
> +			 "scontext=%s tcontext=%s tclass=%s perms=",
> +			 reason, scontext_name, tcontext_name, tclass_name);
> +
> +	for (index = 0; index < 32; index++) {
> +		u32 mask = (1 << index);
> +
> +		if ((mask & permissions) == 0)
> +			continue;
> +
> +		audit_log_format(ab, "%s%s",
> +				 need_comma ? "," : "",
> +				 permission_names[index]
> +				 ? permission_names[index] : "????");
> +		need_comma = true;
> +	}
> +	audit_log_end(ab);
> +
> +	/* release scontext/tcontext */
> +out2:
> +	kfree(tcontext_name);
> +out1:
> +	kfree(scontext_name);
> +out0:
> +	return;
> +}

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: type bounds audit messages

[Daniel J Walsh]

On 06/16/2009 01:18 PM, Stephen Smalley wrote:
> On Tue, 2009-06-16 at 11:19 -0400, Daniel J Walsh wrote:
>> On 06/16/2009 10:55 AM, Eric Paris wrote:
>>> On Tue, 2009-06-16 at 10:40 -0400, Steve Grubb wrote:
>>>> On Tuesday 16 June 2009 10:26:46 am Eric Paris wrote:
>>>>> On Tue, 2009-06-16 at 09:43 +0900, KaiGai Kohei wrote:
>>>>>> Stephen Smalley wrote:
>>>>>>
>>>>>> For example, how do you feel the example on security_compute_av() time?
>>>>>>
>>>>>> type=SELINUX_INFO msg=audit(1245046106.725:65):       \
>>>>>>     op=security_compute_av masked=bounds                \
>>>>>>     scontext=system_u:system_r:user_webapp_t:s0         \
>>>>>>     tcontext=system_u:object_r:httpd_sys_content_t:s0   \
>>>>>>     tclass=file { setattr write }
>>>>>
>>>>> I feel good for all but the { setattr write }
>>>>>
>>>>> It's a new message, we have no parsers which need the old format, how
>>>>> would others feel about
>>>>>
>>>>> perm="setattr,write"   ?
>>>>
>>>> I'd recommend losing the quotes. I think you are doing this because of
>>>> untrusted_string, but I doubt the user can influence this.
>>>
>>> I'm starting to buy into the 'quotes makes it easy to know it's a
>>> string' argument from jdennis.  Figure these are low volume and it
>>> doesn't hurt.  (audit_log_string was actually what I was thinking, not
>>> 'untrustedstring')
>>>
>>>> But I am also wondering if SELINUX_INFO is the most descriptive type name for
>>>> what the record really means? Does this also result in a syscall record if
>>>> audit is enabled?
>>>
>>> Haven't seen the code   :)
>>>
>>> -Eric
>>>
>>>
>>> --
>>> This message was distributed to subscribers of the selinux mailing list.
>>> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
>>> the words "unsubscribe selinux" without quotes as the message.
>>
>> I agree name value pairs is excellent, then we can cleanup the tools
>> that analyze the avcs.  And what is an SELINUX_INFO, if this is a denial
>> it should be a AVC.
>
> It isn't an AVC.  It is an internal inconsistency within the policy,
> where an allow rule gave a child type more permissions than its parent.
> It would be caught at policy link/expand time if expand-check=1 were
> enabled in semanage.conf (same as neverallows), but will be caught at
> runtime otherwise during compute_av processing.
>
> It may later lead to an AVC if/when the particular permission is
> checked.
>
OK, I misunderstood.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: type bounds audit messages

[KaiGai Kohei]

Stephen Smalley wrote:
>> Some of permissions are masked at runtime during security_compute_av()
>> due to RBAC, Constraint, MLS and Type bounds, although TE allows them.
>> It's not clear for me whether it should be considered as an error
>> (SELINUX_ERR) because of an internal inconsistency within the policy,
>> or should be considered as just an information (SELINUX_INFO) from
>> the kernel.
> 
> I assume the desire for a new message type is to make it clear that it
> can be parsed using the new format.  But conceptually it is no different
> than the SELINUX_ERR messages emitted by security_compute_sid or
> security_validate_transition.

OK, I reverted the new SELINUX_INFO and used SELINUX_ERR instead.

>> --------
>> [PATCH] Add audit messages for masked SELinux permissions
>>
>> The following patch adds a few audit messages,
>>  1. when a multithread process switch its performing domain to unbounded
>>     one, and the hardwired rule prevent it.
>>
>>   type=SELINUX_ERR msg=audit(1245207506.618:62):        \
>>       security_bounded_transition: denied for           \
>>       oldcontext=system_u:system_r:httpd_t:s0           \
>>       newcontext=system_u:system_r:guest_webapp_t:s0
> 
> Might as well use the op=security_bounded_transition result=denied style
> of syntax here as well for ease of parsing.

Fixed, as follows:

  type=SELINUX_ERR msg=audit(1245311998.599:17):        \
      op=security_bounded_transition result=denied      \
      oldcontext=system_u:system_r:httpd_t:s0           \
      newcontext=system_u:system_r:guest_webapp_t:s0

>>  2. when RBAC, MLS/Constraints and Type bounds masks permissions allowed
>>     with TE rules on security_compute_av().
>>
>>   * BRAC prevent domain transition
>>   type=UNKNOWN[1418] msg=audit(1245207539.227:67):      \
>>       op=security_compute_av masked=rbac                \
>>       scontext=unconfined_u:unconfined_r:unconfined_t:s0    \
>>       tcontext=staff_u:staff_r:staff_t:s0               \
>>       tclass=process perms=transition
>>
>>   * MCS prevent accesses to *:s0:c0 by *:s0
>>   type=UNKNOWN[1418] msg=audit(1245212024.689:39):      \
>>       op=security_compute_av masked=constraint          \
>>       scontext=system_u:system_r:user_webapp_t:s0       \
>>       tcontext=system_u:object_r:shadow_t:s0:c0         \
>>       tclass=file perms=ioctl,read,write,create,setattr,lock,append,unlink,link,rename
> 
> I'm not sure about adding such messages for RBAC or constraints - this
> will potentially generate a fair number of messages for permissions that
> will never be checked by the AVC, and they don't represent any
> inconsistency within the policy, unlike the boundary violations.  Users
> could potentially see many of these messages and be concerned that they
> represent actual access attempts vs. just av computation.  If we were to
> add such messages, I think we'd want to be able to easily disable them
> (and do so by default), and then only enable them when doing policy
> debugging.

I agree to drop these messages for RBAC and constraints.
It is not too late to add them with actual requirements.

>>   * Then, type bounds prevents accesses to shadow_t
>>   type=UNKNOWN[1418] msg=audit(1245212024.689:39):      \
>>       op=security_compute_av masked=bounds              \
>>       scontext=system_u:system_r:user_webapp_t:s0       \
>>       tcontext=system_u:object_r:shadow_t:s0:c0         \
>>       tclass=file perms=getattr,open
> 
> This looks ok, although I'm not sure masked= is the best name (vs. e.g.
> reason=).

OK, "masked=" was replaced by "reason=", as follows:

  type=SELINUX_ERR msg=audit(1245312836.035:32):	\
      op=security_compute_av reason=bounds              \
      scontext=system_u:object_r:user_webapp_t:s0       \
      tcontext=system_u:object_r:shadow_t:s0:c0         \
      tclass=file perms=getattr,open

> <snip>
>> +static void security_dump_masked_av(struct context *scontext,
>> +				    struct context *tcontext,
>> +				    u16 tclass,
>> +				    u32 permissions,
>> +				    const char *reason)
>> +{
>> +	struct common_datum *common_dat;
>> +	struct class_datum *tclass_dat;
>> +	struct audit_buffer *ab;
>> +	char *tclass_name;
>> +	char *scontext_name;
>> +	char *tcontext_name;
> 
> Initialize the *_name variables to NULL.
> And then you don't need multiple out paths - you can just always kfree()
> them on the way out.

Fixed, and cleaned up.

--------
[PATCH] Add audit messages on type boundary violations

The attached patch adds support to generate audit messages on two cases.

The first one is a case when a multi-thread process tries to switch its
performing security context using setcon(3), but new security context is
not bounded by the old one.

  type=SELINUX_ERR msg=audit(1245311998.599:17):        \
      op=security_bounded_transition result=denied      \
      oldcontext=system_u:system_r:httpd_t:s0           \
      newcontext=system_u:system_r:guest_webapp_t:s0

The other one is a case when security_compute_av() masked any permissions
due to the type boundary violation.

  type=SELINUX_ERR msg=audit(1245312836.035:32):	\
      op=security_compute_av reason=bounds              \
      scontext=system_u:object_r:user_webapp_t:s0       \
      tcontext=system_u:object_r:shadow_t:s0:c0         \
      tclass=file perms=getattr,open


 Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com>
--
 security/selinux/avc.c         |    2 +-
 security/selinux/include/avc.h |    3 -
 security/selinux/ss/services.c |  136 ++++++++++++++++++++++++++++++++++------
 3 files changed, 118 insertions(+), 23 deletions(-)

diff --git a/security/selinux/avc.c b/security/selinux/avc.c
index 7f9b5fa..4bf5d08 100644
--- a/security/selinux/avc.c
+++ b/security/selinux/avc.c
@@ -137,7 +137,7 @@ static inline int avc_hash(u32 ssid, u32 tsid, u16 tclass)
  * @tclass: target security class
  * @av: access vector
  */
-void avc_dump_av(struct audit_buffer *ab, u16 tclass, u32 av)
+static void avc_dump_av(struct audit_buffer *ab, u16 tclass, u32 av)
 {
 	const char **common_pts = NULL;
 	u32 common_base = 0;
diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h
index d12ff1a..46a940d 100644
--- a/security/selinux/include/avc.h
+++ b/security/selinux/include/avc.h
@@ -127,9 +127,6 @@ int avc_add_callback(int (*callback)(u32 event, u32 ssid, u32 tsid,
 		     u32 events, u32 ssid, u32 tsid,
 		     u16 tclass, u32 perms);

-/* Shows permission in human readable form */
-void avc_dump_av(struct audit_buffer *ab, u16 tclass, u32 av);
-
 /* Exported to selinuxfs */
 int avc_get_hash_stats(char *page);
 extern unsigned int avc_cache_threshold;
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index deeec6c..1ad6e17 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -22,6 +22,11 @@
  *
  *  Added validation of kernel classes and permissions
  *
+ * Updated: KaiGai Kohei <kaigai@ak.jp.nec.com>
+ *
+ *  Added support for bounds domain and audit messaged on masked permissions
+ *
+ * Copyright (C) 2008, 2009 NEC Corporation
  * Copyright (C) 2006, 2007 Hewlett-Packard Development Company, L.P.
  * Copyright (C) 2004-2006 Trusted Computer Solutions, Inc.
  * Copyright (C) 2003 - 2004, 2006 Tresys Technology, LLC
@@ -279,6 +284,95 @@ mls_ops:
 }

 /*
+ * security_dump_masked_av - dumps masked permissions during
+ * security_compute_av due to RBAC, MLS/Constraint and Type bounds.
+ */
+static int dump_masked_av_helper(void *k, void *d, void *args)
+{
+	struct perm_datum *pdatum = d;
+	char **permission_names = args;
+
+	BUG_ON(pdatum->value < 1 || pdatum->value > 32);
+
+	permission_names[pdatum->value - 1] = (char *)k;
+
+	return 0;
+}
+
+static void security_dump_masked_av(struct context *scontext,
+				    struct context *tcontext,
+				    u16 tclass,
+				    u32 permissions,
+				    const char *reason)
+{
+	struct common_datum *common_dat;
+	struct class_datum *tclass_dat;
+	struct audit_buffer *ab;
+	char *tclass_name;
+	char *scontext_name = NULL;
+	char *tcontext_name = NULL;
+	char *permission_names[32];
+	int index, length;
+	bool need_comma = false;
+
+	if (!permissions)
+		return;
+
+	tclass_name = policydb.p_class_val_to_name[tclass - 1];
+	tclass_dat = policydb.class_val_to_struct[tclass - 1];
+	common_dat = tclass_dat->comdatum;
+
+	/* init permission_names */
+	if (common_dat &&
+	    hashtab_map(common_dat->permissions.table,
+			dump_masked_av_helper, permission_names) < 0)
+		goto out;
+
+	if (hashtab_map(tclass_dat->permissions.table,
+			dump_masked_av_helper, permission_names) < 0)
+		goto out;
+
+	/* get scontext/tcontext in text form */
+	if (context_struct_to_string(scontext,
+				     &scontext_name, &length) < 0)
+		goto out;
+
+	if (context_struct_to_string(tcontext,
+				     &tcontext_name, &length) < 0)
+		goto out;
+
+	/* audit a message */
+	ab = audit_log_start(current->audit_context,
+			     GFP_ATOMIC, AUDIT_SELINUX_ERR);
+	if (!ab)
+		goto out;
+
+	audit_log_format(ab, "op=security_compute_av reason=%s "
+			 "scontext=%s tcontext=%s tclass=%s perms=",
+			 reason, scontext_name, tcontext_name, tclass_name);
+
+	for (index = 0; index < 32; index++) {
+		u32 mask = (1 << index);
+
+		if ((mask & permissions) == 0)
+			continue;
+
+		audit_log_format(ab, "%s%s",
+				 need_comma ? "," : "",
+				 permission_names[index]
+				 ? permission_names[index] : "????");
+		need_comma = true;
+	}
+	audit_log_end(ab);
+out:
+	/* release scontext/tcontext */
+	kfree(tcontext_name);
+	kfree(scontext_name);
+
+	return;
+}
+
+/*
  * security_boundary_permission - drops violated permissions
  * on boundary constraint.
  */
@@ -347,28 +441,12 @@ static void type_attribute_bounds_av(struct context *scontext,
 	}

 	if (masked) {
-		struct audit_buffer *ab;
-		char *stype_name
-			= policydb.p_type_val_to_name[source->value - 1];
-		char *ttype_name
-			= policydb.p_type_val_to_name[target->value - 1];
-		char *tclass_name
-			= policydb.p_class_val_to_name[tclass - 1];
-
 		/* mask violated permissions */
 		avd->allowed &= ~masked;

-		/* notice to userspace via audit message */
-		ab = audit_log_start(current->audit_context,
-				     GFP_ATOMIC, AUDIT_SELINUX_ERR);
-		if (!ab)
-			return;
-
-		audit_log_format(ab, "av boundary violation: "
-				 "source=%s target=%s tclass=%s",
-				 stype_name, ttype_name, tclass_name);
-		avc_dump_av(ab, tclass, masked);
-		audit_log_end(ab);
+		/* audit masked permissions */
+		security_dump_masked_av(scontext, tcontext,
+					tclass, masked, "bounds");
 	}
 }

@@ -711,6 +789,26 @@ int security_bounded_transition(u32 old_sid, u32 new_sid)
 		}
 		index = type->bounds;
 	}
+
+	if (rc) {
+		char *old_name = NULL;
+		char *new_name = NULL;
+		int length;
+
+		if (!context_struct_to_string(old_context,
+					      &old_name, &length) &&
+		    !context_struct_to_string(new_context,
+					      &new_name, &length)) {
+			audit_log(current->audit_context,
+				  GFP_ATOMIC, AUDIT_SELINUX_ERR,
+				  "op=security_bounded_transition "
+				  "result=denied "
+				  "oldcontext=%s newcontext=%s",
+				  old_name, new_name);
+		}
+		kfree(new_name);
+		kfree(old_name);
+	}
 out:
 	read_unlock(&policy_rwlock);

-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: type bounds audit messages

[KaiGai Kohei]

By the way, we can find 8 of AUDIT_SELINUX_ERR messages more than
type_attribute_bounds_av(), such as:

 at selinux/hooks.c:4316

   audit_log(current->audit_context, GFP_KERNEL, AUDIT_SELINUX_ERR,
             "SELinux:  unrecognized netlink message"
             " type=%hu for sclass=%hu\n",
             nlh->nlmsg_type, isec->sclass);

Should it be replaced to <key>=<value> style?

Thanks,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: type bounds audit messages

[Stephen Smalley]

On Thu, 2009-06-18 at 17:26 +0900, KaiGai Kohei wrote:
> Stephen Smalley wrote:
> >> Some of permissions are masked at runtime during security_compute_av()
> >> due to RBAC, Constraint, MLS and Type bounds, although TE allows them.
> >> It's not clear for me whether it should be considered as an error
> >> (SELINUX_ERR) because of an internal inconsistency within the policy,
> >> or should be considered as just an information (SELINUX_INFO) from
> >> the kernel.
> > 
> > I assume the desire for a new message type is to make it clear that it
> > can be parsed using the new format.  But conceptually it is no different
> > than the SELINUX_ERR messages emitted by security_compute_sid or
> > security_validate_transition.
> 
> OK, I reverted the new SELINUX_INFO and used SELINUX_ERR instead.
> 
> >> --------
> >> [PATCH] Add audit messages for masked SELinux permissions
> >>
> >> The following patch adds a few audit messages,
> >>  1. when a multithread process switch its performing domain to unbounded
> >>     one, and the hardwired rule prevent it.
> >>
> >>   type=SELINUX_ERR msg=audit(1245207506.618:62):        \
> >>       security_bounded_transition: denied for           \
> >>       oldcontext=system_u:system_r:httpd_t:s0           \
> >>       newcontext=system_u:system_r:guest_webapp_t:s0
> > 
> > Might as well use the op=security_bounded_transition result=denied style
> > of syntax here as well for ease of parsing.
> 
> Fixed, as follows:
> 
>   type=SELINUX_ERR msg=audit(1245311998.599:17):        \
>       op=security_bounded_transition result=denied      \
>       oldcontext=system_u:system_r:httpd_t:s0           \
>       newcontext=system_u:system_r:guest_webapp_t:s0
> 
> >>  2. when RBAC, MLS/Constraints and Type bounds masks permissions allowed
> >>     with TE rules on security_compute_av().
> >>
> >>   * BRAC prevent domain transition
> >>   type=UNKNOWN[1418] msg=audit(1245207539.227:67):      \
> >>       op=security_compute_av masked=rbac                \
> >>       scontext=unconfined_u:unconfined_r:unconfined_t:s0    \
> >>       tcontext=staff_u:staff_r:staff_t:s0               \
> >>       tclass=process perms=transition
> >>
> >>   * MCS prevent accesses to *:s0:c0 by *:s0
> >>   type=UNKNOWN[1418] msg=audit(1245212024.689:39):      \
> >>       op=security_compute_av masked=constraint          \
> >>       scontext=system_u:system_r:user_webapp_t:s0       \
> >>       tcontext=system_u:object_r:shadow_t:s0:c0         \
> >>       tclass=file perms=ioctl,read,write,create,setattr,lock,append,unlink,link,rename
> > 
> > I'm not sure about adding such messages for RBAC or constraints - this
> > will potentially generate a fair number of messages for permissions that
> > will never be checked by the AVC, and they don't represent any
> > inconsistency within the policy, unlike the boundary violations.  Users
> > could potentially see many of these messages and be concerned that they
> > represent actual access attempts vs. just av computation.  If we were to
> > add such messages, I think we'd want to be able to easily disable them
> > (and do so by default), and then only enable them when doing policy
> > debugging.
> 
> I agree to drop these messages for RBAC and constraints.
> It is not too late to add them with actual requirements.
> 
> >>   * Then, type bounds prevents accesses to shadow_t
> >>   type=UNKNOWN[1418] msg=audit(1245212024.689:39):      \
> >>       op=security_compute_av masked=bounds              \
> >>       scontext=system_u:system_r:user_webapp_t:s0       \
> >>       tcontext=system_u:object_r:shadow_t:s0:c0         \
> >>       tclass=file perms=getattr,open
> > 
> > This looks ok, although I'm not sure masked= is the best name (vs. e.g.
> > reason=).
> 
> OK, "masked=" was replaced by "reason=", as follows:
> 
>   type=SELINUX_ERR msg=audit(1245312836.035:32):	\
>       op=security_compute_av reason=bounds              \
>       scontext=system_u:object_r:user_webapp_t:s0       \
>       tcontext=system_u:object_r:shadow_t:s0:c0         \
>       tclass=file perms=getattr,open
> 
> > <snip>
> >> +static void security_dump_masked_av(struct context *scontext,
> >> +				    struct context *tcontext,
> >> +				    u16 tclass,
> >> +				    u32 permissions,
> >> +				    const char *reason)
> >> +{
> >> +	struct common_datum *common_dat;
> >> +	struct class_datum *tclass_dat;
> >> +	struct audit_buffer *ab;
> >> +	char *tclass_name;
> >> +	char *scontext_name;
> >> +	char *tcontext_name;
> > 
> > Initialize the *_name variables to NULL.
> > And then you don't need multiple out paths - you can just always kfree()
> > them on the way out.
> 
> Fixed, and cleaned up.
> 
> --------
> [PATCH] Add audit messages on type boundary violations
> 
> The attached patch adds support to generate audit messages on two cases.
> 
> The first one is a case when a multi-thread process tries to switch its
> performing security context using setcon(3), but new security context is
> not bounded by the old one.
> 
>   type=SELINUX_ERR msg=audit(1245311998.599:17):        \
>       op=security_bounded_transition result=denied      \
>       oldcontext=system_u:system_r:httpd_t:s0           \
>       newcontext=system_u:system_r:guest_webapp_t:s0
> 
> The other one is a case when security_compute_av() masked any permissions
> due to the type boundary violation.
> 
>   type=SELINUX_ERR msg=audit(1245312836.035:32):	\
>       op=security_compute_av reason=bounds              \
>       scontext=system_u:object_r:user_webapp_t:s0       \
>       tcontext=system_u:object_r:shadow_t:s0:c0         \
>       tclass=file perms=getattr,open
> 
> 
>  Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com>

Acked-by:  Stephen Smalley <sds@tycho.nsa.gov>

> --
>  security/selinux/avc.c         |    2 +-
>  security/selinux/include/avc.h |    3 -
>  security/selinux/ss/services.c |  136 ++++++++++++++++++++++++++++++++++------
>  3 files changed, 118 insertions(+), 23 deletions(-)
> 
> diff --git a/security/selinux/avc.c b/security/selinux/avc.c
> index 7f9b5fa..4bf5d08 100644
> --- a/security/selinux/avc.c
> +++ b/security/selinux/avc.c
> @@ -137,7 +137,7 @@ static inline int avc_hash(u32 ssid, u32 tsid, u16 tclass)
>   * @tclass: target security class
>   * @av: access vector
>   */
> -void avc_dump_av(struct audit_buffer *ab, u16 tclass, u32 av)
> +static void avc_dump_av(struct audit_buffer *ab, u16 tclass, u32 av)
>  {
>  	const char **common_pts = NULL;
>  	u32 common_base = 0;
> diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h
> index d12ff1a..46a940d 100644
> --- a/security/selinux/include/avc.h
> +++ b/security/selinux/include/avc.h
> @@ -127,9 +127,6 @@ int avc_add_callback(int (*callback)(u32 event, u32 ssid, u32 tsid,
>  		     u32 events, u32 ssid, u32 tsid,
>  		     u16 tclass, u32 perms);
> 
> -/* Shows permission in human readable form */
> -void avc_dump_av(struct audit_buffer *ab, u16 tclass, u32 av);
> -
>  /* Exported to selinuxfs */
>  int avc_get_hash_stats(char *page);
>  extern unsigned int avc_cache_threshold;
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index deeec6c..1ad6e17 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -22,6 +22,11 @@
>   *
>   *  Added validation of kernel classes and permissions
>   *
> + * Updated: KaiGai Kohei <kaigai@ak.jp.nec.com>
> + *
> + *  Added support for bounds domain and audit messaged on masked permissions
> + *
> + * Copyright (C) 2008, 2009 NEC Corporation
>   * Copyright (C) 2006, 2007 Hewlett-Packard Development Company, L.P.
>   * Copyright (C) 2004-2006 Trusted Computer Solutions, Inc.
>   * Copyright (C) 2003 - 2004, 2006 Tresys Technology, LLC
> @@ -279,6 +284,95 @@ mls_ops:
>  }
> 
>  /*
> + * security_dump_masked_av - dumps masked permissions during
> + * security_compute_av due to RBAC, MLS/Constraint and Type bounds.
> + */
> +static int dump_masked_av_helper(void *k, void *d, void *args)
> +{
> +	struct perm_datum *pdatum = d;
> +	char **permission_names = args;
> +
> +	BUG_ON(pdatum->value < 1 || pdatum->value > 32);
> +
> +	permission_names[pdatum->value - 1] = (char *)k;
> +
> +	return 0;
> +}
> +
> +static void security_dump_masked_av(struct context *scontext,
> +				    struct context *tcontext,
> +				    u16 tclass,
> +				    u32 permissions,
> +				    const char *reason)
> +{
> +	struct common_datum *common_dat;
> +	struct class_datum *tclass_dat;
> +	struct audit_buffer *ab;
> +	char *tclass_name;
> +	char *scontext_name = NULL;
> +	char *tcontext_name = NULL;
> +	char *permission_names[32];
> +	int index, length;
> +	bool need_comma = false;
> +
> +	if (!permissions)
> +		return;
> +
> +	tclass_name = policydb.p_class_val_to_name[tclass - 1];
> +	tclass_dat = policydb.class_val_to_struct[tclass - 1];
> +	common_dat = tclass_dat->comdatum;
> +
> +	/* init permission_names */
> +	if (common_dat &&
> +	    hashtab_map(common_dat->permissions.table,
> +			dump_masked_av_helper, permission_names) < 0)
> +		goto out;
> +
> +	if (hashtab_map(tclass_dat->permissions.table,
> +			dump_masked_av_helper, permission_names) < 0)
> +		goto out;
> +
> +	/* get scontext/tcontext in text form */
> +	if (context_struct_to_string(scontext,
> +				     &scontext_name, &length) < 0)
> +		goto out;
> +
> +	if (context_struct_to_string(tcontext,
> +				     &tcontext_name, &length) < 0)
> +		goto out;
> +
> +	/* audit a message */
> +	ab = audit_log_start(current->audit_context,
> +			     GFP_ATOMIC, AUDIT_SELINUX_ERR);
> +	if (!ab)
> +		goto out;
> +
> +	audit_log_format(ab, "op=security_compute_av reason=%s "
> +			 "scontext=%s tcontext=%s tclass=%s perms=",
> +			 reason, scontext_name, tcontext_name, tclass_name);
> +
> +	for (index = 0; index < 32; index++) {
> +		u32 mask = (1 << index);
> +
> +		if ((mask & permissions) == 0)
> +			continue;
> +
> +		audit_log_format(ab, "%s%s",
> +				 need_comma ? "," : "",
> +				 permission_names[index]
> +				 ? permission_names[index] : "????");
> +		need_comma = true;
> +	}
> +	audit_log_end(ab);
> +out:
> +	/* release scontext/tcontext */
> +	kfree(tcontext_name);
> +	kfree(scontext_name);
> +
> +	return;
> +}
> +
> +/*
>   * security_boundary_permission - drops violated permissions
>   * on boundary constraint.
>   */
> @@ -347,28 +441,12 @@ static void type_attribute_bounds_av(struct context *scontext,
>  	}
> 
>  	if (masked) {
> -		struct audit_buffer *ab;
> -		char *stype_name
> -			= policydb.p_type_val_to_name[source->value - 1];
> -		char *ttype_name
> -			= policydb.p_type_val_to_name[target->value - 1];
> -		char *tclass_name
> -			= policydb.p_class_val_to_name[tclass - 1];
> -
>  		/* mask violated permissions */
>  		avd->allowed &= ~masked;
> 
> -		/* notice to userspace via audit message */
> -		ab = audit_log_start(current->audit_context,
> -				     GFP_ATOMIC, AUDIT_SELINUX_ERR);
> -		if (!ab)
> -			return;
> -
> -		audit_log_format(ab, "av boundary violation: "
> -				 "source=%s target=%s tclass=%s",
> -				 stype_name, ttype_name, tclass_name);
> -		avc_dump_av(ab, tclass, masked);
> -		audit_log_end(ab);
> +		/* audit masked permissions */
> +		security_dump_masked_av(scontext, tcontext,
> +					tclass, masked, "bounds");
>  	}
>  }
> 
> @@ -711,6 +789,26 @@ int security_bounded_transition(u32 old_sid, u32 new_sid)
>  		}
>  		index = type->bounds;
>  	}
> +
> +	if (rc) {
> +		char *old_name = NULL;
> +		char *new_name = NULL;
> +		int length;
> +
> +		if (!context_struct_to_string(old_context,
> +					      &old_name, &length) &&
> +		    !context_struct_to_string(new_context,
> +					      &new_name, &length)) {
> +			audit_log(current->audit_context,
> +				  GFP_ATOMIC, AUDIT_SELINUX_ERR,
> +				  "op=security_bounded_transition "
> +				  "result=denied "
> +				  "oldcontext=%s newcontext=%s",
> +				  old_name, new_name);
> +		}
> +		kfree(new_name);
> +		kfree(old_name);
> +	}
>  out:
>  	read_unlock(&policy_rwlock);
> 
-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: type bounds audit messages

[Stephen Smalley]

On Thu, 2009-06-18 at 17:35 +0900, KaiGai Kohei wrote:
> By the way, we can find 8 of AUDIT_SELINUX_ERR messages more than
> type_attribute_bounds_av(), such as:
> 
>  at selinux/hooks.c:4316
> 
>    audit_log(current->audit_context, GFP_KERNEL, AUDIT_SELINUX_ERR,
>              "SELinux:  unrecognized netlink message"
>              " type=%hu for sclass=%hu\n",
>              nlh->nlmsg_type, isec->sclass);
> 
> Should it be replaced to <key>=<value> style?

As long as it doesn't break existing userspace, that is fine with me.
Offhand, the only SELINUX_ERR message that is presently parsed by
userspace is the compute_sid one, by audit2allow/sepolgen (in order to
generate role-type statements when they are missing on a domain
transition).  And even that is a fairly rare case and could perhaps be
changed with minimal disruption.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: type bounds audit messages

[James Morris]

On Thu, 18 Jun 2009, KaiGai Kohei wrote:

> [PATCH] Add audit messages on type boundary violations
> 
> The attached patch adds support to generate audit messages on two cases.
> 
> The first one is a case when a multi-thread process tries to switch its
> performing security context using setcon(3), but new security context is
> not bounded by the old one.
> 
>   type=SELINUX_ERR msg=audit(1245311998.599:17):        \
>       op=security_bounded_transition result=denied      \
>       oldcontext=system_u:system_r:httpd_t:s0           \
>       newcontext=system_u:system_r:guest_webapp_t:s0
> 
> The other one is a case when security_compute_av() masked any permissions
> due to the type boundary violation.
> 
>   type=SELINUX_ERR msg=audit(1245312836.035:32):	\
>       op=security_compute_av reason=bounds              \
>       scontext=system_u:object_r:user_webapp_t:s0       \
>       tcontext=system_u:object_r:shadow_t:s0:c0         \
>       tclass=file perms=getattr,open
> 
> 
>  Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com>


Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next


-- 
James Morris
<jmorris@namei.org>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.