Open item: kerberos warning message - Mailing list pgsql-hackers

From Magnus Hagander
Subject Open item: kerberos warning message
Date
Msg-id 49661EC5.9040602@hagander.net
Whole thread Raw
Responses Re: Open item: kerberos warning message  (Tom Lane <tgl@sss.pgh.pa.us>)
Re: Open item: kerberos warning message  (Stephen Frost <sfrost@snowman.net>)
List pgsql-hackers
Looking at the open item about the new error message shown when Kerberos
is compiled in, and not used:
assword:
FATAL:  password authentication failed for user "mha"
psql: pg_krb5_init: krb5_cc_get_principal: No credentials cache found
FATAL:  password authentication failed for user "mha"



The reason this is happening is that we are initializing Kerberos even
if we're not going to use it. The reason for doing *this*, is that if
kerberos is compiled in, we use it to find out if we should try a
different username than the one logged in to the local system - we look
at the kerberos login.

We don't do this for any other login, including kerberos over GSSAPI.
AFAIK, we've heard no complaints.

I see two ways to fix this, and have attached two patches:

1) Remove the support for getting this username. AFAIK, it's not even
documented. [krberror_remove.patch]

2) Suppress the error message when called from this location. If
Kerberos is actually used, we'll get the error message again later and
show it then. [krberror_suppress.patch]



Thoughts?

//Magnus
*** a/src/interfaces/libpq/fe-auth.c
--- b/src/interfaces/libpq/fe-auth.c
***************
*** 190,217 **** pg_krb5_destroy(struct krb5_info * info)
  }


-
- /*
-  * pg_krb5_authname -- returns a copy of whatever name the user
-  *                       has authenticated to the system, or NULL
-  */
- static char *
- pg_krb5_authname(PQExpBuffer errorMessage)
- {
-     char       *tmp_name;
-     struct krb5_info info;
-
-     info.pg_krb5_initialised = 0;
-
-     if (pg_krb5_init(errorMessage, &info) != STATUS_OK)
-         return NULL;
-     tmp_name = strdup(info.pg_krb5_name);
-     pg_krb5_destroy(&info);
-
-     return tmp_name;
- }
-
-
  /*
   * pg_krb5_sendauth -- client routine to send authentication information to
   *                       the server
--- 190,195 ----
***************
*** 972,980 **** pg_fe_sendauth(AuthRequest areq, PGconn *conn)
  char *
  pg_fe_getauthname(PQExpBuffer errorMessage)
  {
- #ifdef KRB5
-     char       *krb5_name = NULL;
- #endif
      const char *name = NULL;
      char       *authn;

--- 950,955 ----
***************
*** 988,995 **** pg_fe_getauthname(PQExpBuffer errorMessage)
  #endif

      /*
!      * pglock_thread() really only needs to be called around
!      * pg_krb5_authname(), but some users are using configure
       * --enable-thread-safety-force, so we might as well do the locking within
       * our library to protect pqGetpwuid(). In fact, application developers
       * can use getpwuid() in their application if they use the locking call we
--- 963,969 ----
  #endif

      /*
!      * Some users are using configure
       * --enable-thread-safety-force, so we might as well do the locking within
       * our library to protect pqGetpwuid(). In fact, application developers
       * can use getpwuid() in their application if they use the locking call we
***************
*** 998,1014 **** pg_fe_getauthname(PQExpBuffer errorMessage)
       */
      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(errorMessage);
-     name = krb5_name;
- #endif
-
      if (!name)
      {
  #ifdef WIN32
--- 972,977 ----
***************
*** 1022,1033 **** pg_fe_getauthname(PQExpBuffer errorMessage)

      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;
--- 985,990 ----
*** a/src/interfaces/libpq/fe-auth.c
--- b/src/interfaces/libpq/fe-auth.c
***************
*** 124,130 **** struct krb5_info


  static int
! pg_krb5_init(PQExpBuffer errorMessage, struct krb5_info * info)
  {
      krb5_error_code retval;

--- 124,130 ----


  static int
! pg_krb5_init(PQExpBuffer errorMessage, struct krb5_info * info, bool seterrormessage)
  {
      krb5_error_code retval;

***************
*** 134,151 **** pg_krb5_init(PQExpBuffer errorMessage, struct krb5_info * info)
      retval = krb5_init_context(&(info->pg_krb5_context));
      if (retval)
      {
!         printfPQExpBuffer(errorMessage,
!                           "pg_krb5_init: krb5_init_context: %s\n",
!                           error_message(retval));
          return STATUS_ERROR;
      }

      retval = krb5_cc_default(info->pg_krb5_context, &(info->pg_krb5_ccache));
      if (retval)
      {
!         printfPQExpBuffer(errorMessage,
!                           "pg_krb5_init: krb5_cc_default: %s\n",
!                           error_message(retval));
          krb5_free_context(info->pg_krb5_context);
          return STATUS_ERROR;
      }
--- 134,153 ----
      retval = krb5_init_context(&(info->pg_krb5_context));
      if (retval)
      {
!         if (seterrormessage)
!             printfPQExpBuffer(errorMessage,
!                               "pg_krb5_init: krb5_init_context: %s\n",
!                               error_message(retval));
          return STATUS_ERROR;
      }

      retval = krb5_cc_default(info->pg_krb5_context, &(info->pg_krb5_ccache));
      if (retval)
      {
!         if (seterrormessage)
!             printfPQExpBuffer(errorMessage,
!                               "pg_krb5_init: krb5_cc_default: %s\n",
!                               error_message(retval));
          krb5_free_context(info->pg_krb5_context);
          return STATUS_ERROR;
      }
***************
*** 154,162 **** pg_krb5_init(PQExpBuffer errorMessage, struct krb5_info * info)
                                     &(info->pg_krb5_client));
      if (retval)
      {
!         printfPQExpBuffer(errorMessage,
!                           "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;
--- 156,165 ----
                                     &(info->pg_krb5_client));
      if (retval)
      {
!         if (seterrormessage)
!             printfPQExpBuffer(errorMessage,
!                               "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;
***************
*** 165,173 **** pg_krb5_init(PQExpBuffer errorMessage, struct krb5_info * info)
      retval = krb5_unparse_name(info->pg_krb5_context, info->pg_krb5_client, &(info->pg_krb5_name));
      if (retval)
      {
!         printfPQExpBuffer(errorMessage,
!                           "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);
--- 168,177 ----
      retval = krb5_unparse_name(info->pg_krb5_context, info->pg_krb5_client, &(info->pg_krb5_name));
      if (retval)
      {
!         if (seterrormessage)
!             printfPQExpBuffer(errorMessage,
!                               "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);
***************
*** 203,209 **** pg_krb5_authname(PQExpBuffer errorMessage)

      info.pg_krb5_initialised = 0;

!     if (pg_krb5_init(errorMessage, &info) != STATUS_OK)
          return NULL;
      tmp_name = strdup(info.pg_krb5_name);
      pg_krb5_destroy(&info);
--- 207,213 ----

      info.pg_krb5_initialised = 0;

!     if (pg_krb5_init(errorMessage, &info, false) != STATUS_OK)
          return NULL;
      tmp_name = strdup(info.pg_krb5_name);
      pg_krb5_destroy(&info);
***************
*** 235,241 **** pg_krb5_sendauth(PGconn *conn)
          return STATUS_ERROR;
      }

!     ret = pg_krb5_init(&conn->errorMessage, &info);
      if (ret != STATUS_OK)
          return ret;

--- 239,245 ----
          return STATUS_ERROR;
      }

!     ret = pg_krb5_init(&conn->errorMessage, &info, true);
      if (ret != STATUS_OK)
          return ret;


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Proposal: new border setting in psql
Next
From: David Fetter
Date:
Subject: Re: about truncate