Making openssl_tls_init_hook OpenSSL specific
Commit 896fcdb230e72 (sorry for chiming in too late, I missed that thread)
added a TLS init hook which is OpenSSL specific: openssl_tls_init_hook. Since
the rest of the TLS support in the backend is library agnostic, we should IMO
make this hook follow that pattern, else this will make a non-OpenSSL backend
not compile.
If we make the hook generic, extension authors must have a way to tell which
backend invoked it, so maybe the best option is to simply wrap this hook in
USE_OPENSSL ifdefs and keep the name/signature? Looking at the Secure
Transport patch I wrote, there is really no equivalent callsite; the same goes
for a libnss patch which I haven't yet submitted.
The attached adds USE_OPENSSL guards.
cheers ./daniel
Attachments:
openssl_hook_guards.patchapplication/octet-stream; name=openssl_hook_guards.patch; x-unix-mode=0644Download+2-1
On Thu, Apr 16, 2020 at 02:17:33PM +0200, Daniel Gustafsson wrote:
Commit 896fcdb230e72 (sorry for chiming in too late, I missed that thread)
added a TLS init hook which is OpenSSL specific: openssl_tls_init_hook. Since
the rest of the TLS support in the backend is library agnostic, we should IMO
make this hook follow that pattern, else this will make a non-OpenSSL backend
not compile.
Better sooner than later, thanks for the report.
If we make the hook generic, extension authors must have a way to tell which
backend invoked it, so maybe the best option is to simply wrap this hook in
USE_OPENSSL ifdefs and keep the name/signature? Looking at the Secure
Transport patch I wrote, there is really no equivalent callsite; the same goes
for a libnss patch which I haven't yet submitted.The attached adds USE_OPENSSL guards.
I agree that this looks like an oversight of the original commit
introducing the hook as it gets called in the OpenSSL code path of
be_tls_init(), so I think that your patch is right (though I would
have just used #ifdef USE_OPENSSL here). And if the future proves
that this hook has more uses for other SSL implementations, we could
always rework it at this point, if necessary. Andrew, would you
prefer fixing that yourself?
--
Michael
On 4/16/20 9:57 PM, Michael Paquier wrote:
On Thu, Apr 16, 2020 at 02:17:33PM +0200, Daniel Gustafsson wrote:
Commit 896fcdb230e72 (sorry for chiming in too late, I missed that thread)
added a TLS init hook which is OpenSSL specific: openssl_tls_init_hook. Since
the rest of the TLS support in the backend is library agnostic, we should IMO
make this hook follow that pattern, else this will make a non-OpenSSL backend
not compile.Better sooner than later, thanks for the report.
If we make the hook generic, extension authors must have a way to tell which
backend invoked it, so maybe the best option is to simply wrap this hook in
USE_OPENSSL ifdefs and keep the name/signature? Looking at the Secure
Transport patch I wrote, there is really no equivalent callsite; the same goes
for a libnss patch which I haven't yet submitted.The attached adds USE_OPENSSL guards.
I agree that this looks like an oversight of the original commit
introducing the hook as it gets called in the OpenSSL code path of
be_tls_init(), so I think that your patch is right (though I would
have just used #ifdef USE_OPENSSL here). And if the future proves
that this hook has more uses for other SSL implementations, we could
always rework it at this point, if necessary. Andrew, would you
prefer fixing that yourself?
Sure, I'll do it.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services