From: Greg Hudson <ghudson@mit.edu>
Date: Tue, 25 Mar 2025 20:09:28 +0000 (-0400)
Subject: Fix IAKERB error handling
X-Git-Tag: krb5-1.22-beta1~17
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ed56e5b38334e874ea36c68f1ca559da582742db;p=thirdparty%2Fkrb5.git

Fix IAKERB error handling

KRB-ERROR has non-optional realm and sname fields, so we must set a
server field in the krb5_error object before calling krb5_mk_error().

Add KRB-ERROR processing to the initiator state machine.

ticket: 9169 (new)
---

diff --git a/src/lib/gssapi/krb5/iakerb.c b/src/lib/gssapi/krb5/iakerb.c
index 1dd34287be..a0c64403be 100644
--- a/src/lib/gssapi/krb5/iakerb.c
+++ b/src/lib/gssapi/krb5/iakerb.c
@@ -78,6 +78,46 @@ iakerb_release_context(iakerb_ctx_id_t ctx)
     free(ctx);
 }
 
+/* Encode a KRB-ERROR message with the given protocol code.  Use the server
+ * principal from verifier_cred if one is available. */
+static krb5_error_code
+iakerb_mk_error(krb5_context context, gss_cred_id_t verifier_cred,
+                int protocol_err, krb5_data *enc_err)
+{
+    krb5_error error = { 0 };
+    krb5_gss_cred_id_t cred = (krb5_gss_cred_id_t)verifier_cred;
+
+    error.error = protocol_err;
+
+    /* We must provide a server principal, although we expect the recipient to
+     * care chiefly about the error code. */
+    if (cred != NULL && cred->name != NULL)
+        error.server = cred->name->princ;
+    else
+        error.server = (krb5_principal)krb5_anonymous_principal();
+
+    return krb5_mk_error(context, &error, enc_err);
+}
+
+/* Decode a KRB-ERROR message and return the associated com_err code. */
+static krb5_error_code
+iakerb_rd_error(krb5_context context, const krb5_data *enc_err)
+{
+    krb5_error_code ret;
+    krb5_error *error;
+
+    ret = krb5_rd_error(context, enc_err, &error);
+    if (ret)
+        return ret;
+
+    if (error->error > 0 && error->error <= KRB_ERR_MAX)
+        ret = error->error + ERROR_TABLE_BASE_krb5;
+    else
+        ret = KRB5KRB_ERR_GENERIC;
+    krb5_free_error(context, error);
+    return ret;
+}
+
 /*
  * Create a IAKERB-FINISHED structure containing a checksum of
  * the entire IAKERB exchange.
@@ -298,7 +338,6 @@ iakerb_acceptor_realm(iakerb_ctx_id_t ctx, gss_cred_id_t verifier_cred,
     OM_uint32 dummy;
     krb5_gss_cred_id_t cred = (krb5_gss_cred_id_t)verifier_cred;
     krb5_data realm = empty_data(), reply = empty_data();
-    krb5_error error = { 0 };
     char *defrealm = NULL;
 
     /* Get the acceptor realm from the verifier cred if we can; otherwise try
@@ -310,8 +349,8 @@ iakerb_acceptor_realm(iakerb_ctx_id_t ctx, gss_cred_id_t verifier_cred,
         ret = krb5_get_default_realm(ctx->k5c, &defrealm);
         if (ret) {
             /* Generate an error reply if there is no default realm. */
-            error.error = KRB_ERR_GENERIC;
-            ret = krb5_mk_error(ctx->k5c, &error, &reply);
+            ret = iakerb_mk_error(ctx->k5c, verifier_cred, KRB_ERR_GENERIC,
+                                  &reply);
             if (ret)
                 goto cleanup;
         } else {
@@ -346,7 +385,7 @@ iakerb_acceptor_step(iakerb_ctx_id_t ctx, gss_cred_id_t verifier_cred,
     krb5_data request = empty_data(), reply = empty_data();
     krb5_data realm = empty_data();
     OM_uint32 tmp;
-    int tcp_only, use_primary;
+    int tcp_only, use_primary, protocol_err;
     krb5_ui_4 kdc_code;
 
     output_token->length = 0;
@@ -396,15 +435,10 @@ iakerb_acceptor_step(iakerb_ctx_id_t ctx, gss_cred_id_t verifier_cred,
     }
 
     if (code == KRB5_KDC_UNREACH || code == KRB5_REALM_UNKNOWN) {
-        krb5_error error;
-
-        memset(&error, 0, sizeof(error));
-        if (code == KRB5_KDC_UNREACH)
-            error.error = KRB_AP_ERR_IAKERB_KDC_NO_RESPONSE;
-        else if (code == KRB5_REALM_UNKNOWN)
-            error.error = KRB_AP_ERR_IAKERB_KDC_NOT_FOUND;
-
-        code = krb5_mk_error(ctx->k5c, &error, &reply);
+        protocol_err = (code == KRB5_KDC_UNREACH) ?
+            KRB_AP_ERR_IAKERB_KDC_NO_RESPONSE :
+            KRB_AP_ERR_IAKERB_KDC_NOT_FOUND;
+        code = iakerb_mk_error(ctx->k5c, verifier_cred, protocol_err, &reply);
         if (code != 0)
             goto cleanup;
     } else if (code != 0)
@@ -563,6 +597,11 @@ iakerb_initiator_step(iakerb_ctx_id_t ctx,
         code = iakerb_save_token(ctx, input_token);
         if (code != 0)
             goto cleanup;
+
+        if (krb5_is_krb_error(&in)) {
+            code = iakerb_rd_error(ctx->k5c, &in);
+            goto cleanup;
+        }
     }
 
     switch (ctx->state) {
diff --git a/src/tests/gssapi/t_gssapi.py b/src/tests/gssapi/t_gssapi.py
index 5e566563f4..cf57762e43 100755
--- a/src/tests/gssapi/t_gssapi.py
+++ b/src/tests/gssapi/t_gssapi.py
@@ -28,9 +28,14 @@ realm.run(['./t_iakerb', 'p:' + realm.user_princ, password('user'),
 realm.run(['./t_iakerb', 'e:user', password('user'), 'h:host@' + hostname,
            'h:host'])
 
-# Test IAKERB realm discovery without default_realm set.  (Use a
-# GSS_KRB5_NT_PRINCIPAL_NAME acceptor name so that
-# gss_accept_sec_context() knows the realm.)
+# Test IAKERB realm discovery without default_realm set.  We get an
+# error because the acceptor does not know the realm.
+realm.run(['./t_iakerb', 'e:user', password('user'), 'h:host@' + hostname,
+           'h:host'], env=no_default, expected_code=1,
+          expected_msg='Generic error')
+
+# Test again, using a GSS_KRB5_NT_PRINCIPAL_NAME acceptor name so that
+# gss_accept_sec_context() knows the realm.
 realm.run(['./t_iakerb', 'e:user', password('user'), 'h:host@' + hostname,
            'p:' + realm.host_princ], env=no_default)