[virt-tools-list] [virt-manager PATCH 2/2] unattended: Don't log user & admin passwords

[Pavel Hrdina]

On Tue, Jul 02, 2019 at 09:21:45PM +0200, Fabiano Fidêncio wrote:
> Logging user & admin passwords in the command-line is a security issue,
> let's avoid doing so by:
> - Not printing the values set by the user when setting up the
> install-script config file;
> - Removing the values used in the install-scripts, when printing their
> content;
> 
> 'CVE-2019-10183' has been assigned to the virt-install --unattended
> admin-password=xxx disclosure issue.
> 
> Signed-off-by: Fabiano Fidêncio <fidencio at redhat.com>
> ---
>  virtinst/install/unattended.py | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/virtinst/install/unattended.py b/virtinst/install/unattended.py
> index 4f311296..04758563 100644
> --- a/virtinst/install/unattended.py
> +++ b/virtinst/install/unattended.py
> @@ -97,8 +97,6 @@ def _make_installconfig(script, osobj, unattended_data, arch, hostname, url):
>      log.debug("InstallScriptConfig created with the following params:")
>      log.debug("username: %s", config.get_user_login())
>      log.debug("realname: %s", config.get_user_realname())
> -    log.debug("user password: %s", config.get_user_password())
> -    log.debug("admin password: %s", config.get_admin_password())
>      log.debug("target disk: %s", config.get_target_disk())
>      log.debug("hardware arch: %s", config.get_hardware_arch())
>      log.debug("hostname: %s", config.get_hostname())
> @@ -195,6 +193,14 @@ class OSInstallScript:
>          content = self.generate()
>          open(scriptpath, "w").write(content)
>  
> +        user_password = self._config.get_user_password()
> +        if user_password:
> +            content = content.replace(user_password, "[SCRUBBED]")
> +
> +        admin_password = self._config.get_admin_password()
> +        if admin_password:
> +            content = content.replace(admin_password, "[SCRUBBED]")

There is a small issue with this approach, if you for testing purposes
or for any other reason use password that matches anything else in the
script file (kickstart for example) it will be replaced as well:

""""""
%post --erroronfail

useradd -G wheel phrdina # Add user
if [SCRUBBED] -z ''; then
    passwd -d phrdina # Make user account passwordless
else
    echo '' |passwd --stdin phrdina
fi

if [SCRUBBED] -z '[SCRUBBED]'; then
    passwd -d root # Make root account passwordless
else
    echo '[SCRUBBED]' |passwd --stdin root
fi
"""""

Here I used as a password 'test' and you can see the test command was
replaced as well.  Probably not a big deal is it modifies only the log
output, not the actual file but I thought that I should at least point
to this corner case issue.  Do we care about it or not?

Pavel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20190703/1ee980a1/attachment.sig>