createiso: Recompute .treeinfo checksums for images

Running xorriso to modify an ISO image can update content of included
images such as images/eltorito.img, unless we explicitly update the
image, which is undesirable (https://pagure.io/pungi/issue/1647).

However, when the file is changed, the checksum changes and .treeinfo no
longer matches.

This patch implements a workaround: once the DVD is written, it looks
for incorrect checksums, recalculates them and updates the .treeinfo on
the DVD. Since only the checksum is changing and the size of the file
remains the same, this seems to help fix the issue.

An additional step for implanting MD5 is needed again, as that gets
erased by the workaround.

JIRA: RHELCMP-13664
Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>

pungi/createiso.py

@@ -159,15 +159,11 @@ def write_xorriso_commands(opts):
 
     script = os.path.join(opts.script_dir, "xorriso-%s.txt" % id(opts))
     with open(script, "w") as f:
-        emit(f, "-indev %s" % opts.boot_iso)
+        for cmd in iso.xorriso_commands(
-        emit(f, "-outdev %s" % os.path.join(opts.output_dir, opts.iso_name))
+            opts.arch, opts.boot_iso, os.path.join(opts.output_dir, opts.iso_name)
-        emit(f, "-boot_image any replay")
+        ):
+            emit(f, " ".join(cmd))
         emit(f, "-volid %s" % opts.volid)
-        # isoinfo -J uses the Joliet tree, and it's used by virt-install
-        emit(f, "-joliet on")
-        # Support long filenames in the Joliet trees. Repodata is particularly
-        # likely to run into this limit.
-        emit(f, "-compliance joliet_long_names")
 
         with open(opts.graft_points) as gp:
             for line in gp:
@@ -178,10 +174,6 @@ def write_xorriso_commands(opts):
                 emit(f, "%s %s %s" % (cmd, fs_path, iso_path))
                 emit(f, "-chmod 0%o %s" % (_get_perms(fs_path), iso_path))
 
-        if opts.arch == "ppc64le":
-            # This is needed for the image to be bootable.
-            emit(f, "-as mkisofs -U --")
-
         emit(f, "-chown_r 0 /")
         emit(f, "-chgrp_r 0 /")
         emit(f, "-end")

pungi/phases/createiso.py

@@ -14,6 +14,7 @@
 # along with this program; if not, see <https://gnu.org/licenses/>.
 
 
+import itertools
 import os
 import random
 import shutil
@@ -23,7 +24,7 @@ import json
 import productmd.treeinfo
 from productmd.images import Image
 from kobo.threads import ThreadPool, WorkerThread
-from kobo.shortcuts import run, relative_path
+from kobo.shortcuts import run, relative_path, compute_file_checksums
 from six.moves import shlex_quote
 
 from pungi.wrappers import iso
@@ -457,7 +458,14 @@ class CreateIsoThread(WorkerThread):
 
         try:
             run_createiso_command(
-                num, compose, bootable, arch, cmd["cmd"], mounts, log_file
+                num,
+                compose,
+                bootable,
+                arch,
+                cmd["cmd"],
+                mounts,
+                log_file,
+                cmd["iso_path"],
             )
         except Exception:
             self.fail(compose, cmd, variant, arch)
@@ -538,7 +546,9 @@ def add_iso_to_metadata(
     return img
 
 
-def run_createiso_command(num, compose, bootable, arch, cmd, mounts, log_file):
+def run_createiso_command(
+    num, compose, bootable, arch, cmd, mounts, log_file, iso_path
+):
     packages = [
         "coreutils",
         "xorriso" if compose.conf.get("createiso_use_xorrisofs") else "genisoimage",
@@ -580,6 +590,76 @@ def run_createiso_command(num, compose, bootable, arch, cmd, mounts, log_file):
         weight=compose.conf["runroot_weights"].get("createiso"),
     )
 
+    if bootable and compose.conf.get("createiso_use_xorrisofs"):
+        fix_treeinfo_checksums(compose, iso_path, arch)
+
+
+def fix_treeinfo_checksums(compose, iso_path, arch):
+    """It is possible for the ISO to contain a .treefile with incorrect
+    checksums. By modifying the ISO (adding files) some of the images may
+    change.
+
+    This function fixes that after the fact by looking for incorrect checksums,
+    recalculating them and updating the .treeinfo file. Since the size of the
+    file doesn't change, this seems to not change any images.
+    """
+    modified = False
+    with iso.mount(iso_path, compose._logger) as mountpoint:
+        ti = productmd.TreeInfo()
+        ti.load(os.path.join(mountpoint, ".treeinfo"))
+        for image, (type_, expected) in ti.checksums.checksums.items():
+            checksums = compute_file_checksums(os.path.join(mountpoint, image), [type_])
+            actual = checksums[type_]
+            if actual == expected:
+                # Everything fine here, skip to next image.
+                continue
+
+            compose.log_debug("%s: %s: checksum mismatch", iso_path, image)
+            # Update treeinfo with correct checksum
+            ti.checksums.checksums[image] = (type_, actual)
+            modified = True
+
+    if not modified:
+        compose.log_debug("%s: All checksums match, nothing to do.", iso_path)
+        return
+
+    try:
+        tmpdir = compose.mkdtemp(arch, prefix="fix-checksum-")
+        # Write modified .treeinfo
+        ti_path = os.path.join(tmpdir, ".treeinfo")
+        compose.log_debug("Storing modified .treeinfo in %s", ti_path)
+        ti.dump(ti_path)
+        # Write a modified DVD into a temporary path, that is atomically moved
+        # over the original file.
+        fixed_path = os.path.join(tmpdir, "fixed-checksum-dvd.iso")
+        cmd = ["xorriso"]
+        cmd.extend(
+            itertools.chain.from_iterable(
+                iso.xorriso_commands(arch, iso_path, fixed_path)
+            )
+        )
+        cmd.extend(["-map", ti_path, ".treeinfo"])
+        run(
+            cmd,
+            logfile=compose.paths.log.log_file(
+                arch, "checksum-fix_generate_%s" % os.path.basename(iso_path)
+            ),
+        )
+        # The modified ISO no longer has implanted MD5, so that needs to be
+        # fixed again.
+        compose.log_debug("Implanting new MD5 to %s fixed_path")
+        run(
+            iso.get_implantisomd5_cmd(fixed_path, compose.supported),
+            logfile=compose.paths.log.log_file(
+                arch, "checksum-fix_implantisomd5_%s" % os.path.basename(iso_path)
+            ),
+        )
+        # All done, move the updated image to the final location.
+        compose.log_debug("Updating %s", iso_path)
+        os.rename(fixed_path, iso_path)
+    finally:
+        shutil.rmtree(tmpdir)
+
 
 def split_iso(compose, arch, variant, no_split=False, logger=None):
     """

pungi/phases/extra_isos.py

@@ -166,6 +166,7 @@ class ExtraIsosThread(WorkerThread):
                 log_file=compose.paths.log.log_file(
                     arch, "extraiso-%s" % os.path.basename(iso_path)
                 ),
+                iso_path=iso_path,
             )
 
         img = add_iso_to_metadata(

pungi/wrappers/iso.py

@@ -516,3 +516,21 @@ def mount(image, logger=None, use_guestmount=True):
                 util.run_unmount_cmd(["fusermount", "-u", mount_dir], path=mount_dir)
             else:
                 util.run_unmount_cmd(["umount", mount_dir], path=mount_dir)
+
+
+def xorriso_commands(arch, input, output):
+    """List of xorriso commands to modify a bootable image."""
+    commands = [
+        ("-indev", input),
+        ("-outdev", output),
+        # isoinfo -J uses the Joliet tree, and it's used by virt-install
+        ("-joliet", "on"),
+        # Support long filenames in the Joliet trees. Repodata is particularly
+        # likely to run into this limit.
+        ("-compliance", "joliet_long_names"),
+        ("-boot_image", "any", "replay"),
+    ]
+    if arch == "ppc64le":
+        # This is needed for the image to be bootable.
+        commands.append(("-as", "mkisofs", "-U", "--"))
+    return commands

tests/helpers.py

@@ -2,6 +2,7 @@
 
 import difflib
 import errno
+import hashlib
 import os
 import shutil
 import tempfile
@@ -364,3 +365,9 @@ def fake_run_in_threads(func, params, threads=None):
     """Like run_in_threads from Kobo, but actually runs tasks serially."""
     for num, param in enumerate(params):
         func(None, param, num)
+
+
+def hash_string(alg, s):
+    m = hashlib.new(alg)
+    m.update(s.encode("utf-8"))
+    return m.hexdigest()

tests/test_createiso_phase.py

@@ -2,6 +2,7 @@
 
 
 import logging
+import contextlib
 
 try:
     from unittest import mock
@@ -9,6 +10,7 @@ except ImportError:
     import mock
 import six
 
+import productmd
 import os
 
 from tests import helpers
@@ -1607,3 +1609,103 @@ class ComposeConfGetIsoLevelTest(helpers.PungiTestCase):
                 compose, compose.variants["Client"], "x86_64"
             ),
         )
+
+
+def mk_mount(topdir, images):
+    @contextlib.contextmanager
+    def dummy_mount(path, logger):
+        treeinfo = [
+            "[general]",
+            "family = Test",
+            "version = 1.0",
+            "arch = x86_64",
+            "variant = Server",
+            "[checksums]",
+        ]
+        for image in images:
+            helpers.touch(os.path.join(topdir, image.path), image.content)
+            treeinfo.append("%s = sha256:%s" % (image.path, image.checksum))
+        helpers.touch(os.path.join(topdir, ".treeinfo"), "\n".join(treeinfo))
+        yield topdir
+
+    return dummy_mount
+
+
+class _MockRun:
+    """This class replaces kobo.shortcuts.run and validates that the correct
+    two commands are called. The assertions can not be done after the tested
+    function finishes because it will clean up the .treeinfo file that needs to
+    be checked.
+    """
+
+    def __init__(self):
+        self.num_calls = 0
+        self.asserts = [self._assert_xorriso, self._assert_implantisomd5]
+
+    def __call__(self, cmd, logfile):
+        self.num_calls += 1
+        self.asserts.pop(0)(cmd)
+
+    def _assert_xorriso(self, cmd):
+        assert cmd[0] == "xorriso"
+        ti = productmd.TreeInfo()
+        input_iso = None
+        for i, arg in enumerate(cmd):
+            if arg == "-map":
+                ti.load(cmd[i + 1])
+            if arg == "-outdev":
+                self.temp_iso = cmd[i + 1]
+            if arg == "-indev":
+                input_iso = cmd[i + 1]
+        assert self.input_iso == input_iso
+        assert ti.checksums.checksums[self.image_relative_path] == self.image_checksum
+
+    def _assert_implantisomd5(self, cmd):
+        assert cmd[0] == "/usr/bin/implantisomd5"
+        assert cmd[-1] == self.temp_iso
+
+
+class DummyImage:
+    def __init__(self, path, content, checksum=None):
+        self.path = path
+        self.content = content
+        self.checksum = checksum or helpers.hash_string("sha256", content)
+
+
+@mock.patch("os.rename")
+@mock.patch("pungi.phases.createiso.run", new_callable=_MockRun)
+class FixChecksumsTest(helpers.PungiTestCase):
+    def test_checksum_matches(self, mock_run, mock_rename):
+        compose = helpers.DummyCompose(self.topdir, {})
+        arch = "x86_64"
+        iso_path = "DUMMY_ISO"
+
+        with mock.patch(
+            "pungi.wrappers.iso.mount",
+            new=mk_mount(self.topdir, [DummyImage("images/eltorito.img", "eltorito")]),
+        ):
+            createiso.fix_treeinfo_checksums(compose, iso_path, arch)
+
+        self.assertEqual(mock_run.num_calls, 0)
+        self.assertEqual(mock_rename.call_args_list, [])
+
+    def test_checksum_fix(self, mock_run, mock_rename):
+        compose = helpers.DummyCompose(self.topdir, {})
+        arch = "x86_64"
+        img = "images/eltorito.img"
+        content = "eltorito"
+        iso_path = "DUMMY_ISO"
+        mock_run.input_iso = iso_path
+        mock_run.image_relative_path = "images/eltorito.img"
+        mock_run.image_checksum = ("sha256", helpers.hash_string("sha256", content))
+
+        with mock.patch(
+            "pungi.wrappers.iso.mount",
+            new=mk_mount(self.topdir, [DummyImage(img, content, "abc")]),
+        ):
+            createiso.fix_treeinfo_checksums(compose, iso_path, arch)
+
+        # The new image was copied over the old one
+        self.assertEqual(
+            mock_rename.call_args_list, [mock.call(mock_run.temp_iso, iso_path)]
+        )

tests/test_extra_isos_phase.py

@@ -134,6 +134,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase):
                 ),
             ],
         )
+        iso_path = os.path.join(self.topdir, "compose/Server/x86_64/iso/my.iso")
         self.assertEqual(
             rcc.call_args_list,
             [
@@ -152,6 +153,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase):
                     log_file=os.path.join(
                         self.topdir, "logs/x86_64/extraiso-my.iso.x86_64.log"
                     ),
+                    iso_path=iso_path,
                 )
             ],
         )
@@ -162,7 +164,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase):
                     compose,
                     server,
                     "x86_64",
-                    os.path.join(self.topdir, "compose/Server/x86_64/iso/my.iso"),
+                    iso_path,
                     True,
                     additional_variants=["Client"],
                 )
@@ -209,6 +211,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase):
                 ),
             ],
         )
+        iso_path = os.path.join(self.topdir, "compose/Server/x86_64/iso/my.iso")
         self.assertEqual(
             rcc.call_args_list,
             [
@@ -227,6 +230,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase):
                     log_file=os.path.join(
                         self.topdir, "logs/x86_64/extraiso-my.iso.x86_64.log"
                     ),
+                    iso_path=iso_path,
                 )
             ],
         )
@@ -237,7 +241,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase):
                     compose,
                     server,
                     "x86_64",
-                    os.path.join(self.topdir, "compose/Server/x86_64/iso/my.iso"),
+                    iso_path,
                     True,
                     additional_variants=["Client"],
                 )
@@ -282,6 +286,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase):
                 ),
             ],
         )
+        iso_path = os.path.join(self.topdir, "compose/Server/x86_64/iso/my.iso")
         self.assertEqual(
             rcc.call_args_list,
             [
@@ -300,6 +305,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase):
                     log_file=os.path.join(
                         self.topdir, "logs/x86_64/extraiso-my.iso.x86_64.log"
                     ),
+                    iso_path=iso_path,
                 )
             ],
         )
@@ -310,7 +316,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase):
                     compose,
                     server,
                     "x86_64",
-                    os.path.join(self.topdir, "compose/Server/x86_64/iso/my.iso"),
+                    iso_path,
                     True,
                     additional_variants=["Client"],
                 )
@@ -357,6 +363,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase):
                 ),
             ],
         )
+        iso_path = os.path.join(self.topdir, "compose/Server/x86_64/iso/my.iso")
         self.assertEqual(
             rcc.call_args_list,
             [
@@ -375,6 +382,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase):
                     log_file=os.path.join(
                         self.topdir, "logs/x86_64/extraiso-my.iso.x86_64.log"
                     ),
+                    iso_path=iso_path,
                 )
             ],
         )
@@ -385,7 +393,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase):
                     compose,
                     server,
                     "x86_64",
-                    os.path.join(self.topdir, "compose/Server/x86_64/iso/my.iso"),
+                    iso_path,
                     False,
                     additional_variants=["Client"],
                 )
@@ -427,6 +435,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase):
                 ),
             ],
         )
+        iso_path = os.path.join(self.topdir, "compose/Server/source/iso/my.iso")
         self.assertEqual(
             rcc.call_args_list,
             [
@@ -445,6 +454,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase):
                     log_file=os.path.join(
                         self.topdir, "logs/src/extraiso-my.iso.src.log"
                     ),
+                    iso_path=iso_path,
                 )
             ],
         )
@@ -455,7 +465,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase):
                     compose,
                     server,
                     "src",
-                    os.path.join(self.topdir, "compose/Server/source/iso/my.iso"),
+                    iso_path,
                     False,
                     additional_variants=["Client"],
                 )