- krb5_parse_name now allows zero-length realms

- krb5_get_fallback_host_realm made string-safe
  (now takes *krb5_data instead of a maybe-string)
- conditionalized some previously-missed bits of debugging output
  (some of this should probably get nuked entirely since it's
  not really interesting anymore, but keeping easy debugging
  hooks for the core referral logic seems very useful)
- return first-hop referral TGT for ccache
  (but should maybe not do so if already cached?)
- fixed a couple of stupid bugs in fallback realm code
- cleaned up some memory leaks (more surely remain)
- state updated

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

diff --git a/TODO b/TODO
index 593e5efff63b4beb7b162199bde8fb8faf353356..70b76f0a65817f9dc477916ca3e65899a669ba06 100644 (file)
--- a/TODO
+++ b/TODO
@@ -1,11 +1,7 @@
 blocking issues for beta release
 ================================
-- correctly return first-hop TGTs for ccache storage
-  - old code was convoluted and buggy.  replacement underway.
 - referral loop checking
-- fallback uses local realm instead of remote fallback realm
-- switch string comparisons to use krb5_is_referral_realm
-- wrong TGT in use for referrals with non-local specified domain?
+- maybe don't return first-hop referral TGT for ccache if it's cached already?
 - testing, cleanup, documentation
 
 further work:
@@ -16,6 +12,7 @@ further work:
   - review implementation notes against actual implementation, document changes
 - add klist option to print actual credential principal
 - padata parsing for referral data verification and possible principal rewrite
+- KDC support for referrals
 
 testing issues:
 ==============
@@ -23,8 +20,6 @@ testing issues:
 - verify that intermediate TGTs aren't cached
 - Should we do the single non-referral fallback always or only on certain
   KDC failure states?  Probably answer this from testing.
-- credential cacheing unreliable; investiagate
-  - "kvno host/argos.mit.edu@NOT.MS.MIT.EDU" with NOT tickets fills up ccache
 
 low-priority:
 - code (or explicitly punt) edge cases in krb5_get_cred_from_kdc_opt

diff --git a/src/include/krb5/krb5.hin b/src/include/krb5/krb5.hin
index b434a802d140e893cd2ee2c9d8c1a82fdc7c9519..701235f2bde2996aa1dcbc10d7a67cf822665835 100644 (file)
--- a/src/include/krb5/krb5.hin
+++ b/src/include/krb5/krb5.hin
@@ -2268,7 +2268,7 @@ krb5_error_code KRB5_CALLCONV krb5_get_host_realm
                char *** );
 krb5_error_code KRB5_CALLCONV krb5_get_fallback_host_realm
        (krb5_context,
-               const char *,
+               krb5_data *,
                char *** );
 krb5_error_code KRB5_CALLCONV krb5_clean_hostname
        (krb5_context,

diff --git a/src/lib/krb5/krb/gc_frm_kdc.c b/src/lib/krb5/krb/gc_frm_kdc.c
index 736de0714afcd6c12eef65b252307bf0c692b51c..7c797b2627b71993f658eae0aba3831bbf3d03ae 100644 (file)
--- a/src/lib/krb5/krb/gc_frm_kdc.c
+++ b/src/lib/krb5/krb/gc_frm_kdc.c
@@ -761,7 +761,7 @@ krb5_get_cred_from_kdc_opt(krb5_context context, krb5_ccache ccache,
                           krb5_creds *in_cred, krb5_creds **out_cred,
                           krb5_creds ***tgts, int kdcopt)
 {
-    krb5_error_code retval;
+    krb5_error_code retval, subretval;
     krb5_principal client, server, supplied_server, out_supplied_server;
     krb5_creds tgtq, cc_tgt, *tgtptr, *referral_tgts[KRB5_REFERRAL_MAXHOPS];
     krb5_boolean old_use_conf_ktypes;
@@ -787,8 +787,8 @@ krb5_get_cred_from_kdc_opt(krb5_context context, krb5_ccache ccache,
 #ifdef DEBUG_REFERRALS
     /* 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);
+    dbgref_dump_principal("gc_from_kdc initial client", client);
+    dbgref_dump_principal("gc_from_kdc initial server", server);
 #endif
     memset(&cc_tgt, 0, sizeof(cc_tgt));
     memset(&tgtq, 0, sizeof(tgtq));
@@ -805,8 +805,8 @@ krb5_get_cred_from_kdc_opt(krb5_context context, krb5_ccache ccache,
 #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+1)))
-                           return ENOMEM;
+       if (!( server->realm.data = (char *)malloc(client->realm.length+1)))
+           return ENOMEM;
        memcpy(server->realm.data, client->realm.data, client->realm.length);
        server->realm.length = client->realm.length;
        server->realm.data[server->realm.length] = 0;
@@ -852,8 +852,10 @@ 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
+#if 0
         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
 #endif
         retval = krb5_get_cred_via_tkt(context, tgtptr,
                                       KDC_OPT_CANONICALIZE | 
@@ -892,34 +894,26 @@ krb5_get_cred_from_kdc_opt(krb5_context context, krb5_ccache ccache,
        }
        else {
            /* Referral request succeeded; let's see what it is. */
-           if (krb5_principal_compare(context, in_cred->server, out_cred[0]->server)) {
+           if (krb5_principal_compare(context, in_cred->server, (*out_cred)->server)) {
 #ifdef DEBUG_REFERRALS
                printf("gc_from_kdc: request generated ticket for requested server principal\n");
                dbgref_dump_principal("gc_from_kdc final referred reply",in_cred->server);
 #endif
-
-               /*
-                * 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 == NULL) {
-                   /* Alocate returnable TGT list. */
-                   // XXX TBC
-               }
                goto cleanup;
            }
            else {
 #ifdef DEBUG_REFERRALS
                printf("gc_from_kdc: request generated referral tgt\n");
-               dbgref_dump_principal("gc_from_kdc credential received", out_cred[0]->server);
+               dbgref_dump_principal("gc_from_kdc credential received", (*out_cred)->server);
 #endif
-               /* Point current tgt pointer at newly-received TGT. */
-               tgtptr=out_cred[0];
-               /* Save this credential in tgt sequence. */
-               referral_tgts[referral_count]=out_cred[0];
+               /*
+                * Point current tgt pointer at newly-received TGT.
+                */
+               /* XXX Memory leak for the old tgtptr? */
+               tgtptr=*out_cred;
+               /* Make copy of cred for referral_tgts. */
+               retval=krb5_copy_creds(context, *out_cred, &referral_tgts[referral_count]);
+               if(retval) goto cleanup;
                /* 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)))
@@ -941,28 +935,38 @@ krb5_get_cred_from_kdc_opt(krb5_context context, krb5_ccache ccache,
      * to the server principal as originally issued and try the conventional path.
      */
   
-    /* Referrals have failed.  Look up fallback realm if not currently set. */
-    if (krb5_is_referral_realm(&server->realm)) {
+    /* Referrals have failed.  Look up fallback realm if not originally provided. */
+    if (krb5_is_referral_realm(&supplied_server->realm)) {
         if (server->length >= 2) {
-           retval=krb5_get_fallback_host_realm(context, server->data[1].data,
+           retval=krb5_get_fallback_host_realm(context, &server->data[1],
                                                &hrealms);
            if (retval) goto cleanup;
 #ifdef DEBUG_REFERRALS
+#if 0
            printf("gc_from_kdc: using fallback realm of %s\n",hrealms[0]);
 #endif
-           in_cred->server->realm.data=hrealms[0];
+#endif
+           krb5_free_data_contents(context,&in_cred->server->realm);
+           server->realm.data=hrealms[0];
+           server->realm.length=strlen(hrealms[0]);
        }
        else {
-           /* XXX realm tagged for referral but apparently not in a
-              <type>/<host> format.  Fall back in some intelligent way or
-              just punt? */
+           /*
+            * Problem case: Realm tagged for referral but apparently not
+            * in a <type>/<host> format that
+            * krb5_get_fallback_host_realm can deal with.
+            */
 #ifdef DEBUG_REFERRALS
-           printf("gc_from_kdc: referral specified but no fallback realm.  wtf?  exiting.\n"); // XXX
+           printf("gc_from_kdc: referral specified but no fallback realm avaiable!\n");
 #endif
-           exit(1);
-      }
+           return KRB5_ERR_HOST_REALM_UNKNOWN;
+       }
     }
 
+#ifdef DEBUG_REFERRALS
+    dbgref_dump_principal("gc_from_kdc server at fallback after fallback rewrite", server);
+#endif
+
     /*
      * Get a TGT for the target realm.
      */
@@ -1013,21 +1017,58 @@ cleanup:
 #endif
     krb5_free_principal(context, server);
     in_cred->server = supplied_server;
-    if (*out_cred) {
+    if (*out_cred && !retval) {
         krb5_free_principal (context, (*out_cred)->server);
        (*out_cred)->server= out_supplied_server;
     }
     else {
         krb5_free_principal (context, out_supplied_server);
+       if (*out_cred) {
+           krb5_free_creds(context, *out_cred);
+           (*out_cred)=NULL;
+       }
     }
 #ifdef DEBUG_REFERRALS
     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]);
+    /*
+     * Deal with ccache TGT management: If tgts has been set from
+     * 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 the referral from the
+     * local KDC.)  This is part of cleanup because useful received TGTs
+     * should be cached even if the main request resulted in failure.
+     */
+
+    if (*tgts == NULL) {
+        if (referral_tgts[0]) {
+           subretval=1; // XXX This should be something that scans the
+                        // ccache for this ticket, which we presumably
+                        // don't want to cache again.
+           if (subretval) {
+               /* Allocate returnable TGT list. */
+               if (!(*tgts=calloc(sizeof (krb5_creds *), 2)))
+                   return ENOMEM;
+               subretval=krb5_copy_creds(context, referral_tgts[0], &((*tgts)[0]));
+               if(subretval)
+                   return subretval;
+               (*tgts)[1]=NULL;
+#ifdef DEBUG_REFERRALS
+               dbgref_dump_principal("gc_from_kdc: returning referral TGT for ccache",(*tgts)[0]->server);
+#endif
+           }
+       }
+    }
 
+    /* Free any other referral TGTs accumulated. */
+    for (i=0;i<KRB5_REFERRAL_MAXHOPS;i++) {
+        if(referral_tgts[i]) {
+           krb5_free_creds(context, referral_tgts[i]);
+       }
+    }
+#ifdef DEBUG_REFERRALS
+    printf("gc_from_kdc finishing with %s\n", retval?error_message(retval):"no error");
+#endif
     return retval;
 }
 
@@ -1066,8 +1107,10 @@ krb5_boolean krb5_is_referral_realm(krb5_data *r)
    * names, KRB5_REFERRAL_REALM is known to be a string.)
    */
 #ifdef DEBUG_REFERRALS
+#if 0
     printf("krb5_is_ref_realm: checking <%s> for referralness: %s\n",
           r->data,(r->length==0)?"true":"false");
+#endif
 #endif
     assert(strlen(KRB5_REFERRAL_REALM)==0);
     if (r->length==0)

diff --git a/src/lib/krb5/krb/gc_via_tkt.c b/src/lib/krb5/krb/gc_via_tkt.c
index 8a2bc93380e65c3e746bdec8127bf651dab540b5..8629f7278e58b331003bc0010f95343f86bb333d 100644 (file)
--- a/src/lib/krb5/krb/gc_via_tkt.c
+++ b/src/lib/krb5/krb/gc_via_tkt.c
@@ -109,6 +109,8 @@ krb5_get_cred_via_tkt (krb5_context context, krb5_creds *tkt,
 
 #ifdef DEBUG_REFERRALS
     printf("krb5_get_cred_via_tkt starting; referral flag is %s\n", kdcoptions&KDC_OPT_CANONICALIZE?"on":"off");
+    dbgref_dump_principal("krb5_get_cred_via_tkt requested ticket", in_cred->server);
+    dbgref_dump_principal("krb5_get_cred_via_tkt TGT in use", tkt->server);
 #endif
 
     /* tkt->client must be equal to in_cred->client */

diff --git a/src/lib/krb5/krb/parse.c b/src/lib/krb5/krb/parse.c
index b2d87916ec6a8b56a7e8c43ae9b6f38ceab6f895..fbcc49db0d6eea4eef51f716a6e4fb1dbc1262e0 100644 (file)
--- a/src/lib/krb5/krb/parse.c
+++ b/src/lib/krb5/krb/parse.c
@@ -109,10 +109,10 @@ krb5_parse_name(krb5_context context, const char *name, krb5_principal *nprincip
                        size = 0;
                        i++;
                } else if (c == REALM_SEP) {
-                       if (parsed_realm || !*(cp+1)) 
+                       if (parsed_realm)
                                /*
-                                * Multiple realm separaters or null
-                                * realm names are not allowed!
+                                * Multiple realm separaters
+                                * not allowed; zero-length realms are.
                                 */
                                return(KRB5_PARSE_MALFORMED);
                        parsed_realm = cp+1;

diff --git a/src/lib/krb5/os/hst_realm.c b/src/lib/krb5/os/hst_realm.c
index 094f5b7a49c72a42743ce598092c2b28e0513ab9..26ef440684f1c410f965745bc3dfdffcecc6a264 100644 (file)
--- a/src/lib/krb5/os/hst_realm.c
+++ b/src/lib/krb5/os/hst_realm.c
@@ -313,27 +313,35 @@ krb5int_translate_gai_error (int num)
 
 
 /*
- * Ganked from krb5_get_host_realm; handles case where referrals have
- * failed and it's time to go look at TXT records or make a DNS-based
- * assumption.
+ * Ganked from krb5_get_host_realm; handles determining a fallback realm
+ * to try in the case where referrals have failed and it's time to go
+ * look at TXT records or make a DNS-based assumption.
  */
 
 krb5_error_code KRB5_CALLCONV
-krb5_get_fallback_host_realm(krb5_context context, const char *host, char ***realmsp)
+krb5_get_fallback_host_realm(krb5_context context, krb5_data *hdata, char ***realmsp)
 {
     char **retrealms;
     char *default_realm, *realm, *cp, *temp_realm;
     krb5_error_code retval;
-    char local_host[MAXDNAME+1];
+    char local_host[MAXDNAME+1], host[MAXDNAME+1];
+
+    /* Convert what we hope is a hostname to a string. */
+    memcpy(host, hdata->data, hdata->length);
+    host[hdata->length]=0;
 
+#ifdef DEBUG_REFERRALS    
     printf("get_fallback_host_realm(host >%s<) called\n",host);
+#endif
 
     krb5_clean_hostname(context, host, local_host, sizeof local_host);
 
     /* Scan hostname for DNS realm, and save as last-ditch realm
        assumption. */
     cp = local_host;
+#ifdef DEBUG_REFERRALS    
     printf("  local_host: %s\n",local_host);
+#endif
     realm = default_realm = (char *)NULL;
     temp_realm = 0;
     while (cp && !default_realm) {
@@ -347,8 +355,9 @@ krb5_get_fallback_host_realm(krb5_context context, const char *host, char ***rea
            cp = strchr(cp, '.');
        }
     }
+#ifdef DEBUG_REFERRALS
     printf("  done finding DNS-based default realm: >%s<\n",default_realm);
-
+#endif
 
 #ifdef KRB5_DNS_LOOKUP
     if (realm == (char *)NULL) {
@@ -419,7 +428,9 @@ krb5_clean_hostname(krb5_context context, const char *host, char *local_host, si
     int l;
 
     local_host[0]=0;
+#ifdef DEBUG_REFERRALS
     printf("krb5_clean_hostname called: host<%s>, local_host<%s>, size %d\n",host,local_host,lhsize);
+#endif
     if (host) {
        /* Filter out numeric addresses if the caller utterly failed to
           convert them to names.  */
@@ -462,6 +473,8 @@ krb5_clean_hostname(krb5_context context, const char *host, char *local_host, si
     if (l && local_host[l-1] == '.')
            local_host[l-1] = 0;
 
+#ifdef DEBUG_REFERRALS
     printf("krb5_clean_hostname ending: host<%s>, local_host<%s>, size %d\n",host,local_host,lhsize);
+#endif
     return 0;
 }