Re: krb5 & multiple users

[Ed Schaller <schallee(at)darkmist(dot)net>]

> Hmm.  Can we rearrange things so that fe_getauthname is not called till
> later?  I fail to see why it'd be a good idea to be sucking any kerberos
> info in at all during PQconndefaults, so the above suggests to me that
> we've divided up the operations wrongly.

The best way to do this would be to keep the authentication in PGconn or
a sub-struct of that. Then this could be passed down to any functions
that need the information contained there. The reason that the kerberos
stuff is needed for fe_getauthname is that it is possible for the user
name to only be defined in the kerberos credentials that are available.
The problem is that there is no way to share information between the
fe_sendauth and fe_getauthname as they have no common arguments. This
problem is fixed in this case by the static variables. 

Attached is a patch from the default cvs branch that fixes this problem.
It basically follows the third method from my previous email. It will
go and get the kerberos credentials every time fe_getauthname and
fe_sendauth are called. In most cases this will not actually add any
additional overhead.

I have not extensively tested the patch, but it does solve my double
connection test case. If others could test it I would appreciate it.

> BTW you might want to get Bear Giles involved in this, as he seems to be
> thinking hard about authentication issues in libpq.

I'd be happy to. Do you know his email address?

>>>------>

--

+-------------+-----------------------+---------------+
| Ed Schaller | schallee(at)darkmist(dot)net | mistymushroom |
+-------------+-----------------------+---------------+

diff -Naur pgsql/src/interfaces/libpq/fe-auth.c pgsql-patched/src/interfaces/libpq/fe-auth.c
--- pgsql/src/interfaces/libpq/fe-auth.c	Wed Apr 24 23:00:40 2002
+++ pgsql-patched/src/interfaces/libpq/fe-auth.c	Mon May 20 19:00:16 2002
@@ -268,22 +268,33 @@
  * Various krb5 state which is not connection specfic, 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)
+pg_krb5_init(char *PQerrormsg, struct krb5_info *info)
 {
 	krb5_error_code retval;
 
-	if (pg_krb5_initialised)
+	if (info->pg_krb5_initialised)
 		return STATUS_OK;
 
-	retval = krb5_init_context(&pg_krb5_context);
+	retval = krb5_init_context(&(info->pg_krb5_context));
 	if (retval)
 	{
 		snprintf(PQerrormsg, PQERRORMSG_LENGTH,
@@ -292,46 +303,56 @@
 		return STATUS_ERROR;
 	}
 
-	retval = krb5_cc_default(pg_krb5_context, &pg_krb5_ccache);
+	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(pg_krb5_context);
+		krb5_free_context(info->pg_krb5_context);
 		return STATUS_ERROR;
 	}
 
-	retval = krb5_cc_get_principal(pg_krb5_context, pg_krb5_ccache,
-								   &pg_krb5_client);
+	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(pg_krb5_context, pg_krb5_ccache);
-		krb5_free_context(pg_krb5_context);
+		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(pg_krb5_context, pg_krb5_client, &pg_krb5_name);
+	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(pg_krb5_context, pg_krb5_client);
-		krb5_cc_close(pg_krb5_context, pg_krb5_ccache);
-		krb5_free_context(pg_krb5_context);
+		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;
 	}
 
-	pg_krb5_name = pg_an_to_ln(pg_krb5_name);
+	info->pg_krb5_name = pg_an_to_ln(info->pg_krb5_name);
 
-	pg_krb5_initialised = 1;
+	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
@@ -340,10 +361,16 @@
 static const char *
 pg_krb5_authname(char *PQerrormsg)
 {
-	if (pg_krb5_init(PQerrormsg) != STATUS_OK)
+	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 pg_krb5_name;
+	return tmp_name;
 }
 
 
@@ -363,18 +390,21 @@
 	krb5_auth_context auth_context = NULL;
 	krb5_error *err_ret = NULL;
 	int			flags;
+	struct krb5_info info;
+	info.pg_krb5_initialised = 0;
 
-	ret = pg_krb5_init(PQerrormsg);
+	ret = pg_krb5_init(PQerrormsg, &info);
 	if (ret != STATUS_OK)
 		return ret;
 
-	retval = krb5_sname_to_principal(pg_krb5_context, hostname, PG_KRB_SRVNAM,
+	retval = krb5_sname_to_principal(info.pg_krb5_context, hostname, PG_KRB_SRVNAM,
 									 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;
 	}
 
@@ -388,16 +418,17 @@
 	{
 		snprintf(PQerrormsg, PQERRORMSG_LENGTH,
 				 libpq_gettext("could not set socket to blocking mode: %s\n"), strerror(errno));
-		krb5_free_principal(pg_krb5_context, server);
+		krb5_free_principal(info.pg_krb5_context, server);
+		pg_krb5_destroy(&info);
 		return STATUS_ERROR;
 	}
 
-	retval = krb5_sendauth(pg_krb5_context, &auth_context,
+	retval = krb5_sendauth(info.pg_krb5_context, &auth_context,
 						   (krb5_pointer) & sock, PG_KRB_SRVNAM,
-						   pg_krb5_client, server,
+						   info.pg_krb5_client, server,
 						   AP_OPTS_MUTUAL_REQUIRED,
 						   NULL, 0,		/* no creds, use ccache instead */
-						   pg_krb5_ccache, &err_ret, NULL, NULL);
+						   info.pg_krb5_ccache, &err_ret, NULL, NULL);
 	if (retval)
 	{
 		if (retval == KRB5_SENDAUTH_REJECTED && err_ret)
@@ -422,12 +453,12 @@
 		}
 
 		if (err_ret)
-			krb5_free_error(pg_krb5_context, err_ret);
+			krb5_free_error(info.pg_krb5_context, err_ret);
 
 		ret = STATUS_ERROR;
 	}
 
-	krb5_free_principal(pg_krb5_context, server);
+	krb5_free_principal(info.pg_krb5_context, server);
 
 	if (fcntl(sock, F_SETFL, (long) flags))
 	{
@@ -436,6 +467,7 @@
 				 strerror(errno));
 		ret = STATUS_ERROR;
 	}
+	pg_krb5_destroy(&info);
 
 	return ret;
 }
@@ -731,5 +763,9 @@
 
 	if (name && (authn = (char *) malloc(strlen(name) + 1)))
 		strcpy(authn, name);
+#ifdef KRB5
+	if(name)
+		free(name);
+#endif
 	return authn;
 }

Attachment: pgpFyGiydlQR9.pgp
Description: PGP signature