[Devel] [PATCH v4 2/7] add user mismatch test

[Kir Kolyshkin]

On 05/14/2013 03:52 AM, Glauber Costa wrote:
> From: Glauber Costa <glommer at parallels.com>
>
> In theory, we won't be able to run if our private area is not owned by
> ourselves.  We could, if it have very wide open security permissions, but we
> should never set up a container like that.
>
> Aside from a basic sanity check, this is intended to catch problems for the few
> people who may have already created containers that will be owned by root:root,
> and will now try to run it unprivileged.
>
> Signed-off-by: Glauber Costa <glommer at parallels.com>
> ---
>   src/lib/env.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
>
> diff --git a/src/lib/env.c b/src/lib/env.c
> index 898bfc8..96400c0 100644
> --- a/src/lib/env.c
> +++ b/src/lib/env.c
> @@ -30,6 +30,7 @@
>   #include <linux/reboot.h>
>   #include <sys/mount.h>
>   #include <sys/utsname.h>
> +#include <sys/stat.h>
>   
>   #include "vzerror.h"
>   #include "res.h"
> @@ -554,6 +555,21 @@ int vps_start_custom(vps_handler *h, envid_t veid, vps_param *param,
>   		logger(-1, 0, "Container is already running");
>   		return VZ_VE_RUNNING;
>   	}
> +	if (!is_vz_kernel(h) && h->can_join_userns) {
> +		struct stat private_stat;
> +		unsigned long *local_uid = res->misc.local_uid;
> +		unsigned long *local_gid = res->misc.local_gid;
> +
> +		stat(res->fs.private, &private_stat);
> +		if ((local_uid && (private_stat.st_uid != *local_uid)) ||
> +			(local_gid && (private_stat.st_gid != *local_gid))) {

As I just commented at the very end of a previous patch,
indeed it does make sense to check for local_uid and local_gid being 
not-NULL,
since here you dereference both without checking (relying on can_join_userns
which only checks local_uid but not local_gid).

So that comment was right (and taking it into account this code is correct).

> +			logger(-1, 0, "Container private area is owned by %d:%d"
> +			", but configuration file says we should run with %lu:%lu.\n"
> +			"Refusing to run.", private_stat.st_uid, private_stat.st_gid,
> +			*res->misc.local_uid, *res->misc.local_gid);
> +			return VZ_FS_BAD_TMPL;
> +		}
> +	}
>   	if ((ret = check_ub(h, &res->ub)))
>   		return ret;
>