- conditionalized and depersonalized all debugging code

- fixed in_cred memory management problems (double_free/SEGV)
- rearranged storage use to leave in_cred inviolate and use temporary storage
  for referral traversal
- switched most explicit *alloc use to use library storage functions
- cleaned up referral tgt list management
- update TODO state

git-svn-id: svn://anonsvn.mit.edu/krb5/branches/referrals@18567 dc483132-0cff-0310-8789-dd5450dbe970

diff --git a/TODO b/TODO
index 990440827c06a400aa8bee9a91a7a15a55590e63..745ed486f61d763ddba00bb447be032b01226224 100644 (file)
--- a/TODO
+++ b/TODO
@@ -1,13 +1,9 @@
 blocking issues for beta release
 ================================
-- fix memory management problems and any other known crash/assertion-fail 
-  cases
-  - kvno crashes freeing in_cred after the call completes.  why is this?
-    reproduce: "kvno host/maybe.not.ms.mit.edu@NOT.MS.MIT.EDU"
-  - assertion failure: "./ptest argos.mit.edu"
+- check assertion failure case:
+  - reproduce: "./ptest argos.mit.edu"
     - might require NOT tickets and no domain_realm setting
     - no longer reproducible?
-  - fix double-free in gc_from_kdc_opt cleanup
 - correctly return first-hop TGTs for ccache storage
   - old code was convoluted and buggy.  replace.
 - referral loop checking

diff --git a/src/include/krb5/krb5.hin b/src/include/krb5/krb5.hin
index eb82862881e4ee42b043edde2689c01bf83a87fa..f01f3875682da96d225bad014fb1b2769c94a020 100644 (file)
--- a/src/include/krb5/krb5.hin
+++ b/src/include/krb5/krb5.hin
@@ -1,5 +1,3 @@
-#define DEBUG_REFERRALS
-
 /*
  * include/krb5.h
  *
@@ -267,9 +265,13 @@ typedef const krb5_principal_data *krb5_const_principal;
 #define        KRB5_REFERRAL_REALM     ""
 #define        KRB5_REFERRAL_MAXHOPS   5
 
+/*
+ * Referral debugging hooks.
+ */
+#define DEBUG_REFERRALS
+
 #ifdef DEBUG_REFERRALS
-/* temporary hack */
-void amb_dump_principal();
+void dbgref_dump_principal(char *, krb5_principal);
 #endif
 
 /*

diff --git a/src/lib/krb5/krb/gc_frm_kdc.c b/src/lib/krb5/krb/gc_frm_kdc.c
index 189c862b93da12e738eed5101a3aaafa3ccdb050..2e9b8f223790bedc60791064be48873d66969282 100644 (file)
--- a/src/lib/krb5/krb/gc_frm_kdc.c
+++ b/src/lib/krb5/krb/gc_frm_kdc.c
@@ -768,13 +768,22 @@ krb5_get_cred_from_kdc_opt(krb5_context context, krb5_ccache ccache,
     char **hrealms;
     int referral_count, i;
 
+    /* 
+     * Set up client and server pointers.  Make a fresh and modifyable
+     * copy of the in_cred server and save the supplied version.
+     */
     client = in_cred->client;
-    server = in_cred->server;
-    /* XXX hack for testing to force referral */
-    // /* XXX */ server->realm.data[0]=0;
+    if ((retval=krb5_copy_principal(context, in_cred->server, &server)))
+        return retval;
+    supplied_server = in_cred->server;
+    in_cred->server=server;
+
+
 #ifdef DEBUG_REFERRALS
-    amb_dump_principal("krb5_get_cred_from_kdc_opt initial client", client);
-    amb_dump_principal("krb5_get_cred_from_kdc_opt initial server", server);
+    /* Hack for testing to force a referral. */
+    // /* WARNING: uncomment for testing only. */ server->realm.data[0]=0;
+    dbgref_dump_principal("krb5_get_cred_from_kdc_opt initial client", client);
+    dbgref_dump_principal("krb5_get_cred_from_kdc_opt initial server", server);
 #endif
     memset(&cc_tgt, 0, sizeof(cc_tgt));
     memset(&tgtq, 0, sizeof(tgtq));
@@ -784,18 +793,19 @@ krb5_get_cred_from_kdc_opt(krb5_context context, krb5_ccache ccache,
     *tgts = NULL;
     old_use_conf_ktypes = context->use_conf_ktypes;
 
-    /* Copy initially specified principal, then copy client realm for server if no hint. */
-    if ((retval=krb5_copy_principal(context, server, &supplied_server)))
-        return retval;
+    /* Copy client realm to server if no hint. */
     if (!strcmp(server->realm.data, KRB5_REFERRAL_REALM)) {   // XXX a realm is not a string!
         /* Use the client realm. */
+#ifdef DEBUG_REFERRALS
+        printf("gc_from_kdc: no server realm supplied, using client realm.\n");
+#endif
         if (!( server->realm.data = (char *)malloc(client->realm.length)))
            return ENOMEM;
        memcpy(server->realm.data, client->realm.data, client->realm.length);
     }
     /*
      * Retreive initial TGT to match the specified server, either for the
-     * local realm in the default (referral) case or for  the remote
+     * local realm in the default (referral) case or for the remote
      * realm if we're starting someplace non-local.
      */
     retval = tgt_mcred(context, client, server, client, &tgtq);
@@ -823,7 +833,7 @@ krb5_get_cred_from_kdc_opt(krb5_context context, krb5_ccache ccache,
     }
 
 #ifdef DEBUG_REFERRALS
-    amb_dump_principal("gc_from_kdc: server as requested", supplied_server);
+    dbgref_dump_principal("gc_from_kdc: server as requested", supplied_server);
 #endif
 
     /*
@@ -834,8 +844,8 @@ krb5_get_cred_from_kdc_opt(krb5_context context, krb5_ccache ccache,
 
     for (referral_count=0;referral_count<KRB5_REFERRAL_MAXHOPS;referral_count++) {
 #ifdef DEBUG_REFERRALS
-        amb_dump_principal("gc_from_kdc: referral loop: tgt in use", tgtptr->server);
-        amb_dump_principal("gc_from_kdc: referral loop: request is for", server);
+        dbgref_dump_principal("gc_from_kdc: referral loop: tgt in use", tgtptr->server);
+        dbgref_dump_principal("gc_from_kdc: referral loop: request is for", server);
 #endif
         retval = krb5_get_cred_via_tkt(context, tgtptr,
                                       KDC_OPT_CANONICALIZE | 
@@ -877,64 +887,52 @@ krb5_get_cred_from_kdc_opt(krb5_context context, krb5_ccache ccache,
            if (krb5_principal_compare(context, in_cred->server, out_cred[0]->server)) {
 #ifdef DEBUG_REFERRALS
                printf("gc_from_kdc: request generated ticket for requested server principal\n");
-               amb_dump_principal("gc_from_kdc final referred reply",in_cred->server);
+               dbgref_dump_principal("gc_from_kdc final referred reply",in_cred->server);
 #endif
+
                /*
-                * Deal with ccache TGT management:
-                * If we have TGTs already from initial TGT discovery,
-                * leave it alone.  If we started with a local referral,
-                * return the initial TGT for the cache but junk the
-                * rest.
+                * Deal with ccache TGT management: If tgts has been set
+                * from an initial non-referral TGT discovery, leave it
+                * alone.  Otherwise, if referral_tgts[0] exists return
+                * it as the only entry in tgts.  (Further referrals are
+                * never cached, only referrals from the local KDC.)
                 */
-               if (! *tgts) {
-                 /* Free newly-obtained TGTs. */
-                 for (i=0;i<KRB5_REFERRAL_MAXHOPS;i++)
-                     if(referral_tgts[i])
-                         krb5_free_creds(context, referral_tgts[i]);
-               }
-               else {
-                 /* Alocate returnable TGT list. */
-                 // XXX Redo this.
+               if (*tgts == NULL) {
+                   /* Alocate returnable TGT list. */
+                   // XXX TBC
                }
                goto cleanup;
            }
            else {
 #ifdef DEBUG_REFERRALS
                printf("gc_from_kdc: request generated referral tgt\n");
-               amb_dump_principal("gc_from_kdc credential received", out_cred[0]->server);
+               dbgref_dump_principal("gc_from_kdc credential received", out_cred[0]->server);
 #endif
-               /* need to:
-                  1) stash tgt in referral_tgts
-                  2) use this tgt for next referral
-                  3) rewrite server principal per any padata */
+               /* Point current tgt pointer at newly-received TGT. */
                tgtptr=out_cred[0];
-               /* save this credential in tgt sequence */
-               referral_tgts[referral_count]=tgtptr=out_cred[0];
-               /* copy krbtgt realm to server principal */
-               free(server->realm.data);
-               if(!(server->realm.data=malloc(tgtptr->server->data[1].length)))
-                   return ENOMEM;
-               strncpy(server->realm.data, tgtptr->server->data[1].data, tgtptr->server->data[1].length);
-               server->realm.data[tgtptr->server->data[1].length]=0;
-               server->realm.length=tgtptr->server->data[1].length;
+               /* Save this credential in tgt sequence. */
+               referral_tgts[referral_count]=out_cred[0];
+               /* Copy krbtgt realm to server principal. */
+               krb5_free_data_contents(context, &server->realm);
+               if ((retval=krb5int_copy_data_contents(context, &tgtptr->server->data[1], &server->realm)))
+                   return retval;
+               /* Future work: rewrite server principal per any supplied padata. */
            }
        }
     }
 
 #ifdef DEBUG_REFERRALS
-    amb_dump_principal("gc_from_kdc client at fallback", client);
-    amb_dump_principal("gc_from_kdc server at fallback", server);
-    ///* XXX hack for testing */ printf("gc_from_kdc: referral failed; exiting.\n"),exit(1);
+    dbgref_dump_principal("gc_from_kdc client at fallback", client);
+    dbgref_dump_principal("gc_from_kdc server at fallback", server);
+    /* Hack for testing to shut down immediately after referral attempt. */
+    // /* WARNING: uncomment for testing only. */ printf("gc_from_kdc: referral failed; exiting.\n"),exit(1);
 #endif
 
     /*
      * At this point referrals have been tried and have failed.  Go back
      * to the server principal as originally issued and try the conventional path.
      */
-
-    /* Free any referral TGTs accumulated. */
-    // XXX: to be implemented
-    
+  
     /* Referrals have failed.  Look up fallback realm if not currently set. */
     if (!strcmp(server->realm.data, KRB5_REFERRAL_REALM)) { // XXX a realm is not a string!
       if (server->length >= 2) {
@@ -1003,14 +1001,17 @@ cleanup:
     /* Drop the original principal back into in_cred so that it's cached
        in the expected format. */
 #ifdef DEBUG_REFERRALS
-    amb_dump_principal("gc_from_kdc: final server principal at cleanup",server);
+    dbgref_dump_principal("gc_from_kdc: final hacked server principal at cleanup",server);
 #endif
-    // XXX: This blows out as a double-free.  Why?
-    // krb5_free_principal(context, server);
-    server=supplied_server;
+    krb5_free_principal(context, server);
+    in_cred->server=supplied_server;
 #ifdef DEBUG_REFERRALS
-    amb_dump_principal("gc_from_kdc: final server after reversion",server);
+    dbgref_dump_principal("gc_from_kdc: final server after reversion",in_cred->server);
 #endif
+    /* Free any referral TGTs accumulated. */
+    for (i=0;i<KRB5_REFERRAL_MAXHOPS;i++)
+        if(referral_tgts[i])
+           krb5_free_creds(context, referral_tgts[i]);
 
     return retval;
 }
@@ -1043,7 +1044,7 @@ krb5_get_cred_from_kdc_renew(krb5_context context, krb5_ccache ccache,
 }
 
 #ifdef DEBUG_REFERRALS
-void amb_dump_principal(char *d, krb5_principal p)
+void dbgref_dump_principal(char *d, krb5_principal p)
 {
        int n;
 

diff --git a/src/lib/krb5/krb/gc_via_tkt.c b/src/lib/krb5/krb/gc_via_tkt.c
index 441a7816624b805ed9695a5126b7ab79fd6f5f78..8a2bc93380e65c3e746bdec8127bf651dab540b5 100644 (file)
--- a/src/lib/krb5/krb/gc_via_tkt.c
+++ b/src/lib/krb5/krb/gc_via_tkt.c
@@ -236,8 +236,8 @@ krb5_get_cred_via_tkt (krb5_context context, krb5_creds *tkt,
 #if 0
 #ifdef DEBUG_REFERRALS
            printf("gc_via_tkt: in_cred and encoding don't match but referrals requested\n");
-           amb_dump_principal("gc_via_tkt: in_cred",in_cred->server);
-           amb_dump_principal("gc_via_tkt: encoded server",dec_rep->enc_part2->server);
+           dbgref_dump_principal("gc_via_tkt: in_cred",in_cred->server);
+           dbgref_dump_principal("gc_via_tkt: encoded server",dec_rep->enc_part2->server);
 #endif
 #endif
        }

diff --git a/src/lib/krb5/krb/walk_rtree.c b/src/lib/krb5/krb/walk_rtree.c
index 4cbacdf86eb5b8f3f82c1d2375146d8ce6d53711..e6765496120fdc43b3affe9927fdfb77b7b25deb 100644 (file)
--- a/src/lib/krb5/krb/walk_rtree.c
+++ b/src/lib/krb5/krb/walk_rtree.c
@@ -134,9 +134,11 @@ krb5_walk_realm_tree(krb5_context context, const krb5_data *client, const krb5_d
     krb5_error_code cap_code;
 #endif
 
+#ifdef DEBUG_REFERRALS
     printf("krb5_walk_realm_tree starting\n");
-    printf("client is %s\n",client->data);
-    printf("server is %s\n",server->data);
+    printf("  client is %s\n",client->data);
+    printf("  server is %s\n",server->data);
+#endif
 
     if (!(client->data &&server->data))
       return KRB5_NO_TKT_IN_RLM;
@@ -386,12 +388,14 @@ krb5_walk_realm_tree(krb5_context context, const krb5_data *client, const krb5_d
 #endif
     *tree = rettree;
 
+#ifdef DEBUG_REFERRALS
     printf("krb5_walk_realm_tree ending; tree (length %d) is:\n",links);
     for(i=0;i<links+2;i++) {
         if ((*tree)[i])
-           amb_dump_principal("krb5_walk_realm_tree tree",(*tree)[i]);
+           dbgref_dump_principal("krb5_walk_realm_tree tree",(*tree)[i]);
        else
            printf("tree element %i null\n");
     }
+#endif
     return 0;
 }

diff --git a/src/lib/krb5/os/sn2princ.c b/src/lib/krb5/os/sn2princ.c
index 18585f915c9df253ed064cf9d3bb093042e34bfb..fba55972654a4775fee4eb401a28dab87ff77c19 100644 (file)
--- a/src/lib/krb5/os/sn2princ.c
+++ b/src/lib/krb5/os/sn2princ.c
@@ -70,8 +70,10 @@ krb5_sname_to_principal(krb5_context context, const char *hostname, const char *
 
     FILE *log;
 
+#ifdef DEBUG_REFERRALS
     printf("krb5_sname_to_principal(host=%s, sname=%s, type=%d)\n",hostname,sname,type);
     printf("      name types: 0=unknown, 3=srv_host\n");
+#endif
 
     if ((type == KRB5_NT_UNKNOWN) ||
        (type == KRB5_NT_SRV_HST)) {
@@ -109,7 +111,9 @@ krb5_sname_to_principal(krb5_context context, const char *hostname, const char *
        try_getaddrinfo_again:
            err = getaddrinfo(hostname, 0, &hints, &ai);
            if (err) {
-             printf("probably punting due to bad hostname of %s\n",hostname);
+#ifdef DEBUG_REFERRALS
+               printf("sname_to_princ: probably punting due to bad hostname of %s\n",hostname);
+#endif
                if (hints.ai_family == AF_INET) {
                    /* Just in case it's an IPv6-only name.  */
                    hints.ai_family = 0;
@@ -150,13 +154,14 @@ krb5_sname_to_principal(krb5_context context, const char *hostname, const char *
        }
        if (!remote_host)
            return ENOMEM;
-       printf("  hostname <%s> after rdns processing\n",remote_host); /* XXX */
+#ifdef DEBUG_REFERRALS
+       printf("sname_to_princ: hostname <%s> after rdns processing\n",remote_host);
+#endif
 
        if (type == KRB5_NT_SRV_HST)
            for (cp = remote_host; *cp; cp++)
                if (isupper((unsigned char) (*cp)))
                    *cp = tolower((unsigned char) (*cp));
-       printf("  hostname <%s> after case folding\n",remote_host); /* XXX */
 
        /*
         * Windows NT5's broken resolver gratuitously tacks on a
@@ -175,7 +180,9 @@ krb5_sname_to_principal(krb5_context context, const char *hostname, const char *
            return retval;
        }
 
-       printf("  realm <%s> after krb5_get_host_realm\n",hrealms[0]);
+#ifdef DEBUG_REFERRALS
+       printf("sname_to_princ:  realm <%s> after krb5_get_host_realm\n",hrealms[0]);
+#endif
 
        if (!hrealms[0]) {
            free(remote_host);
@@ -190,10 +197,12 @@ krb5_sname_to_principal(krb5_context context, const char *hostname, const char *
 
        krb5_princ_type(context, *ret_princ) = type;
 
+#ifdef DEBUG_REFERRALS
        printf("krb5_sname_to_principal returning\n");
        printf("realm: <%s>, sname: <%s>, remote_host: <%s>\n",
               realm,sname,remote_host);
-       amb_dump_principal("krb5_sname_to_principal",*ret_princ);
+       dbgref_dump_principal("krb5_sname_to_principal",*ret_princ);
+#endif
 
        free(remote_host);