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

Reply via email to