Page MenuHomeFreeBSD

lib/libmd: rework and accelerate SHA1 implementation
Needs ReviewPublic

Authored by fuz on Jun 1 2024, 8:11 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Jul 18, 11:38 AM
Unknown Object (File)
Wed, Jul 3, 11:27 AM
Unknown Object (File)
Mon, Jul 1, 1:15 AM
Unknown Object (File)
Sun, Jun 30, 8:56 AM
Unknown Object (File)
Fri, Jun 28, 2:59 PM
Unknown Object (File)
Jun 15 2024, 8:00 PM
Unknown Object (File)
Jun 8 2024, 12:05 PM
Unknown Object (File)
Jun 4 2024, 5:34 PM

Details

Summary

The implementation of SHA1 is replaced with a transcription of
Go's implementation, which is much easier to read and delivers
similar or better performance than the old SSLeay code.

Some of Go's assembly implementations, paired with hand-written
new code, is used to speed up the functions on popular platforms
amd64 and aarch64. More platforms can be added if there is
sufficient interest.

For amd64, implementations using just scalar instructions, using
AVX2, and using SHANI (SHA new instructions) are provided. For
aarch64, an implementation using just scalar instructions as well
as one using the widespread sha1 extensions are provided.

For a 10 GiB input file, we get the following performance figures:

On amd64 (11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz):

old:    16.7s ( 613 MB/s)
scalar: 14.5s ( 706 MB/s)
avx2:   10.5s ( 975 MB/s)
shani:   5.6s (1829 MB/s)

On aarch64 (Windows 2023 Dev Kit, ARM Cortex A78C / ARM Cortex X1C):

Performance core:
    pre     43.1s   (238 MB/s)
    generic 41.3s   (247 MB/s)
    scalar  35.0s   (293 MB/s)
    sha1    12.8s   (800 MB/s)

Efficiency core:
    pre     54.2s   (189 MB/s)
    generic 55.9s   (183 MB/s)
    scalar  43.0s   (238 MB/s)
    sha1    16.2s   (632 MB/s)
Test Plan

Passes make test, returns the right results when ran manually.
All implementations were individually validated.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 58206
Build 55094: arc lint + arc unit

Event Timeline

fuz requested review of this revision.Jun 1 2024, 8:11 PM

I think I'll need to adjust the code to more closely correspond to the old API. Unfortunately there is no symbol versioning for this library, so it's hard to say what the API is exactly.

Are you sure it is safe to drop the __weak_reference() bits at the bottom? They relate to the fact that there are conflicts between libmd and libcrypt (not to be confused with libcrypto)

Are you sure it is safe to drop the __weak_reference() bits at the bottom? They relate to the fact that there are conflicts between libmd and libcrypt (not to be confused with libcrypto)

Hi Allan,

No, that's indeed not correct. I just didn't get around to fixing that yet. There's also the issue that my code no longer has the sha1_block symbol as the new round function has a different type. But D34498 will get rid of that anyway.

  • lib/libmd: re-add the weak references
  • lib/libmd: add a dummy SHA1_version string

Is the change from SHA_CTX to SHA1_CTX intentional? It's a no-op since SHA1_CTX is defined to SHA_CTX in the header, but still.

In D45444#1037672, @des wrote:

Is the change from SHA_CTX to SHA1_CTX intentional? It's a no-op since SHA1_CTX is defined to SHA_CTX in the header, but still.

Yes, the change is intentional as I wanted to avoid doubt about why an identifier prefixed SHA_ is used for SHA1.

lib/libmd/sha1c.c
503

So what's the justification for getting rid of this? There's that do the #define sha1_block _libmd_sha1_block to avoid symbol clashes and this will give them undefined symbols.

Respond to @imp's comment.

lib/libmd/sha1c.c
503

My new code doesn't actually define sha1_block anymore as my block function has a slightly different signature (it's still ABI compatible I think). It wouldn't be too much work to add a bunch of casts so it works out in the end, but it seemed easier to follow the suggestion of D34498 and just take out this symbol.

If we decide against D34498's proposal, I'll edit this one to put the block symbol in.

  • lib/libmd/aarch64: fix typo in sha1block.S
  • lib/libmd: discard undocumented SHA1 symbols

I think it's indeed best to get rid of SHA1_block, SHA1_Transform, and
SHA1_version. These three symbols were never publicly declared and don't
seem to be particularly useful either. See also D34498, D34499.