On Wed, Mar 01, 2017 at 01:04:57PM +0100, Reyk Floeter wrote:
> Hi,
>
> add the new "fixed lladdr" option: when multiple VMs are connected to
> a switch, it is desirable that an individual VM cannot spoof another
> MAC address, especially when using meta-data*. vmd(8) can enforce
> this by comparing the address in the Ethernet header with the
> configured/generated address of the VM interface.
>
> This somewhat resembles the following features from VMware:
> ethernet0.noforgedsrcaddr = "TRUE"
> ethernet0.nopromisc = "TRUE"
>
> The important parts of the diff** are in the two if statements including
> "dev->fixedmac" below, the rest is infrastructure, config, and
> documentation.
>
> I could have used bridge(4) rules or switch(4) OpenFlow actions, but I
> decided to implement it in vmd(8) directly to make it easier and to
> work in all cases independent from the switch type.
>
> *) https://github.com/reyk/meta-data
> **) this diff conflicts with the previous vmm.c split. Whatever goes
> in first, I can update and resend it accordingly.
>
> OK?
>
> Reyk
>
> Add "fixed 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.
>
After some discussions, this is the updated diff:
- change "fixed" to "locked", adjust manpage
- sync with latest vmm.c split
Reyk
Index: usr.sbin/vmd/config.c
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/config.c,v
retrieving revision 1.25
diff -u -p -u -p -r1.25 config.c
--- usr.sbin/vmd/config.c 1 Mar 2017 07:43:33 -0000 1.25
+++ usr.sbin/vmd/config.c 1 Mar 2017 18:16:20 -0000
@@ -236,7 +236,8 @@ config_setvm(struct privsep *ps, struct
}
/* 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 */
Index: usr.sbin/vmd/parse.y
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/parse.y,v
retrieving revision 1.21
diff -u -p -u -p -r1.21 parse.y
--- usr.sbin/vmd/parse.y 1 Mar 2017 07:43:33 -0000 1.21
+++ usr.sbin/vmd/parse.y 1 Mar 2017 18:16:20 -0000
@@ -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 },
Index: usr.sbin/vmd/priv.c
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/priv.c,v
retrieving revision 1.5
diff -u -p -u -p -r1.5 priv.c
--- usr.sbin/vmd/priv.c 29 Oct 2016 14:56:05 -0000 1.5
+++ usr.sbin/vmd/priv.c 1 Mar 2017 18:16:20 -0000
@@ -295,7 +295,7 @@ vm_priv_ifconfig(struct privsep *ps, str
}
/* 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, str
}
/* 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));
Index: usr.sbin/vmd/virtio.c
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/virtio.c,v
retrieving revision 1.32
diff -u -p -u -p -r1.32 virtio.c
--- usr.sbin/vmd/virtio.c 13 Feb 2017 18:49:01 -0000 1.32
+++ usr.sbin/vmd/virtio.c 1 Mar 2017 18:16:20 -0000
@@ -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
}
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
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)" : "");
}
}
Index: usr.sbin/vmd/virtio.h
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/virtio.h,v
retrieving revision 1.9
diff -u -p -u -p -r1.9 virtio.h
--- usr.sbin/vmd/virtio.h 21 Jan 2017 12:45:41 -0000 1.9
+++ usr.sbin/vmd/virtio.h 1 Mar 2017 18:16:21 -0000
@@ -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 *);
Index: usr.sbin/vmd/vm.c
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/vm.c,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 vm.c
--- usr.sbin/vmd/vm.c 1 Mar 2017 18:00:50 -0000 1.1
+++ usr.sbin/vmd/vm.c 1 Mar 2017 18:16:21 -0000
@@ -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, struc
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 *v
* 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
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
* 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
* !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
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) {
Index: usr.sbin/vmd/vm.conf.5
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/vm.conf.5,v
retrieving revision 1.15
diff -u -p -u -p -r1.15 vm.conf.5
--- usr.sbin/vmd/vm.conf.5 1 Mar 2017 12:29:06 -0000 1.15
+++ usr.sbin/vmd/vm.conf.5 1 Mar 2017 18:16:21 -0000
@@ -130,11 +130,16 @@ rules for several VM interfaces in the s
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
Index: usr.sbin/vmd/vmd.c
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v
retrieving revision 1.52
diff -u -p -u -p -r1.52 vmd.c
--- usr.sbin/vmd/vmd.c 1 Mar 2017 07:43:33 -0000 1.52
+++ usr.sbin/vmd/vmd.c 1 Mar 2017 18:16:21 -0000
@@ -753,6 +753,7 @@ vm_register(struct privsep *ps, struct v
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 v
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;
Index: usr.sbin/vmd/vmd.h
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/vmd.h,v
retrieving revision 1.46
diff -u -p -u -p -r1.46 vmd.h
--- usr.sbin/vmd/vmd.h 1 Mar 2017 18:00:50 -0000 1.46
+++ usr.sbin/vmd/vmd.h 1 Mar 2017 18:16:21 -0000
@@ -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];
