selinux: Better local/forward check in selinux_ip_postroute()

It turns out that checking to see if skb->sk is NULL is not a very good
indicator of a forwarded packet as some locally generated packets also have
skb->sk set to NULL.  Fix this by not only checking the skb->sk field but also
the IP[6]CB(skb)->flags field for the IP[6]SKB_FORWARDED flag.  While we are
at it, we are calling selinux_parse_skb() much earlier than we really should
resulting in potentially wasted cycles parsing packets for information we
might no use; so shuffle the code around a bit to fix this.

Signed-off-by: Paul Moore <paul.moore@hp.com>
Acked-by: James Morris <jmorris@namei.org>

1 files changed, 81 insertions, 45 deletions

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 223f474bee86..b520667a24be 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4070,20 +4070,28 @@ static int selinux_sock_rcv_skb_iptables_compat(struct sock *sk,
 }
 
 static int selinux_sock_rcv_skb_compat(struct sock *sk, struct sk_buff *skb,
-                                       struct avc_audit_data *ad,
-                                       u16 family, char *addrp)
+                                       u16 family)
 {
         int err;
         struct sk_security_struct *sksec = sk->sk_security;
         u32 peer_sid;
         u32 sk_sid = sksec->sid;
+        struct avc_audit_data ad;
+        char *addrp;
+
+        AVC_AUDIT_DATA_INIT(&ad, NET);
+        ad.u.net.netif = skb->iif;
+        ad.u.net.family = family;
+        err = selinux_parse_skb(skb, &ad, &addrp, 1, NULL);
+        if (err)
+                return err;
 
         if (selinux_compat_net)
-                err = selinux_sock_rcv_skb_iptables_compat(sk, skb, ad,
+                err = selinux_sock_rcv_skb_iptables_compat(sk, skb, &ad,
                                                            family, addrp);
         else
                 err = avc_has_perm(sk_sid, skb->secmark, SECCLASS_PACKET,
-                                   PACKET__RECV, ad);
+                                   PACKET__RECV, &ad);
         if (err)
                 return err;
 
@@ -4092,12 +4100,12 @@ static int selinux_sock_rcv_skb_compat(struct sock *sk, struct sk_buff *skb,
                 if (err)
                         return err;
                 err = avc_has_perm(sk_sid, peer_sid,
-                                   SECCLASS_PEER, PEER__RECV, ad);
+                                   SECCLASS_PEER, PEER__RECV, &ad);
         } else {
-                err = selinux_netlbl_sock_rcv_skb(sksec, skb, family, ad);
+                err = selinux_netlbl_sock_rcv_skb(sksec, skb, family, &ad);
                 if (err)
                         return err;
-                err = selinux_xfrm_sock_rcv_skb(sksec->sid, skb, ad);
+                err = selinux_xfrm_sock_rcv_skb(sksec->sid, skb, &ad);
         }
 
         return err;
@@ -4111,6 +4119,8 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
         u32 sk_sid = sksec->sid;
         struct avc_audit_data ad;
         char *addrp;
+        u8 secmark_active;
+        u8 peerlbl_active;
 
         if (family != PF_INET && family != PF_INET6)
                 return 0;
@@ -4119,6 +4129,18 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
         if (family == PF_INET6 && skb->protocol == htons(ETH_P_IP))
                 family = PF_INET;
 
+        /* If any sort of compatibility mode is enabled then handoff processing
+         * to the selinux_sock_rcv_skb_compat() function to deal with the
+         * special handling.  We do this in an attempt to keep this function
+         * as fast and as clean as possible. */
+        if (selinux_compat_net || !selinux_policycap_netpeer)
+                return selinux_sock_rcv_skb_compat(sk, skb, family);
+
+        secmark_active = selinux_secmark_enabled();
+        peerlbl_active = netlbl_enabled() || selinux_xfrm_enabled();
+        if (!secmark_active && !peerlbl_active)
+                return 0;
+
         AVC_AUDIT_DATA_INIT(&ad, NET);
         ad.u.net.netif = skb->iif;
         ad.u.net.family = family;
@@ -4126,15 +4148,7 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
         if (err)
                 return err;
 
-        /* If any sort of compatibility mode is enabled then handoff processing
-         * to the selinux_sock_rcv_skb_compat() function to deal with the
-         * special handling.  We do this in an attempt to keep this function
-         * as fast and as clean as possible. */
-        if (selinux_compat_net || !selinux_policycap_netpeer)
-                return selinux_sock_rcv_skb_compat(sk, skb, &ad,
-                                                   family, addrp);
-
-        if (netlbl_enabled() || selinux_xfrm_enabled()) {
+        if (peerlbl_active) {
                 u32 peer_sid;
 
                 err = selinux_skb_peerlbl_sid(skb, family, &peer_sid);
@@ -4148,7 +4162,7 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
                                    PEER__RECV, &ad);
         }
 
-        if (selinux_secmark_enabled()) {
+        if (secmark_active) {
                 err = avc_has_perm(sk_sid, skb->secmark, SECCLASS_PACKET,
                                    PACKET__RECV, &ad);
                 if (err)
@@ -4396,15 +4410,15 @@ static unsigned int selinux_ip_forward(struct sk_buff *skb, int ifindex,
         if (!secmark_active && !peerlbl_active)
                 return NF_ACCEPT;
 
+        if (selinux_skb_peerlbl_sid(skb, family, &peer_sid) != 0)
+                return NF_DROP;
+
         AVC_AUDIT_DATA_INIT(&ad, NET);
         ad.u.net.netif = ifindex;
         ad.u.net.family = family;
         if (selinux_parse_skb(skb, &ad, &addrp, 1, NULL) != 0)
                 return NF_DROP;
 
-        if (selinux_skb_peerlbl_sid(skb, family, &peer_sid) != 0)
-                return NF_DROP;
-
         if (peerlbl_active)
                 if (selinux_inet_sys_rcv_skb(ifindex, addrp, family,
                                              peer_sid, &ad) != 0)
@@ -4505,30 +4519,36 @@ static int selinux_ip_postroute_iptables_compat(struct sock *sk,
 
 static unsigned int selinux_ip_postroute_compat(struct sk_buff *skb,
                                                 int ifindex,
-                                                struct avc_audit_data *ad,
-                                                u16 family,
-                                                char *addrp,
-                                                u8 proto)
+                                                u16 family)
 {
         struct sock *sk = skb->sk;
         struct sk_security_struct *sksec;
+        struct avc_audit_data ad;
+        char *addrp;
+        u8 proto;
 
         if (sk == NULL)
                 return NF_ACCEPT;
         sksec = sk->sk_security;
 
+        AVC_AUDIT_DATA_INIT(&ad, NET);
+        ad.u.net.netif = ifindex;
+        ad.u.net.family = family;
+        if (selinux_parse_skb(skb, &ad, &addrp, 0, &proto))
+                return NF_DROP;
+
         if (selinux_compat_net) {
                 if (selinux_ip_postroute_iptables_compat(skb->sk, ifindex,
-                                                         ad, family, addrp))
+                                                         &ad, family, addrp))
                         return NF_DROP;
         } else {
                 if (avc_has_perm(sksec->sid, skb->secmark,
-                                 SECCLASS_PACKET, PACKET__SEND, ad))
+                                 SECCLASS_PACKET, PACKET__SEND, &ad))
                         return NF_DROP;
         }
 
         if (selinux_policycap_netpeer)
-                if (selinux_xfrm_postroute_last(sksec->sid, skb, ad, proto))
+                if (selinux_xfrm_postroute_last(sksec->sid, skb, &ad, proto))
                         return NF_DROP;
 
         return NF_ACCEPT;
@@ -4542,23 +4562,15 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, int ifindex,
         struct sock *sk;
         struct avc_audit_data ad;
         char *addrp;
-        u8 proto;
         u8 secmark_active;
         u8 peerlbl_active;
 
-        AVC_AUDIT_DATA_INIT(&ad, NET);
-        ad.u.net.netif = ifindex;
-        ad.u.net.family = family;
-        if (selinux_parse_skb(skb, &ad, &addrp, 0, &proto))
-                return NF_DROP;
-
         /* If any sort of compatibility mode is enabled then handoff processing
          * to the selinux_ip_postroute_compat() function to deal with the
          * special handling.  We do this in an attempt to keep this function
          * as fast and as clean as possible. */
         if (selinux_compat_net || !selinux_policycap_netpeer)
-                return selinux_ip_postroute_compat(skb, ifindex, &ad,
-                                                   family, addrp, proto);
+                return selinux_ip_postroute_compat(skb, ifindex, family);
 
         /* If skb->dst->xfrm is non-NULL then the packet is undergoing an IPsec
          * packet transformation so allow the packet to pass without any checks
@@ -4574,21 +4586,45 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, int ifindex,
         if (!secmark_active && !peerlbl_active)
                 return NF_ACCEPT;
 
-        /* if the packet is locally generated (skb->sk != NULL) then use the
-         * socket's label as the peer label, otherwise the packet is being
-         * forwarded through this system and we need to fetch the peer label
-         * directly from the packet */
+        /* if the packet is being forwarded then get the peer label from the
+         * packet itself; otherwise check to see if it is from a local
+         * application or the kernel, if from an application get the peer label
+         * from the sending socket, otherwise use the kernel's sid */
         sk = skb->sk;
-        if (sk) {
+        if (sk == NULL) {
+                switch (family) {
+                case PF_INET:
+                        if (IPCB(skb)->flags & IPSKB_FORWARDED)
+                                secmark_perm = PACKET__FORWARD_OUT;
+                        else
+                                secmark_perm = PACKET__SEND;
+                        break;
+                case PF_INET6:
+                        if (IP6CB(skb)->flags & IP6SKB_FORWARDED)
+                                secmark_perm = PACKET__FORWARD_OUT;
+                        else
+                                secmark_perm = PACKET__SEND;
+                        break;
+                default:
+                        return NF_DROP;
+                }
+                if (secmark_perm == PACKET__FORWARD_OUT) {
+                        if (selinux_skb_peerlbl_sid(skb, family, &peer_sid))
+                                return NF_DROP;
+                } else
+                        peer_sid = SECINITSID_KERNEL;
+        } else {
                 struct sk_security_struct *sksec = sk->sk_security;
                 peer_sid = sksec->sid;
                 secmark_perm = PACKET__SEND;
-        } else {
-                if (selinux_skb_peerlbl_sid(skb, family, &peer_sid))
-                                return NF_DROP;
-                secmark_perm = PACKET__FORWARD_OUT;
         }
 
+        AVC_AUDIT_DATA_INIT(&ad, NET);
+        ad.u.net.netif = ifindex;
+        ad.u.net.family = family;
+        if (selinux_parse_skb(skb, &ad, &addrp, 0, NULL))
+                return NF_DROP;
+
         if (secmark_active)
                 if (avc_has_perm(peer_sid, skb->secmark,
                                  SECCLASS_PACKET, secmark_perm, &ad))