Re: [PATCH] Add x64 SHA-NI implementation to sha1 and sha256 in git ?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2023-10-12 at 20:04:47, Feiyang Xue wrote:
> Looking for comments on if this is a good idea to add SHA-NI option to git,
> And if so, what'd be a good implementation.
> 
> The attached patch is more of a proof of concept,  using a sha-ni example
> found on internet and have it tapped into git's "hash-ll.h" when it sees make
> flags "SHA1_SHANI=1" and/or "SHA256_SHANI=1" for sha1/sha256 respectively.
> 
> "git hash-object" is about 3-ish times faster for me

You almost certainly don't want to use SHA-1 using native instructions
because it doesn't detect collisions, unlike the default implementation
(SHA-1-DC).  Since SHA-1 collisions are relatively cheap (less than USD
45,000) to make, not detecting collisions would be a security issue
worthy of a CVE.

It's fine for SHA-256, but see below.

> There are few topics that i'm uncertain about.
> 
> 1. Is it good idea to have machine-specific asm codes in git. I read there was
> commit fd1ec82547 which removed assembly implementation for PPC_SHA1. Does that
> imply maintainers doesn't want machine-specific assembly in source?

I'd prefer we didn't.

Nettle has SHA-NI extensions on systems that support it, and so does
OpenSSL.  OpenSSL can't be used by distros for licensing reasons, but
Nettle can, and both of those libraries automatically detect the best
code to use based on the chip.  One of those libraries is almost always
going to be linked into Git at some point because of libcurl anyway.

If we ship the code, then someone has to maintain it, and I don't know
if we have any assembly experts.  We might also have a bug that produced
incorrect results, which would be pretty disastrous for the affected
users.  It's much better for maintainability if we push that off onto
the cryptographic library maintainers who are much more competent in
that regard than I am (I will not speak for others) and let distro
maintainers simply link the appropriate library, which, as I said above,
they're already effectively doing.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux