[28665] in CVS-changelog-for-Kerberos-V5

home help back first fref pref prev next nref lref last post

krb5 commit: Include file ccache name in error messages

daemon@ATHENA.MIT.EDU (Greg Hudson)
Mon Dec 15 17:34:07 2014

Date: Mon, 15 Dec 2014 17:34:02 -0500
From: Greg Hudson <ghudson@mit.edu>
Message-Id: <201412152234.sBFMY2AT003304@drugstore.mit.edu>
To: cvs-krb5@mit.edu
Reply-To: krbdev@mit.edu
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: cvs-krb5-bounces@mit.edu

https://github.com/krb5/krb5/commit/98b55e86d7ec8b0a3b9b9f9b415ffdf78f4fd2e8
commit 98b55e86d7ec8b0a3b9b9f9b415ffdf78f4fd2e8
Author: Nicolas Williams <nico@cryptonector.com>
Date:   Wed Oct 29 19:42:49 2014 -0500

    Include file ccache name in error messages
    
    When a FILE ccache method returns an error, append the filename to the
    standard message for the code.  Remove code to set extended messages
    in helper functions as they would just be overwritten.
    
    Also change the interpretation of errno values.  Treat ENAMETOOLONG as
    KRB5_FCC_NOFILE instead of KRB5_FCC_INTERNAL, since it has an external
    cause and a name that long can't be opened by normal means.  Treat
    EROFS as KRB5_FCC_PERM.  Treat ENOTDIR and ELOOP as KRB5_FCC_NOFILE
    instead of KRB5_FCC_PERM as both errors imply that the full pathname
    doesn't exist.  Treat EBUSY and ETXTBSY as KRB5_CC_IO instead of
    KRB5_FCC_PERM as they indicate a conflict rather than a permission
    issue.
    
    [ghudson@mit.edu: renamed set_error to set_errmsg_filename; removed
    now-inoperative code to set extended messages in helper functions;
    trimmed changes to interpret_errno; clarified and shortened commit
    message]
    
    ticket: 8052 (new)

 src/lib/krb5/ccache/cc_file.c        |   88 +++++++++++++++++----------------
 src/tests/dejagnu/config/default.exp |    2 +-
 src/tests/gssapi/t_client_keytab.py  |    2 +-
 src/tests/t_ccache.py                |    4 +-
 src/tests/t_errmsg.py                |   14 +++---
 5 files changed, 56 insertions(+), 54 deletions(-)

diff --git a/src/lib/krb5/ccache/cc_file.c b/src/lib/krb5/ccache/cc_file.c
index 295f959..de9c968 100644
--- a/src/lib/krb5/ccache/cc_file.c
+++ b/src/lib/krb5/ccache/cc_file.c
@@ -112,6 +112,15 @@ typedef struct _krb5_fcc_cursor {
 
 k5_cc_mutex krb5int_cc_file_mutex = K5_CC_MUTEX_PARTIAL_INITIALIZER;
 
+/* Add fname to the standard error message for ret. */
+static krb5_error_code
+set_errmsg_filename(krb5_context context, krb5_error_code ret,
+                    const char *fname)
+{
+    k5_setmsg(context, ret, "%s (filename: %s)", error_message(ret), fname);
+    return ret;
+}
+
 /* Get the size of the cache file as a size_t, or SIZE_MAX if it is too
  * large to be represented as a size_t. */
 static krb5_error_code
@@ -329,16 +338,8 @@ open_cache_file(krb5_context context, const char *filename,
 
     flags = writable ? (O_RDWR | O_APPEND) : O_RDONLY;
     fd = open(filename, flags | O_BINARY | O_CLOEXEC, 0600);
-    if (fd == -1) {
-        if (errno == ENOENT) {
-            ret = KRB5_FCC_NOFILE;
-            k5_setmsg(context, ret, _("Credentials cache file '%s' not found"),
-                      filename);
-            return ret;
-        } else {
-            return interpret_errno(context, errno);
-        }
-    }
+    if (fd == -1)
+        return interpret_errno(context, errno);
     set_cloexec_fd(fd);
 
     lockmode = writable ? KRB5_LOCKMODE_EXCLUSIVE : KRB5_LOCKMODE_SHARED;
@@ -522,7 +523,7 @@ cleanup:
         close(fd);
     k5_cc_mutex_unlock(context, &data->lock);
     krb5_change_cache();
-    return ret;
+    return set_errmsg_filename(context, ret, data->filename);
 }
 
 /* Release an fcc_data object. */
@@ -648,7 +649,7 @@ cleanup:
     free(id);
 
     krb5_change_cache();
-    return ret;
+    return set_errmsg_filename(context, ret, data->filename);
 }
 
 extern const krb5_cc_ops krb5_fcc_ops;
@@ -737,7 +738,7 @@ cleanup:
     free(fcursor);
     krb5_free_principal(context, princ);
     k5_cc_mutex_unlock(context, &data->lock);
-    return ret;
+    return set_errmsg_filename(context, ret, data->filename);
 }
 
 /* Get the next credential from the cache file. */
@@ -780,7 +781,7 @@ cleanup:
         (void)krb5_unlock_file(context, fileno(fcursor->fp));
     k5_cc_mutex_unlock(context, &data->lock);
     k5_buf_free(&buf);
-    return ret;
+    return set_errmsg_filename(context, ret, data->filename);
 }
 
 /* Release an iteration cursor. */
@@ -896,7 +897,7 @@ err_out:
     k5_cc_mutex_destroy(&data->lock);
     free(data->filename);
     free(data);
-    return ret;
+    return set_errmsg_filename(context, ret, data->filename);
 }
 
 /*
@@ -941,7 +942,7 @@ fcc_get_principal(krb5_context context, krb5_ccache id, krb5_principal *princ)
 cleanup:
     (void)close_cache_file(context, fp);
     k5_cc_mutex_unlock(context, &data->lock);
-    return ret;
+    return set_errmsg_filename(context, ret, data->filename);
 }
 
 /* Search for a credential within the cache file. */
@@ -949,8 +950,10 @@ static krb5_error_code KRB5_CALLCONV
 fcc_retrieve(krb5_context context, krb5_ccache id, krb5_flags whichfields,
              krb5_creds *mcreds, krb5_creds *creds)
 {
-    return k5_cc_retrieve_cred_default(context, id, whichfields, mcreds,
-                                       creds);
+    krb5_error_code ret;
+
+    ret = k5_cc_retrieve_cred_default(context, id, whichfields, mcreds, creds);
+    return set_errmsg_filename(context, ret, ((fcc_data *)id->data)->filename);
 }
 
 /* Store a credential in the cache file. */
@@ -992,7 +995,7 @@ cleanup:
     k5_buf_free(&buf);
     ret2 = close_cache_file(context, fp);
     k5_cc_mutex_unlock(context, &data->lock);
-    return ret ? ret : ret2;
+    return set_errmsg_filename(context, ret ? ret : ret2, data->filename);
 }
 
 /* Non-functional stub for removing a cred from the cache file. */
@@ -1075,7 +1078,7 @@ fcc_ptcursor_next(krb5_context context, krb5_cc_ptcursor cursor,
 
     ret = krb5_cc_resolve(context, defname, &cache);
     if (ret)
-        return ret;
+        return set_errmsg_filename(context, ret, defname);
     *cache_out = cache;
     return 0;
 }
@@ -1112,7 +1115,7 @@ fcc_last_change_time(krb5_context context, krb5_ccache id,
 
     k5_cc_mutex_unlock(context, &data->lock);
 
-    return ret;
+    return set_errmsg_filename(context, ret, data->filename);
 }
 
 /* Lock the cache handle against other threads.  (This does not lock the cache
@@ -1142,6 +1145,13 @@ interpret_errno(krb5_context context, int errnum)
 
     switch (errnum) {
     case ENOENT:
+    case ENOTDIR:
+#ifdef ELOOP
+    case ELOOP:
+#endif
+#ifdef ENAMETOOLONG
+    case ENAMETOOLONG:
+#endif
         ret = KRB5_FCC_NOFILE;
         break;
     case EPERM:
@@ -1149,14 +1159,6 @@ interpret_errno(krb5_context context, int errnum)
 #ifdef EISDIR
     case EISDIR:                /* Mac doesn't have EISDIR */
 #endif
-    case ENOTDIR:
-#ifdef ELOOP
-    case ELOOP:                 /* Bad symlink is like no file. */
-#endif
-#ifdef ETXTBSY
-    case ETXTBSY:
-#endif
-    case EBUSY:
     case EROFS:
         ret = KRB5_FCC_PERM;
         break;
@@ -1164,27 +1166,27 @@ interpret_errno(krb5_context context, int errnum)
     case EEXIST:
     case EFAULT:
     case EBADF:
-#ifdef ENAMETOOLONG
-    case ENAMETOOLONG:
-#endif
 #ifdef EWOULDBLOCK
     case EWOULDBLOCK:
 #endif
         ret = KRB5_FCC_INTERNAL;
         break;
-#ifdef EDQUOT
-    case EDQUOT:
-#endif
-    case ENOSPC:
-    case EIO:
-    case ENFILE:
-    case EMFILE:
-    case ENXIO:
+    /*
+     * The rest all map to KRB5_CC_IO.  These errnos are listed to
+     * document that they've been considered explicitly:
+     *
+     *  - EDQUOT
+     *  - ENOSPC
+     *  - EIO
+     *  - ENFILE
+     *  - EMFILE
+     *  - ENXIO
+     *  - EBUSY
+     *  - ETXTBSY
+     */
     default:
         ret = KRB5_CC_IO;
-        k5_setmsg(context, ret,
-                  _("Credentials cache I/O operation failed (%s)"),
-                  strerror(errnum));
+        break;
     }
     return ret;
 }
diff --git a/src/tests/dejagnu/config/default.exp b/src/tests/dejagnu/config/default.exp
index 0c7a0c7..c163548 100644
--- a/src/tests/dejagnu/config/default.exp
+++ b/src/tests/dejagnu/config/default.exp
@@ -2193,7 +2193,7 @@ proc do_klist_err { testname } {
     spawn $KLIST -5
     # Might say "credentials cache" or "credentials cache file".
     expect {
-	-re "klist: Credentials cache file .* not found.*\r\n" {
+	-re "klist: No credentials cache found.*\r\n" {
 	    verbose "klist started"
 	}
 	timeout {
diff --git a/src/tests/gssapi/t_client_keytab.py b/src/tests/gssapi/t_client_keytab.py
index d26d408..ef27d5e 100644
--- a/src/tests/gssapi/t_client_keytab.py
+++ b/src/tests/gssapi/t_client_keytab.py
@@ -135,7 +135,7 @@ if bob not in out:
     fail('Authenticated as wrong principal')
 # Make sure the tickets we acquired didn't become the default
 out = realm.run([klist], expected_code=1)
-if ' not found' not in out:
+if 'No credentials cache found' not in out:
     fail('Expected error not seen')
 realm.run([kdestroy, '-A'])
 
diff --git a/src/tests/t_ccache.py b/src/tests/t_ccache.py
index ac13ef2..b33026e 100644
--- a/src/tests/t_ccache.py
+++ b/src/tests/t_ccache.py
@@ -33,7 +33,7 @@ test_keyring = (keyctl is not None and
 # Test kdestroy and klist of a non-existent ccache.
 realm.run([kdestroy])
 output = realm.run([klist], expected_code=1)
-if ' not found' not in output:
+if 'No credentials cache found' not in output:
     fail('Expected error message not seen in klist output')
 
 # Test kinit with an inaccessible ccache.
@@ -68,7 +68,7 @@ def collection_test(realm, ccname):
     realm.run([klist, '-A', '-s'])
     realm.run([kdestroy])
     output = realm.run([klist], expected_code=1)
-    if ' not found' not in output:
+    if 'No credentials cache' not in output and 'not found' not in output:
         fail('Initial kdestroy failed to destroy primary cache.')
     output = realm.run([klist, '-l'], expected_code=1)
     if not output.endswith('---\n') or output.count('\n') != 2:
diff --git a/src/tests/t_errmsg.py b/src/tests/t_errmsg.py
index ca78d0f..c9ae663 100644
--- a/src/tests/t_errmsg.py
+++ b/src/tests/t_errmsg.py
@@ -8,21 +8,21 @@ fmt1 = 'FOO Error: %M (see http://localhost:1234/%C for more information)'
 conf1 = {'libdefaults': {'err_fmt': fmt1}}
 e1 = realm.special_env('fmt1', False, krb5_conf=conf1)
 out = realm.run([klist, '-c', 'testdir/xx/yy'], env=e1, expected_code=1)
-if out != ("klist: FOO Error: Credentials cache file 'testdir/xx/yy' not "
-           "found (see http://localhost:1234/-1765328189 for more "
-           "information)\n"):
+if out != ('klist: FOO Error: No credentials cache found (filename: '
+           'testdir/xx/yy) (see http://localhost:1234/-1765328189 for more '
+           'information)\n'):
     fail('err_fmt expansion failed')
 conf2 = {'libdefaults': {'err_fmt': '%M - %C'}}
 e2 = realm.special_env('fmt2', False, krb5_conf=conf2)
 out = realm.run([klist, '-c', 'testdir/xx/yy'], env=e2, expected_code=1)
-if out != ("klist: Credentials cache file 'testdir/xx/yy' not found - "
-           "-1765328189\n"):
+if out != ('klist: No credentials cache found (filename: testdir/xx/yy) - '
+           '-1765328189\n'):
     fail('err_fmt expansion failed')
 conf3 = {'libdefaults': {'err_fmt': '%%%M %-% %C%'}}
 e3 = realm.special_env('fmt3', False, krb5_conf=conf3)
 out = realm.run([klist, '-c', 'testdir/xx/yy'], env=e3, expected_code=1)
-if out != ("klist: %Credentials cache file 'testdir/xx/yy' not found %-% "
-           "-1765328189%\n"):
+if out != ('klist: %No credentials cache found (filename: testdir/xx/yy) %-% '
+           '-1765328189%\n'):
     fail('err_fmt expansion failed')
 
 success('error message tests')
_______________________________________________
cvs-krb5 mailing list
cvs-krb5@mit.edu
https://mailman.mit.edu/mailman/listinfo/cvs-krb5

home help back first fref pref prev next nref lref last post