Re: [PATCH v4 09/10] rockchip: binman: Support use of crc32 for SPL_FIT_SIGNATURE

[Quentin Schulz <quentin.schulz@cherry.de>]

Hi Jonas,

On 4/9/25 5:38 PM, Jonas Karlman wrote:
> Hi Quentin,
> 
> On 2025-04-09 13:06, Quentin Schulz wrote:
>> Hi Jonas,
>>
>> On 3/29/25 4:06 PM, Jonas Karlman wrote:
>>> Use of SHA256 checksum validation on ARMv7 SoCs can be very time
>>> consuming compared to ARMv8 SoCs with Crypto Extensions.
>>>
>>> Add support for use of the crc32 hash algo when SHA256 is not supported.
>>> Also use a HAS_HASH to simplify the ifdefs when no known hash algo is
>>> compiled.
>>>
>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>
>> I don't know enough about general security but this very much looks like
>> a bad idea to me.
>>
>> https://web.archive.org/web/20170210173741/http://www.derkeiler.com/Newsgroups/sci.crypt/2003-07/1451.html
>>
>> """
>> While properly designed CRC's are good at detecting random errors in
>> the data (due to e.g. line noise), the CRC is useless as a secure
>> indicator of intentional manipulation of the data. And this is
>> because it's not hard at all to modify the data to produce any CRC
>> you desire (e.g. the same CRC as the original data, to try to
>> disguise your data manipulation).
>> """
>>
>> (yes I took the "first" link my web search engine returned me, thanks
>> confirmation bias!).
> 
> I am fully aware, and the fallback to use crc32, and current primarily
> use of sha256, should not be considered a security feature. It is
> intended purely for a checksum validation of the binary blob after it
> has been loaded into memory and before U-Boot otherwise unconditionally
> jumps to and execute the loaded blob of binary code.
> 
>>
>> I don't want to give people a false sense of security. If it really
>> comes down to it, I'd rather have an explicit Kconfig symbol to set this
>> value (maybe have a `choice` even) and be very clear about security
>> implications.
> 
> Prior to the addition of the hash { algo=sha256 }, there was no checksum
> test and U-Boot SPL would unconditionally jump to the loaded data, even
> if it had been corrupted, was only halfway written or otherwise
> overwritten.
> 
> The addition of crc32 instead of sha256 is just to allow older board
> variants with weaker SoCs to at least have some sort of checksum
> validation in use without affecting the boot time too much.
> 
> On my rk3228 board, validation using sha256 could take a few seconds,
> and with crc32 it could be measured in ms.
> 
> To me, having at least some default checksum validation is preferred
> over none at all.
> 

Except if this confuses people into thinking they have a secure system 
except they use CRC32 instead of something more appropriate like SHA256.

It seems like the "hash" node presence doesn't mean a FIT conf or image 
node is signed. I also don't see many device trees with a signature 
node... which is kinda odd. Looking a bit into the signature node in the 
spec and binman tests, it's not clear to me if the hash node is reused 
by the signature node if if exists and if their algo should match and 
what exactly is checked by the signature node (like signing the hash 
string contained in the hash node(s) listed in sign-images property, or 
(re-)generating a hash of those and signing it at build time?

If
a) it is not possible to have a hash node's algo differ from a signature 
node's algo, then I'm fine with this as crc32 won't be allowed
b) the signature node regenerates a hash regardless of the hash in the 
hash node, meaning the signature will be "appropriately secure" 
regardless of the algorithm listed in the hash node, then I'm fine with 
it too,
c) it is possible to sign a crc32 hash, don't want it :)

@Simon, do you have some hints about this?

Cheers,
Quentin