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.
diff --git usr.sbin/vmd/config.c usr.sbin/vmd/config.c
index fa5dda1..7863ce1 100644
--- usr.sbin/vmd/config.c
+++ usr.sbin/vmd/config.c
@@ -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 usr.sbin/vmd/parse.y usr.sbin/vmd/parse.y
index 52ea73e..6f35368 100644
--- usr.sbin/vmd/parse.y
+++ usr.sbin/vmd/parse.y
@@ -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 FIXED
%token <v.string> STRING
%token <v.number> NUMBER
%type <v.number> disable
+%type <v.number> fixed
%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);
}
+ | FIXED LLADDR {
+ vsw->sw_flags |= VMIFF_FIXED;
+ }
| 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);
+ | fixed LLADDR lladdr {
+ if ($1)
+ vmc.vmc_ifflags[vcp_nnics] |= VMIFF_FIXED;
+ 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 {
}
;
+fixed : /* empty */ { $$ = 0; }
+ | FIXED { $$ = 1; }
+ ;
+
updown : UP { $$ = 1; }
| DOWN { $$ = 0; }
;
@@ -599,6 +609,7 @@ lookup(char *s)
{ "disk", DISK },
{ "down", DOWN },
{ "enable", ENABLE },
+ { "fixed", FIXED },
{ "group", GROUP },
{ "id", VMID },
{ "include", INCLUDE },
diff --git usr.sbin/vmd/priv.c usr.sbin/vmd/priv.c
index 50cba91..e3ee2c4 100644
--- usr.sbin/vmd/priv.c
+++ usr.sbin/vmd/priv.c
@@ -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 usr.sbin/vmd/virtio.c usr.sbin/vmd/virtio.c
index b642e64..d36dbf1 100644
--- usr.sbin/vmd/virtio.c
+++ usr.sbin/vmd/virtio.c
@@ -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->fixedmac || 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->fixedmac && pktsz >= ETHER_HDR_LEN &&
+ (eh = (struct ether_header *)pkt) &&
+ memcmp(eh->ether_shost, dev->mac,
+ sizeof(eh->ether_shost)) != 0)
+ log_warnx("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].fixedmac =
+ vmc->vmc_ifflags[i] & VMIFF_FIXED ? 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].fixedmac ? " (fixed)" : "");
}
}
diff --git usr.sbin/vmd/virtio.h usr.sbin/vmd/virtio.h
index 862c287..d5831d2 100644
--- usr.sbin/vmd/virtio.h
+++ usr.sbin/vmd/virtio.h
@@ -118,6 +118,7 @@ struct vionet_dev {
uint32_t vm_id;
int irq;
uint8_t mac[6];
+ int fixedmac;
};
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 usr.sbin/vmd/vm.conf.5 usr.sbin/vmd/vm.conf.5
index 3f81469..b7f87ed 100644
--- usr.sbin/vmd/vm.conf.5
+++ usr.sbin/vmd/vm.conf.5
@@ -130,11 +130,14 @@ 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 fixed 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 .
+The guest will not be permitted to change the assigned address if the
+.Cm fixed
+keyword is specified.
.It Cm switch Ar name
Set the virtual switch
by
@@ -222,6 +225,10 @@ This is the default if neither
nor
.Cm disable
is specified.
+.It Cm fixed lladdr
+If this option is specified, the VM guests will not be permitted to
+change the assigned link layer addresses (MAC addresses) of the
+interfaces in this switch.
.It Cm disable
Do not configure this switch.
.It Cm group Ar group-name
diff --git usr.sbin/vmd/vmd.c usr.sbin/vmd/vmd.c
index 1c70bb4..c02a685 100644
--- usr.sbin/vmd/vmd.c
+++ usr.sbin/vmd/vmd.c
@@ -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 usr.sbin/vmd/vmd.h usr.sbin/vmd/vmd.h
index 34f8f21..1692d99 100644
--- usr.sbin/vmd/vmd.h
+++ usr.sbin/vmd/vmd.h
@@ -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_FIXED 0x02
+#define VMIFF_OPTMASK VMIFF_FIXED
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];
diff --git usr.sbin/vmd/vmm.c usr.sbin/vmd/vmm.c
index 5f46d7a..d1b1dcd 100644
--- usr.sbin/vmd/vmm.c
+++ usr.sbin/vmd/vmm.c
@@ -68,7 +68,7 @@ int opentap(char *);
int start_vm(struct imsg *, uint32_t *);
int terminate_vm(struct vm_terminate_params *);
int get_info_vm(struct privsep *, struct imsg *, int);
-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 *event_thread(void *);
void *vcpu_run_loop(void *);
int vcpu_exit(struct vm_run_params *);
@@ -76,7 +76,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 vmm_dispatch_parent(int, struct privsep_proc *, struct imsg *);
@@ -771,7 +771,7 @@ start_vm(struct imsg *imsg, uint32_t *id)
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);
_exit(ret);
}
@@ -1018,9 +1018,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 */
@@ -1059,7 +1060,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);
}
/*
@@ -1070,7 +1071,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
*
@@ -1079,9 +1080,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;
@@ -1122,7 +1124,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) {
