krb5 commit: Modernize kdc_authdata.c

Greg Hudson ghudson at mit.edu
Mon Jun 15 12:56:01 EDT 2015


https://github.com/krb5/krb5/commit/4325964a5d472422cb0a1600676787d7bcfde5d2
commit 4325964a5d472422cb0a1600676787d7bcfde5d2
Author: Greg Hudson <ghudson at mit.edu>
Date:   Sat Sep 27 16:14:02 2014 -0400

    Modernize kdc_authdata.c
    
    Adjust whitespace, identifier names, and in some cases flow control.
    No functional changes.

 src/kdc/kdc_authdata.c |  670 ++++++++++++++++++++----------------------------
 1 files changed, 282 insertions(+), 388 deletions(-)

diff --git a/src/kdc/kdc_authdata.c b/src/kdc/kdc_authdata.c
index afe18f0..704e130 100644
--- a/src/kdc/kdc_authdata.c
+++ b/src/kdc/kdc_authdata.c
@@ -105,26 +105,22 @@ unload_authdata_plugins(krb5_context context)
     return 0;
 }
 
-/*
- * Returns TRUE if authdata should be filtered when copying from
- * untrusted authdata.
- */
+/* Return true if authdata should be filtered when copying from untrusted
+ * authdata.  If desired_type is non-zero, look only for that type. */
 static krb5_boolean
-is_kdc_issued_authdatum (krb5_context context,
-                         krb5_authdata *authdata,
-                         krb5_authdatatype desired_type)
+is_kdc_issued_authdatum(krb5_context context, krb5_authdata *authdata,
+                        krb5_authdatatype desired_type)
 {
-    krb5_boolean ret = FALSE;
+    krb5_boolean result = FALSE;
     krb5_authdatatype ad_type;
     unsigned int i, count = 0;
-    krb5_authdatatype *ad_types = NULL;
+    krb5_authdatatype *ad_types, *containee_types = NULL;
 
     if (authdata->ad_type == KRB5_AUTHDATA_IF_RELEVANT) {
-        if (krb5int_get_authdata_containee_types(context,
-                                                 authdata,
-                                                 &count,
-                                                 &ad_types) != 0)
+        if (krb5int_get_authdata_containee_types(context, authdata, &count,
+                                                 &containee_types) != 0)
             goto cleanup;
+        ad_types = containee_types;
     } else {
         ad_type = authdata->ad_type;
         count = 1;
@@ -136,79 +132,66 @@ is_kdc_issued_authdatum (krb5_context context,
         case KRB5_AUTHDATA_SIGNTICKET:
         case KRB5_AUTHDATA_KDC_ISSUED:
         case KRB5_AUTHDATA_WIN2K_PAC:
-            ret = desired_type ? (desired_type == ad_types[i]) : TRUE;
+            result = desired_type ? (desired_type == ad_types[i]) : TRUE;
             break;
         default:
-            ret = FALSE;
+            result = FALSE;
             break;
         }
-        if (ret)
+        if (result)
             break;
     }
 
 cleanup:
-    if (authdata->ad_type == KRB5_AUTHDATA_IF_RELEVANT &&
-        ad_types != NULL)
-        free(ad_types);
-
-    return ret;
+    free(containee_types);
+    return result;
 }
 
+/* Return true if authdata contains any elements which should only come from
+ * the KDC.  If desired_type is non-zero, look only for that type. */
 static krb5_boolean
-has_kdc_issued_authdata (krb5_context context,
-                         krb5_authdata **authdata,
-                         krb5_authdatatype desired_type)
+has_kdc_issued_authdata(krb5_context context, krb5_authdata **authdata,
+                        krb5_authdatatype desired_type)
 {
     int i;
-    krb5_boolean ret = FALSE;
 
-    if (authdata != NULL) {
-        for (i = 0; authdata[i] != NULL; i++) {
-            if (is_kdc_issued_authdatum(context, authdata[i], desired_type)) {
-                ret = TRUE;
-                break;
-            }
-        }
+    if (authdata == NULL)
+        return FALSE;
+    for (i = 0; authdata[i] != NULL; i++) {
+        if (is_kdc_issued_authdatum(context, authdata[i], desired_type))
+            return TRUE;
     }
-
-    return ret;
+    return FALSE;
 }
 
+/* Return true if authdata contains any mandatory-for-KDC elements. */
 static krb5_boolean
-has_mandatory_for_kdc_authdata (krb5_context context,
-                                krb5_authdata **authdata)
+has_mandatory_for_kdc_authdata(krb5_context context, krb5_authdata **authdata)
 {
     int i;
-    krb5_boolean ret = FALSE;
 
-    if (authdata != NULL) {
-        for (i = 0; authdata[i] != NULL; i++) {
-            if (authdata[i]->ad_type == KRB5_AUTHDATA_MANDATORY_FOR_KDC) {
-                ret = TRUE;
-                break;
-            }
-        }
+    if (authdata == NULL)
+        return FALSE;
+    for (i = 0; authdata[i] != NULL; i++) {
+        if (authdata[i]->ad_type == KRB5_AUTHDATA_MANDATORY_FOR_KDC)
+            return TRUE;
     }
-
-    return ret;
+    return FALSE;
 }
 
 /*
- * Merge authdata.
- *
- * If copy is FALSE, in_authdata is invalid on successful return.
- * If ignore_kdc_issued is TRUE, KDC-issued authdata is not copied.
+ * Add the elements of in_authdata to out_authdata.  If copy is false,
+ * in_authdata is invalid on successful return.  If ignore_kdc_issued is true,
+ * KDC-issued authdata is not copied.
  */
 static krb5_error_code
-merge_authdata (krb5_context context,
-                krb5_authdata **in_authdata,
-                krb5_authdata ***out_authdata,
-                krb5_boolean copy,
-                krb5_boolean ignore_kdc_issued)
+merge_authdata(krb5_context context, krb5_authdata **in_authdata,
+               krb5_authdata ***out_authdata, krb5_boolean copy,
+               krb5_boolean ignore_kdc_issued)
 {
+    krb5_error_code ret;
     size_t i, j, nadata = 0;
     krb5_authdata **in_copy = NULL, **authdata = *out_authdata;
-    krb5_error_code code;
 
     if (in_authdata == NULL || in_authdata[0] == NULL)
         return 0;
@@ -222,9 +205,9 @@ merge_authdata (krb5_context context,
         ;
 
     if (copy) {
-        code = krb5_copy_authdata(context, in_authdata, &in_copy);
-        if (code != 0)
-            return code;
+        ret = krb5_copy_authdata(context, in_authdata, &in_copy);
+        if (ret)
+            return ret;
         in_authdata = in_copy;
     }
 
@@ -239,8 +222,9 @@ merge_authdata (krb5_context context,
             is_kdc_issued_authdatum(context, in_authdata[i], 0)) {
             free(in_authdata[i]->contents);
             free(in_authdata[i]);
-        } else
+        } else {
             authdata[nadata + j++] = in_authdata[i];
+        }
     }
 
     authdata[nadata + j] = NULL;
@@ -260,69 +244,57 @@ merge_authdata (krb5_context context,
 /* Copy TGS-REQ authorization data into the ticket authdata. */
 static krb5_error_code
 copy_request_authdata(krb5_context context, krb5_keyblock *client_key,
-                      krb5_kdc_req *request,
-                      krb5_enc_tkt_part *enc_tkt_request,
+                      krb5_kdc_req *req, krb5_enc_tkt_part *enc_tkt_req,
                       krb5_authdata ***tkt_authdata)
 {
-    krb5_error_code code;
-    krb5_data scratch;
+    krb5_error_code ret;
+    krb5_data plaintext;
 
-    assert(enc_tkt_request != NULL);
+    assert(enc_tkt_req != NULL);
 
-    scratch.length = request->authorization_data.ciphertext.length;
-    scratch.data = malloc(scratch.length);
-    if (scratch.data == NULL)
-        return ENOMEM;
+    ret = alloc_data(&plaintext, req->authorization_data.ciphertext.length);
+    if (ret)
+        return ret;
 
     /*
-     * RFC 4120 requires authdata in the TGS body to be encrypted in
-     * the subkey with usage 5 if a subkey is present, and in the TGS
-     * session key with key usage 4 if it is not.  Prior to krb5 1.7,
-     * we got this wrong, always decrypting the authorization data
-     * with the TGS session key and usage 4.  For the sake of
-     * conservatism, try the decryption the old way (wrong if
-     * client_key is a subkey) first, and then try again the right way
-     * (in the case where client_key is a subkey) if the first way
-     * fails.
-     */
-    code = krb5_c_decrypt(context,
-                          enc_tkt_request->session,
-                          KRB5_KEYUSAGE_TGS_REQ_AD_SESSKEY,
-                          0, &request->authorization_data,
-                          &scratch);
-    if (code != 0)
-        code = krb5_c_decrypt(context,
-                              client_key,
-                              KRB5_KEYUSAGE_TGS_REQ_AD_SUBKEY,
-                              0, &request->authorization_data,
-                              &scratch);
-
-    if (code != 0) {
-        free(scratch.data);
-        return code;
-    }
-
-    /* scratch now has the authorization data, so we decode it, and make
-     * it available to subsequent authdata plugins
+     * RFC 4120 requires authdata in the TGS body to be encrypted in the subkey
+     * with usage 5 if a subkey is present, and in the TGS session key with key
+     * usage 4 if it is not.  Prior to krb5 1.7, we got this wrong, always
+     * decrypting the authorization data with the TGS session key and usage 4.
+     * For the sake of conservatism, try the decryption the old way (wrong if
+     * client_key is a subkey) first, and then try again the right way (in the
+     * case where client_key is a subkey) if the first way fails.
      */
-    code = decode_krb5_authdata(&scratch, &request->unenc_authdata);
-    if (code != 0) {
-        free(scratch.data);
-        return code;
+    ret = krb5_c_decrypt(context, enc_tkt_req->session,
+                         KRB5_KEYUSAGE_TGS_REQ_AD_SESSKEY, 0,
+                         &req->authorization_data, &plaintext);
+    if (ret) {
+        ret = krb5_c_decrypt(context, client_key,
+                             KRB5_KEYUSAGE_TGS_REQ_AD_SUBKEY, 0,
+                             &req->authorization_data, &plaintext);
     }
+    if (ret)
+        goto cleanup;
 
-    free(scratch.data);
+    /* Decode the decrypted authdata and make it available to modules in the
+     * request. */
+    ret = decode_krb5_authdata(&plaintext, &req->unenc_authdata);
+    if (ret)
+        goto cleanup;
 
-    if (has_mandatory_for_kdc_authdata(context, request->unenc_authdata))
-        return KRB5KDC_ERR_POLICY;
+    if (has_mandatory_for_kdc_authdata(context, req->unenc_authdata)) {
+        ret = KRB5KDC_ERR_POLICY;
+        goto cleanup;
+    }
 
-    code = merge_authdata(context,
-                          request->unenc_authdata,
-                          tkt_authdata,
-                          TRUE,            /* copy */
-                          TRUE);    /* ignore_kdc_issued */
+    /* Add a copy of the requested authdata to the ticket, ignoring KDC-issued
+     * types. */
+    ret = merge_authdata(context, req->unenc_authdata, tkt_authdata, TRUE,
+                         TRUE);
 
-    return code;
+cleanup:
+    free(plaintext.data);
+    return ret;
 }
 
 /* Copy TGT authorization data into the ticket authdata. */
@@ -333,11 +305,9 @@ copy_tgt_authdata(krb5_context context, krb5_kdc_req *request,
     if (has_mandatory_for_kdc_authdata(context, tgt_authdata))
         return KRB5KDC_ERR_POLICY;
 
-    return merge_authdata(context,
-                          tgt_authdata,
-                          tkt_authdata,
-                          TRUE,            /* copy */
-                          TRUE);    /* ignore_kdc_issued */
+    /* Add a copy of the TGT authdata to the ticket, ignoring KDC-issued
+     * types. */
+    return merge_authdata(context, tgt_authdata, tkt_authdata, TRUE, TRUE);
 }
 
 /* Fetch authorization data from KDB module. */
@@ -346,13 +316,13 @@ fetch_kdb_authdata(krb5_context context, unsigned int flags,
                    krb5_db_entry *client, krb5_db_entry *server,
                    krb5_db_entry *krbtgt, krb5_keyblock *client_key,
                    krb5_keyblock *server_key, krb5_keyblock *krbtgt_key,
-                   krb5_kdc_req *request, krb5_const_principal for_user_princ,
-                   krb5_enc_tkt_part *enc_tkt_request,
+                   krb5_kdc_req *req, krb5_const_principal for_user_princ,
+                   krb5_enc_tkt_part *enc_tkt_req,
                    krb5_enc_tkt_part *enc_tkt_reply)
 {
-    krb5_error_code code;
+    krb5_error_code ret;
     krb5_authdata **tgt_authdata, **db_authdata = NULL;
-    krb5_boolean tgs_req = (request->msg_type == KRB5_TGS_REQ);
+    krb5_boolean tgs_req = (req->msg_type == KRB5_TGS_REQ);
     krb5_const_principal actual_client;
 
     /*
@@ -366,16 +336,16 @@ fetch_kdb_authdata(krb5_context context, unsigned int flags,
      * be present.
      */
     if (tgs_req) {
-        assert(enc_tkt_request != NULL);
+        assert(enc_tkt_req != NULL);
 
         if (isflagset(server->attributes, KRB5_KDB_NO_AUTH_DATA_REQUIRED))
             return 0;
 
-        if (enc_tkt_request->authorization_data == NULL &&
+        if (enc_tkt_req->authorization_data == NULL &&
             !isflagset(flags, KRB5_KDB_FLAG_CROSS_REALM | KRB5_KDB_FLAGS_S4U))
             return 0;
 
-        assert(enc_tkt_reply->times.authtime == enc_tkt_request->times.authtime);
+        assert(enc_tkt_reply->times.authtime == enc_tkt_req->times.authtime);
     } else {
         if (!isflagset(flags, KRB5_KDB_FLAG_INCLUDE_PAC))
             return 0;
@@ -391,52 +361,42 @@ fetch_kdb_authdata(krb5_context context, unsigned int flags,
     else
         actual_client = enc_tkt_reply->client;
 
-    tgt_authdata = tgs_req ? enc_tkt_request->authorization_data : NULL;
-    code = krb5_db_sign_authdata(context, flags, actual_client, client,
-                                 server, krbtgt, client_key, server_key,
-                                 krbtgt_key, enc_tkt_reply->session,
-                                 enc_tkt_reply->times.authtime, tgt_authdata,
-                                 &db_authdata);
-    if (code == 0) {
-        code = merge_authdata(context,
-                              db_authdata,
-                              &enc_tkt_reply->authorization_data,
-                              FALSE,        /* !copy */
-                              FALSE);        /* !ignore_kdc_issued */
-        if (code != 0)
-            krb5_free_authdata(context, db_authdata);
-    } else if (code == KRB5_PLUGIN_OP_NOTSUPP)
-        code = 0;
-
-    return code;
+    tgt_authdata = tgs_req ? enc_tkt_req->authorization_data : NULL;
+    ret = krb5_db_sign_authdata(context, flags, actual_client, client,
+                                server, krbtgt, client_key, server_key,
+                                krbtgt_key, enc_tkt_reply->session,
+                                enc_tkt_reply->times.authtime, tgt_authdata,
+                                &db_authdata);
+    if (ret)
+        return (ret == KRB5_PLUGIN_OP_NOTSUPP) ? 0 : ret;
+
+    /* Add the KDB authdata to the ticket, without copying or filtering. */
+    ret = merge_authdata(context, db_authdata,
+                         &enc_tkt_reply->authorization_data, FALSE, FALSE);
+    if (ret)
+        krb5_free_authdata(context, db_authdata);
+    return ret;
 }
 
 static krb5_error_code
-make_ad_signedpath_data(krb5_context context,
-                        krb5_const_principal client,
-                        krb5_timestamp authtime,
-                        krb5_principal *deleg_path,
-                        krb5_pa_data **method_data,
-                        krb5_authdata **authdata,
-                        krb5_data **data)
+make_signedpath_data(krb5_context context, krb5_const_principal client,
+                     krb5_timestamp authtime, krb5_principal *deleg_path,
+                     krb5_pa_data **method_data, krb5_authdata **authdata,
+                     krb5_data **data)
 {
-    krb5_ad_signedpath_data         sp_data;
-    krb5_authdata                 **sign_authdata = NULL;
-    int                             i, j;
-    krb5_error_code                 code;
+    krb5_error_code ret;
+    krb5_ad_signedpath_data sp_data;
+    krb5_authdata **sign_authdata = NULL;
+    size_t i, j, count;
 
     memset(&sp_data, 0, sizeof(sp_data));
 
-    if (authdata != NULL) {
-        for (i = 0; authdata[i] != NULL; i++)
-            ;
-    } else
-        i = 0;
-
-    if (i != 0) {
-        sign_authdata = k5calloc(i + 1, sizeof(krb5_authdata *), &code);
+    for (count = 0; authdata != NULL && authdata[count] != NULL; count++);
+    if (count != 0) {
+        /* Make a shallow copy with AD-SIGNTICKET filtered out. */
+        sign_authdata = k5calloc(count + 1, sizeof(krb5_authdata *), &ret);
         if (sign_authdata == NULL)
-            return code;
+            return ret;
 
         for (i = 0, j = 0; authdata[i] != NULL; i++) {
             if (is_kdc_issued_authdatum(context, authdata[i],
@@ -455,71 +415,60 @@ make_ad_signedpath_data(krb5_context context,
     sp_data.method_data = method_data;
     sp_data.authorization_data = sign_authdata;
 
-    code = encode_krb5_ad_signedpath_data(&sp_data, data);
+    ret = encode_krb5_ad_signedpath_data(&sp_data, data);
 
     if (sign_authdata != NULL)
         free(sign_authdata);
 
-    return code;
+    return ret;
 }
 
 static krb5_error_code
-verify_ad_signedpath_checksum(krb5_context context,
-                              krb5_keyblock *krbtgt_key,
-                              krb5_enc_tkt_part *enc_tkt_part,
-                              krb5_principal *deleg_path,
-                              krb5_pa_data **method_data,
-                              krb5_checksum *cksum,
-                              krb5_boolean *valid)
+verify_signedpath_checksum(krb5_context context, krb5_keyblock *krbtgt_key,
+                           krb5_enc_tkt_part *enc_tkt_part,
+                           krb5_principal *deleg_path,
+                           krb5_pa_data **method_data, krb5_checksum *cksum,
+                           krb5_boolean *valid)
 {
-    krb5_error_code                 code;
-    krb5_data                      *data;
+    krb5_error_code ret;
+    krb5_data *data;
 
     *valid = FALSE;
 
     if (!krb5_c_is_keyed_cksum(cksum->checksum_type))
         return KRB5KRB_AP_ERR_INAPP_CKSUM;
 
-    code = make_ad_signedpath_data(context,
-                                   enc_tkt_part->client,
-                                   enc_tkt_part->times.authtime,
-                                   deleg_path,
-                                   method_data,
-                                   enc_tkt_part->authorization_data,
-                                   &data);
-    if (code != 0)
-        return code;
-
-    code = krb5_c_verify_checksum(context,
-                                  krbtgt_key,
-                                  KRB5_KEYUSAGE_AD_SIGNEDPATH,
-                                  data,
-                                  cksum,
-                                  valid);
+    ret = make_signedpath_data(context, enc_tkt_part->client,
+                               enc_tkt_part->times.authtime, deleg_path,
+                               method_data, enc_tkt_part->authorization_data,
+                               &data);
+    if (ret)
+        return ret;
 
+    ret = krb5_c_verify_checksum(context, krbtgt_key,
+                                 KRB5_KEYUSAGE_AD_SIGNEDPATH, data, cksum,
+                                 valid);
     krb5_free_data(context, data);
-    return code;
+    return ret;
 }
 
 
 static krb5_error_code
-verify_ad_signedpath(krb5_context context,
-                     krb5_keyblock *krbtgt_key,
-                     krb5_enc_tkt_part *enc_tkt_part,
-                     krb5_principal **pdelegated,
-                     krb5_boolean *path_is_signed)
+verify_signedpath(krb5_context context, krb5_keyblock *krbtgt_key,
+                  krb5_enc_tkt_part *enc_tkt_part,
+                  krb5_principal **delegated_out, krb5_boolean *pathsigned_out)
 {
-    krb5_error_code                 code;
-    krb5_ad_signedpath             *sp = NULL;
-    krb5_authdata                 **sp_authdata = NULL;
-    krb5_data                       enc_sp;
+    krb5_error_code ret;
+    krb5_ad_signedpath *sp = NULL;
+    krb5_authdata **sp_authdata = NULL;
+    krb5_data enc_sp;
 
-    *pdelegated = NULL;
-    *path_is_signed = FALSE;
+    *delegated_out = NULL;
+    *pathsigned_out = FALSE;
 
-    code = krb5_find_authdata(context, enc_tkt_part->authorization_data, NULL,
-                              KRB5_AUTHDATA_SIGNTICKET, &sp_authdata);
-    if (code != 0)
+    ret = krb5_find_authdata(context, enc_tkt_part->authorization_data, NULL,
+                             KRB5_AUTHDATA_SIGNTICKET, &sp_authdata);
+    if (ret)
         goto cleanup;
 
     if (sp_authdata == NULL ||
@@ -530,71 +479,57 @@ verify_ad_signedpath(krb5_context context,
     enc_sp.data = (char *)sp_authdata[0]->contents;
     enc_sp.length = sp_authdata[0]->length;
 
-    code = decode_krb5_ad_signedpath(&enc_sp, &sp);
-    if (code != 0) {
+    ret = decode_krb5_ad_signedpath(&enc_sp, &sp);
+    if (ret) {
         /* Treat an invalid signedpath authdata element as a missing one, since
          * we believe MS is using the same number for something else. */
-        code = 0;
+        ret = 0;
         goto cleanup;
     }
 
-    code = verify_ad_signedpath_checksum(context,
-                                         krbtgt_key,
-                                         enc_tkt_part,
-                                         sp->delegated,
-                                         sp->method_data,
-                                         &sp->checksum,
-                                         path_is_signed);
-    if (code != 0)
+    ret = verify_signedpath_checksum(context, krbtgt_key, enc_tkt_part,
+                                     sp->delegated, sp->method_data,
+                                     &sp->checksum, pathsigned_out);
+    if (ret)
         goto cleanup;
 
-    if (*path_is_signed) {
-        *pdelegated = sp->delegated;
+    if (*pathsigned_out) {
+        *delegated_out = sp->delegated;
         sp->delegated = NULL;
     }
 
 cleanup:
     krb5_free_ad_signedpath(context, sp);
     krb5_free_authdata(context, sp_authdata);
-
-    return code;
+    return ret;
 }
 
 static krb5_error_code
-make_ad_signedpath_checksum(krb5_context context,
-                            krb5_const_principal for_user_princ,
-                            krb5_keyblock *krbtgt_key,
-                            krb5_enc_tkt_part *enc_tkt_part,
-                            krb5_principal *deleg_path,
-                            krb5_pa_data **method_data,
-                            krb5_checksum *cksum)
+make_signedpath_checksum(krb5_context context,
+                         krb5_const_principal for_user_princ,
+                         krb5_keyblock *krbtgt_key,
+                         krb5_enc_tkt_part *enc_tkt_part,
+                         krb5_principal *deleg_path,
+                         krb5_pa_data **method_data, krb5_checksum *cksum)
 {
-    krb5_error_code                 code;
-    krb5_data                      *data;
-    krb5_cksumtype                  cksumtype;
-    krb5_const_principal            client;
+    krb5_error_code ret;
+    krb5_data *data;
+    krb5_cksumtype cksumtype;
+    krb5_const_principal client;
 
-    if (for_user_princ != NULL)
-        client = for_user_princ;
-    else
-        client = enc_tkt_part->client;
-
-    code = make_ad_signedpath_data(context,
-                                   client,
-                                   enc_tkt_part->times.authtime,
-                                   deleg_path,
-                                   method_data,
-                                   enc_tkt_part->authorization_data,
-                                   &data);
-    if (code != 0)
-        return code;
-
-    code = krb5int_c_mandatory_cksumtype(context,
-                                         krbtgt_key->enctype,
-                                         &cksumtype);
-    if (code != 0) {
+    client = (for_user_princ != NULL) ? for_user_princ : enc_tkt_part->client;
+
+    ret = make_signedpath_data(context, client, enc_tkt_part->times.authtime,
+                               deleg_path, method_data,
+                               enc_tkt_part->authorization_data, &data);
+    if (ret)
+        return ret;
+
+    ret = krb5int_c_mandatory_cksumtype(context, krbtgt_key->enctype,
+                                        &cksumtype);
+    if (ret) {
         krb5_free_data(context, data);
-        return code;
+        return ret;
     }
 
     if (!krb5_c_is_keyed_cksum(cksumtype)) {
@@ -602,74 +537,59 @@ make_ad_signedpath_checksum(krb5_context context,
         return KRB5KRB_AP_ERR_INAPP_CKSUM;
     }
 
-    code = krb5_c_make_checksum(context, cksumtype, krbtgt_key,
-                                KRB5_KEYUSAGE_AD_SIGNEDPATH, data,
-                                cksum);
-
+    ret = krb5_c_make_checksum(context, cksumtype, krbtgt_key,
+                               KRB5_KEYUSAGE_AD_SIGNEDPATH, data, cksum);
     krb5_free_data(context, data);
-
-    return code;
+    return ret;
 }
 
 static krb5_error_code
-make_ad_signedpath(krb5_context context,
-                   krb5_const_principal for_user_princ,
-                   krb5_principal server,
-                   krb5_keyblock *krbtgt_key,
-                   krb5_principal *deleg_path,
-                   krb5_enc_tkt_part *enc_tkt_reply)
+make_signedpath(krb5_context context, krb5_const_principal for_user_princ,
+                krb5_principal server, krb5_keyblock *krbtgt_key,
+                krb5_principal *deleg_path, krb5_enc_tkt_part *enc_tkt_reply)
 {
-    krb5_error_code                 code;
-    krb5_ad_signedpath              sp;
-    int                             i;
-    krb5_data                      *data = NULL;
-    krb5_authdata                   ad_datum, *ad_data[2];
-    krb5_authdata                 **if_relevant = NULL;
+    krb5_error_code ret;
+    krb5_ad_signedpath sp;
+    krb5_data *data = NULL;
+    krb5_authdata ad_datum, *ad_data[2];
+    krb5_authdata **if_relevant = NULL;
+    size_t count;
 
     memset(&sp, 0, sizeof(sp));
 
     sp.enctype = krbtgt_key->enctype;
 
-    if (deleg_path != NULL) {
-        for (i = 0; deleg_path[i] != NULL; i++)
-            ;
-    } else
-        i = 0;
+    for (count = 0; deleg_path != NULL && deleg_path[count] != NULL; count++);
 
-    sp.delegated = k5calloc(i + (server ? 1 : 0) + 1, sizeof(krb5_principal),
-                            &code);
-    if (code != 0)
+    sp.delegated = k5calloc(count + 2, sizeof(krb5_principal), &ret);
+    if (sp.delegated == NULL)
         goto cleanup;
 
     /* Combine existing and new transited services, if any */
     if (deleg_path != NULL)
-        memcpy(sp.delegated, deleg_path, i * sizeof(krb5_principal));
+        memcpy(sp.delegated, deleg_path, count * sizeof(krb5_principal));
     if (server != NULL)
-        sp.delegated[i++] = server;
-    sp.delegated[i] = NULL;
+        sp.delegated[count++] = server;
+    sp.delegated[count] = NULL;
     sp.method_data = NULL;
 
-    code = make_ad_signedpath_checksum(context,
-                                       for_user_princ,
-                                       krbtgt_key,
-                                       enc_tkt_reply,
-                                       sp.delegated,
-                                       sp.method_data,
-                                       &sp.checksum);
-    if (code != 0) {
-        if (code == KRB5KRB_AP_ERR_INAPP_CKSUM) {
+    ret = make_signedpath_checksum(context, for_user_princ, krbtgt_key,
+                                   enc_tkt_reply, sp.delegated, sp.method_data,
+                                   &sp.checksum);
+    if (ret) {
+        if (ret == KRB5KRB_AP_ERR_INAPP_CKSUM) {
             /*
-             * In the hopefully unlikely case the TGS key enctype
-             * has an unkeyed mandatory checksum type, do not fail
-             * so we do not prevent the KDC from servicing requests.
+             * In the hopefully unlikely case the TGS key enctype has an
+             * unkeyed mandatory checksum type, do not fail so we do not
+             * prevent the KDC from servicing requests.
              */
-            code = 0;
+            ret = 0;
         }
         goto cleanup;
     }
 
-    code = encode_krb5_ad_signedpath(&sp, &data);
-    if (code != 0)
+    ret = encode_krb5_ad_signedpath(&sp, &data);
+    if (ret)
         goto cleanup;
 
     ad_datum.ad_type = KRB5_AUTHDATA_SIGNTICKET;
@@ -679,56 +599,45 @@ make_ad_signedpath(krb5_context context,
     ad_data[0] = &ad_datum;
     ad_data[1] = NULL;
 
-    code = krb5_encode_authdata_container(context,
-                                          KRB5_AUTHDATA_IF_RELEVANT,
-                                          ad_data,
-                                          &if_relevant);
-    if (code != 0)
+    ret = krb5_encode_authdata_container(context, KRB5_AUTHDATA_IF_RELEVANT,
+                                         ad_data, &if_relevant);
+    if (ret)
         goto cleanup;
 
-    code = merge_authdata(context,
-                          if_relevant,
-                          &enc_tkt_reply->authorization_data,
-                          FALSE,        /* !copy */
-                          FALSE);       /* !ignore_kdc_issued */
-    if (code != 0)
+    /* Add the authdata to the ticket, without copying or filtering. */
+    ret = merge_authdata(context, if_relevant,
+                         &enc_tkt_reply->authorization_data, FALSE, FALSE);
+    if (ret)
         goto cleanup;
-
-    if_relevant = NULL; /* merge_authdata() freed */
+    if_relevant = NULL;         /* merge_authdata() freed */
 
 cleanup:
-    if (sp.delegated != NULL)
-        free(sp.delegated);
+    free(sp.delegated);
     krb5_free_authdata(context, if_relevant);
     krb5_free_data(context, data);
     krb5_free_checksum_contents(context, &sp.checksum);
     krb5_free_pa_data(context, sp.method_data);
-
-    return code;
+    return ret;
 }
 
 static void
 free_deleg_path(krb5_context context, krb5_principal *deleg_path)
 {
-    if (deleg_path != NULL) {
-        int i;
+    int i;
 
-        for (i = 0; deleg_path[i] != NULL; i++)
-            krb5_free_principal(context, deleg_path[i]);
-        free(deleg_path);
-    }
+    for (i = 0; deleg_path != NULL && deleg_path[i] != NULL; i++)
+        krb5_free_principal(context, deleg_path[i]);
+    free(deleg_path);
 }
 
-/*
- * Returns TRUE if the Windows 2000 PAC is the only element in the
- * supplied authorization data.
- */
+/* Return true if the Windows 2000 PAC is the only element in the supplied
+ * authorization data. */
 static krb5_boolean
 only_pac_p(krb5_context context, krb5_authdata **authdata)
 {
-    return has_kdc_issued_authdata(context,
-                                   authdata, KRB5_AUTHDATA_WIN2K_PAC) &&
-        (authdata[1] == NULL);
+    return has_kdc_issued_authdata(context, authdata,
+                                   KRB5_AUTHDATA_WIN2K_PAC) &&
+        authdata[1] == NULL;
 }
 
 /* Verify AD-SIGNTICKET authdata if we need to, and insert an AD-SIGNEDPATH
@@ -736,12 +645,12 @@ only_pac_p(krb5_context context, krb5_authdata **authdata)
 static krb5_error_code
 handle_signticket(krb5_context context, unsigned int flags,
                   krb5_db_entry *client, krb5_db_entry *server,
-                  krb5_keyblock *krbtgt_key, krb5_kdc_req *request,
+                  krb5_keyblock *krbtgt_key, krb5_kdc_req *req,
                   krb5_const_principal for_user_princ,
-                  krb5_enc_tkt_part *enc_tkt_request,
+                  krb5_enc_tkt_part *enc_tkt_req,
                   krb5_enc_tkt_part *enc_tkt_reply)
 {
-    krb5_error_code code = 0;
+    krb5_error_code ret = 0;
     krb5_principal *deleg_path = NULL;
     krb5_boolean signed_path = FALSE;
     krb5_boolean s4u2proxy;
@@ -752,18 +661,15 @@ handle_signticket(krb5_context context, unsigned int flags,
      * The Windows PAC fulfils the same role as the signed path
      * if it is the only authorization data element.
      */
-    if (request->msg_type == KRB5_TGS_REQ &&
-        !only_pac_p(context, enc_tkt_request->authorization_data)) {
-        code = verify_ad_signedpath(context,
-                                    krbtgt_key,
-                                    enc_tkt_request,
-                                    &deleg_path,
-                                    &signed_path);
-        if (code != 0)
+    if (req->msg_type == KRB5_TGS_REQ &&
+        !only_pac_p(context, enc_tkt_req->authorization_data)) {
+        ret = verify_signedpath(context, krbtgt_key, enc_tkt_req, &deleg_path,
+                                &signed_path);
+        if (ret)
             goto cleanup;
 
         if (s4u2proxy && signed_path == FALSE) {
-            code = KRB5KDC_ERR_BADOPTION;
+            ret = KRB5KDC_ERR_BADOPTION;
             goto cleanup;
         }
     }
@@ -773,90 +679,78 @@ handle_signticket(krb5_context context, unsigned int flags,
     if (!isflagset(server->attributes, KRB5_KDB_NO_AUTH_DATA_REQUIRED) &&
         !is_cross_tgs_principal(server->princ) &&
         !only_pac_p(context, enc_tkt_reply->authorization_data)) {
-        code = make_ad_signedpath(context,
-                                  for_user_princ,
-                                  s4u2proxy ? client->princ : NULL,
-                                  krbtgt_key,
-                                  deleg_path,
-                                  enc_tkt_reply);
-        if (code != 0)
+        ret = make_signedpath(context, for_user_princ,
+                              s4u2proxy ? client->princ : NULL, krbtgt_key,
+                              deleg_path, enc_tkt_reply);
+        if (ret)
             goto cleanup;
     }
 
 cleanup:
     free_deleg_path(context, deleg_path);
-
-    return code;
+    return ret;
 }
 
 krb5_error_code
-handle_authdata (krb5_context context,
-                 unsigned int flags,
-                 krb5_db_entry *client,
-                 krb5_db_entry *server,
-                 krb5_db_entry *krbtgt,
-                 krb5_keyblock *client_key,
-                 krb5_keyblock *server_key,
-                 krb5_keyblock *krbtgt_key,
-                 krb5_data *req_pkt,
-                 krb5_kdc_req *request,
-                 krb5_const_principal for_user_princ,
-                 krb5_enc_tkt_part *enc_tkt_request,
-                 krb5_enc_tkt_part *enc_tkt_reply)
+handle_authdata(krb5_context context, unsigned int flags,
+                krb5_db_entry *client, krb5_db_entry *server,
+                krb5_db_entry *krbtgt, krb5_keyblock *client_key,
+                krb5_keyblock *server_key, krb5_keyblock *krbtgt_key,
+                krb5_data *req_pkt, krb5_kdc_req *req,
+                krb5_const_principal for_user_princ,
+                krb5_enc_tkt_part *enc_tkt_req,
+                krb5_enc_tkt_part *enc_tkt_reply)
 {
     kdcauthdata_handle *h;
-    krb5_error_code code = 0;
+    krb5_error_code ret = 0;
     size_t i;
 
-    if (request->msg_type == KRB5_TGS_REQ &&
-        request->authorization_data.ciphertext.data != NULL) {
+    if (req->msg_type == KRB5_TGS_REQ &&
+        req->authorization_data.ciphertext.data != NULL) {
         /* Copy TGS request authdata.  This must be done first so that modules
          * have access to the unencrypted request authdata. */
-        code = copy_request_authdata(context, client_key, request,
-                                     enc_tkt_request,
-                                     &enc_tkt_reply->authorization_data);
-        if (code)
-            return code;
+        ret = copy_request_authdata(context, client_key, req, enc_tkt_req,
+                                    &enc_tkt_reply->authorization_data);
+        if (ret)
+            return ret;
     }
 
     /* Invoke loaded module handlers. */
     if (!isflagset(enc_tkt_reply->flags, TKT_FLG_ANONYMOUS)) {
         for (i = 0; i < n_authdata_modules; i++) {
             h = &authdata_modules[i];
-            code = h->vt.handle(context, h->data, flags, client, server,
-                                krbtgt, client_key, server_key, krbtgt_key,
-                                req_pkt, request, for_user_princ,
-                                enc_tkt_request, enc_tkt_reply);
-            if (code)
-                kdc_err(context, code, "from authdata module %s", h->vt.name);
+            ret = h->vt.handle(context, h->data, flags, client, server,
+                               krbtgt, client_key, server_key, krbtgt_key,
+                               req_pkt, req, for_user_princ, enc_tkt_req,
+                               enc_tkt_reply);
+            if (ret)
+                kdc_err(context, ret, "from authdata module %s", h->vt.name);
         }
     }
 
-    if (request->msg_type == KRB5_TGS_REQ) {
+    if (req->msg_type == KRB5_TGS_REQ) {
         /* Copy authdata from the TGT to the issued ticket. */
-        code = copy_tgt_authdata(context, request,
-                                 enc_tkt_request->authorization_data,
-                                 &enc_tkt_reply->authorization_data);
-        if (code)
-            return code;
+        ret = copy_tgt_authdata(context, req, enc_tkt_req->authorization_data,
+                                &enc_tkt_reply->authorization_data);
+        if (ret)
+            return ret;
     }
 
     if (!isflagset(enc_tkt_reply->flags, TKT_FLG_ANONYMOUS)) {
         /* Fetch authdata from the KDB if appropriate. */
-        code = fetch_kdb_authdata(context, flags, client, server, krbtgt,
-                                  client_key, server_key, krbtgt_key, request,
-                                  for_user_princ, enc_tkt_request,
-                                  enc_tkt_reply);
-        if (code)
-            return code;
+        ret = fetch_kdb_authdata(context, flags, client, server, krbtgt,
+                                 client_key, server_key, krbtgt_key, req,
+                                 for_user_princ, enc_tkt_req, enc_tkt_reply);
+        if (ret)
+            return ret;
 
         /* Validate and insert AD-SIGNTICKET authdata.  This must happen last
          * since it contains a signature over the other authdata. */
-        code = handle_signticket(context, flags, client, server, krbtgt_key,
-                                 request, for_user_princ, enc_tkt_request,
-                                 enc_tkt_reply);
-        if (code)
-            return code;
+        ret = handle_signticket(context, flags, client, server, krbtgt_key,
+                                req, for_user_princ, enc_tkt_req,
+                                enc_tkt_reply);
+        if (ret)
+            return ret;
     }
 
     return 0;


More information about the cvs-krb5 mailing list