Re: Out-of-tree certificate interferes ssltest

> On 17 Mar 2022, at 09:05, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> wrote:
>
> At Thu, 17 Mar 2022 16:22:14 +0900, Michael Paquier <michael(at)paquier(dot)xyz> wrote in
>> On Thu, Mar 17, 2022 at 02:59:26PM +0900, Michael Paquier wrote:
>>> In both cases, enforcing sslcrl to a value of "invalid" interferes
>>> with the failure scenario we expect from sslcrldir. It is possible to
>>> bypass that with something like the attached, but that's a kind of
>>> ugly hack. Another alternative would be to drop those two tests, and
>>> I am not sure how much we care about these two negative scenarios.

I really don't think we should drop tests based on these premises, at least not
until it's raised as a problem/inconvenience but more hackers. I would prefer
to instead extend the error message with hints that ~/.postgresql contents
could've affected test outcome. But, as the v2 patch handles it this is mostly
academic at this point.

>> Actually, there is a trick I have recalled here: we can enforce sslcrl
>> to an empty value in the connection string after the default. This
>> still ensures that the test won't pick up any SSL data from the local
>> environment and avoids any interferences of OpenSSL's
>> X509_STORE_load_locations(). This gives a much simpler and cleaner
>> patch.

> Ah! I didn't have a thought that we can specify the same parameter
> twice. It looks like clean and robust. $default_ssl_connstr contains
> all required options in PQconninfoOptions[].

+1

One small concern though. This hunk:

+my $default_ssl_connstr = "sslkey=invalid sslcert=invalid sslrootcert=invalid sslcrl=invalid sslcrldir=invalid";
+
$common_connstr =
- "user=ssltestuser dbname=trustdb sslcert=invalid hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
+ "$default_ssl_connstr user=ssltestuser dbname=trustdb hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";

..together with the following changes along the lines of:

- "$common_connstr sslrootcert=invalid sslmode=require",
+ "$common_connstr sslmode=require",

..is making it fairly hard to read the test and visualize what the connection
string is and how the test should behave. I don't have a better idea off the
top of my head right now, but I think this is an area to revisit and improve
on.

--
Daniel Gustafsson https://vmware.com/