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): _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
