Commit 47180068 authored by Paul Moore's avatar Paul Moore
Browse files

selinux: handle TCP SYN-ACK packets correctly in selinux_ip_output() 



In selinux_ip_output() we always label packets based on the parent
socket.  While this approach works in almost all cases, it doesn't
work in the case of TCP SYN-ACK packets when the correct label is not
the label of the parent socket, but rather the label of the larval
socket represented by the request_sock struct.

Unfortunately, since the request_sock isn't queued on the parent
socket until *after* the SYN-ACK packet is sent, we can't lookup the
request_sock to determine the correct label for the packet; at this
point in time the best we can do is simply pass/NF_ACCEPT the packet.
It must be said that simply passing the packet without any explicit
labeling action, while far from ideal, is not terrible as the SYN-ACK
packet will inherit any IP option based labeling from the initial
connection request so the label *should* be correct and all our
access controls remain in place so we shouldn't have to worry about
information leaks.

Reported-by: default avatarJanak Desai <Janak.Desai@gtri.gatech.edu>
Tested-by: default avatarJanak Desai <Janak.Desai@gtri.gatech.edu>
Cc: stable@vger.kernel.org
Signed-off-by: default avatarPaul Moore <pmoore@redhat.com>
parent 0af90164

security/selinux/hooks.c

+23 −2
Original line number Diff line number Diff line
@@ -53,6 +53,7 @@ 
#include <net/ip.h>		/* for local_port_range[] */

#include <net/sock.h>

#include <net/tcp.h>		/* struct or_callable used in sock_rcv_skb */

#include <net/inet_connection_sock.h>

#include <net/net_namespace.h>

#include <net/netlabel.h>

#include <linux/uaccess.h>
@@ -4731,6 +4732,7 @@ static unsigned int selinux_ipv6_forward(unsigned int hooknum, 
static unsigned int selinux_ip_output(struct sk_buff *skb,

				      u16 family)

{

	struct sock *sk;

	u32 sid;



	if (!netlbl_enabled())
@@ -4739,8 +4741,27 @@ static unsigned int selinux_ip_output(struct sk_buff *skb, 
	/* we do this in the LOCAL_OUT path and not the POST_ROUTING path

	 * because we want to make sure we apply the necessary labeling

	 * before IPsec is applied so we can leverage AH protection */

	if (skb->sk) {

		struct sk_security_struct *sksec = skb->sk->sk_security;

	sk = skb->sk;

	if (sk) {

		struct sk_security_struct *sksec;



		if (sk->sk_state == TCP_LISTEN)

			/* if the socket is the listening state then this

			 * packet is a SYN-ACK packet which means it needs to

			 * be labeled based on the connection/request_sock and

			 * not the parent socket.  unfortunately, we can't

			 * lookup the request_sock yet as it isn't queued on

			 * the parent socket until after the SYN-ACK is sent.

			 * the "solution" is to simply pass the packet as-is

			 * as any IP option based labeling should be copied

			 * from the initial connection request (in the IP

			 * layer).  it is far from ideal, but until we get a

			 * security label in the packet itself this is the

			 * best we can do. */

			return NF_ACCEPT;



		/* standard practice, label using the parent socket */

		sksec = sk->sk_security;

		sid = sksec->sid;

	} else

		sid = SECINITSID_KERNEL;

CRA Git | Maintained and supported by SUSTech CRA and CCSE