Re: [PATCH] sha256: add support for Nettle

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 6, 2022, 17:49 UTC
Message-ID: <xmqqedyyi11y.fsf@gitster.g>
In-Reply-To: <20220705230518.713218-1-sandals@crustytoothpaste.net>

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

Show 15 quoted lines

> diff --git a/hash.h b/hash.h
> index 5d40368f18..ea87ae9d92 100644
> --- a/hash.h
> +++ b/hash.h
> @@ -16,7 +16,9 @@
>  #include "block-sha1/sha1.h"
>  #endif
>  
> -#if defined(SHA256_GCRYPT)
> +#if defined(SHA256_NETTLE)
> +#include "sha256/nettle.h"
> +#elif defined(SHA256_GCRYPT)
>  #define SHA256_NEEDS_CLONE_HELPER
>  #include "sha256/gcrypt.h"
>  #elif defined(SHA256_OPENSSL)

When it does not make any semantic difference, it is preferrable to add a new thing after existing things. But this sequence is meant to list them in the order of preference when multiple choices are availble, so it is OK to prepend nettle IF our intention is to favor it over all others.

I am OK with that design choice, and I think the first paragraph of the proposed log message adequately justifies why, but I'd prefer to see it a bit more explicitly stated in the log message.

Show 13 quoted lines

> For SHA-256, we currently have support for OpenSSL and libgcrypt because
> these two libraries contain optimized implementations that can take
> advantage of native processor instructions.  However, OpenSSL is not
> suitable for linking against for Linux distros due to licensing
> incompatibilities with the GPLv2, and libgcrypt has been less favored
> by cryptographers due to some security-related implementation issues.
>
> Let's add another option that's compatible with the GPLv2, which is
> Nettle.  It also has recently gained support for Intel's SHA-NI
> instructions, which compare very favorably to other implementations.
> For example, using this implementation and SHA-1 DC on a machine with
> SHA-NI, hashing a 2 GiB file with SHA-1 takes 7.582 seconds, while
> hashing the same file with SHA-256 takes 2.278 seconds.

Perhaps "Let's add another option ..., which is Nettle, and give it preference over all others when multiple libraries are availalble" or something along that line?

Show 8 quoted lines

> diff --git a/sha256/nettle.h b/sha256/nettle.h
> new file mode 100644
> index 0000000000..9b2845babc
> --- /dev/null
> +++ b/sha256/nettle.h
> @@ -0,0 +1,28 @@
> +#ifndef SHA256_GCRYPT_H
> +#define SHA256_GCRYPT_H

Not really ;-)

> +
> +#include <nettle/sha2.h>