Re: [PATCH v2 2/2] help: include unsafe SHA-1 build info in version

From: Patrick Steinhardt <ps@pks.im>
Date: Apr 2, 2025, 07:38 UTC
Message-ID: <Z-zpau_DsOGVVtjj@pks.im>
In-Reply-To: <20250401203630.285451-3-jltobler@gmail.com>

On Tue, Apr 01, 2025 at 03:36:30PM -0500, Justin Tobler wrote:

Show 14 quoted lines

> diff --git a/Documentation/git-version.adoc b/Documentation/git-version.adoc
> index f06758a7cf..753794988c 100644
> --- a/Documentation/git-version.adoc
> +++ b/Documentation/git-version.adoc
> @@ -25,6 +25,9 @@ OPTIONS
>  +
>  Note that the SHA1 options `SHA1_APPLE`, `SHA1_OPENSSL`, and `SHA1_BLK` do not
>  have collision detection.
> ++
> +If built to use a faster SHA-1 implementation for non-cryptographic purposes,
> +that implementation is denoted as "non-crypto-SHA-1".
>  
>  GIT
>  ---

I got basically the same comment for this new paragraph as for the first one. I'd either drop it or expand it so that readers know what's going on.

Show 13 quoted lines

> diff --git a/help.c b/help.c
> index 3aebfb3681..1238a962b0 100644
> --- a/help.c
> +++ b/help.c
> @@ -772,6 +772,11 @@ char *help_unknown_cmd(const char *cmd)
>  static void get_sha_impl(struct strbuf *buf)
>  {
>  	strbuf_addf(buf, "SHA-1: %s\n", SHA1_BACKEND);
> +
> +#if defined(SHA1_UNSAFE_BACKEND)
> +	strbuf_addf(buf, "non-crypto-SHA-1: %s\n", SHA1_UNSAFE_BACKEND);
> +#endif
> +

Should we maybe print the equivalent of "none" in case no unsafe backend was selected?

I also think we shouldn't name this "non-crypto". The backend still is SHA1, which is a proper cryptogtaphic hash function. It may be somewhat broken nowadays, but that doesn't change the fact that it's a cryptographic primitive.

How about we rename this to "SHA-1 without collision detection:"?

Patrick