Change SHA2 implementation based on OpenSSL to use EVP digest routines
authorMichael Paquier <[email protected]>
Fri, 4 Dec 2020 01:49:23 +0000 (10:49 +0900)
committerMichael Paquier <[email protected]>
Fri, 4 Dec 2020 01:49:23 +0000 (10:49 +0900)
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[email protected]
Discussion: https://postgr.es/m/20180911030250[email protected]

src/backend/replication/basebackup.c
src/backend/utils/resowner/resowner.c
src/common/cryptohash_openssl.c
src/include/utils/resowner_private.h
src/tools/pgindent/typedefs.list

index 22be7ca9d5fceaec6673076d0a6c4807a4f9458b..1d8d1742a73a0b4e49b420b445165bdb2de84866 100644 (file)
@@ -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);
 }
 
 /*
index 8bc2c4e9ea310f97b07de7ce74001f67e4151386..546ad8d1c5fad0b3dd20f514991710f5047701b0 100644 (file)
@@ -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));
+}
index 33f17cac33da16f7db21735872890d2aecd9c6ad..9d9f74b086cb04df363b9fb384b30fc9e708d7ec 100644 (file)
 #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,
 #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,53 @@ 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)
+   state = ALLOC(sizeof(pg_cryptohash_state));
+   if (state == NULL)
    {
-       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;
+       explicit_bzero(ctx, sizeof(pg_cryptohash_ctx));
+       FREE(ctx);
+       return NULL;
    }
 
-   if (ctx->data == 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
+       ereport(ERROR,
+               (errcode(ERRCODE_OUT_OF_MEMORY),
+                errmsg("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 +126,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 +164,13 @@ int
 pg_cryptohash_update(pg_cryptohash_ctx *ctx, const uint8 *data, size_t len)
 {
    int         status = 0;
+   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 +187,13 @@ int
 pg_cryptohash_final(pg_cryptohash_ctx *ctx, uint8 *dest)
 {
    int         status = 0;
+   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 +209,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);
 }
index a781a7a2aa664d2ef8de2f9a5161e79c820348d8..c373788bc13ebb0c9259853188bdbc601b3c2913 100644 (file)
@@ -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 */
index 04464c2e768ccc39b38f76f85b74259f1d0a46de..cf63acbf6f349dd831276864753ec95f04e83acf 100644 (file)
@@ -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