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

[Peter Crowther]

It's an information disclosure vulnerability - if I happen to use a
password that matches something in the script, then a diligent reader of
the log file can discern my password.

Of course, I shouldn't be using that weak a password. But people do.

Peter

On Wed, 3 Jul 2019, 13:43 Fabiano Fidêncio, <fidencio at redhat.com> wrote:

> On Wed, Jul 3, 2019 at 1:54 PM Pavel Hrdina <phrdina at redhat.com> wrote:
> >
> > 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?
>
> I thought about that and I sincerely don't care much about that.
> Otherwise we'll have to learn exactly what to match in all the install
> scripts supported (as in, kickstarts, autoyast, jeos, ...).
>
> >
> > Pavel
>
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20190703/0fba892b/attachment.htm>