Thread: settings to control SSL/TLS protocol version
There have been some requests to be able to select the TLS versions
PostgreSQL is using. We currently only hardcode that SSLv2 and SSLv3
are disabled, but there is also some interest now in disabling TLSv1.0
and TLSv1.1. Also, I've had some issues in some combinations with the
new TLSv1.3, so there is perhaps also some use for disabling at the top end.
Attached is a patch that implements this. For example:
ssl_min_protocol_version = 'TLSv1'
ssl_max_protocol_version = 'any'
For reference, here is similar functionality implemented elsewhere:
https://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_protocols
https://httpd.apache.org/docs/2.4/mod/mod_ssl.html#sslprotocol
Unlike those two, which offer a list of protocols to use, I have gone
with min and max settings. I think that is easier to use, and it also
maps better to the newer OpenSSL API (SSL_CTX_set_min_proto_version()
etc.). The older SSL_CTX_set_options()-based approach is deprecated and
has some very weird behaviors that would make it complicated to use for
anything more than a min/max.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment
> On 1 Oct 2018, at 22:21, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote: > > There have been some requests to be able to select the TLS versions > PostgreSQL is using. We currently only hardcode that SSLv2 and SSLv3 > are disabled, but there is also some interest now in disabling TLSv1.0 > and TLSv1.1. Also, I've had some issues in some combinations with the > new TLSv1.3, so there is perhaps also some use for disabling at the top end. > > Attached is a patch that implements this. For example: > > ssl_min_protocol_version = 'TLSv1' > ssl_max_protocol_version = ‘any' I don’t think ‘any’ is a clear name for a setting which means “the highest supported version”. How about ‘max_supported’ or something similar? > For reference, here is similar functionality implemented elsewhere: > > https://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_protocols > https://httpd.apache.org/docs/2.4/mod/mod_ssl.html#sslprotocol > > Unlike those two, which offer a list of protocols to use, I have gone > with min and max settings. FWIW, libcurl also supports a min/max approach with CURLOPT_SSLVERSION: https://curl.haxx.se/libcurl/c/CURLOPT_SSLVERSION.html +1 for using a min/max approach for setting the version, and it should be trivial to add support for in the pending GnuTLS and Secure Transport patches. cheers ./daniel
On 01/10/2018 23:30, Daniel Gustafsson wrote: >> ssl_min_protocol_version = 'TLSv1' >> ssl_max_protocol_version = ‘any' > > I don’t think ‘any’ is a clear name for a setting which means “the highest > supported version”. How about ‘max_supported’ or something similar? I can see the argument for an alternative, but your suggestion is a mouthful. > +1 for using a min/max approach for setting the version, and it should be > trivial to add support for in the pending GnuTLS and Secure Transport patches. AFAICT, in GnuTLS this is done via the "priorities" setting that also sets the ciphers. There is no separate API for just the TLS version. It would be interesting to see how Secure Transport can do it. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
> On 2 Oct 2018, at 14:23, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote: > > On 01/10/2018 23:30, Daniel Gustafsson wrote: >>> ssl_min_protocol_version = 'TLSv1' >>> ssl_max_protocol_version = ‘any' >> >> I don’t think ‘any’ is a clear name for a setting which means “the highest >> supported version”. How about ‘max_supported’ or something similar? > > I can see the argument for an alternative, but your suggestion is a > mouthful. Agreed, but I can’t think of a better wording. Perhaps just ‘tls_max’? >> +1 for using a min/max approach for setting the version, and it should be >> trivial to add support for in the pending GnuTLS and Secure Transport patches. > > AFAICT, in GnuTLS this is done via the "priorities" setting that also > sets the ciphers. There is no separate API for just the TLS version. > It would be interesting to see how Secure Transport can do it. Secure Transport has a fairly neat API for this, SSLSetProtocolVersionMax() and SSLSetProtocolVersionMin() (available since Lion). cheers ./daniel
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: not tested Documentation: tested, passed I've reviewed the patch and here are my comments. The feature seems useful a lot of application servers are implementing minimal TLS protocol versions. I don't see a way to restrict libpq to only connect with certain protocol versions. Maybe that is a separate patch but itwould make this feature harder to test in the future. I tested with a server configured to via the options to only TLS1.3 and clients without TLSv1.3 support and confirmed thatI couldn't connect with SSL. This is fine I tested with options to restrict the max version to TLSv1 and verified that the clients connected with TLSv1. This is fine I tested with a min protocol version greater than the max. The server started up (Do we want this to be an warning on startup?)but I wasn't able to connect with SSL. The following was in the server log could not accept SSL connection: unknown protocol I tested with a max protocol version set to any. This is fine. I tested putting TLSv1.3 in the config file when my openssl library did not support 1.3. This is fine. I am updating the patch status to ready for committer. The new status of this patch is: Ready for Committer
On Mon, Oct 1, 2018 at 4:21 PM Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote: > There have been some requests to be able to select the TLS versions > PostgreSQL is using. We currently only hardcode that SSLv2 and SSLv3 > are disabled, but there is also some interest now in disabling TLSv1.0 > and TLSv1.1. Also, I've had some issues in some combinations with the > new TLSv1.3, so there is perhaps also some use for disabling at the top end. > > Attached is a patch that implements this. For example: > > ssl_min_protocol_version = 'TLSv1' > ssl_max_protocol_version = 'any' +1. Maybe it would make sense to spell 'any' as the empty string. Intuitively, it makes more sense to me to think about there being no maximum than to think about the maximum being anything. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Mon, Nov 05, 2018 at 03:01:58PM -0500, Robert Haas wrote: > On Mon, Oct 1, 2018 at 4:21 PM Peter Eisentraut > <peter.eisentraut@2ndquadrant.com> wrote: > > There have been some requests to be able to select the TLS versions > > PostgreSQL is using. We currently only hardcode that SSLv2 and SSLv3 > > are disabled, but there is also some interest now in disabling TLSv1.0 > > and TLSv1.1. Also, I've had some issues in some combinations with the > > new TLSv1.3, so there is perhaps also some use for disabling at the top end. > > > > Attached is a patch that implements this. For example: > > > > ssl_min_protocol_version = 'TLSv1' > > ssl_max_protocol_version = 'any' > > +1. Maybe it would make sense to spell 'any' as the empty string. > Intuitively, it makes more sense to me to think about there being no > maximum than to think about the maximum being anything. ..and now, I'm finally beginning to see the reasoning that led Oracle to conflate NULL and empty string. Best, David. -- David Fetter <david(at)fetter(dot)org> http://fetter.org/ Phone: +1 415 235 3778 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
On Monday, November 5, 2018, David Fetter <david@fetter.org> wrote:
On Mon, Nov 05, 2018 at 03:01:58PM -0500, Robert Haas wrote:
> On Mon, Oct 1, 2018 at 4:21 PM Peter Eisentraut
> <peter.eisentraut@2ndquadrant.com> wrote:
> >
> > Attached is a patch that implements this. For example:
> >
> > ssl_min_protocol_version = 'TLSv1'
> > ssl_max_protocol_version = 'any'
>
> +1. Maybe it would make sense to spell 'any' as the empty string.
> Intuitively, it makes more sense to me to think about there being no
> maximum than to think about the maximum being anything.
..and now, I'm finally beginning to see the reasoning that led Oracle
to conflate NULL and empty string.
Seems like a situation for ‘n/a’ though maybe that’s too English-centric...
I’m a bit uncertain about the mix of name and number in something that purports to be a version and thus should be numeric only. SSLv3 and TLSv2 would not be comparable in terms of min/max...but I haven’t delved deeply into the feature either.
David J.
On Mon, Nov 05, 2018 at 03:01:58PM -0500, Robert Haas wrote: > +1. Maybe it would make sense to spell 'any' as the empty string. > Intuitively, it makes more sense to me to think about there being no > maximum than to think about the maximum being anything. I have looked at the code a bit yesterday and the implementation as well as how things are handled with OpenSSL looked sane to me. The suggestion of using an empty string as the default instead of 'any' also makes sense per your argument -- Michael
Attachment
On 04/11/2018 04:24, Steve Singer wrote: > The feature seems useful a lot of application servers are implementing minimal TLS protocol versions. > I don't see a way to restrict libpq to only connect with certain protocol versions. Maybe that is a separate patch butit would make this feature harder to test in the future. Client-side support could be separate patch, yes. It seems less important. > I am updating the patch status to ready for committer. > > The new status of this patch is: Ready for Committer Committed with the change 'any' -> '' that was discussed elsewhere in the thread. Thanks. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services