Fix gss_accept_sec_context error tokens

A GSS krb5 error response contains a KRB-ERROR message, which is
required to have a server principal name, although few recipients
actually use it.  Starting in 1.3, accept_sec_context would fail to
encode the error in the GSS_C_NO_NAME/GSS_C_NO_CREDENTIAL case
(introduced by #1370) because cred->princ (which became
cred->name->princ in 1.8) is unset.

This problem got worse in 1.10 because we stopped setting the server
field in all cases due to the changes for #6855.  In 1.11 the problem
got worse again when a misguided change to the mechglue started
discarding output tokens when the mechanism returns an error; the
mechglue should only do so when it itself causes the error.

Fix krb5 gss_accept_sec_context by unconditionally decoding the AP-REQ
and using krb5_rd_req_decoded, and then using the requested ticket
server in the KRB-ERROR message.  Fix the mechglue
gss_accept_sec_context by reverting that part of commit
56feee187579905c9101b0cdbdd8c6a850adcfc9.  Add a test program which
artificially induces a replay cache failure (the easiest failure we
can produce which has an associated RFC 4120 error code) and checks
that this can be communicated back to the initiator via an error
token.

ticket: 1445
target_version: 1.12
tags: pullup

3 files changed, 21 insertions, 29 deletions

diff --git a/src/lib/gssapi/krb5/accept_sec_context.c b/src/lib/gssapi/krb5/accept_sec_context.c
index 9f9b6c6799..82f03b7cc6 100644
--- a/src/lib/gssapi/krb5/accept_sec_context.c
+++ b/src/lib/gssapi/krb5/accept_sec_context.c
@@ -450,7 +450,6 @@ kg_accept_krb5(minor_status, context_handle,
     krb5_checksum reqcksum;
     krb5_gss_name_t name = NULL;
     krb5_ui_4 gss_flags = 0;
-    int decode_req_message = 0;
     krb5_gss_ctx_id_rec *ctx = NULL;
     krb5_timestamp now;
     gss_buffer_desc token;
@@ -472,6 +471,7 @@ kg_accept_krb5(minor_status, context_handle,
     krb5_enctype negotiated_etype;
     krb5_authdata_context ad_context = NULL;
     krb5_principal accprinc = NULL;
+    krb5_ap_req *request = NULL;
 
     code = krb5int_accessor (&kaccess, KRB5INT_ACCESS_VERSION);
     if (code) {
@@ -586,7 +586,6 @@ kg_accept_krb5(minor_status, context_handle,
 
     sptr = (char *) ptr;
     TREAD_STR(sptr, ap_req.data, ap_req.length);
-    decode_req_message = 1;
 
     /* construct the sender_addr */
 
@@ -603,6 +602,11 @@ kg_accept_krb5(minor_status, context_handle,
     }
 
     /* decode the AP_REQ message */
+    code = decode_krb5_ap_req(&ap_req, &request);
+    if (code) {
+        major_status = GSS_S_FAILURE;
+        goto done;
+    }
 
     /* decode the message */
 
@@ -639,8 +643,9 @@ kg_accept_krb5(minor_status, context_handle,
         }
     }
 
-    code = krb5_rd_req(context, &auth_context, &ap_req, accprinc,
-                       cred->keytab, &ap_req_options, &ticket);
+    code = krb5_rd_req_decoded(context, &auth_context, request, accprinc,
+                               cred->keytab, &ap_req_options, &ticket);
+
     krb5_free_principal(context, accprinc);
     if (code) {
         major_status = GSS_S_FAILURE;
@@ -697,7 +702,6 @@ kg_accept_krb5(minor_status, context_handle,
         }
 
         gss_flags = GSS_C_MUTUAL_FLAG | GSS_C_REPLAY_FLAG | GSS_C_SEQUENCE_FLAG;
-        decode_req_message = 0;
     } else {
         /* gss krb5 v1 */
 
@@ -777,7 +781,6 @@ kg_accept_krb5(minor_status, context_handle,
                                            there's a delegation, we'll set
                                            it below */
 #endif
-        decode_req_message = 0;
 
         /* if the checksum length > 24, there are options to process */
 
@@ -1203,26 +1206,12 @@ fail:
 
     *minor_status = code;
 
-    /*
-     * If decode_req_message is set, then we need to decode the ap_req
-     * message to determine whether or not to send a response token.
-     * We need to do this because for some errors we won't be able to
-     * decode the authenticator to read out the gss_flags field.
-     */
-    if (decode_req_message) {
-        krb5_ap_req      * request;
-
-        if (decode_krb5_ap_req(&ap_req, &request))
-            goto done;
-
-        if (request->ap_options & AP_OPTS_MUTUAL_REQUIRED)
-            gss_flags |= GSS_C_MUTUAL_FLAG;
-        krb5_free_ap_req(context, request);
-    }
-
-    if (cred
-        && ((gss_flags & GSS_C_MUTUAL_FLAG)
-            || (major_status == GSS_S_CONTINUE_NEEDED))) {
+    /* We may have failed before being able to read the GSS flags from the
+     * authenticator, so also check the request AP options. */
+    if (cred != NULL && request != NULL &&
+        ((gss_flags & GSS_C_MUTUAL_FLAG) ||
+         (request->ap_options & AP_OPTS_MUTUAL_REQUIRED) ||
+         major_status == GSS_S_CONTINUE_NEEDED)) {
         unsigned int tmsglen;
         int toktype;
 
@@ -1240,6 +1229,7 @@ fail:
         (void) krb5_us_timeofday(context, &krb_error_data.stime,
                                  &krb_error_data.susec);
 
+        krb_error_data.server = request->ticket->server;
         code = krb5_mk_error(context, &krb_error_data, &scratch);
         if (code)
             goto done;
@@ -1262,6 +1252,7 @@ fail:
     }
 
 done:
+    krb5_free_ap_req(context, request);
     if (cred)
         k5_mutex_unlock(&cred->lock);
     if (defcred)
diff --git a/src/lib/gssapi/mechglue/g_accept_sec_context.c b/src/lib/gssapi/mechglue/g_accept_sec_context.c
index dae83cc44d..b8f128bc48 100644
--- a/src/lib/gssapi/mechglue/g_accept_sec_context.c
+++ b/src/lib/gssapi/mechglue/g_accept_sec_context.c
@@ -269,6 +269,9 @@ gss_cred_id_t *		d_cred;
 			status = temp_status;
 			*minor_status = temp_minor_status;
 			map_error(minor_status, mech);
+			if (output_token->length)
+			    (void) gss_release_buffer(&temp_minor_status,
+						      output_token);
 			goto error_out;
 		    }
 		    *src_name = tmp_src_name;
@@ -357,9 +360,6 @@ error_out:
 	(void) gss_release_buffer(&temp_minor_status,
 				  (gss_buffer_t)tmp_src_name);
 
-    if (output_token->length)
-	(void) gss_release_buffer(&temp_minor_status, output_token);
-
     return (status);
 }
 #endif /* LEAN_CLIENT */
diff --git a/src/lib/krb5_32.def b/src/lib/krb5_32.def
index b3ce21afe1..dd0a16ccc0 100644
--- a/src/lib/krb5_32.def
+++ b/src/lib/krb5_32.def
@@ -451,3 +451,4 @@ EXPORTS
 	krb5_responder_pkinit_set_answer		@422
 	krb5_responder_pkinit_challenge_free		@423
 	krb5_auth_con_setpermetypes			@424 ; PRIVATE GSSAPI
+	krb5_rd_req_decoded				@425 ; PRIVATE GSSAPI