[RFC] usage of mkhash, sha256sum and md5sum

[Paul Spooren]

On 16.07.20 09:36, Felix Fietkau wrote:
> On 2020-07-16 21:18, Paul Spooren wrote:
>> Hi,
>>
>> On 15.07.20 22:54, Felix Fietkau wrote:
>>> On 2020-07-16 04:06, Paul Spooren wrote:
>>>> Hi,
>>>>
>>>> the OpenWrt system requires the calculation of both md5 and sha256 sums
>>>> at various places, this is partly done via a small C file in
>>>> ./scripts/mkhash.c and partly by using a sha256sum binary. A ancient
>>>> wrapper ./scripts/md5sum is added for Mac OS X compatibility.
>>>>
>>>> * Should we create our own crypto by using ./scripts/mkhash.c? I
>>>> remember from some previous discussions on IRC and GitHub that there are
>>>> generally concerns against it, also a motivation for[0]. I understand
>>>> that Felix just reinvent the code but used established sources, however
>>>> it is used for package signing (not image signing). I'm fairly sure less
>>>> eyes look through that code than e.g. the Debian implementation.
>>> This is not "creating our own crypto" at all. I used existing widely
>>> used implementations of MD5/SHA256 (mostly FreeBSD code, if I remember
>>> correctly).
>> Maybe the wording here is wrong, "maintain your own crypto" rather than
>> "create".
> I don't see anything wrong with that when it's just simple commonly used
> hash functions.
>
>>>> * Currently include/package-ipkg.mk uses a host installed `sha256sum`
>>>> binary which is not covered via include/prereq{,-build}.mk. Should it be
>>>> added to prereq or replaced by mkhash?
>>>>
>>>> * Can ./scripts/md5sum be removed or is it still required for Mac OS X
>>>> builds.
>>> I'm not sure if build/host code for some packages still relies on it.
>> I'll ask some fellow Mac OS X builders. However a cleaner solution would
>> be to just remove it and rely on `mkhash md5` only.
>>>> * Any reason not to replace `mkhash <alg>` by using `<alg>sum | cut -d '
>>>> ' -f 1`? Both sha256sum and md5sum seem to be available per default on
>>>> Debian, Alpine and OpenWrt.
>>> There are many calls to mkhash from the build system, some from
>>> performance sensitive parts. Changing it that way will likely make the
>>> build slower (especially in cases where it only checks stamps but
>>> doesn't rebuild anything).
>> I did a quick benchmark and mkhash & sha256sum seem to be the same speed
>> while md5sum is about 8% faster than `mkhash md5`.
>>
>> Details here if anyone cares http://sprunge.us/l7amiR
> You ran the benchmark on a large file, which mostly ignores startup
> overhead. mkhash is frequently called on small files, so startup
> performance matters much more than raw throughput.
>
> I just tried the same benchmark on a small file on my machine, and there
> mkhash is 1.48x faster than *sum | cut
> Results here: http://sprunge.us/KtJTSe

Thanks for the clarification and taking the time to run a benchmark.

I'll create a PR later to remove the single usage of `sha256sums` with 
mkhash sha256`.

Best,
Paul