SELinux: Fix SA selection semantics

Fix the selection of an SA for an outgoing packet to be at the same
context as the originating socket/flow. This eliminates the SELinux
policy's ability to use/sendto SAs with contexts other than the socket's.

With this patch applied, the SELinux policy will require one or more of the
following for a socket to be able to communicate with/without SAs:

1. To enable a socket to communicate without using labeled-IPSec SAs:

allow socket_t unlabeled_t:association { sendto recvfrom }

2. To enable a socket to communicate with labeled-IPSec SAs:

allow socket_t self:association { sendto };
allow socket_t peer_sa_t:association { recvfrom };

Signed-off-by: Venkat Yekkirala <vyekkirala@TrustedCS.com>
Signed-off-by: James Morris <jmorris@namei.org>

diff --git a/include/linux/security.h b/include/linux/security.h
index 84cebcd..83cdefa 100644 (file)
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -886,11 +886,6 @@ struct request_sock;
  *     @xp contains the policy to check for a match.
  *     @fl contains the flow to check for a match.
  *     Return 1 if there is a match.
- * @xfrm_flow_state_match:
- *     @fl contains the flow key to match.
- *     @xfrm points to the xfrm_state to match.
- *     @xp points to the xfrm_policy to match.
- *     Return 1 if there is a match.
  * @xfrm_decode_session:
  *     @skb points to skb to decode.
  *     @secid points to the flow key secid to set.
@@ -1388,8 +1383,6 @@ struct security_operations {
        int (*xfrm_policy_lookup)(struct xfrm_policy *xp, u32 fl_secid, u8 dir);
        int (*xfrm_state_pol_flow_match)(struct xfrm_state *x,
                        struct xfrm_policy *xp, struct flowi *fl);
-       int (*xfrm_flow_state_match)(struct flowi *fl, struct xfrm_state *xfrm,
-                       struct xfrm_policy *xp);
        int (*xfrm_decode_session)(struct sk_buff *skb, u32 *secid, int ckall);
 #endif /* CONFIG_SECURITY_NETWORK_XFRM */
 
@@ -3186,12 +3179,6 @@ static inline int security_xfrm_state_pol_flow_match(struct xfrm_state *x,
        return security_ops->xfrm_state_pol_flow_match(x, xp, fl);
 }
 
-static inline int security_xfrm_flow_state_match(struct flowi *fl,
-                       struct xfrm_state *xfrm, struct xfrm_policy *xp)
-{
-       return security_ops->xfrm_flow_state_match(fl, xfrm, xp);
-}
-
 static inline int security_xfrm_decode_session(struct sk_buff *skb, u32 *secid)
 {
        return security_ops->xfrm_decode_session(skb, secid, 1);
@@ -3255,12 +3242,6 @@ static inline int security_xfrm_state_pol_flow_match(struct xfrm_state *x,
        return 1;
 }
 
-static inline int security_xfrm_flow_state_match(struct flowi *fl,
-                       struct xfrm_state *xfrm, struct xfrm_policy *xp)
-{
-       return 1;
-}
-
 static inline int security_xfrm_decode_session(struct sk_buff *skb, u32 *secid)
 {
        return 0;

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 7736b23..b88b038 100644 (file)
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1894,7 +1894,8 @@ int xfrm_bundle_ok(struct xfrm_policy *pol, struct xfrm_dst *first,
 
                if (fl && !xfrm_selector_match(&dst->xfrm->sel, fl, family))
                        return 0;
-               if (fl && !security_xfrm_flow_state_match(fl, dst->xfrm, pol))
+               if (fl && pol &&
+                   !security_xfrm_state_pol_flow_match(dst->xfrm, pol, fl))
                        return 0;
                if (dst->xfrm->km.state != XFRM_STATE_VALID)
                        return 0;

diff --git a/security/dummy.c b/security/dummy.c
index 0148d15..558795b 100644 (file)
--- a/security/dummy.c
+++ b/security/dummy.c
@@ -886,12 +886,6 @@ static int dummy_xfrm_state_pol_flow_match(struct xfrm_state *x,
        return 1;
 }
 
-static int dummy_xfrm_flow_state_match(struct flowi *fl, struct xfrm_state *xfrm,
-                               struct xfrm_policy *xp)
-{
-       return 1;
-}
-
 static int dummy_xfrm_decode_session(struct sk_buff *skb, u32 *fl, int ckall)
 {
        return 0;
@@ -1126,7 +1120,6 @@ void security_fixup_ops (struct security_operations *ops)
        set_to_dummy_if_null(ops, xfrm_state_delete_security);
        set_to_dummy_if_null(ops, xfrm_policy_lookup);
        set_to_dummy_if_null(ops, xfrm_state_pol_flow_match);
-       set_to_dummy_if_null(ops, xfrm_flow_state_match);
        set_to_dummy_if_null(ops, xfrm_decode_session);
 #endif /* CONFIG_SECURITY_NETWORK_XFRM */
 #ifdef CONFIG_KEYS

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 5bbd599..956137b 100644 (file)
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2889,7 +2889,8 @@ static void selinux_task_to_inode(struct task_struct *p,
 }
 
 /* Returns error only if unable to parse addresses */
-static int selinux_parse_skb_ipv4(struct sk_buff *skb, struct avc_audit_data *ad)
+static int selinux_parse_skb_ipv4(struct sk_buff *skb,
+                       struct avc_audit_data *ad, u8 *proto)
 {
        int offset, ihlen, ret = -EINVAL;
        struct iphdr _iph, *ih;
@@ -2907,6 +2908,9 @@ static int selinux_parse_skb_ipv4(struct sk_buff *skb, struct avc_audit_data *ad
        ad->u.net.v4info.daddr = ih->daddr;
        ret = 0;
 
+       if (proto)
+               *proto = ih->protocol;
+
        switch (ih->protocol) {
         case IPPROTO_TCP: {
                struct tcphdr _tcph, *th;
@@ -2950,7 +2954,8 @@ out:
 #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
 
 /* Returns error only if unable to parse addresses */
-static int selinux_parse_skb_ipv6(struct sk_buff *skb, struct avc_audit_data *ad)
+static int selinux_parse_skb_ipv6(struct sk_buff *skb,
+                       struct avc_audit_data *ad, u8 *proto)
 {
        u8 nexthdr;
        int ret = -EINVAL, offset;
@@ -2971,6 +2976,9 @@ static int selinux_parse_skb_ipv6(struct sk_buff *skb, struct avc_audit_data *ad
        if (offset < 0)
                goto out;
 
+       if (proto)
+               *proto = nexthdr;
+
        switch (nexthdr) {
        case IPPROTO_TCP: {
                struct tcphdr _tcph, *th;
@@ -3007,13 +3015,13 @@ out:
 #endif /* IPV6 */
 
 static int selinux_parse_skb(struct sk_buff *skb, struct avc_audit_data *ad,
-                            char **addrp, int *len, int src)
+                            char **addrp, int *len, int src, u8 *proto)
 {
        int ret = 0;
 
        switch (ad->u.net.family) {
        case PF_INET:
-               ret = selinux_parse_skb_ipv4(skb, ad);
+               ret = selinux_parse_skb_ipv4(skb, ad, proto);
                if (ret || !addrp)
                        break;
                *len = 4;
@@ -3023,7 +3031,7 @@ static int selinux_parse_skb(struct sk_buff *skb, struct avc_audit_data *ad,
 
 #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
        case PF_INET6:
-               ret = selinux_parse_skb_ipv6(skb, ad);
+               ret = selinux_parse_skb_ipv6(skb, ad, proto);
                if (ret || !addrp)
                        break;
                *len = 16;
@@ -3494,7 +3502,7 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
        ad.u.net.netif = skb->dev ? skb->dev->name : "[unknown]";
        ad.u.net.family = family;
 
-       err = selinux_parse_skb(skb, &ad, &addrp, &len, 1);
+       err = selinux_parse_skb(skb, &ad, &addrp, &len, 1, NULL);
        if (err)
                goto out;
 
@@ -3820,6 +3828,7 @@ static unsigned int selinux_ip_postroute_last(unsigned int hooknum,
        struct avc_audit_data ad;
        struct net_device *dev = (struct net_device *)out;
        struct sk_security_struct *sksec;
+       u8 proto;
 
        sk = skb->sk;
        if (!sk)
@@ -3831,7 +3840,7 @@ static unsigned int selinux_ip_postroute_last(unsigned int hooknum,
        ad.u.net.netif = dev->name;
        ad.u.net.family = family;
 
-       err = selinux_parse_skb(skb, &ad, &addrp, &len, 0);
+       err = selinux_parse_skb(skb, &ad, &addrp, &len, 0, &proto);
        if (err)
                goto out;
 
@@ -3845,7 +3854,7 @@ static unsigned int selinux_ip_postroute_last(unsigned int hooknum,
        if (err)
                goto out;
 
-       err = selinux_xfrm_postroute_last(sksec->sid, skb, &ad);
+       err = selinux_xfrm_postroute_last(sksec->sid, skb, &ad, proto);
 out:
        return err ? NF_DROP : NF_ACCEPT;
 }
@@ -4764,7 +4773,6 @@ static struct security_operations selinux_ops = {
        .xfrm_state_delete_security =   selinux_xfrm_state_delete,
        .xfrm_policy_lookup =           selinux_xfrm_policy_lookup,
        .xfrm_state_pol_flow_match =    selinux_xfrm_state_pol_flow_match,
-       .xfrm_flow_state_match =        selinux_xfrm_flow_state_match,
        .xfrm_decode_session =          selinux_xfrm_decode_session,
 #endif
 

diff --git a/security/selinux/include/xfrm.h b/security/selinux/include/xfrm.h
index 2750236..ebd7246 100644 (file)
--- a/security/selinux/include/xfrm.h
+++ b/security/selinux/include/xfrm.h
@@ -19,9 +19,6 @@ int selinux_xfrm_state_delete(struct xfrm_state *x);
 int selinux_xfrm_policy_lookup(struct xfrm_policy *xp, u32 fl_secid, u8 dir);
 int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x,
                        struct xfrm_policy *xp, struct flowi *fl);
-int selinux_xfrm_flow_state_match(struct flowi *fl, struct xfrm_state *xfrm,
-                       struct xfrm_policy *xp);
-
 
 /*
  * Extract the security blob from the sock (it's actually on the socket)
@@ -38,7 +35,7 @@ static inline struct inode_security_struct *get_sock_isec(struct sock *sk)
 int selinux_xfrm_sock_rcv_skb(u32 sid, struct sk_buff *skb,
                        struct avc_audit_data *ad);
 int selinux_xfrm_postroute_last(u32 isec_sid, struct sk_buff *skb,
-                       struct avc_audit_data *ad);
+                       struct avc_audit_data *ad, u8 proto);
 u32 selinux_socket_getpeer_dgram(struct sk_buff *skb);
 int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, int ckall);
 #else
@@ -49,7 +46,7 @@ static inline int selinux_xfrm_sock_rcv_skb(u32 isec_sid, struct sk_buff *skb,
 }
 
 static inline int selinux_xfrm_postroute_last(u32 isec_sid, struct sk_buff *skb,
-                       struct avc_audit_data *ad)
+                       struct avc_audit_data *ad, u8 proto)
 {
        return 0;
 }

diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c
index 8fef742..9b77714 100644 (file)
--- a/security/selinux/xfrm.c
+++ b/security/selinux/xfrm.c
@@ -115,71 +115,40 @@ int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x, struct xfrm_policy *
                        struct flowi *fl)
 {
        u32 state_sid;
-       u32 pol_sid;
-       int err;
+       int rc;
 
-       if (xp->security) {
-               if (!x->security)
-                       /* unlabeled SA and labeled policy can't match */
-                       return 0;
-               else
-                       state_sid = x->security->ctx_sid;
-               pol_sid = xp->security->ctx_sid;
-       } else
+       if (!xp->security)
                if (x->security)
                        /* unlabeled policy and labeled SA can't match */
                        return 0;
                else
                        /* unlabeled policy and unlabeled SA match all flows */
                        return 1;
-
-       err = avc_has_perm(state_sid, pol_sid, SECCLASS_ASSOCIATION,
-                         ASSOCIATION__POLMATCH,
-                         NULL);
-
-       if (err)
-               return 0;
-
-       err = avc_has_perm(fl->secid, state_sid, SECCLASS_ASSOCIATION,
-                         ASSOCIATION__SENDTO,
-                         NULL)? 0:1;
-
-       return err;
-}
-
-/*
- * LSM hook implementation that authorizes that a particular outgoing flow
- * can use a given security association.
- */
-
-int selinux_xfrm_flow_state_match(struct flowi *fl, struct xfrm_state *xfrm,
-                                 struct xfrm_policy *xp)
-{
-       int rc = 0;
-       u32 sel_sid = SECINITSID_UNLABELED;
-       struct xfrm_sec_ctx *ctx;
-
-       if (!xp->security)
-               if (!xfrm->security)
-                       return 1;
-               else
-                       return 0;
        else
-               if (!xfrm->security)
+               if (!x->security)
+                       /* unlabeled SA and labeled policy can't match */
                        return 0;
+               else
+                       if (!selinux_authorizable_xfrm(x))
+                               /* Not a SELinux-labeled SA */
+                               return 0;
 
-       /* Context sid is either set to label or ANY_ASSOC */
-       if ((ctx = xfrm->security)) {
-               if (!selinux_authorizable_ctx(ctx))
-                       return 0;
+       state_sid = x->security->ctx_sid;
 
-               sel_sid = ctx->ctx_sid;
-       }
+       if (fl->secid != state_sid)
+               return 0;
 
-       rc = avc_has_perm(fl->secid, sel_sid, SECCLASS_ASSOCIATION,
+       rc = avc_has_perm(fl->secid, state_sid, SECCLASS_ASSOCIATION,
                          ASSOCIATION__SENDTO,
                          NULL)? 0:1;
 
+       /*
+        * We don't need a separate SA Vs. policy polmatch check
+        * since the SA is now of the same label as the flow and
+        * a flow Vs. policy polmatch check had already happened
+        * in selinux_xfrm_policy_lookup() above.
+        */
+
        return rc;
 }
 
@@ -481,6 +450,13 @@ int selinux_xfrm_sock_rcv_skb(u32 isec_sid, struct sk_buff *skb,
                }
        }
 
+       /*
+        * This check even when there's no association involved is
+        * intended, according to Trent Jaeger, to make sure a
+        * process can't engage in non-ipsec communication unless
+        * explicitly allowed by policy.
+        */
+
        rc = avc_has_perm(isec_sid, sel_sid, SECCLASS_ASSOCIATION,
                          ASSOCIATION__RECVFROM, ad);
 
@@ -492,10 +468,10 @@ int selinux_xfrm_sock_rcv_skb(u32 isec_sid, struct sk_buff *skb,
  * If we have no security association, then we need to determine
  * whether the socket is allowed to send to an unlabelled destination.
  * If we do have a authorizable security association, then it has already been
- * checked in xfrm_policy_lookup hook.
+ * checked in the selinux_xfrm_state_pol_flow_match hook above.
  */
 int selinux_xfrm_postroute_last(u32 isec_sid, struct sk_buff *skb,
-                                       struct avc_audit_data *ad)
+                                       struct avc_audit_data *ad, u8 proto)
 {
        struct dst_entry *dst;
        int rc = 0;
@@ -514,6 +490,27 @@ int selinux_xfrm_postroute_last(u32 isec_sid, struct sk_buff *skb,
                }
        }
 
+       switch (proto) {
+       case IPPROTO_AH:
+       case IPPROTO_ESP:
+       case IPPROTO_COMP:
+               /*
+                * We should have already seen this packet once before
+                * it underwent xfrm(s). No need to subject it to the
+                * unlabeled check.
+                */
+               goto out;
+       default:
+               break;
+       }
+
+       /*
+        * This check even when there's no association involved is
+        * intended, according to Trent Jaeger, to make sure a
+        * process can't engage in non-ipsec communication unless
+        * explicitly allowed by policy.
+        */
+
        rc = avc_has_perm(isec_sid, SECINITSID_UNLABELED, SECCLASS_ASSOCIATION,
                          ASSOCIATION__SENDTO, ad);
 out: