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.