Fix Aboot breakage in sonic package manager in sonic-installer (sonic-net#1625)

> Failure cause

The `get_rootfs_path` contextmanager was repurposed to implement
`get_file_in_image` and later used as a function by leveraging some
python complexity to bypass the restrictions coming with the
contextmanager which were added purposefuly.
It was then called multiple times to compute paths though a simple
path join using `new_image_dir` could have been performed.

The `get_rootfs_path` implementation for Aboot behaved differently
when a rootfs was extracted or kept within the SWI image. It also
behaved differently on secureboot systems.
The updated method was then called on non-existing files for which
the method was never meant to process.

> Context around the failure

Over time, the installation and boot process has slightly diverged from
the ONIE one. There are 3 scenarios to consider.

1) Regular boot similar to ONIE
This one should just work the same as the filesystem layout is
unchanged.

2) docker_inram where dockerfs.tar.gz is extracted in tmpfs at boot
In this scenario the boot is similar to the regular one beside that
dockerfs.tar.gz is preserved intact on the flash and not extracted.
By design this does not fit the sonic-package-manager requierements and
the migration should be skipped which is what I did in this review.
In the coming month this boot mode will look closer to 3) below.

3) Secureboot on Arista devices
In this scenario the SWI image is kept intact and nothing extracted
from it. By ensuring the integrity of the SWI we can guarantee that no
code/data has been tampered with. This mode also relies on
`docker_inram` at the moment.
It could be enhanced when sonic-package-manager can guarantee the
integrity of code and data that is both installed and migrated.

> Solution provided

The method `get_file_in_image` was reverted to its original meaning
`get_rootfs_path` as there is no point in keeping the new one.
It doesn't have the necessary logic to handle more than the rootfs and
the logic can be easily be achieved by the following line.
`os.path.join(bootloader.get_image_path(binary_image_name), 'something')`

A new Bootloader method called `support_package_migration` is
introduced to allow the bootloader to skip the package migration based
on the image (docker_inram) or its own configuration (secureboot).
By default all bootloaders enable the package migration.

That change leads to 1) above running package migration while 2) and 3)
skip it.

Author: Staphylo

sonic_installer/bootloader/aboot.py

@@ -19,25 +19,44 @@
    HOST_PATH,
    IMAGE_DIR_PREFIX,
    IMAGE_PREFIX,
+   ROOTFS_NAME,
    run_command,
    run_command_or_raise,
 )
 from .bootloader import Bootloader
 
 _secureboot = None
 DEFAULT_SWI_IMAGE = 'sonic.swi'
+KERNEL_CMDLINE_NAME = 'kernel-cmdline'
 
 # For the signature format, see: https://github.com/aristanetworks/swi-tools/tree/master/switools
 SWI_SIG_FILE_NAME = 'swi-signature'
 SWIX_SIG_FILE_NAME = 'swix-signature'
 ISSUERCERT = 'IssuerCert'
 
-def isSecureboot():
+def parse_cmdline(cmdline=None):
+    if cmdline is None:
+        with open('/proc/cmdline') as f:
+            cmdline = f.read()
+
+    data = {}
+    for entry in cmdline.split():
+        idx = entry.find('=')
+        if idx == -1:
+            data[entry] = None
+        else:
+            data[entry[:idx]] = entry[idx+1:]
+    return data
+
+def docker_inram(cmdline=None):
+    cmdline = parse_cmdline(cmdline)
+    return cmdline.get('docker_inram') == 'on'
+
+def is_secureboot():
     global _secureboot
     if _secureboot is None:
-        with open('/proc/cmdline') as f:
-            m = re.search(r"secure_boot_enable=[y1]", f.read())
-        _secureboot = bool(m)
+        cmdline = parse_cmdline()
+        _secureboot = cmdline.get('secure_boot_enable') in ['y', '1']
     return _secureboot
 
 class AbootBootloader(Bootloader):
@@ -70,7 +89,7 @@ def _boot_config_set(self, **kwargs):
 
     def _swi_image_path(self, image):
         image_dir = image.replace(IMAGE_PREFIX, IMAGE_DIR_PREFIX)
-        if isSecureboot():
+        if is_secureboot():
            return 'flash:%s/sonic.swi' % image_dir
         return 'flash:%s/.sonic-boot.swi' % image_dir
 
@@ -118,6 +137,25 @@ def remove_image(self, image):
         subprocess.call(['rm','-rf', image_path])
         click.echo('Image removed')
 
+    def _get_image_cmdline(self, image):
+        image_path = self.get_image_path(image)
+        with open(os.path.join(image_path, KERNEL_CMDLINE_NAME)) as f:
+            return f.read()
+
+    def supports_package_migration(self, image):
+        if is_secureboot():
+            # NOTE: unsafe until migration can guarantee migration safety
+            #       packages need to be signed and verified at boot time.
+            return False
+        cmdline = self._get_image_cmdline(image)
+        if docker_inram(cmdline):
+            # NOTE: the docker_inram feature extracts builtin containers at boot
+            #       time in memory. the use of package manager under these
+            #       circumpstances is not possible without a boot time package
+            #       installation mechanism.
+            return False
+        return True
+
     def get_binary_image_version(self, image_path):
         try:
             version = subprocess.check_output(['/usr/bin/unzip', '-qop', image_path, '.imagehash'], text=True)
@@ -140,7 +178,7 @@ def verify_next_image(self):
         return self._verify_secureboot_image(image_path)
 
     def _verify_secureboot_image(self, image_path):
-        if isSecureboot():
+        if is_secureboot():
             cert = self.getCert(image_path)
             return cert is not None
         return True
@@ -188,14 +226,14 @@ def _get_swi_file_offset(self, swipath, filename):
                 return f._fileobj.tell() # pylint: disable=protected-access
 
     @contextmanager
-    def get_path_in_image(self, image_path, path):
-        path_in_image = os.path.join(image_path, path)
-        if os.path.exists(path_in_image) and not isSecureboot():
-            yield path_in_image
+    def get_rootfs_path(self, image_path):
+        path = os.path.join(image_path, ROOTFS_NAME)
+        if os.path.exists(path) and not is_secureboot():
+            yield path
             return
 
         swipath = os.path.join(image_path, DEFAULT_SWI_IMAGE)
-        offset = self._get_swi_file_offset(swipath, path)
+        offset = self._get_swi_file_offset(swipath, ROOTFS_NAME)
         loopdev = subprocess.check_output(['losetup', '-f']).decode('utf8').rstrip()
 
         try:

sonic_installer/bootloader/bootloader.py

@@ -9,6 +9,7 @@
    HOST_PATH,
    IMAGE_DIR_PREFIX,
    IMAGE_PREFIX,
+   ROOTFS_NAME,
 )
 
 class Bootloader(object):
@@ -58,6 +59,10 @@ def verify_next_image(self):
         image_path = self.get_image_path(image)
         return path.exists(image_path)
 
+    def supports_package_migration(self, image):
+        """tells if the image supports package migration"""
+        return True
+
     @classmethod
     def detect(cls):
         """returns True if the bootloader is in use"""
@@ -70,6 +75,6 @@ def get_image_path(cls, image):
         return image.replace(IMAGE_PREFIX, prefix)
 
     @contextmanager
-    def get_path_in_image(self, image_path, path_in_image):
+    def get_rootfs_path(self, image_path):
         """returns the path to the squashfs"""
-        yield path.join(image_path, path_in_image)
+        yield path.join(image_path, ROOTFS_NAME)

sonic_installer/main.py

@@ -1,5 +1,4 @@
 import configparser
-import contextlib
 import os
 import re
 import subprocess
@@ -12,11 +11,9 @@
 from swsscommon.swsscommon import SonicV2Connector
 
 from .bootloader import get_bootloader
-from .bootloader.aboot import AbootBootloader
 from .common import (
     run_command, run_command_or_raise,
     IMAGE_PREFIX,
-    ROOTFS_NAME,
     UPPERDIR_NAME,
     WORKDIR_NAME,
     DOCKERDIR_NAME,
@@ -279,7 +276,7 @@ def update_sonic_environment(bootloader, binary_image_version):
     env_dir = os.path.join(new_image_dir, "sonic-config")
     env_file = os.path.join(env_dir, "sonic-environment")
 
-    with bootloader.get_path_in_image(new_image_dir, ROOTFS_NAME) as new_image_squashfs_path:
+    with bootloader.get_rootfs_path(new_image_dir) as new_image_squashfs_path:
         try:
             mount_squash_fs(new_image_squashfs_path, new_image_mount)
 
@@ -321,21 +318,21 @@ def migrate_sonic_packages(bootloader, binary_image_version):
     packages_path = os.path.join(PACKAGE_MANAGER_DIR, packages_file)
     sonic_version = re.sub(IMAGE_PREFIX, '', binary_image_version)
     new_image_dir = bootloader.get_image_path(binary_image_version)
+    new_image_upper_dir = os.path.join(new_image_dir, UPPERDIR_NAME)
+    new_image_work_dir = os.path.join(new_image_dir, WORKDIR_NAME)
+    new_image_docker_dir = os.path.join(new_image_dir, DOCKERDIR_NAME)
+    new_image_mount = os.path.join("/", tmp_dir, "image-{0}-fs".format(sonic_version))
+    new_image_docker_mount = os.path.join(new_image_mount, "var", "lib", "docker")
+
+    if not os.path.isdir(new_image_docker_dir):
+        # NOTE: This codepath can be reached if the installation process did not
+        #       extract the default dockerfs. This can happen with docker_inram
+        #       though the bootloader class should have disabled the package
+        #       migration which is why this message is a non fatal error message.
+        echo_and_log("Error: SONiC package migration cannot proceed due to missing docker folder", LOG_ERR, fg="red")
+        return
 
-    with contextlib.ExitStack() as stack:
-        def get_path(path):
-            """ Closure to get path by entering
-            a context manager of bootloader.get_path_in_image """
-
-            return stack.enter_context(bootloader.get_path_in_image(new_image_dir, path))
-
-        new_image_squashfs_path = get_path(ROOTFS_NAME)
-        new_image_upper_dir = get_path(UPPERDIR_NAME)
-        new_image_work_dir = get_path(WORKDIR_NAME)
-        new_image_docker_dir = get_path(DOCKERDIR_NAME)
-        new_image_mount = os.path.join("/", tmp_dir, "image-{0}-fs".format(sonic_version))
-        new_image_docker_mount = os.path.join(new_image_mount, "var", "lib", "docker")
-
+    with bootloader.get_rootfs_path(new_image_dir) as new_image_squashfs_path:
         try:
             mount_squash_fs(new_image_squashfs_path, new_image_mount)
             # make sure upper dir and work dir exist
@@ -434,8 +431,8 @@ def install(url, force, skip_migration=False, skip_package_migration=False):
 
         update_sonic_environment(bootloader, binary_image_version)
 
-        if isinstance(bootloader, AbootBootloader) and not skip_package_migration:
-            echo_and_log("Warning: SONiC package migration is not supported currenty on aboot platform due to https://github.com/Azure/sonic-buildimage/issues/7566.", LOG_ERR, fg="red")
+        if not bootloader.supports_package_migration(binary_image_version) and not skip_package_migration:
+            echo_and_log("Warning: SONiC package migration is not supported for this bootloader/image", fg="yellow")
             skip_package_migration = True
 
         if not skip_package_migration: