Pull in fix to improve SPNEGO error messageskrb5-1.11.3-18.fc19

- pull in fix from master to make reporting of errors encountered by the
  SPNEGO mechanism work better (RT#7045, part of #1043962)

2 files changed, 182 insertions, 1 deletions

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 <simo@redhat.com>
+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 <ghudson@mit.edu>
+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 <nalin@redhat.com> - 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 <nalin@redhat.com>
 - 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