Re: [PATCH v4 1/2] NetLabel: secid reconciliation support

[Paul Moore <paul.moore@hp.com>]

Venkat Yekkirala wrote:
>>@@ -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)

Perhaps I'm reading it the wrong way, but I didn't quite follow your
comment.  In an effort to bring some clarity to all of this here are all
of the possibile combinations that I can see using the code above (feel
free to correct me if I made a mistake):

 * SECMARK, XFRM, NetLabel present

   xfrm_sid = <full context from xfrm>
   loc_sid = <full context from secmark>
   nlbl_sid = <xfrm_sid te ctx, netlabel mls ctx>
   ext_sid = nlbl_sid
   final skb->secmark = avc_ok : ext_sid ? unchanged

 * SECMARK, XFRM present

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

 * SECMARK, NetLabel present

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

 * XFRM, NetLabel present

   xfrm_sid = <full context from xfrm>
   loc_sid = SECINITSID_NETMSG
   nlbl_sid = <xfrm_sid te ctx, netlabel mls ctx>
   ext_sid = nlbl_sid
   final skb->secmark = avc_ok : ext_sid ? unchanged

 * 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

 * 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