Horton

Thread

Commits

  1. Add hints about protocol-version-related SSL connection failures.

  2. Change libpq's default ssl_min_protocol_version to TLSv1.2.

  1. should libpq also require TLSv1.2 by default?

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2020-06-24T06:39:26Z

    In PG13, we raised the server-side default of ssl_min_protocol_version 
to TLSv1.2.  We also added a connection setting named 
ssl_min_protocol_version to libpq.  But AFAICT, the default value of the 
libpq setting is empty, so any protocol version will be accepted.  Is 
this what we wanted?  Should we raise the default in libpq as well?

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  2. Re: should libpq also require TLSv1.2 by default?

    Daniel Gustafsson <daniel@yesql.se> — 2020-06-24T08:33:22Z

    > On 24 Jun 2020, at 08:39, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:
> 
> In PG13, we raised the server-side default of ssl_min_protocol_version to TLSv1.2.  We also added a connection setting named ssl_min_protocol_version to libpq.  But AFAICT, the default value of the libpq setting is empty, so any protocol version will be accepted.  Is this what we wanted?  Should we raise the default in libpq as well?

This was discussed [0] when the connection settings were introduced, and the
concensus was to leave them alone [1] to allow for example a new pg_dump to
work against an old server.  Re-reading the thread I think the argument still
holds, but I was about to respond "yes, let's do this" before refreshing my
memory.  Perhaps we should add a comment explaining this along the lines of the
attached?

cheers ./daniel

[0] https://www.postgresql.org/message-id/157800160408.1198.1714906047977693148.pgcf%40coridan.postgresql.org
[1] https://www.postgresql.org/message-id/31993.1578321474%40sss.pgh.pa.us

  3. Re: should libpq also require TLSv1.2 by default?

    Magnus Hagander <magnus@hagander.net> — 2020-06-24T08:46:17Z

    On Wed, Jun 24, 2020 at 10:33 AM Daniel Gustafsson <daniel@yesql.se> wrote:

> > On 24 Jun 2020, at 08:39, Peter Eisentraut <
> peter.eisentraut@2ndquadrant.com> wrote:
> >
> > In PG13, we raised the server-side default of ssl_min_protocol_version
> to TLSv1.2.  We also added a connection setting named
> ssl_min_protocol_version to libpq.  But AFAICT, the default value of the
> libpq setting is empty, so any protocol version will be accepted.  Is this
> what we wanted?  Should we raise the default in libpq as well?
>
> This was discussed [0] when the connection settings were introduced, and
> the
> concensus was to leave them alone [1] to allow for example a new pg_dump to
> work against an old server.  Re-reading the thread I think the argument
> still
> holds, but I was about to respond "yes, let's do this" before refreshing my
> memory.  Perhaps we should add a comment explaining this along the lines
> of the
> attached?
>
>
Another argument for not changing the default is that if you want to use
SSL in any meaningful way you have to *already* change the connection
string (with sslmode=require or verify-*), so it's not unreasonable to make
that consideration at the same time.

It might also be worth noting that it's not really "any protocol version",
it means it will be "whatever the openssl configuration says", I think? For
example, debian buster sets:

[system_default_sect]
MinProtocol = TLSv1.2

Which I believe means that if your libpq app is running on debian buster,
it will be min v1.2 already (and it would likely be more useful to use
ssl_min_protocol_version to *lower* that when connecting to older servers).

//Magnus

  4. Re: should libpq also require TLSv1.2 by default?

    Daniel Gustafsson <daniel@yesql.se> — 2020-06-24T09:01:48Z

    > On 24 Jun 2020, at 10:46, Magnus Hagander <magnus@hagander.net> wrote:

> It might also be worth noting that it's not really "any protocol version", it means it will be "whatever the openssl configuration says", I think? For example, debian buster sets:
> 
> [system_default_sect]
> MinProtocol = TLSv1.2
> 
> Which I believe means that if your libpq app is running on debian buster, it will be min v1.2 already

Correct, that being said I'm not sure how common it is for distributions to set
a default protocol version.  The macOS versions I have handy doesn't enforce a
default version, nor does Ubuntu 20.04, FreeBSD 12 or OpenBSD 6.5 AFAICT.

> (and it would likely be more useful to use ssl_min_protocol_version to *lower* that when connecting to older servers).

That is indeed one use-case for the connection parameter.

cheers ./daniel

  5. Re: should libpq also require TLSv1.2 by default?

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2020-06-24T17:57:31Z

    On 2020-06-24 10:33, Daniel Gustafsson wrote:
>> In PG13, we raised the server-side default of ssl_min_protocol_version to TLSv1.2.  We also added a connection setting named ssl_min_protocol_version to libpq.  But AFAICT, the default value of the libpq setting is empty, so any protocol version will be accepted.  Is this what we wanted?  Should we raise the default in libpq as well?
> 
> This was discussed [0] when the connection settings were introduced, and the
> concensus was to leave them alone [1] to allow for example a new pg_dump to
> work against an old server.  Re-reading the thread I think the argument still
> holds, but I was about to respond "yes, let's do this" before refreshing my
> memory.  Perhaps we should add a comment explaining this along the lines of the
> attached?
> 
> [0] https://www.postgresql.org/message-id/157800160408.1198.1714906047977693148.pgcf%40coridan.postgresql.org
> [1] https://www.postgresql.org/message-id/31993.1578321474%40sss.pgh.pa.us

ISTM that these discussions went through the same questions and 
arguments that were made regarding the server-side change but arrived at 
a different conclusion.  So I suggest to reconsider this so that we 
don't ship with contradictory results.

That doesn't necessarily mean that we have to make a change, but we 
should make sure our rationale is sound.

Note that all OpenSSL versions that do not support TLSv1.2 also do not 
support TLSv1.1.  So by saying, in effect, that TLSv1.2 is too new to 
require, we are saying that we need to keep supporting TLSv1.0 -- which 
is heavily deprecated.  Also note that the first OpenSSL version with 
support for TLSv1.2 shipped on March 14, 2012.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  6. Re: should libpq also require TLSv1.2 by default?

    Bruce Momjian <bruce@momjian.us> — 2020-06-24T18:22:10Z

    On Wed, Jun 24, 2020 at 07:57:31PM +0200, Peter Eisentraut wrote:
> On 2020-06-24 10:33, Daniel Gustafsson wrote:
> > > In PG13, we raised the server-side default of ssl_min_protocol_version to TLSv1.2.  We also added a connection setting named ssl_min_protocol_version to libpq.  But AFAICT, the default value of the libpq setting is empty, so any protocol version will be accepted.  Is this what we wanted?  Should we raise the default in libpq as well?
> > 
> > This was discussed [0] when the connection settings were introduced, and the
> > concensus was to leave them alone [1] to allow for example a new pg_dump to
> > work against an old server.  Re-reading the thread I think the argument still
> > holds, but I was about to respond "yes, let's do this" before refreshing my
> > memory.  Perhaps we should add a comment explaining this along the lines of the
> > attached?
> > 
> > [0] https://www.postgresql.org/message-id/157800160408.1198.1714906047977693148.pgcf%40coridan.postgresql.org
> > [1] https://www.postgresql.org/message-id/31993.1578321474%40sss.pgh.pa.us
> 
> ISTM that these discussions went through the same questions and arguments
> that were made regarding the server-side change but arrived at a different
> conclusion.  So I suggest to reconsider this so that we don't ship with
> contradictory results.
> 
> That doesn't necessarily mean that we have to make a change, but we should
> make sure our rationale is sound.
> 
> Note that all OpenSSL versions that do not support TLSv1.2 also do not
> support TLSv1.1.  So by saying, in effect, that TLSv1.2 is too new to
> require, we are saying that we need to keep supporting TLSv1.0 -- which is
> heavily deprecated.  Also note that the first OpenSSL version with support
> for TLSv1.2 shipped on March 14, 2012.

I do think mismatched SSL requirements between client and server is
confusing, though I can see the back-version pg_dump being an issue. 
Maybe a clear error message would help here.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

  The usefulness of a cup is in its emptiness, Bruce Lee

  7. Re: should libpq also require TLSv1.2 by default?

    Daniel Gustafsson <daniel@yesql.se> — 2020-06-24T22:30:03Z

    > On 24 Jun 2020, at 19:57, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:
> 
> On 2020-06-24 10:33, Daniel Gustafsson wrote:
>>> In PG13, we raised the server-side default of ssl_min_protocol_version to TLSv1.2.  We also added a connection setting named ssl_min_protocol_version to libpq.  But AFAICT, the default value of the libpq setting is empty, so any protocol version will be accepted.  Is this what we wanted?  Should we raise the default in libpq as well?
>> This was discussed [0] when the connection settings were introduced, and the
>> concensus was to leave them alone [1] to allow for example a new pg_dump to
>> work against an old server.  Re-reading the thread I think the argument still
>> holds, but I was about to respond "yes, let's do this" before refreshing my
>> memory.  Perhaps we should add a comment explaining this along the lines of the
>> attached?
>> [0] https://www.postgresql.org/message-id/157800160408.1198.1714906047977693148.pgcf%40coridan.postgresql.org
>> [1] https://www.postgresql.org/message-id/31993.1578321474%40sss.pgh.pa.us
> 
> ISTM that these discussions went through the same questions and arguments that were made regarding the server-side change but arrived at a different conclusion.  So I suggest to reconsider this so that we don't ship with contradictory results.

I don't think anyone argues against safe defaults for communication between
upgraded clients and upgraded servers.  That being said; out of the box, an
upgraded client *will* use TLSv1.2 when connecting to a upgraded server due to
the server defaults requirements (assuming the server hasn't been reconfigured
with a lower TLS version, but since we're talking defaults we have to assume
that).

The problem comes when an updated client needs to talk to an old server which
hasn't been upgraded and which use an older OpenSSL version.  If we set TLSv1.2
as the minimum client version, the user will have to amend a connection string
which used to work when talking to a server which hasn't changed.  If we don't
raise the default, a user to wants nothing lower than TLSv1.2 will have to
amend the connection string.

> That doesn't necessarily mean that we have to make a change, but we should make sure our rationale is sound.

Totally agree.  I think I, FWIW, still vote for keeping it at 1.0 to not break
connections to old servers, since upgraded/new servers will enforce 1.2
anyways.

As mentioned elsewhere in the thread, maybe this is also something which can be
done more easily if we improve the error reporting?  Right now it's fairly
cryptic IMO.

> Note that all OpenSSL versions that do not support TLSv1.2 also do not support TLSv1.1.  So by saying, in effect, that TLSv1.2 is too new to require, we are saying that we need to keep supporting TLSv1.0 -- which is heavily deprecated.  Also note that the first OpenSSL version with support for TLSv1.2 shipped on March 14, 2012.

Correct, this being the 1.0.1 release which is referred to.

cheers ./daniel

  8. Re: should libpq also require TLSv1.2 by default?

    Michael Paquier <michael@paquier.xyz> — 2020-06-25T01:50:26Z

    On Thu, Jun 25, 2020 at 12:30:03AM +0200, Daniel Gustafsson wrote:
> I don't think anyone argues against safe defaults for communication between
> upgraded clients and upgraded servers.  That being said; out of the box, an
> upgraded client *will* use TLSv1.2 when connecting to a upgraded server due to
> the server defaults requirements (assuming the server hasn't been reconfigured
> with a lower TLS version, but since we're talking defaults we have to assume
> that).

My take here is to let things as they are for libpq.  pg_dump is a very
good argument, because we allow backward compatibility with a newer
version of the tool, not upward compatibility.

> The problem comes when an updated client needs to talk to an old server which
> hasn't been upgraded and which use an older OpenSSL version.  If we set TLSv1.2
> as the minimum client version, the user will have to amend a connection string
> which used to work when talking to a server which hasn't changed.  If we don't
> raise the default, a user to wants nothing lower than TLSv1.2 will have to
> amend the connection string.

Yeah, and I would not be surprised to see cases where people complain
to us about that when attempting to reach one of their old boxes,
breaking some stuff they have been relying on for years by forcing the
addition of a tls_min_server_protocol in the connection string.  It is
a more important step that we enforce TLSv1.2 on the server side
actually, and libpq just follows up automatically with that.

> As mentioned elsewhere in the thread, maybe this is also something which can be
> done more easily if we improve the error reporting?  Right now it's fairly
> cryptic IMO.

This part may be tricky to get right I think, because the error comes
directly from OpenSSL when negotiating the protocol used between the
client and the server, like "no protocols available" or such.
--
Michael

  9. Re: should libpq also require TLSv1.2 by default?

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-06-25T02:50:39Z

    Michael Paquier <michael@paquier.xyz> writes:
> On Thu, Jun 25, 2020 at 12:30:03AM +0200, Daniel Gustafsson wrote:
>> As mentioned elsewhere in the thread, maybe this is also something which can be
>> done more easily if we improve the error reporting?  Right now it's fairly
>> cryptic IMO.

> This part may be tricky to get right I think, because the error comes
> directly from OpenSSL when negotiating the protocol used between the
> client and the server, like "no protocols available" or such.

Can we do something comparable to the backend's HINT protocol, where
we add on a comment that's only mostly-likely to be right?

			regards, tom lane

  10. Re: should libpq also require TLSv1.2 by default?

    Michael Paquier <michael@paquier.xyz> — 2020-06-25T04:41:24Z

    On Wed, Jun 24, 2020 at 10:50:39PM -0400, Tom Lane wrote:
> Can we do something comparable to the backend's HINT protocol, where
> we add on a comment that's only mostly-likely to be right?

OpenSSL publishes its error codes as of openssl/sslerr.h, and it looks
like the two error codes we would need to worry about are
SSL_R_UNSUPPORTED_PROTOCOL and SSL_R_NO_PROTOCOLS_AVAILABLE.  So we
could for example amend open_client_SSL() when negotiating the SSL
connection in libpq with error messages or hints that help better than
the current state of things, but that also means an extra maintenance
on our side to make sure that we keep in sync with new error codes
coming from the OpenSSL world.
--
Michael

  11. Re: should libpq also require TLSv1.2 by default?

    Robert Haas <robertmhaas@gmail.com> — 2020-06-25T19:41:55Z

    On Wed, Jun 24, 2020 at 9:50 PM Michael Paquier <michael@paquier.xyz> wrote:
> Yeah, and I would not be surprised to see cases where people complain
> to us about that when attempting to reach one of their old boxes,
> breaking some stuff they have been relying on for years by forcing the
> addition of a tls_min_server_protocol in the connection string.  It is
> a more important step that we enforce TLSv1.2 on the server side
> actually, and libpq just follows up automatically with that.

I wonder how much of a problem this really is. A few quick Google
searches suggests that support for TLSv1.2 was added to OpenSSL in
v1.0.1, which was released in March 2012. If packagers adopted that
version for the following PostgreSQL release, they would have had
TLSv1.2 support from PostgreSQL 9.2 onward. Some people may have taken
longer to adopt it, but even if they waited a year or two, all
versions that they built with older OpenSSL versions would now be out
of support. It doesn't seem that likely that there are going to be
that many people using pg_dump to upgrade directly from PostgreSQL 9.2
-- or even 9.4 -- to PostgreSQL 13. Skipping six or eight major
versions in a single upgrade is a little unusual, in my experience.
And even if someone does want to do that, we haven't broken it; it'll
still work fine if they are connecting through a UNIX socket, and if
not, they can disable SSL or just specify that they're OK with an
older protocol version. That doesn't seem like a big deal, especially
if we can adopt Tom's suggestion of giving them a warning about what
went wrong.

Let's also consider the other side of this argument, which is that a
decent number of PostgreSQL users are operating in environments where
they are required for regulatory compliance to prohibit the use of
TLSv1.0 and TLSv1.1. Those people will be happy if that is the default
on both the client and the server side by default. They will probably
be somewhat happy anyway, because now we have an option for it, which
we didn't before. But they'll be more happy if it's the default. Now,
we can't please everybody here. Is it more important to please people
who would like insecure TLS versions disabled by default, or to please
people who want to use insecure TLS versions to back up old servers?
Seems debatable. Based on my own experience, I'd guess there are more
users who want to avoid insecure TLS versions than there are users who
want to use them to back up very old servers, so I'd tentatively favor
changing the default. However, I don't know whether my experiences are
representative.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  12. Re: should libpq also require TLSv1.2 by default?

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-06-25T19:57:00Z

    Robert Haas <robertmhaas@gmail.com> writes:
> I wonder how much of a problem this really is.

Yeah.  I find Robert's points about that pretty persuasive: by now
needing to connect to a server without TLSv1.2 support, *and* needing to
do so with SSL on, ought to be a tiny niche use case (much smaller than
the number of people who would like a more secure default).  If we can
make the error message about this be reasonably clear then I don't have
an objection to changing libpq's default.

			regards, tom lane

  13. Re: should libpq also require TLSv1.2 by default?

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-06-25T22:44:05Z

    Daniel Gustafsson <daniel@yesql.se> writes:
>> On 24 Jun 2020, at 10:46, Magnus Hagander <magnus@hagander.net> wrote:
>> It might also be worth noting that it's not really "any protocol version", it means it will be "whatever the openssl configuration says", I think? For example, debian buster sets:
>> 
>> [system_default_sect]
>> MinProtocol = TLSv1.2
>> 
>> Which I believe means that if your libpq app is running on debian buster, it will be min v1.2 already

> Correct, that being said I'm not sure how common it is for distributions to set
> a default protocol version.  The macOS versions I have handy doesn't enforce a
> default version, nor does Ubuntu 20.04, FreeBSD 12 or OpenBSD 6.5 AFAICT.

Yeah, this.  I experimented with connecting current libpq to a 9.2-vintage
server I'd built with openssl 0.9.8x, and was surprised to find I couldn't
do so unless I explicitly set "ssl_min_protocol_version=tlsv1".  After
some digging I verified that that's because RHEL8's openssl.cnf sets

MinProtocol = TLSv1.2
MaxProtocol = TLSv1.3

Interestingly, Fedora 32 is laxer:

MinProtocol = TLSv1
MaxProtocol = TLSv1.3

I concur with Daniel's finding that current macOS and FreeBSD don't
enforce anything in this area.  Nonetheless, for a pretty significant
fraction of the world, our claim that the default is to not enforce
any minimum protocol version is a lie.

My feeling now is that we'd be better off defaulting
ssl_min_protocol_version to something nonempty, just to make this
behavior platform-independent.  We certainly can't leave the docs
as they are.

Also, I confirm that the failure looks like

$ psql -h ... -d "dbname=postgres sslmode=require"
psql: error: could not connect to server: SSL error: unsupported protocol

While that's not *that* awful, if you realize that "protocol" means
TLS version, many people probably won't without a hint.  It does not
help any that the message doesn't mention either the offered TLS version
or the version limits being enforced.  I'm not sure we can do anything
about the former, but reducing the number of variables affecting the
latter seems like a smart idea.

BTW, the server-side report of the problem looks like

LOG:  could not accept SSL connection: wrong version number

			regards, tom lane

  14. Re: should libpq also require TLSv1.2 by default?

    Bruce Momjian <bruce@momjian.us> — 2020-06-25T22:55:59Z

    On Thu, Jun 25, 2020 at 06:44:05PM -0400, Tom Lane wrote:
> BTW, the server-side report of the problem looks like
> 
> LOG:  could not accept SSL connection: wrong version number

I like that one.  ;-)

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

  The usefulness of a cup is in its emptiness, Bruce Lee

  15. Re: should libpq also require TLSv1.2 by default?

    Daniel Gustafsson <daniel@yesql.se> — 2020-06-26T12:33:04Z

    > On 26 Jun 2020, at 00:44, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> My feeling now is that we'd be better off defaulting
> ssl_min_protocol_version to something nonempty, just to make this
> behavior platform-independent.  We certainly can't leave the docs
> as they are.

Yeah, given the concensus in this thread and your findings I think we should
default to TLSv1.2 as originally proposed.

I still think there will be instances of existing connections to old servers
that will all of a sudden break, but it's probably true that it's not a common
setup.  Optimizing for the majority and helping the minority with documentation
is IMO the winning move.

> Also, I confirm that the failure looks like
> 
> $ psql -h ... -d "dbname=postgres sslmode=require"
> psql: error: could not connect to server: SSL error: unsupported protocol
> 
> While that's not *that* awful, if you realize that "protocol" means
> TLS version, many people probably won't without a hint.  It does not
> help any that the message doesn't mention either the offered TLS version
> or the version limits being enforced.  I'm not sure we can do anything
> about the former, but reducing the number of variables affecting the
> latter seems like a smart idea.

+1

> BTW, the server-side report of the problem looks like
> 
> LOG:  could not accept SSL connection: wrong version number

I can totally see some thinking that it's the psql version at client side which
is referred to and not the TLS protocol version.  Perhaps we should add a hint
there as well?

cheers ./daniel

  16. Re: should libpq also require TLSv1.2 by default?

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-06-26T13:19:43Z

    Daniel Gustafsson <daniel@yesql.se> writes:
>> On 26 Jun 2020, at 00:44, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> BTW, the server-side report of the problem looks like
>> LOG:  could not accept SSL connection: wrong version number

> I can totally see some thinking that it's the psql version at client side which
> is referred to and not the TLS protocol version.  Perhaps we should add a hint
> there as well?

Not sure.  We can't fix it in the case we're mainly concerned about,
namely an out-of-support server version.  At the same time, it's certainly
true that "version number" is way too under-specified in this context.
Maybe improving this against the day that TLSv2 exists would be smart.

			regards, tom lane

  17. Re: should libpq also require TLSv1.2 by default?

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-06-26T15:36:42Z

    Here is a quick attempt at getting libpq and the server to report
suitable hint messages about SSL version problems.

The main thing I don't like about this as formulated is that I hard-wired
knowledge of the minimum and maximum SSL versions into the hint messages.
That's clearly not very maintainable, but it seems really hard to keep the
messages readable/useful without giving concrete version numbers.
Anybdy have a better idea?  Is there a reasonably direct way to ask
OpenSSL what its min and max versions are?

			regards, tom lane

  18. Re: should libpq also require TLSv1.2 by default?

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-06-26T20:22:37Z

    I wrote:
> Anybdy have a better idea?  Is there a reasonably direct way to ask
> OpenSSL what its min and max versions are?

After some digging, there apparently is not.  At first glance it would
seem that SSL_get_min_proto_version/SSL_get_max_proto_version should
help, but in reality they're just blindingly useless, because they
return zero in most cases of interest.  And when they don't return zero
they might give us a code that we don't recognize, so there's no future
proofing to be had from using them.  Plus they don't exist before
openssl 1.1.1.

It looks like, when they exist, we could use them to discover any
restrictions openssl.cnf has set on the allowed protocol versions ...
but I'm not really convinced that's worth the trouble.  If we up the
libpq default to TLSv1.2 then there probably won't be any real-world
cases where openssl.cnf affects our results.

So I propose the attached.  The hack in openssl.h to guess the
min/max supported versions is certainly nothing but a hack;
but I see no way to do better.

			regards, tom lane

  19. Re: should libpq also require TLSv1.2 by default?

    Daniel Gustafsson <daniel@yesql.se> — 2020-06-26T21:18:58Z

    > On 26 Jun 2020, at 22:22, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> I wrote:
>> Anybdy have a better idea?  Is there a reasonably direct way to ask
>> OpenSSL what its min and max versions are?
> 
> After some digging, there apparently is not.

AFAIK everyone either #ifdef around the TLS1_x_VERSION macros or the OpenSSL
versioning and use hardcoded knowledge based on that.  The latter is fairly
shaky since configure options can disable protocols.  At least in past
versions, the validation for protocol range in OpenSSL ssl_lib was doing pretty
much that too.

> So I propose the attached.

SSL_R_UNKNOWN_PROTOCOL seem to covers cases when someone manages to perform
something which OpenSSL believes is a broken SSLv2 connection, but their own
client-level code use it to refer to SSL as well as TLS.  Maybe it's worth
adding as a belts and suspenders type thing?

I've only had a chance to read the patches, but they read pretty much just like
I had in mind that we could do this. +1 on both patches from an eye-ball POV.

Is this targeting v13 or v14?  In case of the former, the release notes entry
for raising the default minimum version should perhaps be tweaked as it now
just refers to the GUC which is a tad misleading.

> The hack in openssl.h to guess the
> min/max supported versions is certainly nothing but a hack;
> but I see no way to do better.

If anything it might useful to document in the comment that we're only
concerned with TLS versions, SSL2/3 are disabled in the library initialization.

cheers ./daniel

  20. Re: should libpq also require TLSv1.2 by default?

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-06-26T21:27:44Z

    Daniel Gustafsson <daniel@yesql.se> writes:
> SSL_R_UNKNOWN_PROTOCOL seem to covers cases when someone manages to perform
> something which OpenSSL believes is a broken SSLv2 connection, but their own
> client-level code use it to refer to SSL as well as TLS.  Maybe it's worth
> adding as a belts and suspenders type thing?

No objection on my part.

> Is this targeting v13 or v14?  In case of the former, the release notes entry
> for raising the default minimum version should perhaps be tweaked as it now
> just refers to the GUC which is a tad misleading.

I think Peter is proposing that we change this in v13.  I didn't look
at the release notes; usually we cover this sort of thing in-bulk
when we update the release notes later in beta.

> If anything it might useful to document in the comment that we're only
> concerned with TLS versions, SSL2/3 are disabled in the library initialization.

Good point.

			regards, tom lane

  21. Re: should libpq also require TLSv1.2 by default?

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-06-27T16:55:21Z

    I wrote:
> Daniel Gustafsson <daniel@yesql.se> writes:
>> SSL_R_UNKNOWN_PROTOCOL seem to covers cases when someone manages to perform
>> something which OpenSSL believes is a broken SSLv2 connection, but their own
>> client-level code use it to refer to SSL as well as TLS.  Maybe it's worth
>> adding as a belts and suspenders type thing?

> No objection on my part.

>> If anything it might useful to document in the comment that we're only
>> concerned with TLS versions, SSL2/3 are disabled in the library initialization.

> Good point.

Pushed with those corrections.  I also rewrote the comment about which
error codes we'd seen in practice, after realizing that one of my tests
had been affected by the presence of "MinProtocol = TLSv1.2" in
RHEL8's openssl.cnf (causing a max setting less than that to be a local
configuration error, not something the server had rejected).

			regards, tom lane