vboot: Add FIT_SIGNATURE_MAX_SIZE protection

This adds a new config value FIT_SIGNATURE_MAX_SIZE, which controls the
max size of a FIT header's totalsize field. The field is checked before
signature checks are applied to protect from reading past the intended
FIT regions.

This field is not part of the vboot signature so it should be sanity
checked. If the field is corrupted then the structure or string region
reads may have unintended behavior, such as reading from device memory.
A default value of 256MB is set and intended to support most max storage
sizes.

Suggested-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Teddy Reed <teddy.reed@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>

diff --git a/Kconfig b/Kconfig
index 5a82c95dd80f909200b3c6fb24d3b014822cf31e..c8b86cd3843decd86f2f5f477b249717cc4eb365 100644 (file)
--- a/Kconfig
+++ b/Kconfig
@@ -267,6 +267,16 @@ config FIT_SIGNATURE
          format support in this case, enable it using
          CONFIG_IMAGE_FORMAT_LEGACY.
 
+config FIT_SIGNATURE_MAX_SIZE
+       hex "Max size of signed FIT structures"
+       depends on FIT_SIGNATURE
+       default 0x10000000
+       help
+         This option sets a max size in bytes for verified FIT uImages.
+         A sane value of 256MB protects corrupted DTB structures from overlapping
+         device memory. Assure this size does not extend past expected storage
+         space.
+
 config FIT_VERBOSE
        bool "Show verbose messages when FIT images fail"
        help

diff --git a/common/image-sig.c b/common/image-sig.c
index f65d883994be42e81b12e4f8c2921f90346e943f..8d2fd10db669e751c19ceeda8bf51d163da9609a 100644 (file)
--- a/common/image-sig.c
+++ b/common/image-sig.c
@@ -156,6 +156,11 @@ static int fit_image_setup_verify(struct image_sign_info *info,
 {
        char *algo_name;
 
+       if (fdt_totalsize(fit) > CONFIG_FIT_SIGNATURE_MAX_SIZE) {
+               *err_msgp = "Total size too large";
+               return 1;
+       }
+
        if (fit_image_hash_get_algo(fit, noffset, &algo_name)) {
                *err_msgp = "Can't get hash algo property";
                return -1;

diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
index ee939f2034e5aa68889643ff94f650dc5c27661f..3d25ec3d66678fcc44f2946bd811f677ad597543 100644 (file)
--- a/test/py/tests/test_vboot.py
+++ b/test/py/tests/test_vboot.py
@@ -26,6 +26,7 @@ Tests run with both SHA1 and SHA256 hashing.
 
 import pytest
 import sys
+import struct
 import u_boot_utils as util
 
 @pytest.mark.boardspec('sandbox')
@@ -105,6 +106,26 @@ def test_vboot(u_boot_console):
         util.run_and_log(cons, [mkimage, '-F', '-k', tmpdir, '-K', dtb,
                                 '-r', fit])
 
+    def replace_fit_totalsize(size):
+        """Replace FIT header's totalsize with something greater.
+
+        The totalsize must be less than or equal to FIT_SIGNATURE_MAX_SIZE.
+        If the size is greater, the signature verification should return false.
+
+        Args:
+            size: The new totalsize of the header
+
+        Returns:
+            prev_size: The previous totalsize read from the header
+        """
+        total_size = 0
+        with open(fit, 'r+b') as handle:
+            handle.seek(4)
+            total_size = handle.read(4)
+            handle.seek(4)
+            handle.write(struct.pack(">I", size))
+        return struct.unpack(">I", total_size)[0]
+
     def test_with_algo(sha_algo):
         """Test verified boot with the given hash algorithm.
 
@@ -146,6 +167,18 @@ def test_vboot(u_boot_console):
         util.run_and_log(cons, [fit_check_sign, '-f', fit, '-k', tmpdir,
                                 '-k', dtb])
 
+        # Replace header bytes
+        bcfg = u_boot_console.config.buildconfig
+        max_size = int(bcfg.get('config_fit_signature_max_size', 0x10000000), 0)
+        existing_size = replace_fit_totalsize(max_size + 1)
+        run_bootm(sha_algo, 'Signed config with bad hash', 'Bad Data Hash', False)
+        cons.log.action('%s: Check overflowed FIT header totalsize' % sha_algo)
+
+        # Replace with existing header bytes
+        replace_fit_totalsize(existing_size)
+        run_bootm(sha_algo, 'signed config', 'dev+', True)
+        cons.log.action('%s: Check default FIT header totalsize' % sha_algo)
+
         # Increment the first byte of the signature, which should cause failure
         sig = util.run_and_log(cons, 'fdtget -t bx %s %s value' %
                                (fit, sig_node))

diff --git a/tools/Makefile b/tools/Makefile
index 5dd33ed4d5611b9740bad90fcd25516b18f01dfe..0c3341e695ce05408314d97a4efb177053f53eb8 100644 (file)
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -133,6 +133,7 @@ ifdef CONFIG_FIT_SIGNATURE
 # This affects include/image.h, but including the board config file
 # is tricky, so manually define this options here.
 HOST_EXTRACFLAGS       += -DCONFIG_FIT_SIGNATURE
+HOST_EXTRACFLAGS       += -DCONFIG_FIT_SIGNATURE_MAX_SIZE=$(CONFIG_FIT_SIGNATURE_MAX_SIZE)
 endif
 
 ifdef CONFIG_SYS_U_BOOT_OFFS