* [PATCH 0/2] avoid functions deprecated in OpenSSL 3+ @ 2023-08-01 2:54 Eric Wong 2023-08-01 2:54 ` [PATCH 1/2] sha256: " Eric Wong 2023-08-01 2:54 ` [PATCH 2/2] avoid SHA-1 " Eric Wong 0 siblings, 2 replies; 6+ messages in thread From: Eric Wong @ 2023-08-01 2:54 UTC (permalink / raw) To: git 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] sha256: avoid functions deprecated in OpenSSL 3+ 2023-08-01 2:54 [PATCH 0/2] avoid functions deprecated in OpenSSL 3+ Eric Wong @ 2023-08-01 2:54 ` Eric Wong 2023-08-01 2:54 ` [PATCH 2/2] avoid SHA-1 " Eric Wong 1 sibling, 0 replies; 6+ messages in thread From: Eric Wong @ 2023-08-01 2:54 UTC (permalink / raw) To: git 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 */ ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] avoid SHA-1 functions deprecated in OpenSSL 3+ 2023-08-01 2:54 [PATCH 0/2] avoid functions deprecated in OpenSSL 3+ Eric Wong 2023-08-01 2:54 ` [PATCH 1/2] sha256: " Eric Wong @ 2023-08-01 2:54 ` Eric Wong 2023-08-01 16:03 ` Junio C Hamano 1 sibling, 1 reply; 6+ messages in thread From: Eric Wong @ 2023-08-01 2:54 UTC (permalink / raw) To: git 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 */ ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] avoid SHA-1 functions deprecated in OpenSSL 3+ 2023-08-01 2:54 ` [PATCH 2/2] avoid SHA-1 " Eric Wong @ 2023-08-01 16:03 ` Junio C Hamano 2023-08-01 19:53 ` Eric Wong 0 siblings, 1 reply; 6+ messages in thread From: Junio C Hamano @ 2023-08-01 16:03 UTC (permalink / raw) To: Eric Wong; +Cc: git 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] avoid SHA-1 functions deprecated in OpenSSL 3+ 2023-08-01 16:03 ` Junio C Hamano @ 2023-08-01 19:53 ` Eric Wong 2023-08-01 20:17 ` Junio C Hamano 0 siblings, 1 reply; 6+ messages in thread From: Eric Wong @ 2023-08-01 19:53 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] avoid SHA-1 functions deprecated in OpenSSL 3+ 2023-08-01 19:53 ` Eric Wong @ 2023-08-01 20:17 ` Junio C Hamano 0 siblings, 0 replies; 6+ messages in thread From: Junio C Hamano @ 2023-08-01 20:17 UTC (permalink / raw) To: Eric Wong; +Cc: git 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-08-01 20:18 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-08-01 2:54 [PATCH 0/2] avoid functions deprecated in OpenSSL 3+ Eric Wong 2023-08-01 2:54 ` [PATCH 1/2] sha256: " Eric Wong 2023-08-01 2:54 ` [PATCH 2/2] avoid SHA-1 " Eric Wong 2023-08-01 16:03 ` Junio C Hamano 2023-08-01 19:53 ` Eric Wong 2023-08-01 20:17 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).