Skip to content

Commit 4f48a6f

Browse files
michaelpqmichaelpq
committed
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 parent 3f8971d commit 4f48a6f

File tree

5 files changed

+157
-49
lines changed

5 files changed

+157
-49
lines changed

‎src/backend/replication/basebackup.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -729,11 +729,17 @@ perform_base_backup(basebackup_options *opt)
729729
errmsg("checksum verification failure during base backup")));
730730
}
731731

732+
/*
733+
* Make sure to free the manifest before the resource owners as manifests
734+
* use cryptohash contexts that may depend on resource owners (like
735+
* OpenSSL).
736+
*/
737+
FreeBackupManifest(&manifest);
738+
732739
/* clean up the resource owner we created */
733740
WalSndResourceCleanup(true);
734741

735742
pgstat_progress_end_command();
736-
FreeBackupManifest(&manifest);
737743
}
738744

739745
/*

‎src/backend/utils/resowner/resowner.c

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
*/
2121
#include "postgres.h"
2222

23+
#include "common/cryptohash.h"
2324
#include "common/hashfn.h"
2425
#include "jit/jit.h"
2526
#include "storage/bufmgr.h"
@@ -128,6 +129,7 @@ typedef struct ResourceOwnerData
128129
ResourceArray filearr; /* open temporary files */
129130
ResourceArray dsmarr; /* dynamic shmem segments */
130131
ResourceArray jitarr; /* JIT contexts */
132+
ResourceArray cryptohasharr; /* cryptohash contexts */
131133

132134
/* We can remember up to MAX_RESOWNER_LOCKS references to local locks. */
133135
int nlocks; /* number of owned locks */
@@ -175,6 +177,7 @@ static void PrintTupleDescLeakWarning(TupleDesc tupdesc);
175177
static void PrintSnapshotLeakWarning(Snapshot snapshot);
176178
static void PrintFileLeakWarning(File file);
177179
static void PrintDSMLeakWarning(dsm_segment *seg);
180+
static void PrintCryptoHashLeakWarning(Datum handle);
178181

179182

180183
/*****************************************************************************
@@ -444,6 +447,7 @@ ResourceOwnerCreate(ResourceOwner parent, const char *name)
444447
ResourceArrayInit(&(owner->filearr), FileGetDatum(-1));
445448
ResourceArrayInit(&(owner->dsmarr), PointerGetDatum(NULL));
446449
ResourceArrayInit(&(owner->jitarr), PointerGetDatum(NULL));
450+
ResourceArrayInit(&(owner->cryptohasharr), PointerGetDatum(NULL));
447451

448452
return owner;
449453
}
@@ -553,6 +557,17 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
553557

554558
jit_release_context(context);
555559
}
560+
561+
/* Ditto for cryptohash contexts */
562+
while (ResourceArrayGetAny(&(owner->cryptohasharr), &foundres))
563+
{
564+
pg_cryptohash_ctx *context =
565+
(pg_cryptohash_ctx *) PointerGetDatum(foundres);
566+
567+
if (isCommit)
568+
PrintCryptoHashLeakWarning(foundres);
569+
pg_cryptohash_free(context);
570+
}
556571
}
557572
else if (phase == RESOURCE_RELEASE_LOCKS)
558573
{
@@ -725,6 +740,7 @@ ResourceOwnerDelete(ResourceOwner owner)
725740
Assert(owner->filearr.nitems == 0);
726741
Assert(owner->dsmarr.nitems == 0);
727742
Assert(owner->jitarr.nitems == 0);
743+
Assert(owner->cryptohasharr.nitems == 0);
728744
Assert(owner->nlocks == 0 || owner->nlocks == MAX_RESOWNER_LOCKS + 1);
729745

730746
/*
@@ -752,6 +768,7 @@ ResourceOwnerDelete(ResourceOwner owner)
752768
ResourceArrayFree(&(owner->filearr));
753769
ResourceArrayFree(&(owner->dsmarr));
754770
ResourceArrayFree(&(owner->jitarr));
771+
ResourceArrayFree(&(owner->cryptohasharr));
755772

756773
pfree(owner);
757774
}
@@ -1370,3 +1387,48 @@ ResourceOwnerForgetJIT(ResourceOwner owner, Datum handle)
13701387
elog(ERROR, "JIT context %p is not owned by resource owner %s",
13711388
DatumGetPointer(handle), owner->name);
13721389
}
1390+
1391+
/*
1392+
* Make sure there is room for at least one more entry in a ResourceOwner's
1393+
* cryptohash context reference array.
1394+
*
1395+
* This is separate from actually inserting an entry because if we run out of
1396+
* memory, it's critical to do so *before* acquiring the resource.
1397+
*/
1398+
void
1399+
ResourceOwnerEnlargeCryptoHash(ResourceOwner owner)
1400+
{
1401+
ResourceArrayEnlarge(&(owner->cryptohasharr));
1402+
}
1403+
1404+
/*
1405+
* Remember that a cryptohash context is owned by a ResourceOwner
1406+
*
1407+
* Caller must have previously done ResourceOwnerEnlargeCryptoHash()
1408+
*/
1409+
void
1410+
ResourceOwnerRememberCryptoHash(ResourceOwner owner, Datum handle)
1411+
{
1412+
ResourceArrayAdd(&(owner->cryptohasharr), handle);
1413+
}
1414+
1415+
/*
1416+
* Forget that a cryptohash context is owned by a ResourceOwner
1417+
*/
1418+
void
1419+
ResourceOwnerForgetCryptoHash(ResourceOwner owner, Datum handle)
1420+
{
1421+
if (!ResourceArrayRemove(&(owner->cryptohasharr), handle))
1422+
elog(ERROR, "cryptohash context %p is not owned by resource owner %s",
1423+
DatumGetPointer(handle), owner->name);
1424+
}
1425+
1426+
/*
1427+
* Debugging subroutine
1428+
*/
1429+
static void
1430+
PrintCryptoHashLeakWarning(Datum handle)
1431+
{
1432+
elog(WARNING, "cryptohash context reference leak: context %p still referenced",
1433+
DatumGetPointer(handle));
1434+
}

‎src/common/cryptohash_openssl.c

Lines changed: 80 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,14 @@
2121
#include "postgres_fe.h"
2222
#endif
2323

24-
#include <openssl/sha.h>
24+
#include <openssl/evp.h>
2525

2626
#include "common/cryptohash.h"
27+
#ifndef FRONTEND
28+
#include "utils/memutils.h"
29+
#include "utils/resowner.h"
30+
#include "utils/resowner_private.h"
31+
#endif
2732

2833
/*
2934
* In backend, use palloc/pfree to ease the error handling. In frontend,
@@ -37,6 +42,21 @@
3742
#define FREE(ptr) free(ptr)
3843
#endif
3944

45+
/*
46+
* Internal structure for pg_cryptohash_ctx->data.
47+
*
48+
* This tracks the resource owner associated to each EVP context data
49+
* for the backend.
50+
*/
51+
typedef struct pg_cryptohash_state
52+
{
53+
EVP_MD_CTX *evpctx;
54+
55+
#ifndef FRONTEND
56+
ResourceOwner resowner;
57+
#endif
58+
} pg_cryptohash_state;
59+
4060
/*
4161
* pg_cryptohash_create
4262
*
@@ -47,32 +67,53 @@ pg_cryptohash_ctx *
4767
pg_cryptohash_create(pg_cryptohash_type type)
4868
{
4969
pg_cryptohash_ctx *ctx;
70+
pg_cryptohash_state *state;
5071

5172
ctx = ALLOC(sizeof(pg_cryptohash_ctx));
5273
if (ctx == NULL)
5374
return NULL;
5475

55-
ctx->type = type;
56-
57-
switch (type)
76+
state = ALLOC(sizeof(pg_cryptohash_state));
77+
if (state == NULL)
5878
{
59-
case PG_SHA224:
60-
case PG_SHA256:
61-
ctx->data = ALLOC(sizeof(SHA256_CTX));
62-
break;
63-
case PG_SHA384:
64-
case PG_SHA512:
65-
ctx->data = ALLOC(sizeof(SHA512_CTX));
66-
break;
79+
explicit_bzero(ctx, sizeof(pg_cryptohash_ctx));
80+
FREE(ctx);
81+
return NULL;
6782
}
6883

69-
if (ctx->data == NULL)
84+
ctx->data = state;
85+
ctx->type = type;
86+
87+
#ifndef FRONTEND
88+
ResourceOwnerEnlargeCryptoHash(CurrentResourceOwner);
89+
#endif
90+
91+
/*
92+
* Initialization takes care of assigning the correct type for OpenSSL.
93+
*/
94+
state->evpctx = EVP_MD_CTX_create();
95+
96+
if (state->evpctx == NULL)
7097
{
98+
explicit_bzero(state, sizeof(pg_cryptohash_state));
7199
explicit_bzero(ctx, sizeof(pg_cryptohash_ctx));
100+
#ifndef FRONTEND
101+
ereport(ERROR,
102+
(errcode(ERRCODE_OUT_OF_MEMORY),
103+
errmsg("out of memory")));
104+
#else
105+
FREE(state);
72106
FREE(ctx);
73107
return NULL;
108+
#endif
74109
}
75110

111+
#ifndef FRONTEND
112+
state->resowner = CurrentResourceOwner;
113+
ResourceOwnerRememberCryptoHash(CurrentResourceOwner,
114+
PointerGetDatum(ctx));
115+
#endif
116+
76117
return ctx;
77118
}
78119

@@ -85,23 +126,26 @@ int
85126
pg_cryptohash_init(pg_cryptohash_ctx *ctx)
86127
{
87128
int status = 0;
129+
pg_cryptohash_state *state;
88130

89131
if (ctx == NULL)
90132
return 0;
91133

134+
state = (pg_cryptohash_state *) ctx->data;
135+
92136
switch (ctx->type)
93137
{
94138
case PG_SHA224:
95-
status = SHA224_Init((SHA256_CTX *) ctx->data);
139+
status = EVP_DigestInit_ex(state->evpctx, EVP_sha224(), NULL);
96140
break;
97141
case PG_SHA256:
98-
status = SHA256_Init((SHA256_CTX *) ctx->data);
142+
status = EVP_DigestInit_ex(state->evpctx, EVP_sha256(), NULL);
99143
break;
100144
case PG_SHA384:
101-
status = SHA384_Init((SHA512_CTX *) ctx->data);
145+
status = EVP_DigestInit_ex(state->evpctx, EVP_sha384(), NULL);
102146
break;
103147
case PG_SHA512:
104-
status = SHA512_Init((SHA512_CTX *) ctx->data);
148+
status = EVP_DigestInit_ex(state->evpctx, EVP_sha512(), NULL);
105149
break;
106150
}
107151

@@ -120,25 +164,13 @@ int
120164
pg_cryptohash_update(pg_cryptohash_ctx *ctx, const uint8 *data, size_t len)
121165
{
122166
int status = 0;
167+
pg_cryptohash_state *state;
123168

124169
if (ctx == NULL)
125170
return 0;
126171

127-
switch (ctx->type)
128-
{
129-
case PG_SHA224:
130-
status = SHA224_Update((SHA256_CTX *) ctx->data, data, len);
131-
break;
132-
case PG_SHA256:
133-
status = SHA256_Update((SHA256_CTX *) ctx->data, data, len);
134-
break;
135-
case PG_SHA384:
136-
status = SHA384_Update((SHA512_CTX *) ctx->data, data, len);
137-
break;
138-
case PG_SHA512:
139-
status = SHA512_Update((SHA512_CTX *) ctx->data, data, len);
140-
break;
141-
}
172+
state = (pg_cryptohash_state *) ctx->data;
173+
status = EVP_DigestUpdate(state->evpctx, data, len);
142174

143175
/* OpenSSL internals return 1 on success, 0 on failure */
144176
if (status <= 0)
@@ -155,25 +187,13 @@ int
155187
pg_cryptohash_final(pg_cryptohash_ctx *ctx, uint8 *dest)
156188
{
157189
int status = 0;
190+
pg_cryptohash_state *state;
158191

159192
if (ctx == NULL)
160193
return 0;
161194

162-
switch (ctx->type)
163-
{
164-
case PG_SHA224:
165-
status = SHA224_Final(dest, (SHA256_CTX *) ctx->data);
166-
break;
167-
case PG_SHA256:
168-
status = SHA256_Final(dest, (SHA256_CTX *) ctx->data);
169-
break;
170-
case PG_SHA384:
171-
status = SHA384_Final(dest, (SHA512_CTX *) ctx->data);
172-
break;
173-
case PG_SHA512:
174-
status = SHA512_Final(dest, (SHA512_CTX *) ctx->data);
175-
break;
176-
}
195+
state = (pg_cryptohash_state *) ctx->data;
196+
status = EVP_DigestFinal_ex(state->evpctx, dest, 0);
177197

178198
/* OpenSSL internals return 1 on success, 0 on failure */
179199
if (status <= 0)
@@ -189,9 +209,21 @@ pg_cryptohash_final(pg_cryptohash_ctx *ctx, uint8 *dest)
189209
void
190210
pg_cryptohash_free(pg_cryptohash_ctx *ctx)
191211
{
212+
pg_cryptohash_state *state;
213+
192214
if (ctx == NULL)
193215
return;
194-
FREE(ctx->data);
216+
217+
state = (pg_cryptohash_state *) ctx->data;
218+
EVP_MD_CTX_destroy(state->evpctx);
219+
220+
#ifndef FRONTEND
221+
ResourceOwnerForgetCryptoHash(state->resowner,
222+
PointerGetDatum(ctx));
223+
#endif
224+
225+
explicit_bzero(state, sizeof(pg_cryptohash_state));
195226
explicit_bzero(ctx, sizeof(pg_cryptohash_ctx));
227+
FREE(state);
196228
FREE(ctx);
197229
}

‎src/include/utils/resowner_private.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,4 +95,11 @@ extern void ResourceOwnerRememberJIT(ResourceOwner owner,
9595
extern void ResourceOwnerForgetJIT(ResourceOwner owner,
9696
Datum handle);
9797

98+
/* support for cryptohash context management */
99+
extern void ResourceOwnerEnlargeCryptoHash(ResourceOwner owner);
100+
extern void ResourceOwnerRememberCryptoHash(ResourceOwner owner,
101+
Datum handle);
102+
extern void ResourceOwnerForgetCryptoHash(ResourceOwner owner,
103+
Datum handle);
104+
98105
#endif /* RESOWNER_PRIVATE_H */

‎src/tools/pgindent/typedefs.list

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3180,6 +3180,7 @@ pg_conv_map
31803180
pg_crc32
31813181
pg_crc32c
31823182
pg_cryptohash_ctx
3183+
pg_cryptohash_state
31833184
pg_cryptohash_type
31843185
pg_ctype_cache
31853186
pg_enc

0 commit comments

Comments
 (0)