Fix allocation logic of cryptohash context data with OpenSSL
The allocation of the cryptohash context data when building with OpenSSL was happening in the memory context of the caller of pg_cryptohash_create(), which could lead to issues with resowner cleanup if cascading resources are cleaned up on an error. Like other facilities using resowners, move the base allocation to TopMemoryContext to ensure a correct cleanup on failure. The resulting code gets simpler with this commit as the context data is now hold by a unique opaque pointer, so as there is only one single allocation done in TopMemoryContext. After discussion, also change the cryptohash subroutines to return an error if the caller provides NULL for the context data to ease error detection on OOM. Author: Heikki Linnakangas Discussion: https://postgr.es/m/X9xbuEoiU3dlImfa@paquier.xyz
This commit is contained in:
Michael Paquier
parent
9877374bef
commit
55fe26a4b5
|
|
@ -39,6 +39,21 @@
|
|||
#define FREE(ptr) free(ptr)
|
||||
#endif
|
||||
|
||||
/* Internal pg_cryptohash_ctx structure */
|
||||
struct pg_cryptohash_ctx
|
||||
{
|
||||
pg_cryptohash_type type;
|
||||
|
||||
union
|
||||
{
|
||||
pg_md5_ctx md5;
|
||||
pg_sha224_ctx sha224;
|
||||
pg_sha256_ctx sha256;
|
||||
pg_sha384_ctx sha384;
|
||||
pg_sha512_ctx sha512;
|
||||
} data;
|
||||
};
|
||||
|
||||
/*
|
||||
* pg_cryptohash_create
|
||||
*
|
||||
|
|
@ -50,38 +65,18 @@ pg_cryptohash_create(pg_cryptohash_type type)
|
|||
{
|
||||
pg_cryptohash_ctx *ctx;
|
||||
|
||||
/*
|
||||
* Note that this always allocates enough space for the largest hash. A
|
||||
* smaller allocation would be enough for md5, sha224 and sha256, but the
|
||||
* small extra amount of memory does not make it worth complicating this
|
||||
* code.
|
||||
*/
|
||||
ctx = ALLOC(sizeof(pg_cryptohash_ctx));
|
||||
if (ctx == NULL)
|
||||
return NULL;
|
||||
|
||||
memset(ctx, 0, sizeof(pg_cryptohash_ctx));
|
||||
ctx->type = type;
|
||||
|
||||
switch (type)
|
||||
{
|
||||
case PG_MD5:
|
||||
ctx->data = ALLOC(sizeof(pg_md5_ctx));
|
||||
break;
|
||||
case PG_SHA224:
|
||||
ctx->data = ALLOC(sizeof(pg_sha224_ctx));
|
||||
break;
|
||||
case PG_SHA256:
|
||||
ctx->data = ALLOC(sizeof(pg_sha256_ctx));
|
||||
break;
|
||||
case PG_SHA384:
|
||||
ctx->data = ALLOC(sizeof(pg_sha384_ctx));
|
||||
break;
|
||||
case PG_SHA512:
|
||||
ctx->data = ALLOC(sizeof(pg_sha512_ctx));
|
||||
break;
|
||||
}
|
||||
|
||||
if (ctx->data == NULL)
|
||||
{
|
||||
explicit_bzero(ctx, sizeof(pg_cryptohash_ctx));
|
||||
FREE(ctx);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
return ctx;
|
||||
}
|
||||
|
||||
|
|
@ -95,24 +90,24 @@ int
|
|||
pg_cryptohash_init(pg_cryptohash_ctx *ctx)
|
||||
{
|
||||
if (ctx == NULL)
|
||||
return 0;
|
||||
return -1;
|
||||
|
||||
switch (ctx->type)
|
||||
{
|
||||
case PG_MD5:
|
||||
pg_md5_init((pg_md5_ctx *) ctx->data);
|
||||
pg_md5_init(&ctx->data.md5);
|
||||
break;
|
||||
case PG_SHA224:
|
||||
pg_sha224_init((pg_sha224_ctx *) ctx->data);
|
||||
pg_sha224_init(&ctx->data.sha224);
|
||||
break;
|
||||
case PG_SHA256:
|
||||
pg_sha256_init((pg_sha256_ctx *) ctx->data);
|
||||
pg_sha256_init(&ctx->data.sha256);
|
||||
break;
|
||||
case PG_SHA384:
|
||||
pg_sha384_init((pg_sha384_ctx *) ctx->data);
|
||||
pg_sha384_init(&ctx->data.sha384);
|
||||
break;
|
||||
case PG_SHA512:
|
||||
pg_sha512_init((pg_sha512_ctx *) ctx->data);
|
||||
pg_sha512_init(&ctx->data.sha512);
|
||||
break;
|
||||
}
|
||||
|
||||
|
|
@ -123,30 +118,31 @@ pg_cryptohash_init(pg_cryptohash_ctx *ctx)
|
|||
* pg_cryptohash_update
|
||||
*
|
||||
* Update a hash context. Note that this implementation is designed
|
||||
* to never fail, so this always returns 0.
|
||||
* to never fail, so this always returns 0 except if the caller has
|
||||
* given a NULL context.
|
||||
*/
|
||||
int
|
||||
pg_cryptohash_update(pg_cryptohash_ctx *ctx, const uint8 *data, size_t len)
|
||||
{
|
||||
if (ctx == NULL)
|
||||
return 0;
|
||||
return -1;
|
||||
|
||||
switch (ctx->type)
|
||||
{
|
||||
case PG_MD5:
|
||||
pg_md5_update((pg_md5_ctx *) ctx->data, data, len);
|
||||
pg_md5_update(&ctx->data.md5, data, len);
|
||||
break;
|
||||
case PG_SHA224:
|
||||
pg_sha224_update((pg_sha224_ctx *) ctx->data, data, len);
|
||||
pg_sha224_update(&ctx->data.sha224, data, len);
|
||||
break;
|
||||
case PG_SHA256:
|
||||
pg_sha256_update((pg_sha256_ctx *) ctx->data, data, len);
|
||||
pg_sha256_update(&ctx->data.sha256, data, len);
|
||||
break;
|
||||
case PG_SHA384:
|
||||
pg_sha384_update((pg_sha384_ctx *) ctx->data, data, len);
|
||||
pg_sha384_update(&ctx->data.sha384, data, len);
|
||||
break;
|
||||
case PG_SHA512:
|
||||
pg_sha512_update((pg_sha512_ctx *) ctx->data, data, len);
|
||||
pg_sha512_update(&ctx->data.sha512, data, len);
|
||||
break;
|
||||
}
|
||||
|
||||
|
|
@ -157,30 +153,31 @@ pg_cryptohash_update(pg_cryptohash_ctx *ctx, const uint8 *data, size_t len)
|
|||
* pg_cryptohash_final
|
||||
*
|
||||
* Finalize a hash context. Note that this implementation is designed
|
||||
* to never fail, so this always returns 0.
|
||||
* to never fail, so this always returns 0 except if the caller has
|
||||
* given a NULL context.
|
||||
*/
|
||||
int
|
||||
pg_cryptohash_final(pg_cryptohash_ctx *ctx, uint8 *dest)
|
||||
{
|
||||
if (ctx == NULL)
|
||||
return 0;
|
||||
return -1;
|
||||
|
||||
switch (ctx->type)
|
||||
{
|
||||
case PG_MD5:
|
||||
pg_md5_final((pg_md5_ctx *) ctx->data, dest);
|
||||
pg_md5_final(&ctx->data.md5, dest);
|
||||
break;
|
||||
case PG_SHA224:
|
||||
pg_sha224_final((pg_sha224_ctx *) ctx->data, dest);
|
||||
pg_sha224_final(&ctx->data.sha224, dest);
|
||||
break;
|
||||
case PG_SHA256:
|
||||
pg_sha256_final((pg_sha256_ctx *) ctx->data, dest);
|
||||
pg_sha256_final(&ctx->data.sha256, dest);
|
||||
break;
|
||||
case PG_SHA384:
|
||||
pg_sha384_final((pg_sha384_ctx *) ctx->data, dest);
|
||||
pg_sha384_final(&ctx->data.sha384, dest);
|
||||
break;
|
||||
case PG_SHA512:
|
||||
pg_sha512_final((pg_sha512_ctx *) ctx->data, dest);
|
||||
pg_sha512_final(&ctx->data.sha512, dest);
|
||||
break;
|
||||
}
|
||||
|
||||
|
|
@ -198,26 +195,6 @@ pg_cryptohash_free(pg_cryptohash_ctx *ctx)
|
|||
if (ctx == NULL)
|
||||
return;
|
||||
|
||||
switch (ctx->type)
|
||||
{
|
||||
case PG_MD5:
|
||||
explicit_bzero(ctx->data, sizeof(pg_md5_ctx));
|
||||
break;
|
||||
case PG_SHA224:
|
||||
explicit_bzero(ctx->data, sizeof(pg_sha224_ctx));
|
||||
break;
|
||||
case PG_SHA256:
|
||||
explicit_bzero(ctx->data, sizeof(pg_sha256_ctx));
|
||||
break;
|
||||
case PG_SHA384:
|
||||
explicit_bzero(ctx->data, sizeof(pg_sha384_ctx));
|
||||
break;
|
||||
case PG_SHA512:
|
||||
explicit_bzero(ctx->data, sizeof(pg_sha512_ctx));
|
||||
break;
|
||||
}
|
||||
|
||||
FREE(ctx->data);
|
||||
explicit_bzero(ctx, sizeof(pg_cryptohash_ctx));
|
||||
FREE(ctx);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -31,11 +31,12 @@
|
|||
#endif
|
||||
|
||||
/*
|
||||
* In backend, use palloc/pfree to ease the error handling. In frontend,
|
||||
* use malloc to be able to return a failure status back to the caller.
|
||||
* In the backend, use an allocation in TopMemoryContext to count for
|
||||
* resowner cleanup handling. In the frontend, use malloc to be able
|
||||
* to return a failure status back to the caller.
|
||||
*/
|
||||
#ifndef FRONTEND
|
||||
#define ALLOC(size) palloc(size)
|
||||
#define ALLOC(size) MemoryContextAlloc(TopMemoryContext, size)
|
||||
#define FREE(ptr) pfree(ptr)
|
||||
#else
|
||||
#define ALLOC(size) malloc(size)
|
||||
|
|
@ -43,19 +44,21 @@
|
|||
#endif
|
||||
|
||||
/*
|
||||
* Internal structure for pg_cryptohash_ctx->data.
|
||||
* Internal pg_cryptohash_ctx structure.
|
||||
*
|
||||
* This tracks the resource owner associated to each EVP context data
|
||||
* for the backend.
|
||||
*/
|
||||
typedef struct pg_cryptohash_state
|
||||
struct pg_cryptohash_ctx
|
||||
{
|
||||
pg_cryptohash_type type;
|
||||
|
||||
EVP_MD_CTX *evpctx;
|
||||
|
||||
#ifndef FRONTEND
|
||||
ResourceOwner resowner;
|
||||
#endif
|
||||
} pg_cryptohash_state;
|
||||
};
|
||||
|
||||
/*
|
||||
* pg_cryptohash_create
|
||||
|
|
@ -67,49 +70,42 @@ 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;
|
||||
|
||||
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;
|
||||
|
||||
/*
|
||||
* Make sure that the resource owner has space to remember this reference.
|
||||
* This can error out with "out of memory", so do this before any other
|
||||
* allocation to avoid leaking.
|
||||
*/
|
||||
#ifndef FRONTEND
|
||||
ResourceOwnerEnlargeCryptoHash(CurrentResourceOwner);
|
||||
#endif
|
||||
|
||||
ctx = ALLOC(sizeof(pg_cryptohash_ctx));
|
||||
if (ctx == NULL)
|
||||
return NULL;
|
||||
memset(ctx, 0, sizeof(pg_cryptohash_ctx));
|
||||
ctx->type = type;
|
||||
|
||||
/*
|
||||
* Initialization takes care of assigning the correct type for OpenSSL.
|
||||
*/
|
||||
state->evpctx = EVP_MD_CTX_create();
|
||||
ctx->evpctx = EVP_MD_CTX_create();
|
||||
|
||||
if (state->evpctx == NULL)
|
||||
if (ctx->evpctx == NULL)
|
||||
{
|
||||
explicit_bzero(state, sizeof(pg_cryptohash_state));
|
||||
explicit_bzero(ctx, sizeof(pg_cryptohash_ctx));
|
||||
FREE(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;
|
||||
ctx->resowner = CurrentResourceOwner;
|
||||
ResourceOwnerRememberCryptoHash(CurrentResourceOwner,
|
||||
PointerGetDatum(ctx));
|
||||
#endif
|
||||
|
|
@ -126,29 +122,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;
|
||||
return -1;
|
||||
|
||||
switch (ctx->type)
|
||||
{
|
||||
case PG_MD5:
|
||||
status = EVP_DigestInit_ex(state->evpctx, EVP_md5(), NULL);
|
||||
status = EVP_DigestInit_ex(ctx->evpctx, EVP_md5(), NULL);
|
||||
break;
|
||||
case PG_SHA224:
|
||||
status = EVP_DigestInit_ex(state->evpctx, EVP_sha224(), NULL);
|
||||
status = EVP_DigestInit_ex(ctx->evpctx, EVP_sha224(), NULL);
|
||||
break;
|
||||
case PG_SHA256:
|
||||
status = EVP_DigestInit_ex(state->evpctx, EVP_sha256(), NULL);
|
||||
status = EVP_DigestInit_ex(ctx->evpctx, EVP_sha256(), NULL);
|
||||
break;
|
||||
case PG_SHA384:
|
||||
status = EVP_DigestInit_ex(state->evpctx, EVP_sha384(), NULL);
|
||||
status = EVP_DigestInit_ex(ctx->evpctx, EVP_sha384(), NULL);
|
||||
break;
|
||||
case PG_SHA512:
|
||||
status = EVP_DigestInit_ex(state->evpctx, EVP_sha512(), NULL);
|
||||
status = EVP_DigestInit_ex(ctx->evpctx, EVP_sha512(), NULL);
|
||||
break;
|
||||
}
|
||||
|
||||
|
|
@ -167,13 +160,11 @@ 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;
|
||||
return -1;
|
||||
|
||||
state = (pg_cryptohash_state *) ctx->data;
|
||||
status = EVP_DigestUpdate(state->evpctx, data, len);
|
||||
status = EVP_DigestUpdate(ctx->evpctx, data, len);
|
||||
|
||||
/* OpenSSL internals return 1 on success, 0 on failure */
|
||||
if (status <= 0)
|
||||
|
|
@ -190,13 +181,11 @@ int
|
|||
pg_cryptohash_final(pg_cryptohash_ctx *ctx, uint8 *dest)
|
||||
{
|
||||
int status = 0;
|
||||
pg_cryptohash_state *state;
|
||||
|
||||
if (ctx == NULL)
|
||||
return 0;
|
||||
return -1;
|
||||
|
||||
state = (pg_cryptohash_state *) ctx->data;
|
||||
status = EVP_DigestFinal_ex(state->evpctx, dest, 0);
|
||||
status = EVP_DigestFinal_ex(ctx->evpctx, dest, 0);
|
||||
|
||||
/* OpenSSL internals return 1 on success, 0 on failure */
|
||||
if (status <= 0)
|
||||
|
|
@ -212,21 +201,16 @@ 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;
|
||||
|
||||
state = (pg_cryptohash_state *) ctx->data;
|
||||
EVP_MD_CTX_destroy(state->evpctx);
|
||||
EVP_MD_CTX_destroy(ctx->evpctx);
|
||||
|
||||
#ifndef FRONTEND
|
||||
ResourceOwnerForgetCryptoHash(state->resowner,
|
||||
ResourceOwnerForgetCryptoHash(ctx->resowner,
|
||||
PointerGetDatum(ctx));
|
||||
#endif
|
||||
|
||||
explicit_bzero(state, sizeof(pg_cryptohash_state));
|
||||
explicit_bzero(ctx, sizeof(pg_cryptohash_ctx));
|
||||
FREE(state);
|
||||
FREE(ctx);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -25,12 +25,8 @@ typedef enum
|
|||
PG_SHA512
|
||||
} pg_cryptohash_type;
|
||||
|
||||
typedef struct pg_cryptohash_ctx
|
||||
{
|
||||
pg_cryptohash_type type;
|
||||
/* private area used by each hash implementation */
|
||||
void *data;
|
||||
} pg_cryptohash_ctx;
|
||||
/* opaque context, private to each cryptohash implementation */
|
||||
typedef struct pg_cryptohash_ctx pg_cryptohash_ctx;
|
||||
|
||||
extern pg_cryptohash_ctx *pg_cryptohash_create(pg_cryptohash_type type);
|
||||
extern int pg_cryptohash_init(pg_cryptohash_ctx *ctx);
|
||||
|
|
|
|||
|
|
@ -3196,7 +3196,6 @@ pg_conv_map
|
|||
pg_crc32
|
||||
pg_crc32c
|
||||
pg_cryptohash_ctx
|
||||
pg_cryptohash_state
|
||||
pg_cryptohash_type
|
||||
pg_ctype_cache
|
||||
pg_enc
|
||||
|
|
|
|||
Loading…
Reference in New Issue