From eadaba4f045a22f77a57a15b6d6ea95a643faf3b Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Dec 19 2013 16:49:31 +0000 Subject: Pull in fix to improve SPNEGO error messages - pull in fix from master to make reporting of errors encountered by the SPNEGO mechanism work better (RT#7045, part of #1043962) --- diff --git a/krb5-1.11.3-spnego_error_messages.patch b/krb5-1.11.3-spnego_error_messages.patch new file mode 100644 index 0000000..0a14bd3 --- /dev/null +++ b/krb5-1.11.3-spnego_error_messages.patch @@ -0,0 +1,175 @@ +Test tweaked for 1.11.3. + +commit d160bc733a3dbeb6d84f4e175234ff18738d9f66 +Author: Simo Sorce +Date: Tue Dec 17 16:15:14 2013 -0500 + + Let SPNEGO display mechanism errors + + To avoid potential recursion we use a thread local variable that tells + us whether the ancestor was called via spnego_gss_display_name(). If + we detect recursion, we assume that we returned a com_err code like + ENOMEM and call error_message(); in the worst case that will result in + an "Unknown error" message. + + [ghudson@mit.edu: Edited comments and commit message; removed an + unneeded line of code.] + + ticket: 7045 + target_version: 1.12.1 + tags: pullup + +diff --git a/src/include/k5-thread.h b/src/include/k5-thread.h +index 1b7fa69..ab46ec3 100644 +--- a/src/include/k5-thread.h ++++ b/src/include/k5-thread.h +@@ -406,6 +406,7 @@ typedef enum { + K5_KEY_GSS_KRB5_SET_CCACHE_OLD_NAME, + K5_KEY_GSS_KRB5_CCACHE_NAME, + K5_KEY_GSS_KRB5_ERROR_MESSAGE, ++ K5_KEY_GSS_SPNEGO_STATUS, + #if defined(__MACH__) && defined(__APPLE__) + K5_KEY_IPC_CONNECTION_INFO, + #endif +diff --git a/src/lib/gssapi/spnego/spnego_mech.c b/src/lib/gssapi/spnego/spnego_mech.c +index 06cfab0..7e4bf90 100644 +--- a/src/lib/gssapi/spnego/spnego_mech.c ++++ b/src/lib/gssapi/spnego/spnego_mech.c +@@ -85,8 +85,8 @@ extern int gssint_put_der_length(unsigned int, unsigned char **, unsigned int); + + + /* private routines for spnego_mechanism */ +-static spnego_token_t make_spnego_token(char *); +-static gss_buffer_desc make_err_msg(char *); ++static spnego_token_t make_spnego_token(const char *); ++static gss_buffer_desc make_err_msg(const char *); + static int g_token_size(gss_OID_const, unsigned int); + static int g_make_token_header(gss_OID_const, unsigned int, + unsigned char **, unsigned int); +@@ -316,6 +316,12 @@ int gss_krb5int_lib_init(void); + + int gss_spnegoint_lib_init(void) + { ++ int err; ++ ++ err = k5_key_register(K5_KEY_GSS_SPNEGO_STATUS, NULL); ++ if (err) ++ return err; ++ + #ifdef _GSS_STATIC_LINK + return gss_spnegomechglue_init(); + #else +@@ -1791,7 +1797,6 @@ cleanup: + } + #endif /* LEAN_CLIENT */ + +- + /*ARGSUSED*/ + OM_uint32 KRB5_CALLCONV + spnego_gss_display_status( +@@ -1802,6 +1807,9 @@ spnego_gss_display_status( + OM_uint32 *message_context, + gss_buffer_t status_string) + { ++ OM_uint32 maj = GSS_S_COMPLETE; ++ int ret; ++ + dsyslog("Entering display_status\n"); + + *message_context = 0; +@@ -1832,13 +1840,31 @@ spnego_gss_display_status( + "return a valid token")); + break; + default: +- status_string->length = 0; +- status_string->value = ""; ++ /* Not one of our minor codes; might be from a mech. Call back ++ * to gss_display_status, but first check for recursion. */ ++ if (k5_getspecific(K5_KEY_GSS_SPNEGO_STATUS) != NULL) { ++ /* Perhaps we returned a com_err code like ENOMEM. */ ++ const char *err = error_message(status_value); ++ *status_string = make_err_msg(err); ++ break; ++ } ++ /* Set a non-null pointer value; doesn't matter which one. */ ++ ret = k5_setspecific(K5_KEY_GSS_SPNEGO_STATUS, &ret); ++ if (ret != 0) { ++ *minor_status = ret; ++ maj = GSS_S_FAILURE; ++ break; ++ } ++ maj = gss_display_status(minor_status, status_value, ++ status_type, mech_type, ++ message_context, status_string); ++ /* This is unlikely to fail; not much we can do if it does. */ ++ (void)k5_setspecific(K5_KEY_GSS_SPNEGO_STATUS, NULL); + break; + } + + dsyslog("Leaving display_status\n"); +- return (GSS_S_COMPLETE); ++ return maj; + } + + +@@ -3550,13 +3576,13 @@ negotiate_mech(gss_OID_set supported, gss_OID_set received, + * these routines will be changes to return the error string. + */ + static spnego_token_t +-make_spnego_token(char *name) ++make_spnego_token(const char *name) + { + return (spnego_token_t)strdup(name); + } + + static gss_buffer_desc +-make_err_msg(char *name) ++make_err_msg(const char *name) + { + gss_buffer_desc buffer; + +commit 4faca53e3a8ee213d43da8998f6889e7bfd36248 +Author: Greg Hudson +Date: Wed Dec 18 16:03:16 2013 -0500 + + Test SPNEGO error message in t_s4u.py + + Now that #7045 is fixed, we can check for the correct error message + from t_s4u2proxy_krb5 with --spnego. + + ticket: 7045 + +diff --git a/src/tests/gssapi/t_s4u.py b/src/tests/gssapi/t_s4u.py +index 67dc810..e4aa259 100644 +--- a/src/tests/gssapi/t_s4u.py ++++ b/src/tests/gssapi/t_s4u.py +@@ -30,12 +30,12 @@ if ('auth1: ' + realm.user_princ not in output or + 'NOT_ALLOWED_TO_DELEGATE' not in output): + fail('krb5 -> s4u2proxy') + +-# Again with SPNEGO. Bug #7045 prevents us from checking the error +-# message, but we can at least exercise the code. ++# Again with SPNEGO. + output = realm.run_as_server(['./t_s4u2proxy_krb5', '--spnego', usercache, + storagecache, '-', pservice1, pservice2], + expected_code=1) +-if ('auth1: ' + realm.user_princ not in output): ++if ('auth1: ' + realm.user_princ not in output or ++ 'NOT_ALLOWED_TO_DELEGATE' not in output): + fail('krb5 -> s4u2proxy (SPNEGO)') + + # Try krb5 -> S4U2Proxy without forwardable user creds. This should +@@ -66,10 +66,9 @@ if 'NOT_ALLOWED_TO_DELEGATE' not in output: + fail('s4u2self') + + # Again with SPNEGO. This uses SPNEGO for the initial authentication, +-# but still uses krb5 for S4U2Proxy (the delegated cred is returned as ++# but still uses krb5 for S4U2Proxy--the delegated cred is returned as + # a krb5 cred, not a SPNEGO cred, and t_s4u uses the delegated cred +-# directly rather than saving and reacquiring it) so bug #7045 does +-# not apply and we can verify the error message. ++# directly rather than saving and reacquiring it. + output = realm.run_as_server(['./t_s4u', '--spnego', puser, pservice2], + expected_code=1) + if 'NOT_ALLOWED_TO_DELEGATE' not in output: + fail('s4u2self') diff --git a/krb5.spec b/krb5.spec index 4d96aa5..26b955f 100644 --- a/krb5.spec +++ b/krb5.spec @@ -32,7 +32,7 @@ Summary: The Kerberos network authentication system Name: krb5 Version: 1.11.3 -Release: 17%{?dist} +Release: 18%{?dist} # Maybe we should explode from the now-available-to-everybody tarball instead? # http://web.mit.edu/kerberos/dist/krb5/1.11/krb5-1.11.3-signed.tar Source0: krb5-%{version}.tar.gz @@ -109,6 +109,7 @@ Patch144: krb5-master-gss_oid_leak.patch Patch145: krb5-master-keytab_close.patch Patch146: krb5-1.11-preauthcore.patch Patch147: krb5-1.11.3-copy_context.patch +Patch148: krb5-1.11.3-spnego_error_messages.patch # Patches for otp plugin backport Patch201: krb5-1.11.2-keycheck.patch @@ -355,6 +356,7 @@ ln -s NOTICE LICENSE %patch145 -p1 -b .keytab_close %patch146 -p0 -b .preauthcore %patch147 -p1 -b .copy_context +%patch148 -p1 -b .spnego_error_messages %patch201 -p1 -b .keycheck %patch202 -p1 -b .otp @@ -949,6 +951,10 @@ exit 0 %{_sbindir}/uuserver %changelog +* Thu Dec 19 2013 Nalin Dahyabhai - 1.11.3-18 +- pull in fix from master to make reporting of errors encountered by + the SPNEGO mechanism work better (RT#7045, part of #1043962) + * Thu Dec 19 2013 Nalin Dahyabhai - update a test wrapper to properly handle things that the new libkrad does, and add python-pyrad as a build requirement so that we can run its tests