Re: Setting min/max TLS protocol in clientside libpq

From: cary huang <hcary328(at)gmail(dot)com>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Daniel Gustafsson <daniel(at)yesql(dot)se>
Subject: Re: Setting min/max TLS protocol in clientside libpq
Date: 2020-01-02 21:46:44
Message-ID: 157800160408.1198.1714906047977693148.pgcf@coridan.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Hello

I have applied the patch and did some basic testing with various combination of min and max TLS protocol versions. Overall the functionality works and the chosen TLS protocol aligns with the min/max TLS protocol settings on the PG server side.

I agree with Arthur that it makes sense to check the validity of "conn->sslmaxprotocolversion" first before checking if it is larger than "conn->sslminprotocolversion"

A small suggestion here. I see that PG server defaults TLS min version to be TLSv1.2 and max version to none. So by default the server accepts TLSv1.2 and above. I think on the client side, it also makes sense to have the same defaults as the server. In the patch, if the client does not supply "sslminprotocolversion", it will run to "else" statement and sets TLS min version to "INT_MIN", which is a huge negative number and of course openssl won't set it. I think this else statement can be enhanced a little to set "sslminprotocolversion" to TLSv1.2 by default to match the server and provide some warning message that TLS minimum has defaulted to TLSv1.2.

Cary
HighGo Software Canada

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2020-01-02 22:34:55 Re: Commit fest manager for 2020-01
Previous Message Peter Geoghegan 2020-01-02 21:41:25 Re: _bt_delitems_delete() should use XLogRegisterBufData(), not XLogRegisterData()