Making openssl_tls_init_hook OpenSSL specific
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t42240_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t42240_1 && git checkout t42240_1Patchset v1 (message #1) is on t42240_1
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
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