[virt-tools-list] [virt-bootstrap PATCH 3/3] Set SElinux file context on destination folder

[Cole Robinson]

On 6/4/19 6:26 AM, Radostin Stoyanov wrote:
> Set file context to 'container_file_t' on the destination folder when
> SElinux mode is set to enforcing.
> 

I guess this is RH selinux specific but that's probably a theoretical
issue.

> Signed-off-by: Radostin Stoyanov <rstoyanov1 at gmail.com>
> ---
>  src/virtBootstrap/utils.py          | 28 ++++++++++++++++++++++++++++
>  src/virtBootstrap/virt_bootstrap.py |  7 +++++++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/src/virtBootstrap/utils.py b/src/virtBootstrap/utils.py
> index b0f695d..3083c3c 100644
> --- a/src/virtBootstrap/utils.py
> +++ b/src/virtBootstrap/utils.py
> @@ -522,6 +522,34 @@ def write_progress(prog):
>      sys.stdout.flush()
>  
>  
> +def is_selinux_enforcing():
> +    """
> +    Check if SElinux mode is set to enforcing.
> +    """
> +    try:
> +        status = subprocess.Popen(
> +            ['getenforce'],
> +            stdout=subprocess.PIPE).stdout.read()
> +        status = status.decode('utf-8').strip().lower()
> +    except Exception:
> +        return False
> +
> +    if "enforcing" in status:
> +        return True
> +    return False
> +
> +
> +def chcon(path, context, flags="-Rt"):
> +    """
> +    Change file SELinux security context
> +    """
> +    try:
> +        subprocess.check_call(['chcon', flags, context, path])
> +    except Exception:
> +        return False
> +    return True
> +
> +
>  # The implementation for remapping ownership of all files inside a
>  # container's rootfs is inspired by the tool uidmapshift:
>  #
> diff --git a/src/virtBootstrap/virt_bootstrap.py b/src/virtBootstrap/virt_bootstrap.py
> index cfe7aab..7b5a9d0 100755
> --- a/src/virtBootstrap/virt_bootstrap.py
> +++ b/src/virtBootstrap/virt_bootstrap.py
> @@ -135,6 +135,13 @@ def bootstrap(uri, dest,
>          logger.error("No write permissions on destination path '%s'", dest)
>          sys.exit(1)
>  
> +    if utils.is_selinux_enforcing():
> +        logger.debug("SElinux is set to enforcing")
> +        if not utils.chcon(dest, "container_file_t"):
> +            logger.error("Can't set SElinux context on destination path '%s'",
> +                         dest)
> +            sys.exit(1)
> +

Just some thoughts: Maybe just let subprocess.check_call raise the error
if it fails, otherwise we may be throwing away an informative error
message. Or chcon could log the exception at debug level or similar.

I'll leave it up to you whether you want to change it. Either way:

Reviewed-by: Cole Robinson <crobinso at redhat.com>

Thanks,
Cole