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