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 <fidencio@xxxxxxxxxx> --- 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<admin-password=> +=item B<admin-password-file=> -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. -=item B<user-password=> +=item B<user-password-file=> -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<product-key=> diff --git a/tests/cli-test-xml/admin-password.txt b/tests/cli-test-xml/admin-password.txt new file mode 100644 index 00000000..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 00000000..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 --unattended profile=jeos --location http://example.foo", grep="admin-password") # will trigger admin-password required error -c.add_invalid("--os-variant debian9 --unattended profile=desktop,admin-password=foobar --location http://example.foo", grep="user-password") # will trigger user-password required error -c.add_invalid("--os-variant debian9 --unattended profile=FRIBBER,admin-password=foobar --location http://example.foo", grep="Available profiles") # will trigger unknown profile error -c.add_invalid("--os-variant fedora29 --unattended profile=desktop,admin-password=foobar --cdrom %(ISO-F29-LIVE)s", grep="media does not support") # live media doesn't support installscript +c.add_invalid("--os-variant debian9 --unattended profile=desktop,admin-password-file=%(ADMIN-PASSWORD-FILE)s --location http://example.foo", grep="user-password") # will trigger user-password required error +c.add_invalid("--os-variant debian9 --unattended profile=FRIBBER,admin-password-file=%(ADMIN-PASSWORD-FILE)s --location http://example.foo", grep="Available profiles") # will trigger unknown profile error +c.add_invalid("--os-variant fedora29 --unattended profile=desktop,admin-password-file=%(ADMIN-PASSWORD-FILE)s --cdrom %(ISO-F29-LIVE)s", grep="media does not support") # live media doesn't support installscript c.add_invalid("--os-variant msdos --unattended profile=desktop --location http://example.com") # msdos doesn't support unattended install c.add_invalid("--os-variant winxp --unattended profile=desktop --cdrom %(ISO-WIN7)s") # winxp doesn't support expected injection method 'cdrom' c.add_invalid("--connect %(URI-TEST-REMOTE)s --os-variant win7 --cdrom %(EXISTIMG1)s --unattended") # --unattended method=cdrom rejected for remote connections - ############################# # Remote URI specific tests # ############################# @@ -1356,7 +1358,7 @@ _add_argcomplete_cmd("virt-install --install i", "initrd") _add_argcomplete_cmd("virt-install --test-stub", None, nogrep="--test-stub-command") _add_argcomplete_cmd("virt-install --unattended ", "profile=") # will list all --unattended subprops -_add_argcomplete_cmd("virt-install --unattended a", "admin-password=") +_add_argcomplete_cmd("virt-install --unattended a", "admin-password-file=") _add_argcomplete_cmd("virt-clone --preserve", "--preserve-data") _add_argcomplete_cmd("virt-xml --sound mode", "model") _add_argcomplete_cmd("virt-convert --dest", "--destination") diff --git a/virtinst/cli.py b/virtinst/cli.py index e5645d84..3f540373 100644 --- a/virtinst/cli.py +++ b/virtinst/cli.py @@ -1508,8 +1508,8 @@ class ParserUnattended(VirtCLIParser): def _init_class(cls, **kwargs): VirtCLIParser._init_class(**kwargs) cls.add_arg("profile", "profile") - cls.add_arg("admin-password", "admin_password") - cls.add_arg("user-password", "user_password") + cls.add_arg("admin-password-file", "admin_password_file") + cls.add_arg("user-password-file", "user_password_file") cls.add_arg("product-key", "product_key") 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") + def _make_scriptmap(script_list): """ -- 2.21.0 _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list