Re: [PATCH] Add x64 SHA-NI implementation to sha1 and sha256 in git ?
- From
brian m. carlson <sandals@crustytoothpaste.net>
- Date
- Oct 13, 2023, 21:05 UTC
- Message-ID
- <ZSmxFopwJyBGmZY-@tapette.crustytoothpaste.net>
- In-Reply-To
- <20231012200447.3553757-1-hi@fxue.dev>
On 2023-10-12 at 20:04:47, Feiyang Xue wrote:
Show 8 quoted lines
> 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.
Show 5 quoted lines
> 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