Further polish and docs for hierarchical capaths

kdc/krb5tgs.c

@@ -1673,6 +1673,7 @@ server_lookup:
 
 	if ((req_rlm = get_krbtgt_realm(&sp->name)) != NULL) {
             if (capath == NULL) {
+                /* With referalls, hierarchical capaths are always enabled */
                 ret = _krb5_find_capath(context, tgt->crealm, our_realm,
                                         req_rlm, TRUE, &capath, &num_capath);
                 if (ret)

lib/krb5/krb5.conf.5

@@ -395,6 +395,14 @@ specifying order can be useful where tools read and write the
 configuration file without preserving parameter order.
 .Pp
 Malformed rules are ignored.
+.It Li allow_hierarchical_capaths = Va boolean
+When validating cross-realm transit paths, absent any explicit capath from the
+client realm to the server realm, allow a hierarchical transit path via the
+common ancestor domain of the two realms.
+Defaults to true.
+Note, absent an explicit setting, hierarchical capaths are always used by
+the KDC when generating a referral to a destination with which is no direct
+trust.
 .El
 .It Li [domain_realm]
 This is a list of mappings from DNS domain to Kerberos realm.

lib/krb5/transited.c

@@ -411,14 +411,17 @@ _krb5_free_capath(krb5_context context, char **capath)
 struct hier_iter {
     const char *local_realm;
     const char *server_realm;
-    const char *lr;
+    const char *lr;     /* Pointer into tail of local realm */
-    const char *sr;
+    const char *sr;     /* Pointer into tail of server realm */
-    size_t llen;
+    size_t llen;        /* Length of local_realm */
-    size_t slen;
+    size_t slen;        /* Length of server_realm */
-    size_t len;
+    size_t len;         /* Length of common suffix */
-    size_t num;
+    size_t num;         /* Path element count */
 };
 
+/*
+ * Step up from local_realm to common suffix, or else down to server_realm.
+ */
 static const char *
 hier_next(struct hier_iter *state)
 {
@@ -498,7 +501,7 @@ hier_init(struct hier_iter *state, const char *local_realm, const char *server_r
 }
 
 /*
- * Find a referral path from crealm to srealm via our_realm.
+ * Find a referral path from client_realm to server_realm via local_realm.
  * Either via [capaths] or hierarchicaly.
  */
 KRB5_LIB_FUNCTION krb5_error_code KRB5_LIB_CALL
@@ -506,12 +509,10 @@ _krb5_find_capath(krb5_context context,
                   const char *client_realm,
                   const char *local_realm,
                   const char *server_realm,
-                  krb5_boolean use_hierachical,
+                  krb5_boolean use_hierarchical,
                   char ***rpath,
                   size_t *npath)
 {
-    krb5_boolean maybe = (TRUE == 1) ? 2 : 1;   /* Neither TRUE nor FALSE */
-    krb5_boolean b;
     char **confpath;
     char **capath;
     struct hier_iter hier_state;
@@ -529,9 +530,9 @@ _krb5_find_capath(krb5_context context,
     /*
      * With a [capaths] setting from the client to the server we look for our
      * own realm in the list.  If our own realm is not present, we return the
-     * first element.  Otherwise, we return the next element, or possibly NULL.
+     * full list.  Otherwise, we return our realm's successors, or possibly
-     * Ignoring a [capaths] settings risks loops plus would violate explicit
+     * NULL.  Ignoring a [capaths] settings risks loops plus would violate
-     * policy and the principle of least surpise.
+     * explicit policy and the principle of least surpise.
      */
     if (confpath != NULL) {
         char **start = confpath;
@@ -543,12 +544,16 @@ _krb5_find_capath(krb5_context context,
                 start = rp+1;
         n = rp - start;
 
-        if (n == 0)
+        if (n == 0) {
+            krb5_config_free_strings(confpath);
             return 0;
+        }
 
         capath = calloc(n + 1, sizeof(*capath));
-        if (capath == NULL)
+        if (capath == NULL) {
+            krb5_config_free_strings(confpath);
             return krb5_enomem(context);
+        }
 
 	for (i = 0, rp = start; *rp; rp++) {
             if ((capath[i++] = strdup(*rp)) == NULL) {
@@ -564,18 +569,17 @@ _krb5_find_capath(krb5_context context,
         return 0;
     }
 
-    /*
+    /* The use_hierarchical flag makes hierarchical path lookup unconditional */
-     * With hierarchical referrals, we ignore the client realm, and build a
+    if (! use_hierarchical &&
-     * path forward from our own realm!
+        ! krb5_config_get_bool_default(context, NULL, TRUE, "libdefaults",
-     */
+                                       "allow_hierarchical_capaths", NULL))
-    b = krb5_config_get_bool_default(context, NULL, maybe, "realms",
-                                     local_realm, "hier_capaths", NULL);
-    if (b == maybe)
-        b = krb5_config_get_bool_default(context, NULL, TRUE, "libdefaults",
-                                         "hier_capaths", NULL);
-    if (b == FALSE)
         return 0;
 
+    /*
+     * When validating transit paths, local_realm == client_realm.  Otherwise,
+     * with hierarchical referrals, they may differ, and we may be building a
+     * path forward from our own realm!
+     */
     hier_init(&hier_state, local_realm, server_realm);
     if (hier_state.num == 0)
         return 0;
@@ -610,8 +614,9 @@ krb5_check_transited(krb5_context context,
     size_t i = 0;
     size_t j = 0;
 
+    /* In transit checks hierarchical capaths are optional */
     ret = _krb5_find_capath(context, client_realm, client_realm, server_realm,
-                            TRUE, &capath, &num_capath);
+                            FALSE, &capath, &num_capath);
     if (ret)
         return ret;
 

lib/krb5/verify_krb5_conf.c

@@ -410,7 +410,7 @@ struct entry libdefaults_entries[] = {
     { "fcc-mit-ticketflags", krb5_config_string, check_boolean, 0 },
     { "forward", krb5_config_string, check_boolean, 0 },
     { "forwardable", krb5_config_string, check_boolean, 0 },
-    { "hier_capaths", krb5_config_string, check_boolean, 0 },
+    { "allow_hierarchical_capaths", krb5_config_string, check_boolean, 0 },
     { "host_timeout", krb5_config_string, check_time, 0 },
     { "http_proxy", krb5_config_string, check_host /* XXX */, 0 },
     { "ignore_addresses", krb5_config_string, NULL, 0 },
@@ -481,7 +481,7 @@ struct entry realms_entries[] = {
     { "auth_to_local_names", krb5_config_string, NULL, 0 },
     { "default_domain", krb5_config_string, NULL, 0 },
     { "forwardable", krb5_config_string, check_boolean, 0 },
-    { "hier_capaths", krb5_config_string, check_boolean, 0 },
+    { "allow_hierarchical_capaths", krb5_config_string, check_boolean, 0 },
     { "kdc", krb5_config_string, check_host, 0 },
     { "kpasswd_server", krb5_config_string, check_host, 0 },
     { "krb524_server", krb5_config_string, check_host, 0 },