On Fri, Dec 04, 2020 at 09:08:03PM -0500, Bruce Momjian wrote: > Here is an updated patch to handle the new hash API introduced by > commit 87ae9691d2.
+ if (!ossl_initialized) + { +#ifdef HAVE_OPENSSL_INIT_SSL + OPENSSL_init_ssl(OPENSSL_INIT_LOAD_CONFIG, NULL); +#else + OPENSSL_config(NULL); + SSL_library_init(); + SSL_load_error_strings(); +#endif + ossl_initialized = true; This is a duplicate of what's done in be-secure-openssl.c, and it does not strike me as a good idea to do that potentially twice. git diff --check complains. +extern bool pg_HMAC_SHA512(const uint8 *key, + const uint8 *in, int inlen, + uint8 *out); I think that the split done in this patch makes the HMAC handling in the core code messier: - SCRAM makes use of HMAC internally, and we should try to use the HMAC of OpenSSL if building with it even for SCRAM. - For the first reason, I think that we should also have a fallback implementation. - This API layer should not depend directly on the SHA2 used (SCRAM uses SHA256 with HMAC). FWIW, I got plans to work on that once I am done with the business around MD5 and OpenSSL. The refactoring done with the ciphers moved from pgcrypto to src/common/ should be a separate patch. In short, it would be good to rework this patch and split it into pieces that are independently useful. This would make the review much easier as well. -- Michael
signature.asc
Description: PGP signature