|From:||Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>|
|Cc:||peter(dot)eisentraut(at)2ndquadrant(dot)com, LouPicciano(at)comcast(dot)net, tgl(at)sss(dot)pgh(dot)pa(dot)us, bruce(at)momjian(dot)us, pgsql-hackers(at)postgresql(dot)org|
|Subject:||Re: pg_stat_ssl additions|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
At Tue, 29 Jan 2019 13:50:17 +0900, Michael Paquier <michael(at)paquier(dot)xyz> wrote in <20190129045017(dot)GC3121(at)paquier(dot)xyz>
> On Tue, Jan 29, 2019 at 12:18:29PM +0900, Kyotaro HORIGUCHI wrote:
> > At Mon, 28 Jan 2019 14:53:43 +0100, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote in <24783370-5acd-e0f3-8eb7-7f42ff2a026d(at)2ndquadrant(dot)com>
> >> This is strange. The tests work for me, and also on the cfbot. The
> > Agreed. It seemed so also to me.
> The tests look sane to me for what it's worth.
> > When initializing SSL context, it picks up the root certificate
> > from my home directory, not in test installation and I had one
> > there. It is not based on $HOME but pwent so it is unchangeable
> > (and it is the right design for the purpose).
> Oops. I agree that the tests ought to be as much isolated as
> possible, without optionally fetching things depending on the user
> > +# ssl-related properties may defautly set to the files in the users'
> > +# environment. Explicitly provide them a value so that they don't
> > +# point a valid file accidentially. Some other common properties are
> > +# set here together.
> > +# Attach this at the head of $common_connstr.
> > +my $def_connstr = "user=ssltestuser dbname=trustdb sslcert=invalid sslkey=invalid sslrootcert=invalid sslcrl=invalid ";
> > +
> Maybe it is better to just put that in test_connect_ok and
> test_connect_fails for only the SSL parameters? I don't see much
> point in enforcing dbname and user.
Hmm. It is the first thing I did for the issue. Anyway..
Of course dbname and user are not necessary to fix it. Ok, I
attached two patches. The first applies on the current master and
prevent psql from loading ssl-related files from out of testing
environment. The second applies on the 0002 patch and prevent the
patch from loading a wrong sslrootcert.
> > # The server should not accept non-SSL connections.
> > test_connect_fails(
> > @@ -185,7 +192,7 @@ test_connect_ok(
> > # Check that connecting with verify-full fails, when the hostname doesn't
> > # match the hostname in the server's certificate.
> > $common_connstr =
> > - "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
> > + $def_connstr."sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
> I think this is bad, inconsistent style. Adding the variable within
> the quoted section is fine, as much as moving SERVERHOSTADDR out. But
> mixing styles is not.
I Understood. Thanks for the suggestion.
NTT Open Source Software Center
|Next Message||Michael Paquier||2019-01-29 12:36:13||Re: pg_basebackup, walreceiver and wal_sender_timeout|
|Previous Message||Petr Jelinek||2019-01-29 12:19:52||Re: Why does execReplication.c lock tuples?|