Currently, npppd's PRIVSEP_OPEN message (abstracted as priv_open())
accepts arbitrary open() flags and passes a mode argument. That
seems...unwise.
In particular, it never passes O_CREAT, so the mode argument isn't needed.
Indeed, the only open 'flags' it needs are O_RDONLY and O_RDWR. If we
also permit O_NONBLOCK then the caller can skip using ioctl(FIONBIO) too.
So:
* kill the 'mode' argument to PRIVSEP_OPEN and priv_open()
* fail a PRIVSEP_OPEN call if it's 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()
I don't use npppd, so I only know this compiles and haven't actually
tested it in any way.
Tests? oks?
Philip Guenther
Index: usr.sbin/npppd/npppd/npppd_iface.c
===================================================================
RCS file: /cvs/src/usr.sbin/npppd/npppd/npppd_iface.c,v
retrieving revision 1.11
diff -u -p -r1.11 npppd_iface.c
--- usr.sbin/npppd/npppd/npppd_iface.c 20 Jul 2015 18:58:30 -0000 1.11
+++ usr.sbin/npppd/npppd/npppd_iface.c 11 Oct 2015 04:17:40 -0000
@@ -283,15 +283,8 @@ 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;
}
Index: usr.sbin/npppd/npppd/npppd_tun.c
===================================================================
RCS file: /cvs/src/usr.sbin/npppd/npppd/npppd_tun.c,v
retrieving revision 1.5
diff -u -p -r1.5 npppd_tun.c
--- usr.sbin/npppd/npppd/npppd_tun.c 19 Jan 2015 01:48:59 -0000 1.5
+++ usr.sbin/npppd/npppd/npppd_tun.c 11 Oct 2015 04:17:40 -0000
@@ -79,7 +79,7 @@ npppd_tundev_init(npppd *_this, int mino
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);
Index: usr.sbin/npppd/npppd/privsep.c
===================================================================
RCS file: /cvs/src/usr.sbin/npppd/npppd/privsep.c,v
retrieving revision 1.17
diff -u -p -r1.17 privsep.c
--- usr.sbin/npppd/npppd/privsep.c 20 Jul 2015 18:55:35 -0000 1.17
+++ usr.sbin/npppd/npppd/privsep.c 11 Oct 2015 04:17:40 -0000
@@ -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 pr
}
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 imsgbu
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_
{ "/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))
Index: usr.sbin/npppd/npppd/privsep.h
===================================================================
RCS file: /cvs/src/usr.sbin/npppd/npppd/privsep.h,v
retrieving revision 1.6
diff -u -p -r1.6 privsep.h
--- usr.sbin/npppd/npppd/privsep.h 12 Jul 2014 14:04:18 -0000 1.6
+++ usr.sbin/npppd/npppd/privsep.h 11 Oct 2015 04:17:40 -0000
@@ -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 **);
Index: usr.sbin/npppd/pppoe/pppoed.c
===================================================================
RCS file: /cvs/src/usr.sbin/npppd/pppoe/pppoed.c,v
retrieving revision 1.17
diff -u -p -r1.17 pppoed.c
--- usr.sbin/npppd/pppoe/pppoed.c 19 Jan 2015 01:48:59 -0000 1.17
+++ usr.sbin/npppd/pppoe/pppoed.c 11 Oct 2015 04:17:41 -0000
@@ -279,7 +279,7 @@ pppoed_listener_start(pppoed_listener *_
/* 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 */
