Merge pull request #176 from xcp-ng/stormi/varstored-tests-8.3

Adapt UEFI tests to latest 8.3

README.md

@@ -37,11 +37,8 @@ For Guest UEFI Secure Boot tests, the requirements are:
     * `openssl`
 * VM
     * `chattr`
-    * `efitools` for uefistored auth var tests
-    * `util-linux` for uefistored auth var tests in Alpine VMs
-* XCP-ng Host (installed by default on XCP-ng 8.2+)
-    * `uefistored`
-    * `varstored-tools`
+    * `efitools` for uefistored (in 8.2) or varstored (in 8.3+) auth var tests
+    * `util-linux` for uefistored (in 8.2) or varstored (in 8.3+) auth var tests in Alpine VMs
 
 Many tests have specific requirements, detailed in a comment at the top of the test file: minimal number of hosts in a pool, number of pools, VMs with specific characteristics (OS, BIOS vs UEFI, additional tools installed in the VM, additional networks in the pool, presence of an unused disk on one host or every host...). Markers, jobs defined in `jobs.py` (`./jobs.py show JOBNAME` will display the requirements and the reference to a VM or VM group), VMs and VM groups defined in `vm-data.py-dist` may all help understanding what tests can run with what VMs.
 
@@ -110,7 +107,7 @@ Another example:
 
 ```
 # Run secure boot tests that require a Unix VM (as opposed to a Windows VM) and that should ideally be run on a large variety of VMs
-pytest tests/uefistored -m "multi_vms and unix_vm" --hosts=ip_of_poolmaster --vm=http://path/to/unix_vm_1.xva --vm=http://path/to/unix_vm_2.xva --vm=http://path/to/unix_vm_3.xva
+pytest tests/uefi_sb -m "multi_vms and unix_vm" --hosts=ip_of_poolmaster --vm=http://path/to/unix_vm_1.xva --vm=http://path/to/unix_vm_2.xva --vm=http://path/to/unix_vm_3.xva
 ```
 
 
@@ -135,16 +132,20 @@ The output of commands below is given as example and may not reflect the current
 $ ./jobs.py list
 main: a group of not-too-long tests that run either without a VM, or with a single small one
 main-multi: a group of tests that need to run on the largest variety of VMs
+packages: tests that packages can be installed correctly
 quicktest: XAPI's quicktest, not so quick by the way
-storage-main: tests all storage drivers (except linstor), but avoids migrations and reboots
-storage-migrations: tests migrations with all storage drivers (except linstor)
-storage-reboots: storage driver tests that involve rebooting hosts (except linstor and flaky tests)
-sb-main: tests uefistored and SecureBoot using a small unix VM (or no VM when none needed)
-sb-windows: tests uefistored and SecureBoot using a Windows VM
+storage-main: tests all storage drivers, but avoids migrations and reboots
+storage-migrations: tests migrations with all storage drivers
+storage-reboots: storage driver tests that involve rebooting hosts (except flaky tests)
+sb-main: tests uefistored/varstored and SecureBoot using a small unix VM (or no VM when none needed)
+sb-certificates: tests certificate propagation to disk by XAPI, and to VMs by uefistored/varstored
+sb-windows: tests uefistored/varstored and SecureBoot using a Windows VM
 sb-unix-multi: checks basic Secure-Boot support on a variety of Unix VMs
 sb-windows-multi: checks basic Secure-Boot support on a variety of Windows VMs
 tools-unix: tests our unix guest tools on a single small VM
 tools-unix-multi: tests our unix guest tools on a variety of VMs
+xen: Testing of the Xen hypervisor itself
+vtpm: Testing vTPM functionalities
 flaky: tests that usually pass, but sometimes fail unexpectedly
 ```
 
@@ -163,7 +164,7 @@ $ ./jobs.py show sb-unix-multi
         "--vm[]": "multi/uefi_unix"
     },
     "paths": [
-        "tests/uefistored"
+        "tests/uefi_sb"
     ],
     "markers": "multi_vms and unix_vm"
 }
@@ -177,16 +178,19 @@ A very important information is also the `--vm` (single VM) or `--vm[]` (multipl
 There are two more commands that you can use to display information about a job:
 
 ```
-$ ./jobs.py collect sb-unix-multi
+$ ./jobs.py collect tools-unix
 [...]
-collected 175 items / 170 deselected / 5 selected
+collected 6 items
 
-<Package uefistored>
-  <Module test_secure_boot.py>
-    <Class TestGuestLinuxUEFISecureBoot>
-      <Function test_boot_success_when_pool_db_set_and_images_signed[hosts0-http://path/to/vm1.xva]>
-      <Function test_boot_success_when_pool_db_set_and_images_signed[hosts0-http://path/to/vm2.xva]>
-      <Function test_boot_success_when_pool_db_set_and_images_signed[hosts0-http://path/to/vm3.xva]>
+<Package unix>
+  <Module test_guest_tools_unix.py>
+    <Class TestGuestToolsUnix>
+      <Function test_install[hosts0-http://pxe/images/alpine-uefi-minimal-efitools-3.12.0.xva]>
+      <Function test_check_tools[hosts0-http://pxe/images/alpine-uefi-minimal-efitools-3.12.0.xva]>
+      <Function test_check_tools_after_reboot[hosts0-http://pxe/images/alpine-uefi-minimal-efitools-3.12.0.xva]>
+      <Function test_xenstore[hosts0-http://pxe/images/alpine-uefi-minimal-efitools-3.12.0.xva]>
+      <Function test_clean_shutdown[hosts0-http://pxe/images/alpine-uefi-minimal-efitools-3.12.0.xva]>
+      <Function test_storage_migration[hosts0-http://pxe/images/alpine-uefi-minimal-efitools-3.12.0.xva]>
 ```
 
 This lists the tests that are selected by the job. Tests may be repeated if they will run several times, as in the case of this example because there are 3 VMs to test. I chose a job whose output is small for the sake of documentation conciseness, but the output can be a lot bigger!
@@ -196,11 +200,11 @@ Lastly, the `run` command with the `--print-only` switch will display the comman
 ```
 # job with default parameters
 $ ./jobs.py run --print-only sb-unix-multi ip_of_poolmaster
-pytest tests/uefistored -m "multi_vms and unix_vm" --hosts=ip_of_poolmaster --vm=http://path/to/vm1.xva --vm=http://path/to/vm2.xva --vm=http://path/to/vm3.xva
+pytest tests/uefi_sb -m "multi_vms and unix_vm" --hosts=ip_of_poolmaster --vm=http://path/to/vm1.xva --vm=http://path/to/vm2.xva --vm=http://path/to/vm3.xva
 
 # same, but we override the list of VMs
 $ ./jobs.py run --print-only sb-unix-multi ip_of_poolmaster --vm=http://path/to/vm4.xva
-pytest tests/uefistored -m "multi_vms and unix_vm" --hosts=ip_of_poolmaster --vm=http://path/to/vm4.xva
+pytest tests/uefi_sb -m "multi_vms and unix_vm" --hosts=ip_of_poolmaster --vm=http://path/to/vm4.xva
 ```
 
 #### Run a job
@@ -221,7 +225,7 @@ Example:
 ```
 # job with default parameters
 $ ./jobs.py run sb-unix-multi ip_of_poolmaster
-pytest tests/uefistored -m "multi_vms and unix_vm" --hosts=ip_of_poolmaster --vm=http://path/to/vm1.xva --vm=http://path/to/vm2.xva --vm=http://path/to/vm3.xva
+pytest tests/uefi_sb -m "multi_vms and unix_vm" --hosts=ip_of_poolmaster --vm=http://path/to/vm1.xva --vm=http://path/to/vm2.xva --vm=http://path/to/vm3.xva
 [... job executes...]
 ```
 
@@ -230,7 +234,7 @@ Any parameter added at the end of the command will be passed to `pytest`. Any pa
 ```
 # same, but we override the list of VMs
 $ ./jobs.py run --print-only sb-unix-multi ip_of_poolmaster --vm=http://path/to/vm4.xva
-pytest tests/uefistored -m "multi_vms and unix_vm" --hosts=ip_of_poolmaster --vm=http://path/to/vm4.xva
+pytest tests/uefi_sb -m "multi_vms and unix_vm" --hosts=ip_of_poolmaster --vm=http://path/to/vm4.xva
 [... job executes...]
 ```
 

jobs.py

@@ -128,13 +128,17 @@
         "params": {
             "--vm": "single/small_vm_efitools",
         },
-        "paths": ["tests/uefistored/test_auth_var.py", "tests/uefistored/test_secure_boot.py"],
+        "paths": [
+            "tests/uefi_sb/test_auth_var.py",
+            "tests/uefi_sb/test_uefistored_sb.py",
+            "tests/uefi_sb/test_varstored_sb.py"
+        ],
         "markers": "not windows_vm",
     },
     "sb-certificates": {
-        "description": "[8.3+] tests certificate propagation to disk by XAPI, and to VMs by uefistored/varstored",
+        "description": "tests certificate propagation to disk by XAPI, and to VMs by uefistored/varstored",
         "requirements": [
-            "A pool >= 8.2.1. On 8.3+, it needs at least two hosts.",
+            "A pool >= 8.2.1. On 8.3+, it needs at least two hosts. On 8.2, one is enough but more is better.",
             "On 8.3+ only, a second pool, single-host, available for temporarily joining the first pool.",
             "A fast-booting unix UEFI VM with efitools.",
         ],
@@ -143,7 +147,7 @@
         "params": {
             "--vm": "single/small_vm_efitools",
         },
-        "paths": ["tests/uefistored/test_cert_inheritance.py"],
+        "paths": ["tests/uefi_sb/test_uefistored_cert_flow.py", "tests/uefi_sb/test_varstored_cert_flow.py"],
     },
     "sb-windows": {
         "description": "tests uefistored/varstored and SecureBoot using a Windows VM",
@@ -155,7 +159,7 @@
         "params": {
             "--vm": "single/small_vm_windows",
         },
-        "paths": ["tests/uefistored"],
+        "paths": ["tests/uefi_sb"],
         "markers": "windows_vm",
     },
     "sb-unix-multi": {
@@ -169,7 +173,7 @@
         "params": {
             "--vm[]": "multi/uefi_unix",
         },
-        "paths": ["tests/uefistored"],
+        "paths": ["tests/uefi_sb"],
         "markers": "multi_vms and unix_vm",
     },
     "sb-windows-multi": {
@@ -182,7 +186,7 @@
         "params": {
             "--vm[]": "multi/uefi_windows",
         },
-        "paths": ["tests/uefistored"],
+        "paths": ["tests/uefi_sb"],
         "markers": "multi_vms and windows_vm",
     },
     "tools-unix": {

lib/efi.py

@@ -4,6 +4,7 @@
 
 import atexit
 import copy
+import hashlib
 import logging
 import os
 import shutil
@@ -442,6 +443,8 @@ def esl_from_auth_bytes(auth: bytes) -> bytes:
     """
     return auth[auth.index(EFI_CERT_X509_GUID):]
 
+def get_md5sum_from_auth(auth):
+    return hashlib.md5(esl_from_auth_file(auth)).hexdigest()
 
 if __name__ == '__main__':
     import argparse

lib/host.py

@@ -391,6 +391,9 @@ def file_exists(self, filepath, regular_file=True):
     def binary_exists(self, binary):
         return self.ssh_with_result(['which', binary]).returncode == 0
 
+    def is_symlink(self, filepath):
+        return self.ssh_with_result(['test', '-L', filepath]).returncode == 0
+
     def sr_create(self, sr_type, label, device_config, shared=False, verify=False):
         params = {
             'host-uuid': self.uuid,

lib/pool.py

@@ -1,6 +1,8 @@
 import logging
 import traceback
 
+from packaging import version
+
 import lib.commands as commands
 
 from lib.common import safe_split, wait_for, wait_for_not
@@ -97,6 +99,24 @@ def first_shared_sr(self):
         return None
 
     def save_uefi_certs(self):
+        """
+        Save UEFI certificates in order to restore them later. XCP-ng 8.2 only.
+
+        This method was developed for XCP-ng 8.2, because many secureboot tests were dependent
+        on the initial state of the pool certificates, due to how certificates propagate.
+        Also, there were no certificates installed by default (except PK) on XCP-ng 8.2, and
+        we tried to be nice and restore the initial state after the tests.
+
+        On XCP-ng 8.3+, the tests don't depend so much on the pool certificates, and when they do we
+        can simply set custom certificates without erasing the default ones, so there's no real need
+        for saving then restoring the certificates.
+        The method was not reviewed for XCP-ng 8.3, and tests should be written in a way that is not
+        dependent on the initial state of pool certificates. To prevent ourselves from using a method
+        that is not appropriate, assert that the version is lower than 8.3.
+
+        This can be revised later if a need for saving custom certificates in 8.3+ arises.
+        """
+        assert self.master.xcp_version < version.parse("8.3"), "this function should only be needed on XCP-ng 8.2"
         logging.info('Saving pool UEFI certificates')
 
         if int(self.master.ssh(["secureboot-certs", "--version"]).split(".")[0]) < 1:
@@ -136,6 +156,8 @@ def save_uefi_certs(self):
             )
 
     def restore_uefi_certs(self):
+        # See explanation in save_uefi_certs().
+        assert self.master.xcp_version < version.parse("8.3"), "this function should only be needed on XCP-ng 8.2"
         assert self.saved_uefi_certs is not None
         if len(self.saved_uefi_certs) == 0:
             logging.info('We need to clear pool UEFI certificates to restore initial state')
@@ -156,12 +178,29 @@ def restore_uefi_certs(self):
             self.saved_uefi_certs = None
 
     def clear_uefi_certs(self):
+        """
+        Clear UEFI certificates on XCP-ng 8.2.
+
+        On XCP-ng 8.2, clearing the certificates from XAPI doesn't clear them from disk, so we need to do so manually.
+
+        This method is not suitable for XCP-ng 8.3+, where only custom certificates can be modified, and this
+        must all be done through XAPI (which will delete them from disk on each host automatically).
+
+        For XCP-ng 8.3+, see clear_custom_uefi_certificates()
+        """
+        assert self.master.xcp_version < version.parse("8.3"), "function only relevant on XCP-ng 8.2"
         logging.info('Clearing pool UEFI certificates in XAPI and on hosts disks')
         self.master.ssh(['secureboot-certs', 'clear'])
         # remove files on each host
         for host in self.hosts:
             host.ssh(['rm', '-f', f'{host.varstore_dir()}/*'])
 
+    def clear_custom_uefi_certs(self):
+        """ Clear Custom UEFI certificates on XCP-ng 8.3+. """
+        assert self.master.xcp_version >= version.parse("8.3"), "function only relevant on XCP-ng 8.3+"
+        logging.info('Clearing custom pool UEFI certificates')
+        self.master.ssh(['secureboot-certs', 'clear'])
+
     def install_custom_uefi_certs(self, auths):
         host = self.master
         auths_dict = {}

lib/vm.py

@@ -392,6 +392,8 @@ def clear_uefi_variables(self):
 
         This makes it look like the VM is new, in the eyes of uefistored/varstored,
         and so it will propagate certs from disk to its NVRAM when it boots next.
+
+        Some VMs will not boot anymore after such an operation. Seen with debian VMs, for example.
         """
         self.param_remove('NVRAM', 'EFI-variables')
 
@@ -536,3 +538,19 @@ def is_in_uefi_shell(self):
             res_host.ssh(['screen', '-S', session, '-X', 'quit'], check=False)
             res_host.ssh(['rm', '-f', tmp_file], check=False)
         return ret
+
+    def set_uefi_setup_mode(self):
+        # Note that in XCP-ng 8.2, the VM won't stay in setup mode, because uefistored
+        # will add PK and other certs if available when the guest boots.
+        logging.info(f"Set VM {self.uuid} to UEFI setup mode")
+        self.host.ssh(["varstore-sb-state", self.uuid, "setup"])
+
+    def set_uefi_user_mode(self):
+        # Setting user mode propagates the host's certificates to the VM
+        logging.info(f"Set VM {self.uuid} to UEFI user mode")
+        self.host.ssh(["varstore-sb-state", self.uuid, "user"])
+
+    def is_cert_present(vm, key):
+        res = vm.host.ssh(['varstore-get', vm.uuid, efi.get_secure_boot_guid(key).as_str(), key],
+                          check=False, simple_output=False, decode=False)
+        return res.returncode == 0

tests/uefistored/conftest.py → tests/uefi_sb/conftest.py

@@ -1,8 +1,11 @@
 import logging
 import pytest
 
+from packaging import version
+
 @pytest.fixture(scope='module')
 def pool_without_uefi_certs(host):
+    assert host.xcp_version < version.parse("8.3"), "fixture only relevant on XCP-ng 8.2"
     pool = host.pool
 
     # Save the certs.
@@ -22,9 +25,10 @@ def uefi_vm_and_snapshot(uefi_vm):
     vm = uefi_vm
 
     # Any VM that has been booted at least once comes with some
-    # UEFI variable state, so clear the state of UEFI variables.
-    logging.info('Clear VM UEFI certs and set SB to false')
-    vm.clear_uefi_variables()
+    # UEFI variable state, so simply clear the state of
+    # secure boot specific variables
+    vm.set_uefi_setup_mode()
+    logging.info('Set platform.secureboot to false for VM')
     vm.param_set('platform', 'secureboot', False)
     snapshot = vm.snapshot()