diff mbox series

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

Message ID 20250401203630.285451-2-jltobler@gmail.com (mailing list archive)
State New
Headers show
Series help: include SHA build options in version info | expand

Commit Message

Justin Tobler April 1, 2025, 8:36 p.m. UTC
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(+)

Comments

Patrick Steinhardt April 2, 2025, 7:38 a.m. UTC | #1
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
Christian Couder April 2, 2025, 11:26 a.m. UTC | #2
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."
Christian Couder April 2, 2025, 11:27 a.m. UTC | #3
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."
Justin Tobler April 2, 2025, 2:56 p.m. UTC | #4
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
diff mbox series

Patch

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);
 	}
 }