[lxc-devel] [PATCH] Validate container name during creation

[Serge Hallyn]

Quoting Robert Vogelgesang (vogel at users.sourceforge.net):
> Hello,
> 
> On Thu, Feb 05, 2015 at 05:50:39PM +0200, Joel Nider wrote:
> > The name used to identify the container on the host is also used as the 
> > host
> > name of the container itself. Therefore, the name should be restricted to 
> > a
> > legal Linux hostname, which is specified in RFC 1123

Thanks for the patch, Joel.  One other comment below.

> no, this should not be the job of the lxc create API, IMHO.
> It's the template script that re-uses the container name as the
> container's hostname, so it should be the template's job to ensure
> the name conforms to the rules.
> 
> I don't mind checking the value of the configuration item lxc.utsname,
> which actually defines the container's hostname, according to the
> RFC 1123 rules, but please don't do this for the container's name.

So you're suggesting using the same validate_hostname() function
at src/lxc/confile.c:config_utsname() ?

That should still end up being caught at lxc-create since we re-read
the config file at the end of lxcapi_create().

> 	Robert
> 
> > (http://tools.ietf.org/html/rfc1123#page-13). Basically it says the host 
> > name
> > is composed of up to 63 alphanumeric ASCII characters (case insensitive) 
> > as
> > well as '-'.
> > 
> > See this thread for more details:
> > https://lists.linuxcontainers.org/pipermail/lxc-devel/2014-December/011007.html
> > 
> > Signed-off-by: Joel Nider <joeln at il.ibm.com>
> > ---
> >  src/lxc/lxccontainer.c | 42 +++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 41 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> > index e02ee93..7cba771 100644
> > --- a/src/lxc/lxccontainer.c
> > +++ b/src/lxc/lxccontainer.c
> > @@ -36,6 +36,7 @@
> >  #include <stdint.h>
> >  #include <grp.h>
> >  #include <sys/syscall.h>
> > +#include <ctype.h>
> > 
> >  #include <lxc/lxccontainer.h>
> >  #include <lxc/version.h>
> > @@ -66,7 +67,7 @@
> >  #endif
> > 
> >  #define MAX_BUFFER 4096
> > -
> > +#define MAX_LENGTH_HOSTNAME 63
> >  #define NOT_SUPPORTED_ERROR "the requested function %s is not currently 
> > supported with unprivileged containers"
> > 
> >  /* Define faccessat() if missing from the C library */
> > @@ -190,6 +191,37 @@ static void remove_partial(struct lxc_container *c, 
> > int fd)
> >                 SYSERROR("Error unlink partial file %s", path);
> >  }
> > 
> > +/* Ensure requested hostname follows RFC 1123
> > + * In our case, that means simple host name (not FQDN)
> > + * characters in the set {[A-Z], [0-9], '-'} (no '.')
> > + * maximum length of 63 characters
> > + */
> > +static int validate_hostname(struct lxc_container *c)
> > +{
> > +   char *a;
> > +   int count = 0;
> > +
> > +   if (!c)
> > +      return MAX_LENGTH_HOSTNAME;
> > +
> > +   a = c->name;
> > +   while (*a) {
> > +      count++;
> > +      if (count > MAX_LENGTH_HOSTNAME)
> > +         return MAX_LENGTH_HOSTNAME;
> > +
> > +      if (!(isalnum(*a) | (*a == '-')))

Would prefer to see this as

	if (!valid_hostname_char(*a))
		return count;

with valid_hostname_char(const char a) {
	if (isalnum(a))
		return true;
	if (a == '-')
		return true;
	return false;
}

above.

> > +         return count;
> > +
> > +     a++;
> > +   }
> > +
> > +   if (count == 0)
> > +      return MAX_LENGTH_HOSTNAME;
> > +
> > +   return 0;
> > +}
> > +
> >  /* LOCKING
> >   * 1. container_mem_lock(c) protects the struct lxc_container from 
> > multiple threads.
> >   * 2. container_disk_lock(c) protects the on-disk container data - in 
> > particular the
> > @@ -1235,6 +1267,7 @@ static bool lxcapi_create(struct lxc_container *c, 
> > const char *t,
> >         pid_t pid;
> >         char *tpath = NULL;
> >         int partial_fd;
> > +   int err;
> > 
> >         if (!c)
> >                 return false;
> > @@ -1247,6 +1280,13 @@ static bool lxcapi_create(struct lxc_container *c, 
> > const char *t,
> >                 }
> >         }
> > 
> > +   /* validate the container name */
> > +   err = validate_hostname(c);
> > +   if (err) {
> > +      ERROR("Invalid hostname: %s (character %i)", c->name, err);
> > +      goto out;
> > +   }
> > +
> >         /*
> >          * If a template is passed in, and the rootfs already is defined 
> > in
> >          * the container config and exists, then * caller is trying to 
> > create
> > --
> > 1.9.1
> > 
> > 
> > 
> > Joel Nider
> > Virtualization Research
> > IBM Research and Development
> > Haifa Research Lab
> > 
> > Phone: 972-4-829-6326 | Mobile: 972-54-3155635
> > E-mail: JOELN at il.ibm.com
> > 
> > 
> > 
> > _______________________________________________
> > 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