| Message ID | 20250401203630.285451-3-jltobler@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | help: include SHA build options in version info | expand |
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
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
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); }
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(+)