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
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() |