On Wed, Jul 03, 2019 at 02:42:30PM +0200, Fabiano Fidêncio wrote:
> On Wed, Jul 3, 2019 at 1:54 PM Pavel Hrdina <phrdina@xxxxxxxxxx> 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@xxxxxxxxxx>
> > > ---
> > > 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, ...).
You can avoid this kind of matching at all but just using a different
Libosinfo.InstallConfig. eg just copy the master config and set the
password params to a sanitized value & generate a new script.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
