From: Nicolas Williams Date: Thu, 30 Oct 2014 00:42:49 +0000 (-0500) Subject: Include file ccache name in error messages X-Git-Tag: krb5-1.14-alpha1~187 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=98b55e86d7ec8b0a3b9b9f9b415ffdf78f4fd2e8;p=thirdparty%2Fkrb5.git 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 295f959f04..de9c968dc1 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 0c7a0c7d51..c16354818d 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 d26d408cd7..ef27d5e599 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 ac13ef28b5..b33026e42f 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 ca78d0f55b..c9ae6637fa 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')