Обсуждение: 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
Вложения
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