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

[Cole Robinson]

On 7/3/19 10:01 AM, 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 | 26 +++++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/virtinst/install/unattended.py b/virtinst/install/unattended.py
> index ae99bfdb..cf21fc22 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())
> @@ -187,6 +185,26 @@ class OSInstallScript:
>          return self._script.generate_command_line(
>                  self._osobj.get_handle(), self._config)
>  
> +    def _generate_debug(self):
> +        config = Libosinfo.InstallConfig()
> +
> +        config.set_user_login(self._config.get_user_login())
> +        config.set_user_realname(self._config.get_user_realname())
> +        config.set_user_password("[SCRUBBLED]")
> +        config.set_admin_password("[SCRUBBLED]")
> +        config.set_target_disk(self._config.get_target_disk())
> +        config.set_hardware_arch(self._config.get_hardware_arch())
> +        config.set_hostname(self._config.get_hostname())
> +        config.set_l10n_timezone(self._config.get_l10n_timezone())
> +        config.set_l10n_language(self._config.get_l10n_language())
> +        config.set_l10n_keyboard(self._config.get_l10n_keyboard())
> +        if self._config.get_installation_url(): # pylint: disable=no-member
> +            config.set_installation_url(self._config.get_installation_url()) # pylint: disable=no-member
> +        if self._config.get_reg_product_key():
> +            config.set_reg_product_key(self._config.get_reg_product_key())
> +
> +        return self._script.generate(self._osobj.get_handle(), config)
> +

I'm not a fan of this duplication, it means any future config option
will need to be added in two places to get accurate debug output. Not a
priority in the short term to find a better way to do it, it's fine for
this bug fix release

Thanks,
Cole