* [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability
@ 2020-03-18 17:43 Simon Glass
2020-03-18 17:43 ` [PATCH v2 01/14] image: Correct comment for fit_conf_get_node() Simon Glass
` (14 more replies)
0 siblings, 15 replies; 20+ messages in thread
From: Simon Glass @ 2020-03-18 17:43 UTC (permalink / raw
To: u-boot
When booting a FIT, if 'bootm' is used without a specified configuration,
U-Boot will use the default one provided in the FIT. But it does not
actually check that the signature is for that configuration.
This means that it is possible to duplicate a configuration conf-1 to
produce conf-2 (with all the signatures intact), set the default
configuration to conf-2 and then boot the image. U-Boot will verify conf-2
(in fact since hashed-nodes specifies the conf-1 nodes it will effectively
verify conf-1). Then it will happily boot conf-2 even though it might have
a different kernel.
This series corrects this problem and adds a test to verify it. It also
updates fit_check_sign to allow the configuration to be specified.
This vulnerability was found by Dmitry Janushkevich and Andrea Barisani of
F-Secure, who also wrote the vboot_forge script included here.
This is CVE-2020-10648
Changes in v2:
- Bring in new vboot_forge file from the authors
Simon Glass (14):
image: Correct comment for fit_conf_get_node()
image: Be a little more verbose when checking signatures
image: Return an error message from fit_config_verify_sig()
test: vboot: Drop unnecessary parameter for fit_check_sign
test: vboot: Add a test for a forged configuration
test: vboot: Parameterise the test
image: Check hash-nodes when checking configurations
image: Load the correct configuration in fit_check_sign
fit_check_sign: Allow selecting the configuration to verify
test: vboot: Tidy up the code a little
test: vboot: Fix pylint errors
image: Use constants for 'required' and 'key-name-hint'
test: vboot: Move key creation into a function
test: vboot: Reduce fake kernel size to 500 bytes
common/bootm.c | 6 +-
common/image-cipher.c | 2 +-
common/image-fit.c | 26 +--
common/image-sig.c | 49 +++-
include/image.h | 24 +-
lib/rsa/rsa-sign.c | 6 +-
test/py/tests/test_vboot.py | 155 +++++++------
test/py/tests/vboot_forge.py | 423 +++++++++++++++++++++++++++++++++++
tools/fdt_host.h | 3 +-
tools/fit_check_sign.c | 8 +-
tools/image-host.c | 17 +-
11 files changed, 601 insertions(+), 118 deletions(-)
create mode 100644 test/py/tests/vboot_forge.py
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 01/14] image: Correct comment for fit_conf_get_node()
2020-03-18 17:43 [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability Simon Glass
@ 2020-03-18 17:43 ` Simon Glass
2020-03-18 17:43 ` [PATCH v2 02/14] image: Be a little more verbose when checking signatures Simon Glass
` (13 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2020-03-18 17:43 UTC (permalink / raw
To: u-boot
This should mention that conf_uname can be NULL and should be in the
header file. Fix this.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
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,
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 02/14] image: Be a little more verbose when checking signatures
2020-03-18 17:43 [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability Simon Glass
2020-03-18 17:43 ` [PATCH v2 01/14] image: Correct comment for fit_conf_get_node() Simon Glass
@ 2020-03-18 17:43 ` Simon Glass
2020-03-18 17:43 ` [PATCH v2 03/14] image: Return an error message from fit_config_verify_sig() Simon Glass
` (12 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2020-03-18 17:43 UTC (permalink / raw
To: u-boot
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 <sjg@chromium.org>
---
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;
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 03/14] image: Return an error message from fit_config_verify_sig()
2020-03-18 17:43 [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability Simon Glass
2020-03-18 17:43 ` [PATCH v2 01/14] image: Correct comment for fit_conf_get_node() Simon Glass
2020-03-18 17:43 ` [PATCH v2 02/14] image: Be a little more verbose when checking signatures Simon Glass
@ 2020-03-18 17:43 ` Simon Glass
2020-03-18 17:43 ` [PATCH v2 04/14] test: vboot: Drop unnecessary parameter for fit_check_sign Simon Glass
` (11 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2020-03-18 17:43 UTC (permalink / raw
To: u-boot
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 <sjg@chromium.org>
---
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,
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 04/14] test: vboot: Drop unnecessary parameter for fit_check_sign
2020-03-18 17:43 [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability Simon Glass
` (2 preceding siblings ...)
2020-03-18 17:43 ` [PATCH v2 03/14] image: Return an error message from fit_config_verify_sig() Simon Glass
@ 2020-03-18 17:43 ` Simon Glass
2020-03-18 17:43 ` [PATCH v2 05/14] test: vboot: Add a test for a forged configuration Simon Glass
` (10 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2020-03-18 17:43 UTC (permalink / raw
To: u-boot
This tool only uses the last -k parameter provided. Drop the earlier one
since it has no effect.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
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
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 05/14] test: vboot: Add a test for a forged configuration
2020-03-18 17:43 [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability Simon Glass
` (3 preceding siblings ...)
2020-03-18 17:43 ` [PATCH v2 04/14] test: vboot: Drop unnecessary parameter for fit_check_sign Simon Glass
@ 2020-03-18 17:43 ` Simon Glass
2020-03-18 17:44 ` [PATCH v2 06/14] test: vboot: Parameterise the test Simon Glass
` (9 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2020-03-18 17:43 UTC (permalink / raw
To: u-boot
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 <sjg@chromium.org>
---
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 "<Prop(name='%s', value=%s>" % (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 "<Node('%s'), %s, %s>" % (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
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 06/14] test: vboot: Parameterise the test
2020-03-18 17:43 [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability Simon Glass
` (4 preceding siblings ...)
2020-03-18 17:43 ` [PATCH v2 05/14] test: vboot: Add a test for a forged configuration Simon Glass
@ 2020-03-18 17:44 ` Simon Glass
2020-03-18 17:44 ` [PATCH v2 07/14] image: Check hash-nodes when checking configurations Simon Glass
` (8 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2020-03-18 17:44 UTC (permalink / raw
To: u-boot
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 <sjg@chromium.org>
---
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
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 07/14] image: Check hash-nodes when checking configurations
2020-03-18 17:43 [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability Simon Glass
` (5 preceding siblings ...)
2020-03-18 17:44 ` [PATCH v2 06/14] test: vboot: Parameterise the test Simon Glass
@ 2020-03-18 17:44 ` Simon Glass
2020-03-18 17:44 ` [PATCH v2 08/14] image: Load the correct configuration in fit_check_sign Simon Glass
` (7 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2020-03-18 17:44 UTC (permalink / raw
To: u-boot
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 <sjg@chromium.org>
---
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 {
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 08/14] image: Load the correct configuration in fit_check_sign
2020-03-18 17:43 [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability Simon Glass
` (6 preceding siblings ...)
2020-03-18 17:44 ` [PATCH v2 07/14] image: Check hash-nodes when checking configurations Simon Glass
@ 2020-03-18 17:44 ` Simon Glass
2020-03-18 17:44 ` [PATCH v2 09/14] fit_check_sign: Allow selecting the configuration to verify Simon Glass
` (6 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2020-03-18 17:44 UTC (permalink / raw
To: u-boot
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 <sjg@chromium.org>
---
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;
}
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 09/14] fit_check_sign: Allow selecting the configuration to verify
2020-03-18 17:43 [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability Simon Glass
` (7 preceding siblings ...)
2020-03-18 17:44 ` [PATCH v2 08/14] image: Load the correct configuration in fit_check_sign Simon Glass
@ 2020-03-18 17:44 ` Simon Glass
2020-03-18 17:44 ` [PATCH v2 10/14] test: vboot: Tidy up the code a little Simon Glass
` (5 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2020-03-18 17:44 UTC (permalink / raw
To: u-boot
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 <sjg@chromium.org>
---
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;
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 10/14] test: vboot: Tidy up the code a little
2020-03-18 17:43 [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability Simon Glass
` (8 preceding siblings ...)
2020-03-18 17:44 ` [PATCH v2 09/14] fit_check_sign: Allow selecting the configuration to verify Simon Glass
@ 2020-03-18 17:44 ` Simon Glass
2020-03-18 17:44 ` [PATCH v2 11/14] test: vboot: Fix pylint errors Simon Glass
` (4 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2020-03-18 17:44 UTC (permalink / raw
To: u-boot
Fix some long lines and comments. Use a distinct name for the
'required key' test.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
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 + '/'
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 11/14] test: vboot: Fix pylint errors
2020-03-18 17:43 [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability Simon Glass
` (9 preceding siblings ...)
2020-03-18 17:44 ` [PATCH v2 10/14] test: vboot: Tidy up the code a little Simon Glass
@ 2020-03-18 17:44 ` Simon Glass
2020-03-18 17:44 ` [PATCH v2 12/14] image: Use constants for 'required' and 'key-name-hint' Simon Glass
` (3 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2020-03-18 17:44 UTC (permalink / raw
To: u-boot
Fix various minor things noticed by pylint.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
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'
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 12/14] image: Use constants for 'required' and 'key-name-hint'
2020-03-18 17:43 [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability Simon Glass
` (10 preceding siblings ...)
2020-03-18 17:44 ` [PATCH v2 11/14] test: vboot: Fix pylint errors Simon Glass
@ 2020-03-18 17:44 ` Simon Glass
2020-03-18 18:28 ` Philippe REYNES
2020-03-18 17:44 ` [PATCH v2 13/14] test: vboot: Move key creation into a function Simon Glass
` (2 subsequent siblings)
14 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2020-03-18 17:44 UTC (permalink / raw
To: u-boot
These are used in multiple places so update them to use a shared #define.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
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) {
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 13/14] test: vboot: Move key creation into a function
2020-03-18 17:43 [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability Simon Glass
` (11 preceding siblings ...)
2020-03-18 17:44 ` [PATCH v2 12/14] image: Use constants for 'required' and 'key-name-hint' Simon Glass
@ 2020-03-18 17:44 ` Simon Glass
2020-03-18 17:44 ` [PATCH v2 14/14] test: vboot: Reduce fake kernel size to 500 bytes Simon Glass
2020-03-30 23:11 ` [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability Simon Glass
14 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2020-03-18 17:44 UTC (permalink / raw
To: u-boot
This code is repeated so move it into a function with a parameter.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
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:
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 14/14] test: vboot: Reduce fake kernel size to 500 bytes
2020-03-18 17:43 [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability Simon Glass
` (12 preceding siblings ...)
2020-03-18 17:44 ` [PATCH v2 13/14] test: vboot: Move key creation into a function Simon Glass
@ 2020-03-18 17:44 ` Simon Glass
2020-03-30 23:11 ` [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability Simon Glass
14 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2020-03-18 17:44 UTC (permalink / raw
To: u-boot
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 <sjg@chromium.org>
---
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
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 12/14] image: Use constants for 'required' and 'key-name-hint'
2020-03-18 17:44 ` [PATCH v2 12/14] image: Use constants for 'required' and 'key-name-hint' Simon Glass
@ 2020-03-18 18:28 ` Philippe REYNES
0 siblings, 0 replies; 20+ messages in thread
From: Philippe REYNES @ 2020-03-18 18:28 UTC (permalink / raw
To: u-boot
> Objet: [PATCH v2 12/14] image: Use constants for 'required' and 'key-name-hint'
> These are used in multiple places so update them to use a shared #define.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Philippe Reynes <philippe.reynes@softathome.com>
> ---
>
> 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) {
> --
> 2.25.1.481.gfbce0eb801-goog
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability
2020-03-18 17:43 [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability Simon Glass
` (13 preceding siblings ...)
2020-03-18 17:44 ` [PATCH v2 14/14] test: vboot: Reduce fake kernel size to 500 bytes Simon Glass
@ 2020-03-30 23:11 ` Simon Glass
2020-03-31 13:26 ` Tom Rini
14 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2020-03-30 23:11 UTC (permalink / raw
To: u-boot
Hi,
On Wed, 18 Mar 2020 at 11:44, Simon Glass <sjg@chromium.org> wrote:
>
> When booting a FIT, if 'bootm' is used without a specified configuration,
> U-Boot will use the default one provided in the FIT. But it does not
> actually check that the signature is for that configuration.
>
> This means that it is possible to duplicate a configuration conf-1 to
> produce conf-2 (with all the signatures intact), set the default
> configuration to conf-2 and then boot the image. U-Boot will verify conf-2
> (in fact since hashed-nodes specifies the conf-1 nodes it will effectively
> verify conf-1). Then it will happily boot conf-2 even though it might have
> a different kernel.
>
> This series corrects this problem and adds a test to verify it. It also
> updates fit_check_sign to allow the configuration to be specified.
>
> This vulnerability was found by Dmitry Janushkevich and Andrea Barisani of
> F-Secure, who also wrote the vboot_forge script included here.
>
> This is CVE-2020-10648
>
> Changes in v2:
> - Bring in new vboot_forge file from the authors
>
> Simon Glass (14):
> image: Correct comment for fit_conf_get_node()
> image: Be a little more verbose when checking signatures
> image: Return an error message from fit_config_verify_sig()
> test: vboot: Drop unnecessary parameter for fit_check_sign
> test: vboot: Add a test for a forged configuration
> test: vboot: Parameterise the test
> image: Check hash-nodes when checking configurations
> image: Load the correct configuration in fit_check_sign
> fit_check_sign: Allow selecting the configuration to verify
> test: vboot: Tidy up the code a little
> test: vboot: Fix pylint errors
> image: Use constants for 'required' and 'key-name-hint'
> test: vboot: Move key creation into a function
> test: vboot: Reduce fake kernel size to 500 bytes
>
> common/bootm.c | 6 +-
> common/image-cipher.c | 2 +-
> common/image-fit.c | 26 +--
> common/image-sig.c | 49 +++-
> include/image.h | 24 +-
> lib/rsa/rsa-sign.c | 6 +-
> test/py/tests/test_vboot.py | 155 +++++++------
> test/py/tests/vboot_forge.py | 423 +++++++++++++++++++++++++++++++++++
> tools/fdt_host.h | 3 +-
> tools/fit_check_sign.c | 8 +-
> tools/image-host.c | 17 +-
> 11 files changed, 601 insertions(+), 118 deletions(-)
> create mode 100644 test/py/tests/vboot_forge.py
This is applied to dm/master.
Tom, shall I send a pull request?
Regards,
Simon
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability
2020-03-30 23:11 ` [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability Simon Glass
@ 2020-03-31 13:26 ` Tom Rini
0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2020-03-31 13:26 UTC (permalink / raw
To: u-boot
On Mon, Mar 30, 2020 at 05:11:38PM -0600, Simon Glass wrote:
> Hi,
>
> On Wed, 18 Mar 2020 at 11:44, Simon Glass <sjg@chromium.org> wrote:
> >
> > When booting a FIT, if 'bootm' is used without a specified configuration,
> > U-Boot will use the default one provided in the FIT. But it does not
> > actually check that the signature is for that configuration.
> >
> > This means that it is possible to duplicate a configuration conf-1 to
> > produce conf-2 (with all the signatures intact), set the default
> > configuration to conf-2 and then boot the image. U-Boot will verify conf-2
> > (in fact since hashed-nodes specifies the conf-1 nodes it will effectively
> > verify conf-1). Then it will happily boot conf-2 even though it might have
> > a different kernel.
> >
> > This series corrects this problem and adds a test to verify it. It also
> > updates fit_check_sign to allow the configuration to be specified.
> >
> > This vulnerability was found by Dmitry Janushkevich and Andrea Barisani of
> > F-Secure, who also wrote the vboot_forge script included here.
> >
> > This is CVE-2020-10648
> >
> > Changes in v2:
> > - Bring in new vboot_forge file from the authors
> >
> > Simon Glass (14):
> > image: Correct comment for fit_conf_get_node()
> > image: Be a little more verbose when checking signatures
> > image: Return an error message from fit_config_verify_sig()
> > test: vboot: Drop unnecessary parameter for fit_check_sign
> > test: vboot: Add a test for a forged configuration
> > test: vboot: Parameterise the test
> > image: Check hash-nodes when checking configurations
> > image: Load the correct configuration in fit_check_sign
> > fit_check_sign: Allow selecting the configuration to verify
> > test: vboot: Tidy up the code a little
> > test: vboot: Fix pylint errors
> > image: Use constants for 'required' and 'key-name-hint'
> > test: vboot: Move key creation into a function
> > test: vboot: Reduce fake kernel size to 500 bytes
> >
> > common/bootm.c | 6 +-
> > common/image-cipher.c | 2 +-
> > common/image-fit.c | 26 +--
> > common/image-sig.c | 49 +++-
> > include/image.h | 24 +-
> > lib/rsa/rsa-sign.c | 6 +-
> > test/py/tests/test_vboot.py | 155 +++++++------
> > test/py/tests/vboot_forge.py | 423 +++++++++++++++++++++++++++++++++++
> > tools/fdt_host.h | 3 +-
> > tools/fit_check_sign.c | 8 +-
> > tools/image-host.c | 17 +-
> > 11 files changed, 601 insertions(+), 118 deletions(-)
> > create mode 100644 test/py/tests/vboot_forge.py
>
> This is applied to dm/master.
>
> Tom, shall I send a pull request?
Yes please, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200331/8494af8e/attachment.sig>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability
@ 2020-10-16 7:39 guoyujie
2020-10-27 16:11 ` Simon Glass
0 siblings, 1 reply; 20+ messages in thread
From: guoyujie @ 2020-10-16 7:39 UTC (permalink / raw
To: u-boot
Hi,I'm a developer using u-boot-2020.01,where can I get the patch fixing CVE-2020-10648 for u-boot-2020.01?
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability
2020-10-16 7:39 guoyujie
@ 2020-10-27 16:11 ` Simon Glass
0 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2020-10-27 16:11 UTC (permalink / raw
To: u-boot
Hi,
On Fri, 16 Oct 2020 at 01:40, guoyujie (C) <guoyujie6@hisilicon.com> wrote:
>
> Hi,I?m a developer using u-boot-2020.01,where can I get the patch fixing CVE-2020-10648 for u-boot-2020.01?
Upstream commit is 0e29648f8e7 and following
Cover letter is here:
https://lists.denx.de/pipermail/u-boot/2020-March/403409.html
Patchwork here:
http://patchwork.ozlabs.org/project/uboot/list/?series=165191&state=*
Unfortunately only the cover letter mentions CVE-2020-10648 and that
did not make it into the commits.
Regards,
Simon
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2020-10-27 16:11 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-18 17:43 [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability Simon Glass
2020-03-18 17:43 ` [PATCH v2 01/14] image: Correct comment for fit_conf_get_node() Simon Glass
2020-03-18 17:43 ` [PATCH v2 02/14] image: Be a little more verbose when checking signatures Simon Glass
2020-03-18 17:43 ` [PATCH v2 03/14] image: Return an error message from fit_config_verify_sig() Simon Glass
2020-03-18 17:43 ` [PATCH v2 04/14] test: vboot: Drop unnecessary parameter for fit_check_sign Simon Glass
2020-03-18 17:43 ` [PATCH v2 05/14] test: vboot: Add a test for a forged configuration Simon Glass
2020-03-18 17:44 ` [PATCH v2 06/14] test: vboot: Parameterise the test Simon Glass
2020-03-18 17:44 ` [PATCH v2 07/14] image: Check hash-nodes when checking configurations Simon Glass
2020-03-18 17:44 ` [PATCH v2 08/14] image: Load the correct configuration in fit_check_sign Simon Glass
2020-03-18 17:44 ` [PATCH v2 09/14] fit_check_sign: Allow selecting the configuration to verify Simon Glass
2020-03-18 17:44 ` [PATCH v2 10/14] test: vboot: Tidy up the code a little Simon Glass
2020-03-18 17:44 ` [PATCH v2 11/14] test: vboot: Fix pylint errors Simon Glass
2020-03-18 17:44 ` [PATCH v2 12/14] image: Use constants for 'required' and 'key-name-hint' Simon Glass
2020-03-18 18:28 ` Philippe REYNES
2020-03-18 17:44 ` [PATCH v2 13/14] test: vboot: Move key creation into a function Simon Glass
2020-03-18 17:44 ` [PATCH v2 14/14] test: vboot: Reduce fake kernel size to 500 bytes Simon Glass
2020-03-30 23:11 ` [PATCH v2 00/14] vboot: Fix forged-configuration vulnerability Simon Glass
2020-03-31 13:26 ` Tom Rini
-- strict thread matches above, loose matches on Subject: below --
2020-10-16 7:39 guoyujie
2020-10-27 16:11 ` Simon Glass
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.