[virt-tools-list] [virt-bootstrap] [PATCH v3 04/12] set_root_password: Replace chpasswd with script

[Cedric Bosdonnat]

The word 'script' in the title doesn't sound nice and accurate. I would
reinforce the idea that the change is getting rid of root privileges to
change the root password. A title like this would be a better title

"Drop need of root privileges to change root password"

On Thu, 2017-07-20 at 12:29 +0100, Radostin Stoyanov wrote:
> These changes aim to avoid the requirement for root privileges when
> setting the password of root user on root file system.
> 
> The "-R, --root" flag of chpasswd is using chroot to apply changes in
> root file system and this requires root privileges. [1]
> 
> Instead compute hash of the root password using passlib [2] and insert
> the value in the /etc/shadow file in the rootfs.
> 
> [1] https://en.wikipedia.org/wiki/Chroot#Limitations
> [2] http://passlib.readthedocs.io/en/stable/lib/passlib.hosts.html
> ---
>  setup.py                            |  5 +++++
>  src/virtBootstrap/utils.py          | 33 +++++++++++++++++++++++++++++++++
>  src/virtBootstrap/virt_bootstrap.py | 18 +++---------------
>  3 files changed, 41 insertions(+), 15 deletions(-)
> 
> diff --git a/setup.py b/setup.py
> index 70e3e03..b2e17ac 100755
> --- a/setup.py
> +++ b/setup.py
> @@ -112,6 +112,11 @@ setup(
>      cmdclass={
>          'pylint': CheckPylint
>      },
> +
> +    # virt-bootstrap uses passlib to compute the hash of
> +    # root password for root file system.
> +    install_requires=['passlib>=1.7.1'],
> +

Can't that work with older versions of passlib?

>      extras_require={
>          'dev': [
>              'pylint',
> diff --git a/src/virtBootstrap/utils.py b/src/virtBootstrap/utils.py
> index a65d3f5..e1e681c 100644
> --- a/src/virtBootstrap/utils.py
> +++ b/src/virtBootstrap/utils.py
> @@ -32,6 +32,7 @@ import tempfile
>  import logging
>  
>  from subprocess import CalledProcessError, PIPE, Popen
> +import passlib.hosts
>  
>  # pylint: disable=invalid-name
>  # Create logger
> @@ -331,6 +332,38 @@ def str2float(element):
>          return None
>  
>  
> +def set_root_password(rootfs, password):
> +    """
> +    Set password on the root user within root filesystem
> +    """
> +    shadow_file = os.path.join(rootfs, "etc/shadow")
> +
> +    shadow_file_permissions = os.stat(shadow_file)[0]
> +    # Set read-write permissions to shadow file
> +    # 438 = 0110110110 = -rw-rw-rw-
> +    os.chmod(shadow_file, 438)
> +    try:
> +        with open(shadow_file) as orig_file:
> +            shadow_content = orig_file.read().split('\n')
> +
> +        for index, line in enumerate(shadow_content):
> +            if line.startswith('root'):
> +                line_split = line.split(':')
> +                line_split[1] = passlib.hosts.linux_context.hash(password)
> +                shadow_content[index] = ':'.join(line_split)
> +                break
> +
> +        with open(shadow_file, "w") as new_file:
> +            new_file.write('\n'.join(shadow_content))
> +
> +    except Exception:
> +        raise
> +
> +    finally:
> +        # Restore original permissions
> +        os.chmod(shadow_file, shadow_file_permissions)
> +
> +
>  def write_progress(prog):
>      """
>      Write progress output to console
> diff --git a/src/virtBootstrap/virt_bootstrap.py b/src/virtBootstrap/virt_bootstrap.py
> index fe27398..98c629a 100755
> --- a/src/virtBootstrap/virt_bootstrap.py
> +++ b/src/virtBootstrap/virt_bootstrap.py
> @@ -27,7 +27,6 @@ import logging
>  import sys
>  import os
>  from textwrap import dedent
> -from subprocess import CalledProcessError, Popen, PIPE
>  try:
>      from urlparse import urlparse
>  except ImportError:
> @@ -70,18 +69,6 @@ def get_source(source_type):
>          raise Exception("Invalid image URL scheme: '%s'" % source_type)
>  
>  
> -def set_root_password(rootfs, password):
> -    """
> -    Set password on the root user in rootfs
> -    """
> -    users = 'root:%s' % password
> -    args = ['chpasswd', '-R', rootfs]
> -    chpasswd = Popen(args, stdin=PIPE)
> -    chpasswd.communicate(input=users.encode('utf-8'))
> -    if chpasswd.returncode != 0:
> -        raise CalledProcessError(chpasswd.returncode, cmd=args, output=None)
> -
> -
>  # pylint: disable=too-many-arguments
>  def bootstrap(uri, dest,
>                fmt='dir',
> @@ -117,8 +104,9 @@ def bootstrap(uri, dest,
>             no_cache=no_cache,
>             progress=prog).unpack(dest)
>  
> -    if root_password is not None:
> -        set_root_password(dest, root_password)
> +    if fmt == "dir" and root_password is not None:
> +        logger.info("Setting password of the root account")
> +        utils.set_root_password(dest, root_password)

That reminds me that we'll need to handle the qcow2 case as well.


ACK with the new title.

--
Cedric

>  
>  
>  def set_logging_conf(loglevel=None):