Greetings,
* Stephen Frost ([EMAIL PROTECTED]) wrote:
> I've now tested this patch at home w/ 8.2HEAD and it seems to fix the
> bug. I plan on testing it under 8.1.2 at work tommorow with
> mod_auth_krb5, etc, and expect it'll work there. Assuming all goes
> well and unless someone objects I'll forward the patch to -patches.
> It'd be great to have this fixed as it'll allow us to use Kerberos to
> authenticate to phppgadmin and other web-based tools which use
> Postgres.
While playing with this patch under 8.1.2 at home I discovered a
mistake in how I manually applied one of the hunks to fe-auth.c.
Basically, the base code had changed and so the patch needed to be
modified slightly. This is because the code no longer either has a
freeable pointer under 'name' or has 'name' as NULL.
The attached patch correctly frees the string from pg_krb5_authname
(where it had been strdup'd) if and only if pg_krb5_authname returned
a string (as opposed to falling through and having name be set using
name = pw->name;). Also added a comment to this effect.
Please review.
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 -0000 1.110
--- src/interfaces/libpq/fe-auth.c 5 Feb 2006 20:03:21 -0000
***************
*** 101,122 ****
* Various krb5 state which is not connection specific, and a flag to
* indicate whether we have initialised it yet.
*/
static int pg_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 int pg_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_context pg_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));
if (retval)
{
snprintf(PQerrormsg, PQERRORMSG_LENGTH,
"pg_krb5_init: krb5_cc_get_principal: %s\n",
error_message(retval));
! krb5_cc_close(info->pg_krb5_context, info->pg_krb5_ccache);
! krb5_free_context(info->pg_krb5_context);
return STATUS_ERROR;
}
! retval = krb5_unparse_name(info->pg_krb5_context, info->pg_krb5_client,
&(info->pg_krb5_name));
if (retval)
{
snprintf(PQerrormsg, PQERRORMSG_LENGTH,
"pg_krb5_init: krb5_unparse_name: %s\n",
error_message(retval));
! krb5_free_principal(info->pg_krb5_context,
info->pg_krb5_client);
! krb5_cc_close(info->pg_krb5_context, info->pg_krb5_ccache);
! krb5_free_context(info->pg_krb5_context);
return STATUS_ERROR;
}
! info->pg_krb5_name = pg_an_to_ln(info->pg_krb5_name);
! info->pg_krb5_initialised = 1;
return STATUS_OK;
}
+ static void
+ pg_krb5_destroy(struct krb5_info *info)
+ {
+ krb5_free_principal(info->pg_krb5_context, info->pg_krb5_client);
+ krb5_cc_close(info->pg_krb5_context, info->pg_krb5_ccache);
+ krb5_free_context(info->pg_krb5_context);
+ free(info->pg_krb5_name);
+ }
+
+
/*
* pg_krb5_authname -- returns a pointer to static space containing whatever
***************
*** 173,182 ****
static const char *
pg_krb5_authname(char *PQerrormsg)
{
! if (pg_krb5_init(PQerrormsg) != STATUS_OK)
return NULL;
! return pg_krb5_name;
}
--- 194,209 ----
static const char *
pg_krb5_authname(char *PQerrormsg)
{
! char *tmp_name;
! struct krb5_info info;
! info.pg_krb5_initialised = 0;
!
! if (pg_krb5_init(PQerrormsg, &info) != STATUS_OK)
return NULL;
+ tmp_name = strdup(info.pg_krb5_name);
+ pg_krb5_destroy(&info);
! return tmp_name;
}
***************
*** 192,197 ****
--- 219,226 ----
krb5_principal server;
krb5_auth_context auth_context = NULL;
krb5_error *err_ret = NULL;
+ struct krb5_info info;
+ info.pg_krb5_initialised = 0;
if (!hostname)
{
***************
*** 200,216 ****
return STATUS_ERROR;
}
! ret = pg_krb5_init(PQerrormsg);
if (ret != STATUS_OK)
return ret;
! retval = krb5_sname_to_principal(pg_krb5_context, hostname, servicename,
KRB5_NT_SRV_HST, &server);
if (retval)
{
snprintf(PQerrormsg, PQERRORMSG_LENGTH,
"pg_krb5_sendauth: krb5_sname_to_principal:
%s\n",
error_message(retval));
return STATUS_ERROR;
}
--- 229,246 ----
return STATUS_ERROR;
}
! ret = pg_krb5_init(PQerrormsg, &info);
if (ret != STATUS_OK)
return ret;
! retval = krb5_sname_to_principal(info.pg_krb5_context, hostname,
servicename,
KRB5_NT_SRV_HST, &server);
if (retval)
{
snprintf(PQerrormsg, PQERRORMSG_LENGTH,
"pg_krb5_sendauth: krb5_sname_to_principal:
%s\n",
error_message(retval));
+ pg_krb5_destroy(&info);
return STATUS_ERROR;
}
***************
*** 225,240 ****
snprintf(PQerrormsg, PQERRORMSG_LENGTH,
libpq_gettext("could not set socket to
blocking mode: %s\n"), pqStrerror(errno, sebuf, sizeof(sebuf)));
! krb5_free_principal(pg_krb5_context, server);
return STATUS_ERROR;
}
! retval = krb5_sendauth(pg_krb5_context, &auth_context,
(krb5_pointer) & sock, (char
*) servicename,
! pg_krb5_client, server,
AP_OPTS_MUTUAL_REQUIRED,
NULL, 0, /* no
creds, use ccache instead */
! pg_krb5_ccache, &err_ret,
NULL, NULL);
if (retval)
{
if (retval == KRB5_SENDAUTH_REJECTED && err_ret)
--- 255,271 ----
snprintf(PQerrormsg, PQERRORMSG_LENGTH,
libpq_gettext("could not set socket to
blocking mode: %s\n"), pqStrerror(errno, sebuf, sizeof(sebuf)));
! krb5_free_principal(info.pg_krb5_context, server);
! pg_krb5_destroy(&info);
return STATUS_ERROR;
}
! retval = krb5_sendauth(info.pg_krb5_context, &auth_context,
(krb5_pointer) & sock, (char
*) servicename,
! info.pg_krb5_client, server,
AP_OPTS_MUTUAL_REQUIRED,
NULL, 0, /* no
creds, use ccache instead */
! info.pg_krb5_ccache,
&err_ret, NULL, NULL);
if (retval)
{
if (retval == KRB5_SENDAUTH_REJECTED && err_ret)
***************
*** 259,270 ****
}
if (err_ret)
! krb5_free_error(pg_krb5_context, err_ret);
ret = STATUS_ERROR;
}
! krb5_free_principal(pg_krb5_context, server);
if (!pg_set_noblock(sock))
{
--- 290,301 ----
}
if (err_ret)
! krb5_free_error(info.pg_krb5_context, err_ret);
ret = STATUS_ERROR;
}
! krb5_free_principal(info.pg_krb5_context, server);
if (!pg_set_noblock(sock))
{
***************
*** 275,280 ****
--- 306,312 ----
pqStrerror(errno, sebuf, sizeof(sebuf)));
ret = STATUS_ERROR;
}
+ pg_krb5_destroy(&info);
return ret;
}
***************
*** 487,492 ****
--- 519,527 ----
char *
pg_fe_getauthname(char *PQerrormsg)
{
+ #ifdef KRB5
+ const char *krb5_name = NULL;
+ #endif
const char *name = NULL;
char *authn;
***************
*** 511,517 ****
pglock_thread();
#ifdef KRB5
! name = pg_krb5_authname(PQerrormsg);
#endif
if (!name)
--- 546,557 ----
pglock_thread();
#ifdef KRB5
! /* pg_krb5_authname gives us a strdup'd value that we need
! * to free later, however, we don't want to free 'name' directly
! * in case it's *not* a Kerberos login and we fall through to
! * name = pw->pw_name; */
! krb5_name = pg_krb5_authname(PQerrormsg);
! name = krb5_name;
#endif
if (!name)
***************
*** 527,532 ****
--- 567,578 ----
authn = name ? strdup(name) : NULL;
+ #ifdef KRB5
+ /* Free the strdup'd string from pg_krb5_authname, if we got one */
+ if (krb5_name)
+ free(krb5_name);
+ #endif
+
pgunlock_thread();
return authn;
---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend
