mirrors/postgres
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Back out patch pending review.

Browse Source 
---------------------------------------------------------------------------

>   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.

Stephen Frost (sfrost@snowman.net) wrote:
This commit is contained in:
Bruce Momjian 2006-02-12 20:08:29 +00:00
parent 3e682635e3
commit 6e51bcef8e
1 changed files with 27 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

100
src/interfaces/libpq/fe-auth.c
View File

@ -10,7 +10,7 @@
* exceed INITIAL_EXPBUFFER_SIZE (currently 256 bytes). * exceed INITIAL_EXPBUFFER_SIZE (currently 256 bytes).
* *
* IDENTIFICATION * IDENTIFICATION
* $PostgreSQL: pgsql/src/interfaces/libpq/fe-auth.c,v 1.111 2006/02/12 20:04:42 momjian Exp $ * $PostgreSQL: pgsql/src/interfaces/libpq/fe-auth.c,v 1.112 2006/02/12 20:08:29 momjian Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
@ -101,33 +101,22 @@ pg_an_to_ln(char *aname)
* Various krb5 state which is not connection specific, and a flag to * Various krb5 state which is not connection specific, and a flag to
* indicate whether we have initialised it yet. * indicate whether we have initialised it yet.
*/ */
/*
static int pg_krb5_initialised; static int pg_krb5_initialised;
static krb5_context pg_krb5_context; static krb5_context pg_krb5_context;
static krb5_ccache pg_krb5_ccache; static krb5_ccache pg_krb5_ccache;
static krb5_principal pg_krb5_client; static krb5_principal pg_krb5_client;
static char *pg_krb5_name; 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 static int
pg_krb5_init(char *PQerrormsg, struct krb5_info *info) pg_krb5_init(char *PQerrormsg)
{ {
krb5_error_code retval; krb5_error_code retval;
if (info->pg_krb5_initialised) if (pg_krb5_initialised)
return STATUS_OK; return STATUS_OK;
retval = krb5_init_context(&(info->pg_krb5_context)); retval = krb5_init_context(&pg_krb5_context);
if (retval) if (retval)
{ {
snprintf(PQerrormsg, PQERRORMSG_LENGTH, snprintf(PQerrormsg, PQERRORMSG_LENGTH,
@ -136,56 +125,46 @@ pg_krb5_init(char *PQerrormsg, struct krb5_info *info)
return STATUS_ERROR; return STATUS_ERROR;
} }
retval = krb5_cc_default(info->pg_krb5_context, &(info->pg_krb5_ccache)); retval = krb5_cc_default(pg_krb5_context, &pg_krb5_ccache);
if (retval) if (retval)
{ {
snprintf(PQerrormsg, PQERRORMSG_LENGTH, snprintf(PQerrormsg, PQERRORMSG_LENGTH,
"pg_krb5_init: krb5_cc_default: %s\n", "pg_krb5_init: krb5_cc_default: %s\n",
error_message(retval)); error_message(retval));
krb5_free_context(info->pg_krb5_context); krb5_free_context(pg_krb5_context);
return STATUS_ERROR; return STATUS_ERROR;
} }
retval = krb5_cc_get_principal(info->pg_krb5_context, info->pg_krb5_ccache, retval = krb5_cc_get_principal(pg_krb5_context, pg_krb5_ccache,
&(info->pg_krb5_client)); &pg_krb5_client);
if (retval) if (retval)
{ {
snprintf(PQerrormsg, PQERRORMSG_LENGTH, snprintf(PQerrormsg, PQERRORMSG_LENGTH,
"pg_krb5_init: krb5_cc_get_principal: %s\n", "pg_krb5_init: krb5_cc_get_principal: %s\n",
error_message(retval)); error_message(retval));
krb5_cc_close(info->pg_krb5_context, info->pg_krb5_ccache); krb5_cc_close(pg_krb5_context, pg_krb5_ccache);
krb5_free_context(info->pg_krb5_context); krb5_free_context(pg_krb5_context);
return STATUS_ERROR; return STATUS_ERROR;
} }
retval = krb5_unparse_name(info->pg_krb5_context, info->pg_krb5_client, &(info->pg_krb5_name)); retval = krb5_unparse_name(pg_krb5_context, pg_krb5_client, &pg_krb5_name);
if (retval) if (retval)
{ {
snprintf(PQerrormsg, PQERRORMSG_LENGTH, snprintf(PQerrormsg, PQERRORMSG_LENGTH,
"pg_krb5_init: krb5_unparse_name: %s\n", "pg_krb5_init: krb5_unparse_name: %s\n",
error_message(retval)); error_message(retval));
krb5_free_principal(info->pg_krb5_context, info->pg_krb5_client); krb5_free_principal(pg_krb5_context, pg_krb5_client);
krb5_cc_close(info->pg_krb5_context, info->pg_krb5_ccache); krb5_cc_close(pg_krb5_context, pg_krb5_ccache);
krb5_free_context(info->pg_krb5_context); krb5_free_context(pg_krb5_context);
return STATUS_ERROR; return STATUS_ERROR;
} }
info->pg_krb5_name = pg_an_to_ln(info->pg_krb5_name); pg_krb5_name = pg_an_to_ln(pg_krb5_name);
info->pg_krb5_initialised = 1; pg_krb5_initialised = 1;
return STATUS_OK; 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 * pg_krb5_authname -- returns a pointer to static space containing whatever
@ -194,16 +173,10 @@ pg_krb5_destroy(struct krb5_info *info)
static const char * static const char *
pg_krb5_authname(char *PQerrormsg) pg_krb5_authname(char *PQerrormsg)
{ {
char *tmp_name; if (pg_krb5_init(PQerrormsg) != STATUS_OK)
struct krb5_info info;
info.pg_krb5_initialised = 0;
if (pg_krb5_init(PQerrormsg, &info) != STATUS_OK)
return NULL; return NULL;
tmp_name = strdup(info.pg_krb5_name);
pg_krb5_destroy(&info);
return tmp_name; return pg_krb5_name;
} }
@ -219,8 +192,6 @@ pg_krb5_sendauth(char *PQerrormsg, int sock, const char *hostname, const char *s
krb5_principal server; krb5_principal server;
krb5_auth_context auth_context = NULL; krb5_auth_context auth_context = NULL;
krb5_error *err_ret = NULL; krb5_error *err_ret = NULL;
struct krb5_info info;
info.pg_krb5_initialised = 0;
if (!hostname) if (!hostname)
{ {
@ -229,18 +200,17 @@ pg_krb5_sendauth(char *PQerrormsg, int sock, const char *hostname, const char *s
return STATUS_ERROR; return STATUS_ERROR;
} }
ret = pg_krb5_init(PQerrormsg, &info); ret = pg_krb5_init(PQerrormsg);
if (ret != STATUS_OK) if (ret != STATUS_OK)
return ret; return ret;
retval = krb5_sname_to_principal(info.pg_krb5_context, hostname, servicename, retval = krb5_sname_to_principal(pg_krb5_context, hostname, servicename,
KRB5_NT_SRV_HST, &server); KRB5_NT_SRV_HST, &server);
if (retval) if (retval)
{ {
snprintf(PQerrormsg, PQERRORMSG_LENGTH, snprintf(PQerrormsg, PQERRORMSG_LENGTH,
"pg_krb5_sendauth: krb5_sname_to_principal: %s\n", "pg_krb5_sendauth: krb5_sname_to_principal: %s\n",
error_message(retval)); error_message(retval));
pg_krb5_destroy(&info);
return STATUS_ERROR; return STATUS_ERROR;
} }
@ -255,17 +225,16 @@ pg_krb5_sendauth(char *PQerrormsg, int sock, const char *hostname, const char *s
snprintf(PQerrormsg, PQERRORMSG_LENGTH, snprintf(PQerrormsg, PQERRORMSG_LENGTH,
libpq_gettext("could not set socket to blocking mode: %s\n"), pqStrerror(errno, sebuf, sizeof(sebuf))); libpq_gettext("could not set socket to blocking mode: %s\n"), pqStrerror(errno, sebuf, sizeof(sebuf)));
krb5_free_principal(info.pg_krb5_context, server); krb5_free_principal(pg_krb5_context, server);
pg_krb5_destroy(&info);
return STATUS_ERROR; return STATUS_ERROR;
} }
retval = krb5_sendauth(info.pg_krb5_context, &auth_context, retval = krb5_sendauth(pg_krb5_context, &auth_context,
(krb5_pointer) & sock, (char *) servicename, (krb5_pointer) & sock, (char *) servicename,
info.pg_krb5_client, server, pg_krb5_client, server,
AP_OPTS_MUTUAL_REQUIRED, AP_OPTS_MUTUAL_REQUIRED,
NULL, 0, /* no creds, use ccache instead */ NULL, 0, /* no creds, use ccache instead */
info.pg_krb5_ccache, &err_ret, NULL, NULL); pg_krb5_ccache, &err_ret, NULL, NULL);
if (retval) if (retval)
{ {
if (retval == KRB5_SENDAUTH_REJECTED && err_ret) if (retval == KRB5_SENDAUTH_REJECTED && err_ret)
@ -290,12 +259,12 @@ pg_krb5_sendauth(char *PQerrormsg, int sock, const char *hostname, const char *s
} }
if (err_ret) if (err_ret)
krb5_free_error(info.pg_krb5_context, err_ret); krb5_free_error(pg_krb5_context, err_ret);
ret = STATUS_ERROR; ret = STATUS_ERROR;
} }
krb5_free_principal(info.pg_krb5_context, server); krb5_free_principal(pg_krb5_context, server);
if (!pg_set_noblock(sock)) if (!pg_set_noblock(sock))
{ {
@ -306,7 +275,6 @@ pg_krb5_sendauth(char *PQerrormsg, int sock, const char *hostname, const char *s
pqStrerror(errno, sebuf, sizeof(sebuf))); pqStrerror(errno, sebuf, sizeof(sebuf)));
ret = STATUS_ERROR; ret = STATUS_ERROR;
} }
pg_krb5_destroy(&info);
return ret; return ret;
} }
@ -519,9 +487,6 @@ pg_fe_sendauth(AuthRequest areq, PGconn *conn, const char *hostname,
char * char *
pg_fe_getauthname(char *PQerrormsg) pg_fe_getauthname(char *PQerrormsg)
{ {
#ifdef KRB5
const char *krb5_name = NULL;
#endif
const char *name = NULL; const char *name = NULL;
char *authn; char *authn;
@ -546,12 +511,7 @@ pg_fe_getauthname(char *PQerrormsg)
pglock_thread(); pglock_thread();
#ifdef KRB5 #ifdef KRB5
/* pg_krb5_authname gives us a strdup'd value that we need name = pg_krb5_authname(PQerrormsg);
* 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 #endif
if (!name) if (!name)
@ -567,12 +527,6 @@ pg_fe_getauthname(char *PQerrormsg)
authn = name ? strdup(name) : NULL; 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(); pgunlock_thread();
return authn; return authn;