Re: [PATCH 3/4] Makefile: allow specifying a SHA-1 for non-cryptographic uses

From: Patrick Steinhardt <ps@pks.im>
Date: Sep 2, 2024, 13:41 UTC
Message-ID: <ZtXAiIwLgtQNkwyH@tanuki>
In-Reply-To: <682e4c2cc3581c72262ea6a9b488a246fc6fde28.1725206584.git.me@ttaylorr.com>

On Sun, Sep 01, 2024 at 12:03:28PM -0400, Taylor Blau wrote:

Show 27 quoted lines

> diff --git a/Makefile b/Makefile
> index e298c8b55ec..d24f9088802 100644
> @@ -1982,6 +1986,27 @@ endif
>  endif
>  endif
>  
> +ifdef OPENSSL_SHA1_FAST
> +ifndef OPENSSL_SHA1
> +	EXTLIBS += $(LIB_4_CRYPTO)
> +	BASIC_CFLAGS += -DSHA1_OPENSSL_FAST
> +endif
> +else
> +ifdef BLK_SHA1_FAST
> +ifndef BLK_SHA1
> +	LIB_OBJS += block-sha1/sha1.o
> +	BASIC_CFLAGS += -DSHA1_BLK_FAST
> +endif
> +else
> +ifdef APPLE_COMMON_CRYPTO_SHA1_FAST
> +ifndef APPLE_COMMON_CRYPTO_SHA1
> +	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
> +	BASIC_CFLAGS += -DSHA1_APPLE_FAST
> +endif
> +endif
> +endif
> +endif
> +

What a cascade of `endif`s :)

Do we also want to wire up support in config.mak.uname such that the fast variants are default-enabled? Or is there a good reason to not do that?

Show 36 quoted lines

> diff --git a/hash.h b/hash.h
> index f255e5c1e8a..450e579b405 100644
> --- a/hash.h
> +++ b/hash.h
> @@ -15,6 +15,31 @@
>  #include "block-sha1/sha1.h"
>  #endif
>  
> +#if defined(SHA1_APPLE_FAST)
> +#include <CommonCrypto/CommonDigest.h>
> +#define platform_SHA_CTX_fast CC_SHA1_CTX
> +#define platform_SHA1_Init_fast CC_SHA1_Init
> +#define platform_SHA1_Update_fast CC_SHA1_Update
> +#define platform_SHA1_Final_fast CC_SHA1_Final
> +#elif defined(SHA1_OPENSSL_FAST)
> +#  include <openssl/sha.h>
> +#  if defined(OPENSSL_API_LEVEL) && OPENSSL_API_LEVEL >= 3
> +#    define SHA1_NEEDS_CLONE_HELPER_FAST
> +#    include "sha1/openssl.h"
> +#  endif
> +#  define platform_SHA_CTX_fast openssl_SHA1_CTX
> +#  define platform_SHA1_Init_fast openssl_SHA1_Init
> +#  define platform_SHA1_Clone_fast openssl_SHA1_Clone
> +#  define platform_SHA1_Update_fast openssl_SHA1_Update
> +#  define platform_SHA1_Final_fast openssl_SHA1_Final
> +#elif defined(SHA1_BLK_FAST)
> +#include "block-sha1/sha1.h"
> +#define platform_SHA_CTX_fast blk_SHA_CTX
> +#define platform_SHA1_Init_fast blk_SHA1_Init
> +#define platform_SHA1_Update_fast blk_SHA1_Update
> +#define platform_SHA1_Final_fast blk_SHA1_Final
> +#endif
> +
>  #if defined(SHA256_NETTLE)
>  #include "sha256/nettle.h"
>  #elif defined(SHA256_GCRYPT)

Curiously, some of the nested statements here are indented whereas others aren't. We should aim to make that consistent.

Patrick