[lxc-devel] [PATCH 1/1] seccomp: add 'lxc.seccomp.optional'

[Stéphane Graber]

On Mon, Feb 17, 2014 at 05:16:51PM -0500, Stéphane Graber wrote:
> On Mon, Feb 17, 2014 at 02:05:10PM -0600, Serge Hallyn wrote:
> > If that is set, then if reading the policy failed, we continue
> > without trying to load seccomp.  (If reading the policy
> > succeeded, then we do not ignore failure to load the policy;
> > we could consider doing that as well, however the goal here
> > is to have a generic container configuration work whether
> > the host has seccompv2 support or not)
> > 
> > Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
>
> Acked-by: Stéphane Graber <stgraber at ubuntu.com>

Actually, I'll wait a bit before pushing this, the code isn't
technically wrong (that I could see) but it's pretty confusing.

The option isn't documented in lxc.container.conf.sgml and the behavior
of the option is a bit odd too.

Let's take a lxc-clone example:
 If .optional == 1 and .seccomp is invalid => neither option appear in target container
 If .optional == 1 and .seccomp is valid => only .seccomp appears in the target

It's also not possible to query its value or set it through the API.

> 
> > ---
> >  src/lxc/attach.c  | 10 ++++++++--
> >  src/lxc/conf.h    |  1 +
> >  src/lxc/confile.c | 15 +++++++++++++++
> >  src/lxc/start.c   |  9 +++++++--
> >  4 files changed, 31 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/lxc/attach.c b/src/lxc/attach.c
> > index 31a5ae7..600332a 100644
> > --- a/src/lxc/attach.c
> > +++ b/src/lxc/attach.c
> > @@ -613,8 +613,14 @@ static bool fetch_seccomp(const char *name, const char *lxcpath,
> >  	if (!c->lxc_conf)
> >  		return false;
> >  	if (lxc_read_seccomp_config(c->lxc_conf) < 0) {
> > -		ERROR("Error reaading seccomp policy");
> > -		return false;
> > +		if (c->lxc_conf->seccomp_optional) {
> > +			WARN("Ignoring error loading seccomp policy.");
> > +			lxc_container_put(c);
> > +			i->container = NULL;
> > +		} else {
> > +			ERROR("Error reaading seccomp policy");
> > +			return false;
> > +		}
> >  	}
> >  
> >  	return true;
> > diff --git a/src/lxc/conf.h b/src/lxc/conf.h
> > index 4591470..3622287 100644
> > --- a/src/lxc/conf.h
> > +++ b/src/lxc/conf.h
> > @@ -303,6 +303,7 @@ struct lxc_conf {
> >  	char *lsm_se_context;
> >  	int tmp_umount_proc;
> >  	char *seccomp;  // filename with the seccomp rules
> > +	bool seccomp_optional;  // proceed if seccomp_load fails
> >  #if HAVE_SCMP_FILTER_CTX
> >  	scmp_filter_ctx *seccomp_ctx;
> >  #endif
> > diff --git a/src/lxc/confile.c b/src/lxc/confile.c
> > index afc9e32..bc99e45 100644
> > --- a/src/lxc/confile.c
> > +++ b/src/lxc/confile.c
> > @@ -89,6 +89,7 @@ static int config_cap_drop(const char *, const char *, struct lxc_conf *);
> >  static int config_cap_keep(const char *, const char *, struct lxc_conf *);
> >  static int config_console(const char *, const char *, struct lxc_conf *);
> >  static int config_seccomp(const char *, const char *, struct lxc_conf *);
> > +static int config_seccomp_opt(const char *, const char *, struct lxc_conf *);
> >  static int config_includefile(const char *, const char *, struct lxc_conf *);
> >  static int config_network_nic(const char *, const char *, struct lxc_conf *);
> >  static int config_autodev(const char *, const char *, struct lxc_conf *);
> > @@ -143,6 +144,7 @@ static struct lxc_config_t config[] = {
> >  	{ "lxc.cap.drop",             config_cap_drop             },
> >  	{ "lxc.cap.keep",             config_cap_keep             },
> >  	{ "lxc.console",              config_console              },
> > +	{ "lxc.seccomp.optional",     config_seccomp_opt          },
> >  	{ "lxc.seccomp",              config_seccomp              },
> >  	{ "lxc.include",              config_includefile          },
> >  	{ "lxc.autodev",              config_autodev              },
> > @@ -928,6 +930,19 @@ static int config_seccomp(const char *key, const char *value,
> >  	return config_path_item(&lxc_conf->seccomp, value);
> >  }
> >  
> > +static int config_seccomp_opt(const char *key, const char *value,
> > +				 struct lxc_conf *lxc_conf)
> > +{
> > +	int opt, ret;
> > +
> > +	ret = sscanf(value, "%d", &opt);
> > +	if (ret != 1)
> > +		return -1;
> > +	lxc_conf->seccomp_optional = opt == 1;
> > +
> > +	return 0;
> > +}
> > +
> >  static int config_hook(const char *key, const char *value,
> >  				 struct lxc_conf *lxc_conf)
> >  {
> > diff --git a/src/lxc/start.c b/src/lxc/start.c
> > index 5b3b6eb..8c3c40f 100644
> > --- a/src/lxc/start.c
> > +++ b/src/lxc/start.c
> > @@ -381,8 +381,13 @@ struct lxc_handler *lxc_init(const char *name, struct lxc_conf *conf, const char
> >  		goto out_free_name;
> >  
> >  	if (lxc_read_seccomp_config(conf) != 0) {
> > -		ERROR("failed loading seccomp policy");
> > -		goto out_close_maincmd_fd;
> > +		if (conf->seccomp_optional) {
> > +			WARN("Ignoring error loading seccomp policy.");
> > +			lxc_seccomp_free(conf);
> > +		} else {
> > +			ERROR("Exiting on failure to load seccomp policy");
> > +			goto out_close_maincmd_fd;
> > +		}
> >  	}
> >  
> >  	/* Begin by setting the state to STARTING */
> > -- 
> > 1.9.rc1
> > 
> > _______________________________________________
> > 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



> _______________________________________________
> 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: 819 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20140217/2def12bc/attachment.pgp>