[virt-tools-list] ANNOUNCE: virt-manager 2.2.1 released
I'm happy to announce the release of virt-manager 2.2.1! virt-manager is a desktop application for managing KVM, Xen, and LXC virtualization via libvirt. The release can be downloaded from: http://virt-manager.org/download/ This release includes: - CVE-2019-10183: Replace --unattended user-password and admin-password with user-password-file and admin-password-file (Fabiano Fidêncio) - Consistent --memballoon default across non-x86 (Andrea Bolognani) - virt-install: add --numatune memnode.* (Athina Plaskasoviti) - Drop hard dep on gtksourceview4, gtksourceview3 is fine as well Thanks to everyone who has contributed to this release through testing, bug reporting, submitting patches, and otherwise sending in feedback! Thanks, Cole ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH v2 0/2] unattended: Don't expose user & admin passwords
On Wed, Jul 03, 2019 at 01:32:59PM -0400, Cole Robinson wrote:
> On 7/3/19 10:01 AM, Fabiano Fidêncio wrote:
> > Let's not expose user & admin passwords neither by having an option to
> > be used to set those passwords nor in the debug messages.
> >
> > 'CVE-2019-10183' has been assigned to the virt-install --unattended
> > admin-password=xxx disclosure issue.
> >
> > Changes since v1:
> > https://www.redhat.com/archives/virt-tools-list/2019-July/msg00013.html
> > - passowrd -> password;
> > - pwd.read().rstrip("\n\r") -> pwd.readline().rstrip("\n\r") + document
> > this in our manpage;
> > - create a new config, with the sanitised password, and use it to print
> > the script content as a debug message;
> >
> > Fabiano Fidêncio (2):
> > unattended: Read the passwords from a file
> > unattended: Don't log user & admin passwords
> >
> > man/virt-install.pod | 24
> > tests/cli-test-xml/admin-password.txt | 1 +
> > tests/cli-test-xml/user-password.txt | 3 ++
> > tests/clitest.py | 18 +
> > virtinst/cli.py | 4 +-
> > virtinst/install/unattended.py| 56 ---
> > 6 files changed, 76 insertions(+), 30 deletions(-)
> > create mode 100644 tests/cli-test-xml/admin-password.txt
> > create mode 100644 tests/cli-test-xml/user-password.txt
> >
>
> Fixed some pylint warnings and pushed
Thanks for pushing it, I was about to do the same but had to leave
office.
Pavel
signature.asc
Description: PGP signature
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 2/2] unattended: Don't log user & admin passwords
On 7/3/19 2:16 PM, Peter Crowther wrote: > 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. > The pushed patch fixed this issue Thanks, Cole ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 2/2] unattended: Don't log user & admin passwords
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, wrote:
> On Wed, Jul 3, 2019 at 1:54 PM Pavel Hrdina 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
> > > ---
> > > 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@redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH] cloudinit: Add disable=yes|no
On 7/3/19 2:06 AM, Athina Plaskasoviti wrote:
> Cli option to permanently disable cloud-init after first boot by user request.
> Handled so that bare --cloud-init defaults to --cloud-init
> root-password=generate,disable=yes.
>
Text here is quite long. Please keep lines to max 72 chars, that's the
git convention
> Signed-off-by: Athina Plaskasoviti
> ---
> virtinst/cli.py | 4 +++-
> virtinst/install/cloudinit.py | 7 +--
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/virtinst/cli.py b/virtinst/cli.py
> index caa12289..c080dfcf 100644
> --- a/virtinst/cli.py
> +++ b/virtinst/cli.py
> @@ -1614,14 +1614,16 @@ class ParserCloudInit(VirtCLIParser):
> def _init_class(cls, **kwargs):
> VirtCLIParser._init_class(**kwargs)
> cls.add_arg("root-password", "root_password")
> +cls.add_arg("disable", "disable", is_onoff=True)
>
>
> def parse_cloud_init(optstr):
> ret = CloudInitData()
> if optstr == 1:
> # This means bare --cloud-init, so there's nothing to parse.
> -log.warning("Defaulting to --cloud-init root-password=generate")
> +log.warning("Defaulting to --cloud-init
> root-password=generate,disable=yes")
> ret.root_password = "generate"
> +ret.disable = True
> return ret
>
> parser = ParserCloudInit(optstr)
> diff --git a/virtinst/install/cloudinit.py b/virtinst/install/cloudinit.py
> index 8041cc86..032d65e4 100644
> --- a/virtinst/install/cloudinit.py
> +++ b/virtinst/install/cloudinit.py
> @@ -6,6 +6,7 @@ from ..logger import log
>
>
> class CloudInitData():
> +disable = None
> root_password = None
>
>
> @@ -40,8 +41,10 @@ def create_userdata(scratchdir, cloudinit_data):
> content += "root:%s\n" % rootpass
> content += " expire: True\n"
>
> -content += "runcmd:\n"
> -content += "- [ sudo, touch, /etc/cloud/cloud-init.disabled ]\n"
> +disable = cloudinit_data.disable
> +if disable is True:
> +content += "runcmd:\n"
> +content += "- [ sudo, touch, /etc/cloud/cloud-init.disabled ]\n"
This can be simplified to just:
if cloudinit_data.disable:
I fixed that and pushed it to the cloudinit branch
Thanks,
Cole
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH] cloudinit: Clean up create_metadata and create_userdata functions from unused/unwanted arguments
On 7/3/19 1:37 AM, Athina Plaskasoviti wrote:
> Signed-off-by: Athina Plaskasoviti
> ---
> virtinst/install/cloudinit.py | 17 -
> 1 file changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/virtinst/install/cloudinit.py b/virtinst/install/cloudinit.py
> index 3cb13484..8041cc86 100644
> --- a/virtinst/install/cloudinit.py
> +++ b/virtinst/install/cloudinit.py
> @@ -9,31 +9,22 @@ class CloudInitData():
> root_password = None
>
>
> -def create_metadata(scratchdir, hostname=None):
> -if hostname:
> -instance = hostname
> -else:
> -hostname = instance = "localhost"
> -content = 'instance-id: %s\n' % instance
> -content += 'hostname: %s\n' % hostname
> -log.debug("Generated cloud-init metadata:\n%s", content)
> +def create_metadata(scratchdir):
>
I dropped the newline here, and pushed this to the cloudinit branch
Thanks,
Cole
> fileobj = tempfile.NamedTemporaryFile(
> prefix="virtinst-", suffix="-metadata",
> dir=scratchdir, delete=False)
> filename = fileobj.name
>
> +content = ""
> with open(filename, "w") as f:
> f.write(content)
> +log.debug("Generated cloud-init metadata\n%s", content)
> return filename
>
>
> -def create_userdata(scratchdir, cloudinit_data, username=None,
> password=None):
> +def create_userdata(scratchdir, cloudinit_data):
> content = "#cloud-config\n"
> -if username:
> -content += "name: %s\n" % username
> -if password:
> -content += "password: %s\n" % password
>
> rootpass = cloudinit_data.root_password
> if rootpass == "generate":
>
- Cole
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH v2 2/2] unattended: Don't log user & admin passwords
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
> ---
> 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
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH v2 0/2] unattended: Don't expose user & admin passwords
On 7/3/19 10:01 AM, Fabiano Fidêncio wrote:
> Let's not expose user & admin passwords neither by having an option to
> be used to set those passwords nor in the debug messages.
>
> 'CVE-2019-10183' has been assigned to the virt-install --unattended
> admin-password=xxx disclosure issue.
>
> Changes since v1:
> https://www.redhat.com/archives/virt-tools-list/2019-July/msg00013.html
> - passowrd -> password;
> - pwd.read().rstrip("\n\r") -> pwd.readline().rstrip("\n\r") + document
> this in our manpage;
> - create a new config, with the sanitised password, and use it to print
> the script content as a debug message;
>
> Fabiano Fidêncio (2):
> unattended: Read the passwords from a file
> unattended: Don't log user & admin passwords
>
> man/virt-install.pod | 24
> tests/cli-test-xml/admin-password.txt | 1 +
> tests/cli-test-xml/user-password.txt | 3 ++
> tests/clitest.py | 18 +
> virtinst/cli.py | 4 +-
> virtinst/install/unattended.py| 56 ---
> 6 files changed, 76 insertions(+), 30 deletions(-)
> create mode 100644 tests/cli-test-xml/admin-password.txt
> create mode 100644 tests/cli-test-xml/user-password.txt
>
Fixed some pylint warnings and pushed
Thanks,
Cole
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-manager PATCH v2 2/2] unattended: Don't log user & admin passwords
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
---
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)
+
def write(self):
fileobj = tempfile.NamedTemporaryFile(
prefix="virtinst-unattended-script", delete=False)
@@ -195,8 +213,10 @@ class OSInstallScript:
content = self.generate()
open(scriptpath, "w").write(content)
+debug_content = self._generate_debug()
+
log.debug("Generated unattended script: %s", scriptpath)
-log.debug("Generated script contents:\n%s", content)
+log.debug("Generated script contents:\n%s", debug_content)
return scriptpath
--
2.21.0
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-manager PATCH v2 0/2] unattended: Don't expose user & admin passwords
Let's not expose user & admin passwords neither by having an option to
be used to set those passwords nor in the debug messages.
'CVE-2019-10183' has been assigned to the virt-install --unattended
admin-password=xxx disclosure issue.
Changes since v1:
https://www.redhat.com/archives/virt-tools-list/2019-July/msg00013.html
- passowrd -> password;
- pwd.read().rstrip("\n\r") -> pwd.readline().rstrip("\n\r") + document
this in our manpage;
- create a new config, with the sanitised password, and use it to print
the script content as a debug message;
Fabiano Fidêncio (2):
unattended: Read the passwords from a file
unattended: Don't log user & admin passwords
man/virt-install.pod | 24
tests/cli-test-xml/admin-password.txt | 1 +
tests/cli-test-xml/user-password.txt | 3 ++
tests/clitest.py | 18 +
virtinst/cli.py | 4 +-
virtinst/install/unattended.py| 56 ---
6 files changed, 76 insertions(+), 30 deletions(-)
create mode 100644 tests/cli-test-xml/admin-password.txt
create mode 100644 tests/cli-test-xml/user-password.txt
--
2.21.0
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-manager PATCH v2 1/2] unattended: Read the passwords from a file
Let's not expose the user/root password in the CLI and, instead, let's
rely on a file passed by the admin and read the password from there.
'CVE-2019-10183' has been assigned to the virt-install --unattended
admin-password=xxx disclosure issue.
Signed-off-by: Fabiano Fidêncio
---
man/virt-install.pod | 24 ++---
tests/cli-test-xml/admin-password.txt | 1 +
tests/cli-test-xml/user-password.txt | 3 +++
tests/clitest.py | 18 +---
virtinst/cli.py | 4 ++--
virtinst/install/unattended.py| 30 ++-
6 files changed, 53 insertions(+), 27 deletions(-)
create mode 100644 tests/cli-test-xml/admin-password.txt
create mode 100644 tests/cli-test-xml/user-password.txt
diff --git a/man/virt-install.pod b/man/virt-install.pod
index d8bd4127..081f28c3 100644
--- a/man/virt-install.pod
+++ b/man/virt-install.pod
@@ -612,13 +612,23 @@ Choose which libosinfo unattended profile to use. Most
distros have
a 'desktop' and a 'jeos' profile. virt-install will default to 'desktop'
if this is unspecified.
-=item B
-
-Set the VM OS admin/root password
-
-=item B
-
-Set the VM user password. The username is your current host username
+=item B
+
+A file used to set the VM OS admin/root password from. This option can
+be used either as "admin-password-file=/path/to/password-file" or as
+"admin-password-file=/dev/fd/n", being n the file descriptor of the
+password-file.
+Note that only the first line of the file will be considered, including
+any whitespace characters and excluding new-line.
+
+=item B
+
+A file used to set the VM user password. This option can be used either as
+"user-password-file=/path/to/password-file" or as
+"user-password-file=/dev/fd/n", being n the file descriptor of the
+password-file. The username is your current host username.
+Note that only the first line of the file will be considered, including
+any whitespace characters and excluding new-line.
=item B
diff --git a/tests/cli-test-xml/admin-password.txt
b/tests/cli-test-xml/admin-password.txt
new file mode 100644
index ..323fae03
--- /dev/null
+++ b/tests/cli-test-xml/admin-password.txt
@@ -0,0 +1 @@
+foobar
diff --git a/tests/cli-test-xml/user-password.txt
b/tests/cli-test-xml/user-password.txt
new file mode 100644
index ..625999ba
--- /dev/null
+++ b/tests/cli-test-xml/user-password.txt
@@ -0,0 +1,3 @@
+blah
+
+
diff --git a/tests/clitest.py b/tests/clitest.py
index 1165baed..2618e04b 100644
--- a/tests/clitest.py
+++ b/tests/clitest.py
@@ -9,6 +9,7 @@ import os
import shlex
import shutil
import sys
+import tempfile
import traceback
import unittest
@@ -89,6 +90,8 @@ test_files = {
'ISO-F29-LIVE': iso_links[5],
'TREEDIR': "%s/fakefedoratree" % XMLDIR,
'COLLIDE': "/dev/default-pool/collidevol1.img",
+'ADMIN-PASSWORD-FILE': "%s/admin-password.txt" % XMLDIR,
+'USER-PASSWORD-FILE': "%s/user-password.txt" % XMLDIR,
}
@@ -872,22 +875,21 @@ c.add_valid("--connect %s --pxe --disk size=1" %
utils.URIs.test_defaultpool_col
c = vinst.add_category("unattended-install", "--connect %(URI-KVM)s
--nographics --noautoconsole --disk none", prerun_check=no_osinfo_unattend_cb)
-c.add_compare("--install fedora26 --unattended
profile=desktop,admin-password=foobar,user-password=blah,product-key=1234",
"osinfo-url-unattended") # unattended install for fedora, using initrd
injection
-c.add_compare("--cdrom %(ISO-WIN7)s --unattended
profile=desktop,admin-password=foobar", "osinfo-win7-unattended") # unattended
install for win7
-c.add_compare("--os-variant fedora26 --unattended
profile=jeos,admin-password=123456 --location %(ISO-F26-NETINST)s",
"osinfo-netinst-unattended") # triggering the special netinst checking code
+c.add_compare("--install fedora26 --unattended
profile=desktop,admin-password-file=%(ADMIN-PASSWORD-FILE)s,user-password-file=%(USER-PASSWORD-FILE)s,product-key=1234",
"osinfo-url-unattended") # unattended install for fedora, using initrd
injection
+c.add_compare("--cdrom %(ISO-WIN7)s --unattended
profile=desktop,admin-password-file=%(ADMIN-PASSWORD-FILE)s",
"osinfo-win7-unattended") # unattended install for win7
+c.add_compare("--os-variant fedora26 --unattended
profile=jeos,admin-password-file=%(ADMIN-PASSWORD-FILE)s --location
%(ISO-F26-NETINST)s", "osinfo-netinst-unattended") # triggering the special
netinst checking code
c.add_compare("--os-variant silverblue29 --location http://example.com;,
"network-install-resources") # triggering network-install resources override
c.add_valid("--pxe --os-variant fedora26 --unattended", grep="Using unattended
profile 'desktop'") # filling in default 'desktop' profile
c.add_invalid("--os-variant fedora26 --unattended profile=jeos --location
http://example.foo;, grep="admin-password") # will trigger admin-password
required error
c.add_invalid("--os-variant fedora26
Re: [virt-tools-list] [virt-manager PATCH 2/2] unattended: Don't log user & admin passwords
On 7/3/19 8:42 AM, Fabiano Fidêncio wrote:
> On Wed, Jul 3, 2019 at 1:54 PM Pavel Hrdina 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
>>> ---
>>> 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, ...).
I'm not much worried about it either. But presumably we could overwrite
the admin/user pass in the config object with [SCRUBBED], generate the
output, log it, fix the config object and generate the real output and
save. But it's not a requirement IMO
- Cole
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 2/2] unattended: Don't log user & admin passwords
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 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
> > > ---
> > > 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@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 2/2] unattended: Don't log user & admin passwords
On Wed, Jul 3, 2019 at 1:54 PM Pavel Hrdina 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
> > ---
> > 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@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 2/2] unattended: Don't log user & admin passwords
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
> ---
> 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
signature.asc
Description: PGP signature
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 1/2] unattended: Read the passwords from a file
On Wed, Jul 03, 2019 at 12:20:23PM +0200, Fabiano Fidêncio wrote:
> On Wed, Jul 3, 2019 at 10:53 AM Pavel Hrdina wrote:
> >
> > On Tue, Jul 02, 2019 at 09:21:44PM +0200, Fabiano Fidêncio wrote:
> > > Let's not expose the user/root password in the CLI and, instead, let's
> > > rely on a file passed by the admin and read the password from there.
> > >
> > > 'CVE-2019-10183' has been assigned to the virt-install --unattended
> > > admin-password=xxx disclosure issue.
> > >
> > > Signed-off-by: Fabiano Fidêncio
> > > ---
> > > man/virt-install.pod | 14 ++
> > > tests/cli-test-xml/admin-password.txt | 1 +
> > > tests/cli-test-xml/user-password.txt | 3 +++
> > > tests/clitest.py | 18 +
> > > virtinst/cli.py | 4 ++--
> > > virtinst/install/unattended.py| 28 +--
> > > 6 files changed, 44 insertions(+), 24 deletions(-)
> > > create mode 100644 tests/cli-test-xml/admin-password.txt
> > > create mode 100644 tests/cli-test-xml/user-password.txt
> > >
> > > diff --git a/man/virt-install.pod b/man/virt-install.pod
> > > index d8bd4127..0e655fef 100644
> > > --- a/man/virt-install.pod
> > > +++ b/man/virt-install.pod
> > > @@ -612,13 +612,19 @@ Choose which libosinfo unattended profile to use.
> > > Most distros have
> > > a 'desktop' and a 'jeos' profile. virt-install will default to 'desktop'
> > > if this is unspecified.
> > >
> > > -=item B
> > > +=item B
> > >
> > > -Set the VM OS admin/root password
> > > +A file used to set the VM OS admin/root password from. This option can
> > > +be used either as "admin-password-file=/path/to/passowrd-file" or as
> > > +"admin-password-file=/dev/fd/n", being n the file descriptor of the
> > > +password-file.
> >
> > Typo fixed in my local branch.
> >
> > > -=item B
> > > +=item B
> > >
> > > -Set the VM user password. The username is your current host username
> > > +A file used to set the VM user password. This option can be used either
> > > as
> > > +"user-password-file=/path/to/password-file" or as
> > > +"user-password-file=/dev/fd/n", being n the file descriptor of the
> > > +password-file. The username is your current host username.
> > >
> > > =item B
> > >
> > > diff --git a/tests/cli-test-xml/admin-password.txt
> > > b/tests/cli-test-xml/admin-password.txt
> > > new file mode 100644
> > > index ..323fae03
> > > --- /dev/null
> > > +++ b/tests/cli-test-xml/admin-password.txt
> > > @@ -0,0 +1 @@
> > > +foobar
> > > diff --git a/tests/cli-test-xml/user-password.txt
> > > b/tests/cli-test-xml/user-password.txt
> > > new file mode 100644
> > > index ..625999ba
> > > --- /dev/null
> > > +++ b/tests/cli-test-xml/user-password.txt
> > > @@ -0,0 +1,3 @@
> > > +blah
> > > +
> > > +
> >
> > Are these random white spaces intentional? There are two lines and
> > both with different number of white spaces. If not I can fix it before
> > pushing.
>
> Those are intentional ...
>
> >
> > [...]
> >
> > > diff --git a/virtinst/install/unattended.py
> > > b/virtinst/install/unattended.py
> > > index ea3f9066..4f311296 100644
> > > --- a/virtinst/install/unattended.py
> > > +++ b/virtinst/install/unattended.py
> > > @@ -39,23 +39,21 @@ def _make_installconfig(script, osobj,
> > > unattended_data, arch, hostname, url):
> > >
> > > # Set user-password.
> > > # In case it's required and not passed, just raise a RuntimeError.
> > > -if script.requires_user_password() and not
> > > unattended_data.user_password:
> > > +if (script.requires_user_password() and
> > > +not unattended_data.get_user_password()):
> > > raise RuntimeError(
> > > _("%s requires the user-password to be set.") %
> > > osobj.name)
> > > -config.set_user_password(
> > > -unattended_data.user_password if unattended_data.user_password
> > > -else "")
> > > +config.set_user_password(unattended_data.get_user_password() or "")
> > >
> > > # Set the admin-password:
> > > # In case it's required and not passed, just raise a RuntimeError.
> > > -if script.requires_admin_password() and not
> > > unattended_data.admin_password:
> > > +if (script.requires_admin_password() and
> > > +not unattended_data.get_admin_password()):
> > > raise RuntimeError(
> > > _("%s requires the admin-password to be set.") %
> > > osobj.name)
> > > -config.set_admin_password(
> > > -unattended_data.admin_password if unattended_data.admin_password
> > > -else "")
> > > +config.set_admin_password(unattended_data.get_admin_password() or "")
> > >
> > > # Set the target disk.
> > > # virtiodisk is the preferred way, in case it's supported, otherwise
> > > @@ -205,10 +203,20 @@ class OSInstallScript:
> > >
> > > class UnattendedData():
> > > profile = None
> > > -admin_password = None
> > > -user_password
Re: [virt-tools-list] [virt-manager PATCH 1/2] unattended: Read the passwords from a file
On Wed, Jul 3, 2019 at 10:53 AM Pavel Hrdina wrote:
>
> On Tue, Jul 02, 2019 at 09:21:44PM +0200, Fabiano Fidêncio wrote:
> > Let's not expose the user/root password in the CLI and, instead, let's
> > rely on a file passed by the admin and read the password from there.
> >
> > 'CVE-2019-10183' has been assigned to the virt-install --unattended
> > admin-password=xxx disclosure issue.
> >
> > Signed-off-by: Fabiano Fidêncio
> > ---
> > man/virt-install.pod | 14 ++
> > tests/cli-test-xml/admin-password.txt | 1 +
> > tests/cli-test-xml/user-password.txt | 3 +++
> > tests/clitest.py | 18 +
> > virtinst/cli.py | 4 ++--
> > virtinst/install/unattended.py| 28 +--
> > 6 files changed, 44 insertions(+), 24 deletions(-)
> > create mode 100644 tests/cli-test-xml/admin-password.txt
> > create mode 100644 tests/cli-test-xml/user-password.txt
> >
> > diff --git a/man/virt-install.pod b/man/virt-install.pod
> > index d8bd4127..0e655fef 100644
> > --- a/man/virt-install.pod
> > +++ b/man/virt-install.pod
> > @@ -612,13 +612,19 @@ Choose which libosinfo unattended profile to use.
> > Most distros have
> > a 'desktop' and a 'jeos' profile. virt-install will default to 'desktop'
> > if this is unspecified.
> >
> > -=item B
> > +=item B
> >
> > -Set the VM OS admin/root password
> > +A file used to set the VM OS admin/root password from. This option can
> > +be used either as "admin-password-file=/path/to/passowrd-file" or as
> > +"admin-password-file=/dev/fd/n", being n the file descriptor of the
> > +password-file.
>
> Typo fixed in my local branch.
>
> > -=item B
> > +=item B
> >
> > -Set the VM user password. The username is your current host username
> > +A file used to set the VM user password. This option can be used either as
> > +"user-password-file=/path/to/password-file" or as
> > +"user-password-file=/dev/fd/n", being n the file descriptor of the
> > +password-file. The username is your current host username.
> >
> > =item B
> >
> > diff --git a/tests/cli-test-xml/admin-password.txt
> > b/tests/cli-test-xml/admin-password.txt
> > new file mode 100644
> > index ..323fae03
> > --- /dev/null
> > +++ b/tests/cli-test-xml/admin-password.txt
> > @@ -0,0 +1 @@
> > +foobar
> > diff --git a/tests/cli-test-xml/user-password.txt
> > b/tests/cli-test-xml/user-password.txt
> > new file mode 100644
> > index ..625999ba
> > --- /dev/null
> > +++ b/tests/cli-test-xml/user-password.txt
> > @@ -0,0 +1,3 @@
> > +blah
> > +
> > +
>
> Are these random white spaces intentional? There are two lines and
> both with different number of white spaces. If not I can fix it before
> pushing.
Those are intentional ...
>
> [...]
>
> > diff --git a/virtinst/install/unattended.py b/virtinst/install/unattended.py
> > index ea3f9066..4f311296 100644
> > --- a/virtinst/install/unattended.py
> > +++ b/virtinst/install/unattended.py
> > @@ -39,23 +39,21 @@ def _make_installconfig(script, osobj, unattended_data,
> > arch, hostname, url):
> >
> > # Set user-password.
> > # In case it's required and not passed, just raise a RuntimeError.
> > -if script.requires_user_password() and not
> > unattended_data.user_password:
> > +if (script.requires_user_password() and
> > +not unattended_data.get_user_password()):
> > raise RuntimeError(
> > _("%s requires the user-password to be set.") %
> > osobj.name)
> > -config.set_user_password(
> > -unattended_data.user_password if unattended_data.user_password
> > -else "")
> > +config.set_user_password(unattended_data.get_user_password() or "")
> >
> > # Set the admin-password:
> > # In case it's required and not passed, just raise a RuntimeError.
> > -if script.requires_admin_password() and not
> > unattended_data.admin_password:
> > +if (script.requires_admin_password() and
> > +not unattended_data.get_admin_password()):
> > raise RuntimeError(
> > _("%s requires the admin-password to be set.") %
> > osobj.name)
> > -config.set_admin_password(
> > -unattended_data.admin_password if unattended_data.admin_password
> > -else "")
> > +config.set_admin_password(unattended_data.get_admin_password() or "")
> >
> > # Set the target disk.
> > # virtiodisk is the preferred way, in case it's supported, otherwise
> > @@ -205,10 +203,20 @@ class OSInstallScript:
> >
> > class UnattendedData():
> > profile = None
> > -admin_password = None
> > -user_password = None
> > +admin_password_file = None
> > +user_password_file = None
> > product_key = None
> >
> > +def get_user_password(self):
> > +if self.user_password_file:
> > +with open(self.user_password_file) as pwd:
> > +return pwd.read().rstrip("\n\r")
> > +
> > +def
Re: [virt-tools-list] [virt-manager PATCH 1/2] unattended: Read the passwords from a file
On Tue, Jul 02, 2019 at 09:21:44PM +0200, Fabiano Fidêncio wrote:
> Let's not expose the user/root password in the CLI and, instead, let's
> rely on a file passed by the admin and read the password from there.
>
> 'CVE-2019-10183' has been assigned to the virt-install --unattended
> admin-password=xxx disclosure issue.
>
> Signed-off-by: Fabiano Fidêncio
> ---
> man/virt-install.pod | 14 ++
> tests/cli-test-xml/admin-password.txt | 1 +
> tests/cli-test-xml/user-password.txt | 3 +++
> tests/clitest.py | 18 +
> virtinst/cli.py | 4 ++--
> virtinst/install/unattended.py| 28 +--
> 6 files changed, 44 insertions(+), 24 deletions(-)
> create mode 100644 tests/cli-test-xml/admin-password.txt
> create mode 100644 tests/cli-test-xml/user-password.txt
>
> diff --git a/man/virt-install.pod b/man/virt-install.pod
> index d8bd4127..0e655fef 100644
> --- a/man/virt-install.pod
> +++ b/man/virt-install.pod
> @@ -612,13 +612,19 @@ Choose which libosinfo unattended profile to use. Most
> distros have
> a 'desktop' and a 'jeos' profile. virt-install will default to 'desktop'
> if this is unspecified.
>
> -=item B
> +=item B
>
> -Set the VM OS admin/root password
> +A file used to set the VM OS admin/root password from. This option can
> +be used either as "admin-password-file=/path/to/passowrd-file" or as
> +"admin-password-file=/dev/fd/n", being n the file descriptor of the
> +password-file.
Typo fixed in my local branch.
> -=item B
> +=item B
>
> -Set the VM user password. The username is your current host username
> +A file used to set the VM user password. This option can be used either as
> +"user-password-file=/path/to/password-file" or as
> +"user-password-file=/dev/fd/n", being n the file descriptor of the
> +password-file. The username is your current host username.
>
> =item B
>
> diff --git a/tests/cli-test-xml/admin-password.txt
> b/tests/cli-test-xml/admin-password.txt
> new file mode 100644
> index ..323fae03
> --- /dev/null
> +++ b/tests/cli-test-xml/admin-password.txt
> @@ -0,0 +1 @@
> +foobar
> diff --git a/tests/cli-test-xml/user-password.txt
> b/tests/cli-test-xml/user-password.txt
> new file mode 100644
> index ..625999ba
> --- /dev/null
> +++ b/tests/cli-test-xml/user-password.txt
> @@ -0,0 +1,3 @@
> +blah
> +
> +
Are these random white spaces intentional? There are two lines and
both with different number of white spaces. If not I can fix it before
pushing.
[...]
> diff --git a/virtinst/install/unattended.py b/virtinst/install/unattended.py
> index ea3f9066..4f311296 100644
> --- a/virtinst/install/unattended.py
> +++ b/virtinst/install/unattended.py
> @@ -39,23 +39,21 @@ def _make_installconfig(script, osobj, unattended_data,
> arch, hostname, url):
>
> # Set user-password.
> # In case it's required and not passed, just raise a RuntimeError.
> -if script.requires_user_password() and not unattended_data.user_password:
> +if (script.requires_user_password() and
> +not unattended_data.get_user_password()):
> raise RuntimeError(
> _("%s requires the user-password to be set.") %
> osobj.name)
> -config.set_user_password(
> -unattended_data.user_password if unattended_data.user_password
> -else "")
> +config.set_user_password(unattended_data.get_user_password() or "")
>
> # Set the admin-password:
> # In case it's required and not passed, just raise a RuntimeError.
> -if script.requires_admin_password() and not
> unattended_data.admin_password:
> +if (script.requires_admin_password() and
> +not unattended_data.get_admin_password()):
> raise RuntimeError(
> _("%s requires the admin-password to be set.") %
> osobj.name)
> -config.set_admin_password(
> -unattended_data.admin_password if unattended_data.admin_password
> -else "")
> +config.set_admin_password(unattended_data.get_admin_password() or "")
>
> # Set the target disk.
> # virtiodisk is the preferred way, in case it's supported, otherwise
> @@ -205,10 +203,20 @@ class OSInstallScript:
>
> class UnattendedData():
> profile = None
> -admin_password = None
> -user_password = None
> +admin_password_file = None
> +user_password_file = None
> product_key = None
>
> +def get_user_password(self):
> +if self.user_password_file:
> +with open(self.user_password_file) as pwd:
> +return pwd.read().rstrip("\n\r")
> +
> +def get_admin_password(self):
> +if self.admin_password_file:
> +with open(self.admin_password_file) as pwd:
> +return pwd.read().rstrip("\n\r")
> +
I guess this is related to my question above, we will strip trailing
lines, everything else is considered as a valid
Re: [virt-tools-list] [virt-manager PATCH 1/2] unattended: Read the passwords from a file
On Tue, 2019-07-02 at 21:21 +0200, Fabiano Fidêncio wrote: > -=item B > +=item B > > -Set the VM OS admin/root password > +A file used to set the VM OS admin/root password from. This option can > +be used either as "admin-password-file=/path/to/passowrd-file" or as s/passowrd/password/ -- Andrea Bolognani / Red Hat / Virtualization ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-manager PATCH] cloudinit: Add disable=yes|no
Cli option to permanently disable cloud-init after first boot by user request.
Handled so that bare --cloud-init defaults to --cloud-init
root-password=generate,disable=yes.
Signed-off-by: Athina Plaskasoviti
---
virtinst/cli.py | 4 +++-
virtinst/install/cloudinit.py | 7 +--
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/virtinst/cli.py b/virtinst/cli.py
index caa12289..c080dfcf 100644
--- a/virtinst/cli.py
+++ b/virtinst/cli.py
@@ -1614,14 +1614,16 @@ class ParserCloudInit(VirtCLIParser):
def _init_class(cls, **kwargs):
VirtCLIParser._init_class(**kwargs)
cls.add_arg("root-password", "root_password")
+cls.add_arg("disable", "disable", is_onoff=True)
def parse_cloud_init(optstr):
ret = CloudInitData()
if optstr == 1:
# This means bare --cloud-init, so there's nothing to parse.
-log.warning("Defaulting to --cloud-init root-password=generate")
+log.warning("Defaulting to --cloud-init
root-password=generate,disable=yes")
ret.root_password = "generate"
+ret.disable = True
return ret
parser = ParserCloudInit(optstr)
diff --git a/virtinst/install/cloudinit.py b/virtinst/install/cloudinit.py
index 8041cc86..032d65e4 100644
--- a/virtinst/install/cloudinit.py
+++ b/virtinst/install/cloudinit.py
@@ -6,6 +6,7 @@ from ..logger import log
class CloudInitData():
+disable = None
root_password = None
@@ -40,8 +41,10 @@ def create_userdata(scratchdir, cloudinit_data):
content += "root:%s\n" % rootpass
content += " expire: True\n"
-content += "runcmd:\n"
-content += "- [ sudo, touch, /etc/cloud/cloud-init.disabled ]\n"
+disable = cloudinit_data.disable
+if disable is True:
+content += "runcmd:\n"
+content += "- [ sudo, touch, /etc/cloud/cloud-init.disabled ]\n"
log.debug("Generated cloud-init userdata:\n%s", content)
--
2.20.1
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
