Add "locked lladdr" option to prevent VMs from spoofing MAC addresses.

This is especially useful when multiple VMs share a switch, the
implementation is independent from the underlying switch or bridge.

no objections mlarkin@

9 files changed, 94 insertions, 39 deletions

diff --git a/usr.sbin/vmd/config.c b/usr.sbin/vmd/config.c
index fa5dda14abc..b0abf53b773 100644
--- a/usr.sbin/vmd/config.c
+++ b/usr.sbin/vmd/config.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: config.c,v 1.25 2017/03/01 07:43:33 reyk Exp $	*/
+/*	$OpenBSD: config.c,v 1.26 2017/03/02 07:33:37 reyk Exp $	*/
 
 /*
  * Copyright (c) 2015 Reyk Floeter <reyk@openbsd.org>
@@ -236,7 +236,8 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid)
 		}
 
 		/* Set the interface status */
-		vif->vif_flags = vmc->vmc_ifflags[i] & IFF_UP;
+		vif->vif_flags =
+		    vmc->vmc_ifflags[i] & (VMIFF_UP|VMIFF_OPTMASK);
 	}
 
 	/* Open TTY */
diff --git a/usr.sbin/vmd/parse.y b/usr.sbin/vmd/parse.y
index 52ea73e3505..24b234c8abb 100644
--- a/usr.sbin/vmd/parse.y
+++ b/usr.sbin/vmd/parse.y
@@ -1,4 +1,4 @@
-/*	$OpenBSD: parse.y,v 1.21 2017/03/01 07:43:33 reyk Exp $	*/
+/*	$OpenBSD: parse.y,v 1.22 2017/03/02 07:33:37 reyk Exp $	*/
 
 /*
  * Copyright (c) 2007-2016 Reyk Floeter <reyk@openbsd.org>
@@ -116,10 +116,11 @@ typedef struct {
 
 %token	INCLUDE ERROR
 %token	ADD DISK DOWN GROUP INTERFACE NIFS PATH SIZE SWITCH UP VMID
-%token	ENABLE DISABLE VM KERNEL LLADDR MEMORY OWNER
+%token	ENABLE DISABLE VM KERNEL LLADDR MEMORY OWNER LOCKED
 %token	<v.string>	STRING
 %token  <v.number>	NUMBER
 %type	<v.number>	disable
+%type	<v.number>	locked
 %type	<v.number>	updown
 %type	<v.lladdr>	lladdr
 %type	<v.string>	string
@@ -174,7 +175,7 @@ switch		: SWITCH string			{
 
 			vsw->sw_id = env->vmd_nswitches + 1;
 			vsw->sw_name = $2;
-			vsw->sw_flags = IFF_UP;
+			vsw->sw_flags = VMIFF_UP;
 			snprintf(vsw->sw_ifname, sizeof(vsw->sw_ifname),
 			    "%s%u", vsw_type, vsw_unit++);
 			TAILQ_INIT(&vsw->sw_ifs);
@@ -241,11 +242,14 @@ switch_opts	: disable			{
 			}
 			free($2);
 		}
+		| LOCKED LLADDR			{
+			vsw->sw_flags |= VMIFF_LOCKED;
+		}
 		| updown			{
 			if ($1)
-				vsw->sw_flags |= IFF_UP;
+				vsw->sw_flags |= VMIFF_UP;
 			else
-				vsw->sw_flags &= ~IFF_UP;
+				vsw->sw_flags &= ~VMIFF_UP;
 		}
 		;
 
@@ -503,14 +507,16 @@ iface_opts	: SWITCH string			{
 			    sizeof(vmc.vmc_ifgroup[i]));
 			free($2);
 		}
-		| LLADDR lladdr			{
-			memcpy(vcp->vcp_macs[vcp_nnics], $2, ETHER_ADDR_LEN);
+		| locked LLADDR lladdr		{
+			if ($1)
+				vmc.vmc_ifflags[vcp_nnics] |= VMIFF_LOCKED;
+			memcpy(vcp->vcp_macs[vcp_nnics], $3, ETHER_ADDR_LEN);
 		}
 		| updown			{
 			if ($1)
-				vmc.vmc_ifflags[vcp_nnics] |= IFF_UP;
+				vmc.vmc_ifflags[vcp_nnics] |= VMIFF_UP;
 			else
-				vmc.vmc_ifflags[vcp_nnics] &= ~IFF_UP;
+				vmc.vmc_ifflags[vcp_nnics] &= ~VMIFF_UP;
 		}
 		;
 
@@ -541,6 +547,10 @@ lladdr		: STRING			{
 		}
 		;
 
+locked		: /* empty */			{ $$ = 0; }
+		| LOCKED			{ $$ = 1; }
+		;
+
 updown		: UP				{ $$ = 1; }
 		| DOWN				{ $$ = 0; }
 		;
@@ -606,6 +616,7 @@ lookup(char *s)
 		{ "interfaces",		NIFS },
 		{ "kernel",		KERNEL },
 		{ "lladdr",		LLADDR },
+		{ "locked",		LOCKED },
 		{ "memory",		MEMORY },
 		{ "owner",		OWNER },
 		{ "size",		SIZE },
diff --git a/usr.sbin/vmd/priv.c b/usr.sbin/vmd/priv.c
index 50cba913289..2b99a246259 100644
--- a/usr.sbin/vmd/priv.c
+++ b/usr.sbin/vmd/priv.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: priv.c,v 1.5 2016/10/29 14:56:05 edd Exp $	*/
+/*	$OpenBSD: priv.c,v 1.6 2017/03/02 07:33:37 reyk Exp $	*/
 
 /*
  * Copyright (c) 2016 Reyk Floeter <reyk@openbsd.org>
@@ -295,7 +295,7 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
 		}
 
 		/* Set the new interface status to up or down */
-		proc_compose(ps, PROC_PRIV, (vif->vif_flags & IFF_UP) ?
+		proc_compose(ps, PROC_PRIV, (vif->vif_flags & VMIFF_UP) ?
 		    IMSG_VMDOP_PRIV_IFUP : IMSG_VMDOP_PRIV_IFDOWN,
 		    &vfr, sizeof(vfr));
 	}
@@ -339,7 +339,7 @@ vm_priv_brconfig(struct privsep *ps, struct vmd_switch *vsw)
 	}
 
 	/* Set the new interface status to up or down */
-	proc_compose(ps, PROC_PRIV, (vsw->sw_flags & IFF_UP) ?
+	proc_compose(ps, PROC_PRIV, (vsw->sw_flags & VMIFF_UP) ?
 	    IMSG_VMDOP_PRIV_IFUP : IMSG_VMDOP_PRIV_IFDOWN,
 	    &vfr, sizeof(vfr));
 
diff --git a/usr.sbin/vmd/virtio.c b/usr.sbin/vmd/virtio.c
index b642e64653a..2d4f46fefb8 100644
--- a/usr.sbin/vmd/virtio.c
+++ b/usr.sbin/vmd/virtio.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: virtio.c,v 1.32 2017/02/13 18:49:01 reyk Exp $	*/
+/*	$OpenBSD: virtio.c,v 1.33 2017/03/02 07:33:37 reyk Exp $	*/
 
 /*
  * Copyright (c) 2015 Mike Larkin <mlarkin@openbsd.org>
@@ -897,6 +897,7 @@ vionet_rx(struct vionet_dev *dev)
 {
 	char buf[PAGE_SIZE];
 	int hasdata, num_enq = 0, spc = 0;
+	struct ether_header *eh;
 	ssize_t sz;
 
 	do {
@@ -909,9 +910,14 @@ vionet_rx(struct vionet_dev *dev)
 			if (errno != EAGAIN)
 				log_warn("unexpected read error on vionet "
 				    "device");
-		} else if (sz != 0)
-			num_enq += vionet_enq_rx(dev, buf, sz, &spc);
-		else if (sz == 0) {
+		} else if (sz != 0) {
+			eh = (struct ether_header *)buf;
+			if (!dev->lockedmac || sz < ETHER_HDR_LEN ||
+			    ETHER_IS_MULTICAST(eh->ether_dhost) ||
+			    memcmp(eh->ether_dhost, dev->mac,
+			    sizeof(eh->ether_dhost)) == 0)
+				num_enq += vionet_enq_rx(dev, buf, sz, &spc);
+		} else if (sz == 0) {
 			log_debug("process_rx: no data");
 			hasdata = 0;
 			break;
@@ -1044,6 +1050,7 @@ vionet_notifyq(struct vionet_dev *dev)
 	struct vring_desc *desc, *pkt_desc, *hdr_desc;
 	struct vring_avail *avail;
 	struct vring_used *used;
+	struct ether_header *eh;
 
 	vr = pkt = NULL;
 	ret = 0;
@@ -1152,8 +1159,16 @@ vionet_notifyq(struct vionet_dev *dev)
 			goto out;
 		}
 
+		/* reject other source addresses */
+		if (dev->lockedmac && pktsz >= ETHER_HDR_LEN &&
+		    (eh = (struct ether_header *)pkt) &&
+		    memcmp(eh->ether_shost, dev->mac,
+		    sizeof(eh->ether_shost)) != 0)
+			log_debug("vionet: wrong source address %s for vm %d",
+			    ether_ntoa((struct ether_addr *)
+			    eh->ether_shost), dev->vm_id);
 		/* XXX signed vs unsigned here, funky cast */
-		if (write(dev->fd, pkt, pktsz) != (int)pktsz) {
+		else if (write(dev->fd, pkt, pktsz) != (int)pktsz) {
 			log_warnx("vionet: tx failed writing to tap: "
 			    "%d", errno);
 			goto out;
@@ -1298,8 +1313,9 @@ vmmci_io(int dir, uint16_t reg, uint32_t *data, uint8_t *intr,
 }
 
 void
-virtio_init(struct vm_create_params *vcp, int *child_disks, int *child_taps)
+virtio_init(struct vmop_create_params *vmc, int *child_disks, int *child_taps)
 {
+	struct vm_create_params *vcp = &vmc->vmc_params;
 	static const uint8_t zero_mac[6];
 	uint8_t id;
 	uint8_t i;
@@ -1458,10 +1474,13 @@ virtio_init(struct vm_create_params *vcp, int *child_disks, int *child_taps)
 				vionet[i].mac[4] = rng;
 				vionet[i].mac[5] = rng >> 8;
 			}
+			vionet[i].lockedmac =
+			    vmc->vmc_ifflags[i] & VMIFF_LOCKED ? 1 : 0;
 
-			log_debug("%s: vm \"%s\" vio%u lladdr %s",
+			log_debug("%s: vm \"%s\" vio%u lladdr %s%s",
 			    __func__, vcp->vcp_name, i,
-			    ether_ntoa((void *)vionet[i].mac));
+			    ether_ntoa((void *)vionet[i].mac),
+			    vionet[i].lockedmac ? " (locked)" : "");
 		}
 	}
 
diff --git a/usr.sbin/vmd/virtio.h b/usr.sbin/vmd/virtio.h
index 862c287f388..291d5deeb69 100644
--- a/usr.sbin/vmd/virtio.h
+++ b/usr.sbin/vmd/virtio.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: virtio.h,v 1.9 2017/01/21 12:45:41 mlarkin Exp $	*/
+/*	$OpenBSD: virtio.h,v 1.10 2017/03/02 07:33:37 reyk Exp $	*/
 
 /*
  * Copyright (c) 2015 Mike Larkin <mlarkin@openbsd.org>
@@ -118,6 +118,7 @@ struct vionet_dev {
 	uint32_t vm_id;
 	int irq;
 	uint8_t mac[6];
+	int lockedmac;
 };
 
 struct virtio_net_hdr {
@@ -150,7 +151,7 @@ struct vmmci_dev {
 	int irq;
 };
 
-void virtio_init(struct vm_create_params *, int *, int *);
+void virtio_init(struct vmop_create_params *, int *, int *);
 uint32_t vring_size(uint32_t);
 
 int virtio_rnd_io(int, uint16_t, uint32_t *, uint8_t *, void *);
diff --git a/usr.sbin/vmd/vm.c b/usr.sbin/vmd/vm.c
index 76d213e8adf..f6fed52466b 100644
--- a/usr.sbin/vmd/vm.c
+++ b/usr.sbin/vmd/vm.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vm.c,v 1.1 2017/03/01 18:00:50 reyk Exp $	*/
+/*	$OpenBSD: vm.c,v 1.2 2017/03/02 07:33:37 reyk Exp $	*/
 
 /*
  * Copyright (c) 2015 Mike Larkin <mlarkin@openbsd.org>
@@ -63,7 +63,7 @@
 
 io_fn_t ioports_map[MAX_PORTS];
 
-int run_vm(int *, int *, struct vm_create_params *, struct vcpu_reg_state *);
+int run_vm(int *, int *, struct vmop_create_params *, struct vcpu_reg_state *);
 void vm_dispatch_vmm(int, short, void *);
 void *event_thread(void *);
 void *vcpu_run_loop(void *);
@@ -72,7 +72,7 @@ int vcpu_reset(uint32_t, uint32_t, struct vcpu_reg_state *);
 void create_memory_map(struct vm_create_params *);
 int alloc_guest_mem(struct vm_create_params *);
 int vmm_create_vm(struct vm_create_params *);
-void init_emulated_hw(struct vm_create_params *, int *, int *);
+void init_emulated_hw(struct vmop_create_params *, int *, int *);
 void vcpu_exit_inout(struct vm_run_params *);
 uint8_t vcpu_exit_pci(struct vm_run_params *);
 int vcpu_pic_intr(uint32_t, uint32_t, uint8_t);
@@ -236,7 +236,7 @@ start_vm(struct vmd_vm *vm, int fd)
 		fatal("setup vm pipe");
 
 	/* Execute the vcpu run loop(s) for this VM */
-	ret = run_vm(vm->vm_disks, nicfds, vcp, &vrs);
+	ret = run_vm(vm->vm_disks, nicfds, &vm->vm_params, &vrs);
 
 	return (ret);
 }
@@ -493,9 +493,10 @@ vmm_create_vm(struct vm_create_params *vcp)
  * Initializes the userspace hardware emulation
  */
 void
-init_emulated_hw(struct vm_create_params *vcp, int *child_disks,
+init_emulated_hw(struct vmop_create_params *vmc, int *child_disks,
     int *child_taps)
 {
+	struct vm_create_params *vcp = &vmc->vmc_params;
 	int i;
 
 	/* Reset the IO port map */
@@ -534,7 +535,7 @@ init_emulated_hw(struct vm_create_params *vcp, int *child_disks,
 	pci_init();
 
 	/* Initialize virtio devices */
-	virtio_init(vcp, child_disks, child_taps);
+	virtio_init(vmc, child_disks, child_taps);
 }
 
 /*
@@ -545,7 +546,7 @@ init_emulated_hw(struct vm_create_params *vcp, int *child_disks,
  * Parameters:
  *  child_disks: previously-opened child VM disk file file descriptors
  *  child_taps: previously-opened child tap file descriptors
- *  vcp: vm_create_params struct containing the VM's desired creation
+ *  vmc: vmop_create_params struct containing the VM's desired creation
  *      configuration
  *  vrs: VCPU register state to initialize
  *
@@ -554,9 +555,10 @@ init_emulated_hw(struct vm_create_params *vcp, int *child_disks,
  *  !0 : the VM exited abnormally or failed to start
  */
 int
-run_vm(int *child_disks, int *child_taps, struct vm_create_params *vcp,
+run_vm(int *child_disks, int *child_taps, struct vmop_create_params *vmc,
     struct vcpu_reg_state *vrs)
 {
+	struct vm_create_params *vcp = &vmc->vmc_params;
 	uint8_t evdone = 0;
 	size_t i;
 	int ret;
@@ -597,7 +599,7 @@ run_vm(int *child_disks, int *child_taps, struct vm_create_params *vcp,
 	log_debug("%s: initializing hardware for vm %s", __func__,
 	    vcp->vcp_name);
 
-	init_emulated_hw(vcp, child_disks, child_taps);
+	init_emulated_hw(vmc, child_disks, child_taps);
 
 	ret = pthread_mutex_init(&threadmutex, NULL);
 	if (ret) {
diff --git a/usr.sbin/vmd/vm.conf.5 b/usr.sbin/vmd/vm.conf.5
index 2ed456df4a4..e58a4bd604a 100644
--- a/usr.sbin/vmd/vm.conf.5
+++ b/usr.sbin/vmd/vm.conf.5
@@ -1,4 +1,4 @@
-.\" $OpenBSD: vm.conf.5,v 1.15 2017/03/01 12:29:06 jmc Exp $
+.\" $OpenBSD: vm.conf.5,v 1.16 2017/03/02 07:33:37 reyk Exp $
 .\"
 .\" Copyright (c) 2015 Mike Larkin <mlarkin@openbsd.org>
 .\" Copyright (c) 2015 Reyk Floeter <reyk@openbsd.org>
@@ -15,7 +15,7 @@
 .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 .\"
-.Dd $Mdocdate: March 1 2017 $
+.Dd $Mdocdate: March 2 2017 $
 .Dt VM.CONF 5
 .Os
 .Sh NAME
@@ -130,11 +130,16 @@ rules for several VM interfaces in the same group.
 The
 .Ar group-name
 must not end with a digit.
-.It Cm lladdr Ar etheraddr
+.It Oo Cm locked Oc Cm lladdr Op Ar etheraddr
 Change the link layer address (MAC address) of the interface on the
 VM guest side.
 If not specified, a randomized address will be assigned by
 .Xr vmd 8 .
+If the
+.Cm locked
+keyword is specified,
+.Xr vmd 8
+will drop packets from the VM with altered source addresses.
 .It Cm switch Ar name
 Set the virtual switch
 by
@@ -221,6 +226,11 @@ This is the default if neither
 nor
 .Cm disable
 is specified.
+.It Cm locked lladdr
+If this option is specified,
+.Xr vmd 8
+will drop packets with altered sources addresses that do not match the
+link layer addresses (MAC addresses) of the VM interfaces in this switch.
 .It Cm disable
 Do not configure this switch.
 .It Cm group Ar group-name
diff --git a/usr.sbin/vmd/vmd.c b/usr.sbin/vmd/vmd.c
index 1c70bb4b6e9..36b17c8a5b9 100644
--- a/usr.sbin/vmd/vmd.c
+++ b/usr.sbin/vmd/vmd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vmd.c,v 1.52 2017/03/01 07:43:33 reyk Exp $	*/
+/*	$OpenBSD: vmd.c,v 1.53 2017/03/02 07:33:37 reyk Exp $	*/
 
 /*
  * Copyright (c) 2015 Reyk Floeter <reyk@openbsd.org>
@@ -753,6 +753,7 @@ vm_register(struct privsep *ps, struct vmop_create_params *vmc,
 	struct vmd_vm		*vm = NULL;
 	struct vm_create_params	*vcp = &vmc->vmc_params;
 	unsigned int		 i;
+	struct vmd_switch	*sw;
 
 	errno = 0;
 	*ret_vm = NULL;
@@ -801,13 +802,20 @@ vm_register(struct privsep *ps, struct vmop_create_params *vmc,
 		goto fail;
 
 	memcpy(&vm->vm_params, vmc, sizeof(vm->vm_params));
+	vmc = &vm->vm_params;
 	vm->vm_pid = -1;
 	vm->vm_tty = -1;
 
 	for (i = 0; i < vcp->vcp_ndisks; i++)
 		vm->vm_disks[i] = -1;
-	for (i = 0; i < vcp->vcp_nnics; i++)
+	for (i = 0; i < vcp->vcp_nnics; i++) {
 		vm->vm_ifs[i].vif_fd = -1;
+
+		if ((sw = switch_getbyname(vmc->vmc_ifswitch[i])) != NULL) {
+			/* inherit per-interface flags from the switch */
+			vmc->vmc_ifflags[i] |= (sw->sw_flags & VMIFF_OPTMASK);
+		}
+	}
 	vm->vm_kernel = -1;
 	vm->vm_iev.ibuf.fd = -1;
 
diff --git a/usr.sbin/vmd/vmd.h b/usr.sbin/vmd/vmd.h
index 3046bb36c2b..2afc04f997b 100644
--- a/usr.sbin/vmd/vmd.h
+++ b/usr.sbin/vmd/vmd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vmd.h,v 1.46 2017/03/01 18:00:50 reyk Exp $	*/
+/*	$OpenBSD: vmd.h,v 1.47 2017/03/02 07:33:37 reyk Exp $	*/
 
 /*
  * Copyright (c) 2015 Mike Larkin <mlarkin@openbsd.org>
@@ -113,6 +113,9 @@ struct vmop_create_params {
 
 	/* userland-only part of the create params */
 	unsigned int		 vmc_ifflags[VMM_MAX_NICS_PER_VM];
+#define VMIFF_UP		0x01
+#define VMIFF_LOCKED		0x02
+#define VMIFF_OPTMASK		VMIFF_LOCKED
 	char			 vmc_ifnames[VMM_MAX_NICS_PER_VM][IF_NAMESIZE];
 	char			 vmc_ifswitch[VMM_MAX_NICS_PER_VM][VM_NAME_MAX];
 	char			 vmc_ifgroup[VMM_MAX_NICS_PER_VM][IF_NAMESIZE];