Change SHA2 implementation based on OpenSSL to use EVP digest routines

The use of low-level hash routines is not recommended by upstream
OpenSSL since 2000, and pgcrypto already switched to EVP as of 5ff4a67.
This takes advantage of the refactoring done in 87ae969 that has
introduced the allocation and free routines for cryptographic hashes.

Since 1.1.0, OpenSSL does not publish the contents of the cryptohash
contexts, forcing any consumers to rely on OpenSSL for all allocations.
Hence, the resource owner callback mechanism gains a new set of routines
to track and free cryptohash contexts when using OpenSSL, preventing any
risks of leaks in the backend.  Nothing is needed in the frontend thanks
to the refactoring of 87ae969, and the resowner knowledge is isolated
into cryptohash_openssl.c.

Note that this also fixes a failure with SCRAM authentication when using
FIPS in OpenSSL, but as there have been few complaints about this
problem and as this causes an ABI breakage, no backpatch is done.

Author: Michael Paquier
Reviewed-by: Daniel Gustafsson, Heikki Linnakangas
Discussion: https://postgr.es/m/20200924025314.GE7405@paquier.xyz
Discussion: https://postgr.es/m/20180911030250.GA27115@paquier.xyz

1 files changed, 62 insertions, 0 deletions

diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c
index 8bc2c4e9ea3..546ad8d1c5f 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));
+}