* [BUG] ut lib lib_rsa_verify_valid crashes on qemu_arm if RSA_VERIFY_WITH_PKEY=y
@ 2020-06-08 21:08 Heinrich Schuchardt
2020-06-09 1:42 ` AKASHI Takahiro
0 siblings, 1 reply; 7+ messages in thread
From: Heinrich Schuchardt @ 2020-06-08 21:08 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [BUG] ut lib lib_rsa_verify_valid crashes on qemu_arm if RSA_VERIFY_WITH_PKEY=y
2020-06-08 21:08 [BUG] ut lib lib_rsa_verify_valid crashes on qemu_arm if RSA_VERIFY_WITH_PKEY=y Heinrich Schuchardt
@ 2020-06-09 1:42 ` AKASHI Takahiro
2020-06-09 3:54 ` Heinrich Schuchardt
0 siblings, 1 reply; 7+ messages in thread
From: AKASHI Takahiro @ 2020-06-09 1:42 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [BUG] ut lib lib_rsa_verify_valid crashes on qemu_arm if RSA_VERIFY_WITH_PKEY=y
2020-06-09 1:42 ` AKASHI Takahiro
@ 2020-06-09 3:54 ` Heinrich Schuchardt
2020-06-09 9:22 ` AKASHI Takahiro
0 siblings, 1 reply; 7+ messages in thread
From: Heinrich Schuchardt @ 2020-06-09 3:54 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [BUG] ut lib lib_rsa_verify_valid crashes on qemu_arm if RSA_VERIFY_WITH_PKEY=y
2020-06-09 3:54 ` Heinrich Schuchardt
@ 2020-06-09 9:22 ` AKASHI Takahiro
2020-06-09 10:11 ` Heiko Stübner
0 siblings, 1 reply; 7+ messages in thread
From: AKASHI Takahiro @ 2020-06-09 9:22 UTC (permalink / raw)
To: u-boot
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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [BUG] ut lib lib_rsa_verify_valid crashes on qemu_arm if RSA_VERIFY_WITH_PKEY=y
2020-06-09 9:22 ` AKASHI Takahiro
@ 2020-06-09 10:11 ` Heiko Stübner
2020-06-09 10:22 ` Heinrich Schuchardt
0 siblings, 1 reply; 7+ messages in thread
From: Heiko Stübner @ 2020-06-09 10:11 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [BUG] ut lib lib_rsa_verify_valid crashes on qemu_arm if RSA_VERIFY_WITH_PKEY=y
2020-06-09 10:11 ` Heiko Stübner
@ 2020-06-09 10:22 ` Heinrich Schuchardt
2020-06-09 10:45 ` Heiko Stübner
0 siblings, 1 reply; 7+ messages in thread
From: Heinrich Schuchardt @ 2020-06-09 10:22 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [BUG] ut lib lib_rsa_verify_valid crashes on qemu_arm if RSA_VERIFY_WITH_PKEY=y
2020-06-09 10:22 ` Heinrich Schuchardt
@ 2020-06-09 10:45 ` Heiko Stübner
0 siblings, 0 replies; 7+ messages in thread
From: Heiko Stübner @ 2020-06-09 10:45 UTC (permalink / raw)
To: u-boot
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-06-09 10:45 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-08 21:08 [BUG] ut lib lib_rsa_verify_valid crashes on qemu_arm if RSA_VERIFY_WITH_PKEY=y Heinrich Schuchardt
2020-06-09 1:42 ` AKASHI Takahiro
2020-06-09 3:54 ` Heinrich Schuchardt
2020-06-09 9:22 ` AKASHI Takahiro
2020-06-09 10:11 ` Heiko Stübner
2020-06-09 10:22 ` Heinrich Schuchardt
2020-06-09 10:45 ` Heiko Stübner
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.