Re: [PATCHES] [HACKERS] Krb5 multiple DB connections
* Tom Lane ([EMAIL PROTECTED]) wrote: Stephen Frost [EMAIL PROTECTED] writes: I have little idea of how expensive the operations called by pg_krb5_init really are. If they are expensive then it'd probably make sense to keep the current static variables but treat 'em as a one-element cache, ie, recompute if a new user name is being demanded. If not, we ought to be able to simplify some things. We'd have to recompute based on the KRB5CCNAME environment variable changing, which is certainly an option. It's not necessairly the case that the username is changing, possibly just the cache. Hm, apparently I completely misunderstand the problem here. What I thought the bug was was that the cache wasn't recomputed given an attempt to connect as a different Postgres username than the first time. If that's not the issue, then what is? The specific problem which I and the original reporter ran into is this: KRB5CCACHE=/tmp/krb5cc_apache_aev0kF pg_connect() -- works fine pg_close() -- works fine rm /tmp/krb5cc_apache_aev0kF KRB5CCACHE=/tmp/krb5cc_apache_cVMRtA pg_connect() -- Doesn't work, Kerberos error is no credentials cache What's happening here is that for every connection to apache by the client a new credentials cache is created, and then destroyed when the connection closes. When using PHP (ie: phppgadmin) and mod_php (as is common) the Apache process is the one actually making the connection to the database and a given Apache process usually serves multiple requests in its lifetime, sometimes to the same user, sometimes to different users. The static variables being used are for: krb5_init_context, krb5_cc_default, krb5_cc_get_principal, and krb5_unparse_name. Technically, between one connection and the next, krb5_cc_default, krb5_cc_get_principal and krb5_unparse_name could reasonably return different values. krb5_init_context is pretty unlikely to change as that would mean /etc/krb5.conf changed. Looking through the krb5 source code it appears that the only one which checks for something existing is krb5_cc_default_name (called by krb5_cc_default), which will just return the current ccache name if one has been set. (src/lib/krb5/os/ccdefname.c:278, krb5-1.4.3) We initially brought up this issue with the Kerberos folks actually: http://pch.mit.edu/pipermail/kerberos/2006-February/009225.html They pretty clearly felt that the application was responsible for handling the cache in the event it changes. Unfortunately, it's not the application which is talking to Kerberos but another library in this case which doesn't expose the Kerberos API to the application in such a way to allow the application to notify Kerberos of the cache change. Looking through the Kerberos API again it looks like it might be possible to use krb5_cc_set_default_name(context, NULL) to force krb5_cc_default_name() (from krb5_cc_default()) to re-find the cache. Finding the cache again is reasonably inexpensive. I've tried a couple of things along these lines now but I havn't found a workable solution which doesn't reinitialize the main Kerberos context, unfortunately. I'll keep working on it as time allows though honestly I don't believe it's generally terribly expensive to reinitialize the context... Sorry it took so long to reply, running down the paths through the various libraries takes a bit of time and I was really hoping to be able to suggest an alternative solution using krb5_cc_set_default_name. Thanks, Stephen signature.asc Description: Digital signature
Re: [PATCHES] [HACKERS] Krb5 multiple DB connections
Stephen Frost [EMAIL PROTECTED] writes: The attached patch fixes a bug which was originally brought up in May of 2002 in this thread: Now that I've looked at it, I find this patch seems fairly wrongheaded. AFAICS the entire point of the original coding is to allow the setup work needed to create the krb5_context etc to be amortized across multiple connections. The patch destroys that advantage, but yet keeps as much as it can of the notational cruft induced by the original design -- for instance, there seems little point in the pg_krb5_initialised flag if we aren't ever going to have any pre-initialized state. I have little idea of how expensive the operations called by pg_krb5_init really are. If they are expensive then it'd probably make sense to keep the current static variables but treat 'em as a one-element cache, ie, recompute if a new user name is being demanded. If not, we ought to be able to simplify some things. Another point here is how all this interacts with thread safety. If we get rid of the static variables, do we still need the pglock_thread() operations? regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] Krb5 multiple DB connections
* Tom Lane ([EMAIL PROTECTED]) wrote: Stephen Frost [EMAIL PROTECTED] writes: The attached patch fixes a bug which was originally brought up in May of 2002 in this thread: Now that I've looked at it, I find this patch seems fairly wrongheaded. AFAICS the entire point of the original coding is to allow the setup work needed to create the krb5_context etc to be amortized across multiple connections. The patch destroys that advantage, but yet keeps as much as it can of the notational cruft induced by the original design -- for instance, there seems little point in the pg_krb5_initialised flag if we aren't ever going to have any pre-initialized state. I'm honestly not entirely sure I agree about that being the point of the original coding but regardless the crux of the problem here is that there's no way to get libpq to use a cache other than the one it's initialized with for a given session. That part of the Kerberos API which supports that isn't exposed in any way beyond the KRB5CCNAME environment variable and the call to ask for the 'default' ccache is called with the static variable the second time and it ignores the request when there's an already valid (it thinks) ccache. I have little idea of how expensive the operations called by pg_krb5_init really are. If they are expensive then it'd probably make sense to keep the current static variables but treat 'em as a one-element cache, ie, recompute if a new user name is being demanded. If not, we ought to be able to simplify some things. We'd have to recompute based on the KRB5CCNAME environment variable changing, which is certainly an option. It's not necessairly the case that the username is changing, possibly just the cache. Additionally, the calls themselves are not very expensive when being called on an existing cache, the most expensive thing is reaching out to the KDC to get a new service ticket which will either need to be done, or won't, depending on if a valid service ticket already exists in the cache or not. Another point here is how all this interacts with thread safety. If we get rid of the static variables, do we still need the pglock_thread() operations? Good question, I'm afraid probably not. I'd have to look through it again but last I checked MIT Kerberos prior to 1.4 (and I'm not 100% sure it's resolved in 1.4) wasn't threadsafe itself. I'd certainly be happy to rework the patch based on these comments, of course. Honestly, I'm pretty sure the original patch was intended to be minimal (and is for the most part). These changes would introduce more logic but if that's alright I'd be happy to do it. Thanks, Stephen signature.asc Description: Digital signature
Re: [PATCHES] [HACKERS] Krb5 multiple DB connections
Stephen Frost [EMAIL PROTECTED] writes: I have little idea of how expensive the operations called by pg_krb5_init really are. If they are expensive then it'd probably make sense to keep the current static variables but treat 'em as a one-element cache, ie, recompute if a new user name is being demanded. If not, we ought to be able to simplify some things. We'd have to recompute based on the KRB5CCNAME environment variable changing, which is certainly an option. It's not necessairly the case that the username is changing, possibly just the cache. Hm, apparently I completely misunderstand the problem here. What I thought the bug was was that the cache wasn't recomputed given an attempt to connect as a different Postgres username than the first time. If that's not the issue, then what is? regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] Krb5 multiple DB connections
Greetings,
The attached patch fixes a bug which was originally brought up in May
of 2002 in this thread:
http://archives.postgresql.org/pgsql-interfaces/2002-05/msg00083.php
The original bug reporter also supplied a patch to fix the problem:
http://archives.postgresql.org/pgsql-interfaces/2002-05/msg00090.php
I ran into exactly the same issue using 8.1.2. I've updated the
original patch to work for HEAD and 8.1.2. I've tested the patch on
both HEAD and 8.1.2, both at home and at work, and it works quite
nicely. In fact, I hope to have a patch to phppgadmin which will make
it properly handle Kerberized logins.
Thanks,
Stephen
Index: src/interfaces/libpq/fe-auth.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-auth.c,v
retrieving revision 1.110
diff -c -r1.110 fe-auth.c
*** src/interfaces/libpq/fe-auth.c 26 Dec 2005 14:58:05 - 1.110
--- src/interfaces/libpq/fe-auth.c 5 Feb 2006 20:03:21 -
***
*** 101,122
* Various krb5 state which is not connection specific, and a flag to
* indicate whether we have initialised it yet.
*/
static intpg_krb5_initialised;
static krb5_context pg_krb5_context;
static krb5_ccache pg_krb5_ccache;
static krb5_principal pg_krb5_client;
static char *pg_krb5_name;
static int
! pg_krb5_init(char *PQerrormsg)
{
krb5_error_code retval;
! if (pg_krb5_initialised)
return STATUS_OK;
! retval = krb5_init_context(pg_krb5_context);
if (retval)
{
snprintf(PQerrormsg, PQERRORMSG_LENGTH,
--- 101,133
* Various krb5 state which is not connection specific, and a flag to
* indicate whether we have initialised it yet.
*/
+ /*
static intpg_krb5_initialised;
static krb5_context pg_krb5_context;
static krb5_ccache pg_krb5_ccache;
static krb5_principal pg_krb5_client;
static char *pg_krb5_name;
+ */
+
+ struct krb5_info
+ {
+ int pg_krb5_initialised;
+ krb5_contextpg_krb5_context;
+ krb5_ccache pg_krb5_ccache;
+ krb5_principal pg_krb5_client;
+ char*pg_krb5_name;
+ };
static int
! pg_krb5_init(char *PQerrormsg, struct krb5_info *info)
{
krb5_error_code retval;
! if (info-pg_krb5_initialised)
return STATUS_OK;
! retval = krb5_init_context((info-pg_krb5_context));
if (retval)
{
snprintf(PQerrormsg, PQERRORMSG_LENGTH,
***
*** 125,170
return STATUS_ERROR;
}
! retval = krb5_cc_default(pg_krb5_context, pg_krb5_ccache);
if (retval)
{
snprintf(PQerrormsg, PQERRORMSG_LENGTH,
pg_krb5_init: krb5_cc_default: %s\n,
error_message(retval));
! krb5_free_context(pg_krb5_context);
return STATUS_ERROR;
}
! retval = krb5_cc_get_principal(pg_krb5_context, pg_krb5_ccache,
!
pg_krb5_client);
if (retval)
{
snprintf(PQerrormsg, PQERRORMSG_LENGTH,
pg_krb5_init: krb5_cc_get_principal: %s\n,
error_message(retval));
! krb5_cc_close(pg_krb5_context, pg_krb5_ccache);
! krb5_free_context(pg_krb5_context);
return STATUS_ERROR;
}
! retval = krb5_unparse_name(pg_krb5_context, pg_krb5_client,
pg_krb5_name);
if (retval)
{
snprintf(PQerrormsg, PQERRORMSG_LENGTH,
pg_krb5_init: krb5_unparse_name: %s\n,
error_message(retval));
! krb5_free_principal(pg_krb5_context, pg_krb5_client);
! krb5_cc_close(pg_krb5_context, pg_krb5_ccache);
! krb5_free_context(pg_krb5_context);
return STATUS_ERROR;
}
! pg_krb5_name = pg_an_to_ln(pg_krb5_name);
! pg_krb5_initialised = 1;
return STATUS_OK;
}
/*
* pg_krb5_authname -- returns a pointer to static space containing whatever
--- 136,191
return STATUS_ERROR;
}
! retval = krb5_cc_default(info-pg_krb5_context,
(info-pg_krb5_ccache));
if (retval)
{
snprintf(PQerrormsg, PQERRORMSG_LENGTH,
pg_krb5_init: krb5_cc_default: %s\n,
error_message(retval));
! krb5_free_context(info-pg_krb5_context);
return STATUS_ERROR;
}
! retval = krb5_cc_get_principal(info-pg_krb5_context,
info-pg_krb5_ccache,
!
(info-pg_krb5_client));
