netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] [PATCH 0/2] Updated NetLabel/secid-reconciliation bits and a bugfix
@ 2006-10-04 15:46 paul.moore
  2006-10-04 15:46 ` [PATCH v4 1/2] NetLabel: secid reconciliation support paul.moore
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: paul.moore @ 2006-10-04 15:46 UTC (permalink / raw)
  To: netdev, selinux; +Cc: eparis, jmorris, sds, vyekkirala

This patchset includes an update to the NetLabel/secid-reconciliation patch,
replacing my "v3" patch from earlier this week, and a bugfix patch to cure a
race condition found during testing this week.  The bugfix patch does not
rely on the secid patches and should be merged regardless as it fixes a bug
which has been around since the very first NetLabel patches (not sure why I
didn't see this sooner).

--
paul moore
linux security @ hp


^ permalink raw reply	[flat|nested] 16+ messages in thread
* RE: [PATCH v4 1/2] NetLabel: secid reconciliation support
@ 2006-10-04 16:59 Venkat Yekkirala
  2006-10-04 18:45 ` Paul Moore
  0 siblings, 1 reply; 16+ messages in thread
From: Venkat Yekkirala @ 2006-10-04 16:59 UTC (permalink / raw)
  To: paul.moore, netdev, selinux; +Cc: eparis, jmorris, sds, Venkat Yekkirala

> @@ -3714,19 +3714,34 @@ static int selinux_skb_flow_in(struct sk
>  	if (skb->dev == &loopback_dev)
>  		return 1;
>  
> +	if (skb->secmark)
> +		loc_sid = skb->secmark;
> +	else
> +		loc_sid = SECINITSID_NETMSG;
> +
>  	err = selinux_xfrm_decode_session(skb, &xfrm_sid, 0);
>  	BUG_ON(err);
> -
> -	err = avc_has_perm(xfrm_sid, skb->secmark? : SECINITSID_NETMSG,
> -					SECCLASS_PACKET,
> -					PACKET__FLOW_IN, NULL);
> +	err = selinux_netlbl_skb_sid(skb,
> +				     xfrm_sid ? xfrm_sid : loc_sid,
> +				     &nlbl_sid);
>  	if (err)
>  		goto out;
>  
> -	if (xfrm_sid)
> -		skb->secmark = xfrm_sid;
> +	if (nlbl_sid)
> +		ext_sid = nlbl_sid;
> +	else
> +		ext_sid = xfrm_sid;

There's a problem here in that it would require 2 different policies
depending on whether one is using netlabel or xfrm. Specifically, in
the absence of matching iptables contexts (secmark), the skb here
will get:

- 0 (xfrm case)
- network_t (netlabel)

This has implications for getpeercon() where it would

- fail with ENOPROTOOPT (xfrm case)
- returns network_t (netlabel)

I would still argue that the nature of the domain being carried by
the packet is still unlabeled_t as implied by the null secmark. While
I view secmark/point as specifying BOTH a flow control point and a
default domain (incidentally using the same label more because of
implementation constrainst), I view network_t as purely a flow control
point.

But I also realize there can be equally forceful arguments for what this
patch does.

What does the community think? We need to resolve it one way or the
other unless the above differences in behavior are desired or somehow
accounted for in policy and apps.

> +
> +	err = avc_has_perm(ext_sid,
> +			   loc_sid,
> +			   SECCLASS_PACKET,
> +			   PACKET__FLOW_IN,
> +			   NULL);
> +	if (err)
> +		goto out;
>  
> -	/* See if NetLabel can flow in thru the current secmark here */
> +	if (ext_sid)
> +		skb->secmark = ext_sid;
>  
>  out:
>  	return err ? 0 : 1;
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread
* RE: [PATCH v4 1/2] NetLabel: secid reconciliation support
@ 2006-10-04 17:09 Venkat Yekkirala
  0 siblings, 0 replies; 16+ messages in thread
From: Venkat Yekkirala @ 2006-10-04 17:09 UTC (permalink / raw)
  To: Venkat Yekkirala, 'paul.moore@hp.com',
	'netdev@vger.kernel.org', 'selinux@tycho.nsa.gov'
  Cc: 'eparis@redhat.com', 'jmorris@namei.org',
	'sds@tycho.nsa.gov'

> > @@ -3714,19 +3714,34 @@ static int selinux_skb_flow_in(struct sk
> >  	if (skb->dev == &loopback_dev)
> >  		return 1;
> >  
> > +	if (skb->secmark)
> > +		loc_sid = skb->secmark;
> > +	else
> > +		loc_sid = SECINITSID_NETMSG;
> > +
> >  	err = selinux_xfrm_decode_session(skb, &xfrm_sid, 0);
> >  	BUG_ON(err);
> > -
> > -	err = avc_has_perm(xfrm_sid, skb->secmark? : SECINITSID_NETMSG,
> > -					SECCLASS_PACKET,
> > -					PACKET__FLOW_IN, NULL);
> > +	err = selinux_netlbl_skb_sid(skb,
> > +				     xfrm_sid ? xfrm_sid : loc_sid,
> > +				     &nlbl_sid);
> >  	if (err)
> >  		goto out;
> >  
> > -	if (xfrm_sid)
> > -		skb->secmark = xfrm_sid;
> > +	if (nlbl_sid)
> > +		ext_sid = nlbl_sid;
> > +	else
> > +		ext_sid = xfrm_sid;
> 
> There's a problem here in that it would require 2 different policies
> depending on whether one is using netlabel or xfrm. Specifically, in
> the absence of matching iptables contexts (secmark),

as well as any external labeling via ipsec/NetLabel,

> the skb here
> will get:
> 
> - 0 (xfrm case)
> - network_t (netlabel)
> 
> This has implications for getpeercon() where it would
> 
> - fail with ENOPROTOOPT (xfrm case)
> - returns network_t (netlabel)
> 
> I would still argue that the nature of the domain being carried by
> the packet is still unlabeled_t as implied by the null secmark. While
> I view secmark/point as specifying BOTH a flow control point and a
> default domain (incidentally using the same label more because of
> implementation constrainst), I view network_t as purely a flow control
> point.
> 
> But I also realize there can be equally forceful arguments 
> for what this
> patch does.
> 
> What does the community think? We need to resolve it one way or the
> other unless the above differences in behavior are desired or somehow
> accounted for in policy and apps.
> 
> > +
> > +	err = avc_has_perm(ext_sid,
> > +			   loc_sid,
> > +			   SECCLASS_PACKET,
> > +			   PACKET__FLOW_IN,
> > +			   NULL);
> > +	if (err)
> > +		goto out;
> >  
> > -	/* See if NetLabel can flow in thru the current secmark here */
> > +	if (ext_sid)
> > +		skb->secmark = ext_sid;
> >  
> >  out:
> >  	return err ? 0 : 1;
> > 
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread
* RE: [PATCH v4 1/2] NetLabel: secid reconciliation support
@ 2006-10-04 19:02 Venkat Yekkirala
  2006-10-04 19:27 ` Paul Moore
  0 siblings, 1 reply; 16+ messages in thread
From: Venkat Yekkirala @ 2006-10-04 19:02 UTC (permalink / raw)
  To: Paul Moore, Venkat Yekkirala; +Cc: netdev, selinux, eparis, jmorris, sds

>  * XFRM present
> 
>    xfrm_sid = <full context from xfrm>
>    loc_sid = SECINITSID_NETMSG
>    nlbl_sid = SECSID_NULL/0
>    ext_sid = xfrm_sid
>    final skb->secmark = avc_ok : ext_sid ? unchanged
> 
>  * NetLabel present
> 
>    xfrm_sid = SECSID_NULL/0
>    loc_sid = SECSID_NULL/0
>    nlbl_sid = <SECINITSID_NETMSG te ctx, netlabel mls ctx>
>    ext_sid = nlbl_sid
>    final skb->secmark = avc_ok : ext_sid ? unchanged

I was referring to the differences in what getpeercon would
return in the above 2 scenarios.

In the xfrm case:     final skb->secmark would be 0 resulting in getpeercon
to return EPROTONOOPT
In the NetLabel case: final skb->secmark would be network_t resulting in
getpeercon to return network_t

> 
>  * Nothing
> 
>    xfrm_sid = SECSID_NULL/0
>    loc_sid = SECSID_NULL/0
>    nlbl_sid = SECSID_NULL/0
>    ext_sid = xfrm_sid
>    final skb->secmark = avc_ok : ext_sid ? unchanged
> 
> > This has implications for getpeercon() where it would
> > 
> > - fail with ENOPROTOOPT (xfrm case)
> > - returns network_t (netlabel)
> > 
> > I would still argue that the nature of the domain being carried by
> > the packet is still unlabeled_t as implied by the null 
> secmark. While
> > I view secmark/point as specifying BOTH a flow control point and a
> > default domain (incidentally using the same label more because of
> > implementation constrainst), I view network_t as purely a 
> flow control
> > point.
> > 
> > But I also realize there can be equally forceful arguments 
> for what this
> > patch does.
> 
> I know the two of us have talked about this before on the mail lists,
> but I don't believe anyone else has ever made a comment in 
> this regard.
>  I tend to feel rather strongly that when using just NetLabel on a
> connection you shouldn't see an "unlabeled_t" type as I feel that the
> connotation associated with this type is misleading, even 
> though from a
> TE point of view there may be an argument made that it is appropriate.
> 
> > What does the community think? We need to resolve it one way or the
> > other unless the above differences in behavior are desired 
> or somehow
> > accounted for in policy and apps.
> 
> I agree - I'd like to hear what others (namely Stephen Smalley, James
> Morris and all of the Tresys folks <past and present>) have to say on
> this issue.
> 
> -- 
> paul moore
> linux security @ hp
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread
* RE: [PATCH v4 1/2] NetLabel: secid reconciliation support
@ 2006-10-04 19:37 Venkat Yekkirala
  2006-10-04 19:52 ` Paul Moore
  0 siblings, 1 reply; 16+ messages in thread
From: Venkat Yekkirala @ 2006-10-04 19:37 UTC (permalink / raw)
  To: Paul Moore, Venkat Yekkirala; +Cc: netdev, selinux, eparis, jmorris, sds

> >> * XFRM present
> >>
> >>   xfrm_sid = <full context from xfrm>
> >>   loc_sid = SECINITSID_NETMSG
> >>   nlbl_sid = SECSID_NULL/0
> >>   ext_sid = xfrm_sid
> >>   final skb->secmark = avc_ok : ext_sid ? unchanged

Actually, I meant to cite the following instead of the above:

 * Nothing

   xfrm_sid = SECSID_NULL/0
   loc_sid = SECSID_NULL/0
   nlbl_sid = SECSID_NULL/0
   ext_sid = xfrm_sid
   final skb->secmark = avc_ok : ext_sid ? unchanged

> >>
> >> * NetLabel present
> >>
> >>   xfrm_sid = SECSID_NULL/0
> >>   loc_sid = SECSID_NULL/0
> >>   nlbl_sid = <SECINITSID_NETMSG te ctx, netlabel mls ctx>
> >>   ext_sid = nlbl_sid
> >>   final skb->secmark = avc_ok : ext_sid ? unchanged
> > 
> > I was referring to the differences in what getpeercon would
> > return in the above 2 scenarios.
> > 
> > In the xfrm case:     final skb->secmark would be 0 
> resulting in getpeercon
> > to return EPROTONOOPT
> 
> In the "XFRM present" case above if the access is allowed (avc_ok is
> true) then the final skb->secmark value is going to be set to 
> the value
> of ext_sid which is the xfrm_sid.  Any calls made to 
> getpeercon() would
> return success with the context matching xfrm_sid.

You are right, but I was actually referring to the "Nothing"
case above.

> 
> I have a hunch we are still on different pages here ...
> 
> > In the NetLabel case: final skb->secmark would be network_t 
> resulting in
> > getpeercon to return network_t
> 
> Yep, and I understand you would like to see it as 
> unlabeled_t.  I think
> we both have valid arguments for either case and we are just 
> waiting to
> hear from others.
> 
> -- 
> paul moore
> linux security @ hp
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread
* RE: [PATCH v4 1/2] NetLabel: secid reconciliation support
@ 2006-10-04 19:53 Venkat Yekkirala
  2006-10-04 20:08 ` Paul Moore
  0 siblings, 1 reply; 16+ messages in thread
From: Venkat Yekkirala @ 2006-10-04 19:53 UTC (permalink / raw)
  To: Stephen Smalley, Paul Moore
  Cc: Venkat Yekkirala, netdev, selinux, eparis, jmorris

> On Wed, 2006-10-04 at 15:27 -0400, Paul Moore wrote:
> > Venkat Yekkirala wrote:
> > >> * XFRM present
> > >>
> > >>   xfrm_sid = <full context from xfrm>
> > >>   loc_sid = SECINITSID_NETMSG
> > >>   nlbl_sid = SECSID_NULL/0
> > >>   ext_sid = xfrm_sid
> > >>   final skb->secmark = avc_ok : ext_sid ? unchanged

As noted subsequently, I actually meant to refer to the below
instead of the XFRM case above:

 * Nothing

   xfrm_sid = SECSID_NULL/0
   loc_sid = SECSID_NULL/0
   nlbl_sid = SECSID_NULL/0
   ext_sid = xfrm_sid
   final skb->secmark = avc_ok : ext_sid ? unchanged

> > >>
> > >> * NetLabel present
> > >>
> > >>   xfrm_sid = SECSID_NULL/0
> > >>   loc_sid = SECSID_NULL/0
> > >>   nlbl_sid = <SECINITSID_NETMSG te ctx, netlabel mls ctx>
> > >>   ext_sid = nlbl_sid
> > >>   final skb->secmark = avc_ok : ext_sid ? unchanged
> > > 
> > > I was referring to the differences in what getpeercon would
> > > return in the above 2 scenarios.
> > > 
> > > In the xfrm case:     final skb->secmark would be 0 
> resulting in getpeercon
> > > to return EPROTONOOPT
> > 
> > In the "XFRM present" case above if the access is allowed (avc_ok is
> > true) then the final skb->secmark value is going to be set 
> to the value
> > of ext_sid which is the xfrm_sid.  Any calls made to 
> getpeercon() would
> > return success with the context matching xfrm_sid.
> > 
> > I have a hunch we are still on different pages here ...
> > 
> > > In the NetLabel case: final skb->secmark would be 
> network_t resulting in
> > > getpeercon to return network_t
> > 
> > Yep, and I understand you would like to see it as 
> unlabeled_t.  I think
> > we both have valid arguments for either case and we are 
> just waiting to
> > hear from others.
> 
> I don't understand the argument for network_t, and it seems to violate
> our goals of 1) having consistent policy regardless of 
> network labeling
> mechanism, and 2) having getpeercon() always return a subject 
> label that
> can be used as a basis for avc_has_perm() and setexeccon() calls.

But in the case where there's no domain info, but a cipso label,
getpeercon will fail (EPROTONOOPT). Do I rememember that Paul was
trying to use a special SID to use as a base sid in this case?

^ permalink raw reply	[flat|nested] 16+ messages in thread
end of thread, other threads:[~2006-10-04 22:56 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-10-04 15:46 [PATCH 0/2] [PATCH 0/2] Updated NetLabel/secid-reconciliation bits and a bugfix paul.moore
2006-10-04 15:46 ` [PATCH v4 1/2] NetLabel: secid reconciliation support paul.moore
2006-10-04 15:46 ` [PATCH 2/2] NetLabel: fix a cache race condition paul.moore
2006-10-04 18:44 ` [PATCH 0/2] [PATCH 0/2] Updated NetLabel/secid-reconciliation bits and a bugfix James Morris
2006-10-04 18:54   ` Paul Moore
2006-10-04 22:56     ` James Morris
  -- strict thread matches above, loose matches on Subject: below --
2006-10-04 16:59 [PATCH v4 1/2] NetLabel: secid reconciliation support Venkat Yekkirala
2006-10-04 18:45 ` Paul Moore
2006-10-04 17:09 Venkat Yekkirala
2006-10-04 19:02 Venkat Yekkirala
2006-10-04 19:27 ` Paul Moore
2006-10-04 19:45   ` Stephen Smalley
2006-10-04 19:37 Venkat Yekkirala
2006-10-04 19:52 ` Paul Moore
2006-10-04 19:53 Venkat Yekkirala
2006-10-04 20:08 ` Paul Moore

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).