[lxc-devel] [PATCH RFC] introduce lxc.cap.keep

Serge Hallyn serge.hallyn at ubuntu.com
Fri Jun 14 03:47:56 UTC 2013


Thanks github for sending mails from mangled addresses.  Just replying
to myself with Walter's real email address so he sees a copy.

Please reply to this email, not the parent, if possible, so you don't
reply to notifications at github.com.

Quoting Serge Hallyn (serge.hallyn at ubuntu.com):
> The lxc configuration file currently supports 'lxc.cap.drop', a list of
> capabilities to be dropped (using the bounding set) from the container.
> The problem with this is that over time new capabilities are added.  So
> an older container configuration file may, over time, become insecure.
> 
> Walter has in the past suggested replacing lxc.cap.drop with
> lxc.cap.preserve, which would have the inverse sense - any capabilities
> in that set would be kept, any others would be dropped.
> 
> Realistically both have the same problem - the sendmail capabilities
> bug proved that running code with unexpectedly dropped privilege can be
> dangerous.  This patch gives the admin a choice:  You can use either
> lxc.cap.keep or lxc.cap.drop, not both.
> 
> Both continue to be ignored if a user namespace is in use.
> 
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
> ---
>  doc/lxc.conf.sgml.in |   11 ++++++
>  src/lxc/conf.c       |   93 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  src/lxc/conf.h       |    5 ++-
>  src/lxc/confile.c    |   70 +++++++++++++++++++++++++++++++++++++
>  4 files changed, 176 insertions(+), 3 deletions(-)
> 
> diff --git a/doc/lxc.conf.sgml.in b/doc/lxc.conf.sgml.in
> index 6500e50..7c289a0 100644
> --- a/doc/lxc.conf.sgml.in
> +++ b/doc/lxc.conf.sgml.in
> @@ -771,6 +771,17 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>  	    </para>
>  	  </listitem>
>  	</varlistentry>
> +	<varlistentry>
> +	  <term>
> +	    <option>lxc.cap.keep</option>
> +	  </term>
> +	  <listitem>
> +	    <para>
> +	      Specify the capability to be kept in the container. All other
> +	      capabilities will be dropped.
> +	    </para>
> +	  </listitem>
> +	</varlistentry>
>        </variablelist>
>      </refsect2>
>  
> diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> index c309485..14df7c2 100644
> --- a/src/lxc/conf.c
> +++ b/src/lxc/conf.c
> @@ -1804,7 +1804,73 @@ static int setup_caps(struct lxc_list *caps)
>  
>  	}
>  
> -	DEBUG("capabilities has been setup");
> +	DEBUG("capabilities have been setup");
> +
> +	return 0;
> +}
> +
> +static int dropcaps_except(struct lxc_list *caps)
> +{
> +	struct lxc_list *iterator;
> +	char *keep_entry;
> +	char *ptr;
> +	int i, capid;
> +	int numcaps = lxc_caps_last_cap() + 1;
> +	INFO("found %d capabilities\n", numcaps);
> +
> +	// caplist[i] is 1 if we keep capability i
> +	int *caplist = alloca(numcaps * sizeof(int));
> +	memset(caplist, 0, numcaps * sizeof(int));
> +
> +	lxc_list_for_each(iterator, caps) {
> +
> +		keep_entry = iterator->elem;
> +
> +		capid = -1;
> +
> +		for (i = 0; i < sizeof(caps_opt)/sizeof(caps_opt[0]); i++) {
> +
> +			if (strcmp(keep_entry, caps_opt[i].name))
> +				continue;
> +
> +			capid = caps_opt[i].value;
> +			break;
> +		}
> +
> +		if (capid < 0) {
> +			/* try to see if it's numeric, so the user may specify
> +			* capabilities  that the running kernel knows about but
> +			* we don't */
> +			capid = strtol(keep_entry, &ptr, 10);
> +			if (!ptr || *ptr != '\0' ||
> +			capid == LONG_MIN || capid == LONG_MAX)
> +				/* not a valid number */
> +				capid = -1;
> +			else if (capid > lxc_caps_last_cap())
> +				/* we have a number but it's not a valid
> +				* capability */
> +				capid = -1;
> +		}
> +
> +	        if (capid < 0) {
> +			ERROR("unknown capability %s", keep_entry);
> +			return -1;
> +		}
> +
> +		DEBUG("drop capability '%s' (%d)", keep_entry, capid);
> +
> +		caplist[capid] = 1;
> +	}
> +	for (i=0; i<numcaps; i++) {
> +		if (caplist[i])
> +			continue;
> +		if (prctl(PR_CAPBSET_DROP, i, 0, 0, 0)) {
> +                       SYSERROR("failed to remove capability %d", i);
> +                       return -1;
> +                }
> +	}
> +
> +	DEBUG("capabilities have been setup");
>  
>  	return 0;
>  }
> @@ -2140,6 +2206,7 @@ struct lxc_conf *lxc_conf_init(void)
>  	lxc_list_init(&new->network);
>  	lxc_list_init(&new->mount_list);
>  	lxc_list_init(&new->caps);
> +	lxc_list_init(&new->keepcaps);
>  	lxc_list_init(&new->id_map);
>  	for (i=0; i<NUM_LXC_HOOKS; i++)
>  		lxc_list_init(&new->hooks[i]);
> @@ -2884,7 +2951,16 @@ int lxc_setup(const char *name, struct lxc_conf *lxc_conf)
>  	}
>  
>  	if (lxc_list_empty(&lxc_conf->id_map)) {
> -		if (setup_caps(&lxc_conf->caps)) {
> +		if (!lxc_list_empty(&lxc_conf->keepcaps)) {
> +			if (!lxc_list_empty(&lxc_conf->caps)) {
> +				ERROR("Simultaneously requested dropping and keeping caps");
> +				return -1;
> +			}
> +			if (dropcaps_except(&lxc_conf->keepcaps)) {
> +				ERROR("failed to keep requested caps\n");
> +				return -1;
> +			}
> +		} else if (setup_caps(&lxc_conf->caps)) {
>  			ERROR("failed to drop capabilities");
>  			return -1;
>  		}
> @@ -3070,6 +3146,18 @@ int lxc_clear_config_caps(struct lxc_conf *c)
>  	return 0;
>  }
>  
> +int lxc_clear_config_keepcaps(struct lxc_conf *c)
> +{
> +	struct lxc_list *it,*next;
> +
> +	lxc_list_for_each_safe(it, &c->keepcaps, next) {
> +		lxc_list_del(it);
> +		free(it->elem);
> +		free(it);
> +	}
> +	return 0;
> +}
> +
>  int lxc_clear_cgroups(struct lxc_conf *c, const char *key)
>  {
>  	struct lxc_list *it,*next;
> @@ -3169,6 +3257,7 @@ void lxc_conf_free(struct lxc_conf *conf)
>  #endif
>  	lxc_seccomp_free(conf);
>  	lxc_clear_config_caps(conf);
> +	lxc_clear_config_keepcaps(conf);
>  	lxc_clear_cgroups(conf, "lxc.cgroup");
>  	lxc_clear_hooks(conf, "lxc.hook");
>  	lxc_clear_mount_entries(conf);
> diff --git a/src/lxc/conf.h b/src/lxc/conf.h
> index 2fd3ab1..5ca30ec 100644
> --- a/src/lxc/conf.h
> +++ b/src/lxc/conf.h
> @@ -232,7 +232,8 @@ struct lxc_rootfs {
>   * @network    : network configuration
>   * @utsname    : container utsname
>   * @fstab      : path to a fstab file format
> - * @caps       : list of the capabilities
> + * @caps       : list of the capabilities to drop
> + * @keepcaps   : list of the capabilities to keep
>   * @tty_info   : tty data
>   * @console    : console data
>   * @ttydir     : directory (under /dev) in which to create console and ttys
> @@ -265,6 +266,7 @@ struct lxc_conf {
>  	int num_savednics;
>  	struct lxc_list mount_list;
>  	struct lxc_list caps;
> +	struct lxc_list keepcaps;
>  	struct lxc_tty_info tty_info;
>  	struct lxc_console console;
>  	struct lxc_rootfs rootfs;
> @@ -315,6 +317,7 @@ extern void lxc_delete_tty(struct lxc_tty_info *tty_info);
>  extern int lxc_clear_config_network(struct lxc_conf *c);
>  extern int lxc_clear_nic(struct lxc_conf *c, const char *key);
>  extern int lxc_clear_config_caps(struct lxc_conf *c);
> +extern int lxc_clear_config_keepcaps(struct lxc_conf *c);
>  extern int lxc_clear_cgroups(struct lxc_conf *c, const char *key);
>  extern int lxc_clear_mount_entries(struct lxc_conf *c);
>  extern int lxc_clear_hooks(struct lxc_conf *c, const char *key);
> diff --git a/src/lxc/confile.c b/src/lxc/confile.c
> index a7db117..a4006bc 100644
> --- a/src/lxc/confile.c
> +++ b/src/lxc/confile.c
> @@ -85,6 +85,7 @@ static int config_network_script(const char *, const char *, struct lxc_conf *);
>  static int config_network_ipv6(const char *, const char *, struct lxc_conf *);
>  static int config_network_ipv6_gateway(const char *, const char *, struct lxc_conf *);
>  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_includefile(const char *, const char *, struct lxc_conf *);
> @@ -136,6 +137,7 @@ static struct lxc_config_t config[] = {
>  	/* config_network_nic must come after all other 'lxc.network.*' entries */
>  	{ "lxc.network.",             config_network_nic          },
>  	{ "lxc.cap.drop",             config_cap_drop             },
> +	{ "lxc.cap.keep",             config_cap_keep             },
>  	{ "lxc.console",              config_console              },
>  	{ "lxc.seccomp",              config_seccomp              },
>  	{ "lxc.include",              config_includefile          },
> @@ -1264,6 +1266,52 @@ static int config_mount(const char *key, const char *value,
>  	return 0;
>  }
>  
> +static int config_cap_keep(const char *key, const char *value,
> +			   struct lxc_conf *lxc_conf)
> +{
> +	char *keepcaps, *keepptr, *sptr, *token;
> +	struct lxc_list *keeplist;
> +	int ret = -1;
> +
> +	if (!strlen(value))
> +		return -1;
> +
> +	keepcaps = strdup(value);
> +	if (!keepcaps) {
> +		SYSERROR("failed to dup '%s'", value);
> +		return -1;
> +	}
> +
> +	/* in case several capability keep is specified in a single line
> +	 * split these caps in a single element for the list */
> +	for (keepptr = keepcaps;;keepptr = NULL) {
> +                token = strtok_r(keepptr, " \t", &sptr);
> +                if (!token) {
> +			ret = 0;
> +                        break;
> +		}
> +
> +		keeplist = malloc(sizeof(*keeplist));
> +		if (!keeplist) {
> +			SYSERROR("failed to allocate keepcap list");
> +			break;
> +		}
> +
> +		keeplist->elem = strdup(token);
> +		if (!keeplist->elem) {
> +			SYSERROR("failed to dup '%s'", token);
> +			free(keeplist);
> +			break;
> +		}
> +
> +		lxc_list_add_tail(&lxc_conf->keepcaps, keeplist);
> +        }
> +
> +	free(keepcaps);
> +
> +	return ret;
> +}
> +
>  static int config_cap_drop(const char *key, const char *value,
>  			   struct lxc_conf *lxc_conf)
>  {
> @@ -1630,6 +1678,22 @@ static int lxc_get_item_cap_drop(struct lxc_conf *c, char *retv, int inlen)
>  	return fulllen;
>  }
>  
> +static int lxc_get_item_cap_keep(struct lxc_conf *c, char *retv, int inlen)
> +{
> +	int len, fulllen = 0;
> +	struct lxc_list *it;
> +
> +	if (!retv)
> +		inlen = 0;
> +	else
> +		memset(retv, 0, inlen);
> +
> +	lxc_list_for_each(it, &c->keepcaps) {
> +		strprint(retv, inlen, "%s\n", (char *)it->elem);
> +	}
> +	return fulllen;
> +}
> +
>  static int lxc_get_mount_entries(struct lxc_conf *c, char *retv, int inlen)
>  {
>  	int len, fulllen = 0;
> @@ -1810,6 +1874,8 @@ int lxc_get_config_item(struct lxc_conf *c, const char *key, char *retv,
>  		v = c->rootfs.pivot;
>  	else if (strcmp(key, "lxc.cap.drop") == 0)
>  		return lxc_get_item_cap_drop(c, retv, inlen);
> +	else if (strcmp(key, "lxc.cap.keep") == 0)
> +		return lxc_get_item_cap_keep(c, retv, inlen);
>  	else if (strncmp(key, "lxc.hook", 8) == 0)
>  		return lxc_get_item_hooks(c, retv, inlen, key);
>  	else if (strcmp(key, "lxc.network") == 0)
> @@ -1833,6 +1899,8 @@ int lxc_clear_config_item(struct lxc_conf *c, const char *key)
>  		return lxc_clear_nic(c, key + 12);
>  	else if (strcmp(key, "lxc.cap.drop") == 0)
>  		return lxc_clear_config_caps(c);
> +	else if (strcmp(key, "lxc.cap.keep") == 0)
> +		return lxc_clear_config_keepcaps(c);
>  	else if (strncmp(key, "lxc.cgroup", 10) == 0)
>  		return lxc_clear_cgroups(c, key);
>  	else if (strcmp(key, "lxc.mount.entries") == 0)
> @@ -1945,6 +2013,8 @@ void write_config(FILE *fout, struct lxc_conf *c)
>  	}
>  	lxc_list_for_each(it, &c->caps)
>  		fprintf(fout, "lxc.cap.drop = %s\n", (char *)it->elem);
> +	lxc_list_for_each(it, &c->keepcaps)
> +		fprintf(fout, "lxc.cap.keep = %s\n", (char *)it->elem);
>  	for (i=0; i<NUM_LXC_HOOKS; i++) {
>  		lxc_list_for_each(it, &c->hooks[i])
>  			fprintf(fout, "lxc.hook.%s = %s\n",
> -- 
> 1.7.9.5
> 
> 
> ------------------------------------------------------------------------------
> This SF.net email is sponsored by Windows:
> 
> Build for Windows Store.
> 
> http://p.sf.net/sfu/windows-dev2dev
> _______________________________________________
> Lxc-devel mailing list
> Lxc-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/lxc-devel




More information about the lxc-devel mailing list