Re: [PATCH 7/7] secid reconciliation-v02: Enforcement for SELinux
Venkat Yekkirala wrote:
>>>+static int selinux_skb_policy_check(struct sk_buff *skb,
>>
>>unsigned short
>>
>>>family) +{
>>>+u32 xfrm_sid, trans_sid;
>>>+int err;
>>>+
>>>+if (selinux_compat_net)
>>>+return 1;
>>>+
>>>+err = selinux_xfrm_decode_session(skb, &xfrm_sid, 0);
>>>+BUG_ON(err);
>>
>>First, any reason against including the "struct sock *" in
>>the LSM hook? At a
>>quick glance it looks like it is available at each place
>>security_skb_policy_check() is invoked? If there are no
>>objections I would
>>like to see it included in the hook.
>
> There's no sock available (NULL) for forward, no-sock, time-wait cases, etc.
... which would be why I should have taken a closer look :)
> What you are trying to accomplish with the sock here anyway?
Actually this is no longer an issue because of something else - you can
ignore this now.
--
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-v02: Enforcement for SELinux
> > +static int selinux_skb_policy_check(struct sk_buff *skb,
> unsigned short
> > family) +{
> > + u32 xfrm_sid, trans_sid;
> > + int err;
> > +
> > + if (selinux_compat_net)
> > + return 1;
> > +
> > + err = selinux_xfrm_decode_session(skb, &xfrm_sid, 0);
> > + BUG_ON(err);
>
> First, any reason against including the "struct sock *" in
> the LSM hook? At a
> quick glance it looks like it is available at each place
> security_skb_policy_check() is invoked? If there are no
> objections I would
> like to see it included in the hook.
There's no sock available (NULL) for forward, no-sock, time-wait cases, etc.
What you are trying to accomplish with the sock here anyway?
>
> Second, I wonder if it would be better to do a NetLabel/CIPSO
> query here using
> the xfrm_sid as the NetLabel "base_sid" instead of at the end
> of the function
> (see your comment)? This way we wouldn't have to duplicate the
> avc_has_perm() and security_transition_sid() calls for both xfrm and
> NetLabel.
There's a need for an additional avc_has_perm check anyway between
the cipso label and the ipsec/transition label, to check to make sure
the cipso level falls within the range on the IPSec/transition SA.
No need for a new transition between ipsec/transition label and the cipso
label since the cipso label would be sharing the TE portion with the
ipsec/transition label (this could change in the future, when you get round
to doing entire SELinux contexts over the wire). For now, you would just
set the secmark to the cipso label if the label could come thru
(i.e. if the avc_has_perm succeeds).
-
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-v02: Enforcement for SELinux
On Friday 08 September 2006 12:50 pm, Venkat Yekkirala wrote:
> This defines SELinux enforcement of the 2 new LSM hooks.
{snip}
> +static int selinux_skb_policy_check(struct sk_buff *skb, unsigned short
> family) +{
> + u32 xfrm_sid, trans_sid;
> + int err;
> +
> + if (selinux_compat_net)
> + return 1;
> +
> + err = selinux_xfrm_decode_session(skb, &xfrm_sid, 0);
> + BUG_ON(err);
First, any reason against including the "struct sock *" in the LSM hook? At a
quick glance it looks like it is available at each place
security_skb_policy_check() is invoked? If there are no objections I would
like to see it included in the hook.
Second, I wonder if it would be better to do a NetLabel/CIPSO query here using
the xfrm_sid as the NetLabel "base_sid" instead of at the end of the function
(see your comment)? This way we wouldn't have to duplicate the
avc_has_perm() and security_transition_sid() calls for both xfrm and
NetLabel. It just seems to be more inline with the whole secid
reconciliation concept.
I don't feel too strongly either way, I just thought it was worth exploring -
thoughts?
> + 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 */
> +
> +out:
> + return err ? 0 : 1;
> +};
--
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-v02: Enforcement for SELinux
> On Fri, 8 Sep 2006, Venkat Yekkirala wrote:
>
> > + if (selinux_compat_net) {
> > + err = selinux_xfrm_decode_session(skb, &peersid, 0);
> > + BUG_ON(err);
>
> I'm pretty sure this should not be a BUG_ON. IIUC, you want
> to panic the
> kernel because one of the nested SAs has a different security context.
No, we are sending in 0 for the ckall param by which we are telling
the function NOT to do any checks, but to simply set the return param
peersid to the secid on the first xfrm if any and succeed by returning 0.
Must not fail.
-
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-v02: Enforcement for SELinux
On Fri, 8 Sep 2006, Venkat Yekkirala wrote: > This defines SELinux enforcement of the 2 new LSM hooks. > I think this looks ok in general (I have a couple more technical issues), athough I believe that Stephen has some question about policy construction. Please rename these hooks: + * @skb_policy_check: + * @skb_netfilter_check to: skb_flow_in skb_flow_out - 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-v02: Enforcement for SELinux
On Fri, 8 Sep 2006, Venkat Yekkirala wrote:
> + if (selinux_compat_net) {
> + err = selinux_xfrm_decode_session(skb, &peersid, 0);
> + BUG_ON(err);
I'm pretty sure this should not be a BUG_ON. IIUC, you want to panic the
kernel because one of the nested SAs has a different security context.
> + err = selinux_xfrm_decode_session(skb, &xfrm_sid, 0);
> + BUG_ON(err);
Same.
--
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
[PATCH 7/7] secid reconciliation-v02: Enforcement for SELinux
This defines SELinux enforcement of the 2 new LSM hooks.
Signed-off-by: Venkat Yekkirala <[EMAIL PROTECTED]>
---
security/selinux/hooks.c| 125 --
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, 136 insertions(+), 26 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 5a66c4c..044e452 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,78 @@ static void selinux_req_classify_flow(co
fl->secid = req->secid;
}
+static int selinux_skb_policy_check(struct sk_buff *skb, unsigned short family)
+{
+ u32 xfrm_sid, trans_sid;
+ int err;
+
+ if (selinux_compat_net)
+ 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) {
+ 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 */
+
+out:
+ return err ? 0 : 1;
+};
+
+static int selinux_skb_netfilter_check(struct sk_buff *skb, u32 nf_secid)
+{
+ u32 xfrm_sid;
+ u32 trans_sid;
+ int err;
+
+ if (selinux_compat_net)
+ return 1;
+
+ if (!skb->secmark && skb->sk) {
+ struct sk_security_struct *sksec = skb->sk->sk_security;
+ skb->secmark = sksec->sid;
+ }
+
+ selinux_skb_xfrm_sid(skb, &xfrm_sid);
+
+ err = avc_has_perm(skb->secmark, xfrm_sid, SECCLASS_PACKET,
+ PACKET__FLOW_OUT, 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;
+ }
+
+ err = avc_has_perm(skb->secmark, nf_secid, SECCLASS_PACKET,
+ PACKET__FLOW_OUT, NULL);
+
+out:
+ /* Signal postroute_last that we are done with this skb */
+ skb->secmark = SECSID_WILD;
+
+ return err ? 0 : 1;
+}
+
static int selinux_nlmsg_perm(struct sock *sk, struct sk_buff *skb)
{
int err = 0;
@@ -3700,7 +3775,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 +3786,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 +3847,11 @@ static int selinux_ip_postroute_last_com
err = avc_has_perm(isec->sid, port_sid, isec->sclass,
send_perm, ad);
+
