{"thread":{"id":"60048","subject":"[PATCH 0/2] avoid functions deprecated in OpenSSL 3+","startedAt":"2023-08-01T02:54:57Z","lastAt":"2023-08-01T20:18:03Z","messageCount":6,"participants":["Eric Wong","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"480033","messageId":"20230801025454.1137802-1-e@80x24.org","threadId":"60048","inReplyTo":null,"subject":"[PATCH 0/2] avoid functions deprecated in OpenSSL 3+","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2023-08-01T02:54:52Z","receivedAt":"2023-08-01T02:54:57Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"OpenSSL appears to be getting rid of the SHA1* and SHA256*\nfunctions in favor of the more generic EVP_* APIs. The EVP_*\nAPIs unfortunately require more attention to be paid to memory\nmanagement and require specialized copy functions (like gcrypt),\nso I'm only using them with OpenSSL 3.x (I've tested 1.1.1n, too).\n\nI'm in favor of keeping OpenSSL support since its development\nheaders/libraries are more likely to be already-installed on\ndevelopers' systems than nettle or gcrypt.\n\nOn Debian systems participating in popularity-contest:\nlibssl-dev is in 21.95% of systems, while nettle-dev and\nlibgcrypt20-dev is are only in 4.08% and 2.94%, respectively:\n\n https://qa.debian.org/popcon.php?package=openssl\n https://qa.debian.org/popcon.php?package=nettle\n https://qa.debian.org/popcon.php?package=libgcrypt20\n\nEric Wong (2):\n sha256: avoid functions deprecated in OpenSSL 3+\n avoid SHA-1 functions deprecated in OpenSSL 3+\n\n Makefile | 6 ++++++\n hash-ll.h | 18 ++++++++++++++++--\n sha1/openssl.h | 49 ++++++++++++++++++++++++++++++++++++++++++++++++\n sha256/openssl.h | 49 ++++++++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 120 insertions(+), 2 deletions(-)\n create mode 100644 sha1/openssl.h\n create mode 100644 sha256/openssl.h\n"},{"id":"480034","messageId":"20230801025454.1137802-2-e@80x24.org","threadId":"60048","inReplyTo":"20230801025454.1137802-1-e@80x24.org","subject":"[PATCH 1/2] sha256: avoid functions deprecated in OpenSSL 3+","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2023-08-01T02:54:53Z","receivedAt":"2023-08-01T02:55:04Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"OpenSSL 3+ deprecates the SHA256_Init, SHA256_Update, and SHA256_Final\nfunctions, leading to errors when building with `DEVELOPER=1'.\n\nUse the newer EVP_* API with OpenSSL 3+ despite being more\nerror-prone and less efficient due to heap allocations.\n\nSigned-off-by: Eric Wong \n---\n Makefile | 3 +++\n hash-ll.h | 6 +++++-\n sha256/openssl.h | 49 ++++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 57 insertions(+), 1 deletion(-)\n create mode 100644 sha256/openssl.h\n\ndiff --git a/Makefile b/Makefile\nindex fb541dedc9..a499c5d7f2 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3216,6 +3216,9 @@ $(SP_OBJ): %.sp: %.c %.o\n sparse: $(SP_OBJ)\n \n EXCEPT_HDRS := $(GENERATED_H) unicode-width.h compat/% xdiff/%\n+ifndef OPENSSL_SHA256\n+\tEXCEPT_HDRS += sha256/openssl.h\n+endif\n ifndef NETTLE_SHA256\n \tEXCEPT_HDRS += sha256/nettle.h\n endif\ndiff --git a/hash-ll.h b/hash-ll.h\nindex 8d7973769f..087b421bd5 100644\n--- a/hash-ll.h\n+++ b/hash-ll.h\n@@ -17,7 +17,11 @@\n #define SHA256_NEEDS_CLONE_HELPER\n #include \"sha256/gcrypt.h\"\n #elif defined(SHA256_OPENSSL)\n-#include \n+# include \n+# if defined(OPENSSL_API_LEVEL) && OPENSSL_API_LEVEL >= 3\n+# define SHA256_NEEDS_CLONE_HELPER\n+# include \"sha256/openssl.h\"\n+# endif\n #else\n #include \"sha256/block/sha256.h\"\n #endif\ndiff --git a/sha256/openssl.h b/sha256/openssl.h\nnew file mode 100644\nindex 0000000000..c1083d9491\n--- /dev/null\n+++ b/sha256/openssl.h\n@@ -0,0 +1,49 @@\n+/* wrappers for the EVP API of OpenSSL 3+ */\n+#ifndef SHA256_OPENSSL_H\n+#define SHA256_OPENSSL_H\n+#include \n+\n+struct openssl_SHA256_CTX {\n+\tEVP_MD_CTX *ectx;\n+};\n+\n+typedef struct openssl_SHA256_CTX openssl_SHA256_CTX;\n+\n+static inline void openssl_SHA256_Init(struct openssl_SHA256_CTX *ctx)\n+{\n+\tconst EVP_MD *type = EVP_sha256();\n+\n+\tctx->ectx = EVP_MD_CTX_new();\n+\tif (!ctx->ectx)\n+\t\tdie(\"EVP_MD_CTX_new: out of memory\");\n+\n+\tEVP_DigestInit_ex(ctx->ectx, type, NULL);\n+}\n+\n+static inline void openssl_SHA256_Update(struct openssl_SHA256_CTX *ctx,\n+\t\t\t\t\tconst void *data,\n+\t\t\t\t\tsize_t len)\n+{\n+\tEVP_DigestUpdate(ctx->ectx, data, len);\n+}\n+\n+static inline void openssl_SHA256_Final(unsigned char *digest,\n+\t\t\t\t struct openssl_SHA256_CTX *ctx)\n+{\n+\tEVP_DigestFinal_ex(ctx->ectx, digest, NULL);\n+\tEVP_MD_CTX_free(ctx->ectx);\n+}\n+\n+static inline void openssl_SHA256_Clone(struct openssl_SHA256_CTX *dst,\n+\t\t\t\t\tconst struct openssl_SHA256_CTX *src)\n+{\n+\tEVP_MD_CTX_copy_ex(dst->ectx, src->ectx);\n+}\n+\n+#define platform_SHA256_CTX openssl_SHA256_CTX\n+#define platform_SHA256_Init openssl_SHA256_Init\n+#define platform_SHA256_Clone openssl_SHA256_Clone\n+#define platform_SHA256_Update openssl_SHA256_Update\n+#define platform_SHA256_Final openssl_SHA256_Final\n+\n+#endif /* SHA256_OPENSSL_H */\n"},{"id":"480035","messageId":"20230801025454.1137802-3-e@80x24.org","threadId":"60048","inReplyTo":"20230801025454.1137802-1-e@80x24.org","subject":"[PATCH 2/2] avoid SHA-1 functions deprecated in OpenSSL 3+","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2023-08-01T02:54:54Z","receivedAt":"2023-08-01T02:55:11Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"OpenSSL 3+ deprecates the SHA1_Init, SHA1_Update, and SHA1_Final\nfunctions, leading to errors when building with `DEVELOPER=1'.\n\nUse the newer EVP_* API with OpenSSL 3+ (only) despite being more\nerror-prone and less efficient due to heap allocations.\n\nSigned-off-by: Eric Wong \n---\n Makefile | 3 +++\n hash-ll.h | 12 +++++++++++-\n sha1/openssl.h | 49 +++++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 63 insertions(+), 1 deletion(-)\n create mode 100644 sha1/openssl.h\n\ndiff --git a/Makefile b/Makefile\nindex a499c5d7f2..ace3e5a506 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3216,6 +3216,9 @@ $(SP_OBJ): %.sp: %.c %.o\n sparse: $(SP_OBJ)\n \n EXCEPT_HDRS := $(GENERATED_H) unicode-width.h compat/% xdiff/%\n+ifndef OPENSSL_SHA1\n+\tEXCEPT_HDRS += sha1/openssl.h\n+endif\n ifndef OPENSSL_SHA256\n \tEXCEPT_HDRS += sha256/openssl.h\n endif\ndiff --git a/hash-ll.h b/hash-ll.h\nindex 087b421bd5..10d84cc208 100644\n--- a/hash-ll.h\n+++ b/hash-ll.h\n@@ -4,7 +4,11 @@\n #if defined(SHA1_APPLE)\n #include \n #elif defined(SHA1_OPENSSL)\n-#include \n+# include \n+# if defined(OPENSSL_API_LEVEL) && OPENSSL_API_LEVEL >= 3\n+# define SHA1_NEEDS_CLONE_HELPER\n+# include \"sha1/openssl.h\"\n+# endif\n #elif defined(SHA1_DC)\n #include \"sha1dc_git.h\"\n #else /* SHA1_BLK */\n@@ -45,6 +49,10 @@\n #define git_SHA1_Update\t\tplatform_SHA1_Update\n #define git_SHA1_Final\t\tplatform_SHA1_Final\n \n+#ifdef platform_SHA1_Clone\n+#define git_SHA1_Clone\tplatform_SHA1_Clone\n+#endif\n+\n #ifndef platform_SHA256_CTX\n #define platform_SHA256_CTX\tSHA256_CTX\n #define platform_SHA256_Init\tSHA256_Init\n@@ -67,10 +75,12 @@\n #define git_SHA1_Update\t\tgit_SHA1_Update_Chunked\n #endif\n \n+#ifndef SHA1_NEEDS_CLONE_HELPER\n static inline void git_SHA1_Clone(git_SHA_CTX *dst, const git_SHA_CTX *src)\n {\n \tmemcpy(dst, src, sizeof(*dst));\n }\n+#endif\n \n #ifndef SHA256_NEEDS_CLONE_HELPER\n static inline void git_SHA256_Clone(git_SHA256_CTX *dst, const git_SHA256_CTX *src)\ndiff --git a/sha1/openssl.h b/sha1/openssl.h\nnew file mode 100644\nindex 0000000000..006c1f4ba5\n--- /dev/null\n+++ b/sha1/openssl.h\n@@ -0,0 +1,49 @@\n+/* wrappers for the EVP API of OpenSSL 3+ */\n+#ifndef SHA1_OPENSSL_H\n+#define SHA1_OPENSSL_H\n+#include \n+\n+struct openssl_SHA1_CTX {\n+\tEVP_MD_CTX *ectx;\n+};\n+\n+typedef struct openssl_SHA1_CTX openssl_SHA1_CTX;\n+\n+static inline void openssl_SHA1_Init(struct openssl_SHA1_CTX *ctx)\n+{\n+\tconst EVP_MD *type = EVP_sha1();\n+\n+\tctx->ectx = EVP_MD_CTX_new();\n+\tif (!ctx->ectx)\n+\t\tdie(\"EVP_MD_CTX_new: out of memory\");\n+\n+\tEVP_DigestInit_ex(ctx->ectx, type, NULL);\n+}\n+\n+static inline void openssl_SHA1_Update(struct openssl_SHA1_CTX *ctx,\n+\t\t\t\t\tconst void *data,\n+\t\t\t\t\tsize_t len)\n+{\n+\tEVP_DigestUpdate(ctx->ectx, data, len);\n+}\n+\n+static inline void openssl_SHA1_Final(unsigned char *digest,\n+\t\t\t\t struct openssl_SHA1_CTX *ctx)\n+{\n+\tEVP_DigestFinal_ex(ctx->ectx, digest, NULL);\n+\tEVP_MD_CTX_free(ctx->ectx);\n+}\n+\n+static inline void openssl_SHA1_Clone(struct openssl_SHA1_CTX *dst,\n+\t\t\t\t\tconst struct openssl_SHA1_CTX *src)\n+{\n+\tEVP_MD_CTX_copy_ex(dst->ectx, src->ectx);\n+}\n+\n+#define platform_SHA_CTX openssl_SHA1_CTX\n+#define platform_SHA1_Init openssl_SHA1_Init\n+#define platform_SHA1_Clone openssl_SHA1_Clone\n+#define platform_SHA1_Update openssl_SHA1_Update\n+#define platform_SHA1_Final openssl_SHA1_Final\n+\n+#endif /* SHA1_OPENSSL_H */\n"},{"id":"480050","messageId":"xmqqsf92eomq.fsf@gitster.g","threadId":"60048","inReplyTo":"20230801025454.1137802-3-e@80x24.org","subject":"Re: [PATCH 2/2] avoid SHA-1 functions deprecated in OpenSSL 3+","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-01T16:03:25Z","receivedAt":"2023-08-01T16:03:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> diff --git a/hash-ll.h b/hash-ll.h\n> index 087b421bd5..10d84cc208 100644\n> --- a/hash-ll.h\n> +++ b/hash-ll.h\n> @@ -45,6 +49,10 @@\n> #define git_SHA1_Update\t\tplatform_SHA1_Update\n> #define git_SHA1_Final\t\tplatform_SHA1_Final\n> \n> +#ifdef platform_SHA1_Clone\n> +#define git_SHA1_Clone\tplatform_SHA1_Clone\n> +#endif\n> +\n> ...\n> +#ifndef SHA1_NEEDS_CLONE_HELPER\n> static inline void git_SHA1_Clone(git_SHA_CTX *dst, const git_SHA_CTX *src)\n> {\n> \tmemcpy(dst, src, sizeof(*dst));\n> }\n> +#endif\n\nThis smelled a bit strange in that all the other platform_* stuff is\n\"if a platform sha-1 header implements platform_SHA1_*, we will use\nit to define git_SHA1_* (which is the symbol we use in the code)\"\nplus its inverse \"if there is no specific platform_SHA1_*, we assume\nOpenSSL compatible ones and use them as platform_SHA1_* (which in\nturn will be used as git_SHA1-*)\".\n\nAnd that is why \"#ifndef platform_SHA1_CTX\" block gave us default\nvalues for them. And from that point of view, the first hunk\n(i.e. \"if SHA1_CLONE is defined for the platform, we use it\") is\nentirely sensible.\n\nBut I did not get why we guard the other hunk with a different CPP\nmacro. If we have platform_SHA1_Clone already defined, and then\nNEEDS_CLONE_HELPER not defined, we end up creating an static inline\nplatform_SHA1_CLONE here, and I was not sure if that is what we\nwanted to do.\n\nThe answer to the above puzzle (at least it was a puzzle to me) is\nthat the new header \"sha1/openssl.h\" added by this series does have\nplatform_SHA1_Clone defined, and the code that includes it define\nNEEDS_CLONE_HELPER to avoid this \"static inline\", so the CPP macro\nSHA1_NEEDS_CLONE_HELPER means \"we need more than just a straight\nbitwise copy to clone the SHA context, which is provided elsewhere\nin the form of platform_SHA1_Clone\".\n\nSo everything evens out. If we are with newer OpenSSL, we will\ninclude sha1/openssl.h and get both platform_SHA1_Clone and\nSHA1_NEEDS_CLONE_HELPER defined. If we are with older OpenSSL or\nnon-OpenSSL, we do not get platform_SHA1_Clone (because the \"#ifndef\nplatform_SHA1_CTX\" block does not have a fallback default defined)\nand we do not get SHA1_NEEDS_CLONE_HELPER either. We either use the\nmemcpy() fallback only when we are not working with newer OpenSSL or\nwhatever defines its own platform_SHA1_Clone. So the patch smelled\na bit strange, but there isn't anything incorrect per-se.\n\nBut then is this making folks unnecessary work when they add\nnon-OpenSSL support that needs more than just memcpy() to clone the\ncontext? What breaks if we turn these two hunks into\n\n\t#ifdef platform_SHA1_Clone\n\t#define git_SHA1_Clone platform_SHA1_Clone\n\t#else\n\tstatic inline void git_SHA1_Clone(git_SHA_CTX *dst, git_SHA_CTX *src)\n\t{\n\t\tmemcpy(dst, src, sizeof(*dst));\n\t}\n\t#endif\n\nand drop the requirement that they must define SHA1_NEEDS_CLONE_HELPER\nif they want to define their own platform_SHA1_Clone()?\n\nThanks. Everything else in the patch made sense (even though I am\nnot familiar with the EVP API) to me.\n\n"},{"id":"480080","messageId":"20230801195325.M746978@dcvr","threadId":"60048","inReplyTo":"xmqqsf92eomq.fsf@gitster.g","subject":"Re: [PATCH 2/2] avoid SHA-1 functions deprecated in OpenSSL 3+","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2023-08-01T19:53:25Z","receivedAt":"2023-08-01T19:53:28Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano wrote:\n> Eric Wong writes:\n> \n> > diff --git a/hash-ll.h b/hash-ll.h\n> > index 087b421bd5..10d84cc208 100644\n> > --- a/hash-ll.h\n> > +++ b/hash-ll.h\n> > @@ -45,6 +49,10 @@\n> > #define git_SHA1_Update\t\tplatform_SHA1_Update\n> > #define git_SHA1_Final\t\tplatform_SHA1_Final\n> > \n> > +#ifdef platform_SHA1_Clone\n> > +#define git_SHA1_Clone\tplatform_SHA1_Clone\n> > +#endif\n> > +\n> > ...\n> > +#ifndef SHA1_NEEDS_CLONE_HELPER\n> > static inline void git_SHA1_Clone(git_SHA_CTX *dst, const git_SHA_CTX *src)\n> > {\n> > \tmemcpy(dst, src, sizeof(*dst));\n> > }\n> > +#endif\n> \n> This smelled a bit strange in that all the other platform_* stuff is\n> \"if a platform sha-1 header implements platform_SHA1_*, we will use\n> it to define git_SHA1_* (which is the symbol we use in the code)\"\n> plus its inverse \"if there is no specific platform_SHA1_*, we assume\n> OpenSSL compatible ones and use them as platform_SHA1_* (which in\n> turn will be used as git_SHA1-*)\".\n> \n> And that is why \"#ifndef platform_SHA1_CTX\" block gave us default\n> values for them. And from that point of view, the first hunk\n> (i.e. \"if SHA1_CLONE is defined for the platform, we use it\") is\n> entirely sensible.\n> \n> But I did not get why we guard the other hunk with a different CPP\n> macro. If we have platform_SHA1_Clone already defined, and then\n> NEEDS_CLONE_HELPER not defined, we end up creating an static inline\n> platform_SHA1_CLONE here, and I was not sure if that is what we\n> wanted to do.\n> \n> The answer to the above puzzle (at least it was a puzzle to me) is\n> that the new header \"sha1/openssl.h\" added by this series does have\n> platform_SHA1_Clone defined, and the code that includes it define\n> NEEDS_CLONE_HELPER to avoid this \"static inline\", so the CPP macro\n> SHA1_NEEDS_CLONE_HELPER means \"we need more than just a straight\n> bitwise copy to clone the SHA context, which is provided elsewhere\n> in the form of platform_SHA1_Clone\".\n> \n> So everything evens out. If we are with newer OpenSSL, we will\n> include sha1/openssl.h and get both platform_SHA1_Clone and\n> SHA1_NEEDS_CLONE_HELPER defined. If we are with older OpenSSL or\n> non-OpenSSL, we do not get platform_SHA1_Clone (because the \"#ifndef\n> platform_SHA1_CTX\" block does not have a fallback default defined)\n> and we do not get SHA1_NEEDS_CLONE_HELPER either. We either use the\n> memcpy() fallback only when we are not working with newer OpenSSL or\n> whatever defines its own platform_SHA1_Clone. So the patch smelled\n> a bit strange, but there isn't anything incorrect per-se.\n> \n> But then is this making folks unnecessary work when they add\n> non-OpenSSL support that needs more than just memcpy() to clone the\n> context? What breaks if we turn these two hunks into\n> \n> \t#ifdef platform_SHA1_Clone\n> \t#define git_SHA1_Clone platform_SHA1_Clone\n> \t#else\n> \tstatic inline void git_SHA1_Clone(git_SHA_CTX *dst, git_SHA_CTX *src)\n> \t{\n> \t\tmemcpy(dst, src, sizeof(*dst));\n> \t}\n> \t#endif\n> \n> and drop the requirement that they must define SHA1_NEEDS_CLONE_HELPER\n> if they want to define their own platform_SHA1_Clone()?\n\nI just copied the existing SHA256 stuff and mostly did a\ns/SHA256/SHA1/ in patch 2/2. I'm not sure why\nSHA256_NEEDS_CLONE_HELPER was needed, either, but I decided\nto keep the SHA1 and SHA256 code as similar as possible for\nconsistency.\n\nWe could probably drop both *_NEEDS_CLONE_HELPER macros,\nbut that's a separate patch.\n"},{"id":"480081","messageId":"xmqq3512bjps.fsf@gitster.g","threadId":"60048","inReplyTo":"20230801195325.M746978@dcvr","subject":"Re: [PATCH 2/2] avoid SHA-1 functions deprecated in OpenSSL 3+","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-01T20:17:51Z","receivedAt":"2023-08-01T20:18:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> I just copied the existing SHA256 stuff and mostly did a\n> s/SHA256/SHA1/ in patch 2/2. I'm not sure why\n> SHA256_NEEDS_CLONE_HELPER was needed, either, but I decided\n> to keep the SHA1 and SHA256 code as similar as possible for\n> consistency.\n>\n> We could probably drop both *_NEEDS_CLONE_HELPER macros,\n> but that's a separate patch.\n\nFair enough. Thanks.\n"}]}