using palloc/pfree for OpenSSL allocations with CRYPTO_set_mem_functions
I noticed that OpenSSL has a CRYPTO_set_mem_functions
<https://www.openssl.org/docs/man3.2/man3/CRYPTO_set_mem_functions.html>
function:
If no allocations have been done, it is possible to “swap out” the default
implementations for OPENSSL_malloc(), OPENSSL_realloc() and OPENSSL_free()
and replace them with alternate versions.
But a different technique is used in contrib/pgcrypto/openssl.c
<https://github.com/postgres/postgres/blob/REL_16_STABLE/contrib/pgcrypto/openssl.c#L52-L56>
to work around the different allocation system of OpenSSL:
To make sure we don't leak OpenSSL handles on abort, we keep OSSLCipher
objects in a linked list, allocated in TopMemoryContext. We use the
ResourceOwner mechanism to free them on abort.
I see the particulars might have changed on the master branch though!
CRYPTO_set_mem_functions must be called *before* any uses of malloc/free
though, so I believe it needs to be called right after the OPENSSL_init_ssl
<https://www.openssl.org/docs/man3.2/man3/OPENSSL_init_ssl.html> calls
(e.g. in src/backend/libpq/be-secure-openssl.c
<https://github.com/postgres/postgres/blob/REL_16_STABLE/src/backend/libpq/be-secure-openssl.c#L102>
and src/interfaces/libpq/fe-secure-openssl.c
<https://github.com/postgres/postgres/blob/REL_16_STABLE/src/interfaces/libpq/fe-secure-openssl.c#L858>)
for this to be possible.
Would it be desirable to do this? If not, why is the TopMemoryContext
approach a better option? I do not understand the code quite well enough to
evaluate the tradeoffs myself yet!
Best,
Evan
P.S. Searcthing all time with this query
<https://www.postgresql.org/search/?m=1&q=CRYPTO_set_mem_functions&l=&d=-1&s=r>,
I found one thread in 2018 that mentions CRYPTO_set_mem_functions, but in a
different capacity. I hope I did not miss some other mention of it on the
email lists!
Evan Czaplicki <evancz@gmail.com> writes:
I noticed that OpenSSL has a CRYPTO_set_mem_functions
<https://www.openssl.org/docs/man3.2/man3/CRYPTO_set_mem_functions.html>
function:
If no allocations have been done, it is possible to “swap out” the default
implementations for OPENSSL_malloc(), OPENSSL_realloc() and OPENSSL_free()
and replace them with alternate versions.
But a different technique is used in contrib/pgcrypto/openssl.c
To make sure we don't leak OpenSSL handles on abort, we keep OSSLCipher
objects in a linked list, allocated in TopMemoryContext. We use the
ResourceOwner mechanism to free them on abort.
Would it be desirable to do this? If not, why is the TopMemoryContext
approach a better option? I do not understand the code quite well enough to
evaluate the tradeoffs myself yet!
Seems to me that these address different purposes. If we put in a
CRYPTO_set_mem_functions layer, I doubt that we'd have any good idea
of which allocations are used for what. So we could not replace what
pgcrypto is doing with a simple MemoryContextReset (even if we cared
to assume that freeing an OSSLCipher involves only free() operations
and no other resources). I think the only real win we'd get from
such a layer is that OpenSSL's allocations would be better exposed
for accounting purposes, eg the pg_backend_memory_contexts view.
That's not negligible, but I don't find it a compelling reason to
do the work, either.
regards, tom lane
Thank you for the explanation! I looked into what OpenSSL does when freeing
the underlying OSSLCipher resources, and as you say, it is more than just a
free. There is sometimes aditional values to free, and they go through and
overwrite the data in a special way regardless. So I see why the
TopMemoryContext approach is needed either way.
My particular interest was in using the BIGNUM implementation from OpenSSL
within my project, but the BIGNUM frees also do more than just freeing. So
it seems there is no way around the TopMemoryContext in my case either.
I could imagine that there may be some effect on the particular character
of memory fragmentation that comes with their malloc/free vs palloc/pfree,
but that's definitely outside of my personal purposes and outside of my
expertise to evaluate even at a high level!
So thank you again for helping me understand this! Happy to have a much
clear understanding of the TopMemoryContext approach!
Evan
On Thu, Feb 1, 2024 at 5:01 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Show quoted text
Evan Czaplicki <evancz@gmail.com> writes:
I noticed that OpenSSL has a CRYPTO_set_mem_functions
<https://www.openssl.org/docs/man3.2/man3/CRYPTO_set_mem_functions.html>
function:If no allocations have been done, it is possible to “swap out” the
default
implementations for OPENSSL_malloc(), OPENSSL_realloc() and
OPENSSL_free()
and replace them with alternate versions.
But a different technique is used in contrib/pgcrypto/openssl.c
To make sure we don't leak OpenSSL handles on abort, we keep OSSLCipher
objects in a linked list, allocated in TopMemoryContext. We use the
ResourceOwner mechanism to free them on abort.Would it be desirable to do this? If not, why is the TopMemoryContext
approach a better option? I do not understand the code quite well enoughto
evaluate the tradeoffs myself yet!
Seems to me that these address different purposes. If we put in a
CRYPTO_set_mem_functions layer, I doubt that we'd have any good idea
of which allocations are used for what. So we could not replace what
pgcrypto is doing with a simple MemoryContextReset (even if we cared
to assume that freeing an OSSLCipher involves only free() operations
and no other resources). I think the only real win we'd get from
such a layer is that OpenSSL's allocations would be better exposed
for accounting purposes, eg the pg_backend_memory_contexts view.
That's not negligible, but I don't find it a compelling reason to
do the work, either.regards, tom lane