[CRIU] [PATCH 1/2] security: chown imgs to 0, 0 when creating, and check owner, group and mode when reading

[Pavel Emelyanov]

On 09/13/2014 02:12 PM, Ruslan Kuprieiev wrote:
> Signed-off-by: Ruslan Kuprieiev <kupruser at gmail.com>
> ---
>  image.c           | 12 ++++++++++++
>  include/crtools.h |  1 +
>  security.c        | 43 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 56 insertions(+)
> 
> diff --git a/image.c b/image.c
> index 566073b..9f49887 100644
> --- a/image.c
> +++ b/image.c
> @@ -218,6 +218,18 @@ int open_image_at(int dfd, int type, unsigned long flags, ...)
>  		goto err;
>  	}
>  
> +	if (flags == O_RDONLY) {
> +		if (!check_file_ids(ret)) {
> +			pr_err("User has no rights to open image %s\n", path);
> +			goto err;
> +		}
> +	} else {
> +		if (fchown(ret, 0, 0)) {

This looks strange. Chown is root-only operation. If we're not root,
this will fail, if we are -- this is pointless.

> +			pr_perror("Can't chown image %s to uid 0, gid 0", path);
> +			goto err;
> +		}
> +	}
> +
>  	if (fdset_template[type].magic == RAW_IMAGE_MAGIC)
>  		goto skip_magic;
>  
> diff --git a/include/crtools.h b/include/crtools.h
> index 75047fc..0f73f27 100644
> --- a/include/crtools.h
> +++ b/include/crtools.h
> @@ -29,5 +29,6 @@ struct proc_status_creds;
>  extern bool may_dump(struct proc_status_creds *);
>  struct _CredsEntry;
>  extern bool may_restore(struct _CredsEntry *);
> +extern bool check_file_ids(int fd);
>  
>  #endif /* __CR_CRTOOLS_H__ */
> diff --git a/security.c b/security.c
> index a801005..75086dc 100644
> --- a/security.c
> +++ b/security.c
> @@ -4,6 +4,7 @@
>  #include <limits.h>
>  #include <stdlib.h>
>  #include <string.h>
> +#include <sys/stat.h>
>  
>  #include "crtools.h"
>  #include "proc_parse.h"
> @@ -164,3 +165,45 @@ bool may_restore(CredsEntry *creds)
>  		check_groups(creds->groups, creds->n_groups) &&
>  		check_caps(creds->cap_inh, creds->cap_eff, creds->cap_prm);
>  }
> +
> +static char *mode_str(char *buf, mode_t mode)
> +{
> +	buf[0] = (mode & S_IRUSR) ? 'r' : '-';
> +	buf[1] = (mode & S_IWUSR) ? 'w' : '-';
> +	buf[2] = (mode & S_IXUSR) ? 'x' : '-';
> +	buf[3] = (mode & S_IRGRP) ? 'r' : '-';
> +	buf[4] = (mode & S_IWGRP) ? 'w' : '-';
> +	buf[5] = (mode & S_IXGRP) ? 'x' : '-';
> +	buf[6] = (mode & S_IROTH) ? 'r' : '-';
> +	buf[7] = (mode & S_IWOTH) ? 'w' : '-';
> +	buf[8] = (mode & S_IXOTH) ? 'x' : '-';
> +	buf[9] = '\0';
> +
> +	return buf;
> +}
> +
> +bool check_file_ids(int fd)
> +{
> +	struct stat st;
> +	char buf[10];
> +
> +	if (cr_uid == 0 && cr_gid == 0)
> +		return true;
> +
> +	if (fstat(fd, &st)) {
> +		pr_perror("Can't stat file");
> +		return false;
> +	}
> +
> +	if (!(st.st_mode & CR_FD_PERM)) {
> +		pr_err("File mode %s != %s\n", mode_str(buf, st.st_mode), mode_str(buf, CR_FD_PERM));
> +		return false;
> +	}
> +
> +	if (st.st_uid != 0 || st.st_gid != 0) {
> +		pr_err("File uid/gid (%d,%d) != (0,0)\n", st.st_uid, st.st_gid);
> +		return false;
> +	}
> +
> +	return true;
> +}
>