[lxc-devel] hwaddr tuning in templates
Michael H. Warfield
mhw at WittsEnd.com
Fri Jan 3 20:55:11 UTC 2014
On Fri, 2014-01-03 at 14:59 -0500, Stéphane Graber wrote:
> On Thu, Jan 02, 2014 at 10:50:03AM -0600, Serge Hallyn wrote:
> > Quoting Guillaume ZITTA (lxc at zitta.fr):
> > > >>If not, my proposal:
> > > >> I set an incomplete (prefix) lxc.network.hwaddr in default conf.
> > > >> lxc-create complete it randomly and write it to the container's
> > > >>config.
> > > >>
> > > >>Does it make sense?
> > > >
> > > >I think so. Just 'xx' in place of real numbers?
> > > >
> > > >>I do not/can't code in C, but perhaps I should try ...
> > > >>
> > > >>Guillaume ZITTA
> > >
> > > Please be kind, it's the second time I do "C" of my life :)
> > >
> > > Note about randinit():
> > > that kind of util can be used in network.c/lxc_mkifname() and
> > > lxccontainer.c/new_hwaddr()
> > >
> > > It works for me, I did 1000 destroy/create (with fake template) in
> > > 43 sec without mac duplicate.
> > >
> > > If I'm not completely doing wrong, I'll provide patch for manpage
> > > and HAVE_RAND_R ifdef.
> > >
> > > please comment.
> >
> > Looks good, except for one comment below, and if you do submit
> > this please do add a Signed-off-by line.
> >
> > >
> > > diff --git a/src/lxc/confile.c b/src/lxc/confile.c
> > > index c5ec4f3..5721e05 100644
> > > --- a/src/lxc/confile.c
> > > +++ b/src/lxc/confile.c
> > > @@ -509,6 +509,24 @@ static int macvlan_mode(int *valuep, const char
> > > *value)
> > > return -1;
> > > }
> > >
> > > +char *rand_complete_hwaddr(const char *hwaddr)
> > > +{
> > > + const char hex[] = "0123456789abcdef";
> > > + char *retval=NULL;
> > > + retval = strdup(hwaddr);
> > > + char *curs = retval;
> > > +
> > > + randinit();
> > > +
> > > + while (*curs != '\0')
> > > + {
> > > + if ( *curs == 'x' || *curs == 'X' )
> > > + *curs = hex[rand() & 0x0F];
> > > + curs++;
> > > + }
> > > + return retval;
> > > +}
> It's also unfortunate that this implementation will allow people to
> generate invalid MAC addresses but it's not clear whether we should
> catch those or not in this function.
> Specifically what should be discouraged is:
> - least significant bit of first byte set to 1 (multicast)
> - second least significant bit of first byte set to 0 with the first 3
> bytes not in one of the VM ranges (xen, vmware, ...).
> Only the first is really harmful, the latter may create collision and
> invalid detection of the manufacturer by network analysis tools.
Yeah the first case is really the only one you NEED to worry about.
The second case is of limited importance since the locally managed bit
is a non-enforced standard and convention on the physical cards
themselves. Even most hardware cards will allow you to locally set the
MAC addresses to values you "shouldn't" be using (they're not "invalid"
per se but they are non-conforming) without that bit enabled.
That lets a lot of us the security business to some really wicked MAC
spoofing on WiFi networks by setting our MAC addresses to match sniffed
MAC addresses (after they've gone quiet). Yeah, so much for MAC
filtering or app gateway security. If it's not even enforced at the
hardware level, why should we be overly worried about it at our level?
I use to do this a lot with OpenVZ using the manufacturer OUI / range
for the ancient WD 8003 series 8 bit ISA cards (I have a stack of those
old beasts). I know perfectly well I'll never see a conflict with THOSE
on any of my networks. Since we can, in most cases, set a hardware card
to those "non-conforming" addresses, I wouldn't have too much heartburn
over enforcing the locally managed bit if an admin chose to do so. I
just wouldn't do it in the autogeneration code itself as a part of the
default MAC logic. IETF mantra: "Be conservative in what you generate,
be liberal in what you accept."
> Ideally we'd have a range allocated for LXC but the paperwork seems a
> bit tedious and there are costs involved in getting one of those.
> If we were to have one, we could simply hardcode the first 3 bytes to
> our OUI and have the function only fill the last 3 with random values.
VMware has done this but even they have overrides if you are really
really insistent (MAC OUI detection has been used by malware for
virtualization detection). I think the others do too. Do the right
thing but then still let the user do his own thing as long as it doesn't
break something (the multicast bit). Then it's on his head and not on
ours.
Regards,
Mike
> Details at: http://en.wikipedia.org/wiki/MAC_address#Address_details
>
> > > +
> > > static int config_network_flags(const char *key, const char *value,
> > > struct lxc_conf *lxc_conf)
> > > {
> > > @@ -576,11 +594,13 @@ static int config_network_hwaddr(const char
> > > *key, const char *value,
> > > {
> > > struct lxc_netdev *netdev;
> > >
> > > - netdev = network_netdev(key, value, &lxc_conf->network);
> > > + const char *newval = rand_complete_hwaddr(value);
> >
> > newval got strdup()d above, but is never freed. config_string_item()
> > will actually strdup that again for the value added to the
> > configuration. So you need to free newval on exiting this function.
> >
> > > +
> > > + netdev = network_netdev(key, newval, &lxc_conf->network);
> > > if (!netdev)
> > > return -1;
> > >
> > > - return config_string_item(&netdev->hwaddr, value);
> > > + return config_string_item(&netdev->hwaddr, newval);
> > > }
> > >
> > > static int config_network_vlan_id(const char *key, const char *value,
> > > diff --git a/src/lxc/utils.c b/src/lxc/utils.c
> > > index 785f3e6..204179e 100644
> > > --- a/src/lxc/utils.c
> > > +++ b/src/lxc/utils.c
> > > @@ -1126,3 +1126,25 @@ void **lxc_append_null_to_array(void **array,
> > > size_t count)
> > > }
> > > return array;
> > > }
> > > +
> > > +void randinit(void)
> > > +{
> > > + /*
> > > + srand pre-seed function based on /dev/urandom
> > > + */
> > > + FILE *f;
> > > + process_lock();
> > > + f = fopen("/dev/urandom", "r");
> > > + process_unlock();
> > > + if (f) {
> > > + unsigned int seed;
> > > + int ret = fread(&seed, sizeof(seed), 1, f);
> > > + if (ret != 1)
> > > + seed = time(NULL);
> > > + process_lock();
> > > + fclose(f);
> > > + process_unlock();
> > > + srand(seed);
> > > + } else
> > > + srand(time(NULL));
> > > +}
> > > \ No newline at end of file
> > > diff --git a/src/lxc/utils.h b/src/lxc/utils.h
> > > index 945f1de..f20339b 100644
> > > --- a/src/lxc/utils.h
> > > +++ b/src/lxc/utils.h
> > > @@ -270,4 +270,7 @@ extern void **lxc_dup_array(void **array,
> > > lxc_dup_fn element_dup_fn, lxc_free_fn
> > > extern void **lxc_append_null_to_array(void **array, size_t count);
> > >
> > > extern void dump_stacktrace(void);
> > > +
> > > +//initialize rand with urandom
> > > +extern void randinit(void);
> > > #endif
> > >
> > > _______________________________________________
> > > lxc-devel mailing list
> > > lxc-devel at lists.linuxcontainers.org
> > > http://lists.linuxcontainers.org/listinfo/lxc-devel
> > _______________________________________________
> > lxc-devel mailing list
> > lxc-devel at lists.linuxcontainers.org
> > http://lists.linuxcontainers.org/listinfo/lxc-devel
>
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
--
Michael H. Warfield (AI4NB) | (770) 978-7061 | mhw at WittsEnd.com
/\/\|=mhw=|\/\/ | (678) 463-0932 | http://www.wittsend.com/mhw/
NIC whois: MHW9 | An optimist believes we live in the best of all
PGP Key: 0x674627FF | possible worlds. A pessimist is sure of it!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 482 bytes
Desc: This is a digitally signed message part
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20140103/24fa6a92/attachment.pgp>
More information about the lxc-devel
mailing list