[PATCH 0/2] avoid functions deprecated in OpenSSL 3+

[PATCH 0/2] avoid functions deprecated in OpenSSL 3+

[Eric Wong]

OpenSSL appears to be getting rid of the SHA1* and SHA256*
functions in favor of the more generic EVP_* APIs.  The EVP_*
APIs unfortunately require more attention to be paid to memory
management and require specialized copy functions (like gcrypt),
so I'm only using them with OpenSSL 3.x (I've tested 1.1.1n, too).

I'm in favor of keeping OpenSSL support since its development
headers/libraries are more likely to be already-installed on
developers' systems than nettle or gcrypt.

On Debian systems participating in popularity-contest:
libssl-dev is in 21.95% of systems, while nettle-dev and
libgcrypt20-dev is are only in 4.08% and 2.94%, respectively:

  https://qa.debian.org/popcon.php?package=openssl
  https://qa.debian.org/popcon.php?package=nettle
  https://qa.debian.org/popcon.php?package=libgcrypt20

Eric Wong (2):
  sha256: avoid functions deprecated in OpenSSL 3+
  avoid SHA-1 functions deprecated in OpenSSL 3+

 Makefile         |  6 ++++++
 hash-ll.h        | 18 ++++++++++++++++--
 sha1/openssl.h   | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
 sha256/openssl.h | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 120 insertions(+), 2 deletions(-)
 create mode 100644 sha1/openssl.h
 create mode 100644 sha256/openssl.h

[PATCH 1/2] sha256: avoid functions deprecated in OpenSSL 3+

[Eric Wong]

OpenSSL 3+ deprecates the SHA256_Init, SHA256_Update, and SHA256_Final
functions, leading to errors when building with `DEVELOPER=1'.

Use the newer EVP_* API with OpenSSL 3+ despite being more
error-prone and less efficient due to heap allocations.

Signed-off-by: Eric Wong <e@80x24.org>
---
 Makefile         |  3 +++
 hash-ll.h        |  6 +++++-
 sha256/openssl.h | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 57 insertions(+), 1 deletion(-)
 create mode 100644 sha256/openssl.h

diff --git a/Makefile b/Makefile
index fb541dedc9..a499c5d7f2 100644
--- a/Makefile
+++ b/Makefile
@@ -3216,6 +3216,9 @@ $(SP_OBJ): %.sp: %.c %.o
 sparse: $(SP_OBJ)
 
 EXCEPT_HDRS := $(GENERATED_H) unicode-width.h compat/% xdiff/%
+ifndef OPENSSL_SHA256
+	EXCEPT_HDRS += sha256/openssl.h
+endif
 ifndef NETTLE_SHA256
 	EXCEPT_HDRS += sha256/nettle.h
 endif
diff --git a/hash-ll.h b/hash-ll.h
index 8d7973769f..087b421bd5 100644
--- a/hash-ll.h
+++ b/hash-ll.h
@@ -17,7 +17,11 @@
 #define SHA256_NEEDS_CLONE_HELPER
 #include "sha256/gcrypt.h"
 #elif defined(SHA256_OPENSSL)
-#include <openssl/sha.h>
+#  include <openssl/sha.h>
+#  if defined(OPENSSL_API_LEVEL) && OPENSSL_API_LEVEL >= 3
+#    define SHA256_NEEDS_CLONE_HELPER
+#    include "sha256/openssl.h"
+#  endif
 #else
 #include "sha256/block/sha256.h"
 #endif
diff --git a/sha256/openssl.h b/sha256/openssl.h
new file mode 100644
index 0000000000..c1083d9491
--- /dev/null
+++ b/sha256/openssl.h
@@ -0,0 +1,49 @@
+/* wrappers for the EVP API of OpenSSL 3+ */
+#ifndef SHA256_OPENSSL_H
+#define SHA256_OPENSSL_H
+#include <openssl/evp.h>
+
+struct openssl_SHA256_CTX {
+	EVP_MD_CTX *ectx;
+};
+
+typedef struct openssl_SHA256_CTX openssl_SHA256_CTX;
+
+static inline void openssl_SHA256_Init(struct openssl_SHA256_CTX *ctx)
+{
+	const EVP_MD *type = EVP_sha256();
+
+	ctx->ectx = EVP_MD_CTX_new();
+	if (!ctx->ectx)
+		die("EVP_MD_CTX_new: out of memory");
+
+	EVP_DigestInit_ex(ctx->ectx, type, NULL);
+}
+
+static inline void openssl_SHA256_Update(struct openssl_SHA256_CTX *ctx,
+					const void *data,
+					size_t len)
+{
+	EVP_DigestUpdate(ctx->ectx, data, len);
+}
+
+static inline void openssl_SHA256_Final(unsigned char *digest,
+				       struct openssl_SHA256_CTX *ctx)
+{
+	EVP_DigestFinal_ex(ctx->ectx, digest, NULL);
+	EVP_MD_CTX_free(ctx->ectx);
+}
+
+static inline void openssl_SHA256_Clone(struct openssl_SHA256_CTX *dst,
+					const struct openssl_SHA256_CTX *src)
+{
+	EVP_MD_CTX_copy_ex(dst->ectx, src->ectx);
+}
+
+#define platform_SHA256_CTX openssl_SHA256_CTX
+#define platform_SHA256_Init openssl_SHA256_Init
+#define platform_SHA256_Clone openssl_SHA256_Clone
+#define platform_SHA256_Update openssl_SHA256_Update
+#define platform_SHA256_Final openssl_SHA256_Final
+
+#endif /* SHA256_OPENSSL_H */

[PATCH 2/2] avoid SHA-1 functions deprecated in OpenSSL 3+

[Eric Wong]

OpenSSL 3+ deprecates the SHA1_Init, SHA1_Update, and SHA1_Final
functions, leading to errors when building with `DEVELOPER=1'.

Use the newer EVP_* API with OpenSSL 3+ (only) despite being more
error-prone and less efficient due to heap allocations.

Signed-off-by: Eric Wong <e@80x24.org>
---
 Makefile       |  3 +++
 hash-ll.h      | 12 +++++++++++-
 sha1/openssl.h | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 1 deletion(-)
 create mode 100644 sha1/openssl.h

diff --git a/Makefile b/Makefile
index a499c5d7f2..ace3e5a506 100644
--- a/Makefile
+++ b/Makefile
@@ -3216,6 +3216,9 @@ $(SP_OBJ): %.sp: %.c %.o
 sparse: $(SP_OBJ)
 
 EXCEPT_HDRS := $(GENERATED_H) unicode-width.h compat/% xdiff/%
+ifndef OPENSSL_SHA1
+	EXCEPT_HDRS += sha1/openssl.h
+endif
 ifndef OPENSSL_SHA256
 	EXCEPT_HDRS += sha256/openssl.h
 endif
diff --git a/hash-ll.h b/hash-ll.h
index 087b421bd5..10d84cc208 100644
--- a/hash-ll.h
+++ b/hash-ll.h
@@ -4,7 +4,11 @@
 #if defined(SHA1_APPLE)
 #include <CommonCrypto/CommonDigest.h>
 #elif defined(SHA1_OPENSSL)
-#include <openssl/sha.h>
+#  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)
 #include "sha1dc_git.h"
 #else /* SHA1_BLK */
@@ -45,6 +49,10 @@
 #define git_SHA1_Update		platform_SHA1_Update
 #define git_SHA1_Final		platform_SHA1_Final
 
+#ifdef platform_SHA1_Clone
+#define git_SHA1_Clone	platform_SHA1_Clone
+#endif
+
 #ifndef platform_SHA256_CTX
 #define platform_SHA256_CTX	SHA256_CTX
 #define platform_SHA256_Init	SHA256_Init
@@ -67,10 +75,12 @@
 #define git_SHA1_Update		git_SHA1_Update_Chunked
 #endif
 
+#ifndef SHA1_NEEDS_CLONE_HELPER
 static inline void git_SHA1_Clone(git_SHA_CTX *dst, const git_SHA_CTX *src)
 {
 	memcpy(dst, src, sizeof(*dst));
 }
+#endif
 
 #ifndef SHA256_NEEDS_CLONE_HELPER
 static inline void git_SHA256_Clone(git_SHA256_CTX *dst, const git_SHA256_CTX *src)
diff --git a/sha1/openssl.h b/sha1/openssl.h
new file mode 100644
index 0000000000..006c1f4ba5
--- /dev/null
+++ b/sha1/openssl.h
@@ -0,0 +1,49 @@
+/* wrappers for the EVP API of OpenSSL 3+ */
+#ifndef SHA1_OPENSSL_H
+#define SHA1_OPENSSL_H
+#include <openssl/evp.h>
+
+struct openssl_SHA1_CTX {
+	EVP_MD_CTX *ectx;
+};
+
+typedef struct openssl_SHA1_CTX openssl_SHA1_CTX;
+
+static inline void openssl_SHA1_Init(struct openssl_SHA1_CTX *ctx)
+{
+	const EVP_MD *type = EVP_sha1();
+
+	ctx->ectx = EVP_MD_CTX_new();
+	if (!ctx->ectx)
+		die("EVP_MD_CTX_new: out of memory");
+
+	EVP_DigestInit_ex(ctx->ectx, type, NULL);
+}
+
+static inline void openssl_SHA1_Update(struct openssl_SHA1_CTX *ctx,
+					const void *data,
+					size_t len)
+{
+	EVP_DigestUpdate(ctx->ectx, data, len);
+}
+
+static inline void openssl_SHA1_Final(unsigned char *digest,
+				       struct openssl_SHA1_CTX *ctx)
+{
+	EVP_DigestFinal_ex(ctx->ectx, digest, NULL);
+	EVP_MD_CTX_free(ctx->ectx);
+}
+
+static inline void openssl_SHA1_Clone(struct openssl_SHA1_CTX *dst,
+					const struct openssl_SHA1_CTX *src)
+{
+	EVP_MD_CTX_copy_ex(dst->ectx, src->ectx);
+}
+
+#define platform_SHA_CTX openssl_SHA1_CTX
+#define platform_SHA1_Init openssl_SHA1_Init
+#define platform_SHA1_Clone openssl_SHA1_Clone
+#define platform_SHA1_Update openssl_SHA1_Update
+#define platform_SHA1_Final openssl_SHA1_Final
+
+#endif /* SHA1_OPENSSL_H */

Re: [PATCH 2/2] avoid SHA-1 functions deprecated in OpenSSL 3+

[Junio C Hamano]

Eric Wong <e@80x24.org> writes:

> diff --git a/hash-ll.h b/hash-ll.h
> index 087b421bd5..10d84cc208 100644
> --- a/hash-ll.h
> +++ b/hash-ll.h
> @@ -45,6 +49,10 @@
>  #define git_SHA1_Update		platform_SHA1_Update
>  #define git_SHA1_Final		platform_SHA1_Final
>  
> +#ifdef platform_SHA1_Clone
> +#define git_SHA1_Clone	platform_SHA1_Clone
> +#endif
> +
> ...
> +#ifndef SHA1_NEEDS_CLONE_HELPER
>  static inline void git_SHA1_Clone(git_SHA_CTX *dst, const git_SHA_CTX *src)
>  {
>  	memcpy(dst, src, sizeof(*dst));
>  }
> +#endif

This smelled a bit strange in that all the other platform_* stuff is
"if a platform sha-1 header implements platform_SHA1_*, we will use
it to define git_SHA1_* (which is the symbol we use in the code)"
plus its inverse "if there is no specific platform_SHA1_*, we assume
OpenSSL compatible ones and use them as platform_SHA1_* (which in
turn will be used as git_SHA1-*)".

And that is why "#ifndef platform_SHA1_CTX" block gave us default
values for them.  And from that point of view, the first hunk
(i.e. "if SHA1_CLONE is defined for the platform, we use it") is
entirely sensible.

But I did not get why we guard the other hunk with a different CPP
macro.  If we have platform_SHA1_Clone already defined, and then
NEEDS_CLONE_HELPER not defined, we end up creating an static inline
platform_SHA1_CLONE here, and I was not sure if that is what we
wanted to do.

The answer to the above puzzle (at least it was a puzzle to me) is
that the new header "sha1/openssl.h" added by this series does have
platform_SHA1_Clone defined, and the code that includes it define
NEEDS_CLONE_HELPER to avoid this "static inline", so the CPP macro
SHA1_NEEDS_CLONE_HELPER means "we need more than just a straight
bitwise copy to clone the SHA context, which is provided elsewhere
in the form of platform_SHA1_Clone".

So everything evens out.  If we are with newer OpenSSL, we will
include sha1/openssl.h and get both platform_SHA1_Clone and
SHA1_NEEDS_CLONE_HELPER defined.  If we are with older OpenSSL or
non-OpenSSL, we do not get platform_SHA1_Clone (because the "#ifndef
platform_SHA1_CTX" block does not have a fallback default defined)
and we do not get SHA1_NEEDS_CLONE_HELPER either.  We either use the
memcpy() fallback only when we are not working with newer OpenSSL or
whatever defines its own platform_SHA1_Clone.  So the patch smelled
a bit strange, but there isn't anything incorrect per-se.

But then is this making folks unnecessary work when they add
non-OpenSSL support that needs more than just memcpy() to clone the
context?  What breaks if we turn these two hunks into

	#ifdef platform_SHA1_Clone
	#define git_SHA1_Clone platform_SHA1_Clone
	#else
	static inline void git_SHA1_Clone(git_SHA_CTX *dst, git_SHA_CTX *src)
	{
		memcpy(dst, src, sizeof(*dst));
	}
	#endif

and drop the requirement that they must define SHA1_NEEDS_CLONE_HELPER
if they want to define their own platform_SHA1_Clone()?

Thanks.  Everything else in the patch made sense (even though I am
not familiar with the EVP API) to me.

Re: [PATCH 2/2] avoid SHA-1 functions deprecated in OpenSSL 3+

[Eric Wong]

Junio C Hamano <gitster@pobox.com> wrote:
> Eric Wong <e@80x24.org> writes:
> 
> > diff --git a/hash-ll.h b/hash-ll.h
> > index 087b421bd5..10d84cc208 100644
> > --- a/hash-ll.h
> > +++ b/hash-ll.h
> > @@ -45,6 +49,10 @@
> >  #define git_SHA1_Update		platform_SHA1_Update
> >  #define git_SHA1_Final		platform_SHA1_Final
> >  
> > +#ifdef platform_SHA1_Clone
> > +#define git_SHA1_Clone	platform_SHA1_Clone
> > +#endif
> > +
> > ...
> > +#ifndef SHA1_NEEDS_CLONE_HELPER
> >  static inline void git_SHA1_Clone(git_SHA_CTX *dst, const git_SHA_CTX *src)
> >  {
> >  	memcpy(dst, src, sizeof(*dst));
> >  }
> > +#endif
> 
> This smelled a bit strange in that all the other platform_* stuff is
> "if a platform sha-1 header implements platform_SHA1_*, we will use
> it to define git_SHA1_* (which is the symbol we use in the code)"
> plus its inverse "if there is no specific platform_SHA1_*, we assume
> OpenSSL compatible ones and use them as platform_SHA1_* (which in
> turn will be used as git_SHA1-*)".
> 
> And that is why "#ifndef platform_SHA1_CTX" block gave us default
> values for them.  And from that point of view, the first hunk
> (i.e. "if SHA1_CLONE is defined for the platform, we use it") is
> entirely sensible.
> 
> But I did not get why we guard the other hunk with a different CPP
> macro.  If we have platform_SHA1_Clone already defined, and then
> NEEDS_CLONE_HELPER not defined, we end up creating an static inline
> platform_SHA1_CLONE here, and I was not sure if that is what we
> wanted to do.
> 
> The answer to the above puzzle (at least it was a puzzle to me) is
> that the new header "sha1/openssl.h" added by this series does have
> platform_SHA1_Clone defined, and the code that includes it define
> NEEDS_CLONE_HELPER to avoid this "static inline", so the CPP macro
> SHA1_NEEDS_CLONE_HELPER means "we need more than just a straight
> bitwise copy to clone the SHA context, which is provided elsewhere
> in the form of platform_SHA1_Clone".
> 
> So everything evens out.  If we are with newer OpenSSL, we will
> include sha1/openssl.h and get both platform_SHA1_Clone and
> SHA1_NEEDS_CLONE_HELPER defined.  If we are with older OpenSSL or
> non-OpenSSL, we do not get platform_SHA1_Clone (because the "#ifndef
> platform_SHA1_CTX" block does not have a fallback default defined)
> and we do not get SHA1_NEEDS_CLONE_HELPER either.  We either use the
> memcpy() fallback only when we are not working with newer OpenSSL or
> whatever defines its own platform_SHA1_Clone.  So the patch smelled
> a bit strange, but there isn't anything incorrect per-se.
> 
> But then is this making folks unnecessary work when they add
> non-OpenSSL support that needs more than just memcpy() to clone the
> context?  What breaks if we turn these two hunks into
> 
> 	#ifdef platform_SHA1_Clone
> 	#define git_SHA1_Clone platform_SHA1_Clone
> 	#else
> 	static inline void git_SHA1_Clone(git_SHA_CTX *dst, git_SHA_CTX *src)
> 	{
> 		memcpy(dst, src, sizeof(*dst));
> 	}
> 	#endif
> 
> and drop the requirement that they must define SHA1_NEEDS_CLONE_HELPER
> if they want to define their own platform_SHA1_Clone()?

I just copied the existing SHA256 stuff and mostly did a
s/SHA256/SHA1/ in patch 2/2.  I'm not sure why
SHA256_NEEDS_CLONE_HELPER was needed, either, but I decided
to keep the SHA1 and SHA256 code as similar as possible for
consistency.

We could probably drop both *_NEEDS_CLONE_HELPER macros,
but that's a separate patch.

Re: [PATCH 2/2] avoid SHA-1 functions deprecated in OpenSSL 3+

[Junio C Hamano]

Eric Wong <e@80x24.org> writes:

> I just copied the existing SHA256 stuff and mostly did a
> s/SHA256/SHA1/ in patch 2/2.  I'm not sure why
> SHA256_NEEDS_CLONE_HELPER was needed, either, but I decided
> to keep the SHA1 and SHA256 code as similar as possible for
> consistency.
>
> We could probably drop both *_NEEDS_CLONE_HELPER macros,
> but that's a separate patch.

Fair enough.  Thanks.