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@

diff --git a/usr.sbin/npppd/npppd/npppd_iface.c b/usr.sbin/npppd/npppd/npppd_iface.c
index 3e11a0a..4a746ea 100644 (file)
--- 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 9eddf7d..451a4c6 100644 (file)
--- 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 6f0b65c..9726366 100644 (file)
--- 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 389b503..e226fdf 100644 (file)
--- 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 8ee0272..bcb71b2 100644 (file)
--- 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 */