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.

Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>

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<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/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<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.
+Note that only the first line of the file will be considered, including
+any whitespace characters and excluding new-line.
 
 =item B<product-key=>
 

tests/cli-test-xml/admin-password.txt

@@ -0,0 +1 @@
+foobar

tests/cli-test-xml/user-password.txt

@@ -0,0 +1,3 @@
+blah
+     
+        

tests/clitest.py

@@ -89,6 +89,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,
 }
 
 
@@ -881,22 +883,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 #
 #############################
@@ -1367,7 +1368,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")

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")
 
 

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,22 @@ 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_password(self, pwdfile):
+        with open(pwdfile, "r") as fobj:
+            return fobj.readline().rstrip("\n\r")
+
+    def get_user_password(self):
+        if self.user_password_file:
+            return self._get_password(self.user_password_file)
+
+    def get_admin_password(self):
+        if self.admin_password_file:
+            return self._get_password(self.admin_password_file)
+
 
 def _make_scriptmap(script_list):
     """