From patchwork Wed Mar 18 17:43:55 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 243854 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Wed, 18 Mar 2020 11:43:55 -0600 Subject: [PATCH v2 01/14] image: Correct comment for fit_conf_get_node() In-Reply-To: <20200318174408.77473-1-sjg@chromium.org> References: <20200318174408.77473-1-sjg@chromium.org> Message-ID: <20200318174408.77473-2-sjg@chromium.org> This should mention that conf_uname can be NULL and should be in the header file. Fix this. Signed-off-by: Simon Glass --- Changes in v2: None common/image-fit.c | 18 ------------------ include/image.h | 20 ++++++++++++++++++++ 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/common/image-fit.c b/common/image-fit.c index 4435bc4f1d..7cf02b4574 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -1712,24 +1712,6 @@ int fit_conf_find_compat(const void *fit, const void *fdt) return best_match_offset; } -/** - * fit_conf_get_node - get node offset for configuration of a given unit name - * @fit: pointer to the FIT format image header - * @conf_uname: configuration node unit name - * - * fit_conf_get_node() finds a configuration (within the '/configurations' - * parent node) of a provided unit name. If configuration is found its node - * offset is returned to the caller. - * - * When NULL is provided in second argument fit_conf_get_node() will search - * for a default configuration node instead. Default configuration node unit - * name is retrieved from FIT_DEFAULT_PROP property of the '/configurations' - * node. - * - * returns: - * configuration node offset when found (>=0) - * negative number on failure (FDT_ERR_* code) - */ int fit_conf_get_node(const void *fit, const char *conf_uname) { int noffset, confs_noffset; diff --git a/include/image.h b/include/image.h index b316d167d8..512243f159 100644 --- a/include/image.h +++ b/include/image.h @@ -1092,7 +1092,27 @@ int fit_image_check_comp(const void *fit, int noffset, uint8_t comp); int fit_check_format(const void *fit); int fit_conf_find_compat(const void *fit, const void *fdt); + +/** + * fit_conf_get_node - get node offset for configuration of a given unit name + * @fit: pointer to the FIT format image header + * @conf_uname: configuration node unit name (NULL to use default) + * + * fit_conf_get_node() finds a configuration (within the '/configurations' + * parent node) of a provided unit name. If configuration is found its node + * offset is returned to the caller. + * + * When NULL is provided in second argument fit_conf_get_node() will search + * for a default configuration node instead. Default configuration node unit + * name is retrieved from FIT_DEFAULT_PROP property of the '/configurations' + * node. + * + * returns: + * configuration node offset when found (>=0) + * negative number on failure (FDT_ERR_* code) + */ int fit_conf_get_node(const void *fit, const char *conf_uname); + int fit_conf_get_prop_node_count(const void *fit, int noffset, const char *prop_name); int fit_conf_get_prop_node_index(const void *fit, int noffset, From patchwork Wed Mar 18 17:43:56 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 243855 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Wed, 18 Mar 2020 11:43:56 -0600 Subject: [PATCH v2 02/14] image: Be a little more verbose when checking signatures In-Reply-To: <20200318174408.77473-1-sjg@chromium.org> References: <20200318174408.77473-1-sjg@chromium.org> Message-ID: <20200318174408.77473-3-sjg@chromium.org> It is useful to be a little more specific about what is being checked. Update a few messages to help with this. Signed-off-by: Simon Glass --- Changes in v2: None common/image-fit.c | 2 +- tools/image-host.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/common/image-fit.c b/common/image-fit.c index 7cf02b4574..a5c85ede8d 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -1951,7 +1951,7 @@ int fit_image_load(bootm_headers_t *images, ulong addr, fit_uname = fit_get_name(fit, noffset, NULL); } if (noffset < 0) { - puts("Could not find subimage node\n"); + printf("Could not find subimage node type '%s'\n", prop_name); bootstage_error(bootstage_id + BOOTSTAGE_SUB_SUBNODE); return -ENOENT; } diff --git a/tools/image-host.c b/tools/image-host.c index 76a361b9d6..b3ec197dc9 100644 --- a/tools/image-host.c +++ b/tools/image-host.c @@ -1034,7 +1034,8 @@ int fit_check_sign(const void *fit, const void *key) if (!cfg_noffset) return -1; - printf("Verifying Hash Integrity ... "); + printf("Verifying Hash Integrity for node '%s'... ", + fdt_get_name(fit, cfg_noffset, NULL)); ret = fit_config_verify(fit, cfg_noffset); if (ret) return ret; From patchwork Wed Mar 18 17:43:57 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 243856 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Wed, 18 Mar 2020 11:43:57 -0600 Subject: [PATCH v2 03/14] image: Return an error message from fit_config_verify_sig() In-Reply-To: <20200318174408.77473-1-sjg@chromium.org> References: <20200318174408.77473-1-sjg@chromium.org> Message-ID: <20200318174408.77473-4-sjg@chromium.org> This function only returns an error message sometimes. Update it to always return an error message if one is available. This makes it easier to see what went wrong. Signed-off-by: Simon Glass --- Changes in v2: None common/image-sig.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/common/image-sig.c b/common/image-sig.c index 639a112450..13ccd50bc5 100644 --- a/common/image-sig.c +++ b/common/image-sig.c @@ -499,13 +499,14 @@ static int fit_config_verify_sig(const void *fit, int conf_noffset, goto error; } - return verified ? 0 : -EPERM; + if (verified) + return 0; error: printf(" error!\n%s for '%s' hash node in '%s' config node\n", err_msg, fit_get_name(fit, noffset, NULL), fit_get_name(fit, conf_noffset, NULL)); - return -1; + return -EPERM; } int fit_config_verify_required_sigs(const void *fit, int conf_noffset, From patchwork Wed Mar 18 17:43:58 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 243857 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Wed, 18 Mar 2020 11:43:58 -0600 Subject: [PATCH v2 04/14] test: vboot: Drop unnecessary parameter for fit_check_sign In-Reply-To: <20200318174408.77473-1-sjg@chromium.org> References: <20200318174408.77473-1-sjg@chromium.org> Message-ID: <20200318174408.77473-5-sjg@chromium.org> This tool only uses the last -k parameter provided. Drop the earlier one since it has no effect. Signed-off-by: Simon Glass --- Changes in v2: None test/py/tests/test_vboot.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py index 9c41ee56b1..3dd8e3cb66 100644 --- a/test/py/tests/test_vboot.py +++ b/test/py/tests/test_vboot.py @@ -180,8 +180,7 @@ def test_vboot(u_boot_console): cons.log.action('%s: Check signed config on the host' % sha_algo) - util.run_and_log(cons, [fit_check_sign, '-f', fit, '-k', tmpdir, - '-k', dtb]) + util.run_and_log(cons, [fit_check_sign, '-f', fit, '-k', dtb]) # Replace header bytes bcfg = u_boot_console.config.buildconfig From patchwork Wed Mar 18 17:43:59 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 243859 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Wed, 18 Mar 2020 11:43:59 -0600 Subject: [PATCH v2 05/14] test: vboot: Add a test for a forged configuration In-Reply-To: <20200318174408.77473-1-sjg@chromium.org> References: <20200318174408.77473-1-sjg@chromium.org> Message-ID: <20200318174408.77473-6-sjg@chromium.org> Add a check to make sure that it is not possible to add a new configuration and use the hashed nodes and hash of another configuration. Signed-off-by: Simon Glass --- Changes in v2: - Bring in new vboot_forge file from the authors test/py/tests/test_vboot.py | 18 +- test/py/tests/vboot_forge.py | 423 +++++++++++++++++++++++++++++++++++ 2 files changed, 440 insertions(+), 1 deletion(-) create mode 100644 test/py/tests/vboot_forge.py diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py index 3dd8e3cb66..22c79ef313 100644 --- a/test/py/tests/test_vboot.py +++ b/test/py/tests/test_vboot.py @@ -28,6 +28,7 @@ import pytest import sys import struct import u_boot_utils as util +import vboot_forge @pytest.mark.boardspec('sandbox') @pytest.mark.buildconfigspec('fit_signature') @@ -182,7 +183,22 @@ def test_vboot(u_boot_console): util.run_and_log(cons, [fit_check_sign, '-f', fit, '-k', dtb]) - # Replace header bytes + # Make sure that U-Boot checks that the config is in the list of hashed + # nodes. If it isn't, a security bypass is possible. + with open(fit, 'rb') as fp: + root, strblock = vboot_forge.read_fdt(fp) + root, strblock = vboot_forge.manipulate(root, strblock) + with open(fit, 'w+b') as fp: + vboot_forge.write_fdt(root, strblock, fp) + util.run_and_log_expect_exception(cons, + [fit_check_sign, '-f', fit, '-k', dtb], + 1, 'Failed to verify required signature') + + run_bootm(sha_algo, 'forged config', 'Bad Data Hash', False) + + # Create a new properly signed fit and replace header bytes + make_fit('sign-configs-%s%s.its' % (sha_algo, padding)) + sign_fit(sha_algo) 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) diff --git a/test/py/tests/vboot_forge.py b/test/py/tests/vboot_forge.py new file mode 100644 index 0000000000..0fb7ef4024 --- /dev/null +++ b/test/py/tests/vboot_forge.py @@ -0,0 +1,423 @@ +#!/usr/bin/python3 +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2020, F-Secure Corporation, https://foundry.f-secure.com +# +# pylint: disable=E1101,W0201,C0103 + +""" +Verified boot image forgery tools and utilities + +This module provides services to both take apart and regenerate FIT images +in a way that preserves all existing verified boot signatures, unless you +manipulate nodes in the process. +""" + +import struct +import binascii +from io import BytesIO + +# +# struct parsing helpers +# + +class BetterStructMeta(type): + """ + Preprocesses field definitions and creates a struct.Struct instance from them + """ + def __new__(cls, clsname, superclasses, attributedict): + if clsname != 'BetterStruct': + fields = attributedict['__fields__'] + field_types = [_[0] for _ in fields] + field_names = [_[1] for _ in fields if _[1] is not None] + attributedict['__names__'] = field_names + s = struct.Struct(attributedict.get('__endian__', '') + ''.join(field_types)) + attributedict['__struct__'] = s + attributedict['size'] = s.size + return type.__new__(cls, clsname, superclasses, attributedict) + +class BetterStruct(metaclass=BetterStructMeta): + """ + Base class for better structures + """ + def __init__(self): + for t, n in self.__fields__: + if 's' in t: + setattr(self, n, '') + elif t in ('Q', 'I', 'H', 'B'): + setattr(self, n, 0) + + @classmethod + def unpack_from(cls, buffer, offset=0): + """ + Unpack structure instance from a buffer + """ + fields = cls.__struct__.unpack_from(buffer, offset) + instance = cls() + for n, v in zip(cls.__names__, fields): + setattr(instance, n, v) + return instance + + def pack(self): + """ + Pack structure instance into bytes + """ + return self.__struct__.pack(*[getattr(self, n) for n in self.__names__]) + + def __str__(self): + items = ["'%s': %s" % (n, repr(getattr(self, n))) for n in self.__names__ if n is not None] + return '(' + ', '.join(items) + ')' + +# +# some defs for flat DT data +# + +class HeaderV17(BetterStruct): + __endian__ = '>' + __fields__ = [ + ('I', 'magic'), + ('I', 'totalsize'), + ('I', 'off_dt_struct'), + ('I', 'off_dt_strings'), + ('I', 'off_mem_rsvmap'), + ('I', 'version'), + ('I', 'last_comp_version'), + ('I', 'boot_cpuid_phys'), + ('I', 'size_dt_strings'), + ('I', 'size_dt_struct'), + ] + +class RRHeader(BetterStruct): + __endian__ = '>' + __fields__ = [ + ('Q', 'address'), + ('Q', 'size'), + ] + +class PropHeader(BetterStruct): + __endian__ = '>' + __fields__ = [ + ('I', 'value_size'), + ('I', 'name_offset'), + ] + +# magical constants for DTB format +OF_DT_HEADER = 0xd00dfeed +OF_DT_BEGIN_NODE = 1 +OF_DT_END_NODE = 2 +OF_DT_PROP = 3 +OF_DT_END = 9 + +class StringsBlock: + """ + Represents a parsed device tree string block + """ + def __init__(self, values=None): + if values is None: + self.values = [] + else: + self.values = values + + def __getitem__(self, at): + if isinstance(at, str): + offset = 0 + for value in self.values: + if value == at: + break + offset += len(value) + 1 + else: + self.values.append(at) + return offset + + if isinstance(at, int): + offset = 0 + for value in self.values: + if offset == at: + return value + offset += len(value) + 1 + raise IndexError('no string found corresponding to the given offset') + + raise TypeError('only strings and integers are accepted') + +class Prop: + """ + Represents a parsed device tree property + """ + def __init__(self, name=None, value=None): + self.name = name + self.value = value + + def clone(self): + return Prop(self.name, self.value) + + def __repr__(self): + return "" % (self.name, repr(self.value)) + +class Node: + """ + Represents a parsed device tree node + """ + def __init__(self, name=None): + self.name = name + self.props = [] + self.children = [] + + def clone(self): + o = Node(self.name) + o.props = [x.clone() for x in self.props] + o.children = [x.clone() for x in self.children] + return o + + def __getitem__(self, index): + return self.children[index] + + def __repr__(self): + return "" % (self.name, repr(self.props), repr(self.children)) + +# +# flat DT to memory +# + +def parse_strings(strings): + """ + Converts the bytes into a StringsBlock instance so it is convenient to work with + """ + strings = strings.split(b'\x00') + return StringsBlock(strings) + +def parse_struct(stream): + """ + Parses DTB structure(s) into a Node or Prop instance + """ + tag = bytearray(stream.read(4))[3] + if tag == OF_DT_BEGIN_NODE: + name = b'' + while b'\x00' not in name: + name += stream.read(4) + name = name.rstrip(b'\x00') + node = Node(name) + + item = parse_struct(stream) + while item is not None: + if isinstance(item, Node): + node.children.append(item) + elif isinstance(item, Prop): + node.props.append(item) + item = parse_struct(stream) + + return node + + if tag == OF_DT_PROP: + h = PropHeader.unpack_from(stream.read(PropHeader.size)) + length = (h.value_size + 3) & (~3) + value = stream.read(length)[:h.value_size] + prop = Prop(h.name_offset, value) + return prop + + if tag in (OF_DT_END_NODE, OF_DT_END): + return None + + raise ValueError('unexpected tag value') + +def read_fdt(fp): + """ + Reads and parses the flattened device tree (or derivatives like FIT) + """ + header = HeaderV17.unpack_from(fp.read(HeaderV17.size)) + if header.magic != OF_DT_HEADER: + raise ValueError('invalid magic value %08x; expected %08x' % (header.magic, OF_DT_HEADER)) + # TODO: read/parse reserved regions + fp.seek(header.off_dt_struct) + structs = fp.read(header.size_dt_struct) + fp.seek(header.off_dt_strings) + strings = fp.read(header.size_dt_strings) + strblock = parse_strings(strings) + root = parse_struct(BytesIO(structs)) + + return root, strblock + +# +# memory to flat DT +# + +def compose_structs_r(item): + """ + Recursive part of composing Nodes and Props into a bytearray + """ + t = bytearray() + + if isinstance(item, Node): + t.extend(struct.pack('>I', OF_DT_BEGIN_NODE)) + if isinstance(item.name, str): + item.name = bytes(item.name, 'utf-8') + name = item.name + b'\x00' + if len(name) & 3: + name += b'\x00' * (4 - (len(name) & 3)) + t.extend(name) + for p in item.props: + t.extend(compose_structs_r(p)) + for c in item.children: + t.extend(compose_structs_r(c)) + t.extend(struct.pack('>I', OF_DT_END_NODE)) + + elif isinstance(item, Prop): + t.extend(struct.pack('>I', OF_DT_PROP)) + value = item.value + h = PropHeader() + h.name_offset = item.name + if value: + h.value_size = len(value) + t.extend(h.pack()) + if len(value) & 3: + value += b'\x00' * (4 - (len(value) & 3)) + t.extend(value) + else: + h.value_size = 0 + t.extend(h.pack()) + + return t + +def compose_structs(root): + """ + Composes the parsed Nodes into a flat bytearray instance + """ + t = compose_structs_r(root) + t.extend(struct.pack('>I', OF_DT_END)) + return t + +def compose_strings(strblock): + """ + Composes the StringsBlock instance back into a bytearray instance + """ + b = bytearray() + for s in strblock.values: + b.extend(s) + b.append(0) + return bytes(b) + +def write_fdt(root, strblock, fp): + """ + Writes out a complete flattened device tree (or FIT) + """ + header = HeaderV17() + header.magic = OF_DT_HEADER + header.version = 17 + header.last_comp_version = 16 + fp.write(header.pack()) + + header.off_mem_rsvmap = fp.tell() + fp.write(RRHeader().pack()) + + structs = compose_structs(root) + header.off_dt_struct = fp.tell() + header.size_dt_struct = len(structs) + fp.write(structs) + + strings = compose_strings(strblock) + header.off_dt_strings = fp.tell() + header.size_dt_strings = len(strings) + fp.write(strings) + + header.totalsize = fp.tell() + + fp.seek(0) + fp.write(header.pack()) + +# +# pretty printing / converting to DT source +# + +def as_bytes(value): + return ' '.join(["%02X" % x for x in value]) + +def prety_print_value(value): + """ + Formats a property value as appropriate depending on the guessed data type + """ + if not value: + return '""' + if value[-1] == b'\x00': + printable = True + for x in value[:-1]: + x = ord(x) + if x != 0 and (x < 0x20 or x > 0x7F): + printable = False + break + if printable: + value = value[:-1] + return ', '.join('"' + x + '"' for x in value.split(b'\x00')) + if len(value) > 0x80: + return '[' + as_bytes(value[:0x80]) + ' ... ]' + return '[' + as_bytes(value) + ']' + +def pretty_print_r(node, strblock, indent=0): + """ + Prints out a single node, recursing further for each of its children + """ + spaces = ' ' * indent + print((spaces + '%s {' % (node.name.decode('utf-8') if node.name else '/'))) + for p in node.props: + print((spaces + ' %s = %s;' % (strblock[p.name].decode('utf-8'), prety_print_value(p.value)))) + for c in node.children: + pretty_print_r(c, strblock, indent+1) + print((spaces + '};')) + +def pretty_print(node, strblock): + """ + Generates an almost-DTS formatted printout of the parsed device tree + """ + print('/dts-v1/;') + pretty_print_r(node, strblock, 0) + +# +# manipulating the DT structure +# + +def manipulate(root, strblock): + """ + Maliciously manipulates the structure to create a crafted FIT file + """ + # locate /images/kernel at 1 (frankly, it just expects it to be the first one) + kernel_node = root[0][0] + # clone it to save time filling all the properties + fake_kernel = kernel_node.clone() + # rename the node + fake_kernel.name = b'kernel at 2' + # get rid of signatures/hashes + fake_kernel.children = [] + # NOTE: this simply replaces the first prop... either description or data + # should be good for testing purposes + fake_kernel.props[0].value = b'Super 1337 kernel\x00' + # insert the new kernel node under /images + root[0].children.append(fake_kernel) + + # modify the default configuration + root[1].props[0].value = b'conf at 2\x00' + # clone the first (only?) configuration + fake_conf = root[1][0].clone() + # rename and change kernel and fdt properties to select the crafted kernel + fake_conf.name = b'conf at 2' + fake_conf.props[0].value = b'kernel at 2\x00' + fake_conf.props[1].value = b'fdt at 1\x00' + # insert the new configuration under /configurations + root[1].children.append(fake_conf) + + return root, strblock + +def main(argv): + with open(argv[1], 'rb') as fp: + root, strblock = read_fdt(fp) + + print("Before:") + pretty_print(root, strblock) + + root, strblock = manipulate(root, strblock) + print("After:") + pretty_print(root, strblock) + + with open('blah', 'w+b') as fp: + write_fdt(root, strblock, fp) + +if __name__ == '__main__': + import sys + main(sys.argv) +# EOF From patchwork Wed Mar 18 17:44:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 243858 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Wed, 18 Mar 2020 11:44:00 -0600 Subject: [PATCH v2 06/14] test: vboot: Parameterise the test In-Reply-To: <20200318174408.77473-1-sjg@chromium.org> References: <20200318174408.77473-1-sjg@chromium.org> Message-ID: <20200318174408.77473-7-sjg@chromium.org> This test is actually made up of five separate tests. Split them out so that they appear as separate tests. Unfortunately this restarts U-Boot multiple times which adds about a second to the already-long vboot test, about 8 seconds total on my machine. We could add a special 'teardown' test afterwards but if the tests are executed out of order that would not work. Changing test_vboot into a class causes it not to be discovered and makes it different from all other tests. Signed-off-by: Simon Glass --- Changes in v2: None test/py/tests/test_vboot.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py index 22c79ef313..b1badaad73 100644 --- a/test/py/tests/test_vboot.py +++ b/test/py/tests/test_vboot.py @@ -30,13 +30,22 @@ import struct import u_boot_utils as util import vboot_forge +TESTDATA = [ + ['sha1', '', False], + ['sha1', '-pss', False], + ['sha256', '', False], + ['sha256', '-pss', False], + ['sha256', '-pss', True], +] + @pytest.mark.boardspec('sandbox') @pytest.mark.buildconfigspec('fit_signature') @pytest.mark.requiredtool('dtc') @pytest.mark.requiredtool('fdtget') @pytest.mark.requiredtool('fdtput') @pytest.mark.requiredtool('openssl') -def test_vboot(u_boot_console): + at pytest.mark.parametrize("sha_algo,padding,required", TESTDATA) +def test_vboot(u_boot_console, sha_algo, padding, required): """Test verified boot signing with mkimage and verification with 'bootm'. This works using sandbox only as it needs to update the device tree used @@ -297,11 +306,10 @@ def test_vboot(u_boot_console): # afterwards. old_dtb = cons.config.dtb cons.config.dtb = dtb - test_with_algo('sha1','') - test_with_algo('sha1','-pss') - test_with_algo('sha256','') - test_with_algo('sha256','-pss') - test_required_key('sha256','-pss') + if required: + test_required_key(sha_algo, padding) + else: + test_with_algo(sha_algo, padding) finally: # Go back to the original U-Boot with the correct dtb. cons.config.dtb = old_dtb From patchwork Wed Mar 18 17:44:01 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 243860 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Wed, 18 Mar 2020 11:44:01 -0600 Subject: [PATCH v2 07/14] image: Check hash-nodes when checking configurations In-Reply-To: <20200318174408.77473-1-sjg@chromium.org> References: <20200318174408.77473-1-sjg@chromium.org> Message-ID: <20200318174408.77473-8-sjg@chromium.org> It is currently possible to use a different configuration's signature and thus bypass the configuration check. Make sure that the configuration node that was hashed matches the one being checked, to catch this problem. Also add a proper function comment to fit_config_check_sig() and make it static. Signed-off-by: Simon Glass --- Changes in v2: None common/image-sig.c | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/common/image-sig.c b/common/image-sig.c index 13ccd50bc5..03143a4040 100644 --- a/common/image-sig.c +++ b/common/image-sig.c @@ -359,20 +359,39 @@ int fit_image_verify_required_sigs(const void *fit, int image_noffset, return 0; } -int fit_config_check_sig(const void *fit, int noffset, int required_keynode, - char **err_msgp) +/** + * fit_config_check_sig() - Check the signature of a config + * + * @fit: FIT to check + * @noffset: Offset of configuration node (e.g. /configurations/conf-1) + * @required_keynode: Offset in the control FDT of the required key node, + * if any. If this is given, then the configuration wil not + * pass verification unless that key is used. If this is + * -1 then any signature will do. + * @conf_noffset: Offset of the configuration subnode being checked (e.g. + * /configurations/conf-1/kernel) + * @err_msgp: In the event of an error, this will be pointed to a + * help error string to display to the user. + * @return 0 if all verified ok, <0 on error + */ +static int fit_config_check_sig(const void *fit, int noffset, + int required_keynode, int conf_noffset, + char **err_msgp) { char * const exc_prop[] = {"data"}; const char *prop, *end, *name; struct image_sign_info info; const uint32_t *strings; + const char *config_name; uint8_t *fit_value; int fit_value_len; + bool found_config; int max_regions; int i, prop_len; char path[200]; int count; + config_name = fit_get_name(fit, conf_noffset, NULL); debug("%s: fdt=%p, conf='%s', sig='%s'\n", __func__, gd_fdt_blob(), fit_get_name(fit, noffset, NULL), fit_get_name(gd_fdt_blob(), required_keynode, NULL)); @@ -413,9 +432,20 @@ int fit_config_check_sig(const void *fit, int noffset, int required_keynode, char *node_inc[count]; debug("Hash nodes (%d):\n", count); + found_config = false; for (name = prop, i = 0; name < end; name += strlen(name) + 1, i++) { debug(" '%s'\n", name); node_inc[i] = (char *)name; + if (!strncmp(FIT_CONFS_PATH, name, strlen(FIT_CONFS_PATH)) && + name[sizeof(FIT_CONFS_PATH) - 1] == '/' && + !strcmp(name + sizeof(FIT_CONFS_PATH), config_name)) { + debug(" (found config node %s)", config_name); + found_config = true; + } + } + if (!found_config) { + *err_msgp = "Selected config not in hashed nodes"; + return -1; } /* @@ -483,7 +513,7 @@ static int fit_config_verify_sig(const void *fit, int conf_noffset, if (!strncmp(name, FIT_SIG_NODENAME, strlen(FIT_SIG_NODENAME))) { ret = fit_config_check_sig(fit, noffset, sig_offset, - &err_msg); + conf_noffset, &err_msg); if (ret) { puts("- "); } else { From patchwork Wed Mar 18 17:44:02 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 243861 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Wed, 18 Mar 2020 11:44:02 -0600 Subject: [PATCH v2 08/14] image: Load the correct configuration in fit_check_sign In-Reply-To: <20200318174408.77473-1-sjg@chromium.org> References: <20200318174408.77473-1-sjg@chromium.org> Message-ID: <20200318174408.77473-9-sjg@chromium.org> At present bootm_host_load_images() is passed the configuration that has been verified, but ignores it and just uses the default configuration. This may not be the same. Update this function to use the selected configuration. Signed-off-by: Simon Glass --- Changes in v2: None common/bootm.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/common/bootm.c b/common/bootm.c index 902c13880d..db4362a643 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -819,7 +819,8 @@ void __weak switch_to_non_secure_mode(void) #else /* USE_HOSTCC */ #if defined(CONFIG_FIT_SIGNATURE) -static int bootm_host_load_image(const void *fit, int req_image_type) +static int bootm_host_load_image(const void *fit, int req_image_type, + int cfg_noffset) { const char *fit_uname_config = NULL; ulong data, len; @@ -831,6 +832,7 @@ static int bootm_host_load_image(const void *fit, int req_image_type) void *load_buf; int ret; + fit_uname_config = fdt_get_name(fit, cfg_noffset, NULL); memset(&images, '\0', sizeof(images)); images.verify = 1; noffset = fit_image_load(&images, (ulong)fit, @@ -878,7 +880,7 @@ int bootm_host_load_images(const void *fit, int cfg_noffset) for (i = 0; i < ARRAY_SIZE(image_types); i++) { int ret; - ret = bootm_host_load_image(fit, image_types[i]); + ret = bootm_host_load_image(fit, image_types[i], cfg_noffset); if (!err && ret && ret != -ENOENT) err = ret; } From patchwork Wed Mar 18 17:44:03 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 243862 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Wed, 18 Mar 2020 11:44:03 -0600 Subject: [PATCH v2 09/14] fit_check_sign: Allow selecting the configuration to verify In-Reply-To: <20200318174408.77473-1-sjg@chromium.org> References: <20200318174408.77473-1-sjg@chromium.org> Message-ID: <20200318174408.77473-10-sjg@chromium.org> This tool always verifies the default configuration. It is useful to be able to verify a specific one. Add a command-line flag for this and plumb the logic through. Signed-off-by: Simon Glass --- Changes in v2: None tools/fdt_host.h | 3 ++- tools/fit_check_sign.c | 8 ++++++-- tools/image-host.c | 6 ++++-- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/tools/fdt_host.h b/tools/fdt_host.h index 99b009b221..15c07c7a96 100644 --- a/tools/fdt_host.h +++ b/tools/fdt_host.h @@ -27,6 +27,7 @@ */ int fdt_remove_unused_strings(const void *old, void *new); -int fit_check_sign(const void *working_fdt, const void *key); +int fit_check_sign(const void *fit, const void *key, + const char *fit_uname_config); #endif /* __FDT_HOST_H__ */ diff --git a/tools/fit_check_sign.c b/tools/fit_check_sign.c index 4528743792..9375d5cf72 100644 --- a/tools/fit_check_sign.c +++ b/tools/fit_check_sign.c @@ -41,6 +41,7 @@ int main(int argc, char **argv) void *fit_blob; char *fdtfile = NULL; char *keyfile = NULL; + char *config_name = NULL; char cmdname[256]; int ret; void *key_blob; @@ -48,7 +49,7 @@ int main(int argc, char **argv) strncpy(cmdname, *argv, sizeof(cmdname) - 1); cmdname[sizeof(cmdname) - 1] = '\0'; - while ((c = getopt(argc, argv, "f:k:")) != -1) + while ((c = getopt(argc, argv, "f:k:c:")) != -1) switch (c) { case 'f': fdtfile = optarg; @@ -56,6 +57,9 @@ int main(int argc, char **argv) case 'k': keyfile = optarg; break; + case 'c': + config_name = optarg; + break; default: usage(cmdname); break; @@ -78,7 +82,7 @@ int main(int argc, char **argv) return EXIT_FAILURE; image_set_host_blob(key_blob); - ret = fit_check_sign(fit_blob, key_blob); + ret = fit_check_sign(fit_blob, key_blob, config_name); if (!ret) { ret = EXIT_SUCCESS; fprintf(stderr, "Signature check OK\n"); diff --git a/tools/image-host.c b/tools/image-host.c index b3ec197dc9..dfea48e894 100644 --- a/tools/image-host.c +++ b/tools/image-host.c @@ -1025,12 +1025,13 @@ int fit_add_verification_data(const char *keydir, void *keydest, void *fit, } #ifdef CONFIG_FIT_SIGNATURE -int fit_check_sign(const void *fit, const void *key) +int fit_check_sign(const void *fit, const void *key, + const char *fit_uname_config) { int cfg_noffset; int ret; - cfg_noffset = fit_conf_get_node(fit, NULL); + cfg_noffset = fit_conf_get_node(fit, fit_uname_config); if (!cfg_noffset) return -1; @@ -1039,6 +1040,7 @@ int fit_check_sign(const void *fit, const void *key) ret = fit_config_verify(fit, cfg_noffset); if (ret) return ret; + printf("Verified OK, loading images\n"); ret = bootm_host_load_images(fit, cfg_noffset); return ret; From patchwork Wed Mar 18 17:44:04 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 243866 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Wed, 18 Mar 2020 11:44:04 -0600 Subject: [PATCH v2 10/14] test: vboot: Tidy up the code a little In-Reply-To: <20200318174408.77473-1-sjg@chromium.org> References: <20200318174408.77473-1-sjg@chromium.org> Message-ID: <20200318174408.77473-11-sjg@chromium.org> Fix some long lines and comments. Use a distinct name for the 'required key' test. Signed-off-by: Simon Glass --- Changes in v2: None test/py/tests/test_vboot.py | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py index b1badaad73..817f2a99d2 100644 --- a/test/py/tests/test_vboot.py +++ b/test/py/tests/test_vboot.py @@ -91,7 +91,8 @@ def test_vboot(u_boot_console, sha_algo, padding, required): if boots: assert('sandbox: continuing, as we cannot run' in ''.join(output)) else: - assert('sandbox: continuing, as we cannot run' not in ''.join(output)) + assert('sandbox: continuing, as we cannot run' + not in ''.join(output)) def make_fit(its): """Make a new FIT from the .its source file. @@ -211,7 +212,8 @@ def test_vboot(u_boot_console, sha_algo, padding, required): 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) + 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 @@ -229,7 +231,8 @@ def test_vboot(u_boot_console, sha_algo, padding, required): util.run_and_log(cons, 'fdtput -t bx %s %s value %s' % (fit, sig_node, sig)) - run_bootm(sha_algo, 'Signed config with bad hash', 'Bad Data Hash', False) + run_bootm(sha_algo, 'Signed config with bad hash', 'Bad Data Hash', + False) cons.log.action('%s: Check bad config on the host' % sha_algo) util.run_and_log_expect_exception(cons, [fit_check_sign, '-f', fit, @@ -238,12 +241,11 @@ def test_vboot(u_boot_console, sha_algo, padding, required): def test_required_key(sha_algo, padding): """Test verified boot with the given hash algorithm. - This function test if u-boot reject an image when a required - key isn't used to sign a FIT. + This function tests if U-Boot rejects an image when a required key isn't + used to sign a FIT. Args: - sha_algo: Either 'sha1' or 'sha256', to select the algorithm to - use. + sha_algo: Either 'sha1' or 'sha256', to select the algorithm to use """ # Compile our device tree files for kernel and U-Boot. These are # regenerated here since mkimage will modify them (by adding a @@ -251,18 +253,24 @@ def test_vboot(u_boot_console, sha_algo, padding, required): dtc('sandbox-kernel.dts') dtc('sandbox-u-boot.dts') - # Build the FIT with prod key (keys required) - # Build the FIT with dev key (keys NOT required) - # The dtb contain the key prod and dev and the key prod are set as required. - # Then try to boot the FIT with dev key - # This FIT should not be accepted by u-boot because the key prod is required cons.log.action('%s: Test FIT with configs images' % sha_algo) + + # Build the FIT with prod key (keys required) and sign it. This puts the + # signature into sandbox-u-boot.dtb, marked 'required' make_fit('sign-configs-%s%s-prod.its' % (sha_algo , padding)) sign_fit(sha_algo) + + # Build the FIT with dev key (keys NOT required). This adds the + # signature into sandbox-u-boot.dtb, NOT marked 'required'. make_fit('sign-configs-%s%s.its' % (sha_algo , padding)) sign_fit(sha_algo) - run_bootm(sha_algo, 'signed configs', '', False) + # So now sandbox-u-boot.dtb two signatures, for the prod and dev keys. + # Only the prod key is set as 'required'. But FIT we just built has + # a dev signature only (sign_fit() overwrites the FIT). + # Try to boot the FIT with dev key. This FIT should not be accepted by + # U-Boot because the prod key is required. + run_bootm(sha_algo, 'required key', '', False) cons = u_boot_console tmpdir = cons.config.result_dir + '/' From patchwork Wed Mar 18 17:44:05 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 243863 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Wed, 18 Mar 2020 11:44:05 -0600 Subject: [PATCH v2 11/14] test: vboot: Fix pylint errors In-Reply-To: <20200318174408.77473-1-sjg@chromium.org> References: <20200318174408.77473-1-sjg@chromium.org> Message-ID: <20200318174408.77473-12-sjg@chromium.org> Fix various minor things noticed by pylint. Signed-off-by: Simon Glass --- Changes in v2: None test/py/tests/test_vboot.py | 53 +++++++++++++------------------------ 1 file changed, 19 insertions(+), 34 deletions(-) diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py index 817f2a99d2..0b8bd9ac0b 100644 --- a/test/py/tests/test_vboot.py +++ b/test/py/tests/test_vboot.py @@ -24,9 +24,8 @@ For configuration verification: Tests run with both SHA1 and SHA256 hashing. """ -import pytest -import sys import struct +import pytest import u_boot_utils as util import vboot_forge @@ -85,11 +84,11 @@ def test_vboot(u_boot_console, sha_algo, padding, required): with cons.log.section('Verified boot %s %s' % (sha_algo, test_type)): output = cons.run_command_list( ['host load hostfs - 100 %stest.fit' % tmpdir, - 'fdt addr 100', - 'bootm 100']) - assert(expect_string in ''.join(output)) + 'fdt addr 100', + 'bootm 100']) + assert expect_string in ''.join(output) if boots: - assert('sandbox: continuing, as we cannot run' in ''.join(output)) + assert 'sandbox: continuing, as we cannot run' in ''.join(output) else: assert('sandbox: continuing, as we cannot run' not in ''.join(output)) @@ -119,20 +118,6 @@ def test_vboot(u_boot_console, sha_algo, padding, required): util.run_and_log(cons, [mkimage, '-F', '-k', tmpdir, '-K', dtb, '-r', fit]) - def sign_fit_norequire(sha_algo): - """Sign the FIT - - Signs the FIT and writes the signature into it. It also writes the - public key into the dtb. - - Args: - sha_algo: Either 'sha1' or 'sha256', to select the algorithm to - use. - """ - cons.log.action('%s: Sign images' % sha_algo) - util.run_and_log(cons, [mkimage, '-F', '-k', tmpdir, '-K', dtb, - fit]) - def replace_fit_totalsize(size): """Replace FIT header's totalsize with something greater. @@ -171,7 +156,7 @@ def test_vboot(u_boot_console, sha_algo, padding, required): # Build the FIT, but don't sign anything yet cons.log.action('%s: Test FIT with signed images' % sha_algo) - make_fit('sign-images-%s%s.its' % (sha_algo , padding)) + make_fit('sign-images-%s%s.its' % (sha_algo, padding)) run_bootm(sha_algo, 'unsigned images', 'dev-', True) # Sign images with our dev keys @@ -182,7 +167,7 @@ def test_vboot(u_boot_console, sha_algo, padding, required): dtc('sandbox-u-boot.dts') cons.log.action('%s: Test FIT with signed configuration' % sha_algo) - make_fit('sign-configs-%s%s.its' % (sha_algo , padding)) + make_fit('sign-configs-%s%s.its' % (sha_algo, padding)) run_bootm(sha_algo, 'unsigned config', '%s+ OK' % sha_algo, True) # Sign images with our dev keys @@ -195,14 +180,14 @@ def test_vboot(u_boot_console, sha_algo, padding, required): # Make sure that U-Boot checks that the config is in the list of hashed # nodes. If it isn't, a security bypass is possible. - with open(fit, 'rb') as fp: - root, strblock = vboot_forge.read_fdt(fp) + with open(fit, 'rb') as fd: + root, strblock = vboot_forge.read_fdt(fd) root, strblock = vboot_forge.manipulate(root, strblock) - with open(fit, 'w+b') as fp: - vboot_forge.write_fdt(root, strblock, fp) - util.run_and_log_expect_exception(cons, - [fit_check_sign, '-f', fit, '-k', dtb], - 1, 'Failed to verify required signature') + with open(fit, 'w+b') as fd: + vboot_forge.write_fdt(root, strblock, fd) + util.run_and_log_expect_exception( + cons, [fit_check_sign, '-f', fit, '-k', dtb], + 1, 'Failed to verify required signature') run_bootm(sha_algo, 'forged config', 'Bad Data Hash', False) @@ -235,8 +220,9 @@ def test_vboot(u_boot_console, sha_algo, padding, required): False) cons.log.action('%s: Check bad config on the host' % sha_algo) - util.run_and_log_expect_exception(cons, [fit_check_sign, '-f', fit, - '-k', dtb], 1, 'Failed to verify required signature') + util.run_and_log_expect_exception( + cons, [fit_check_sign, '-f', fit, '-k', dtb], + 1, 'Failed to verify required signature') def test_required_key(sha_algo, padding): """Test verified boot with the given hash algorithm. @@ -257,12 +243,12 @@ def test_vboot(u_boot_console, sha_algo, padding, required): # Build the FIT with prod key (keys required) and sign it. This puts the # signature into sandbox-u-boot.dtb, marked 'required' - make_fit('sign-configs-%s%s-prod.its' % (sha_algo , padding)) + make_fit('sign-configs-%s%s-prod.its' % (sha_algo, padding)) sign_fit(sha_algo) # Build the FIT with dev key (keys NOT required). This adds the # signature into sandbox-u-boot.dtb, NOT marked 'required'. - make_fit('sign-configs-%s%s.its' % (sha_algo , padding)) + make_fit('sign-configs-%s%s.its' % (sha_algo, padding)) sign_fit(sha_algo) # So now sandbox-u-boot.dtb two signatures, for the prod and dev keys. @@ -274,7 +260,6 @@ def test_vboot(u_boot_console, sha_algo, padding, required): cons = u_boot_console tmpdir = cons.config.result_dir + '/' - tmp = tmpdir + 'vboot.tmp' datadir = cons.config.source_dir + '/test/py/tests/vboot/' fit = '%stest.fit' % tmpdir mkimage = cons.config.build_dir + '/tools/mkimage' From patchwork Wed Mar 18 17:44:06 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 243864 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Wed, 18 Mar 2020 11:44:06 -0600 Subject: [PATCH v2 12/14] image: Use constants for 'required' and 'key-name-hint' In-Reply-To: <20200318174408.77473-1-sjg@chromium.org> References: <20200318174408.77473-1-sjg@chromium.org> Message-ID: <20200318174408.77473-13-sjg@chromium.org> These are used in multiple places so update them to use a shared #define. Signed-off-by: Simon Glass Reviewed-by: Philippe Reynes --- Changes in v2: None common/image-cipher.c | 2 +- common/image-fit.c | 6 +++--- common/image-sig.c | 8 +++++--- include/image.h | 4 +++- lib/rsa/rsa-sign.c | 6 +++--- tools/image-host.c | 8 ++++---- 6 files changed, 19 insertions(+), 15 deletions(-) diff --git a/common/image-cipher.c b/common/image-cipher.c index cee3b03ee5..f50c3d31bd 100644 --- a/common/image-cipher.c +++ b/common/image-cipher.c @@ -88,7 +88,7 @@ static int fit_image_setup_decrypt(struct image_cipher_info *info, return -1; } - info->keyname = fdt_getprop(fit, cipher_noffset, "key-name-hint", NULL); + info->keyname = fdt_getprop(fit, cipher_noffset, FIT_KEY_HINT, NULL); if (!info->keyname) { printf("Can't get key name\n"); return -1; diff --git a/common/image-fit.c b/common/image-fit.c index a5c85ede8d..c8ff77526c 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -168,7 +168,7 @@ static void fit_image_print_data(const void *fit, int noffset, const char *p, int value_len; char *algo; const char *padding; - int required; + bool required; int ret, i; debug("%s %s node: '%s'\n", p, type, @@ -179,8 +179,8 @@ static void fit_image_print_data(const void *fit, int noffset, const char *p, return; } printf("%s", algo); - keyname = fdt_getprop(fit, noffset, "key-name-hint", NULL); - required = fdt_getprop(fit, noffset, "required", NULL) != NULL; + keyname = fdt_getprop(fit, noffset, FIT_KEY_HINT, NULL); + required = fdt_getprop(fit, noffset, FIT_KEY_REQUIRED, NULL) != NULL; if (keyname) printf(":%s", keyname); if (required) diff --git a/common/image-sig.c b/common/image-sig.c index 03143a4040..6563effcf3 100644 --- a/common/image-sig.c +++ b/common/image-sig.c @@ -229,7 +229,7 @@ static int fit_image_setup_verify(struct image_sign_info *info, padding_name = RSA_DEFAULT_PADDING_NAME; memset(info, '\0', sizeof(*info)); - info->keyname = fdt_getprop(fit, noffset, "key-name-hint", NULL); + info->keyname = fdt_getprop(fit, noffset, FIT_KEY_HINT, NULL); info->fit = (void *)fit; info->node_offset = noffset; info->name = algo_name; @@ -340,7 +340,8 @@ int fit_image_verify_required_sigs(const void *fit, int image_noffset, const char *required; int ret; - required = fdt_getprop(sig_blob, noffset, "required", NULL); + required = fdt_getprop(sig_blob, noffset, FIT_KEY_REQUIRED, + NULL); if (!required || strcmp(required, "image")) continue; ret = fit_image_verify_sig(fit, image_noffset, data, size, @@ -557,7 +558,8 @@ int fit_config_verify_required_sigs(const void *fit, int conf_noffset, const char *required; int ret; - required = fdt_getprop(sig_blob, noffset, "required", NULL); + required = fdt_getprop(sig_blob, noffset, FIT_KEY_REQUIRED, + NULL); if (!required || strcmp(required, "conf")) continue; ret = fit_config_verify_sig(fit, conf_noffset, sig_blob, diff --git a/include/image.h b/include/image.h index 512243f159..3ffc0fdd68 100644 --- a/include/image.h +++ b/include/image.h @@ -939,12 +939,14 @@ int booti_setup(ulong image, ulong *relocated_addr, ulong *size, #define FIT_IMAGES_PATH "/images" #define FIT_CONFS_PATH "/configurations" -/* hash/signature node */ +/* hash/signature/key node */ #define FIT_HASH_NODENAME "hash" #define FIT_ALGO_PROP "algo" #define FIT_VALUE_PROP "value" #define FIT_IGNORE_PROP "uboot-ignore" #define FIT_SIG_NODENAME "signature" +#define FIT_KEY_REQUIRED "required" +#define FIT_KEY_HINT "key-name-hint" /* cipher node */ #define FIT_CIPHER_NODENAME "cipher" diff --git a/lib/rsa/rsa-sign.c b/lib/rsa/rsa-sign.c index 6400ef63d6..580c744709 100644 --- a/lib/rsa/rsa-sign.c +++ b/lib/rsa/rsa-sign.c @@ -792,8 +792,8 @@ int rsa_add_verify_data(struct image_sign_info *info, void *keydest) } if (!ret) { - ret = fdt_setprop_string(keydest, node, "key-name-hint", - info->keyname); + ret = fdt_setprop_string(keydest, node, FIT_KEY_HINT, + info->keyname); } if (!ret) ret = fdt_setprop_u32(keydest, node, "rsa,num-bits", bits); @@ -815,7 +815,7 @@ int rsa_add_verify_data(struct image_sign_info *info, void *keydest) info->name); } if (!ret && info->require_keys) { - ret = fdt_setprop_string(keydest, node, "required", + ret = fdt_setprop_string(keydest, node, FIT_KEY_REQUIRED, info->require_keys); } done: diff --git a/tools/image-host.c b/tools/image-host.c index dfea48e894..4e57ddea96 100644 --- a/tools/image-host.c +++ b/tools/image-host.c @@ -170,7 +170,7 @@ static int fit_image_setup_sig(struct image_sign_info *info, memset(info, '\0', sizeof(*info)); info->keydir = keydir; - info->keyname = fdt_getprop(fit, noffset, "key-name-hint", NULL); + info->keyname = fdt_getprop(fit, noffset, FIT_KEY_HINT, NULL); info->fit = fit; info->node_offset = noffset; info->name = strdup(algo_name); @@ -249,7 +249,7 @@ static int fit_image_process_sig(const char *keydir, void *keydest, free(value); /* Get keyname again, as FDT has changed and invalidated our pointer */ - info.keyname = fdt_getprop(fit, noffset, "key-name-hint", NULL); + info.keyname = fdt_getprop(fit, noffset, FIT_KEY_HINT, NULL); /* * Write the public key into the supplied FDT file; this might fail @@ -337,7 +337,7 @@ static int fit_image_setup_cipher(struct image_cipher_info *info, info->keydir = keydir; /* Read the key name */ - info->keyname = fdt_getprop(fit, noffset, "key-name-hint", NULL); + info->keyname = fdt_getprop(fit, noffset, FIT_KEY_HINT, NULL); if (!info->keyname) { printf("Can't get key name for cipher '%s' in image '%s'\n", node_name, image_name); @@ -886,7 +886,7 @@ static int fit_config_process_sig(const char *keydir, void *keydest, free(region_prop); /* Get keyname again, as FDT has changed and invalidated our pointer */ - info.keyname = fdt_getprop(fit, noffset, "key-name-hint", NULL); + info.keyname = fdt_getprop(fit, noffset, FIT_KEY_HINT, NULL); /* Write the public key into the supplied FDT file */ if (keydest) { From patchwork Wed Mar 18 17:44:07 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 243865 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Wed, 18 Mar 2020 11:44:07 -0600 Subject: [PATCH v2 13/14] test: vboot: Move key creation into a function In-Reply-To: <20200318174408.77473-1-sjg@chromium.org> References: <20200318174408.77473-1-sjg@chromium.org> Message-ID: <20200318174408.77473-14-sjg@chromium.org> This code is repeated so move it into a function with a parameter. Signed-off-by: Simon Glass --- Changes in v2: None test/py/tests/test_vboot.py | 39 +++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py index 0b8bd9ac0b..91305a4f0f 100644 --- a/test/py/tests/test_vboot.py +++ b/test/py/tests/test_vboot.py @@ -138,6 +138,22 @@ def test_vboot(u_boot_console, sha_algo, padding, required): handle.write(struct.pack(">I", size)) return struct.unpack(">I", total_size)[0] + def create_rsa_pair(name): + """Generate a new RSA key paid and certificate + + Args: + name: Name of of the key (e.g. 'dev') + """ + public_exponent = 65537 + util.run_and_log(cons, 'openssl genpkey -algorithm RSA -out %s%s.key ' + '-pkeyopt rsa_keygen_bits:2048 ' + '-pkeyopt rsa_keygen_pubexp:%d' % + (tmpdir, name, public_exponent)) + + # Create a certificate containing the public key + util.run_and_log(cons, 'openssl req -batch -new -x509 -key %s%s.key ' + '-out %s%s.crt' % (tmpdir, name, tmpdir, name)) + def test_with_algo(sha_algo, padding): """Test verified boot with the given hash algorithm. @@ -268,27 +284,8 @@ def test_vboot(u_boot_console, sha_algo, padding, required): dtb = '%ssandbox-u-boot.dtb' % tmpdir sig_node = '/configurations/conf-1/signature' - # Create an RSA key pair - public_exponent = 65537 - util.run_and_log(cons, 'openssl genpkey -algorithm RSA -out %sdev.key ' - '-pkeyopt rsa_keygen_bits:2048 ' - '-pkeyopt rsa_keygen_pubexp:%d' % - (tmpdir, public_exponent)) - - # Create a certificate containing the public key - util.run_and_log(cons, 'openssl req -batch -new -x509 -key %sdev.key -out ' - '%sdev.crt' % (tmpdir, tmpdir)) - - # Create an RSA key pair (prod) - public_exponent = 65537 - util.run_and_log(cons, 'openssl genpkey -algorithm RSA -out %sprod.key ' - '-pkeyopt rsa_keygen_bits:2048 ' - '-pkeyopt rsa_keygen_pubexp:%d' % - (tmpdir, public_exponent)) - - # Create a certificate containing the public key (prod) - util.run_and_log(cons, 'openssl req -batch -new -x509 -key %sprod.key -out ' - '%sprod.crt' % (tmpdir, tmpdir)) + create_rsa_pair('dev') + create_rsa_pair('prod') # Create a number kernel image with zeroes with open('%stest-kernel.bin' % tmpdir, 'w') as fd: From patchwork Wed Mar 18 17:44:08 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 243867 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Wed, 18 Mar 2020 11:44:08 -0600 Subject: [PATCH v2 14/14] test: vboot: Reduce fake kernel size to 500 bytes In-Reply-To: <20200318174408.77473-1-sjg@chromium.org> References: <20200318174408.77473-1-sjg@chromium.org> Message-ID: <20200318174408.77473-15-sjg@chromium.org> We don't need 5KB to test things out. A smaller size makes it easier to look at the FIT with fdtdump. Signed-off-by: Simon Glass --- Changes in v2: None test/py/tests/test_vboot.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py index 91305a4f0f..e67f2b3d0f 100644 --- a/test/py/tests/test_vboot.py +++ b/test/py/tests/test_vboot.py @@ -289,7 +289,7 @@ def test_vboot(u_boot_console, sha_algo, padding, required): # Create a number kernel image with zeroes with open('%stest-kernel.bin' % tmpdir, 'w') as fd: - fd.write(5000 * chr(0)) + fd.write(500 * chr(0)) try: # We need to use our own device tree file. Remember to restore it