Re: [PATCHES] SSPI authentication - patch

2007-07-20 Thread Magnus Hagander
Tom Lane wrote:
> Magnus Hagander <[EMAIL PROTECTED]> writes:
>> Stephen Frost wrote:
>>> That's true, but if we used upper-case with something NEW (SSPI) while
>>> keeping it the same for the OLD (KRB5, and I'd vote GSSAPI) then we're
>>> not breaking backwards compatibility while also catering to the masses.
>>> I guess I don't see too many people using SSPI w/ an MIT KDC, and it
>>> wasn't possible previously anyway.
>>>
>>> What do you think?
> 
>> Hmm. It makes the default a lot less clear, and opens up for confusion.
>> So I'm not so sure I like it :-)
> 
> A non-backward-compatible behavior change is going to cause a lot of
> confusion too.

Yeah.


> If I have things straight (and I'm not sure I do) then we are treating
> sspi as a different type of auth method.  It would be sane, or at least
> explainable, to have a different default name for the different auth
> method.  I think a platform-dependent default would seriously suck,
> and changing the default behavior for existing configurations would
> break things.  So Stephen's suggestion seemed plausible to me.

We use SSPI *both* as a protocol (windows talking to windows) and as an
API to go GSSAPI authentication (windows talking to unix, or windows
talking to windows with extra mit krb libraries).

Now, we can have two different defaults both for SSPI, but that's just
going to be too confusing I think. It's better to just keep the default
at "postgres" in that case, and tell people that if they use AD as their
KDC, they need to change it.

SSPI windows to windows will actually work without doing that, because
it will fallback to NTLM authentication if it's wrong. Windows to Unix
will not.

//Magnus

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] SSPI authentication - patch

2007-07-20 Thread Tom Lane
Magnus Hagander <[EMAIL PROTECTED]> writes:
> Stephen Frost wrote:
>> That's true, but if we used upper-case with something NEW (SSPI) while
>> keeping it the same for the OLD (KRB5, and I'd vote GSSAPI) then we're
>> not breaking backwards compatibility while also catering to the masses.
>> I guess I don't see too many people using SSPI w/ an MIT KDC, and it
>> wasn't possible previously anyway.
>> 
>> What do you think?

> Hmm. It makes the default a lot less clear, and opens up for confusion.
> So I'm not so sure I like it :-)

A non-backward-compatible behavior change is going to cause a lot of
confusion too.

If I have things straight (and I'm not sure I do) then we are treating
sspi as a different type of auth method.  It would be sane, or at least
explainable, to have a different default name for the different auth
method.  I think a platform-dependent default would seriously suck,
and changing the default behavior for existing configurations would
break things.  So Stephen's suggestion seemed plausible to me.

regards, tom lane

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match


Re: [PATCHES] SSPI authentication - patch

2007-07-20 Thread Magnus Hagander
Stephen Frost wrote:
> * Magnus Hagander ([EMAIL PROTECTED]) wrote:
>> On Thu, Jul 19, 2007 at 06:22:57PM -0400, Stephen Frost wrote:
>>> My thinking would be to have the autoconf to disable it, but enable it
>>> by default.  I don't feel particularly strongly about it though.
>> Do you see a use-case where someone would disable it? I'll be happy to add
>> the switch if you do, it's not hard to do, but adding a switch just for the
>> sake of adding a switch is not something I lik e:-)
> 
> Eh, I could contrive one but, as I said, I don't feel particularly
> strongly about it.  How about we go w/o it for now and see if anyone
> asks for it.

Sounds like a plan.


>> The change is there to because the majority of windows installs will
>> be using Active Directory, at least that's what I would expect. Certainly
>> not all, but most. It's a way of lowering the bar for the majority, at the
>> expense of the minority ;-)
> 
> It's also at the expense of backwards compatibility. :/  People who are
> currently using the krb5 auth mechanism with AD are used to having to
> flip that or set the environment variable while people who have been
> using it with an MIT KDC may get suprised by it.

Yeah, that's certainly the expense of it :-( It's helping the newbies
though.



>> That said, I actually intended to submit that as a separate patch for
>> separate discussion. If people are against it, I'll be happy to drop that
>> part.
> 
> My main concern is that it's a backward-incompatible change.  I realize
> that it's likely going in the direction of the majority on Windows but
> it seems to make like it's not something we should just 'do'.  That
> said, I don't see it as a problem for me since I've got a reasonably
> small user-base (10s, not 100s or 1000s) of Windows users and setting
> the environment variable shouldn't be an issue.

Right. For now, I'll pull it out of that patch, and we can have a
separate discussion about it. I'd certainly like to hear someone else
than just me and you say something about it :-)


>> Again, it's not related to the library used, it's related to the KDC. And
>> we can't detect that, at least not early enough.
> 
> That's true, but if we used upper-case with something NEW (SSPI) while
> keeping it the same for the OLD (KRB5, and I'd vote GSSAPI) then we're
> not breaking backwards compatibility while also catering to the masses.
> I guess I don't see too many people using SSPI w/ an MIT KDC, and it
> wasn't possible previously anyway.
> 
> What do you think?

Hmm. It makes the default a lot less clear, and opens up for confusion.
So I'm not so sure I like it :-)

Plus, it's not as easy to implement - you have to consider how it gets
affected by say manual specification of --with-krbsrvnam etc.

//Magnus

---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [PATCHES] SSPI authentication - patch

2007-07-20 Thread Stephen Frost
* Magnus Hagander ([EMAIL PROTECTED]) wrote:
> On Thu, Jul 19, 2007 at 06:22:57PM -0400, Stephen Frost wrote:
> > My thinking would be to have the autoconf to disable it, but enable it
> > by default.  I don't feel particularly strongly about it though.
> 
> Do you see a use-case where someone would disable it? I'll be happy to add
> the switch if you do, it's not hard to do, but adding a switch just for the
> sake of adding a switch is not something I lik e:-)

Eh, I could contrive one but, as I said, I don't feel particularly
strongly about it.  How about we go w/o it for now and see if anyone
asks for it.

> > I understand that SSPI is case-insensitive, or folds to uppercase, or
> > whatever, but this is *not* used only by the SSPI code.  Please correct
> > me if I'm wrong, but this will break existing krb-auth using client
> > applications/setups that went with the previous default, no?  I realize
> > it's on Windows, but there are people out there with that
> > configuration (yes, like me... :)...
> 
> Ok, first to clearify the facts:
> * SSPI is case-insensitive, case-preserving
> * The problem is not from SSPI. It's Active Directory. If you use AD as the
> KDC, you must use uppercase SPNs - regardless of SSPI. For example, it's
> needed for anybody wanting to use the old krb5 auth in 8.x together with
> Active Directory - like I do :-)

Ah, thanks for clearing up where the problem arises from.

> The change is there to because the majority of windows installs will
> be using Active Directory, at least that's what I would expect. Certainly
> not all, but most. It's a way of lowering the bar for the majority, at the
> expense of the minority ;-)

It's also at the expense of backwards compatibility. :/  People who are
currently using the krb5 auth mechanism with AD are used to having to
flip that or set the environment variable while people who have been
using it with an MIT KDC may get suprised by it.

> That said, I actually intended to submit that as a separate patch for
> separate discussion. If people are against it, I'll be happy to drop that
> part.

My main concern is that it's a backward-incompatible change.  I realize
that it's likely going in the direction of the majority on Windows but
it seems to make like it's not something we should just 'do'.  That
said, I don't see it as a problem for me since I've got a reasonably
small user-base (10s, not 100s or 1000s) of Windows users and setting
the environment variable shouldn't be an issue.

> Again, it's not related to the library used, it's related to the KDC. And
> we can't detect that, at least not early enough.

That's true, but if we used upper-case with something NEW (SSPI) while
keeping it the same for the OLD (KRB5, and I'd vote GSSAPI) then we're
not breaking backwards compatibility while also catering to the masses.
I guess I don't see too many people using SSPI w/ an MIT KDC, and it
wasn't possible previously anyway.

What do you think?

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [PATCHES] SSPI authentication - patch

2007-07-20 Thread Magnus Hagander
On Thu, Jul 19, 2007 at 06:22:57PM -0400, Stephen Frost wrote:
> * Magnus Hagander ([EMAIL PROTECTED]) wrote:
> > Here's an updated version of this patch. This version has full SSPI support
> > in the server as well, so I can do both kerberos and NTLM between two
> > windows machines using the negotiate method.
> 
> Great!  Also, I've tested that it works under Windows using
> PGGSSLIB=gssapi with the MIT GSS libraries.  I did have to set the
> PGKRBSRVNAME to 'postgres'.  It worked excellently. :)

Thanks!


> > Since SSPI and GSSAPI can now both be used, my plan is not to have an
> > autoconf to disable SSPI, but to just enable it unconditionally on win32.
> > Or does this seem like a bad idea?
> 
> My thinking would be to have the autoconf to disable it, but enable it
> by default.  I don't feel particularly strongly about it though.

Do you see a use-case where someone would disable it? I'll be happy to add
the switch if you do, it's not hard to do, but adding a switch just for the
sake of adding a switch is not something I lik e:-)

> 
> > Comments welcome.
> 
> It looks good in general to me (though I'm not super-familiar with
> SSPI).  My one big concern is this:
> 
> >   /* Define to the name of the default PostgreSQL service principal in 
> > Kerberos.
> >  (--with-krb-srvnam=NAME) */
> > ! #define PG_KRB_SRVNAM "postgres"
> >   
> >   /* A string containing the version number, platform, and C compiler */
> >   #define PG_VERSION_STR "Uninitialized version string (win32)"
> > --- 582,588 
> >   
> >   /* Define to the name of the default PostgreSQL service principal in 
> > Kerberos.
> >  (--with-krb-srvnam=NAME) */
> > ! #define PG_KRB_SRVNAM "POSTGRES"
> 
> I understand that SSPI is case-insensitive, or folds to uppercase, or
> whatever, but this is *not* used only by the SSPI code.  Please correct
> me if I'm wrong, but this will break existing krb-auth using client
> applications/setups that went with the previous default, no?  I realize
> it's on Windows, but there are people out there with that
> configuration (yes, like me... :)...

Ok, first to clearify the facts:
* SSPI is case-insensitive, case-preserving
* The problem is not from SSPI. It's Active Directory. If you use AD as the
KDC, you must use uppercase SPNs - regardless of SSPI. For example, it's
needed for anybody wanting to use the old krb5 auth in 8.x together with
Active Directory - like I do :-)

The change is there to because the majority of windows installs will
be using Active Directory, at least that's what I would expect. Certainly
not all, but most. It's a way of lowering the bar for the majority, at the
expense of the minority ;-)

That said, I actually intended to submit that as a separate patch for
separate discussion. If people are against it, I'll be happy to drop that
part.


> I don't particularly like it but, honestly, it seems like it might be
> better to set it based on what's being used (GSSAPI/SSPI/KRB5)?  This
> would be for the client-side, as I guess we've decided it's okay to just
> pick whatever keytab the users provide that's in our server-side
> keytab.

Again, it's not related to the library used, it's related to the KDC. And
we can't detect that, at least not early enough.

//Magnus

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [PATCHES] SSPI authentication - patch

2007-07-19 Thread Stephen Frost
* Magnus Hagander ([EMAIL PROTECTED]) wrote:
> Here's an updated version of this patch. This version has full SSPI support
> in the server as well, so I can do both kerberos and NTLM between two
> windows machines using the negotiate method.

Great!  Also, I've tested that it works under Windows using
PGGSSLIB=gssapi with the MIT GSS libraries.  I did have to set the
PGKRBSRVNAME to 'postgres'.  It worked excellently. :)

> Since SSPI and GSSAPI can now both be used, my plan is not to have an
> autoconf to disable SSPI, but to just enable it unconditionally on win32.
> Or does this seem like a bad idea?

My thinking would be to have the autoconf to disable it, but enable it
by default.  I don't feel particularly strongly about it though.

> Comments welcome.

It looks good in general to me (though I'm not super-familiar with
SSPI).  My one big concern is this:

>   /* Define to the name of the default PostgreSQL service principal in 
> Kerberos.
>  (--with-krb-srvnam=NAME) */
> ! #define PG_KRB_SRVNAM "postgres"
>   
>   /* A string containing the version number, platform, and C compiler */
>   #define PG_VERSION_STR "Uninitialized version string (win32)"
> --- 582,588 
>   
>   /* Define to the name of the default PostgreSQL service principal in 
> Kerberos.
>  (--with-krb-srvnam=NAME) */
> ! #define PG_KRB_SRVNAM "POSTGRES"

I understand that SSPI is case-insensitive, or folds to uppercase, or
whatever, but this is *not* used only by the SSPI code.  Please correct
me if I'm wrong, but this will break existing krb-auth using client
applications/setups that went with the previous default, no?  I realize
it's on Windows, but there are people out there with that
configuration (yes, like me... :)...

I don't particularly like it but, honestly, it seems like it might be
better to set it based on what's being used (GSSAPI/SSPI/KRB5)?  This
would be for the client-side, as I guess we've decided it's okay to just
pick whatever keytab the users provide that's in our server-side
keytab.

Thanks again!

Stephen


signature.asc
Description: Digital signature


Re: [PATCHES] SSPI authentication - patch

2007-07-19 Thread Magnus Hagander
On Wed, Jul 18, 2007 at 12:16:42PM +0200, Magnus Hagander wrote:
> Attached is the patch to support SSPI authentication in libpq. With this
> patch, I can authenticate windows clients against a linux server using
> kerberos *without* reqiring setting up MIT kerberos on the windows side.
> Protocol has not changed at all.

Here's an updated version of this patch. This version has full SSPI support
in the server as well, so I can do both kerberos and NTLM between two
windows machines using the negotiate method.

I've added a libpq connection parameter gsslib and the corresponding
environment variable PGGSSLIB. If it's set to "gssapi", libpq will use the
MIT GSSAPI implementation to authenticate to GSSAPI servers. If it's not
set, or set to anything else, SSPI will be used in Kerberos mode. SSPI in
negotiate mode will only be used if the server requests "sspi"
authentication instead of "gss".

Server-side, I've added the new authentication method "sspi" so the server
can inform the client that it wants to do SSPI "negotiate" auth instead of
plain Kerberos.

Since SSPI and GSSAPI can now both be used, my plan is not to have an
autoconf to disable SSPI, but to just enable it unconditionally on win32.
Or does this seem like a bad idea?

Comments welcome.

//Magnus
Index: src/backend/libpq/auth.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/libpq/auth.c,v
retrieving revision 1.153
diff -c -r1.153 auth.c
*** src/backend/libpq/auth.c12 Jul 2007 20:36:11 -  1.153
--- src/backend/libpq/auth.c19 Jul 2007 11:25:34 -
***
*** 464,473 
--- 464,477 
/*
 * Negotiation generated data to be sent to the client.
 */
+   OM_uint32   lmin_s;
+ 
elog(DEBUG4, "sending GSS response token of length %u",
 (unsigned int) port->gss->outbuf.length);
  
sendAuthRequest(port, AUTH_REQ_GSS_CONT);
+ 
+   gss_release_buffer(&lmin_s, &port->gss->outbuf);
}
  
if (maj_stat != GSS_S_COMPLETE && maj_stat != 
GSS_S_CONTINUE_NEEDED)
***
*** 536,542 
return STATUS_OK;
  }
  
! #else /* no ENABLE_GSS */
  static int
  pg_GSS_recvauth(Port *port)
  {
--- 540,546 
return STATUS_OK;
  }
  
! #else /* no ENABLE_GSS */
  static int
  pg_GSS_recvauth(Port *port)
  {
***
*** 547,552 
--- 551,795 
  }
  #endif/* ENABLE_GSS */
  
+ #ifdef ENABLE_SSPI
+ static void
+ pg_SSPI_error(int severity, char *errmsg, SECURITY_STATUS r)
+ {
+   char sysmsg[256];
+ 
+   if (FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM, NULL, r, 0, sysmsg, 
sizeof(sysmsg), NULL) == 0)
+   ereport(severity,
+   (errmsg_internal("%s", errmsg),
+   errdetail("sspi error %x", r)));
+   else
+   ereport(severity,
+   (errmsg_internal("%s", errmsg),
+   errdetail("%s (%x)", sysmsg, r)));
+ }
+ 
+ 
+ static int
+ pg_SSPI_recvauth(Port *port)
+ {
+   int mtype;
+   StringInfoData  buf;
+   SECURITY_STATUS r;
+   CredHandle  sspicred;
+   CtxtHandle  *sspictx = NULL,
+   newctx;
+   TimeStamp   expiry;
+   ULONG   contextattr;
+   SecBufferDesc   inbuf;
+   SecBufferDesc   outbuf;
+   SecBuffer   OutBuffers[1];
+   SecBuffer   InBuffers[1];
+   HANDLE  token;
+   TOKEN_USER  *tokenuser;
+   DWORD   retlen;
+   characcountname[MAXPGPATH];
+   chardomainname[MAXPGPATH];
+   DWORD   accountnamesize = sizeof(accountname);
+   DWORD   domainnamesize = sizeof(domainname);
+   SID_NAME_USEaccountnameuse;
+ 
+ 
+   /*
+* Acquire a handle to the server credentials.
+*/
+   r = AcquireCredentialsHandle(NULL,
+   "negotiate",
+   SECPKG_CRED_INBOUND,
+   NULL,
+   NULL,
+   NULL,
+   NULL,
+   &sspicred,
+   &expiry);
+   if (r != SEC_E_OK)
+   pg_SSPI_error(ERROR, 
+   gettext_noop("could not acquire SSPI 
credentials handle"), r);
+ 
+   /*
+* Loop through SSPI message exchange. This exchange can consist
+* of multiple messags sent in both directions. First message is always
+* from the client. All messages from client to server are password
+* packets (type 'p').
+*/
+   do 
+   {
+   mtype = pq_getbyte();
+   if (mtype != 'p