Re: [PATCH v16] GSSAPI encryption support

[Nico Williams]

On Mon, Jun 11, 2018 at 01:31:12PM -0400, Andrew Dunstan wrote:
> On 06/11/2018 01:13 PM, Nico Williams wrote:
> >Well, all the free CIs like Travis and Appveyor do it this way.  You
> >don't have to *use* it just because the .yml files are in the source
> >tree.  But you have to have the .yml files in the source tree in order
> >to use these CIs.  It'd be nice to be able to point somewhere else for
> >them, but whatever, that's not something we get much choice in at this
> >time.
> 
> That's not true, at least for Appveyor (can't speak about travis - I have no
> first hand experience). For appveyor, you can supply a custom appveyor.yml
> file, which can be a complete URL. In fact, if you use a plain git source as
> opposed to one of the managed git services it supports, you have to do it
> that way - it ignores an appveyor.yml in your repo. I found this out the
> very hard way over the last few days, and they very kindly don't warn you at
> all about this.

OK, that's.. nice, maybe, I guess, but I'd still want version control
for these yml files -- why not have them in-tree?  I'd rather have them
in-tree unless there's a good reason not to have them there.

In other projects I definitely find it better to have these files
in-tree.

Nico
-- 

Re: [PATCH v16] GSSAPI encryption support

[Andrew Dunstan]

On 06/11/2018 01:13 PM, Nico Williams wrote:


Well, all the free CIs like Travis and Appveyor do it this way.  You
don't have to *use* it just because the .yml files are in the source
tree.  But you have to have the .yml files in the source tree in order
to use these CIs.  It'd be nice to be able to point somewhere else for
them, but whatever, that's not something we get much choice in at this
time.




That's not true, at least for Appveyor (can't speak about travis - I 
have no first hand experience). For appveyor, you can supply a custom 
appveyor.yml file, which can be a complete URL. In fact, if you use a 
plain git source as opposed to one of the managed git services it 
supports, you have to do it that way - it ignores an appveyor.yml in 
your repo. I found this out the very hard way over the last few days, 
and they very kindly don't warn you at all about this.


cheers

andrew

--
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [PATCH v16] GSSAPI encryption support

[Nico Williams]

On Mon, Jun 11, 2018 at 09:27:17AM -0400, Robert Haas wrote:
> On Thu, Jun 7, 2018 at 6:11 PM, Thomas Munro
>  wrote:
> >> Cool!  Is there any reason that your patch for Travis and AppVeyor
> >> integration is not just committed to master?
> >
> > I think that's a good idea and I know that some others are in favour.
> 
> One problem is that was discussed at PGCon it commits us to one
> particular build configuration i.e. one set of --with-whatever options
> to configure.  It's not bad to provide a reasonable set of defaults,
> but it means that patches which are best tested with some other set of
> values will have to modify the file (I guess?). Patches that need to
> be tested with multiple sets of flags are ... maybe just out of luck?

Hmm, that's not really true.  You can have a build and test matrix with
more than one row in it.

For example, look at: https://travis-ci.org/heimdal/heimdal

You'll see that Heimdal's Travis-CI integration has four builds:

 - Linux w/ GCC
 - Linux w/ Clang
 - OS X w/ Clang
 - Linux code coverage w/ GCC

We could easily add more options for Heimdal, if we felt we needed to
build and test more with Travis-CI.

Appveyor also has matrix support (though I'm not using it in Heimdal's
Appveyor-CI integration).

Now, of course if we had a very large set of configurations to test,
things might get slow, and the CIs might find it abusive.  It would be
best to use a maximal build configuration and go from there.  So, for
example, two configurations, one with and w/o JIT, but with all the
optional libraries (GSS, LDAP, ICU, Perl, Python, ...), and with two
different compilers (GCC and Clang, with Clang only for the JIT), plus
one OS X build (with JIT), and so on:

 - Linux w/ GCC
 - Linux w/ Clang (   JIT)
 - Linux w/ Clang (no JIT)
 - Linux code coverage
 - OS X  w/ Clang (   JIT)

and similarly for Windows on Appveyor.

> I really don't understand the notion of putting the build script
> inside the source tree.  It's all fine if there's One Way To Do It but
> often TMTOWTDII.  If the build configurations are described outside
> the source tree then you can have as many of them as you need.

Well, all the free CIs like Travis and Appveyor do it this way.  You
don't have to *use* it just because the .yml files are in the source
tree.  But you have to have the .yml files in the source tree in order
to use these CIs.  It'd be nice to be able to point somewhere else for
them, but whatever, that's not something we get much choice in at this
time.

The .yml files are unobstrusive anyways, and it's handy to have them
in-tree anyways.  It also makes it easier to do things like:

 - get build passing/failing buttons on wiki / build status pages 

 - make sure that the .yml files stay up to date as the source tree gets
   changed

It also makes it somewhat easier to get hooked on github and such, but
a bit of discipline will make that a non-issue.

Nico
-- 

Re: [PATCH v16] GSSAPI encryption support

[Robert Haas]

On Thu, Jun 7, 2018 at 6:11 PM, Thomas Munro
 wrote:
>> Cool!  Is there any reason that your patch for Travis and AppVeyor
>> integration is not just committed to master?
>
> I think that's a good idea and I know that some others are in favour.

One problem is that was discussed at PGCon it commits us to one
particular build configuration i.e. one set of --with-whatever options
to configure.  It's not bad to provide a reasonable set of defaults,
but it means that patches which are best tested with some other set of
values will have to modify the file (I guess?). Patches that need to
be tested with multiple sets of flags are ... maybe just out of luck?

I really don't understand the notion of putting the build script
inside the source tree.  It's all fine if there's One Way To Do It but
often TMTOWTDII.  If the build configurations are described outside
the source tree then you can have as many of them as you need.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [PATCH v16] GSSAPI encryption support

[Nico Williams]

On Fri, Jun 08, 2018 at 10:11:52AM +1200, Thomas Munro wrote:
> On Fri, Jun 8, 2018 at 9:00 AM, Nico Williams  wrote:
> > Cool!  Is there any reason that your patch for Travis and AppVeyor
> > integration is not just committed to master?
> 
> I think that's a good idea and I know that some others are in favour.
> The AppVeyor one is not good enough to propose just yet: it's
> cargo-culted and skips a test that doesn't pass and only runs the
> basic check tests (not contribs, isolation, TAP etc).  Fortunately
> Andrew Dunstan has recently figured out how to run the official build
> farm script inside AppVeyor[1], and it looks like we might be close to
> figuring out that one test that doesn't work.  That makes me wonder if
> we should get the Travis version to use the build farm scripts too.
> If we can get all of that sorted out, then yes, I would like to
> propose that we stick the .XXX.yml files in the tree.  Another thing
> not yet explored is Travis's macOS build support.

I use AppVeyor for Heimdal and for jq...  Maybe I can help out.

As for Travis' OS X support... the problem there is that their build
farm is very small, so using theirs means waiting and waiting.

> Someone might argue that we shouldn't depend on particular external
> services, or that there are other CI services out there and we should
> use those too/instead for some reason, or that we don't want all that
> junk at top level in the tree.  But it seems to me that as long as
> they're dot prefixed files, we could carry control files for any
> number of CI services without upsetting anyone.  Having them in the
> tree would allow anyone who has a publicly accessible git repo
> (bitbucket, GitHub, ...) to go to any CI service that interests them
> and enable it with a couple of clicks.

Carrying the .yml files causes no harm beyond dependence, but that's a
nice problem to have when the alternative is to not have a CI at all.

> Then cfbot would still need to create new branches automatically (that
> is fundamentally what it does: converts patches on the mailing list
> into branches on GitHub), but it wouldn't need to add those control
> files anymore, just the submitted patches.

You wouldn't need it to.  Instead the CF page could let submitters link
their CI status pages/buttons...

> > Is there a way to get my CF entry building in cfbot, or will it get
> > there when it gets there?
> 
> Urgh, due to a bug in my new rate limiting logic it stopped pushing
> new branches for a day or two.  Fixed, and I see it's just picked up
> your submission #1319.  Usually it picks things up within minutes (it
> rescans threads whenever the 'last mail' date changes on the
> Commitfest web page), and then also rechecks each submission every
> couple of days.

Thanks!

> In a nice demonstration of the complexity of these systems, I see that
> the build for your submission on Travis failed because apt couldn't
> update its package index because repo.mongodb.org's key has expired.
> Other recent builds are OK so that seems to be a weird transient
> failure; possibly out of data in some cache, or some fraction of their
> repo server farm hasn't been updated yet or ... whatever.  Bleugh.

Oof.

> > I know I can just apply your patch, push to my fork, and have Travis and
> > AppVeyor build it.  And I just might, but cfbot is a neato service!
> 
> Thanks.  The next step is to show cfbot's results in the Commitfest
> app, and Magnus and I have started working on that.  I gave a talk
> about all this at PGCon last week, and the slides are up[2] in case
> anyone is interested:

OK.  I think that will be a huge improvement.  I find CF to be fantastic
as it is, but this will make it even better.

Thanks,

Nico
-- 

Re: [PATCH v16] GSSAPI encryption support

[Thomas Munro]

On Fri, Jun 8, 2018 at 9:00 AM, Nico Williams  wrote:
> On Tue, Jun 05, 2018 at 12:16:31PM +1200, Thomas Munro wrote:
>> On Sat, May 26, 2018 at 6:58 AM, Robbie Harwood  wrote:
>> > Me and the bot are having an argument.  This should green Linux but I
>> > dunno about Windows.
>>
>> BTW if you're looking for a way to try stuff out on Windows exactly
>> the way cfbot does it without posting a new patch to the mailing list,
>> I put some instructions here:
>>
>> https://wiki.postgresql.org/wiki/Continuous_Integration
>>
>> The .patch there could certainly be improved (ideally, I think, it'd
>> run the whole build farm animal script) but it's a start.
>
> Cool!  Is there any reason that your patch for Travis and AppVeyor
> integration is not just committed to master?

I think that's a good idea and I know that some others are in favour.
The AppVeyor one is not good enough to propose just yet: it's
cargo-culted and skips a test that doesn't pass and only runs the
basic check tests (not contribs, isolation, TAP etc).  Fortunately
Andrew Dunstan has recently figured out how to run the official build
farm script inside AppVeyor[1], and it looks like we might be close to
figuring out that one test that doesn't work.  That makes me wonder if
we should get the Travis version to use the build farm scripts too.
If we can get all of that sorted out, then yes, I would like to
propose that we stick the .XXX.yml files in the tree.  Another thing
not yet explored is Travis's macOS build support.

Someone might argue that we shouldn't depend on particular external
services, or that there are other CI services out there and we should
use those too/instead for some reason, or that we don't want all that
junk at top level in the tree.  But it seems to me that as long as
they're dot prefixed files, we could carry control files for any
number of CI services without upsetting anyone.  Having them in the
tree would allow anyone who has a publicly accessible git repo
(bitbucket, GitHub, ...) to go to any CI service that interests them
and enable it with a couple of clicks.

Then cfbot would still need to create new branches automatically (that
is fundamentally what it does: converts patches on the mailing list
into branches on GitHub), but it wouldn't need to add those control
files anymore, just the submitted patches.

> Is there a way to get my CF entry building in cfbot, or will it get
> there when it gets there?

Urgh, due to a bug in my new rate limiting logic it stopped pushing
new branches for a day or two.  Fixed, and I see it's just picked up
your submission #1319.  Usually it picks things up within minutes (it
rescans threads whenever the 'last mail' date changes on the
Commitfest web page), and then also rechecks each submission every
couple of days.

In a nice demonstration of the complexity of these systems, I see that
the build for your submission on Travis failed because apt couldn't
update its package index because repo.mongodb.org's key has expired.
Other recent builds are OK so that seems to be a weird transient
failure; possibly out of data in some cache, or some fraction of their
repo server farm hasn't been updated yet or ... whatever.  Bleugh.

> I know I can just apply your patch, push to my fork, and have Travis and
> AppVeyor build it.  And I just might, but cfbot is a neato service!

Thanks.  The next step is to show cfbot's results in the Commitfest
app, and Magnus and I have started working on that.  I gave a talk
about all this at PGCon last week, and the slides are up[2] in case
anyone is interested:

[1] 
https://www.postgresql.org/message-id/flat/0f3d44a1-1ac5-599c-3e15-16d058d54e9a%402ndQuadrant.com
[2] https://www.pgcon.org/2018/schedule/events/1234.en.html

-- 
Thomas Munro
http://www.enterprisedb.com

Re: [PATCH v16] GSSAPI encryption support

[Nico Williams]

On Tue, Jun 05, 2018 at 12:16:31PM +1200, Thomas Munro wrote:
> On Sat, May 26, 2018 at 6:58 AM, Robbie Harwood  wrote:
> > Me and the bot are having an argument.  This should green Linux but I
> > dunno about Windows.
> 
> BTW if you're looking for a way to try stuff out on Windows exactly
> the way cfbot does it without posting a new patch to the mailing list,
> I put some instructions here:
> 
> https://wiki.postgresql.org/wiki/Continuous_Integration
> 
> The .patch there could certainly be improved (ideally, I think, it'd
> run the whole build farm animal script) but it's a start.

Cool!  Is there any reason that your patch for Travis and AppVeyor
integration is not just committed to master?

Is there a way to get my CF entry building in cfbot, or will it get
there when it gets there?

I know I can just apply your patch, push to my fork, and have Travis and
AppVeyor build it.  And I just might, but cfbot is a neato service!

Nico
-- 

Re: [PATCH v16] GSSAPI encryption support

[Thomas Munro]

On Sat, May 26, 2018 at 6:58 AM, Robbie Harwood  wrote:
> Me and the bot are having an argument.  This should green Linux but I
> dunno about Windows.

BTW if you're looking for a way to try stuff out on Windows exactly
the way cfbot does it without posting a new patch to the mailing list,
I put some instructions here:

https://wiki.postgresql.org/wiki/Continuous_Integration

The .patch there could certainly be improved (ideally, I think, it'd
run the whole build farm animal script) but it's a start.

-- 
Thomas Munro
http://www.enterprisedb.com

Re: [PATCH v16] GSSAPI encryption support

[Robbie Harwood]

Robbie Harwood  writes:

> Thomas Munro  writes:
>
>> On Thu, May 24, 2018 at 8:00 AM, Robbie Harwood  wrote:
>>
>>> Zombie patch is back from the dead.
>>
>> Hi Robbie,
>>
>> Robots[1] vs zombies:
>>
>> + $postgres->RemoveFile('src/backennd/libpq/be-gssapi-common.c');
>>
>> Typo, breaks on Windows.
>>
>> runtime.sgml:2032: parser error : Opening and ending tag mismatch:
>> para line 2026 and sect1
>>  
>>  ^
>>
>> Docs malformed.
>>
>> [1] http://cfbot.cputube.org/robbie-harwood.html
>
> Hah, this is great!  Looks like I failed to build the docs.
>
> Here's an updated version that should fix those.

Me and the bot are having an argument.  This should green Linux but I
dunno about Windows.

Thanks,
--Robbie


signature.asc
Description: PGP signature
>From 1bf21be9d9cd05bf2bcb37c474888b4d8ff6fb63 Mon Sep 17 00:00:00 2001
From: Robbie Harwood 
Date: Thu, 10 May 2018 16:12:03 -0400
Subject: [PATCH] libpq GSSAPI encryption support

On both the frontend and backend, prepare for GSSAPI encryption
support by moving common code for error handling into a separate file.
Fix a TODO for handling multiple status messages in the process.
Eliminate the OIDs, which have not been needed for some time.

Add frontend and backend encryption support functions.  Keep the
context initiation for authentication-only separate on both the
frontend and backend in order to avoid concerns about changing the
requested flags to include encryption support.

In postmaster, pull GSSAPI authorization checking into a shared
function.  Also share the initiator name between the encryption and
non-encryption codepaths.

Modify pqsecure_write() to take a non-const pointer.  The pointer will
not be modified, but this change (or a const-discarding cast, or a
malloc()+memcpy()) is necessary for GSSAPI due to const/struct
interactions in C.

For HBA, add "hostgss" and "hostnogss" entries that behave similarly
to their SSL counterparts.  "hostgss" requires either "gss", "trust",
or "reject" for its authentication.

Simiarly, add a "gssmode" parameter to libpq.  Supported values are
"disable", "require", and "prefer".  Notably, negotiation will only be
attempted if credentials can be acquired.  Move credential acquisition
into its own function to support this behavior.

Finally, add documentation for everything new, and update existing
documentation on connection security.

Thanks to Michael Paquier for the Windows fixes.
---
 doc/src/sgml/client-auth.sgml   |  75 --
 doc/src/sgml/libpq.sgml |  57 +++-
 doc/src/sgml/runtime.sgml   |  77 +-
 src/backend/libpq/Makefile  |   4 +
 src/backend/libpq/auth.c| 103 +++
 src/backend/libpq/be-gssapi-common.c|  64 +
 src/backend/libpq/be-gssapi-common.h|  26 ++
 src/backend/libpq/be-secure-gssapi.c| 319 ++
 src/backend/libpq/be-secure.c   |  16 ++
 src/backend/libpq/hba.c |  51 +++-
 src/backend/postmaster/pgstat.c |   3 +
 src/backend/postmaster/postmaster.c |  64 -
 src/include/libpq/hba.h |   4 +-
 src/include/libpq/libpq-be.h|  11 +-
 src/include/libpq/libpq.h   |   3 +
 src/include/libpq/pqcomm.h  |   5 +-
 src/include/pgstat.h|   3 +-
 src/interfaces/libpq/Makefile   |   4 +
 src/interfaces/libpq/fe-auth.c  |  90 +--
 src/interfaces/libpq/fe-connect.c   | 232 +++-
 src/interfaces/libpq/fe-gssapi-common.c | 128 +
 src/interfaces/libpq/fe-gssapi-common.h |  23 ++
 src/interfaces/libpq/fe-secure-gssapi.c | 343 
 src/interfaces/libpq/fe-secure.c|  16 +-
 src/interfaces/libpq/libpq-fe.h |   3 +-
 src/interfaces/libpq/libpq-int.h|  30 ++-
 src/tools/msvc/Mkvcbuild.pm |  24 +-
 27 files changed, 1576 insertions(+), 202 deletions(-)
 create mode 100644 src/backend/libpq/be-gssapi-common.c
 create mode 100644 src/backend/libpq/be-gssapi-common.h
 create mode 100644 src/backend/libpq/be-secure-gssapi.c
 create mode 100644 src/interfaces/libpq/fe-gssapi-common.c
 create mode 100644 src/interfaces/libpq/fe-gssapi-common.h
 create mode 100644 src/interfaces/libpq/fe-secure-gssapi.c

diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 656d5f9417..38cf32e3be 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -108,6 +108,8 @@ hostnossl  database  user
 host   database  user  IP-address  IP-mask  auth-method  auth-options
 hostssldatabase  user  IP-address  IP-mask  auth-method  auth-options
 hostnossl  database  user  IP-address  IP-mask  auth-method  auth-options
+hostgssdatabase  user  IP-address  IP-mask  auth-method  auth-options
+hostnogss  database  user  IP-address  IP-mask  auth-method  auth-options
 
The meaning of