image-fit: Validate external data offset and size
fit_image_get_data() uses the data-position, data-offset, and data-size FIT properties without bounds checking. A crafted FIT image can specify values that cause out-of-bounds read during signature verification of an untrusted FIT. Validate that the external data offset and size are non-negative, and that the data region fits within the FIT image bounds. Signed-off-by: Anton Ivanov <anton@binarly.io> Reviewed-by: Simon Glass <sjg@chromium.org>
This commit is contained in:
Anton Ivanov
Tom Rini
+51
-2
@@ -1073,23 +1073,72 @@ int fit_image_get_data(const void *fit, int noffset, const void **data,
|
||||
int offset;
|
||||
int len;
|
||||
int ret;
|
||||
size_t fdt_total_size_aligned;
|
||||
uintptr_t max_offset;
|
||||
|
||||
if (!fit_image_get_data_position(fit, noffset, &offset)) {
|
||||
if (offset < 0) {
|
||||
printf("Invalid external data position: %d\n", offset);
|
||||
return -EINVAL;
|
||||
}
|
||||
|
||||
external_data = true;
|
||||
} else if (!fit_image_get_data_offset(fit, noffset, &offset)) {
|
||||
external_data = true;
|
||||
/*
|
||||
* For FIT with external data, figure out where
|
||||
* the external images start. This is the base
|
||||
* for the data-offset properties in each image.
|
||||
*/
|
||||
offset += ((fdt_totalsize(fit) + 3) & ~3);
|
||||
fdt_total_size_aligned = ((fdt_totalsize(fit) + 3) & ~3);
|
||||
/* The resulting offset cannot exceed INT_MAX */
|
||||
if (offset < 0 || fdt_total_size_aligned > INT_MAX - offset) {
|
||||
printf("Invalid external data offset: %d\n", offset);
|
||||
return -EINVAL;
|
||||
}
|
||||
offset += fdt_total_size_aligned;
|
||||
|
||||
external_data = true;
|
||||
}
|
||||
|
||||
if (external_data) {
|
||||
debug("External Data\n");
|
||||
|
||||
max_offset = UINTPTR_MAX - (uintptr_t)fit;
|
||||
/* Check that external data offset is within the addressable range */
|
||||
if (offset > max_offset) {
|
||||
printf("Invalid external data offset: %d\n", offset);
|
||||
return -EINVAL;
|
||||
}
|
||||
|
||||
ret = fit_image_get_data_size(fit, noffset, &len);
|
||||
if (!ret) {
|
||||
if (len < 0) {
|
||||
printf("Invalid external data size: %d\n", len);
|
||||
return -EINVAL;
|
||||
}
|
||||
/*
|
||||
* For non-signed FIT images, we can only check that
|
||||
* (offset + len) doesn't exceed the addressable range.
|
||||
* For signed FITs, we can additionally check that
|
||||
* (offset + len) doesn't exceed the allowed FIT image
|
||||
* maximum size.
|
||||
*/
|
||||
if (len > max_offset - offset
|
||||
/*
|
||||
* #if (not a runtime if) is required: FIT_SIGNATURE_MAX_SIZE
|
||||
* depends on FIT_SIGNATURE, so CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE)
|
||||
* is undefined when signing is disabled and referencing it
|
||||
* here would fail to compile.
|
||||
*/
|
||||
#if CONFIG_IS_ENABLED(FIT_SIGNATURE)
|
||||
|| offset > CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE) ||
|
||||
len > CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE) - offset
|
||||
#endif
|
||||
) {
|
||||
printf("FIT external data is out of bounds (offset=%d, size=%d)\n",
|
||||
offset, len);
|
||||
return -EINVAL;
|
||||
}
|
||||
*data = fit + offset;
|
||||
*size = len;
|
||||
}
|
||||
|
||||
@@ -589,6 +589,171 @@ def test_vboot(ubman, name, sha_algo, padding, sign_options, required,
|
||||
ubman.restart_uboot()
|
||||
|
||||
|
||||
@pytest.mark.boardspec('sandbox')
|
||||
@pytest.mark.buildconfigspec('fit_signature')
|
||||
@pytest.mark.requiredtool('dtc')
|
||||
@pytest.mark.requiredtool('fdtput')
|
||||
@pytest.mark.requiredtool('openssl')
|
||||
def test_vboot_ext_data_bounds(ubman):
|
||||
"""Test that malformed external-data properties are rejected.
|
||||
|
||||
A signed FIT with external data exposes 'data-position', 'data-offset' and
|
||||
'data-size' properties. U-Boot must validate these before hashing the image
|
||||
components, otherwise a crafted FIT could trigger an out-of-bounds access
|
||||
during signature verification.
|
||||
|
||||
These checks are independent of the hashing algorithm, so a single signing
|
||||
configuration is enough.
|
||||
|
||||
This works using sandbox only as it needs to update the device tree used
|
||||
by U-Boot to hold public keys from the signing process.
|
||||
"""
|
||||
sha_algo = 'sha256'
|
||||
|
||||
def run_bootm(test_type, expect_string):
|
||||
"""Run a 'bootm' command in U-Boot and expect it to fail.
|
||||
|
||||
This always starts a fresh U-Boot instance since the device tree may
|
||||
contain a new public key.
|
||||
|
||||
Args:
|
||||
test_type: A string identifying the test type.
|
||||
expect_string: A string which is expected in the output.
|
||||
"""
|
||||
ubman.restart_uboot()
|
||||
with ubman.log.section('Verified boot %s %s' % (sha_algo, test_type)):
|
||||
output = ubman.run_command_list(
|
||||
['host load hostfs - 100 %s' % fit,
|
||||
'fdt addr 100',
|
||||
'bootm 100'])
|
||||
assert expect_string in ''.join(output)
|
||||
assert 'sandbox: continuing, as we cannot run' not in ''.join(output)
|
||||
|
||||
def sign_fit(options):
|
||||
"""Sign the FIT
|
||||
|
||||
Signs the FIT and writes the signature into it. It also writes the
|
||||
public key into the dtb.
|
||||
|
||||
Args:
|
||||
options: Options to provide to mkimage.
|
||||
"""
|
||||
args = [mkimage, '-F', '-k', tmpdir, '-K', dtb, '-r', fit]
|
||||
if options:
|
||||
args += options.split(' ')
|
||||
ubman.log.action('%s: Sign images' % sha_algo)
|
||||
utils.run_and_log(ubman, args)
|
||||
|
||||
def create_rsa_pair(name):
|
||||
"""Generate a new RSA key pair and certificate.
|
||||
|
||||
Args:
|
||||
name: Name of the key (e.g. 'dev')
|
||||
"""
|
||||
public_exponent = 65537
|
||||
utils.run_and_log(ubman, '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
|
||||
utils.run_and_log(ubman, 'openssl req -batch -new -x509 -key %s%s.key '
|
||||
'-out %s%s.crt' % (tmpdir, name, tmpdir, name))
|
||||
|
||||
def set_external_data(prop, value):
|
||||
"""Set an external-data property of the kernel image.
|
||||
|
||||
Args:
|
||||
prop: Property name
|
||||
value: The new value of the property
|
||||
"""
|
||||
utils.run_and_log(
|
||||
ubman, 'fdtput -t x %s /images/kernel %s %#x' % (fit, prop, value)
|
||||
)
|
||||
|
||||
def make_signed_fit():
|
||||
"""Build a fresh signed FIT with external data.
|
||||
|
||||
sign_fit() overwrites the FIT, so a new one is built before each test
|
||||
case mutates its external-data properties.
|
||||
"""
|
||||
make_fit('sign-configs-%s.its' % sha_algo, ubman, mkimage, dtc_args,
|
||||
datadir, fit)
|
||||
sign_fit('-E')
|
||||
|
||||
tmpdir = os.path.join(ubman.config.result_dir, 'ext-data-bounds') + '/'
|
||||
if not os.path.exists(tmpdir):
|
||||
os.mkdir(tmpdir)
|
||||
datadir = ubman.config.source_dir + '/test/py/tests/vboot/'
|
||||
fit = '%stest.fit' % tmpdir
|
||||
mkimage = ubman.config.build_dir + '/tools/mkimage'
|
||||
dtc_args = '-I dts -O dtb -i %s' % tmpdir
|
||||
dtb = '%ssandbox-u-boot.dtb' % tmpdir
|
||||
|
||||
bcfg = ubman.config.buildconfig
|
||||
max_size = int(bcfg.get('config_fit_signature_max_size', 0x10000000), 0)
|
||||
|
||||
create_rsa_pair('dev')
|
||||
|
||||
# Create a kernel image filled with zeroes
|
||||
with open('%stest-kernel.bin' % tmpdir, 'wb') as fd:
|
||||
fd.write(500 * b'\0')
|
||||
|
||||
testcases = [
|
||||
('negative data-position',
|
||||
{'data-position': 0xffffffff}, 'Invalid external data position'),
|
||||
('negative data-offset',
|
||||
{'data-offset': 0xffffffff}, 'Invalid external data offset'),
|
||||
('negative data-size',
|
||||
{'data-size': 0xffffffff}, 'Invalid external data size'),
|
||||
('off-bounds data-position',
|
||||
{'data-position': 0x7fffffff}, 'FIT external data is out of bounds'),
|
||||
('off-bounds data-offset',
|
||||
{'data-offset': 0x10000000}, 'FIT external data is out of bounds'),
|
||||
('oversized data-size',
|
||||
{'data-size': 0x7fffffff}, 'FIT external data is out of bounds'),
|
||||
('off-bounds data-position',
|
||||
{'data-position': max_size + 1, 'data-size': 0},
|
||||
'FIT external data is out of bounds'),
|
||||
('off-bounds data-offset',
|
||||
{'data-offset': max_size + 1, 'data-size': 0},
|
||||
'FIT external data is out of bounds'),
|
||||
('oversized data-size',
|
||||
{'data-position': 0x0, 'data-size': max_size + 1},
|
||||
'FIT external data is out of bounds'),
|
||||
('in-bounds data-position',
|
||||
{'data-position': max_size, 'data-size': 0}, 'Bad Data Hash'),
|
||||
('in-bounds data-offset',
|
||||
{'data-offset': max_size, 'data-size': 0}, 'Bad Data Hash'),
|
||||
('in-bounds data-size',
|
||||
{'data-position': 0x0, 'data-size': max_size}, 'Bad Data Hash'),
|
||||
]
|
||||
|
||||
# We need to use our own device tree file. Remember to restore it
|
||||
# afterwards.
|
||||
old_dtb = ubman.config.dtb
|
||||
try:
|
||||
ubman.config.dtb = dtb
|
||||
|
||||
# Compile our device tree files for kernel and U-Boot. These are
|
||||
# regenerated here since mkimage will modify them (by adding a
|
||||
# public key) below.
|
||||
dtc('sandbox-kernel.dts', ubman, dtc_args, datadir, tmpdir, dtb)
|
||||
dtc('sandbox-u-boot.dts', ubman, dtc_args, datadir, tmpdir, dtb)
|
||||
|
||||
ubman.log.action(
|
||||
'%s: Test signed FIT with malformed external-data properties' % sha_algo)
|
||||
for desc, props, expect_string in testcases:
|
||||
make_signed_fit()
|
||||
for prop, value in props.items():
|
||||
set_external_data(prop, value)
|
||||
run_bootm('Signed config with %s' % desc, expect_string)
|
||||
finally:
|
||||
# Go back to the original U-Boot with the correct dtb.
|
||||
ubman.config.dtb = old_dtb
|
||||
ubman.restart_uboot()
|
||||
|
||||
|
||||
TESTDATA_IN = [
|
||||
['sha1-basic', 'sha1', '', None, False],
|
||||
['sha1-pad', 'sha1', '', '-E -p 0x10000', False],
|
||||
|
||||
Reference in New Issue
Block a user