From owner-freebsd-net@FreeBSD.ORG Mon Oct 11 20:33:32 2004 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 76DE716A4CE; Mon, 11 Oct 2004 20:33:32 +0000 (GMT) Received: from fledge.watson.org (fledge.watson.org [204.156.12.50]) by mx1.FreeBSD.org (Postfix) with ESMTP id E00FE43D49; Mon, 11 Oct 2004 20:33:31 +0000 (GMT) (envelope-from robert@fledge.watson.org) Received: from fledge.watson.org (localhost [127.0.0.1]) by fledge.watson.org (8.13.1/8.13.1) with ESMTP id i9BKVwRW054823; Mon, 11 Oct 2004 16:31:58 -0400 (EDT) (envelope-from robert@fledge.watson.org) Received: from localhost (robert@localhost)i9BKVwDQ054820; Mon, 11 Oct 2004 16:31:58 -0400 (EDT) (envelope-from robert@fledge.watson.org) Date: Mon, 11 Oct 2004 16:31:57 -0400 (EDT) From: Robert Watson X-Sender: robert@fledge.watson.org To: swp@swp.pp.ru In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII cc: freebsd-net@freebsd.org cc: csjp@freebsd.org Subject: Re: IP options broken for raw sockets on cred downgrade (was: Re: why required root privileges to set multicast options now?) X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 11 Oct 2004 20:33:32 -0000 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@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;