[lxc-devel] [PATCH] handle unprivileged user calls more gracefully (v2)
S.Çağlar Onur
caglar at 10ur.org
Sat Jan 18 04:58:17 UTC 2014
On Fri, Jan 17, 2014 at 11:49 PM, Stéphane Graber <stgraber at ubuntu.com> wrote:
> On Fri, Jan 17, 2014 at 11:34:50PM -0500, S.Çağlar Onur wrote:
>> Return an error if the function is not supposed to be called by an unprivileged user.
>> Otherwise those calls fail in the middle of their execution with different reasons.
>>
>> changes since v1:
>> - am_unpriv is now a simple geteuid check,
>> - API functions are now providing error messages,
>> - lxc-info, lxc-attach and lxc-execute are now checking geteuid
>>
>
> That looks much better to me, though I disagree that lxc-execute doesn't
> work. It certainly is a bit restricted compared ot its system
> equivalent, but all of those limitations will be the same as with
> lxc-start.
>
> stgraber at castiana:~$ lxc-execute -n p1 -- /bin/bash
> lxc-init: failed to mount /proc : Device or resource busy
> lxc-init: failed to mount /dev/shm : No such file or directory
> root at p1:/#
Hmm then I must be having some kind of trouble here with it. I can
start/stop containers just fine
[caglar at oOo:~/go/src/github.com/caglar10ur/lxc/examples] lxc-start -d -n rubik
[caglar at oOo:~/go/src/github.com/caglar10ur/lxc/examples] lxc-ls --fancy
NAME STATE IPV4 IPV6 AUTOSTART
-------------------------------------------
rubik RUNNING UNKNOWN UNKNOWN NO
[caglar at oOo:~/go/src/github.com/caglar10ur/lxc/examples] lxc-stop -n rubik
sudo variant works just fine
[caglar at oOo:~/go/src/github.com/caglar10ur/lxc/examples] sudo
lxc-execute -n p1 -- /bin/bash
root at oOo:/home/caglar/go/src/github.com/caglar10ur/lxc/examples# exit
[caglar at oOo:~/go/src/github.com/caglar10ur/lxc/examples]
but plain one gives this
[caglar at oOo:~/go/src/github.com/caglar10ur/lxc/examples] lxc-execute
-n p1 -- /bin/bash
lxc-execute: Operation not permitted - failed to clone
lxc-execute: failed to clone(0x6c020000): Operation not permitted
lxc-execute: Operation not permitted - failed to fork into a new namespace
lxc-execute: failed to spawn 'p1'
> It may be worth teaching lxc_init a bit about unprivileged containers
> but besides that, if all I wanted to do was run a command using that
> container rootfs, it'd work.
>
> The rest I think looks good and hopefully we can drop quite a few of
> those before the 1.0 release. It shouldn't be too hard to get basic dir
> cloning working and a good chunk of the rest would get resolved if we
> had attach working.
>
> Ideally, we'd release with only the snapshot functions and the
> add/remove device function disabled as those simply aren't doable at
> this point.
>
>> Signed-off-by: S.Çağlar Onur <caglar at 10ur.org>
>> ---
>> src/lxc/lxc_attach.c | 5 ++++
>> src/lxc/lxc_execute.c | 5 ++++
>> src/lxc/lxc_info.c | 20 +++++++------
>> src/lxc/lxccontainer.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++----
>> 4 files changed, 95 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/lxc/lxc_attach.c b/src/lxc/lxc_attach.c
>> index 6744c05..1159d09 100644
>> --- a/src/lxc/lxc_attach.c
>> +++ b/src/lxc/lxc_attach.c
>> @@ -188,6 +188,11 @@ int main(int argc, char *argv[])
>> lxc_attach_options_t attach_options = LXC_ATTACH_OPTIONS_DEFAULT;
>> lxc_attach_command_t command;
>>
>> + if (geteuid() != 0) {
>> + ERROR("lxc-attach is not currently supported with unprivileged containers");
>> + return -1;
>> + }
>> +
>> ret = lxc_caps_init();
>> if (ret)
>> return ret;
>> diff --git a/src/lxc/lxc_execute.c b/src/lxc/lxc_execute.c
>> index 6a54bf6..8177ed3 100644
>> --- a/src/lxc/lxc_execute.c
>> +++ b/src/lxc/lxc_execute.c
>> @@ -92,6 +92,11 @@ int main(int argc, char *argv[])
>> char *rcfile;
>> struct lxc_conf *conf;
>>
>> + if (geteuid() != 0) {
>> + ERROR("lxc-execute is not currently supported with unprivileged containers");
>> + return -1;
>> + }
>> +
>> lxc_list_init(&defines);
>>
>> if (lxc_caps_init())
>> diff --git a/src/lxc/lxc_info.c b/src/lxc/lxc_info.c
>> index b515087..0990036 100644
>> --- a/src/lxc/lxc_info.c
>> +++ b/src/lxc/lxc_info.c
>> @@ -310,15 +310,19 @@ static int print_info(const char *name, const char *lxcpath)
>> }
>>
>> if (ips) {
>> - char **addresses = c->get_ips(c, NULL, NULL, 0);
>> - if (addresses) {
>> - char *address;
>> - i = 0;
>> - while (addresses[i]) {
>> - address = addresses[i];
>> - print_info_msg_str("IP:", address);
>> - i++;
>> + if (geteuid() == 0) {
>> + char **addresses = c->get_ips(c, NULL, NULL, 0);
>> + if (addresses) {
>> + char *address;
>> + i = 0;
>> + while (addresses[i]) {
>> + address = addresses[i];
>> + print_info_msg_str("IP:", address);
>> + i++;
>> + }
>> }
>> + } else {
>> + print_info_msg_str("IP:", "UNKNOWN");
>> }
>> }
>>
>> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
>> index 0bebdff..368cb46 100644
>> --- a/src/lxc/lxccontainer.c
>> +++ b/src/lxc/lxccontainer.c
>> @@ -62,8 +62,14 @@
>>
>> #define MAX_BUFFER 4096
>>
>> +#define NOT_SUPPORTED_ERROR "the requested function %s is not currently supported with unprivileged containers"
>> +
>> lxc_log_define(lxc_container, lxc);
>>
>> +inline static bool am_unpriv() {
>> + return geteuid() != 0;
>> +}
>> +
>> static bool file_exists(const char *f)
>> {
>> struct stat statbuf;
>> @@ -1489,6 +1495,11 @@ static char** lxcapi_get_interfaces(struct lxc_container *c)
>> char **interfaces = NULL;
>> int old_netns = -1, new_netns = -1;
>>
>> + if (am_unpriv()) {
>> + ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
>> + goto out;
>> + }
>> +
>> if (!enter_to_ns(c, &old_netns, &new_netns))
>> goto out;
>>
>> @@ -1538,6 +1549,11 @@ static char** lxcapi_get_ips(struct lxc_container *c, const char* interface, con
>> char *address = NULL;
>> int old_netns = -1, new_netns = -1;
>>
>> + if (am_unpriv()) {
>> + ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
>> + goto out;
>> + }
>> +
>> if (!enter_to_ns(c, &old_netns, &new_netns))
>> goto out;
>>
>> @@ -1818,7 +1834,7 @@ static int lxc_rmdir_onedev_wrapper(void *data)
>> static bool lxcapi_destroy(struct lxc_container *c)
>> {
>> struct bdev *r = NULL;
>> - bool bret = false, am_unpriv;
>> + bool bret = false;
>> int ret;
>>
>> if (!c || !lxcapi_is_defined(c))
>> @@ -1833,14 +1849,12 @@ static bool lxcapi_destroy(struct lxc_container *c)
>> goto out;
>> }
>>
>> - am_unpriv = c->lxc_conf && geteuid() != 0 && !lxc_list_empty(&c->lxc_conf->id_map);
>> -
>> if (c->lxc_conf && has_snapshots(c)) {
>> ERROR("container %s has dependent snapshots", c->name);
>> goto out;
>> }
>>
>> - if (!am_unpriv && c->lxc_conf->rootfs.path && c->lxc_conf->rootfs.mount) {
>> + if (!am_unpriv() && c->lxc_conf->rootfs.path && c->lxc_conf->rootfs.mount) {
>> r = bdev_init(c->lxc_conf->rootfs.path, c->lxc_conf->rootfs.mount, NULL);
>> if (r) {
>> if (r->ops->destroy(r) < 0) {
>> @@ -1857,7 +1871,7 @@ static bool lxcapi_destroy(struct lxc_container *c)
>> const char *p1 = lxcapi_get_config_path(c);
>> char *path = alloca(strlen(p1) + strlen(c->name) + 2);
>> sprintf(path, "%s/%s", p1, c->name);
>> - if (am_unpriv)
>> + if (am_unpriv())
>> ret = userns_exec_1(c->lxc_conf, lxc_rmdir_onedev_wrapper, path);
>> else
>> ret = lxc_rmdir_onedev(path);
>> @@ -2406,6 +2420,11 @@ static struct lxc_container *lxcapi_clone(struct lxc_container *c, const char *n
>> if (!c || !c->is_defined(c))
>> return NULL;
>>
>> + if (am_unpriv()) {
>> + ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
>> + return NULL;
>> + }
>> +
>> if (container_mem_lock(c))
>> return NULL;
>>
>> @@ -2515,6 +2534,11 @@ static bool lxcapi_rename(struct lxc_container *c, const char *newname)
>> if (!c || !c->name || !c->config_path)
>> return false;
>>
>> + if (am_unpriv()) {
>> + ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
>> + return false;
>> + }
>> +
>> bdev = bdev_init(c->lxc_conf->rootfs.path, c->lxc_conf->rootfs.mount, NULL);
>> if (!bdev) {
>> ERROR("Failed to find original backing store type");
>> @@ -2543,6 +2567,11 @@ static int lxcapi_attach(struct lxc_container *c, lxc_attach_exec_t exec_functio
>> if (!c)
>> return -1;
>>
>> + if (am_unpriv()) {
>> + ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
>> + return -1;
>> + }
>> +
>> return lxc_attach(c->name, c->config_path, exec_function, exec_payload, options, attached_process);
>> }
>>
>> @@ -2555,6 +2584,11 @@ static int lxcapi_attach_run_wait(struct lxc_container *c, lxc_attach_options_t
>> if (!c)
>> return -1;
>>
>> + if (am_unpriv()) {
>> + ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
>> + return -1;
>> + }
>> +
>> command.program = (char*)program;
>> command.argv = (char**)argv;
>> r = lxc_attach(c->name, c->config_path, lxc_attach_run_command, &command, options, &pid);
>> @@ -2587,6 +2621,11 @@ static int lxcapi_snapshot(struct lxc_container *c, const char *commentfile)
>> struct lxc_container *c2;
>> char snappath[MAXPATHLEN], newname[20];
>>
>> + if (am_unpriv()) {
>> + ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
>> + return -1;
>> + }
>> +
>> // /var/lib/lxc -> /var/lib/lxcsnaps \0
>> ret = snprintf(snappath, MAXPATHLEN, "%ssnaps/%s", c->config_path, c->name);
>> if (ret < 0 || ret >= MAXPATHLEN)
>> @@ -2724,6 +2763,12 @@ static int lxcapi_snapshot_list(struct lxc_container *c, struct lxc_snapshot **r
>>
>> if (!c || !lxcapi_is_defined(c))
>> return -1;
>> +
>> + if (am_unpriv()) {
>> + ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
>> + return -1;
>> + }
>> +
>> // snappath is ${lxcpath}snaps/${lxcname}/
>> dirlen = snprintf(snappath, MAXPATHLEN, "%ssnaps/%s", c->config_path, c->name);
>> if (dirlen < 0 || dirlen >= MAXPATHLEN) {
>> @@ -2802,6 +2847,11 @@ static bool lxcapi_snapshot_restore(struct lxc_container *c, const char *snapnam
>> if (!c || !c->name || !c->config_path)
>> return false;
>>
>> + if (am_unpriv()) {
>> + ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
>> + return false;
>> + }
>> +
>> bdev = bdev_init(c->lxc_conf->rootfs.path, c->lxc_conf->rootfs.mount, NULL);
>> if (!bdev) {
>> ERROR("Failed to find original backing store type");
>> @@ -2851,6 +2901,11 @@ static bool lxcapi_snapshot_destroy(struct lxc_container *c, const char *snapnam
>> if (!c || !c->name || !c->config_path)
>> return false;
>>
>> + if (am_unpriv()) {
>> + ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
>> + return false;
>> + }
>> +
>> ret = snprintf(clonelxcpath, MAXPATHLEN, "%ssnaps/%s", c->config_path, c->name);
>> if (ret < 0 || ret >= MAXPATHLEN)
>> goto err;
>> @@ -2967,11 +3022,19 @@ out:
>>
>> static bool lxcapi_add_device_node(struct lxc_container *c, const char *src_path, const char *dest_path)
>> {
>> + if (am_unpriv()) {
>> + ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
>> + return false;
>> + }
>> return add_remove_device_node(c, src_path, dest_path, true);
>> }
>>
>> static bool lxcapi_remove_device_node(struct lxc_container *c, const char *src_path, const char *dest_path)
>> {
>> + if (am_unpriv()) {
>> + ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
>> + return false;
>> + }
>> return add_remove_device_node(c, src_path, dest_path, false);
>> }
>>
>> @@ -2984,6 +3047,11 @@ static int lxcapi_attach_run_waitl(struct lxc_container *c, lxc_attach_options_t
>> if (!c)
>> return -1;
>>
>> + if (am_unpriv()) {
>> + ERROR(NOT_SUPPORTED_ERROR, __FUNCTION__);
>> + return -1;
>> + }
>> +
>> va_start(ap, arg);
>> argv = lxc_va_arg_list_to_argv_const(ap, 1);
>> va_end(ap);
>> --
>> 1.8.3.2
>>
>> _______________________________________________
>> 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
>
--
S.Çağlar Onur <caglar at 10ur.org>
More information about the lxc-devel
mailing list