Re: Improve errors when setting incorrect bounds for SSL protocols

On Tue, Jan 14, 2020 at 11:21:53AM +0100, Daniel Gustafsson wrote:
> On 14 Jan 2020, at 04:54, Michael Paquier <michael@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).
--
Michael