From 116a5cae52969a2c9d78d06ad3d89b6396ab42f7 Mon Sep 17 00:00:00 2001 From: guenther Date: Sun, 11 Oct 2015 07:32:06 +0000 Subject: [PATCH] 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@ --- usr.sbin/npppd/npppd/npppd_iface.c | 13 +++---------- usr.sbin/npppd/npppd/npppd_tun.c | 12 +++--------- usr.sbin/npppd/npppd/privsep.c | 13 +++++++------ usr.sbin/npppd/npppd/privsep.h | 4 ++-- usr.sbin/npppd/pppoe/pppoed.c | 6 +++--- 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 @@ -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 @@ -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 /* ALIGN */ #include @@ -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 */ -- 2.20.1