> On 15 Jan 2020, at 03:28, Michael Paquier <mich...@paquier.xyz> wrote: > > On Tue, Jan 14, 2020 at 11:21:53AM +0100, Daniel Gustafsson wrote: >> On 14 Jan 2020, at 04:54, Michael Paquier <mich...@paquier.xyz> wrote: >>> Please note that OpenSSL 1.1.0 has added two routines to be able to >>> get the min/max protocols set in a context, called >>> SSL_CTX_get_min/max_proto_version. Thinking about older versions of >>> OpenSSL I think that it is better to use >>> ssl_protocol_version_to_openssl to do the parsing work. I also found >>> that it is easier to check for compatible versions after setting both >>> bounds in the SSL context, so as there is no need to worry about >>> invalid values depending on the build of OpenSSL used. >> >> I'm not convinced that it's a good idea to check for incompatible protocol >> range in the OpenSSL backend. We've spent a lot of energy to make the TLS >> code >> library agnostic and pluggable, and since identifying a basic configuration >> error isn't OpenSSL specific I think it should be in the guc code. That >> would >> keep the layering as well as ensure that we don't mistakenly treat this >> differently should we get a second TLS backend. > > Good points. And the get routines are not that portable in OpenSSL > either even if HEAD supports 1.0.1 and newer versions... Attached is > an updated patch which uses a GUC check for both parameters, and > provides a hint on top of the original error message. The SSL context > does not get reloaded if there is an error, so the errors from OpenSSL > cannot be triggered as far as I checked (after mixing a couple of > corrent and incorrect combinations manually).
This is pretty much exactly the patch I was intending to write for this, so +1 from me. cheers ./daniel
