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)

diff --git a/src/lib/krb5/ccache/cc_file.c b/src/lib/krb5/ccache/cc_file.c
index 295f959f04d12d0775d7ea2b3ae245a1a99fba01..de9c968dc1fbc6b159d605d16c711e0462e13e52 100644 (file)
--- 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 0c7a0c7d516efcbd0f5095d5226d7f5ebb14e6df..c16354818d55f84a3ac592ac8171ba70b16f76cd 100644 (file)
--- 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 d26d408cd7cbe0f8165f3a03bbd6e5215a6dc49a..ef27d5e599bddca94ba3ee41e8534136467b9111 100644 (file)
--- 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 ac13ef28b50a53abce69c05bd53c51f5fd34972d..b33026e42f150433baa82e2dfeea954998d578b4 100644 (file)
--- 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 ca78d0f55bb63ea6e55dba9d6067ff48076da770..c9ae6637fa12c137a5a47ea3463d1b75231e8d7c 100644 (file)
--- 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')