verified boot changes since 2020.04
Hi,
I'm trying to keep our board in sync with upstream, but when trying to port it to v2020.10-rc4, the kernel verification fails:
## Loading kernel from FIT Image at 03000000 ... Using 'conf-def.dtb' configuration Verifying Hash Integrity ... sha1,rsa2048:dev- error! Verification failed for '<NULL>' hash node in 'conf-def.dtb' config node Failed to verify required signature 'key-dev' Bad Data Hash ERROR: can't get kernel image!
Our current board code is based on v2020.04 where everything works as expected.
I have checked that U-Boot's .dtb has identical /signature nodes between the two versions, both from within U-Boot with 'fdt print /signature' and using fdtdump:
=> fdt print /signature signature { key-dev { required = "conf"; algo = "sha1,rsa2048"; rsa,r-squared = ... rsa,modulus = ... rsa,exponent = ... rsa,n0-inverse = ... rsa,num-bits = <0x00000800>; key-name-hint = "dev"; }; };
(except that apparently the new version of U-Boot no longer abbreviates the r-squared and modulus values to an "* adress [length]" format).
I wanted to try using tools/fit_check_sign as a quick way to bisect this, unfortunately the v2020.10-rc4 version (also) says that the kernel image is correctly signed.
Does anyone have a crystal ball that says what might have changed to cause this? The board in question is based on mpc8309, i.e. big-endian powerpc.
Thanks, Rasmus
On 05/10/2020 16.10, Rasmus Villemoes wrote:
I wanted to try using tools/fit_check_sign as a quick way to bisect this, unfortunately the v2020.10-rc4 version (also) says that the kernel image is correctly signed.
Does anyone have a crystal ball that says what might have changed to cause this? The board in question is based on mpc8309, i.e. big-endian powerpc.
I had a hunch that is was an endianness issue since fit_check_sign (run on the little-endian build host) reported success, and indeed it is.
Rasmus Villemoes (1): rsa: fix retrieving public exponent on big-endian systems
lib/rsa/rsa-mod-exp.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
Commit fdf0819afb (rsa: fix alignment issue when getting public exponent) changed the logic to avoid doing an 8-byte access to a possibly-not-8-byte-aligned address.
However, using rsa_convert_big_endian is wrong: That function converts an array of big-endian (32-bit) words with the most significant word first (aka a BE byte array) to an array of cpu-endian words with the least significant word first. While the exponent is indeed _stored_ as a big-endian 64-bit word (two BE words with MSW first), we want to extract it as a cpu-endian 64 bit word. On a little-endian host, swapping the words and byte-swapping each 32-bit word works, because that's the same as byte-swapping the whole 64 bit word. But on a big-endian host, the fdt32_to_cpu are no-ops, but rsa_convert_big_endian() still does the word-swapping, breaking verified boot.
To fix that, while still ensuring we don't do unaligned accesses, add a little helper that first memcpy's the bytes to a local fdt64_t, then applies fdt64_to_cpu(). [The name is chosen based on the [bl]eXX_to_cpup in linux/byteorder/generic.h].
Fixes: fdf0819afb ("rsa: fix alignment issue when getting public exponent") Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk --- lib/rsa/rsa-mod-exp.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/lib/rsa/rsa-mod-exp.c b/lib/rsa/rsa-mod-exp.c index a437cbe26f..78c688d14c 100644 --- a/lib/rsa/rsa-mod-exp.c +++ b/lib/rsa/rsa-mod-exp.c @@ -25,6 +25,14 @@ #define get_unaligned_be32(a) fdt32_to_cpu(*(uint32_t *)a) #define put_unaligned_be32(a, b) (*(uint32_t *)(b) = cpu_to_fdt32(a))
+static inline uint64_t fdt64_to_cpup(const void *p) +{ + fdt64_t w; + + memcpy(&w, p, sizeof(w)); + return fdt64_to_cpu(w); +} + /* Default public exponent for backward compatibility */ #define RSA_DEFAULT_PUBEXP 65537
@@ -263,8 +271,7 @@ int rsa_mod_exp_sw(const uint8_t *sig, uint32_t sig_len, if (!prop->public_exponent) key.exponent = RSA_DEFAULT_PUBEXP; else - rsa_convert_big_endian((uint32_t *)&key.exponent, - prop->public_exponent, 2); + key.exponent = fdt64_to_cpup(prop->public_exponent);
if (!key.len || !prop->modulus || !prop->rr) { debug("%s: Missing RSA key info", __func__);
Hi Rasmus,
On Tue, 6 Oct 2020 at 04:10, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
Commit fdf0819afb (rsa: fix alignment issue when getting public exponent) changed the logic to avoid doing an 8-byte access to a possibly-not-8-byte-aligned address.
However, using rsa_convert_big_endian is wrong: That function converts an array of big-endian (32-bit) words with the most significant word first (aka a BE byte array) to an array of cpu-endian words with the least significant word first. While the exponent is indeed _stored_ as a big-endian 64-bit word (two BE words with MSW first), we want to extract it as a cpu-endian 64 bit word. On a little-endian host, swapping the words and byte-swapping each 32-bit word works, because that's the same as byte-swapping the whole 64 bit word. But on a big-endian host, the fdt32_to_cpu are no-ops, but rsa_convert_big_endian() still does the word-swapping, breaking verified boot.
To fix that, while still ensuring we don't do unaligned accesses, add a little helper that first memcpy's the bytes to a local fdt64_t, then applies fdt64_to_cpu(). [The name is chosen based on the [bl]eXX_to_cpup in linux/byteorder/generic.h].
Fixes: fdf0819afb ("rsa: fix alignment issue when getting public exponent") Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
lib/rsa/rsa-mod-exp.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Is there a way to add a test for this?
Regards, Simon
On 07/10/2020 00.02, Simon Glass wrote:
Hi Rasmus,
On Tue, 6 Oct 2020 at 04:10, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
Commit fdf0819afb (rsa: fix alignment issue when getting public exponent) changed the logic to avoid doing an 8-byte access to a possibly-not-8-byte-aligned address.
However, using rsa_convert_big_endian is wrong: That function converts an array of big-endian (32-bit) words with the most significant word first (aka a BE byte array) to an array of cpu-endian words with the least significant word first. While the exponent is indeed _stored_ as a big-endian 64-bit word (two BE words with MSW first), we want to extract it as a cpu-endian 64 bit word. On a little-endian host, swapping the words and byte-swapping each 32-bit word works, because that's the same as byte-swapping the whole 64 bit word. But on a big-endian host, the fdt32_to_cpu are no-ops, but rsa_convert_big_endian() still does the word-swapping, breaking verified boot.
To fix that, while still ensuring we don't do unaligned accesses, add a little helper that first memcpy's the bytes to a local fdt64_t, then applies fdt64_to_cpu(). [The name is chosen based on the [bl]eXX_to_cpup in linux/byteorder/generic.h].
Fixes: fdf0819afb ("rsa: fix alignment issue when getting public exponent") Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
lib/rsa/rsa-mod-exp.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Is there a way to add a test for this?
Not that I can think of, other than finding some BE board and hooking it up in some CI. Apparently not very many people use verified boot on BE platforms :( or at least they don't follow upstream U-Boot closely.
Rasmus
On Wed, Oct 07, 2020 at 12:17:56AM +0200, Rasmus Villemoes wrote:
On 07/10/2020 00.02, Simon Glass wrote:
Hi Rasmus,
On Tue, 6 Oct 2020 at 04:10, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
Commit fdf0819afb (rsa: fix alignment issue when getting public exponent) changed the logic to avoid doing an 8-byte access to a possibly-not-8-byte-aligned address.
However, using rsa_convert_big_endian is wrong: That function converts an array of big-endian (32-bit) words with the most significant word first (aka a BE byte array) to an array of cpu-endian words with the least significant word first. While the exponent is indeed _stored_ as a big-endian 64-bit word (two BE words with MSW first), we want to extract it as a cpu-endian 64 bit word. On a little-endian host, swapping the words and byte-swapping each 32-bit word works, because that's the same as byte-swapping the whole 64 bit word. But on a big-endian host, the fdt32_to_cpu are no-ops, but rsa_convert_big_endian() still does the word-swapping, breaking verified boot.
To fix that, while still ensuring we don't do unaligned accesses, add a little helper that first memcpy's the bytes to a local fdt64_t, then applies fdt64_to_cpu(). [The name is chosen based on the [bl]eXX_to_cpup in linux/byteorder/generic.h].
Fixes: fdf0819afb ("rsa: fix alignment issue when getting public exponent") Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
lib/rsa/rsa-mod-exp.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Is there a way to add a test for this?
Not that I can think of, other than finding some BE board and hooking it up in some CI. Apparently not very many people use verified boot on BE platforms :( or at least they don't follow upstream U-Boot closely.
We have tests for verified boot for sandbox. Can we not expand them to run on qemu* including ppce500?
On 09/10/2020 15.08, Tom Rini wrote:
On Wed, Oct 07, 2020 at 12:17:56AM +0200, Rasmus Villemoes wrote:
On 07/10/2020 00.02, Simon Glass wrote:
Reviewed-by: Simon Glass sjg@chromium.org
Is there a way to add a test for this?
Not that I can think of, other than finding some BE board and hooking it up in some CI. Apparently not very many people use verified boot on BE platforms :( or at least they don't follow upstream U-Boot closely.
We have tests for verified boot for sandbox. Can we not expand them to run on qemu* including ppce500?
Perhaps. I didn't know sandbox was supposed to be buildable for other arches than x86ish. I just tried doing the naive thing,
$ export ARCH=powerpc $ export CROSS_COMPILE=powerpc-linux-gnu- $ make sandbox_defconfig $ make -j8 include/config_distro_bootcmd.h:335:3: error: #error "sandbox EFI support is only supported on ARM and x86"
After trimming the .config to git rid of that, I get a million other warnings and errors, so that doesn't seem to be the right way.
Can we please apply the patch to unbreak actual boards even if there's no regression test for it?
Rasmus
On Mon, Oct 12, 2020 at 09:04:28AM +0200, Rasmus Villemoes wrote:
On 09/10/2020 15.08, Tom Rini wrote:
On Wed, Oct 07, 2020 at 12:17:56AM +0200, Rasmus Villemoes wrote:
On 07/10/2020 00.02, Simon Glass wrote:
Reviewed-by: Simon Glass sjg@chromium.org
Is there a way to add a test for this?
Not that I can think of, other than finding some BE board and hooking it up in some CI. Apparently not very many people use verified boot on BE platforms :( or at least they don't follow upstream U-Boot closely.
We have tests for verified boot for sandbox. Can we not expand them to run on qemu* including ppce500?
Perhaps. I didn't know sandbox was supposed to be buildable for other arches than x86ish. I just tried doing the naive thing,
$ export ARCH=powerpc $ export CROSS_COMPILE=powerpc-linux-gnu- $ make sandbox_defconfig $ make -j8 include/config_distro_bootcmd.h:335:3: error: #error "sandbox EFI support is only supported on ARM and x86"
After trimming the .config to git rid of that, I get a million other warnings and errors, so that doesn't seem to be the right way.
Sorry, I meant the literal QEMU targets that we use.
Can we please apply the patch to unbreak actual boards even if there's no regression test for it?
Yes, sorry it wasn't clear. I will pick this up soon, with a bunch of other stuff.
Hi Rasmus,
On Mon, 12 Oct 2020 at 01:04, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
On 09/10/2020 15.08, Tom Rini wrote:
On Wed, Oct 07, 2020 at 12:17:56AM +0200, Rasmus Villemoes wrote:
On 07/10/2020 00.02, Simon Glass wrote:
Reviewed-by: Simon Glass sjg@chromium.org
Is there a way to add a test for this?
Not that I can think of, other than finding some BE board and hooking it up in some CI. Apparently not very many people use verified boot on BE platforms :( or at least they don't follow upstream U-Boot closely.
We have tests for verified boot for sandbox. Can we not expand them to run on qemu* including ppce500?
Perhaps. I didn't know sandbox was supposed to be buildable for other arches than x86ish. I just tried doing the naive thing,
$ export ARCH=powerpc $ export CROSS_COMPILE=powerpc-linux-gnu- $ make sandbox_defconfig $ make -j8 include/config_distro_bootcmd.h:335:3: error: #error "sandbox EFI support is only supported on ARM and x86"
After trimming the .config to git rid of that, I get a million other warnings and errors, so that doesn't seem to be the right way.
For this you would need to build on a power PC linux machine. I think someone did get it running on ARM but I'm not sure if anyone else has tried with
As Tom said, he was referring to building a board target (not sandbox) and running a test under qemu. Probably could just have a little test that calls rsa_mod_exp_sw(), I suppose. But it is a lot of setup for just a small thing.
Can we please apply the patch to unbreak actual boards even if there's no regression test for it?
Regards, Simon
On Tue, Oct 06, 2020 at 12:09:45PM +0200, Rasmus Villemoes wrote:
Commit fdf0819afb (rsa: fix alignment issue when getting public exponent) changed the logic to avoid doing an 8-byte access to a possibly-not-8-byte-aligned address.
However, using rsa_convert_big_endian is wrong: That function converts an array of big-endian (32-bit) words with the most significant word first (aka a BE byte array) to an array of cpu-endian words with the least significant word first. While the exponent is indeed _stored_ as a big-endian 64-bit word (two BE words with MSW first), we want to extract it as a cpu-endian 64 bit word. On a little-endian host, swapping the words and byte-swapping each 32-bit word works, because that's the same as byte-swapping the whole 64 bit word. But on a big-endian host, the fdt32_to_cpu are no-ops, but rsa_convert_big_endian() still does the word-swapping, breaking verified boot.
To fix that, while still ensuring we don't do unaligned accesses, add a little helper that first memcpy's the bytes to a local fdt64_t, then applies fdt64_to_cpu(). [The name is chosen based on the [bl]eXX_to_cpup in linux/byteorder/generic.h].
Fixes: fdf0819afb ("rsa: fix alignment issue when getting public exponent") Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
Hi Rasmus,
On Mon, 5 Oct 2020 at 08:10, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
Hi,
I'm trying to keep our board in sync with upstream, but when trying to port it to v2020.10-rc4, the kernel verification fails:
## Loading kernel from FIT Image at 03000000 ... Using 'conf-def.dtb' configuration Verifying Hash Integrity ... sha1,rsa2048:dev- error! Verification failed for '<NULL>' hash node in 'conf-def.dtb' config node Failed to verify required signature 'key-dev' Bad Data Hash ERROR: can't get kernel image!
Our current board code is based on v2020.04 where everything works as expected.
I have checked that U-Boot's .dtb has identical /signature nodes between the two versions, both from within U-Boot with 'fdt print /signature' and using fdtdump:
=> fdt print /signature signature { key-dev { required = "conf"; algo = "sha1,rsa2048"; rsa,r-squared = ... rsa,modulus = ... rsa,exponent = ... rsa,n0-inverse = ... rsa,num-bits = <0x00000800>; key-name-hint = "dev"; }; };
(except that apparently the new version of U-Boot no longer abbreviates the r-squared and modulus values to an "* adress [length]" format).
I wanted to try using tools/fit_check_sign as a quick way to bisect this, unfortunately the v2020.10-rc4 version (also) says that the kernel image is correctly signed.
Does anyone have a crystal ball that says what might have changed to cause this? The board in question is based on mpc8309, i.e. big-endian powerpc.
It seems that big endian was broken and you have sent a patch, thank you.
- Simon
participants (3)
-
Rasmus Villemoes -
Simon Glass -
Tom Rini