Re: [HACKERS] Broken SSL tests in master

2016-12-14 Thread Michael Paquier
On Tue, Dec 13, 2016 at 10:07 PM, Robert Haas wrote: > On Mon, Dec 12, 2016 at 1:34 AM, Heikki Linnakangas wrote: >> On 12/01/2016 09:45 PM, Andres Freund wrote: >>> >>> And nobody has added a buildfarm module to run it manually on their >>> servers either

Re: [HACKERS] Broken SSL tests in master

2016-12-13 Thread Robert Haas
On Mon, Dec 12, 2016 at 1:34 AM, Heikki Linnakangas wrote: > On 12/01/2016 09:45 PM, Andres Freund wrote: >> >> And nobody has added a buildfarm module to run it manually on their >> servers either :( > > I just added a module to run "make -C src/test/ssl check" in chipmunk. So

Re: [HACKERS] Broken SSL tests in master

2016-12-11 Thread Heikki Linnakangas
On 12/01/2016 09:45 PM, Andres Freund wrote: And nobody has added a buildfarm module to run it manually on their servers either :( I just added a module to run "make -C src/test/ssl check" in chipmunk. So at least that's covered now.

Re: [HACKERS] Broken SSL tests in master

2016-12-05 Thread Mithun Cy
On Fri, Dec 2, 2016 at 9:18 AM, Mithun Cy wrote: >On Fri, Dec 2, 2016 at 2:26 AM, Robert Haas wrote: >>Yeah, we should change that. Are you going to write a patch? > Thanks, will work on this will produce a patch to patch to fix. This is

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Robert Haas
On Thu, Dec 1, 2016 at 9:31 PM, Michael Paquier wrote: > On Thu, Dec 01, 2016 at 03:17:34PM -0500, Robert Haas wrote: >> It might be that (as suggested downthread) we should consider >> supporting multiple IPs in the hostaddr string as well, but that >> requires some

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Mithun Cy
On Fri, Dec 2, 2016 at 2:26 AM, Robert Haas wrote: >Yeah, we should change that. Are you going to write a patch? Thanks, will work on this will produce a patch to patch to fix. -- Thanks and Regards Mithun C Y EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Michael Paquier
On Thu, Dec 01, 2016 at 03:17:34PM -0500, Robert Haas wrote: > It might be that (as suggested downthread) we should consider > supporting multiple IPs in the hostaddr string as well, but that > requires some thought. For example, what happens if, for example, the > host and hostaddr lists are of

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Peter Eisentraut
On 12/1/16 4:53 PM, Michael Paquier wrote: > The reason why the SSL test suite is not in check-world is that SSL > cannot be used with unix domain sockets, making it unfit in shared > environments. If that is it, that could be changed. -- Peter Eisentraut

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Michael Paquier
On Fri, Dec 2, 2016 at 10:26 AM, Tom Lane wrote: > Michael Paquier writes: >> Would it be better to return NULL instead then. > > That would likely just result in application core dumps. > See notes for commit 490cb21f7. That's 40cb21f7 actually.

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Tom Lane
Michael Paquier writes: > On Fri, Dec 2, 2016 at 8:36 AM, Robert Haas wrote: >> Correct, but I'm defining that as user error. If hostaddr is >> specified, host is not used to decide what to connect to, so it makes >> no sense for it to be a

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Michael Paquier
On Fri, Dec 2, 2016 at 8:36 AM, Robert Haas wrote: > On Thu, Dec 1, 2016 at 4:42 PM, Michael Paquier > wrote: >> On Fri, Dec 2, 2016 at 5:17 AM, Robert Haas wrote: >>> On Thu, Nov 24, 2016 at 4:38 PM, Andreas Karlsson

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Robert Haas
On Thu, Dec 1, 2016 at 4:42 PM, Michael Paquier wrote: > On Fri, Dec 2, 2016 at 5:17 AM, Robert Haas wrote: >> On Thu, Nov 24, 2016 at 4:38 PM, Andreas Karlsson wrote: >>> As you can see, after the patch libpq will now look at

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Michael Paquier
On Fri, Dec 2, 2016 at 5:46 AM, Robert Haas wrote: > On Thu, Dec 1, 2016 at 3:40 PM, Tom Lane wrote: >> Robert Haas writes: >>> On Thu, Dec 1, 2016 at 2:56 PM, Tom Lane wrote: check-world isn't a magic

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Michael Paquier
On Fri, Dec 2, 2016 at 5:56 AM, Robert Haas wrote: > On Fri, Nov 25, 2016 at 4:16 AM, Mithun Cy wrote: >> Reason is we first decode the URI(percent encoded character) then try to >> split the string into multiple host assuming they are separated

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Michael Paquier
On Fri, Dec 2, 2016 at 5:17 AM, Robert Haas wrote: > On Thu, Nov 24, 2016 at 4:38 PM, Andreas Karlsson wrote: >> As you can see, after the patch libpq will now look at hostaddr rather than >> host when validating the server certificate because that is

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Robert Haas
On Fri, Nov 25, 2016 at 4:16 AM, Mithun Cy wrote: > On Fri, Nov 25, 2016 at 12:03 PM, Andreas Karlsson > wrote: >> Another thought about this code: should we not check if it is a unix >> socket first before splitting the host? While I doubt that it

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Robert Haas
On Thu, Dec 1, 2016 at 3:40 PM, Tom Lane wrote: > Robert Haas writes: >> On Thu, Dec 1, 2016 at 2:56 PM, Tom Lane wrote: >>> check-world isn't a magic bullet. > >> No, but deliberately leaving things out that could be run isn't >>

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Tom Lane
Robert Haas writes: > On Thu, Dec 1, 2016 at 2:56 PM, Tom Lane wrote: >> check-world isn't a magic bullet. > No, but deliberately leaving things out that could be run isn't > helping anything either. Tests that open security holes while running aren't

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Robert Haas
On Thu, Dec 1, 2016 at 2:56 PM, Tom Lane wrote: > Robert Haas writes: >> Well, if people are unwilling to add test suites to 'make >> check-world', we can add 'make check-universe' and I'll run that >> instead. And that can come with a big shiny

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Robert Haas
On Thu, Nov 24, 2016 at 4:38 PM, Andreas Karlsson wrote: > As you can see, after the patch libpq will now look at hostaddr rather than > host when validating the server certificate because that is what is stored > in the first (and only) entry of conn->connhost, and therefore

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Tom Lane
Robert Haas writes: > Well, if people are unwilling to add test suites to 'make > check-world', we can add 'make check-universe' and I'll run that > instead. And that can come with a big shiny disclaimer. I just want > a way to compile and run EVERYTHING that people care

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Tom Lane
Andres Freund writes: > On 2016-12-01 14:43:04 -0500, Robert Haas wrote: >> I get that, but this is the second time in very recent history that >> I've broken something because there was code that wasn't compiled or >> tests that weren't run by 'make check-world'. > Well, I

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Robert Haas
On Thu, Dec 1, 2016 at 2:45 PM, Andres Freund wrote: > Well, I don't quite know what the alternative is. For some reason, which > I don't quite understand personally, people care about security during > regression tests runs. So we can't run the test automatedly. And nobody >

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Andres Freund
On 2016-12-01 14:43:04 -0500, Robert Haas wrote: > On Thu, Dec 1, 2016 at 2:40 PM, Andres Freund wrote: > > On 2016-12-01 14:22:19 -0500, Robert Haas wrote: > >> On Thu, Nov 24, 2016 at 4:38 PM, Andreas Karlsson > >> wrote: > >> > The SSL test suite

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Robert Haas
On Thu, Dec 1, 2016 at 2:40 PM, Andres Freund wrote: > On 2016-12-01 14:22:19 -0500, Robert Haas wrote: >> On Thu, Nov 24, 2016 at 4:38 PM, Andreas Karlsson wrote: >> > The SSL test suite (src/test/ssl) is broken in the master since commit >> >

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Andres Freund
On 2016-12-01 14:22:19 -0500, Robert Haas wrote: > On Thu, Nov 24, 2016 at 4:38 PM, Andreas Karlsson wrote: > > The SSL test suite (src/test/ssl) is broken in the master since commit > > 9a1d0af4ad2cbd419115b453d811c141b80d872b, which is Robert's refactoring of > > getting the

Re: [HACKERS] Broken SSL tests in master

2016-12-01 Thread Robert Haas
On Thu, Nov 24, 2016 at 4:38 PM, Andreas Karlsson wrote: > The SSL test suite (src/test/ssl) is broken in the master since commit > 9a1d0af4ad2cbd419115b453d811c141b80d872b, which is Robert's refactoring of > getting the server hostname for GSS, SSPI, and SSL in libpq. So, we

Re: [HACKERS] Broken SSL tests in master

2016-11-30 Thread Michael Paquier
On Fri, Nov 25, 2016 at 3:33 PM, Andreas Karlsson wrote: > On 11/25/2016 07:19 AM, Tsunakawa, Takayuki wrote: >> >> Specifying multiple hosts is a new feature to be introduced in v10, so >> that's here: >> >> https://www.postgresql.org/docs/devel/static/libpq-connect.html > > >

Re: [HACKERS] Broken SSL tests in master

2016-11-25 Thread Mithun Cy
On Fri, Nov 25, 2016 at 12:03 PM, Andreas Karlsson wrote: > Another thought about this code: should we not check if it is a unix socket first before splitting the host? While I doubt that it is common to have a unix >socket in a directory with comma in the path it is a bit

Re: [HACKERS] Broken SSL tests in master

2016-11-24 Thread Andreas Karlsson
On 11/25/2016 07:19 AM, Tsunakawa, Takayuki wrote: Specifying multiple hosts is a new feature to be introduced in v10, so that's here: https://www.postgresql.org/docs/devel/static/libpq-connect.html Thanks, I had missed that patch. If we add support for multiple hosts I think we should also

Re: [HACKERS] Broken SSL tests in master

2016-11-24 Thread Tsunakawa, Takayuki
From: pgsql-hackers-ow...@postgresql.org > sense to add support for multiple hostaddrs. For consitency's sake if > nothing else. Yes, consistency and performance. The purpose of hostaddr is to speed up connection by eliminating DNS lookup, isn't it? Then, some users should want to specify

Re: [HACKERS] Broken SSL tests in master

2016-11-24 Thread Mithun Cy
On Fri, Nov 25, 2016 at 10:41 AM, Tsunakawa, Takayuki < tsunakawa.ta...@jp.fujitsu.com> wrote: > I agree that pg_conn_host should have hostaddr in addition to host, and PQhost() return host when host is specified with/without hostaddr specified. typedef struct pg_conn_host +{ *+ char *host; /*

Re: [HACKERS] Broken SSL tests in master

2016-11-24 Thread Andreas Karlsson
On 11/25/2016 06:11 AM, Tsunakawa, Takayuki wrote: However, I wonder whether the hostaddr parameter should also accept multiple IP addresses. Yeah, I too thought about if we should fix that. I feel like it would make sense to add support for multiple hostaddrs. For consitency's sake if

Re: [HACKERS] Broken SSL tests in master

2016-11-24 Thread Tsunakawa, Takayuki
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Andreas Karlsson > On 11/24/2016 10:38 PM, Andreas Karlsson wrote: > > To me it feels like the proper fix would be to make PQHost() return > > the value of the host parameter rather than the

Re: [HACKERS] Broken SSL tests in master

2016-11-24 Thread Michael Paquier
On Fri, Nov 25, 2016 at 8:15 AM, Andreas Karlsson wrote: > On 11/24/2016 10:38 PM, Andreas Karlsson wrote: >> To me it feels like the proper fix would be to make PQHost() return the >> value of the host parameter rather than the hostaddr (maybe add a new >> field in the

Re: [HACKERS] Broken SSL tests in master

2016-11-24 Thread Andreas Karlsson
On 11/24/2016 10:38 PM, Andreas Karlsson wrote: To me it feels like the proper fix would be to make PQHost() return the value of the host parameter rather than the hostaddr (maybe add a new field in the pg_conn_host struct). But would be a behaviour change which might break someones application.

[HACKERS] Broken SSL tests in master

2016-11-24 Thread Andreas Karlsson
Hi, The SSL test suite (src/test/ssl) is broken in the master since commit 9a1d0af4ad2cbd419115b453d811c141b80d872b, which is Robert's refactoring of getting the server hostname for GSS, SSPI, and SSL in libpq. The error we get in the test suite: # Running: psql -X -A -t -c SELECT