<div dir="auto">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.<div dir="auto"> </div><div dir="auto">Of course, I shouldn't be using that weak a password. But people do. </div><div dir="auto"> </div><div dir="auto">Peter</div></div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 3 Jul 2019, 13:43 Fabiano Fidêncio, <<a href="mailto:fidencio@redhat.com">fidencio@redhat.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Wed, Jul 3, 2019 at 1:54 PM Pavel Hrdina <<a href="mailto:phrdina@redhat.com" target="_blank" rel="noreferrer">phrdina@redhat.com</a>> 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 <<a href="mailto:fidencio@redhat.com" target="_blank" rel="noreferrer">fidencio@redhat.com</a>> > > --- > > 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 <a href="mailto:virt-tools-list@redhat.com" target="_blank" rel="noreferrer">virt-tools-list@redhat.com</a> <a href="https://www.redhat.com/mailman/listinfo/virt-tools-list" rel="noreferrer noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/virt-tools-list</a></blockquote></div>