IP options broken for raw sockets on cred downgrade (was: Re: why required root privileges to set multicast options now?)

[Robert Watson]

On Mon, 11 Oct 2004, Robert Watson wrote:

> > This appears to have been introduced as a result of changes to permit
> > root to bind raw sockets in jail.  In particular, the likely control
> > flow path to get the above errors was to perform setsockopt() on a UDP
> > socket, which probaly works its way down to in_control() to
> > ip_ctloutput().  This would also explain why sdr stopped working for me
> > a little while ago (I figured it was a bad package build).  I've CC'd
> > Christian as he might have some insight into how to clean this up. 
> 
> The bug is now neatly illustrated by the ipsockopt regression test:
...
> The socket option operation works fine except in the case where a raw
> socket was created as root, and then privilege was downgraded to the
> normal user, at which point the process tries a socket option operation
> (apparently of any sort, not just multicast).  I'm surprised more things
> haven't broken, such as aspects of ping(8).  Maybe they have and nobody
> has noticed :-). 

Could you try the attached patch?

Thanks,

Robert N M Watson             FreeBSD Core Team, TrustedBSD Projects
robert at fledge.watson.org      Principal Research Scientist, McAfee Research

Index: raw_ip.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/raw_ip.c,v
retrieving revision 1.144
diff -u -r1.144 raw_ip.c
--- raw_ip.c	5 Sep 2004 02:34:12 -0000	1.144
+++ raw_ip.c	11 Oct 2004 20:08:25 -0000
@@ -326,15 +326,14 @@
 /*
  * Raw IP socket option processing.
  *
- * Note that access to all of the IP administrative functions here is
- * implicitly protected by suser() as gaining access to a raw socket
- * requires either that the thread pass a suser() check, or that it be
- * passed a raw socket by another thread that has passed a suser() check.
- * If FreeBSD moves to a more fine-grained access control mechanism,
- * additional checks will need to be placed here if the raw IP attachment
- * check is not equivilent the the check required for these
- * administrative operations; in some cases, these checks are already
- * present.
+ * NOTE: Regarding access control.  Raw sockets may only be created by
+ * privileged processes; however, as a result of jailed processes and the
+ * ability for processes to downgrade privilege yet retain a reference to the
+ * raw socket.  As such, explicit access control is required here, or when
+ * unimplemented requests are passed to ip_ctloutput(), are required there.
+ *
+ * When adding new socket options here, make sure to add access control if
+ * necessary.
  */
 int
 rip_ctloutput(struct socket *so, struct sockopt *sopt)
@@ -345,18 +344,7 @@
 	if (sopt->sopt_level != IPPROTO_IP)
 		return (EINVAL);
 
-	/*
-	 * Even though super-user is required to create a raw socket, the
-	 * calling cred could be prison root. If so we want to restrict the
-	 * access to IP_HDRINCL only.
-	 */
-	if (sopt->sopt_name != IP_HDRINCL) {
-		error = suser(curthread);
-		if (error != 0)
-			return (error);
-	}
 	error = 0;
-
 	switch (sopt->sopt_dir) {
 	case SOPT_GET:
 		switch (sopt->sopt_name) {
@@ -369,6 +357,9 @@
 		case IP_FW_GET:
 		case IP_FW_TABLE_GETSIZE:
 		case IP_FW_TABLE_LIST:
+			error = suser(curthread);
+			if (error != 0)
+				return (error);
 			if (ip_fw_ctl_ptr != NULL)
 				error = ip_fw_ctl_ptr(sopt);
 			else
@@ -376,6 +367,9 @@
 			break;
 
 		case IP_DUMMYNET_GET:
+			error = suser(curthread);
+			if (error != 0)
+				return (error);
 			if (ip_dn_ctl_ptr != NULL)
 				error = ip_dn_ctl_ptr(sopt);
 			else
@@ -394,6 +388,9 @@
 		case MRT_API_CONFIG:
 		case MRT_ADD_BW_UPCALL:
 		case MRT_DEL_BW_UPCALL:
+			error = suser(curthread);
+			if (error != 0)
+				return (error);
 			error = ip_mrouter_get ? ip_mrouter_get(so, sopt) :
 				EOPNOTSUPP;
 			break;
@@ -425,6 +422,9 @@
 		case IP_FW_TABLE_ADD:
 		case IP_FW_TABLE_DEL:
 		case IP_FW_TABLE_FLUSH:
+			error = suser(curthread);
+			if (error != 0)
+				return (error);
 			if (ip_fw_ctl_ptr != NULL)
 				error = ip_fw_ctl_ptr(sopt);
 			else
@@ -434,6 +434,9 @@
 		case IP_DUMMYNET_CONFIGURE:
 		case IP_DUMMYNET_DEL:
 		case IP_DUMMYNET_FLUSH:
+			error = suser(curthread);
+			if (error != 0)
+				return (error);
 			if (ip_dn_ctl_ptr != NULL)
 				error = ip_dn_ctl_ptr(sopt);
 			else
@@ -441,15 +444,24 @@
 			break ;
 
 		case IP_RSVP_ON:
+			error = suser(curthread);
+			if (error != 0)
+				return (error);
 			error = ip_rsvp_init(so);
 			break;
 
 		case IP_RSVP_OFF:
+			error = suser(curthread);
+			if (error != 0)
+				return (error);
 			error = ip_rsvp_done();
 			break;
 
 		case IP_RSVP_VIF_ON:
 		case IP_RSVP_VIF_OFF:
+			error = suser(curthread);
+			if (error != 0)
+				return (error);
 			error = ip_rsvp_vif ?
 				ip_rsvp_vif(so, sopt) : EINVAL;
 			break;
@@ -466,6 +478,9 @@
 		case MRT_API_CONFIG:
 		case MRT_ADD_BW_UPCALL:
 		case MRT_DEL_BW_UPCALL:
+			error = suser(curthread);
+			if (error != 0)
+				return (error);
 			error = ip_mrouter_set ? ip_mrouter_set(so, sopt) :
 					EOPNOTSUPP;
 			break;