Re: Improve errors when setting incorrect bounds for SSL protocols

Daniel Gustafsson <daniel@yesql.se>

From: Daniel Gustafsson <daniel@yesql.se>
To: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2020-03-19T21:54:35Z
Lists: pgsql-hackers
> On 7 Feb 2020, at 01:33, Michael Paquier <michael@paquier.xyz> wrote:
> 
> On Thu, Feb 06, 2020 at 11:30:40PM +0100, Daniel Gustafsson wrote:
>> Or change to the v1 patch in this thread, which avoids the problem by doing it
>> in the OpenSSL code.  It's a shame to have generic TLS functionality be OpenSSL
>> specific when everything else TLS has been abstracted, but not working is
>> clearly a worse option.
> 
> The v1 would work just fine considering that, as the code would be
> invoked in a context where all GUCs are already loaded.  That's too
> late before the release though, so I have reverted 41aadee, and
> attached is a new patch to consider with improvements compared to v1
> mainly in the error messages.

Having gone back to look at this, I can't think of a better way to implement
this and I think we should go ahead with the proposed patch.

In this message we aren't quoting the TLS protocol setting:
+          (errmsg("%s setting %s not supported by this build",
..but in this detail we are:
+           errdetail("\"%s\" cannot be higher than \"%s\"",
Perhaps we should be consistent across all ereports?

Marking as ready for committer.

cheers ./daniel




Commits

  1. Fix check for conflicting SSL min/max protocol settings

  2. Add bound checks for ssl_min_protocol_version and ssl_max_protocol_version

  3. Revert "Add GUC checks for ssl_min_protocol_version and ssl_max_protocol_version"

  4. Add GUC checks for ssl_min_protocol_version and ssl_max_protocol_version