[BUG] ut lib lib_rsa_verify_valid crashes on qemu_arm if RSA_VERIFY_WITH_PKEY=y
Hello Takahiro,
when trying to execute command
ut lib lib_rsa_verify_valid
on qemu_arm_defconfig with CONFIG_UNIT_TEST=y and CONFIG_RSA_VERIFY_WITH_PKEY=y it crashes in
free((void *)prop->modulus) called from rsa_free_key_prop() called from rsa_verify_key() called from rsa_verify_with_pkey().
Without CONFIG_RSA_VERIFY_WITH_PKEY=y the problem does not occur. On qemu_arm64_defconfig the problem does not occur.
Could you, please, have a look.
Best regards
Heinrich
Heinrich,
On Mon, Jun 08, 2020 at 11:08:53PM +0200, Heinrich Schuchardt wrote:
Hello Takahiro,
when trying to execute command
ut lib lib_rsa_verify_valid
on qemu_arm_defconfig with CONFIG_UNIT_TEST=y and CONFIG_RSA_VERIFY_WITH_PKEY=y it crashes in
free((void *)prop->modulus) called from rsa_free_key_prop() called from rsa_verify_key() called from rsa_verify_with_pkey().
Without CONFIG_RSA_VERIFY_WITH_PKEY=y the problem does not occur. On qemu_arm64_defconfig the problem does not occur.
I can't reproduce your problem on v2020.07-rc4 exactly with qemu_arm64_defconfig + PKEY=y:
U-Boot 2020.07-rc4-dirty (Jun 09 2020 - 10:33:30 +0900)
...
=> ut lib Running 11 lib tests Test: lib_asn1_pkcs7 Test: lib_asn1_pkey Test: lib_asn1_x509 Test: lib_memcpy Test: lib_memmove Test: lib_memset Test: lib_rsa_verify_invalid Test: lib_rsa_verify_valid Test: lib_test_bin2hex Test: lib_test_hex2bin Test: lib_test_hex_to_bin Failures: 0
-Takahiro Akashi
Could you, please, have a look.
Best regards
Heinrich
Am June 9, 2020 1:42:14 AM UTC schrieb AKASHI Takahiro takahiro.akashi@linaro.org:
Heinrich,
On Mon, Jun 08, 2020 at 11:08:53PM +0200, Heinrich Schuchardt wrote:
Hello Takahiro,
when trying to execute command
ut lib lib_rsa_verify_valid
on qemu_arm_defconfig with CONFIG_UNIT_TEST=y and CONFIG_RSA_VERIFY_WITH_PKEY=y it crashes in
free((void *)prop->modulus) called from rsa_free_key_prop() called from rsa_verify_key() called from rsa_verify_with_pkey().
Without CONFIG_RSA_VERIFY_WITH_PKEY=y the problem does not occur. On qemu_arm64_defconfig the problem does not occur.
I can't reproduce your problem on v2020.07-rc4 exactly with qemu_arm64_defconfig + PKEY=y:
U-Boot 2020.07-rc4-dirty (Jun 09 2020 - 10:33:30 +0900)
...
=> ut lib Running 11 lib tests Test: lib_asn1_pkcs7 Test: lib_asn1_pkey Test: lib_asn1_x509 Test: lib_memcpy Test: lib_memmove Test: lib_memset Test: lib_rsa_verify_invalid Test: lib_rsa_verify_valid Test: lib_test_bin2hex Test: lib_test_hex2bin Test: lib_test_hex_to_bin Failures: 0
-Takahiro Akashi
As said I only see the problem with 32 bit qemu_arm_defconfig.
Best regards
Heinrich
Could you, please, have a look.
Best regards
Heinrich
Heinrich,
On Tue, Jun 09, 2020 at 03:54:44AM +0000, Heinrich Schuchardt wrote:
Am June 9, 2020 1:42:14 AM UTC schrieb AKASHI Takahiro takahiro.akashi@linaro.org:
Heinrich,
On Mon, Jun 08, 2020 at 11:08:53PM +0200, Heinrich Schuchardt wrote:
Hello Takahiro,
when trying to execute command
ut lib lib_rsa_verify_valid
on qemu_arm_defconfig with CONFIG_UNIT_TEST=y and CONFIG_RSA_VERIFY_WITH_PKEY=y it crashes in
free((void *)prop->modulus) called from rsa_free_key_prop() called from rsa_verify_key() called from rsa_verify_with_pkey().
Without CONFIG_RSA_VERIFY_WITH_PKEY=y the problem does not occur. On qemu_arm64_defconfig the problem does not occur.
I can't reproduce your problem on v2020.07-rc4 exactly with qemu_arm64_defconfig + PKEY=y:
U-Boot 2020.07-rc4-dirty (Jun 09 2020 - 10:33:30 +0900)
...
=> ut lib Running 11 lib tests Test: lib_asn1_pkcs7 Test: lib_asn1_pkey Test: lib_asn1_x509 Test: lib_memcpy Test: lib_memmove Test: lib_memset Test: lib_rsa_verify_invalid Test: lib_rsa_verify_valid Test: lib_test_bin2hex Test: lib_test_hex2bin Test: lib_test_hex_to_bin Failures: 0
-Takahiro Akashi
As said I only see the problem with 32 bit qemu_arm_defconfig.
Okay. I think that the size of rrtmp variable is not good enough and when it is handed over to br_i32_decode(), it accidentally destroys (*prop)->modulus.
Heiko's patch: https://lists.denx.de/pipermail/u-boot/2020-May/413101.html will also fix this issue, but I'm not yet confident that the solution here, doubling max_rsa_size, is a right approach.
-Takahiro Akashi
Best regards
Heinrich
Could you, please, have a look.
Best regards
Heinrich
Hi Akashi,
Am Dienstag, 9. Juni 2020, 11:22:35 CEST schrieb AKASHI Takahiro:
Heinrich,
On Tue, Jun 09, 2020 at 03:54:44AM +0000, Heinrich Schuchardt wrote:
Am June 9, 2020 1:42:14 AM UTC schrieb AKASHI Takahiro takahiro.akashi@linaro.org:
Heinrich,
On Mon, Jun 08, 2020 at 11:08:53PM +0200, Heinrich Schuchardt wrote:
Hello Takahiro,
when trying to execute command
ut lib lib_rsa_verify_valid
on qemu_arm_defconfig with CONFIG_UNIT_TEST=y and CONFIG_RSA_VERIFY_WITH_PKEY=y it crashes in
free((void *)prop->modulus) called from rsa_free_key_prop() called from rsa_verify_key() called from rsa_verify_with_pkey().
Without CONFIG_RSA_VERIFY_WITH_PKEY=y the problem does not occur. On qemu_arm64_defconfig the problem does not occur.
I can't reproduce your problem on v2020.07-rc4 exactly with qemu_arm64_defconfig + PKEY=y:
U-Boot 2020.07-rc4-dirty (Jun 09 2020 - 10:33:30 +0900)
...
=> ut lib Running 11 lib tests Test: lib_asn1_pkcs7 Test: lib_asn1_pkey Test: lib_asn1_x509 Test: lib_memcpy Test: lib_memmove Test: lib_memset Test: lib_rsa_verify_invalid Test: lib_rsa_verify_valid Test: lib_test_bin2hex Test: lib_test_hex2bin Test: lib_test_hex_to_bin Failures: 0
-Takahiro Akashi
As said I only see the problem with 32 bit qemu_arm_defconfig.
Okay. I think that the size of rrtmp variable is not good enough and when it is handed over to br_i32_decode(), it accidentally destroys (*prop)->modulus.
Heiko's patch: https://lists.denx.de/pipermail/u-boot/2020-May/413101.html will also fix this issue, but I'm not yet confident that the solution here, doubling max_rsa_size, is a right approach.
The algorithm does write outside its memory area with a big enough key and I have to confess I had a hard time understanding it ;-) .
So what would be needed to get more confidence?
Because for me it looks like either the algorithm does strange things or the memory area is too small - I don't really see a third option ;-) .
Heiko
On 09.06.20 12:11, Heiko Stübner wrote:
Hi Akashi,
Am Dienstag, 9. Juni 2020, 11:22:35 CEST schrieb AKASHI Takahiro:
Heinrich,
On Tue, Jun 09, 2020 at 03:54:44AM +0000, Heinrich Schuchardt wrote:
Am June 9, 2020 1:42:14 AM UTC schrieb AKASHI Takahiro takahiro.akashi@linaro.org:
Heinrich,
On Mon, Jun 08, 2020 at 11:08:53PM +0200, Heinrich Schuchardt wrote:
Hello Takahiro,
when trying to execute command
ut lib lib_rsa_verify_valid
on qemu_arm_defconfig with CONFIG_UNIT_TEST=y and CONFIG_RSA_VERIFY_WITH_PKEY=y it crashes in
free((void *)prop->modulus) called from rsa_free_key_prop() called from rsa_verify_key() called from rsa_verify_with_pkey().
Without CONFIG_RSA_VERIFY_WITH_PKEY=y the problem does not occur. On qemu_arm64_defconfig the problem does not occur.
I can't reproduce your problem on v2020.07-rc4 exactly with qemu_arm64_defconfig + PKEY=y:
U-Boot 2020.07-rc4-dirty (Jun 09 2020 - 10:33:30 +0900)
...
=> ut lib Running 11 lib tests Test: lib_asn1_pkcs7 Test: lib_asn1_pkey Test: lib_asn1_x509 Test: lib_memcpy Test: lib_memmove Test: lib_memset Test: lib_rsa_verify_invalid Test: lib_rsa_verify_valid Test: lib_test_bin2hex Test: lib_test_hex2bin Test: lib_test_hex_to_bin Failures: 0
-Takahiro Akashi
As said I only see the problem with 32 bit qemu_arm_defconfig.
Okay. I think that the size of rrtmp variable is not good enough and when it is handed over to br_i32_decode(), it accidentally destroys (*prop)->modulus.
Heiko's patch: https://lists.denx.de/pipermail/u-boot/2020-May/413101.html will also fix this issue, but I'm not yet confident that the solution here, doubling max_rsa_size, is a right approach.
The algorithm does write outside its memory area with a big enough key and I have to confess I had a hard time understanding it ;-) .
So what would be needed to get more confidence?
Because for me it looks like either the algorithm does strange things or the memory area is too small - I don't really see a third option ;-) .
Your patch allocates unnecessary memory for key sizes below 4096 and allocates too little for key sizes above 4096 bits. So with a bad key size you can crash the code again.
Best regards
Heinrich
Am Dienstag, 9. Juni 2020, 12:22:36 CEST schrieb Heinrich Schuchardt:
On 09.06.20 12:11, Heiko Stübner wrote:
Hi Akashi,
Am Dienstag, 9. Juni 2020, 11:22:35 CEST schrieb AKASHI Takahiro:
Heinrich,
On Tue, Jun 09, 2020 at 03:54:44AM +0000, Heinrich Schuchardt wrote:
Am June 9, 2020 1:42:14 AM UTC schrieb AKASHI Takahiro takahiro.akashi@linaro.org:
Heinrich,
On Mon, Jun 08, 2020 at 11:08:53PM +0200, Heinrich Schuchardt wrote:
Hello Takahiro,
when trying to execute command
ut lib lib_rsa_verify_valid
on qemu_arm_defconfig with CONFIG_UNIT_TEST=y and CONFIG_RSA_VERIFY_WITH_PKEY=y it crashes in
free((void *)prop->modulus) called from rsa_free_key_prop() called from rsa_verify_key() called from rsa_verify_with_pkey().
Without CONFIG_RSA_VERIFY_WITH_PKEY=y the problem does not occur. On qemu_arm64_defconfig the problem does not occur.
I can't reproduce your problem on v2020.07-rc4 exactly with qemu_arm64_defconfig + PKEY=y:
U-Boot 2020.07-rc4-dirty (Jun 09 2020 - 10:33:30 +0900)
...
=> ut lib Running 11 lib tests Test: lib_asn1_pkcs7 Test: lib_asn1_pkey Test: lib_asn1_x509 Test: lib_memcpy Test: lib_memmove Test: lib_memset Test: lib_rsa_verify_invalid Test: lib_rsa_verify_valid Test: lib_test_bin2hex Test: lib_test_hex2bin Test: lib_test_hex_to_bin Failures: 0
-Takahiro Akashi
As said I only see the problem with 32 bit qemu_arm_defconfig.
Okay. I think that the size of rrtmp variable is not good enough and when it is handed over to br_i32_decode(), it accidentally destroys (*prop)->modulus.
Heiko's patch: https://lists.denx.de/pipermail/u-boot/2020-May/413101.html will also fix this issue, but I'm not yet confident that the solution here, doubling max_rsa_size, is a right approach.
The algorithm does write outside its memory area with a big enough key and I have to confess I had a hard time understanding it ;-) .
So what would be needed to get more confidence?
Because for me it looks like either the algorithm does strange things or the memory area is too small - I don't really see a third option ;-) .
Your patch allocates unnecessary memory for key sizes below 4096 and allocates too little for key sizes above 4096 bits. So with a bad key size you can crash the code again.
valid point :-) .
I guess I got sidetracked with u-boot signature handling only doing up to 4096bit right now and was not questioning the existing usage of max_rsa_size enough.
participants (3)
-
AKASHI Takahiro -
Heiko Stübner -
Heinrich Schuchardt