Simplify and lock down priv_open():

 * kill the 'mode' argument
 * fail if passed any flags other than O_ACCMODE OR O_NONBLOCK
 * paranoia: mask O_CREAT when calling open() with only two arguments
 * instead of using ioctl(FIONBIO) after the fact, pass O_NONBLOCK to
   priv_open()

"good start" deraadt@
ok yasuoka@

5 files changed, 18 insertions, 30 deletions

diff --git a/usr.sbin/npppd/npppd/npppd_iface.c b/usr.sbin/npppd/npppd/npppd_iface.c
index 3e11a0a59ef..4a746ea55d7 100644
--- a/usr.sbin/npppd/npppd/npppd_iface.c
+++ b/usr.sbin/npppd/npppd/npppd_iface.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: npppd_iface.c,v 1.11 2015/07/20 18:58:30 yasuoka Exp $ */
+/*	$OpenBSD: npppd_iface.c,v 1.12 2015/10/11 07:32:06 guenther Exp $ */
 
 /*-
  * Copyright (c) 2009 Internet Initiative Japan Inc.
@@ -25,7 +25,7 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  */
-/* $Id: npppd_iface.c,v 1.11 2015/07/20 18:58:30 yasuoka Exp $ */
+/* $Id: npppd_iface.c,v 1.12 2015/10/11 07:32:06 guenther Exp $ */
 /**@file
  * The interface of npppd and kernel.
  * This is an implementation to use tun(4) or pppx(4).
@@ -283,18 +283,11 @@ npppd_iface_start(npppd_iface *_this)
 
 	/* open device file */
 	snprintf(buf, sizeof(buf), "/dev/%s", _this->ifname);
-	if ((_this->devf = priv_open(buf, O_RDWR, 0600)) < 0) {
+	if ((_this->devf = priv_open(buf, O_RDWR | O_NONBLOCK)) < 0) {
 		npppd_iface_log(_this, LOG_ERR, "open(%s) failed: %m", buf);
 		goto fail;
 	}
 
-	x = 1;
-	if (ioctl(_this->devf, FIONBIO, &x) != 0) {
-		npppd_iface_log(_this, LOG_ERR,
-		    "ioctl(FIONBIO) failed in %s(): %m", __func__);
-		goto fail;
-	}
-
 	if (_this->using_pppx == 0) {
 		x = IFF_BROADCAST;
 		if (ioctl(_this->devf, TUNSIFMODE, &x) != 0) {
diff --git a/usr.sbin/npppd/npppd/npppd_tun.c b/usr.sbin/npppd/npppd/npppd_tun.c
index 9eddf7df632..451a4c61f2f 100644
--- a/usr.sbin/npppd/npppd/npppd_tun.c
+++ b/usr.sbin/npppd/npppd/npppd_tun.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: npppd_tun.c,v 1.5 2015/01/19 01:48:59 deraadt Exp $ */
+/*	$OpenBSD: npppd_tun.c,v 1.6 2015/10/11 07:32:06 guenther Exp $ */
 
 /*-
  * Copyright (c) 2009 Internet Initiative Japan Inc.
@@ -79,7 +79,7 @@ npppd_tundev_init(npppd *_this, int minor)
 int
 npppd_tundev_start(npppd *_this)
 {
-	int x, sock;
+	int sock;
 	char buf[PATH_MAX];
 	struct ifaliasreq ifra;
 	struct sockaddr_in *sin0;
@@ -87,7 +87,7 @@ npppd_tundev_start(npppd *_this)
 	NPPPD_TUN_ASSERT(_this != NULL);
 
 	snprintf(buf, sizeof(buf), "/dev/tun%d", _this->tun_minor);
-	if ((_this->tun_file = open(buf, O_RDWR, 0600)) < 0) {
+	if ((_this->tun_file = open(buf, O_RDWR | O_NONBLOCK)) < 0) {
 		log_printf(LOG_ERR, "open(%s) failed in %s(): %m",
 		    buf, __func__);
 		goto fail;
@@ -123,12 +123,6 @@ npppd_tundev_start(npppd *_this)
 	}
 	close(sock);
 
-	x = 1;
-	if (ioctl(_this->tun_file, FIONBIO, &x) != 0) {
-		log_printf(LOG_ERR, "ioctl(FIONBIO) failed in %s(): %m",
-		    __func__);
-		goto fail;
-	}
 	event_set(&_this->ev_tun, _this->tun_file, EV_READ | EV_PERSIST,
 	    npppd_tundev_io_event_handler, _this);
 	event_add(&_this->ev_tun, NULL);
diff --git a/usr.sbin/npppd/npppd/privsep.c b/usr.sbin/npppd/npppd/privsep.c
index 6f0b65c3371..9726366c43f 100644
--- a/usr.sbin/npppd/npppd/privsep.c
+++ b/usr.sbin/npppd/npppd/privsep.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: privsep.c,v 1.17 2015/07/20 18:55:35 yasuoka Exp $ */
+/*	$OpenBSD: privsep.c,v 1.18 2015/10/11 07:32:06 guenther Exp $ */
 
 /*
  * Copyright (c) 2010 Yasuoka Masahiko <yasuoka@openbsd.org>
@@ -60,7 +60,6 @@ enum imsg_code {
 struct PRIVSEP_OPEN_ARG {
 	char			 path[PATH_MAX];
 	int			 flags;
-	mode_t			 mode;
 };
 
 struct PRIVSEP_SOCKET_ARG {
@@ -263,13 +262,12 @@ priv_socket(int domain, int type, int protocol)
 }
 
 int
-priv_open(const char *path, int flags, mode_t mode)
+priv_open(const char *path, int flags)
 {
 	struct PRIVSEP_OPEN_ARG a;
 
 	strlcpy(a.path, path, sizeof(a.path));
 	a.flags = flags;
-	a.mode = mode;
 	(void)imsg_compose(&privsep_ibuf, PRIVSEP_OPEN, 0, 0, -1,
 	    &a, sizeof(a));
 	imsg_flush(&privsep_ibuf);
@@ -283,7 +281,7 @@ priv_fopen(const char *path)
 	int f;
 	FILE *fp;
 
-	if ((f = priv_open(path, O_RDONLY, 0600)) < 0)
+	if ((f = priv_open(path, O_RDONLY)) < 0)
 		return (NULL);
 
 	if ((fp = fdopen(f, "r")) == NULL) {
@@ -596,7 +594,7 @@ privsep_priv_dispatch_imsg(struct imsgbuf *ibuf)
 			else if (privsep_npppd_check_open(a))
 				r.rerrno = EACCES;
 			else {
-				if ((f = open(a->path, a->flags, a->mode))
+				if ((f = open(a->path, a->flags & ~O_CREAT))
 				    == -1)
 					r.rerrno = errno;
 				else
@@ -994,6 +992,9 @@ privsep_npppd_check_open(struct PRIVSEP_OPEN_ARG *arg)
 		{ "/dev/pppx",		1,	0 }
 	};
 
+	/* O_NONBLOCK is the only 'extra' flag permitted */
+	if (arg->flags & ~(O_ACCMODE | O_NONBLOCK))
+		return (1);
 	for (i = 0; i < (int)nitems(allow_paths); i++) {
 		if (allow_paths[i].path_is_prefix) {
 			if (!startswith(arg->path, allow_paths[i].path))
diff --git a/usr.sbin/npppd/npppd/privsep.h b/usr.sbin/npppd/npppd/privsep.h
index 389b5039939..e226fdf1ad8 100644
--- a/usr.sbin/npppd/npppd/privsep.h
+++ b/usr.sbin/npppd/npppd/privsep.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: privsep.h,v 1.6 2014/07/12 14:04:18 yasuoka Exp $ */
+/*	$OpenBSD: privsep.h,v 1.7 2015/10/11 07:32:06 guenther Exp $ */
 
 /*
  * Copyright (c) 2010 Yasuoka Masahiko <yasuoka@openbsd.org>
@@ -36,7 +36,7 @@ FILE  *priv_fopen (const char *);
 int   priv_bind (int, const struct sockaddr *, socklen_t);
 int   priv_unlink (const char *);
 int   priv_socket (int, int, int);
-int   priv_open (const char *, int, mode_t);
+int   priv_open (const char *, int);
 int   priv_send (int, const void *, int, int);
 int   priv_sendto (int, const void *, int, int, const struct sockaddr *, socklen_t);
 int   priv_get_user_info(const char *, const char *, npppd_auth_user **);
diff --git a/usr.sbin/npppd/pppoe/pppoed.c b/usr.sbin/npppd/pppoe/pppoed.c
index 8ee027209c6..bcb71b2f7e8 100644
--- a/usr.sbin/npppd/pppoe/pppoed.c
+++ b/usr.sbin/npppd/pppoe/pppoed.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pppoed.c,v 1.17 2015/01/19 01:48:59 deraadt Exp $	*/
+/*	$OpenBSD: pppoed.c,v 1.18 2015/10/11 07:32:06 guenther Exp $	*/
 
 /*-
  * Copyright (c) 2009 Internet Initiative Japan Inc.
@@ -28,7 +28,7 @@
 /**@file
  * This file provides the PPPoE(RFC2516) server(access concentrator)
  * implementaion.
- * $Id: pppoed.c,v 1.17 2015/01/19 01:48:59 deraadt Exp $
+ * $Id: pppoed.c,v 1.18 2015/10/11 07:32:06 guenther Exp $
  */
 #include <sys/param.h>	/* ALIGN */
 #include <sys/types.h>
@@ -279,7 +279,7 @@ pppoed_listener_start(pppoed_listener *_this, int restart)
 	/* FIXME: /dev/bpf of NetBSD3.0 can simultaneity open */
 	for (i = 0; i < 256; i++) {
 		snprintf(buf, sizeof(buf), "/dev/bpf%d", i);
-		if ((_this->bpf = priv_open(buf, O_RDWR, 0600)) >= 0) {
+		if ((_this->bpf = priv_open(buf, O_RDWR)) >= 0) {
 			break;
 		} else if (errno == ENXIO || errno == ENOENT)
 			break;	/* no more entries */