Hi, On Wed, 4 Sept 2024 at 06:48, Peter Robinson <pbrobin...@gmail.com> wrote: > > Hi Simon, > > > I wonder if we could leave out the SHA stuff? The algorithms are > > One of the big advantages of the mbedtls when it comes to all things > security is that it's seen a wide audit of it's code which for a lot > of usecases is very useful from a security PoV, I'm not sure the > amount of audit the U-Boot in project code has had, I'm sure there has > been but I've not seen anything published.
Definitely the hash routines and signing have been audited, in fact there was at least one bug that came out of it, although not in the hash routines themselves, which are solid. > > > stable and this would seem to avoid much of the size growth, and all > > the pain of trying to integrate another yet another hashing layer (we > > already have normal, progressive and h/w acceleration, plus > > What's the difference between the first two? Normal hashes the whole lot at once. Progressive does things a chunk at a time, to be watchdog-friendly. If CPUs are fast enough (and watchdogs lax enough), perhaps we don't need that code? > > > UCLASS_HASH which h/w acceleration should use but that migration never > > How hard would it be for UCLASS_HASH to use the mbed hashing underneath? I really don't see the point as there is nothing wrong with U-Boot's hashing, so far as I know. Half the patches in this series would likely not be necessary? But there is a hash_sw driver for UCLASS_HASH, which results in using software hashing, if hardware acceleration is not available. > > > happened). I struggle to see any benefit in replacing U-Boot's very > > solid hashing infra with something else, particularly as this series > > I would need to look at the HW support in both U-Boot and mbedtls but > given wider use of mbedtls I bet adding HW support there that U-Boot > could utilise may be more apertising to most HW vendors as it means > they only have to write one set of code and have it used much more > widely. I'm really not sure that mbedtls has wider use than U-Boot :-) Yes, anything is possible in software, but I worry we might create Frankenstein's monster. U-Boot's acceleration stuff works fine and has a proper drive model. For example, with ast2500 it automatically does the right thing with the "aspeed,ast2600-hace" (compatible) node. > > > adds yet another. Better to invest the time to refactor it. I asked > > about this before and was told that it would happen 'later'. Let's > > just not change it at all, then it is more likely someone will sort it > > What, like the HW support in UCLASS_HASH? Things clearly don't work like that. Well, the big challenge here was the difficulty of avoiding *any* size growth, when I wrote common/hash.c - perhaps with the large features going in, this might be less important. > > > Also, if MbedTLS is wanting to be a general library for TLS (I assume > > transport-local security, not thread-local storage) perhaps it might > > consider changing to non-Windows newlines, or perhaps even kernel code > > style? > > I think the newlines might be a possible ask, they are generally > receptive to change (they relicensed it to be a dual license > compatible with U-Boot when asked), I don't think forcing a separate > to the kernel project to a kernel code style is a fair request. OK. So long as I don't have to change the code... but with this series there is an increase in the code debt in common/hash.c which I'm really not keen on, sorry. Regards, Simon
