From: David Gwynne <david@gwynne.id.au>
Subject: neuter tun(4)/tap(4) ioctls that change interface flags
To: tech@openbsd.org
Date: Wed, 9 Oct 2024 15:34:06 +1000

once upon a time there was just tun(4) and it handled both layer 3 (ipv4
and ipv6) and ethernet. flipping interface type around at runtime was a
recipe for disaster, so we've been progressively locking this down
as time goes by splitting ethernet support out into the tap(4)
interface. tun(4) is set up as a p2p interface with the right interface
flags for p2p, and tap(4) is set up as an ethernet interface with the
right interface flags for ethernet.

we locked down the ability for userland to reconfigure the interface
type of tun/tap interfaces at runtime, but we still allow the flags to
change. this diff removes that ability. it still allows the TUNSIFMODE
and TUNSIFINFO ioctls, but you have to specify the interface flags
appropriate for the interface type.

it also removes the ability to set IFF_UP using these ioctls.

nothing in base uses them, so if there's going to be any fallout it's
from ports. eg, openvpn does TUNGIFINFO, sets IFF_MULTICAST, and then
TUNSIFINFO to apply it. because IFF_MULTICAST is set for both tun and
tap, this is just a waste of cpu time more than anything else, but is
otherwise handled by this diff. other software may be cleverer^Wdumber
though.

thoughts? ok?

Index: if_tun.c
===================================================================
RCS file: /cvs/src/sys/net/if_tun.c,v
diff -u -p -r1.240 if_tun.c
--- if_tun.c	23 Dec 2023 10:52:54 -0000	1.240
+++ if_tun.c	8 Oct 2024 12:09:25 -0000
@@ -101,8 +101,8 @@ int	tundebug = TUN_DEBUG;
 #define TUNDEBUG(a)	/* (tundebug? printf a : 0) */
 #endif
 
-/* Only these IFF flags are changeable by TUNSIFINFO */
-#define TUN_IFF_FLAGS (IFF_UP|IFF_POINTOPOINT|IFF_MULTICAST|IFF_BROADCAST)
+/* Pretend that these IFF flags are changeable by TUNSIFINFO */
+#define TUN_IFF_FLAGS (IFF_POINTOPOINT|IFF_MULTICAST|IFF_BROADCAST)
 
 void	tunattach(int);
 
@@ -709,17 +709,18 @@ tun_dev_ioctl(dev_t dev, u_long cmd, voi
 			error = EINVAL;
 			break;
 		}
+		if (tunp->flags != (sc->sc_if.if_flags & TUN_IFF_FLAGS)) {
+			error = EINVAL;
+			break;
+		}
 		sc->sc_if.if_mtu = tunp->mtu;
-		sc->sc_if.if_flags =
-		    (tunp->flags & TUN_IFF_FLAGS) |
-		    (sc->sc_if.if_flags & ~TUN_IFF_FLAGS);
 		sc->sc_if.if_baudrate = tunp->baudrate;
 		break;
 	case TUNGIFINFO:
 		tunp = (struct tuninfo *)data;
 		tunp->mtu = sc->sc_if.if_mtu;
 		tunp->type = sc->sc_if.if_type;
-		tunp->flags = sc->sc_if.if_flags;
+		tunp->flags = sc->sc_if.if_flags & TUN_IFF_FLAGS;
 		tunp->baudrate = sc->sc_if.if_baudrate;
 		break;
 #ifdef TUN_DEBUG
@@ -731,13 +732,7 @@ tun_dev_ioctl(dev_t dev, u_long cmd, voi
 		break;
 #endif
 	case TUNSIFMODE:
-		switch (*(int *)data & (IFF_POINTOPOINT|IFF_BROADCAST)) {
-		case IFF_POINTOPOINT:
-		case IFF_BROADCAST:
-			sc->sc_if.if_flags &= ~TUN_IFF_FLAGS;
-			sc->sc_if.if_flags |= *(int *)data & TUN_IFF_FLAGS;
-			break;
-		default:
+		if (*(int *)data != (sc->sc_if.if_flags & TUN_IFF_FLAGS)) {
 			error = EINVAL;
 			break;
 		}