RE: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
In the case above I am concerned about the situation where the skb-secmark == 0 and there is a IPv4 option (i.e. it is NetLabel labeled) on the packet. It's unfortunate that you cut out the code in your reply. It's even more unfortunate that you should say this. The proper thing to do is to simply repaste portions that you feel like you need to. Remember, we aren't opponents standing for election this season. :) - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
On Thu, 2006-09-28 at 23:52 -0400, Joshua Brindle wrote: Venkat Yekkirala wrote: snip + + err = avc_has_perm(xfrm_sid, skb-secmark, SECCLASS_PACKET, + PACKET__FLOW_IN, NULL); + if (err) + goto out; + + if (xfrm_sid) { + err = security_transition_sid(xfrm_sid, skb-secmark, + SECCLASS_PACKET, trans_sid); + if (err) + goto out; + I thought we weren't doing transitions to label packets anymore per the conference call? No, transitions are still part of the reconciliation process. By default, this just means that we end up with the xfrm_sid (which is what you want). But it allows us the freedom to define transitions on the secmark label if desired, and those transitions can still yield subject labels. -- Stephen Smalley National Security Agency - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
On Fri, 2006-09-29 at 08:59 -0400, Stephen Smalley wrote: On Thu, 2006-09-28 at 23:52 -0400, Joshua Brindle wrote: Venkat Yekkirala wrote: snip + + err = avc_has_perm(xfrm_sid, skb-secmark, SECCLASS_PACKET, + PACKET__FLOW_IN, NULL); + if (err) + goto out; + + if (xfrm_sid) { + err = security_transition_sid(xfrm_sid, skb-secmark, + SECCLASS_PACKET, trans_sid); + if (err) + goto out; + I thought we weren't doing transitions to label packets anymore per the conference call? No, transitions are still part of the reconciliation process. By default, this just means that we end up with the xfrm_sid (which is what you want). But it allows us the freedom to define transitions on the secmark label if desired, and those transitions can still yield subject labels. This is not consistent with my perception of the decision made in the conference call. I thought that the secid was either going to be 1) the secmark label if no external labeling is present or 2) the external label if it is present. The flow_in permission would be checked between the external label and the secmark label in either case (unlabeled in the case of #1) How is this different from the implementation before the call? - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
On Fri, 2006-09-29 at 10:00 -0400, Joshua Brindle wrote: On Fri, 2006-09-29 at 08:59 -0400, Stephen Smalley wrote: On Thu, 2006-09-28 at 23:52 -0400, Joshua Brindle wrote: Venkat Yekkirala wrote: snip + + err = avc_has_perm(xfrm_sid, skb-secmark, SECCLASS_PACKET, + PACKET__FLOW_IN, NULL); + if (err) + goto out; + + if (xfrm_sid) { + err = security_transition_sid(xfrm_sid, skb-secmark, + SECCLASS_PACKET, trans_sid); + if (err) + goto out; + I thought we weren't doing transitions to label packets anymore per the conference call? No, transitions are still part of the reconciliation process. By default, this just means that we end up with the xfrm_sid (which is what you want). But it allows us the freedom to define transitions on the secmark label if desired, and those transitions can still yield subject labels. This is not consistent with my perception of the decision made in the conference call. I thought that the secid was either going to be 1) the secmark label if no external labeling is present or 2) the external label if it is present. The flow_in permission would be checked between the external label and the secmark label in either case (unlabeled in the case of #1) The behavior you describe is precisely what will happen in the absence of any type_transition rules on packet class in the policy, given the way in which he has defined security_transition_sid on packet class. The only question is whether there is any value in allowing a transition to be configured in policy (and if such a transition is configured, whether the resulting SID requires its own separate permission check, which is what is usually done - the transition doesn't implicitly authorize anything). I don't recall a particular decision on the transition issue during the call nor do I see it in the posted notes. However, since the transition was removed in the flow_out case, it would be logical to remove it from the flow_in case as well, and that would have the side benefit of less overhead. -- Stephen Smalley National Security Agency - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
On Fri, 29 Sep 2006, Stephen Smalley wrote: However, since the transition was removed in the flow_out case, it would be logical to remove it from the flow_in case as well, and that would have the side benefit of less overhead. How about adding secmark transitions later, if needed, perhaps with an /selinux config control ? It does keep things simpler for now, in terms of getting this code merged, deployed into distros and likely certified. - James -- James Morris [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
Stephen Smalley wrote: On Fri, 2006-09-29 at 10:33 -0400, James Morris wrote: On Fri, 29 Sep 2006, Stephen Smalley wrote: However, since the transition was removed in the flow_out case, it would be logical to remove it from the flow_in case as well, and that would have the side benefit of less overhead. How about adding secmark transitions later, if needed, perhaps with an /selinux config control ? It does keep things simpler for now, in terms of getting this code merged, deployed into distros and likely certified. Fine with me, unless Venkat has an immediate use case for such transitions in the flow_in case (but I think this is mostly my fault for suggesting transitions a while ago). Unless I'm confusing something, there still may be a need for transitions if we want to support both IPsec and NetLabel labeling on the same connection. If we don't support transitions and allow both labeling methods on the same connection we'll need to decide how to handle resolving the two - maybe use a transition is this one case? -- paul moore linux security @ hp - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
On Fri, 29 Sep 2006, Paul Moore wrote: Unless I'm confusing something, there still may be a need for transitions if we want to support both IPsec and NetLabel labeling on the same connection. I'd prefer not to support this, as it's too complicated, and CIPSO is a legacy protocol. Normal IPsec protection applied to CIPSO: yes, but not IPsec labeling and CIPSO labeling on the same connection. - James -- James Morris [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
Fine with me, unless Venkat has an immediate use case for such transitions in the flow_in case (but I think this is mostly my fault for suggesting transitions a while ago). I don't have a use case currently. Unless I'm confusing something, there still may be a need for transitions if we want to support both IPsec and NetLabel labeling on the same connection. If we don't support transitions and allow both labeling methods on the same connection we'll need to decide how to handle resolving the two - maybe use a transition is this one case? Since CIPSO doesn't do full contexts currently, it would be just a matter of an additional flow_in check. The base sid used here would be the current secmark at that point (which will be the xfrm sid if xfrm was used). So, no transitions needed here currently. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
Venkat Yekkirala wrote: Fine with me, unless Venkat has an immediate use case for such transitions in the flow_in case (but I think this is mostly my fault for suggesting transitions a while ago). I don't have a use case currently. Unless I'm confusing something, there still may be a need for transitions if we want to support both IPsec and NetLabel labeling on the same connection. If we don't support transitions and allow both labeling methods on the same connection we'll need to decide how to handle resolving the two - maybe use a transition is this one case? Since CIPSO doesn't do full contexts currently, it would be just a matter of an additional flow_in check. The base sid used here would be the current secmark at that point (which will be the xfrm sid if xfrm was used). So, no transitions needed here currently. That's fine by me, I just wanted to make sure something like that would be acceptable. So, in summary, we would do the normal flow_in checks for both IPsec and NetLabel and then set the secmark using the IPsec label as the base sid for the NetLabel's generated SID? -- paul moore linux security @ hp - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
Unless I'm confusing something, there still may be a need for transitions if we want to support both IPsec and NetLabel labeling on the same connection. I'd prefer not to support this, as it's too complicated, Actually, from my vantage point, it actually seems natural. and CIPSO is a legacy protocol. Sure. Normal IPsec protection applied to CIPSO: yes, but not IPsec labeling and CIPSO labeling on the same connection. One use case example can be one SA for Secret in combination with any/all/none of the compartments. And another SA for Top Secret ... - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
That's fine by me, I just wanted to make sure something like that would be acceptable. So, in summary, we would do the normal flow_in checks for both IPsec and NetLabel and then set the secmark using the IPsec label as the base sid for the NetLabel's generated SID? That's correct (in short you won't care if IPSec was in use or not, you would just use the secmark at that point as the base sid in coming up with the NetLabel sid, and if it flow controls fine vis a vis the secmark you would replace secmark with the NetLabel sid. The logic flow is quite natural and intuitive for the users as well). - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
I tend to agree, I just can't see it being all that useful in the real world. However, each time it comes up (including the conference call earlier this week) it seems that people would prefer to use both at the same time. A matter of providing options to users. It seems more of a pain to actually prevent their use at the same time and/or explain strange/unnatural behavior. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
Venkat Yekkirala wrote: I tend to agree, I just can't see it being all that useful in the real world. However, each time it comes up (including the conference call earlier this week) it seems that people would prefer to use both at the same time. A matter of providing options to users. As long as those options receive the blessings of the maintainers ;) It seems more of a pain to actually prevent their use at the same time and/or explain strange/unnatural behavior. Agreed, the solution that we agreed upon is much easier to implement and explain than a lot of the alternatives. -- paul moore linux security @ hp - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
Venkat Yekkirala wrote: This defines SELinux enforcement of the 2 new LSM hooks as well as related changes elsewhere in the SELinux code. As soon as I hit send on this mail I'll start working on the related patch to provide the missing NetLabel support. Additional comments are below. Signed-off-by: Venkat Yekkirala [EMAIL PROTECTED] --- security/selinux/hooks.c| 129 +++--- security/selinux/include/xfrm.h |5 + security/selinux/ss/mls.c |2 security/selinux/ss/services.c |2 security/selinux/xfrm.c | 28 ++ 5 files changed, 139 insertions(+), 27 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 5a66c4c..143b4b8 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3449,8 +3449,12 @@ static int selinux_sock_rcv_skb_compat(s err = avc_has_perm(sock_sid, port_sid, sock_class, recv_perm, ad); + if (err) + goto out; } + err = selinux_xfrm_sock_rcv_skb(sock_sid, skb, ad); + out: return err; } @@ -3489,10 +3493,6 @@ static int selinux_socket_sock_rcv_skb(s goto out; err = selinux_netlbl_sock_rcv_skb(sksec, skb, ad); - if (err) - goto out; - - err = selinux_xfrm_sock_rcv_skb(sksec-sid, skb, ad); out: return err; } @@ -3626,13 +3626,16 @@ static int selinux_inet_conn_request(str return 0; } - err = selinux_xfrm_decode_session(skb, peersid, 0); - BUG_ON(err); + if (selinux_compat_net) { + err = selinux_xfrm_decode_session(skb, peersid, 0); + BUG_ON(err); - if (peersid == SECSID_NULL) { - req-secid = sksec-sid; - return 0; - } + if (peersid == SECSID_NULL) { + req-secid = sksec-sid; + return 0; + } + } else + peersid = skb-secmark; err = security_sid_mls_copy(sksec-sid, peersid, newsid); if (err) @@ -3662,6 +3665,69 @@ static void selinux_req_classify_flow(co fl-secid = req-secid; } +static int selinux_skb_flow_in(struct sk_buff *skb, unsigned short family) +{ + u32 xfrm_sid, trans_sid; + int err; + + if (selinux_compat_net) + return 1; + + /* xfrm/cipso inapplicable for loopback traffic */ + if (skb-dev == loopback_dev) + return 1; Just to clarify (I believe this is the case from your remarks as well as the code, but better safe than sorry) the secmark for loopback traffic is set by the originating socket, yes? Beware: a bit of a nit follows :) While I understand that NetLabel currently only supports CIPSO, it is a framework that can support multiple labeling procotols (i.e. RIPSO support is planned). Please change the comment from cipso/CIPSO to netlabel/NetLabel so it is consistent with the rest of the code which makes use of NetLabel. + err = selinux_xfrm_decode_session(skb, xfrm_sid, 0); + BUG_ON(err); + + err = avc_has_perm(xfrm_sid, skb-secmark, SECCLASS_PACKET, + PACKET__FLOW_IN, NULL); + if (err) + goto out; + + if (xfrm_sid) { + err = security_transition_sid(xfrm_sid, skb-secmark, + SECCLASS_PACKET, trans_sid); + if (err) + goto out; + + skb-secmark = trans_sid; + } + /* See if CIPSO can flow in thru the current secmark here */ Same nit applies about the use of CIPSO vs NetLabel. +out: + return err ? 0 : 1; +}; + +static int selinux_skb_flow_out(struct sk_buff *skb, u32 nf_secid) +{ + u32 trans_sid; + int err; + + if (selinux_compat_net) + return 1; + + if (!skb-secmark) { + u32 xfrm_sid; + + selinux_skb_xfrm_sid(skb, xfrm_sid); + + if (xfrm_sid) + skb-secmark = xfrm_sid; + else if (skb-sk) { + struct sk_security_struct *sksec = skb-sk-sk_security; + skb-secmark = sksec-sid; + } + } + + err = avc_has_perm(skb-secmark, nf_secid, SECCLASS_PACKET, + PACKET__FLOW_OUT, NULL); While I don't see any explicit mention of it in the documentation or your comments, I assume we would want a flow_out check for NetLabel here as well? +out: + return err ? 0 : 1; +} + static int selinux_nlmsg_perm(struct sock *sk, struct sk_buff *skb) { int err = 0; @@ -3700,7 +3766,8 @@ out: #ifdef CONFIG_NETFILTER -static int selinux_ip_postroute_last_compat(struct sock *sk, struct net_device *dev, +static int selinux_ip_postroute_last_compat(struct sock *sk, struct sk_buff *skb, +
Re: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
On Fri, 29 Sep 2006, Paul Moore wrote: It seems more of a pain to actually prevent their use at the same time and/or explain strange/unnatural behavior. Agreed, the solution that we agreed upon is much easier to implement and explain than a lot of the alternatives. Ok, can you please explain it further? i.e. show me what the policy looks like, exactly what the user is trying to achieve, and explain what happens to each packet exactly in terms of labeling on the input and output paths. -- James Morris [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
+static int selinux_skb_flow_in(struct sk_buff *skb, unsigned short family) +{ + u32 xfrm_sid, trans_sid; + int err; + + if (selinux_compat_net) + return 1; + + /* xfrm/cipso inapplicable for loopback traffic */ + if (skb-dev == loopback_dev) + return 1; Just to clarify (I believe this is the case from your remarks as well as the code, but better safe than sorry) the secmark for loopback traffic is set by the originating socket, yes? That's correct. Beware: a bit of a nit follows :) While I understand that NetLabel currently only supports CIPSO, it is a framework that can support multiple labeling procotols (i.e. RIPSO support is planned). Please change the comment from cipso/CIPSO to netlabel/NetLabel so it is consistent with the rest of the code which makes use of NetLabel. Sure. + err = selinux_xfrm_decode_session(skb, xfrm_sid, 0); + BUG_ON(err); + + err = avc_has_perm(xfrm_sid, skb-secmark, SECCLASS_PACKET, + PACKET__FLOW_IN, NULL); + if (err) + goto out; + + if (xfrm_sid) { + err = security_transition_sid(xfrm_sid, skb-secmark, + SECCLASS_PACKET, trans_sid); + if (err) + goto out; + + skb-secmark = trans_sid; + } + /* See if CIPSO can flow in thru the current secmark here */ Same nit applies about the use of CIPSO vs NetLabel. Sure. +out: + return err ? 0 : 1; +}; + +static int selinux_skb_flow_out(struct sk_buff *skb, u32 nf_secid) +{ + u32 trans_sid; + int err; + + if (selinux_compat_net) + return 1; + + if (!skb-secmark) { + u32 xfrm_sid; + + selinux_skb_xfrm_sid(skb, xfrm_sid); + + if (xfrm_sid) + skb-secmark = xfrm_sid; + else if (skb-sk) { + struct sk_security_struct *sksec = skb-sk-sk_security; + skb-secmark = sksec-sid; + } + } + + err = avc_has_perm(skb-secmark, nf_secid, SECCLASS_PACKET, + PACKET__FLOW_OUT, NULL); While I don't see any explicit mention of it in the documentation or your comments, I assume we would want a flow_out check for NetLabel here as well? I don't believe we do. By this time, the packet is or should already be carrying the CIPSO/NetLabel option which should already be the right one (derived from the socket or flow as appropriate), but you would want to audit the code to make sure. IOW, the label option in the IP header should already be reflecting the secmark on the skb. But again, you may want to audit the code to make sure. +out: + return err ? 0 : 1; +} + static int selinux_nlmsg_perm(struct sock *sk, struct sk_buff *skb) { int err = 0; @@ -3700,7 +3766,8 @@ out: #ifdef CONFIG_NETFILTER -static int selinux_ip_postroute_last_compat(struct sock *sk, struct net_device *dev, +static int selinux_ip_postroute_last_compat(struct sock *sk, struct sk_buff *skb, + struct net_device *dev, struct avc_audit_data *ad, u16 family, char *addrp, int len) { @@ -3710,6 +3777,9 @@ static int selinux_ip_postroute_last_com struct inode *inode; struct inode_security_struct *isec; + if (!sk) + goto out; + sock = sk-sk_socket; if (!sock) goto out; @@ -3768,7 +3838,11 @@ static int selinux_ip_postroute_last_com err = avc_has_perm(isec-sid, port_sid, isec-sclass, send_perm, ad); + if (err) + goto out; } + + err = selinux_xfrm_postroute_last(isec-sid, skb, ad); out: return err; } @@ -3782,17 +3856,9 @@ static unsigned int selinux_ip_postroute { char *addrp; int len, err = 0; - struct sock *sk; struct sk_buff *skb = *pskb; struct avc_audit_data ad; struct net_device *dev = (struct net_device *)out; - struct sk_security_struct *sksec; - - sk = skb-sk; - if (!sk) - goto out; - - sksec = sk-sk_security; AVC_AUDIT_DATA_INIT(ad, NET); ad.u.net.netif = dev-name; @@ -3803,16 +3869,25 @@ static unsigned int selinux_ip_postroute goto out; if (selinux_compat_net) - err = selinux_ip_postroute_last_compat(sk, dev, ad, + err = selinux_ip_postroute_last_compat(skb-sk, skb, dev, ad, family, addrp, len); - else - err = avc_has_perm(sksec-sid, skb-secmark, SECCLASS_PACKET, - PACKET__SEND, ad); + else { + if (!skb-secmark) { +
Re: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
On Fri, 29 Sep 2006, James Morris wrote: On Fri, 29 Sep 2006, Paul Moore wrote: It seems more of a pain to actually prevent their use at the same time and/or explain strange/unnatural behavior. Agreed, the solution that we agreed upon is much easier to implement and explain than a lot of the alternatives. Ok, can you please explain it further? i.e. show me what the policy looks like, exactly what the user is trying to achieve, and explain what happens to each packet exactly in terms of labeling on the input and output paths. Also, why can't this be done just with xfrm labeling? CIPSO is not there to provide a mechanism for separating the label of the connection from the label of the data, it's only there to provide interop with legacy systems. If you need to have two labels for a packet (the object and the domain), then this needs to be supported directly by xfrm labeling. - James -- James Morris [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
Venkat Yekkirala wrote: +static int selinux_skb_flow_out(struct sk_buff *skb, u32 nf_secid) +{ +u32 trans_sid; +int err; + +if (selinux_compat_net) +return 1; + +if (!skb-secmark) { +u32 xfrm_sid; + +selinux_skb_xfrm_sid(skb, xfrm_sid); + +if (xfrm_sid) +skb-secmark = xfrm_sid; +else if (skb-sk) { +struct sk_security_struct *sksec = skb-sk-sk_security; +skb-secmark = sksec-sid; +} +} + +err = avc_has_perm(skb-secmark, nf_secid, SECCLASS_PACKET, +PACKET__FLOW_OUT, NULL); While I don't see any explicit mention of it in the documentation or your comments, I assume we would want a flow_out check for NetLabel here as well? I don't believe we do. By this time, the packet is or should already be carrying the CIPSO/NetLabel option which should already be the right one (derived from the socket or flow as appropriate), but you would want to audit the code to make sure. IOW, the label option in the IP header should already be reflecting the secmark on the skb. But again, you may want to audit the code to make sure. In the case above I am concerned about the situation where the skb-secmark == 0 and there is a IPv4 option (i.e. it is NetLabel labeled) on the packet. It would seem that the right/consistent thing to do would be to adjust the secmark accordingly, similar to what we would do on the flow_in case, yes? +static int selinux_ip_postroute_last_compat(struct sock *sk, struct sk_buff *skb, +struct net_device *dev, struct avc_audit_data *ad, u16 family, char *addrp, int len) { @@ -3710,6 +3777,9 @@ static int selinux_ip_postroute_last_com struct inode *inode; struct inode_security_struct *isec; +if (!sk) +goto out; + sock = sk-sk_socket; if (!sock) goto out; @@ -3768,7 +3838,11 @@ static int selinux_ip_postroute_last_com err = avc_has_perm(isec-sid, port_sid, isec-sclass, send_perm, ad); +if (err) +goto out; } + +err = selinux_xfrm_postroute_last(isec-sid, skb, ad); out: return err; } @@ -3782,17 +3856,9 @@ static unsigned int selinux_ip_postroute { char *addrp; int len, err = 0; -struct sock *sk; struct sk_buff *skb = *pskb; struct avc_audit_data ad; struct net_device *dev = (struct net_device *)out; -struct sk_security_struct *sksec; - -sk = skb-sk; -if (!sk) -goto out; - -sksec = sk-sk_security; AVC_AUDIT_DATA_INIT(ad, NET); ad.u.net.netif = dev-name; @@ -3803,16 +3869,25 @@ static unsigned int selinux_ip_postroute goto out; if (selinux_compat_net) -err = selinux_ip_postroute_last_compat(sk, dev, ad, +err = selinux_ip_postroute_last_compat(skb-sk, skb, dev, ad, family, addrp, len); -else -err = avc_has_perm(sksec-sid, skb-secmark, SECCLASS_PACKET, - PACKET__SEND, ad); +else { +if (!skb-secmark) { +u32 xfrm_sid; -if (err) -goto out; +selinux_skb_xfrm_sid(skb, xfrm_sid); -err = selinux_xfrm_postroute_last(sksec-sid, skb, ad); +if (xfrm_sid) +skb-secmark = xfrm_sid; +else if (skb-sk) { +struct sk_security_struct *sksec = +skb-sk-sk_security; +skb-secmark = sksec-sid; +} +} +err = avc_has_perm(skb-secmark, SECINITSID_UNLABELED, + SECCLASS_PACKET, PACKET__FLOW_OUT, ad); +} This looks nearly identical to selinux_skb_flow_out() as implemented above, the only real differences being two of the args to the avc_has_perm() call. Any reason you don't abstract out the common parts to a separate function? May be in the future. Also, the same comment/question about NetLabel support in selinux_skb_flow_out() applies here as well I suspect. My comments earlier should apply here as well. Mine too :) -- paul moore linux security @ hp - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
James Morris wrote: On Fri, 29 Sep 2006, Paul Moore wrote: It seems more of a pain to actually prevent their use at the same time and/or explain strange/unnatural behavior. Agreed, the solution that we agreed upon is much easier to implement and explain than a lot of the alternatives. Ok, can you please explain it further? i.e. show me what the policy looks like, exactly what the user is trying to achieve, and explain what happens to each packet exactly in terms of labeling on the input and output paths. All right, here is my take on it, perhaps Venkat can chime in too. * packet labeling, input Code below is taken from the secid patchset, minus the SID transition code as it sounds like that is going to be dropped. Please note that this is not a patch just something to help explain. +static int selinux_skb_flow_in(struct sk_buff *skb, unsigned short family) +{ + u32 xfrm_sid, trans_sid; u32 nlbl_sid; + int err; + + if (selinux_compat_net) + return 1; + + /* xfrm/cipso inapplicable for loopback traffic */ + if (skb-dev == loopback_dev) + return 1; + + err = selinux_xfrm_decode_session(skb, xfrm_sid, 0); + BUG_ON(err); + + err = avc_has_perm(xfrm_sid, skb-secmark, SECCLASS_PACKET, + PACKET__FLOW_IN, NULL); + if (err) + goto out; + if (xfrm_sid) skb-secmark = xfrm_sid; err = selinux_netlbl_skbuff_getsid(skb, skb-secmark, nlbl_sid); if (err) goto out; if (nlbl_sid != SECINITSID_UNLABELED) { /* not sure if we want this avc check above this if * block? */ err = avc_has_perm(nlbl_sid, skb-secmark, SECCLASS_PACKET, PACKET__FLOW_IN, NULL); if (err) goto out; skb-secmark = nlbl_sid; } + +out: + return err ? 0 : 1; +}; * packet labeling, output This is currently being discussed, so take this with a few grains of salt. +static int selinux_skb_flow_out(struct sk_buff *skb, u32 nf_secid) +{ + u32 trans_sid; u32 nlbl_sid; + int err; + + if (selinux_compat_net) + return 1; + + if (!skb-secmark) { + u32 xfrm_sid; + + selinux_skb_xfrm_sid(skb, xfrm_sid); + + if (xfrm_sid) + skb-secmark = xfrm_sid; + else if (skb-sk) { + struct sk_security_struct *sksec = + skb-sk-sk_security; + skb-secmark = sksec-sid; + } err = selinux_netlbl_skbuff_getsid(skb, skb-secmark, nlbl_sid); if (err) goto out; if (nlbl_sid != SECINITSID_UNLABELED) { err = avc_has_perm(nlbl_sid, skb-secmark, SECCLASS_PACKET, PACKET__FLOW_OUT, NULL); if (err) goto out; skb-secmark = nlbl_sid; } + } + + err = avc_has_perm(skb-secmark, nf_secid, SECCLASS_PACKET, + PACKET__FLOW_OUT, NULL); + +out: + return err ? 0 : 1; +} * pseudo policy Since NetLabel presently only provides a MLS label I don't believe there would be any additional SELinux allow rules beyond those needed for IPsec labeling (maybe in the flow_out, not sure). The MLS/MCS constraints would be the only thing affected I believe, and even then the existing constraints should suffice. -- paul moore linux security @ hp - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
James Morris wrote: On Fri, 29 Sep 2006, James Morris wrote: On Fri, 29 Sep 2006, Paul Moore wrote: It seems more of a pain to actually prevent their use at the same time and/or explain strange/unnatural behavior. Agreed, the solution that we agreed upon is much easier to implement and explain than a lot of the alternatives. Ok, can you please explain it further? i.e. show me what the policy looks like, exactly what the user is trying to achieve, and explain what happens to each packet exactly in terms of labeling on the input and output paths. Also, why can't this be done just with xfrm labeling? I believe the issue Venkat and I were discussing was how to handle the case of multiple external labeling protocols, i.e. what to do if we get a packet through labeled SA which has a CIPSO option. As I've said before, I don't believe this is something we will see much in practice but I think we need to decide what to do: handle it somehow or just punt on the problem and drop it. Several people with experience with external labeling have commented on how supporting both external labeling protocols would be a good idea so Venkat and I are trying to come up with a solution that works. Please see my reponse with the pseudo code/policy examples as this might help clear things up. -- paul moore linux security @ hp - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
On Fri, 29 Sep 2006, Paul Moore wrote: James Morris wrote: Ok, can you please explain it further? i.e. show me what the policy looks like, exactly what the user is trying to achieve, and explain what happens to each packet exactly in terms of labeling on the input and output paths. All right, here is my take on it, perhaps Venkat can chime in too. Thanks, that cleared up many things, but how does this interact with CONNSECMARK? Please provide some example iptables rules, SELinux policy statements, racoon config and netlabel config. I need to understand exactly what happens to each packet in, say, an FTP session and how you envisage the configuration. Here's a sample scenario for the above (let me know if this is not how you expect this to be used): Say that the SA is labeled secret and you have two FTP clients connecting to a server via xinetd on this SA. Each client additionally labels their packets via CIPSO as secret:c1 and secret:c2 respectively. xinetd launches an FTP server for each at the correct level. - James -- James Morris [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
While I don't see any explicit mention of it in the documentation or your comments, I assume we would want a flow_out check for NetLabel here as well? I don't believe we do. By this time, the packet is or should already be carrying the CIPSO/NetLabel option which should already be the right one (derived from the socket or flow as appropriate), but you would want to audit the code to make sure. IOW, the label option in the IP header should already be reflecting the secmark on the skb. But again, you may want to audit the code to make sure. In the case above I am concerned about the situation where the skb-secmark == 0 and there is a IPv4 option (i.e. it is NetLabel labeled) on the packet. Where we initialize the secmark should be immaterial from the NetLabel point of view. The kernel mechanisms should assure that the IP option reflects the MLS portion (or a label in the SA range) elsewhere. In any case, a flow_out check doesn't make sense since the IP option and the secmark are (should be) mirroring each other and there's in actuality no flow out happening; they are just 2 representation of the SAME label. Your suggestion as to adjusting the secmark per the IP option might be fraught with danger since, in certain cases, I believe, you just return the incoming options in the outgoing packet (timewait, openreq, etc.?), and there's no assurance that that's a valid enough option that you can retrieve a sid with it, correct? - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
James Morris wrote: On Fri, 29 Sep 2006, Paul Moore wrote: James Morris wrote: Ok, can you please explain it further? i.e. show me what the policy looks like, exactly what the user is trying to achieve, and explain what happens to each packet exactly in terms of labeling on the input and output paths. All right, here is my take on it, perhaps Venkat can chime in too. Thanks, that cleared up many things, but how does this interact with CONNSECMARK? Please provide some example iptables rules, SELinux policy statements, racoon config and netlabel config. I need to understand exactly what happens to each packet in, say, an FTP session and how you envisage the configuration. Hopefully Venkat can talk to the iptables rule, policy statements, and racoon config. He has the best understanding of how this works with the secid patches. There really is no specific NetLabel config as the NetLabel config only specifies how to create the explicit packet label (CIPSO IPv4 option) using the socket's SID. NetLabel, like SECMARK, is just a packet labeling mechanism. I think the key thing to remember is that the only change brought about by the pseudo-code I posted earlier is that the secmark's MLS label would be adjusted to match the value of the NetLabel (CIPSO option) assuming it passes the avc flow_in checks. Here's a sample scenario for the above (let me know if this is not how you expect this to be used): Say that the SA is labeled secret and you have two FTP clients connecting to a server via xinetd on this SA. Each client additionally labels their packets via CIPSO as secret:c1 and secret:c2 respectively. xinetd launches an FTP server for each at the correct level. I believe Venkat can address this. -- paul moore linux security @ hp - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
Venkat Yekkirala wrote: While I don't see any explicit mention of it in the documentation or your comments, I assume we would want a flow_out check for NetLabel here as well? I don't believe we do. By this time, the packet is or should already be carrying the CIPSO/NetLabel option which should already be the right one (derived from the socket or flow as appropriate), but you would want to audit the code to make sure. IOW, the label option in the IP header should already be reflecting the secmark on the skb. But again, you may want to audit the code to make sure. In the case above I am concerned about the situation where the skb-secmark == 0 and there is a IPv4 option (i.e. it is NetLabel labeled) on the packet. It's unfortunate that you cut out the code in your reply. Where we initialize the secmark should be immaterial from the NetLabel point of view. The kernel mechanisms should assure that the IP option reflects the MLS portion (or a label in the SA range) elsewhere. Yep, I agree. In any case, a flow_out check doesn't make sense since the IP option and the secmark are (should be) mirroring each other and there's in actuality no flow out happening; they are just 2 representation of the SAME label. Well, reading the code I see that if the secmark is zero upon entering the function you query the XFRM subsystem to query the SA label and set the secmark accordingly, yes? All I am suggesting is that we do the same thing for NetLabel. Please see the mail I sent earlier with the code in it to address James' concerns, this has a proposed solution for the flow_out case. Your suggestion as to adjusting the secmark per the IP option might be fraught with danger since, in certain cases, I believe, you just return the incoming options in the outgoing packet (timewait, openreq, etc.?), and there's no assurance that that's a valid enough option that you can retrieve a sid with it, correct? As implemented in the code snippets I sent earlier the generated NetLabel SID would reflect the secmark as determined by the IPsec label and the IP options on the packet. I believe this is what we want as the resulting secmark value would directly represent the security attributes of the packet. -- paul moore linux security @ hp - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
On Fri, 29 Sep 2006, Paul Moore wrote: Say that the SA is labeled secret and you have two FTP clients connecting to a server via xinetd on this SA. Each client additionally labels their packets via CIPSO as secret:c1 and secret:c2 respectively. xinetd launches an FTP server for each at the correct level. I believe Venkat can address this. Ok, I'd still really like to see a worked example of just Netlabel + secmark/connseckmark, to see what happens to the connection marks. It seems that the connection mark should always be correct, and restored to the packet. In which case, what happens when a CIPSO label on an established or related packet doesn't match, or you get no CIPSO label (e.g. ICMP from intermediate router) ? Or, is would you be always overwriting secmark/connsecmark labeling, and if so, how/why are you using them? Venkat, With xfrm labeling, the external packets are always going to be protocol ESP or AH, and we can't connection track the inner protocols. So, external labeling when using xfrm labeling seems somewhat superfluous, except for the case of setting a label based on the interface the packets arrived on. Correct? If so, all you can realistically do with the flow permissions is bind the ESP/AH packets to types of interfaces (which does seem useful for some folk). -- James Morris [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
James Morris wrote: On Fri, 29 Sep 2006, Paul Moore wrote: Say that the SA is labeled secret and you have two FTP clients connecting to a server via xinetd on this SA. Each client additionally labels their packets via CIPSO as secret:c1 and secret:c2 respectively. xinetd launches an FTP server for each at the correct level. I believe Venkat can address this. Ok, I'd still really like to see a worked example of just Netlabel + secmark/connseckmark, to see what happens to the connection marks. Fair enough. It seems that the connection mark should always be correct, and restored to the packet. In which case, what happens when a CIPSO label on an established {connection} ... The following would happen, in order, in selinux_skb_flow_in() (I'm ommitting the IPsec relevant portions of this function for clarity): 1. A NetLabel SID would be generated using the secmark as the base_sid; this means that the NetLabel would be the concatenation of the secmark's TE context and the NetLabel's MLS label. The secmark is not yet modified. 2. The NetLabel SID would be avc checked against the secmark: avc_has_perm(nlbl_sid, skb-secmark, SECCLASS_PACKET, PACKET__FLOW_IN, NULL) Note that in practice this is basically just a MLS label check. 3. If the NetLabel SID != 0 the secmark would be replaced with the NetLabel SID. I am trying to make NetLabel behave in a similar fashion as to how labeled IPsec works in the secid patches; I believe the above steps acomplish this. If not please let me know and I'll make the necessary changes. ... or related packet doesn't match ... Not sure what you mean by related packet, but if the check in step #2 fails then the packet would be dropped. The only case where I can see this happening is that if the MLS/MCS constraints did not allow the flow_in permission. This allows administrators to set specific MLS labels for any iptables object. ... or you get no CIPSO label (e.g. ICMP from intermediate router) ... If there is no packet label that NetLabel recognizes and NetLabel is configured to allow unlabeled traffic then the NetLabel SID generated in step #1 above would be 0. Or, is would you be always overwriting secmark/connsecmark labeling, and if so, how/why are you using them? I think I addressed that above. FYI: in between emails I'm working on a patch against Venkat's secid patches which implements this, hopefully Venkat can roll this patch into his secid patchset so this is all committed/rejected at once. -- paul moore linux security @ hp - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
On Fri, 29 Sep 2006, Paul Moore wrote: ... or related packet doesn't match ... Not sure what you mean by related packet, A related packet with conntrack would be an FTP data packet related to a specific FTP control session (where the conntrack is initialized, and has a secmark saved to it). but if the check in step #2 fails then the packet would be dropped. Ok. The only case where I can see this happening is that if the MLS/MCS constraints did not allow the flow_in permission. This allows administrators to set specific MLS labels for any iptables object. Yep, one thing to watch for here is packets arriving on a different interface for valid reasons (e.g. routing changes), but that's a policy issue. ... or you get no CIPSO label (e.g. ICMP from intermediate router) ... If there is no packet label that NetLabel recognizes and NetLabel is configured to allow unlabeled traffic then the NetLabel SID generated in step #1 above would be 0. Well, conntrack will say that this packet is related to the connection and CONNSECMARK will restore the secmark label to it (i.e. it'll have the same secmark as the initial syn packet). But, no CIPSO label. I guess this needs to be considered in any case, secmark or not. - James -- James Morris [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
James Morris wrote: On Fri, 29 Sep 2006, Paul Moore wrote: ... or you get no CIPSO label (e.g. ICMP from intermediate router) ... If there is no packet label that NetLabel recognizes and NetLabel is configured to allow unlabeled traffic then the NetLabel SID generated in step #1 above would be 0. Well, conntrack will say that this packet is related to the connection and CONNSECMARK will restore the secmark label to it (i.e. it'll have the same secmark as the initial syn packet). But, no CIPSO label. I guess this needs to be considered in any case, secmark or not. Yep, I would categorize this case as 'external label not present, internal label present'. I believe the code as described would do the right thing in allowing admins to control this, it's just up to how you configure the system and what your policy dictates. -- paul moore linux security @ hp - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
Venkat, With xfrm labeling, the external packets are always going to be protocol ESP or AH, and we can't connection track the inner protocols. So, Are you sure? This doesn't compare to what my limited testing seems to have turned up (normal netfiltering of inner protos followed by xfrms, interspersed with their own netfiltering). external labeling when using xfrm labeling seems somewhat superfluous, except for the case of setting a label based on the interface the packets arrived on. Correct? If so, all you can realistically do with the flow permissions is bind the ESP/AH packets to types of interfaces (which does seem useful for some folk). - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
It seems more of a pain to actually prevent their use at the same time and/or explain strange/unnatural behavior. Agreed, the solution that we agreed upon is much easier to implement and explain than a lot of the alternatives. Ok, can you please explain it further? i.e. show me what the policy looks like, exactly what the user is trying to achieve, and explain what happens to each packet exactly in terms of labeling on the input and output paths. Also, why can't this be done just with xfrm labeling? I believe the issue Venkat and I were discussing was how to handle the case of multiple external labeling protocols, i.e. what to do if we get a packet through labeled SA which has a CIPSO option. As I've said before, I don't believe this is something we will see much in practice but I think we need to decide what to do: handle it somehow or just punt on the problem and drop it. Several people with experience with external labeling have commented on how supporting both external labeling protocols would be a good idea so Venkat and I are trying to come up with a solution that works. Here's the scoop on using xfrm and cipso at the same time: JUST REDUNDANT I was only paying attention to the inbound side when I was saying a ranged SA could be used in conjunction with fine-grained cipso labeling. In actuality, this isn't possible because of the change we are instituting for SA selection to be based on the context of the sock (specifically, the sock's MLS range must equal the range on the SA, and cipso at this point would be representing this same range). So, if someone were to configure CIPSO along with labeled ipsec, BOTH mechanisms will be used, with BOTH labels carrying the same MLS range, and with CIPSO always overriding the SA (after a flow_in check) on the inbound. Should just be additional overhead, that's all. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] secid reconciliation-v03: Enforcement for SELinux
Venkat Yekkirala wrote: snip + + err = avc_has_perm(xfrm_sid, skb-secmark, SECCLASS_PACKET, + PACKET__FLOW_IN, NULL); + if (err) + goto out; + + if (xfrm_sid) { + err = security_transition_sid(xfrm_sid, skb-secmark, + SECCLASS_PACKET, trans_sid); + if (err) + goto out; + I thought we weren't doing transitions to label packets anymore per the conference call? - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html