On Thu, Sep 26, 2019 at 06:24:22PM +0200, Peter Eisentraut wrote:
> Here is my proposed patch, currently completely untested.
I have tested compilation of REL_12_STABLE with the top of OpenSSL
0.9.8, 1.0.0, 1.0.1, 1.0.2, 1.1.0 and 1.1.1. Our SSL tests also pass
in all the setups I have tested.
Your patch does not issue a ereport(LOG/FATAL) in the event of a
failure with SSL_CTX_set_max_proto_version(), which is something done
when ssl_protocol_version_to_openssl()'s result is -1. Wouldn't it be
better to report that properly to the user?
Some more nits about the patch I have. Would it be worth copying the
comment from min_proto_version() to SSL_CTX_set_max_proto_version()?
I would add a newline before the comment block as well.
Note: We have a failure with ssl/t/002_scram.pl because of the
introduction of the recent channel_binding parameter if you try to run
the SSL tests on HEAD with at least 0.9.8 as we forgot to add a
conditional check for HAVE_X509_GET_SIGNATURE_NID as c3d41cc did.
I'll send a patch for that separately. That's why I have checked the
patch only with REL_12_STABLE.
--
Michael