v7-0001-Switch-cryptohash_openssl.c-to-use-EVP.patch

From d2c9f2a9c9a36d80a014d588334e21a709a1c27d Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Wed, 2 Dec 2020 11:51:43 +0900
Subject: [PATCH v7] Switch cryptohash_openssl.c to use EVP

Postgres is two decades late for this switch.
---
 src/include/utils/resowner_private.h  |   7 ++
 src/backend/replication/basebackup.c  |   8 +-
 src/backend/utils/resowner/resowner.c |  62 ++++++++++++
 src/common/cryptohash_openssl.c       | 132 ++++++++++++++++----------
 src/tools/pgindent/typedefs.list      |   1 +
 5 files changed, 158 insertions(+), 52 deletions(-)

diff --git a/src/include/utils/resowner_private.h b/src/include/utils/resowner_private.h
index a781a7a2aa..c373788bc1 100644
--- a/src/include/utils/resowner_private.h
+++ b/src/include/utils/resowner_private.h
@@ -95,4 +95,11 @@ extern void ResourceOwnerRememberJIT(ResourceOwner owner,
 extern void ResourceOwnerForgetJIT(ResourceOwner owner,
 								   Datum handle);
 
+/* support for cryptohash context management */
+extern void ResourceOwnerEnlargeCryptoHash(ResourceOwner owner);
+extern void ResourceOwnerRememberCryptoHash(ResourceOwner owner,
+											Datum handle);
+extern void ResourceOwnerForgetCryptoHash(ResourceOwner owner,
+										  Datum handle);
+
 #endif							/* RESOWNER_PRIVATE_H */
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 22be7ca9d5..79b458c185 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -729,11 +729,17 @@ perform_base_backup(basebackup_options *opt)
 				 errmsg("checksum verification failure during base backup")));
 	}
 
+	/*
+	 * Make sure to free the manifest before the resource owners as
+	 * manifests use cryptohash contexts that may depend on resource
+	 * owners (like OpenSSL).
+	 */
+	FreeBackupManifest(&manifest);
+
 	/* clean up the resource owner we created */
 	WalSndResourceCleanup(true);
 
 	pgstat_progress_end_command();
-	FreeBackupManifest(&manifest);
 }
 
 /*
diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c
index 8bc2c4e9ea..546ad8d1c5 100644
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -20,6 +20,7 @@
  */
 #include "postgres.h"
 
+#include "common/cryptohash.h"
 #include "common/hashfn.h"
 #include "jit/jit.h"
 #include "storage/bufmgr.h"
@@ -128,6 +129,7 @@ typedef struct ResourceOwnerData
 	ResourceArray filearr;		/* open temporary files */
 	ResourceArray dsmarr;		/* dynamic shmem segments */
 	ResourceArray jitarr;		/* JIT contexts */
+	ResourceArray cryptohasharr;	/* cryptohash contexts */
 
 	/* We can remember up to MAX_RESOWNER_LOCKS references to local locks. */
 	int			nlocks;			/* number of owned locks */
@@ -175,6 +177,7 @@ static void PrintTupleDescLeakWarning(TupleDesc tupdesc);
 static void PrintSnapshotLeakWarning(Snapshot snapshot);
 static void PrintFileLeakWarning(File file);
 static void PrintDSMLeakWarning(dsm_segment *seg);
+static void PrintCryptoHashLeakWarning(Datum handle);
 
 
 /*****************************************************************************
@@ -444,6 +447,7 @@ ResourceOwnerCreate(ResourceOwner parent, const char *name)
 	ResourceArrayInit(&(owner->filearr), FileGetDatum(-1));
 	ResourceArrayInit(&(owner->dsmarr), PointerGetDatum(NULL));
 	ResourceArrayInit(&(owner->jitarr), PointerGetDatum(NULL));
+	ResourceArrayInit(&(owner->cryptohasharr), PointerGetDatum(NULL));
 
 	return owner;
 }
@@ -553,6 +557,17 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
 
 			jit_release_context(context);
 		}
+
+		/* Ditto for cryptohash contexts */
+		while (ResourceArrayGetAny(&(owner->cryptohasharr), &foundres))
+		{
+			pg_cryptohash_ctx *context =
+			(pg_cryptohash_ctx *) PointerGetDatum(foundres);
+
+			if (isCommit)
+				PrintCryptoHashLeakWarning(foundres);
+			pg_cryptohash_free(context);
+		}
 	}
 	else if (phase == RESOURCE_RELEASE_LOCKS)
 	{
@@ -725,6 +740,7 @@ ResourceOwnerDelete(ResourceOwner owner)
 	Assert(owner->filearr.nitems == 0);
 	Assert(owner->dsmarr.nitems == 0);
 	Assert(owner->jitarr.nitems == 0);
+	Assert(owner->cryptohasharr.nitems == 0);
 	Assert(owner->nlocks == 0 || owner->nlocks == MAX_RESOWNER_LOCKS + 1);
 
 	/*
@@ -752,6 +768,7 @@ ResourceOwnerDelete(ResourceOwner owner)
 	ResourceArrayFree(&(owner->filearr));
 	ResourceArrayFree(&(owner->dsmarr));
 	ResourceArrayFree(&(owner->jitarr));
+	ResourceArrayFree(&(owner->cryptohasharr));
 
 	pfree(owner);
 }
@@ -1370,3 +1387,48 @@ ResourceOwnerForgetJIT(ResourceOwner owner, Datum handle)
 		elog(ERROR, "JIT context %p is not owned by resource owner %s",
 			 DatumGetPointer(handle), owner->name);
 }
+
+/*
+ * Make sure there is room for at least one more entry in a ResourceOwner's
+ * cryptohash context reference array.
+ *
+ * This is separate from actually inserting an entry because if we run out of
+ * memory, it's critical to do so *before* acquiring the resource.
+ */
+void
+ResourceOwnerEnlargeCryptoHash(ResourceOwner owner)
+{
+	ResourceArrayEnlarge(&(owner->cryptohasharr));
+}
+
+/*
+ * Remember that a cryptohash context is owned by a ResourceOwner
+ *
+ * Caller must have previously done ResourceOwnerEnlargeCryptoHash()
+ */
+void
+ResourceOwnerRememberCryptoHash(ResourceOwner owner, Datum handle)
+{
+	ResourceArrayAdd(&(owner->cryptohasharr), handle);
+}
+
+/*
+ * Forget that a cryptohash context is owned by a ResourceOwner
+ */
+void
+ResourceOwnerForgetCryptoHash(ResourceOwner owner, Datum handle)
+{
+	if (!ResourceArrayRemove(&(owner->cryptohasharr), handle))
+		elog(ERROR, "cryptohash context %p is not owned by resource owner %s",
+			 DatumGetPointer(handle), owner->name);
+}
+
+/*
+ * Debugging subroutine
+ */
+static void
+PrintCryptoHashLeakWarning(Datum handle)
+{
+	elog(WARNING, "cryptohash context reference leak: context %p still referenced",
+		 DatumGetPointer(handle));
+}
diff --git a/src/common/cryptohash_openssl.c b/src/common/cryptohash_openssl.c
index 8e2c69b48b..d0f58b81be 100644
--- a/src/common/cryptohash_openssl.c
+++ b/src/common/cryptohash_openssl.c
@@ -21,9 +21,14 @@
 #include "postgres_fe.h"
 #endif
 
-#include <openssl/sha.h>
+#include <openssl/evp.h>
 
 #include "common/cryptohash.h"
+#ifndef FRONTEND
+#include "utils/memutils.h"
+#include "utils/resowner.h"
+#include "utils/resowner_private.h"
+#endif
 
 /*
  * In backend, use palloc/pfree to ease the error handling.  In frontend,
@@ -37,6 +42,21 @@
 #define FREE(ptr) free(ptr)
 #endif
 
+/*
+ * Internal structure for pg_cryptohash_ctx->data.
+ *
+ * This tracks the resource owner associated to each EVP context data
+ * for the backend.
+ */
+typedef struct pg_cryptohash_state
+{
+	EVP_MD_CTX *evpctx;
+
+#ifndef FRONTEND
+	ResourceOwner resowner;
+#endif
+} pg_cryptohash_state;
+
 /*
  * pg_cryptohash_create
  *
@@ -47,32 +67,51 @@ pg_cryptohash_ctx *
 pg_cryptohash_create(pg_cryptohash_type type)
 {
 	pg_cryptohash_ctx *ctx;
+	pg_cryptohash_state *state;
 
 	ctx = ALLOC(sizeof(pg_cryptohash_ctx));
 	if (ctx == NULL)
 		return NULL;
 
-	ctx->type = type;
-
-	switch (type)
-	{
-		case PG_SHA224:
-		case PG_SHA256:
-			ctx->data = ALLOC(sizeof(SHA256_CTX));
-			break;
-		case PG_SHA384:
-		case PG_SHA512:
-			ctx->data = ALLOC(sizeof(SHA512_CTX));
-			break;
-	}
-
-	if (ctx->data == NULL)
+	state = ALLOC(sizeof(pg_cryptohash_state));
+	if (state == NULL)
 	{
 		explicit_bzero(ctx, sizeof(pg_cryptohash_ctx));
 		FREE(ctx);
 		return NULL;
 	}
 
+	ctx->data = state;
+	ctx->type = type;
+
+#ifndef FRONTEND
+	ResourceOwnerEnlargeCryptoHash(CurrentResourceOwner);
+#endif
+
+	/*
+	 * Initialization takes care of assigning the correct type for OpenSSL.
+	 */
+	state->evpctx = EVP_MD_CTX_create();
+
+	if (state->evpctx == NULL)
+	{
+		explicit_bzero(state, sizeof(pg_cryptohash_state));
+		explicit_bzero(ctx, sizeof(pg_cryptohash_ctx));
+#ifndef FRONTEND
+		elog(ERROR, "out of memory");
+#else
+		FREE(state);
+		FREE(ctx);
+		return NULL;
+#endif
+	}
+
+#ifndef FRONTEND
+	state->resowner = CurrentResourceOwner;
+	ResourceOwnerRememberCryptoHash(CurrentResourceOwner,
+									PointerGetDatum(ctx));
+#endif
+
 	return ctx;
 }
 
@@ -85,23 +124,26 @@ int
 pg_cryptohash_init(pg_cryptohash_ctx *ctx)
 {
 	int			status = 0;
+	pg_cryptohash_state *state;
 
 	if (ctx == NULL)
 		return 0;
 
+	state = (pg_cryptohash_state *) ctx->data;
+
 	switch (ctx->type)
 	{
 		case PG_SHA224:
-			status = SHA224_Init((SHA256_CTX *) ctx->data);
+			status = EVP_DigestInit_ex(state->evpctx, EVP_sha224(), NULL);
 			break;
 		case PG_SHA256:
-			status = SHA256_Init((SHA256_CTX *) ctx->data);
+			status = EVP_DigestInit_ex(state->evpctx, EVP_sha256(), NULL);
 			break;
 		case PG_SHA384:
-			status = SHA384_Init((SHA512_CTX *) ctx->data);
+			status = EVP_DigestInit_ex(state->evpctx, EVP_sha384(), NULL);
 			break;
 		case PG_SHA512:
-			status = SHA512_Init((SHA512_CTX *) ctx->data);
+			status = EVP_DigestInit_ex(state->evpctx, EVP_sha512(), NULL);
 			break;
 	}
 
@@ -120,25 +162,13 @@ int
 pg_cryptohash_update(pg_cryptohash_ctx *ctx, const uint8 *data, size_t len)
 {
 	int			status;
+	pg_cryptohash_state *state;
 
 	if (ctx == NULL)
 		return 0;
 
-	switch (ctx->type)
-	{
-		case PG_SHA224:
-			status = SHA224_Update((SHA256_CTX *) ctx->data, data, len);
-			break;
-		case PG_SHA256:
-			status = SHA256_Update((SHA256_CTX *) ctx->data, data, len);
-			break;
-		case PG_SHA384:
-			status = SHA384_Update((SHA512_CTX *) ctx->data, data, len);
-			break;
-		case PG_SHA512:
-			status = SHA512_Update((SHA512_CTX *) ctx->data, data, len);
-			break;
-	}
+	state = (pg_cryptohash_state *) ctx->data;
+	status = EVP_DigestUpdate(state->evpctx, data, len);
 
 	/* OpenSSL internals return 1 on success, 0 on failure */
 	if (status <= 0)
@@ -155,25 +185,13 @@ int
 pg_cryptohash_final(pg_cryptohash_ctx *ctx, uint8 *dest)
 {
 	int			status;
+	pg_cryptohash_state *state;
 
 	if (ctx == NULL)
 		return 0;
 
-	switch (ctx->type)
-	{
-		case PG_SHA224:
-			status = SHA224_Final(dest, (SHA256_CTX *) ctx->data);
-			break;
-		case PG_SHA256:
-			status = SHA256_Final(dest, (SHA256_CTX *) ctx->data);
-			break;
-		case PG_SHA384:
-			status = SHA384_Final(dest, (SHA512_CTX *) ctx->data);
-			break;
-		case PG_SHA512:
-			status = SHA512_Final(dest, (SHA512_CTX *) ctx->data);
-			break;
-	}
+	state = (pg_cryptohash_state *) ctx->data;
+	status = EVP_DigestFinal_ex(state->evpctx, dest, 0);
 
 	/* OpenSSL internals return 1 on success, 0 on failure */
 	if (status <= 0)
@@ -189,9 +207,21 @@ pg_cryptohash_final(pg_cryptohash_ctx *ctx, uint8 *dest)
 void
 pg_cryptohash_free(pg_cryptohash_ctx *ctx)
 {
+	pg_cryptohash_state *state;
+
 	if (ctx == NULL)
 		return;
-	FREE(ctx->data);
+
+	state = (pg_cryptohash_state *) ctx->data;
+	EVP_MD_CTX_destroy(state->evpctx);
+
+#ifndef FRONTEND
+	ResourceOwnerForgetCryptoHash(state->resowner,
+								  PointerGetDatum(ctx));
+#endif
+
+	explicit_bzero(state, sizeof(pg_cryptohash_state));
 	explicit_bzero(ctx, sizeof(pg_cryptohash_ctx));
+	FREE(state);
 	FREE(ctx);
 }
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 04464c2e76..cf63acbf6f 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3180,6 +3180,7 @@ pg_conv_map
 pg_crc32
 pg_crc32c
 pg_cryptohash_ctx
+pg_cryptohash_state
 pg_cryptohash_type
 pg_ctype_cache
 pg_enc
-- 
2.29.2