Re: vmd: fixed lladdr for VM guests to prevent MAC spoofing
On Wed, Mar 01, 2017 at 07:17:10PM +0100, Reyk Floeter wrote:
> 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
>
no objections
> 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 - 1.25
> +++ usr.sbin/vmd/config.c 1 Mar 2017 18:16:20 -
> @@ -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 - 1.21
> +++ usr.sbin/vmd/parse.y 1 Mar 2017 18:16:20 -
> @@ -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 STRING
> %token NUMBER
> %type disable
> +%type locked
> %type updown
> %type lladdr
> %type 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_U
Re: vmd: fixed lladdr for VM guests to prevent MAC spoofing
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 - 1.25
+++ usr.sbin/vmd/config.c 1 Mar 2017 18:16:20 -
@@ -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.y1 Mar 2017 07:43:33 - 1.21
+++ usr.sbin/vmd/parse.y1 Mar 2017 18:16:20 -
@@ -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 STRING
%token NUMBER
%typedisable
+%typelocked
%typeupdown
%typelladdr
%typestring
@@ -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
vmd: fixed lladdr for VM guests to prevent MAC spoofing
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 STRING
%token NUMBER
%typedisable
+%typefixed
%typeupdown
%typelladdr
%typestring
@@ -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",
