Hello,
On 2019/12/04 2:37, Daniel Gustafsson wrote:
The attached patch implements two new connection string variables for minimum
and maximum TLS protocol version, mimicking how it's done in the backend. This
does duplicate a bit of code from be-secure-openssl.c to cope with older
versions of OpenSSL, but it seemed a too trivial duplication to create
common/openssl.c (but others might disagree).
I've looked at the patch and I have a couple comments.
+ if (ssl_max_ver < ssl_min_ver)
+ {
+ printfPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("invalid
maximum SSL version specified, must be higher than minimum SSL version: %s\n"),
+
conn->sslmaxprotocolversion);
+ return -1;
+ }
+
+ if (ssl_max_ver == -1)
+ {
+ printfPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("invalid
maximum SSL version specified: %s\n"),
+
conn->sslmaxprotocolversion);
+ return -1;
+ }
I think we should raise the error "invalid maximum SSL version
specified" earlier. If ssl_protocol_version_to_openssl() returns -1 and
ssl_min_ver is valid we never reach the condition "ssl_max_ver == -1".
Also it might confuse users to get the error "invalid maximum SSL
version specified, must be higher than minimum SSL version" instead of
former one.
Secondly I think the error "invalid maximum SSL version specified"
itself might confuse users, in the case if the input is good but a build
doesn't support desired version. So I think it is better to do two
checks here: check for a correct input and check if a build supports it.
In the second case we may raise "SSL version %s not supported by this
build". It is actually like backend does: guc.c checks for correct input
using ssl_protocol_versions_info and ssl_protocol_version_to_openssl()
checks if a build supports the version.
--
Arthur
