Skip to content

Commit 5c6c268

Browse files
committed
selinux: process labeled IPsec TCP SYN-ACK packets properly in selinux_ip_postroute()
Due to difficulty in arriving at the proper security label for TCP SYN-ACK packets in selinux_ip_postroute(), we need to check packets while/before they are undergoing XFRM transforms instead of waiting until afterwards so that we can determine the correct security label. Reported-by: Janak Desai <[email protected]> Cc: [email protected] Signed-off-by: Paul Moore <[email protected]>
1 parent 5b67c49 commit 5c6c268

File tree

1 file changed

+35
-7
lines changed

1 file changed

+35
-7
lines changed

security/selinux/hooks.c

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4846,22 +4846,31 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, int ifindex,
48464846
* as fast and as clean as possible. */
48474847
if (!selinux_policycap_netpeer)
48484848
return selinux_ip_postroute_compat(skb, ifindex, family);
4849+
4850+
secmark_active = selinux_secmark_enabled();
4851+
peerlbl_active = selinux_peerlbl_enabled();
4852+
if (!secmark_active && !peerlbl_active)
4853+
return NF_ACCEPT;
4854+
4855+
sk = skb->sk;
4856+
48494857
#ifdef CONFIG_XFRM
48504858
/* If skb->dst->xfrm is non-NULL then the packet is undergoing an IPsec
48514859
* packet transformation so allow the packet to pass without any checks
48524860
* since we'll have another chance to perform access control checks
48534861
* when the packet is on it's final way out.
48544862
* NOTE: there appear to be some IPv6 multicast cases where skb->dst
4855-
* is NULL, in this case go ahead and apply access control. */
4856-
if (skb_dst(skb) != NULL && skb_dst(skb)->xfrm != NULL)
4863+
* is NULL, in this case go ahead and apply access control.
4864+
* NOTE: if this is a local socket (skb->sk != NULL) that is in the
4865+
* TCP listening state we cannot wait until the XFRM processing
4866+
* is done as we will miss out on the SA label if we do;
4867+
* unfortunately, this means more work, but it is only once per
4868+
* connection. */
4869+
if (skb_dst(skb) != NULL && skb_dst(skb)->xfrm != NULL &&
4870+
!(sk != NULL && sk->sk_state == TCP_LISTEN))
48574871
return NF_ACCEPT;
48584872
#endif
4859-
secmark_active = selinux_secmark_enabled();
4860-
peerlbl_active = selinux_peerlbl_enabled();
4861-
if (!secmark_active && !peerlbl_active)
4862-
return NF_ACCEPT;
48634873

4864-
sk = skb->sk;
48654874
if (sk == NULL) {
48664875
/* Without an associated socket the packet is either coming
48674876
* from the kernel or it is being forwarded; check the packet
@@ -4889,6 +4898,25 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, int ifindex,
48894898
struct sk_security_struct *sksec = sk->sk_security;
48904899
if (selinux_skb_peerlbl_sid(skb, family, &skb_sid))
48914900
return NF_DROP;
4901+
/* At this point, if the returned skb peerlbl is SECSID_NULL
4902+
* and the packet has been through at least one XFRM
4903+
* transformation then we must be dealing with the "final"
4904+
* form of labeled IPsec packet; since we've already applied
4905+
* all of our access controls on this packet we can safely
4906+
* pass the packet. */
4907+
if (skb_sid == SECSID_NULL) {
4908+
switch (family) {
4909+
case PF_INET:
4910+
if (IPCB(skb)->flags & IPSKB_XFRM_TRANSFORMED)
4911+
return NF_ACCEPT;
4912+
break;
4913+
case PF_INET6:
4914+
if (IP6CB(skb)->flags & IP6SKB_XFRM_TRANSFORMED)
4915+
return NF_ACCEPT;
4916+
default:
4917+
return NF_DROP_ERR(-ECONNREFUSED);
4918+
}
4919+
}
48924920
if (selinux_conn_sid(sksec->sid, skb_sid, &peer_sid))
48934921
return NF_DROP;
48944922
secmark_perm = PACKET__SEND;

0 commit comments

Comments
 (0)