diff --git a/src/common/cryptohash.c b/src/common/cryptohash.c index 1921a33b34..efedadd626 100644 --- a/src/common/cryptohash.c +++ b/src/common/cryptohash.c @@ -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); } diff --git a/src/common/cryptohash_openssl.c b/src/common/cryptohash_openssl.c index 0fd6622576..551ec392b6 100644 --- a/src/common/cryptohash_openssl.c +++ b/src/common/cryptohash_openssl.c @@ -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); } diff --git a/src/include/common/cryptohash.h b/src/include/common/cryptohash.h index 4c3df8e5ae..3ecaf62113 100644 --- a/src/include/common/cryptohash.h +++ b/src/include/common/cryptohash.h @@ -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); diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 9cd047ba25..f3957bad6c 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -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