Re: [PATCH v3] GSSAPI encryption support

* Craig Ringer (craig(at)2ndquadrant(dot)com) wrote:
> On 16 October 2015 at 21:34, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> >> It's a different auth request, but the handling in be-auth.c is
> >> co-mingled to handle the cases:
> >
> > be-auth.c? You mean src/backend/libpq/auth.c?
>
> Ahem. Yes.

No worries. :)

> Also, I was actually thinking of the libpq front-end code, as it turns
> out. The backend's handling of each method is much better separated.

Ok.

> > src/backend/libpq/auth.c and src/interfaces/libpq/fe-auth.c have pretty
> > clearly independent paths for SSPI and GSSAPI
>
> fe-auth.c doesn't. See pg_fe_sendauth(...)'s case AUTH_REQ_GSS: and
> AUTH_REQ_SSPI:
>
> Not what I call clearly independent. I've had to deal with that tangle
> of joy a few times...

Hmm, yea, the actual choice of how to respond to the auth method the
server wants us to use is kind of tricky, but once we've settled on one
or the other, the rest of the code is pretty independent.

Still, I agree with your point that we need to be careful to not break
what we have there by, say, trying to move to encrypted GSSAPI with an
SSPI server.

I'm not sure that I want to really change that fe-auth.c code but I
really think there may be a simpler way to have the same behavior. That
mishmash of #ifdef's and swith/case statements strikes me as possibly
being overly cute. Clear blocks of "we got this message, this is what
we do when we're compiled with X, or Y, or X+Y" would probably be a lot
easier to follow.

> > In a
> > properly set up environment, the client could be using either, but it's
> > not like we try to figure out (or care) which the client is using from
> > the server's perspective.
>
> Exactly. I want to make sure we still don't have to care.

Agreed.

> > Now, if you set up GSSAPI on the client side and tell it to require
> > encryption and you're using SSPI on the server side (or the other way
> > around) it's not likely to work without changes to the SSPI code paths,
> > which we should be looking at doing.
>
> So long as setting up gssapi auth on the backend without requiring
> encryption still works with sspi clients, like it did before, I'm
> happy. My concern is avoiding a regression in what works, not
> requiring that the new functionality be supported everywhere.

Great, we're definitely agreed on that.

> >> > There is no reason that using GSSAPI from, e.g., MIT Kerberos for
> >> > Windows, should not work here.
> >>
> >> Except that *nobody* does it. The EDB installer builds Pg with SSPI,
> >> and that's what the overwhelming majority of Windows users use.
> >
> > Apparently *somebody* uses MIT KfW
>
> Yeah, I meant nobody uses it with Pg on Windows.

It'd be nice if it did, in case any of those users *do* end up wanting
to use PG.

> > I doubt anyone's tried building or running PG with GSSAPI under Windows
> > though. I agree it'd be good to test and see and perhaps even the EDB
> > installers could be changed to add support for it back in (not sure if
> > Dave really wants to though as it was a bit of a pain..).
>
> It's a lot of a pain, and not just because of maintenance. Kerberos on
> Windows needs access to various innards that you don't need when just
> using SSPI to delegate SSO to the OS. It's less secure, fiddly, and
> annoying. At least it was last time I had to deal with it, when I was
> working around some SSPI bugs in psqlODBC before landing up patching
> them in the driver instead.

Yeah, I remember working with KfW back-in-the-day. I never played with
the new 4.0 version, so perhaps it's better, but I'm not particularly
anxious to get into that mess again..

As for this patch, the reason I've not been as involved (beyond being
ridiculously busy) is that Michael's environment, which at least appears
perfectly reasonable (and works with PG unpatched) isn't working. If we
can get that working (and I've not looked at what's happening, so I have
no idea how easy or hard that would be), then I'd be a lot more excited
to spend time doing review of the patch.

Thanks!

Stephen