[PATCH 0/2] help: include SHA build options in version info

[PATCH 0/2] help: include SHA build options in version info

[Justin Tobler]

Greetings,

Additional information regarding how Git was build can be found via the
`--build-options` flag for git-verison(1). This currectly does not
include information about the SHA-1 and SHA-256 implementations Git is
built with.

This short series adds build option info for the SHA-1, SHA-256, and
unsafe-SHA-1 (if any) implementations which may be useful for diagnostic
purposes.

Regarding "unsafe-SHA-1", I wonder if we should use a different name in
the printed build options that sounds a little less scary. I was
thinking maybe "fast-SHA-1" and document its meaning appropriately. I'm
interested to know if anyone has thoughts on this.

Thanks,
-Justin

Justin Tobler (2):
  help: include SHA implementation in version info
  help: include unsafe SHA-1 build info in version

 Documentation/git-version.adoc |  3 +++
 help.c                         | 36 ++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+)


base-commit: 683c54c999c301c2cd6f715c411407c413b1d84e
-- 
2.49.0

[PATCH 1/2] help: include SHA implementation in version info

[Justin Tobler]

When the `--build-options` flag is used with git-version(1), additional
information about the built version of Git is printed. During build
time, different SHA implementations may be configured, but this
information is not included in the version info.

Add the SHA implementations Git is built with to the version info.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 help.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/help.c b/help.c
index c54bd9918a..32b5d4e6f5 100644
--- a/help.c
+++ b/help.c
@@ -768,6 +768,33 @@ char *help_unknown_cmd(const char *cmd)
 	exit(1);
 }
 
+static void get_sha_impl(struct strbuf *buf)
+{
+#if defined(SHA1_OPENSSL)
+	strbuf_addstr(buf, "SHA-1: OpenSSL\n");
+#elif defined(SHA1_BLK)
+	strbuf_addstr(buf, "SHA-1: blk\n");
+#elif defined(SHA1_APPLE)
+	strbuf_addstr(buf, "SHA-1: Apple CommonCrypto\n");
+#elif defined(DC_SHA1_EXTERNAL)
+	strbuf_addstr(buf, "SHA-1: Collision Detection (External)\n");
+#elif defined(DC_SHA1_SUBMODULE)
+	strbuf_addstr(buf, "SHA-1: Collision Detection (Submodule)\n");
+#elif defined(SHA1_DC)
+	strbuf_addstr(buf, "SHA-1: Collision Detection\n");
+#endif
+
+#if defined(SHA256_OPENSSL)
+	strbuf_addstr(buf, "SHA-256: OpenSSL\n");
+#elif defined(SHA256_NETTLE)
+	strbuf_addstr(buf, "SHA-256: Nettle\n");
+#elif defined(SHA256_GCRYPT)
+	strbuf_addstr(buf, "SHA-256: gcrypt\n");
+#elif defined(SHA256_BLK)
+	strbuf_addstr(buf, "SHA-256: blk\n");
+#endif
+}
+
 void get_version_info(struct strbuf *buf, int show_build_options)
 {
 	/*
@@ -803,6 +830,7 @@ void get_version_info(struct strbuf *buf, int show_build_options)
 #elif defined ZLIB_VERSION
 		strbuf_addf(buf, "zlib: %s\n", ZLIB_VERSION);
 #endif
+		get_sha_impl(buf);
 	}
 }
 
-- 
2.49.0

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

[Justin Tobler]

In 06c92dafb8 (Makefile: allow specifying a SHA-1 for non-cryptographic
uses, 2024-09-26), support for unsafe SHA-1 is added. Add the unsafe
SHA-1 build info to `git version --build-info` and update corresponding
documentation.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 Documentation/git-version.adoc | 3 +++
 help.c                         | 8 ++++++++
 2 files changed, 11 insertions(+)

diff --git a/Documentation/git-version.adoc b/Documentation/git-version.adoc
index 80fa7754a6..53c8ba74c1 100644
--- a/Documentation/git-version.adoc
+++ b/Documentation/git-version.adoc
@@ -22,6 +22,9 @@ OPTIONS
 --build-options::
 	Include additional information about how git was built for diagnostic
 	purposes.
++
+If built to use a faster SHA-1 implementation for non-cryptographic purposes,
+the implmentation used is denoted as "unsafe-SHA-1".
 
 GIT
 ---
diff --git a/help.c b/help.c
index 32b5d4e6f5..7670e0a64a 100644
--- a/help.c
+++ b/help.c
@@ -784,6 +784,14 @@ static void get_sha_impl(struct strbuf *buf)
 	strbuf_addstr(buf, "SHA-1: Collision Detection\n");
 #endif
 
+#if defined(SHA1_OPENSSL_UNSAFE)
+	strbuf_addstr(buf, "unsafe-SHA-1: OpenSSL\n");
+#elif defined(SHA1_BLK_UNSAFE)
+	strbuf_addstr(buf, "unsafe-SHA-1: blk\n");
+#elif defined(SHA1_APPLE_UNSAFE)
+	strbuf_addstr(buf, "unsafe-SHA-1: Apple CommonCrypto\n");
+#endif
+
 #if defined(SHA256_OPENSSL)
 	strbuf_addstr(buf, "SHA-256: OpenSSL\n");
 #elif defined(SHA256_NETTLE)
-- 
2.49.0

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

[Christian Couder]

On Fri, Mar 28, 2025 at 6:05 PM Justin Tobler <jltobler@gmail.com> wrote:
>
> In 06c92dafb8 (Makefile: allow specifying a SHA-1 for non-cryptographic
> uses, 2024-09-26), support for unsafe SHA-1 is added. Add the unsafe
> SHA-1 build info to `git version --build-info` and update corresponding
> documentation.
>
> Signed-off-by: Justin Tobler <jltobler@gmail.com>
> ---
>  Documentation/git-version.adoc | 3 +++
>  help.c                         | 8 ++++++++
>  2 files changed, 11 insertions(+)
>
> diff --git a/Documentation/git-version.adoc b/Documentation/git-version.adoc
> index 80fa7754a6..53c8ba74c1 100644
> --- a/Documentation/git-version.adoc
> +++ b/Documentation/git-version.adoc
> @@ -22,6 +22,9 @@ OPTIONS
>  --build-options::
>         Include additional information about how git was built for diagnostic
>         purposes.
> ++
> +If built to use a faster SHA-1 implementation for non-cryptographic purposes,
> +the implmentation used is denoted as "unsafe-SHA-1".

s/the implmentation used/that implementation/

Re: [PATCH 0/2] help: include SHA build options in version info

[Christian Couder]

On Fri, Mar 28, 2025 at 6:05 PM Justin Tobler <jltobler@gmail.com> wrote:
>
> Greetings,
>
> Additional information regarding how Git was build can be found via the

Maybe: s/build/built/

> `--build-options` flag for git-verison(1). This currectly does not

s/git-verison/git-version/
s/currectly/currently/

> include information about the SHA-1 and SHA-256 implementations Git is
> built with.
>
> This short series adds build option info for the SHA-1, SHA-256, and
> unsafe-SHA-1 (if any) implementations which may be useful for diagnostic
> purposes.
>
> Regarding "unsafe-SHA-1", I wonder if we should use a different name in
> the printed build options that sounds a little less scary. I was
> thinking maybe "fast-SHA-1" and document its meaning appropriately. I'm
> interested to know if anyone has thoughts on this.

Maybe we could use just "SHA-1" if a single algorithm is used for
everything, and both "SHA-1 for crypto" and "SHA-1 for non-crypto"
otherwise.

Related to this I wonder if we should warn in some ways if a non
collision detection algorithm is used for crypto. For example we could
print "SHA-1: OpenSSL (No collision detection!!!)" instead of just
"SHA-1: OpenSSL". And yeah that should be documented.

Thanks!

Re: [PATCH 1/2] help: include SHA implementation in version info

[Junio C Hamano]

Justin Tobler <jltobler@gmail.com> writes:

> When the `--build-options` flag is used with git-version(1), additional
> information about the built version of Git is printed. During build
> time, different SHA implementations may be configured, but this
> information is not included in the version info.
>
> Add the SHA implementations Git is built with to the version info.
> ...
> +static void get_sha_impl(struct strbuf *buf)
> +{
> +#if defined(SHA1_OPENSSL)
> +	strbuf_addstr(buf, "SHA-1: OpenSSL\n");
> +#elif defined(SHA1_BLK)
> +	strbuf_addstr(buf, "SHA-1: blk\n");
> +#elif defined(SHA1_APPLE)
> +	strbuf_addstr(buf, "SHA-1: Apple CommonCrypto\n");
> +#elif defined(DC_SHA1_EXTERNAL)
> +	strbuf_addstr(buf, "SHA-1: Collision Detection (External)\n");
> +#elif defined(DC_SHA1_SUBMODULE)
> +	strbuf_addstr(buf, "SHA-1: Collision Detection (Submodule)\n");
> +#elif defined(SHA1_DC)
> +	strbuf_addstr(buf, "SHA-1: Collision Detection\n");
> +#endif
> +
> +#if defined(SHA256_OPENSSL)
> +	strbuf_addstr(buf, "SHA-256: OpenSSL\n");
> +#elif defined(SHA256_NETTLE)
> +	strbuf_addstr(buf, "SHA-256: Nettle\n");
> +#elif defined(SHA256_GCRYPT)
> +	strbuf_addstr(buf, "SHA-256: gcrypt\n");
> +#elif defined(SHA256_BLK)
> +	strbuf_addstr(buf, "SHA-256: blk\n");
> +#endif
> +}

While I agree with the objective of the change, I am not sure how I
feel about the implementation.  Given that

 - The code here, and probably the existing code paths that depend
   on these SHA1_$WHOSE symbols, assume that only one of them is
   defined;

 - The "git help --build-options" is not an end-user thing but more
   is a developer thing.

The thing I am most worried about is that it is unclear how the
order in which the SHA1_$WHOSE symbols are inspected here and
elsewhere in the code are kept in sync.  What happens when, for
example, SHA1_OPENSSL and SHA1_APPLE_UNSAFE are both defined?  The
above code will report that we are using SHA1_OPENSSL, but hash.h
would probably use SHA1_APPLE as it has its own if/elif/endif
cascade.

Perhaps it does not matter, if the build infrastructure ensures that
the build fails unless one and only one of SHA1_$WHOSE is defined.

But with the way how this part is written with an if/elif/endif
cascade, it makes readers spend time wondering how the precedence
order here is kept in sync throughout the system.  If I am not
mistaken, the top-level Makefile has its own ifdef/else/if/endif*
cascade.

I imagine that making all of the above not if/elif/endif chain, but
make them pretend as if they are independent and orthogonal choices,
would make it simpler to understand and also it will help us catch a
misconfiguration where more than one is defined, i.e.

        static void get_sha_impl(struct strbuf *buf)
        {
        #if defined(SHA1_OPENSSL)
                strbuf_addstr(buf, "SHA-1: OpenSSL\n");
        #endif
        #if defined(SHA1_BLK)
                strbuf_addstr(buf, "SHA-1: blk\n");
        #endif
        #if defined(SHA1_APPLE)
        ...


That way, we wouldn't force future devlopers who are plugging new
implementations of SHA-256 wonder where is the right place in the
existing if/elif/endif cascade their new one fits.  It also allows
us to catch misconfigurations to define more then one of them at the
same time, if such a thing becomes ever possible.

Also, wouldn't it make more sense to just reuse the internal symbol
for reporting, i.e.

	strbuf_addstr(buf, "SHA-1: SHA1_OPENSSL\n");

instead of having to come up with "human readable" name here

Re: [PATCH 1/2] help: include SHA implementation in version info

[Patrick Steinhardt]

On Sat, Mar 29, 2025 at 04:36:45AM -0700, Junio C Hamano wrote:
> Justin Tobler <jltobler@gmail.com> writes:
> 
> > When the `--build-options` flag is used with git-version(1), additional
> > information about the built version of Git is printed. During build
> > time, different SHA implementations may be configured, but this
> > information is not included in the version info.
> >
> > Add the SHA implementations Git is built with to the version info.
> > ...
> > +static void get_sha_impl(struct strbuf *buf)
> > +{
> > +#if defined(SHA1_OPENSSL)
> > +	strbuf_addstr(buf, "SHA-1: OpenSSL\n");
> > +#elif defined(SHA1_BLK)
> > +	strbuf_addstr(buf, "SHA-1: blk\n");
> > +#elif defined(SHA1_APPLE)
> > +	strbuf_addstr(buf, "SHA-1: Apple CommonCrypto\n");
> > +#elif defined(DC_SHA1_EXTERNAL)
> > +	strbuf_addstr(buf, "SHA-1: Collision Detection (External)\n");
> > +#elif defined(DC_SHA1_SUBMODULE)
> > +	strbuf_addstr(buf, "SHA-1: Collision Detection (Submodule)\n");
> > +#elif defined(SHA1_DC)
> > +	strbuf_addstr(buf, "SHA-1: Collision Detection\n");
> > +#endif
> > +
> > +#if defined(SHA256_OPENSSL)
> > +	strbuf_addstr(buf, "SHA-256: OpenSSL\n");
> > +#elif defined(SHA256_NETTLE)
> > +	strbuf_addstr(buf, "SHA-256: Nettle\n");
> > +#elif defined(SHA256_GCRYPT)
> > +	strbuf_addstr(buf, "SHA-256: gcrypt\n");
> > +#elif defined(SHA256_BLK)
> > +	strbuf_addstr(buf, "SHA-256: blk\n");
> > +#endif
> > +}
> 
> While I agree with the objective of the change, I am not sure how I
> feel about the implementation.  Given that
> 
>  - The code here, and probably the existing code paths that depend
>    on these SHA1_$WHOSE symbols, assume that only one of them is
>    defined;
> 
>  - The "git help --build-options" is not an end-user thing but more
>    is a developer thing.
> 
> The thing I am most worried about is that it is unclear how the
> order in which the SHA1_$WHOSE symbols are inspected here and
> elsewhere in the code are kept in sync.  What happens when, for
> example, SHA1_OPENSSL and SHA1_APPLE_UNSAFE are both defined?  The
> above code will report that we are using SHA1_OPENSSL, but hash.h
> would probably use SHA1_APPLE as it has its own if/elif/endif
> cascade.
> 
> Perhaps it does not matter, if the build infrastructure ensures that
> the build fails unless one and only one of SHA1_$WHOSE is defined.
> 
> But with the way how this part is written with an if/elif/endif
> cascade, it makes readers spend time wondering how the precedence
> order here is kept in sync throughout the system.  If I am not
> mistaken, the top-level Makefile has its own ifdef/else/if/endif*
> cascade.
> 
> I imagine that making all of the above not if/elif/endif chain, but
> make them pretend as if they are independent and orthogonal choices,
> would make it simpler to understand and also it will help us catch a
> misconfiguration where more than one is defined, i.e.
> 
>         static void get_sha_impl(struct strbuf *buf)
>         {
>         #if defined(SHA1_OPENSSL)
>                 strbuf_addstr(buf, "SHA-1: OpenSSL\n");
>         #endif
>         #if defined(SHA1_BLK)
>                 strbuf_addstr(buf, "SHA-1: blk\n");
>         #endif
>         #if defined(SHA1_APPLE)
>         ...
> 
> 
> That way, we wouldn't force future devlopers who are plugging new
> implementations of SHA-256 wonder where is the right place in the
> existing if/elif/endif cascade their new one fits.  It also allows
> us to catch misconfigurations to define more then one of them at the
> same time, if such a thing becomes ever possible.

Another option: we could ask the implementations themselves to define a
symbol `SHA1_BACKEND` and use it here. This would automatically ensure
that any implementation must define the symbol as we'd otherwise get a
compile error. We could also conditionally define `SHA1_UNSAFE_BACKEND`
depending on whether or not we have it.

Patrick

Re: [PATCH 1/2] help: include SHA implementation in version info

[Justin Tobler]

On 25/03/29 04:36AM, Junio C Hamano wrote:
> Justin Tobler <jltobler@gmail.com> writes:
> 
> > When the `--build-options` flag is used with git-version(1), additional
> > information about the built version of Git is printed. During build
> > time, different SHA implementations may be configured, but this
> > information is not included in the version info.
> >
> > Add the SHA implementations Git is built with to the version info.
> > ...
> > +static void get_sha_impl(struct strbuf *buf)
> > +{
> > +#if defined(SHA1_OPENSSL)
> > +	strbuf_addstr(buf, "SHA-1: OpenSSL\n");
> > +#elif defined(SHA1_BLK)
> > +	strbuf_addstr(buf, "SHA-1: blk\n");
> > +#elif defined(SHA1_APPLE)
> > +	strbuf_addstr(buf, "SHA-1: Apple CommonCrypto\n");
> > +#elif defined(DC_SHA1_EXTERNAL)
> > +	strbuf_addstr(buf, "SHA-1: Collision Detection (External)\n");
> > +#elif defined(DC_SHA1_SUBMODULE)
> > +	strbuf_addstr(buf, "SHA-1: Collision Detection (Submodule)\n");
> > +#elif defined(SHA1_DC)
> > +	strbuf_addstr(buf, "SHA-1: Collision Detection\n");
> > +#endif
> > +
> > +#if defined(SHA256_OPENSSL)
> > +	strbuf_addstr(buf, "SHA-256: OpenSSL\n");
> > +#elif defined(SHA256_NETTLE)
> > +	strbuf_addstr(buf, "SHA-256: Nettle\n");
> > +#elif defined(SHA256_GCRYPT)
> > +	strbuf_addstr(buf, "SHA-256: gcrypt\n");
> > +#elif defined(SHA256_BLK)
> > +	strbuf_addstr(buf, "SHA-256: blk\n");
> > +#endif
> > +}
> 
> While I agree with the objective of the change, I am not sure how I
> feel about the implementation.  Given that
> 
>  - The code here, and probably the existing code paths that depend
>    on these SHA1_$WHOSE symbols, assume that only one of them is
>    defined;
> 
>  - The "git help --build-options" is not an end-user thing but more
>    is a developer thing.
> 
> The thing I am most worried about is that it is unclear how the
> order in which the SHA1_$WHOSE symbols are inspected here and
> elsewhere in the code are kept in sync.  What happens when, for
> example, SHA1_OPENSSL and SHA1_APPLE_UNSAFE are both defined?  The
> above code will report that we are using SHA1_OPENSSL, but hash.h
> would probably use SHA1_APPLE as it has its own if/elif/endif
> cascade.
> 
> Perhaps it does not matter, if the build infrastructure ensures that
> the build fails unless one and only one of SHA1_$WHOSE is defined.
> 
> But with the way how this part is written with an if/elif/endif
> cascade, it makes readers spend time wondering how the precedence
> order here is kept in sync throughout the system.  If I am not
> mistaken, the top-level Makefile has its own ifdef/else/if/endif*
> cascade.

Good point! Both hash.h and the Makefile have their own precedence
defined which makes this even more confusing to understand what we would
should use to keep things in sync.

> I imagine that making all of the above not if/elif/endif chain, but
> make them pretend as if they are independent and orthogonal choices,
> would make it simpler to understand and also it will help us catch a
> misconfiguration where more than one is defined, i.e.
> 
>         static void get_sha_impl(struct strbuf *buf)
>         {
>         #if defined(SHA1_OPENSSL)
>                 strbuf_addstr(buf, "SHA-1: OpenSSL\n");
>         #endif
>         #if defined(SHA1_BLK)
>                 strbuf_addstr(buf, "SHA-1: blk\n");
>         #endif
>         #if defined(SHA1_APPLE)
>         ...
> 
> 
> That way, we wouldn't force future devlopers who are plugging new
> implementations of SHA-256 wonder where is the right place in the
> existing if/elif/endif cascade their new one fits.  It also allows
> us to catch misconfigurations to define more then one of them at the
> same time, if such a thing becomes ever possible.

Keeping each of the options independent certainly keeps things more
simple and avoids having to also manage the precedence of each option
here. For most of these SHA related options we only expect one to be set
at a time anyway. I'll do this in the next version.

One exception though is that we do expect that when SHA1_DC is set,
either DC_SHA1_SUBMODULE or DC_SHA1_EXTERNAL may also be set. In this
scenario, maybe it would be fine for the printed build options to
include both. Another option would be to treat the DC_SHA1_* options
separately from the SHA1_* ones and have a separate prefix key. Maybe
something like `DC-SHA-1: {DC_SHA1_SUBMODULE,DC_SHA1_EXTERNAL}`.

I'm currently leaning towards the latter option here.

> Also, wouldn't it make more sense to just reuse the internal symbol
> for reporting, i.e.
> 
> 	strbuf_addstr(buf, "SHA-1: SHA1_OPENSSL\n");
> 
> instead of having to come up with "human readable" name here

Ya, using the internal symbol instead of a new human readable name is a
better idea. I'll update in the next version.

Thanks,
-Justin

Re: [PATCH 1/2] help: include SHA implementation in version info

[Justin Tobler]

On 25/03/31 09:19AM, Patrick Steinhardt wrote:
> On Sat, Mar 29, 2025 at 04:36:45AM -0700, Junio C Hamano wrote:
> > While I agree with the objective of the change, I am not sure how I
> > feel about the implementation.  Given that
> > 
> >  - The code here, and probably the existing code paths that depend
> >    on these SHA1_$WHOSE symbols, assume that only one of them is
> >    defined;
> > 
> >  - The "git help --build-options" is not an end-user thing but more
> >    is a developer thing.
> > 
> > The thing I am most worried about is that it is unclear how the
> > order in which the SHA1_$WHOSE symbols are inspected here and
> > elsewhere in the code are kept in sync.  What happens when, for
> > example, SHA1_OPENSSL and SHA1_APPLE_UNSAFE are both defined?  The
> > above code will report that we are using SHA1_OPENSSL, but hash.h
> > would probably use SHA1_APPLE as it has its own if/elif/endif
> > cascade.
> > 
> > Perhaps it does not matter, if the build infrastructure ensures that
> > the build fails unless one and only one of SHA1_$WHOSE is defined.
> > 
> > But with the way how this part is written with an if/elif/endif
> > cascade, it makes readers spend time wondering how the precedence
> > order here is kept in sync throughout the system.  If I am not
> > mistaken, the top-level Makefile has its own ifdef/else/if/endif*
> > cascade.
> > 
> > I imagine that making all of the above not if/elif/endif chain, but
> > make them pretend as if they are independent and orthogonal choices,
> > would make it simpler to understand and also it will help us catch a
> > misconfiguration where more than one is defined, i.e.
> > 
> >         static void get_sha_impl(struct strbuf *buf)
> >         {
> >         #if defined(SHA1_OPENSSL)
> >                 strbuf_addstr(buf, "SHA-1: OpenSSL\n");
> >         #endif
> >         #if defined(SHA1_BLK)
> >                 strbuf_addstr(buf, "SHA-1: blk\n");
> >         #endif
> >         #if defined(SHA1_APPLE)
> >         ...
> > 
> > 
> > That way, we wouldn't force future devlopers who are plugging new
> > implementations of SHA-256 wonder where is the right place in the
> > existing if/elif/endif cascade their new one fits.  It also allows
> > us to catch misconfigurations to define more then one of them at the
> > same time, if such a thing becomes ever possible.
> 
> Another option: we could ask the implementations themselves to define a
> symbol `SHA1_BACKEND` and use it here. This would automatically ensure
> that any implementation must define the symbol as we'd otherwise get a
> compile error. We could also conditionally define `SHA1_UNSAFE_BACKEND`
> depending on whether or not we have it.

The SHA backends get selected in hash.h, so we could conditionally
define symbol values based on the backend that gets selected there. This
has the benefit of centralizing backend selection in one place and the
printed build options could just depend on that.

I'll implement this approach instead in the next version.

-Justin

Re: [PATCH 0/2] help: include SHA build options in version info

[Justin Tobler]

On 25/03/29 09:58AM, Christian Couder wrote:
> On Fri, Mar 28, 2025 at 6:05 PM Justin Tobler <jltobler@gmail.com> wrote:
> > Regarding "unsafe-SHA-1", I wonder if we should use a different name in
> > the printed build options that sounds a little less scary. I was
> > thinking maybe "fast-SHA-1" and document its meaning appropriately. I'm
> > interested to know if anyone has thoughts on this.
> 
> Maybe we could use just "SHA-1" if a single algorithm is used for
> everything, and both "SHA-1 for crypto" and "SHA-1 for non-crypto"
> otherwise.

Maybe we could leave "SHA-1:" alone and when unsafe is set use
"non-crypto-SHA-1" instead of "unsafe-SHA-1"? It's a little wordy, but
probably not too bad.

> Related to this I wonder if we should warn in some ways if a non
> collision detection algorithm is used for crypto. For example we could
> print "SHA-1: OpenSSL (No collision detection!!!)" instead of just
> "SHA-1: OpenSSL". And yeah that should be documented.

Elsewhere in this thread it was suggested that we use the internal
symbol names instead of coming up with new human readable names [1].
I'll append this warning to the appropriate options as well though and
document it.

Thanks,
-Justin

[1]: <xmqq8qoodq5u.fsf@gitster.g>

Re: [PATCH 1/2] help: include SHA implementation in version info

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> Another option: we could ask the implementations themselves to define a
> symbol `SHA1_BACKEND` and use it here. This would automatically ensure
> that any implementation must define the symbol as we'd otherwise get a
> compile error. We could also conditionally define `SHA1_UNSAFE_BACKEND`
> depending on whether or not we have it.

Much simpler and less error prone.

[PATCH v2 0/2] help: include SHA build options in version info

[Justin Tobler]

Greetings,

Additional information regarding how Git was built can be found via the
`--build-options` flag for git-version(1). This currently does not
include information about the SHA-1 and SHA-256 implementations Git is
built with.

This short series adds build option info for the SHA-1, SHA-256, and
non-crypto-SHA-1 (if any) implementations which may be useful for
diagnostic purposes

Changes since V1:

        - Each SHA backend is expected to define either `SHA1_BACKEND`,
          `SHA1_UNSAFE_BACKEND`, or `SHA256_BACKEND` as appropriate.
          These symbols are then used to print the SHA build options in
          the additional version info.

        - The names of the build options are used instead of
          human-readable names.

        - Appended "(No collision detection)" to warn about SHA1
          backends without collision detection.

        - Renamed "unsafe-SHA-1" to "non-crypto-SHA-1" in the printed
          build options.

        - Small updates to documentation.

Thanks,
-Justin

Justin Tobler (2):
  help: include SHA implementation in version info
  help: include unsafe SHA-1 build info in version

 Documentation/git-version.adoc |  6 ++++++
 hash.h                         | 11 +++++++++++
 help.c                         | 13 +++++++++++++
 3 files changed, 30 insertions(+)


base-commit: 683c54c999c301c2cd6f715c411407c413b1d84e
-- 
2.49.0

[PATCH v2 1/2] help: include SHA implementation in version info

[Justin Tobler]

When the `--build-options` flag is used with git-version(1), additional
information about the built version of Git is printed. During build
time, different SHA implementations may be configured, but this
information is not included in the version info.

Add the SHA implementations Git is built with to the version info by
requiring each backend to define a SHA1_BACKEND or SHA256_BACKEND symbol
as appropriate and use the value in the printed build options.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 Documentation/git-version.adoc | 3 +++
 hash.h                         | 8 ++++++++
 help.c                         | 8 ++++++++
 3 files changed, 19 insertions(+)

diff --git a/Documentation/git-version.adoc b/Documentation/git-version.adoc
index 80fa7754a6..f06758a7cf 100644
--- a/Documentation/git-version.adoc
+++ b/Documentation/git-version.adoc
@@ -22,6 +22,9 @@ OPTIONS
 --build-options::
 	Include additional information about how git was built for diagnostic
 	purposes.
++
+Note that the SHA1 options `SHA1_APPLE`, `SHA1_OPENSSL`, and `SHA1_BLK` do not
+have collision detection.
 
 GIT
 ---
diff --git a/hash.h b/hash.h
index 4367acfec5..51cd0ec7b6 100644
--- a/hash.h
+++ b/hash.h
@@ -2,16 +2,20 @@
 #define HASH_H
 
 #if defined(SHA1_APPLE)
+#define SHA1_BACKEND "SHA1_APPLE (No collision detection)"
 #include <CommonCrypto/CommonDigest.h>
 #elif defined(SHA1_OPENSSL)
+#  define SHA1_BACKEND "SHA1_OPENSSL (No collision detection)"
 #  include <openssl/sha.h>
 #  if defined(OPENSSL_API_LEVEL) && OPENSSL_API_LEVEL >= 3
 #    define SHA1_NEEDS_CLONE_HELPER
 #    include "sha1/openssl.h"
 #  endif
 #elif defined(SHA1_DC)
+#define SHA1_BACKEND "SHA1_DC"
 #include "sha1dc_git.h"
 #else /* SHA1_BLK */
+#define SHA1_BACKEND "SHA1_BLK (No collision detection)"
 #include "block-sha1/sha1.h"
 #endif
 
@@ -46,17 +50,21 @@
 #endif
 
 #if defined(SHA256_NETTLE)
+#define SHA256_BACKEND "SHA256_NETTLE"
 #include "sha256/nettle.h"
 #elif defined(SHA256_GCRYPT)
+#define SHA256_BACKEND "SHA256_GCRYPT"
 #define SHA256_NEEDS_CLONE_HELPER
 #include "sha256/gcrypt.h"
 #elif defined(SHA256_OPENSSL)
+#  define SHA256_BACKEND "SHA256_OPENSSL"
 #  include <openssl/sha.h>
 #  if defined(OPENSSL_API_LEVEL) && OPENSSL_API_LEVEL >= 3
 #    define SHA256_NEEDS_CLONE_HELPER
 #    include "sha256/openssl.h"
 #  endif
 #else
+#define SHA256_BACKEND "SHA256_BLK"
 #include "sha256/block/sha256.h"
 #endif
 
diff --git a/help.c b/help.c
index c54bd9918a..3aebfb3681 100644
--- a/help.c
+++ b/help.c
@@ -9,6 +9,7 @@
 #include "run-command.h"
 #include "levenshtein.h"
 #include "gettext.h"
+#include "hash.h"
 #include "help.h"
 #include "command-list.h"
 #include "string-list.h"
@@ -768,6 +769,12 @@ char *help_unknown_cmd(const char *cmd)
 	exit(1);
 }
 
+static void get_sha_impl(struct strbuf *buf)
+{
+	strbuf_addf(buf, "SHA-1: %s\n", SHA1_BACKEND);
+	strbuf_addf(buf, "SHA-256: %s\n", SHA256_BACKEND);
+}
+
 void get_version_info(struct strbuf *buf, int show_build_options)
 {
 	/*
@@ -803,6 +810,7 @@ void get_version_info(struct strbuf *buf, int show_build_options)
 #elif defined ZLIB_VERSION
 		strbuf_addf(buf, "zlib: %s\n", ZLIB_VERSION);
 #endif
+		get_sha_impl(buf);
 	}
 }
 
-- 
2.49.0

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

[Justin Tobler]

In 06c92dafb8 (Makefile: allow specifying a SHA-1 for non-cryptographic
uses, 2024-09-26), support for unsafe SHA-1 is added. Add the unsafe
SHA-1 build info to `git version --build-info` and update corresponding
documentation.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 Documentation/git-version.adoc | 3 +++
 hash.h                         | 3 +++
 help.c                         | 5 +++++
 3 files changed, 11 insertions(+)

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
 ---
diff --git a/hash.h b/hash.h
index 51cd0ec7b6..72334d3506 100644
--- a/hash.h
+++ b/hash.h
@@ -20,12 +20,14 @@
 #endif
 
 #if defined(SHA1_APPLE_UNSAFE)
+#  define SHA1_UNSAFE_BACKEND "SHA1_APPLE_UNSAFE"
 #  include <CommonCrypto/CommonDigest.h>
 #  define platform_SHA_CTX_unsafe CC_SHA1_CTX
 #  define platform_SHA1_Init_unsafe CC_SHA1_Init
 #  define platform_SHA1_Update_unsafe CC_SHA1_Update
 #  define platform_SHA1_Final_unsafe CC_SHA1_Final
 #elif defined(SHA1_OPENSSL_UNSAFE)
+#  define SHA1_UNSAFE_BACKEND "SHA1_OPENSSL_UNSAFE"
 #  include <openssl/sha.h>
 #  if defined(OPENSSL_API_LEVEL) && OPENSSL_API_LEVEL >= 3
 #    define SHA1_NEEDS_CLONE_HELPER_UNSAFE
@@ -42,6 +44,7 @@
 #    define platform_SHA1_Final_unsafe SHA1_Final
 #  endif
 #elif defined(SHA1_BLK_UNSAFE)
+#  define SHA1_UNSAFE_BACKEND "SHA1_BLK_UNSAFE"
 #  include "block-sha1/sha1.h"
 #  define platform_SHA_CTX_unsafe blk_SHA_CTX
 #  define platform_SHA1_Init_unsafe blk_SHA1_Init
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
+
 	strbuf_addf(buf, "SHA-256: %s\n", SHA256_BACKEND);
 }
 
-- 
2.49.0

Re: [PATCH v2 1/2] help: include SHA implementation in version info

[Patrick Steinhardt]

On Tue, Apr 01, 2025 at 03:36:29PM -0500, Justin Tobler wrote:
> diff --git a/Documentation/git-version.adoc b/Documentation/git-version.adoc
> index 80fa7754a6..f06758a7cf 100644
> --- a/Documentation/git-version.adoc
> +++ b/Documentation/git-version.adoc
> @@ -22,6 +22,9 @@ OPTIONS
>  --build-options::
>  	Include additional information about how git was built for diagnostic
>  	purposes.
> ++
> +Note that the SHA1 options `SHA1_APPLE`, `SHA1_OPENSSL`, and `SHA1_BLK` do not
> +have collision detection.
>  
>  GIT
>  ---

I think this note is somewhat funny for an unsuspecting reader. On the
one hand they're going to be puzzled why you're talking about SHA1 in
the first place because it isn't mentioned at all beforehand. And on the
other hand they will wonder what collision detection even is in the
first place.

So I would either drop this paragraph completely or expand it to give a
bit more context.

> diff --git a/hash.h b/hash.h
> index 4367acfec5..51cd0ec7b6 100644
> --- a/hash.h
> +++ b/hash.h
> @@ -2,16 +2,20 @@
>  #define HASH_H
>  
>  #if defined(SHA1_APPLE)
> +#define SHA1_BACKEND "SHA1_APPLE (No collision detection)"
>  #include <CommonCrypto/CommonDigest.h>
>  #elif defined(SHA1_OPENSSL)
> +#  define SHA1_BACKEND "SHA1_OPENSSL (No collision detection)"
>  #  include <openssl/sha.h>
>  #  if defined(OPENSSL_API_LEVEL) && OPENSSL_API_LEVEL >= 3
>  #    define SHA1_NEEDS_CLONE_HELPER
>  #    include "sha1/openssl.h"
>  #  endif
>  #elif defined(SHA1_DC)
> +#define SHA1_BACKEND "SHA1_DC"
>  #include "sha1dc_git.h"
>  #else /* SHA1_BLK */
> +#define SHA1_BACKEND "SHA1_BLK (No collision detection)"
>  #include "block-sha1/sha1.h"
>  #endif
>  

This feels way less fragile indeed, thanks for adapting.

> diff --git a/help.c b/help.c
> index c54bd9918a..3aebfb3681 100644
> --- a/help.c
> +++ b/help.c
> @@ -768,6 +769,12 @@ char *help_unknown_cmd(const char *cmd)
>  	exit(1);
>  }
>  
> +static void get_sha_impl(struct strbuf *buf)
> +{
> +	strbuf_addf(buf, "SHA-1: %s\n", SHA1_BACKEND);
> +	strbuf_addf(buf, "SHA-256: %s\n", SHA256_BACKEND);
> +}
> +
>  void get_version_info(struct strbuf *buf, int show_build_options)
>  {
>  	/*
> @@ -803,6 +810,7 @@ void get_version_info(struct strbuf *buf, int show_build_options)
>  #elif defined ZLIB_VERSION
>  		strbuf_addf(buf, "zlib: %s\n", ZLIB_VERSION);
>  #endif
> +		get_sha_impl(buf);

I don't quite see the need for a new function, but don't mind it too
much, either.

Patrick

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

[Patrick Steinhardt]

On Tue, Apr 01, 2025 at 03:36:30PM -0500, Justin Tobler wrote:
> 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.

> 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

Re: [PATCH v2 1/2] help: include SHA implementation in version info

[Christian Couder]

On Wed, Apr 2, 2025 at 9:38 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Tue, Apr 01, 2025 at 03:36:29PM -0500, Justin Tobler wrote:
> > diff --git a/Documentation/git-version.adoc b/Documentation/git-version.adoc
> > index 80fa7754a6..f06758a7cf 100644
> > --- a/Documentation/git-version.adoc
> > +++ b/Documentation/git-version.adoc
> > @@ -22,6 +22,9 @@ OPTIONS
> >  --build-options::
> >       Include additional information about how git was built for diagnostic
> >       purposes.
> > ++
> > +Note that the SHA1 options `SHA1_APPLE`, `SHA1_OPENSSL`, and `SHA1_BLK` do not
> > +have collision detection.
>
> I think this note is somewhat funny for an unsuspecting reader. On the
> one hand they're going to be puzzled why you're talking about SHA1 in
> the first place because it isn't mentioned at all beforehand. And on the
> other hand they will wonder what collision detection even is in the
> first place.
>
> So I would either drop this paragraph completely or expand it to give a
> bit more context.

Yeah, I think it's worth giving more information, like perhaps:

"For the libraries used to implement the SHA-1 and SHA-2 algorithms
only symbolic information, like `SHA-1: SHA1_APPLE` or `SHA-256:
SHA256_NETTLE` is displayed. Note that the SHA1 options `SHA1_APPLE`,
`SHA1_OPENSSL`, and `SHA1_BLK` mean that no collision detection
algorithm is used, so known SHA-1 attacks might be possible, see
https://en.wikipedia.org/wiki/SHA-1."

Re: [PATCH v2 1/2] help: include SHA implementation in version info

[Christian Couder]

On Wed, Apr 2, 2025 at 1:26 PM Christian Couder
<christian.couder@gmail.com> wrote:
>
> On Wed, Apr 2, 2025 at 9:38 AM Patrick Steinhardt <ps@pks.im> wrote:
> >
> > On Tue, Apr 01, 2025 at 03:36:29PM -0500, Justin Tobler wrote:
> > > diff --git a/Documentation/git-version.adoc b/Documentation/git-version.adoc
> > > index 80fa7754a6..f06758a7cf 100644
> > > --- a/Documentation/git-version.adoc
> > > +++ b/Documentation/git-version.adoc
> > > @@ -22,6 +22,9 @@ OPTIONS
> > >  --build-options::
> > >       Include additional information about how git was built for diagnostic
> > >       purposes.
> > > ++
> > > +Note that the SHA1 options `SHA1_APPLE`, `SHA1_OPENSSL`, and `SHA1_BLK` do not
> > > +have collision detection.
> >
> > I think this note is somewhat funny for an unsuspecting reader. On the
> > one hand they're going to be puzzled why you're talking about SHA1 in
> > the first place because it isn't mentioned at all beforehand. And on the
> > other hand they will wonder what collision detection even is in the
> > first place.
> >
> > So I would either drop this paragraph completely or expand it to give a
> > bit more context.
>
> Yeah, I think it's worth giving more information, like perhaps:
>
> "For the libraries used to implement the SHA-1 and SHA-2 algorithms

s/SHA-2/SHA-256/

> only symbolic information, like `SHA-1: SHA1_APPLE` or `SHA-256:
> SHA256_NETTLE` is displayed. Note that the SHA1 options `SHA1_APPLE`,
> `SHA1_OPENSSL`, and `SHA1_BLK` mean that no collision detection
> algorithm is used, so known SHA-1 attacks might be possible, see
> https://en.wikipedia.org/wiki/SHA-1."

Re: [PATCH v2 1/2] help: include SHA implementation in version info

[Justin Tobler]

On 25/04/02 01:26PM, Christian Couder wrote:
> On Wed, Apr 2, 2025 at 9:38 AM Patrick Steinhardt <ps@pks.im> wrote:
> >
> > On Tue, Apr 01, 2025 at 03:36:29PM -0500, Justin Tobler wrote:
> > > diff --git a/Documentation/git-version.adoc b/Documentation/git-version.adoc
> > > index 80fa7754a6..f06758a7cf 100644
> > > --- a/Documentation/git-version.adoc
> > > +++ b/Documentation/git-version.adoc
> > > @@ -22,6 +22,9 @@ OPTIONS
> > >  --build-options::
> > >       Include additional information about how git was built for diagnostic
> > >       purposes.
> > > ++
> > > +Note that the SHA1 options `SHA1_APPLE`, `SHA1_OPENSSL`, and `SHA1_BLK` do not
> > > +have collision detection.
> >
> > I think this note is somewhat funny for an unsuspecting reader. On the
> > one hand they're going to be puzzled why you're talking about SHA1 in
> > the first place because it isn't mentioned at all beforehand. And on the
> > other hand they will wonder what collision detection even is in the
> > first place.
> >
> > So I would either drop this paragraph completely or expand it to give a
> > bit more context.
> 
> Yeah, I think it's worth giving more information, like perhaps:
> 
> "For the libraries used to implement the SHA-1 and SHA-2 algorithms
> only symbolic information, like `SHA-1: SHA1_APPLE` or `SHA-256:
> SHA256_NETTLE` is displayed. Note that the SHA1 options `SHA1_APPLE`,
> `SHA1_OPENSSL`, and `SHA1_BLK` mean that no collision detection
> algorithm is used, so known SHA-1 attacks might be possible, see
> https://en.wikipedia.org/wiki/SHA-1."

Ya the documentation here should be expanded to provide some more
context. I'll adapt in my next version to something like this:

"The libraries used to implement the SHA-1 and SHA-256 algorithms are
displayed in the form `SHA-1: <option>` and `SHA-256: <option>`
respectively. Note that the SHA-1 options `SHA1_APPLE`, `SHA1_OPENSSL`,
and `SHA1_BLK` do not use a collision detection algorithm and thus may
be vulnerable to known SHA-1 collision attacks."

-Justin

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

[Justin Tobler]

On 25/04/02 09:38AM, Patrick Steinhardt wrote:
> On Tue, Apr 01, 2025 at 03:36:30PM -0500, Justin Tobler wrote:
> > 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.

Ya, this should also be expanded a bit. I think in combination with the
expanded documentation for the prior patch, something like this might be
a bit better.

"When a faster SHA-1 implementation without collision detection is used
for only non-cryptographic purposes, the algorithm is diplayed in the form
`non-collision-detecting-SHA-1: <option>`."

> > 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?

It is suggested later to rename "non-crypto-SHA-1" to "SHA-1 without
collision detection", which could lead to something like this:

    SHA-1: SHA1_OPENSSL (No collision detection)
    SHA-1 without collision detection: none

which could be a bit misleading IMO. It might be best to leave the
option omitted if it is not defined.

> 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.

I was trying to indicate that this SHA-1 backend was used only in
non-cryptographic scenarios, but I agree that this name is not great. 
Calling it "SHA-1 used for non-cryptographic purposes" is a bit of a
mouthful, but maybe that is fine?

Another idea I had was to call it "fast-SHA-1:" since it's intended as a
performance optimization used in certain cases.

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

Being verbose here is probably best. I'll probably use something like
"non-collision-detecting-SHA-1:" in the next version.

-Justin

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

[Patrick Steinhardt]

On Wed, Apr 02, 2025 at 10:59:16AM -0500, Justin Tobler wrote:
> On 25/04/02 09:38AM, Patrick Steinhardt wrote:
> > On Tue, Apr 01, 2025 at 03:36:30PM -0500, Justin Tobler wrote:
> > > 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?
> 
> It is suggested later to rename "non-crypto-SHA-1" to "SHA-1 without
> collision detection", which could lead to something like this:
> 
>     SHA-1: SHA1_OPENSSL (No collision detection)
>     SHA-1 without collision detection: none
> 
> which could be a bit misleading IMO. It might be best to leave the
> option omitted if it is not defined.

The problem of leaving the info away entirely is that it also makes it
undiscoverable. Anyway -- I think it would be nice to always print this
line and improve the format a bit to make it less awkward, but I won't
resist if you decide to leave it as-is. After all we're already showing
strictly more information than before, so it's a net win regardless.

Patrick

[PATCH v3 0/2] help: include SHA build options in version info

[Justin Tobler]

Greetings,

Additional information regarding how Git was built can be found via the
`--build-options` flag for git-version(1). This currently does not
include information about the SHA-1 and SHA-256 implementations Git is
built with.

This short series adds build option info for the SHA-1, SHA-256, and
non-crypto-SHA-1 (if any) implementations which may be useful for
diagnostic purposes

Changes since V2:

  - Updates to documentation to provide additional context.

  - Inlined `get_sha_impl()` function.

Changes since V1:

  - Each SHA backend is expected to define either `SHA1_BACKEND`,
    `SHA1_UNSAFE_BACKEND`, or `SHA256_BACKEND` as appropriate.
    These symbols are then used to print the SHA build options in
    the additional version info.

  - The names of the build options are used instead of
    human-readable names.

  - Appended "(No collision detection)" to warn about SHA1
    backends without collision detection.

  - Renamed "unsafe-SHA-1" to "non-crypto-SHA-1" in the printed
    build options.

  - Small updates to documentation.

Thanks,
-Justin

Justin Tobler (2):
  help: include SHA implementation in version info
  help: include unsafe SHA-1 build info in version

 Documentation/git-version.adoc |  8 ++++++++
 hash.h                         | 11 +++++++++++
 help.c                         |  7 +++++++
 3 files changed, 26 insertions(+)

Range-diff against v2:
1:  aa0f464c52 ! 1:  b01e5a18cb help: include SHA implementation in version info
    @@ Documentation/git-version.adoc: OPTIONS
      	Include additional information about how git was built for diagnostic
      	purposes.
     ++
    -+Note that the SHA1 options `SHA1_APPLE`, `SHA1_OPENSSL`, and `SHA1_BLK` do not
    -+have collision detection.
    ++The libraries used to implement the SHA-1 and SHA-256 algorithms are displayed
    ++in the form `SHA-1: <option>` and `SHA-256: <option>`. Note that the SHA-1
    ++options `SHA1_APPLE`, `SHA1_OPENSSL`, and `SHA1_BLK` do not use a collision
    ++detection algorithm and thus may be vulnerable to known SHA-1 collision
    ++attacks.
      
      GIT
      ---
    @@ help.c
      #include "help.h"
      #include "command-list.h"
      #include "string-list.h"
    -@@ help.c: char *help_unknown_cmd(const char *cmd)
    - 	exit(1);
    - }
    - 
    -+static void get_sha_impl(struct strbuf *buf)
    -+{
    -+	strbuf_addf(buf, "SHA-1: %s\n", SHA1_BACKEND);
    -+	strbuf_addf(buf, "SHA-256: %s\n", SHA256_BACKEND);
    -+}
    -+
    - void get_version_info(struct strbuf *buf, int show_build_options)
    - {
    - 	/*
     @@ help.c: void get_version_info(struct strbuf *buf, int show_build_options)
      #elif defined ZLIB_VERSION
      		strbuf_addf(buf, "zlib: %s\n", ZLIB_VERSION);
      #endif
    -+		get_sha_impl(buf);
    ++		strbuf_addf(buf, "SHA-1: %s\n", SHA1_BACKEND);
    ++		strbuf_addf(buf, "SHA-256: %s\n", SHA256_BACKEND);
      	}
      }
      
2:  95c92a05df ! 2:  cf33e4ac9e help: include unsafe SHA-1 build info in version
    @@ Commit message
         Signed-off-by: Justin Tobler <jltobler@gmail.com>
     
      ## Documentation/git-version.adoc ##
    -@@ Documentation/git-version.adoc: 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".
    +@@ Documentation/git-version.adoc: The libraries used to implement the SHA-1 and SHA-256 algorithms are displayed
    + in the form `SHA-1: <option>` and `SHA-256: <option>`. Note that the SHA-1
    + options `SHA1_APPLE`, `SHA1_OPENSSL`, and `SHA1_BLK` do not use a collision
    + detection algorithm and thus may be vulnerable to known SHA-1 collision
    +-attacks.
    ++attacks. When a faster SHA-1 implementation without collision detection is used
    ++for only non-cryptographic purposes, the algorithm is displayed in the form
    ++`non-collision-detecting-SHA-1: <option>`.
      
      GIT
      ---
    @@ hash.h
      #  define platform_SHA1_Init_unsafe blk_SHA1_Init
     
      ## help.c ##
    -@@ help.c: 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);
    +@@ help.c: void get_version_info(struct strbuf *buf, int show_build_options)
    + 		strbuf_addf(buf, "zlib: %s\n", ZLIB_VERSION);
    + #endif
    + 		strbuf_addf(buf, "SHA-1: %s\n", SHA1_BACKEND);
    ++#if defined SHA1_UNSAFE_BACKEND
    ++		strbuf_addf(buf, "non-collision-detecting-SHA-1: %s\n",
    ++			    SHA1_UNSAFE_BACKEND);
     +#endif
    -+
    - 	strbuf_addf(buf, "SHA-256: %s\n", SHA256_BACKEND);
    + 		strbuf_addf(buf, "SHA-256: %s\n", SHA256_BACKEND);
    + 	}
      }
    - 

base-commit: 683c54c999c301c2cd6f715c411407c413b1d84e
-- 
2.49.0

[PATCH v3 1/2] help: include SHA implementation in version info

[Justin Tobler]

When the `--build-options` flag is used with git-version(1), additional
information about the built version of Git is printed. During build
time, different SHA implementations may be configured, but this
information is not included in the version info.

Add the SHA implementations Git is built with to the version info by
requiring each backend to define a SHA1_BACKEND or SHA256_BACKEND symbol
as appropriate and use the value in the printed build options.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 Documentation/git-version.adoc | 6 ++++++
 hash.h                         | 8 ++++++++
 help.c                         | 3 +++
 3 files changed, 17 insertions(+)

diff --git a/Documentation/git-version.adoc b/Documentation/git-version.adoc
index 80fa7754a6..913ebf147d 100644
--- a/Documentation/git-version.adoc
+++ b/Documentation/git-version.adoc
@@ -22,6 +22,12 @@ OPTIONS
 --build-options::
 	Include additional information about how git was built for diagnostic
 	purposes.
++
+The libraries used to implement the SHA-1 and SHA-256 algorithms are displayed
+in the form `SHA-1: <option>` and `SHA-256: <option>`. Note that the SHA-1
+options `SHA1_APPLE`, `SHA1_OPENSSL`, and `SHA1_BLK` do not use a collision
+detection algorithm and thus may be vulnerable to known SHA-1 collision
+attacks.
 
 GIT
 ---
diff --git a/hash.h b/hash.h
index 4367acfec5..51cd0ec7b6 100644
--- a/hash.h
+++ b/hash.h
@@ -2,16 +2,20 @@
 #define HASH_H
 
 #if defined(SHA1_APPLE)
+#define SHA1_BACKEND "SHA1_APPLE (No collision detection)"
 #include <CommonCrypto/CommonDigest.h>
 #elif defined(SHA1_OPENSSL)
+#  define SHA1_BACKEND "SHA1_OPENSSL (No collision detection)"
 #  include <openssl/sha.h>
 #  if defined(OPENSSL_API_LEVEL) && OPENSSL_API_LEVEL >= 3
 #    define SHA1_NEEDS_CLONE_HELPER
 #    include "sha1/openssl.h"
 #  endif
 #elif defined(SHA1_DC)
+#define SHA1_BACKEND "SHA1_DC"
 #include "sha1dc_git.h"
 #else /* SHA1_BLK */
+#define SHA1_BACKEND "SHA1_BLK (No collision detection)"
 #include "block-sha1/sha1.h"
 #endif
 
@@ -46,17 +50,21 @@
 #endif
 
 #if defined(SHA256_NETTLE)
+#define SHA256_BACKEND "SHA256_NETTLE"
 #include "sha256/nettle.h"
 #elif defined(SHA256_GCRYPT)
+#define SHA256_BACKEND "SHA256_GCRYPT"
 #define SHA256_NEEDS_CLONE_HELPER
 #include "sha256/gcrypt.h"
 #elif defined(SHA256_OPENSSL)
+#  define SHA256_BACKEND "SHA256_OPENSSL"
 #  include <openssl/sha.h>
 #  if defined(OPENSSL_API_LEVEL) && OPENSSL_API_LEVEL >= 3
 #    define SHA256_NEEDS_CLONE_HELPER
 #    include "sha256/openssl.h"
 #  endif
 #else
+#define SHA256_BACKEND "SHA256_BLK"
 #include "sha256/block/sha256.h"
 #endif
 
diff --git a/help.c b/help.c
index c54bd9918a..991a9525db 100644
--- a/help.c
+++ b/help.c
@@ -9,6 +9,7 @@
 #include "run-command.h"
 #include "levenshtein.h"
 #include "gettext.h"
+#include "hash.h"
 #include "help.h"
 #include "command-list.h"
 #include "string-list.h"
@@ -803,6 +804,8 @@ void get_version_info(struct strbuf *buf, int show_build_options)
 #elif defined ZLIB_VERSION
 		strbuf_addf(buf, "zlib: %s\n", ZLIB_VERSION);
 #endif
+		strbuf_addf(buf, "SHA-1: %s\n", SHA1_BACKEND);
+		strbuf_addf(buf, "SHA-256: %s\n", SHA256_BACKEND);
 	}
 }
 
-- 
2.49.0

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

[Justin Tobler]

In 06c92dafb8 (Makefile: allow specifying a SHA-1 for non-cryptographic
uses, 2024-09-26), support for unsafe SHA-1 is added. Add the unsafe
SHA-1 build info to `git version --build-info` and update corresponding
documentation.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 Documentation/git-version.adoc | 4 +++-
 hash.h                         | 3 +++
 help.c                         | 4 ++++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-version.adoc b/Documentation/git-version.adoc
index 913ebf147d..9462043a14 100644
--- a/Documentation/git-version.adoc
+++ b/Documentation/git-version.adoc
@@ -27,7 +27,9 @@ The libraries used to implement the SHA-1 and SHA-256 algorithms are displayed
 in the form `SHA-1: <option>` and `SHA-256: <option>`. Note that the SHA-1
 options `SHA1_APPLE`, `SHA1_OPENSSL`, and `SHA1_BLK` do not use a collision
 detection algorithm and thus may be vulnerable to known SHA-1 collision
-attacks.
+attacks. When a faster SHA-1 implementation without collision detection is used
+for only non-cryptographic purposes, the algorithm is displayed in the form
+`non-collision-detecting-SHA-1: <option>`.
 
 GIT
 ---
diff --git a/hash.h b/hash.h
index 51cd0ec7b6..72334d3506 100644
--- a/hash.h
+++ b/hash.h
@@ -20,12 +20,14 @@
 #endif
 
 #if defined(SHA1_APPLE_UNSAFE)
+#  define SHA1_UNSAFE_BACKEND "SHA1_APPLE_UNSAFE"
 #  include <CommonCrypto/CommonDigest.h>
 #  define platform_SHA_CTX_unsafe CC_SHA1_CTX
 #  define platform_SHA1_Init_unsafe CC_SHA1_Init
 #  define platform_SHA1_Update_unsafe CC_SHA1_Update
 #  define platform_SHA1_Final_unsafe CC_SHA1_Final
 #elif defined(SHA1_OPENSSL_UNSAFE)
+#  define SHA1_UNSAFE_BACKEND "SHA1_OPENSSL_UNSAFE"
 #  include <openssl/sha.h>
 #  if defined(OPENSSL_API_LEVEL) && OPENSSL_API_LEVEL >= 3
 #    define SHA1_NEEDS_CLONE_HELPER_UNSAFE
@@ -42,6 +44,7 @@
 #    define platform_SHA1_Final_unsafe SHA1_Final
 #  endif
 #elif defined(SHA1_BLK_UNSAFE)
+#  define SHA1_UNSAFE_BACKEND "SHA1_BLK_UNSAFE"
 #  include "block-sha1/sha1.h"
 #  define platform_SHA_CTX_unsafe blk_SHA_CTX
 #  define platform_SHA1_Init_unsafe blk_SHA1_Init
diff --git a/help.c b/help.c
index 991a9525db..6ef90838f1 100644
--- a/help.c
+++ b/help.c
@@ -805,6 +805,10 @@ void get_version_info(struct strbuf *buf, int show_build_options)
 		strbuf_addf(buf, "zlib: %s\n", ZLIB_VERSION);
 #endif
 		strbuf_addf(buf, "SHA-1: %s\n", SHA1_BACKEND);
+#if defined SHA1_UNSAFE_BACKEND
+		strbuf_addf(buf, "non-collision-detecting-SHA-1: %s\n",
+			    SHA1_UNSAFE_BACKEND);
+#endif
 		strbuf_addf(buf, "SHA-256: %s\n", SHA256_BACKEND);
 	}
 }
-- 
2.49.0

Re: [PATCH v3 0/2] help: include SHA build options in version info

[Patrick Steinhardt]

On Thu, Apr 03, 2025 at 09:05:27AM -0500, Justin Tobler wrote:
> Greetings,
> 
> Additional information regarding how Git was built can be found via the
> `--build-options` flag for git-version(1). This currently does not
> include information about the SHA-1 and SHA-256 implementations Git is
> built with.
> 
> This short series adds build option info for the SHA-1, SHA-256, and
> non-crypto-SHA-1 (if any) implementations which may be useful for
> diagnostic purposes
> 
> Changes since V2:
> 
>   - Updates to documentation to provide additional context.
> 
>   - Inlined `get_sha_impl()` function.

Thanks, I'm happy with this version.

Patrick

Re: [PATCH v3 0/2] help: include SHA build options in version info

[Christian Couder]

On Fri, Apr 4, 2025 at 11:20 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Thu, Apr 03, 2025 at 09:05:27AM -0500, Justin Tobler wrote:
> > Greetings,
> >
> > Additional information regarding how Git was built can be found via the
> > `--build-options` flag for git-version(1). This currently does not
> > include information about the SHA-1 and SHA-256 implementations Git is
> > built with.
> >
> > This short series adds build option info for the SHA-1, SHA-256, and
> > non-crypto-SHA-1 (if any) implementations which may be useful for
> > diagnostic purposes
> >
> > Changes since V2:
> >
> >   - Updates to documentation to provide additional context.
> >
> >   - Inlined `get_sha_impl()` function.
>
> Thanks, I'm happy with this version.

I am fine with it as well. Thanks.

Re: [PATCH v3 0/2] help: include SHA build options in version info

[Junio C Hamano]

Christian Couder <christian.couder@gmail.com> writes:

> On Fri, Apr 4, 2025 at 11:20 AM Patrick Steinhardt <ps@pks.im> wrote:
>>
>> On Thu, Apr 03, 2025 at 09:05:27AM -0500, Justin Tobler wrote:
>> >
>> Thanks, I'm happy with this version.
>
> I am fine with it as well. Thanks.

Thanks, all.  Will replace.  Let me mark the topic for 'next'.