[lxc-devel] hwaddr tuning in templates

[Stéphane Graber]

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.

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.

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

-- 
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20140103/2e73b840/attachment.pgp>