Brad Bishop | 1a4b7ee | 2018-12-16 17:11:34 -0800 | [diff] [blame] | 1 | From 72239fc85f3eda078547956608c063ab965e90e9 Mon Sep 17 00:00:00 2001 |
| 2 | From: Teddy Reed <teddy.reed@gmail.com> |
| 3 | Date: Sat, 9 Jun 2018 11:38:05 -0400 |
| 4 | Subject: [PATCH] vboot: Add FIT_SIGNATURE_MAX_SIZE protection |
| 5 | |
| 6 | This adds a new config value FIT_SIGNATURE_MAX_SIZE, which controls the |
| 7 | max size of a FIT header's totalsize field. The field is checked before |
| 8 | signature checks are applied to protect from reading past the intended |
| 9 | FIT regions. |
| 10 | |
| 11 | This field is not part of the vboot signature so it should be sanity |
| 12 | checked. If the field is corrupted then the structure or string region |
| 13 | reads may have unintended behavior, such as reading from device memory. |
| 14 | A default value of 256MB is set and intended to support most max storage |
| 15 | sizes. |
| 16 | |
| 17 | Suggested-by: Simon Glass <sjg@chromium.org> |
| 18 | Signed-off-by: Teddy Reed <teddy.reed@gmail.com> |
| 19 | Reviewed-by: Simon Glass <sjg@chromium.org> |
| 20 | |
| 21 | Upstream-Status: Backport[http://git.denx.de/?p=u-boot.git;a=commit; |
| 22 | h=72239fc85f3eda078547956608c063ab965e90e9] |
| 23 | |
| 24 | CVE: CVE-2018-1000205 |
| 25 | |
| 26 | Signed-off-by: Changqing Li <changqing.li@windriver.com> |
| 27 | --- |
| 28 | Kconfig | 10 ++++++++++ |
| 29 | common/image-sig.c | 5 +++++ |
| 30 | test/py/tests/test_vboot.py | 33 +++++++++++++++++++++++++++++++++ |
| 31 | tools/Makefile | 1 + |
| 32 | 4 files changed, 49 insertions(+) |
| 33 | |
| 34 | diff --git a/Kconfig b/Kconfig |
| 35 | index 5a82c95..c8b86cd 100644 |
| 36 | --- a/Kconfig |
| 37 | +++ b/Kconfig |
| 38 | @@ -267,6 +267,16 @@ config FIT_SIGNATURE |
| 39 | format support in this case, enable it using |
| 40 | CONFIG_IMAGE_FORMAT_LEGACY. |
| 41 | |
| 42 | +config FIT_SIGNATURE_MAX_SIZE |
| 43 | + hex "Max size of signed FIT structures" |
| 44 | + depends on FIT_SIGNATURE |
| 45 | + default 0x10000000 |
| 46 | + help |
| 47 | + This option sets a max size in bytes for verified FIT uImages. |
| 48 | + A sane value of 256MB protects corrupted DTB structures from overlapping |
| 49 | + device memory. Assure this size does not extend past expected storage |
| 50 | + space. |
| 51 | + |
| 52 | config FIT_VERBOSE |
| 53 | bool "Show verbose messages when FIT images fail" |
| 54 | help |
| 55 | diff --git a/common/image-sig.c b/common/image-sig.c |
| 56 | index f65d883..8d2fd10 100644 |
| 57 | --- a/common/image-sig.c |
| 58 | +++ b/common/image-sig.c |
| 59 | @@ -156,6 +156,11 @@ static int fit_image_setup_verify(struct image_sign_info *info, |
| 60 | { |
| 61 | char *algo_name; |
| 62 | |
| 63 | + if (fdt_totalsize(fit) > CONFIG_FIT_SIGNATURE_MAX_SIZE) { |
| 64 | + *err_msgp = "Total size too large"; |
| 65 | + return 1; |
| 66 | + } |
| 67 | + |
| 68 | if (fit_image_hash_get_algo(fit, noffset, &algo_name)) { |
| 69 | *err_msgp = "Can't get hash algo property"; |
| 70 | return -1; |
| 71 | diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py |
| 72 | index ee939f2..3d25ec3 100644 |
| 73 | --- a/test/py/tests/test_vboot.py |
| 74 | +++ b/test/py/tests/test_vboot.py |
| 75 | @@ -26,6 +26,7 @@ Tests run with both SHA1 and SHA256 hashing. |
| 76 | |
| 77 | import pytest |
| 78 | import sys |
| 79 | +import struct |
| 80 | import u_boot_utils as util |
| 81 | |
| 82 | @pytest.mark.boardspec('sandbox') |
| 83 | @@ -105,6 +106,26 @@ def test_vboot(u_boot_console): |
| 84 | util.run_and_log(cons, [mkimage, '-F', '-k', tmpdir, '-K', dtb, |
| 85 | '-r', fit]) |
| 86 | |
| 87 | + def replace_fit_totalsize(size): |
| 88 | + """Replace FIT header's totalsize with something greater. |
| 89 | + |
| 90 | + The totalsize must be less than or equal to FIT_SIGNATURE_MAX_SIZE. |
| 91 | + If the size is greater, the signature verification should return false. |
| 92 | + |
| 93 | + Args: |
| 94 | + size: The new totalsize of the header |
| 95 | + |
| 96 | + Returns: |
| 97 | + prev_size: The previous totalsize read from the header |
| 98 | + """ |
| 99 | + total_size = 0 |
| 100 | + with open(fit, 'r+b') as handle: |
| 101 | + handle.seek(4) |
| 102 | + total_size = handle.read(4) |
| 103 | + handle.seek(4) |
| 104 | + handle.write(struct.pack(">I", size)) |
| 105 | + return struct.unpack(">I", total_size)[0] |
| 106 | + |
| 107 | def test_with_algo(sha_algo): |
| 108 | """Test verified boot with the given hash algorithm. |
| 109 | |
| 110 | @@ -146,6 +167,18 @@ def test_vboot(u_boot_console): |
| 111 | util.run_and_log(cons, [fit_check_sign, '-f', fit, '-k', tmpdir, |
| 112 | '-k', dtb]) |
| 113 | |
| 114 | + # Replace header bytes |
| 115 | + bcfg = u_boot_console.config.buildconfig |
| 116 | + max_size = int(bcfg.get('config_fit_signature_max_size', 0x10000000), 0) |
| 117 | + existing_size = replace_fit_totalsize(max_size + 1) |
| 118 | + run_bootm(sha_algo, 'Signed config with bad hash', 'Bad Data Hash', False) |
| 119 | + cons.log.action('%s: Check overflowed FIT header totalsize' % sha_algo) |
| 120 | + |
| 121 | + # Replace with existing header bytes |
| 122 | + replace_fit_totalsize(existing_size) |
| 123 | + run_bootm(sha_algo, 'signed config', 'dev+', True) |
| 124 | + cons.log.action('%s: Check default FIT header totalsize' % sha_algo) |
| 125 | + |
| 126 | # Increment the first byte of the signature, which should cause failure |
| 127 | sig = util.run_and_log(cons, 'fdtget -t bx %s %s value' % |
| 128 | (fit, sig_node)) |
| 129 | diff --git a/tools/Makefile b/tools/Makefile |
| 130 | index 5dd33ed..0c3341e 100644 |
| 131 | --- a/tools/Makefile |
| 132 | +++ b/tools/Makefile |
| 133 | @@ -133,6 +133,7 @@ ifdef CONFIG_FIT_SIGNATURE |
| 134 | # This affects include/image.h, but including the board config file |
| 135 | # is tricky, so manually define this options here. |
| 136 | HOST_EXTRACFLAGS += -DCONFIG_FIT_SIGNATURE |
| 137 | +HOST_EXTRACFLAGS += -DCONFIG_FIT_SIGNATURE_MAX_SIZE=$(CONFIG_FIT_SIGNATURE_MAX_SIZE) |
| 138 | endif |
| 139 | |
| 140 | ifdef CONFIG_SYS_U_BOOT_OFFS |
| 141 | -- |
| 142 | 2.7.4 |
| 143 | |