[virt-tools-list] [virt-manager PATCH 1/2] unattended: Read the passwords from a file
Fabiano Fidêncio
fidencio at redhat.com
Tue Jul 2 19:21:44 UTC 2019
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 at redhat.com>
---
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
More information about the virt-tools-list
mailing list