From 1501740952a815fbe10ab8345bb38cfb0ded9504 Mon Sep 17 00:00:00 2001 From: Viktor Dukhovni Date: Fri, 29 Jul 2016 12:30:07 -0400 Subject: [PATCH] Fix transit path validation Also implement KDC hierarchical transit policy checks. The "hier_capaths" parameter defaults to "yes" in [libdefaults] or can be set explicitly in [realms] per-realm. --- lib/krb5/libkrb5-exports.def.in | 3 +- lib/krb5/transited.c | 235 +++++++++++++++++++++++++++++--- lib/krb5/verify_krb5_conf.c | 2 + lib/krb5/version-script.map | 4 + tests/kdc/krb5.conf.in | 1 + 5 files changed, 224 insertions(+), 21 deletions(-) diff --git a/lib/krb5/libkrb5-exports.def.in b/lib/krb5/libkrb5-exports.def.in index 60cf24c122..ede4db5557 100644 --- a/lib/krb5/libkrb5-exports.def.in +++ b/lib/krb5/libkrb5-exports.def.in @@ -797,4 +797,5 @@ EXPORTS _krb5_plugin_find; _krb5_plugin_free; _krb5_expand_path_tokensv; - + _krb5_find_capath; + _krb5_free_capath; diff --git a/lib/krb5/transited.c b/lib/krb5/transited.c index 8390df4659..c84d11147d 100644 --- a/lib/krb5/transited.c +++ b/lib/krb5/transited.c @@ -398,6 +398,204 @@ krb5_domain_x500_encode(char **realms, unsigned int num_realms, return 0; } +KRB5_LIB_FUNCTION void KRB5_LIB_CALL +_krb5_free_capath(krb5_context context, char **capath) +{ + char **s; + + for (s = capath; s && *s; ++s) + free(*s); + free(capath); +} + +struct hier_iter { + const char *local_realm; + const char *server_realm; + const char *lr; + const char *sr; + size_t llen; + size_t slen; + size_t len; + size_t num; +}; + +static const char * +hier_next(struct hier_iter *state) +{ + const char *lr = state->lr; + const char *sr = state->sr; + const char *lsuffix = state->local_realm + state->llen - state->len; + const char *server_realm = state->server_realm; + + if (lr != NULL) { + while (lr < lsuffix) + if (*lr++ == '.') + return state->lr = lr; + state->lr = NULL; + } + if (sr != NULL) { + while (--sr >= server_realm) + if (sr == server_realm || sr[-1] == '.') + return state->sr = sr; + state->sr = NULL; + } + return NULL; +} + +static void +hier_init(struct hier_iter *state, const char *local_realm, const char *server_realm) +{ + size_t llen; + size_t slen; + size_t len; + const char *lr; + const char *sr; + + state->local_realm = local_realm; + state->server_realm = server_realm; + state->llen = llen = strlen(local_realm); + state->slen = slen = strlen(server_realm); + state->len = 0; + state->num = 0; + + if (slen == 0 || llen == 0) + return; + + /* Find first difference from the back */ + for (lr = local_realm + llen, sr = server_realm + slen; + lr != local_realm && sr != server_realm; + --lr, --sr) { + if (lr[-1] != sr[-1]) + break; + if (lr[-1] == '.') + len = llen - (lr - local_realm); + } + + /* Nothing in common? */ + if (*lr == '\0') + return; + + /* Everything in common? */ + if (llen == slen && lr == local_realm) + return; + + /* Is one realm is a suffix of the other? */ + if ((llen < slen && lr == local_realm && sr[-1] == '.') || + (llen > slen && sr == server_realm && lr[-1] == '.')) + len = llen - (lr - local_realm); + + state->len = len; + /* `lr` starts at local realm and walks up the tree to common suffix */ + state->lr = local_realm; + /* `sr` starts at common suffix in server realm and walks down the tree */ + state->sr = server_realm + slen - len; + + /* Count elements and reset */ + while (hier_next(state) != NULL) + ++state->num; + state->lr = local_realm; + state->sr = server_realm + slen - len; +} + +/* + * Find a referral path from crealm to srealm via our_realm. + * Either via [capaths] or hierarchicaly. + */ +KRB5_LIB_FUNCTION krb5_error_code KRB5_LIB_CALL +_krb5_find_capath(krb5_context context, + const char *client_realm, + const char *local_realm, + const char *server_realm, + krb5_boolean use_hierachical, + 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; + char **rp; + const char *r; + + *rpath = NULL; + *npath = 0; + + confpath = krb5_config_get_strings(context, NULL, "capaths", + client_realm, server_realm, NULL); + if (confpath == NULL) + confpath = krb5_config_get_strings(context, NULL, "capaths", + local_realm, server_realm, NULL); + /* + * 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. + * Ignoring a [capaths] settings risks loops plus would violate explicit + * policy and the principle of least surpise. + */ + if (confpath != NULL) { + char **start = confpath; + size_t i; + size_t n; + + for (rp = start; *rp; rp++) + if (strcmp(*rp, local_realm) == 0) + start = rp+1; + n = rp - start; + + if (n == 0) + return 0; + + capath = calloc(n + 1, sizeof(*capath)); + if (capath == NULL) + return krb5_enomem(context); + + for (i = 0, rp = start; *rp; rp++) { + if ((capath[i++] = strdup(*rp)) == NULL) { + _krb5_free_capath(context, capath); + krb5_config_free_strings(confpath); + return krb5_enomem(context); + } + } + krb5_config_free_strings(confpath); + capath[i] = NULL; + *rpath = capath; + *npath = n; + return 0; + } + + /* + * With hierarchical referrals, we ignore the client realm, and build a + * path forward from our own realm! + */ + 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; + + hier_init(&hier_state, local_realm, server_realm); + if (hier_state.num == 0) + return 0; + + rp = capath = calloc(hier_state.num + 1, sizeof(*capath)); + if (capath == NULL) + return krb5_enomem(context); + while ((r = hier_next(&hier_state)) != NULL) { + if ((*rp++ = strdup(r)) == NULL) { + _krb5_free_capath(context, capath); + return krb5_enomem(context); + } + } + + *rp = NULL; + *rpath = capath; + *npath = hier_state.num; + return 0; +} + KRB5_LIB_FUNCTION krb5_error_code KRB5_LIB_CALL krb5_check_transited(krb5_context context, krb5_const_realm client_realm, @@ -406,35 +604,33 @@ krb5_check_transited(krb5_context context, unsigned int num_realms, int *bad_realm) { - char **tr_realms; - char **p; - size_t i; - - if(num_realms == 0) - return 0; - - tr_realms = krb5_config_get_strings(context, NULL, - "capaths", - client_realm, - server_realm, - NULL); - for(i = 0; i < num_realms; i++) { - for(p = tr_realms; p && *p; p++) { - if(strcmp(*p, realms[i]) == 0) + krb5_error_code ret = 0; + char **capath = NULL; + size_t num_capath = 0; + size_t i = 0; + size_t j = 0; + + ret = _krb5_find_capath(context, client_realm, client_realm, server_realm, + TRUE, &capath, &num_capath); + + for (i = 0; i < num_realms; i++) { + for (j = 0; j < num_capath; ++j) { + if (strcmp(realms[i], capath[j]) == 0) break; } - if(p == NULL || *p == NULL) { - krb5_config_free_strings(tr_realms); + if (j == num_capath) { + _krb5_free_capath(context, capath); krb5_set_error_message (context, KRB5KRB_AP_ERR_ILL_CR_TKT, N_("no transit allowed " "through realm %s from %s to %s", ""), realms[i], client_realm, server_realm); - if(bad_realm) + if (bad_realm) *bad_realm = i; return KRB5KRB_AP_ERR_ILL_CR_TKT; } } - krb5_config_free_strings(tr_realms); + + _krb5_free_capath(context, capath); return 0; } @@ -487,4 +683,3 @@ main(int argc, char **argv) return 0; } #endif - diff --git a/lib/krb5/verify_krb5_conf.c b/lib/krb5/verify_krb5_conf.c index 084f251899..30b2c2d777 100644 --- a/lib/krb5/verify_krb5_conf.c +++ b/lib/krb5/verify_krb5_conf.c @@ -410,6 +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 }, { "host_timeout", krb5_config_string, check_time, 0 }, { "http_proxy", krb5_config_string, check_host /* XXX */, 0 }, { "ignore_addresses", krb5_config_string, NULL, 0 }, @@ -480,6 +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 }, { "kdc", krb5_config_string, check_host, 0 }, { "kpasswd_server", krb5_config_string, check_host, 0 }, { "krb524_server", krb5_config_string, check_host, 0 }, diff --git a/lib/krb5/version-script.map b/lib/krb5/version-script.map index dc3cc63dc1..e2ae545812 100644 --- a/lib/krb5/version-script.map +++ b/lib/krb5/version-script.map @@ -784,6 +784,10 @@ HEIMDAL_KRB5_2.0 { _krb5_fast_cf2; _krb5_fast_armor_key; + # TGS + _krb5_find_capath; + _krb5_free_capath; + local: *; }; diff --git a/tests/kdc/krb5.conf.in b/tests/kdc/krb5.conf.in index 6248a057e6..aa3b44c645 100644 --- a/tests/kdc/krb5.conf.in +++ b/tests/kdc/krb5.conf.in @@ -129,4 +129,5 @@ TEST4.H5L.SE = TEST3.H5L.SE SOME-REALM6.US = SOME-REALM5.FR SOME-REALM7.UK = SOME-REALM6.US + SOME-REALM7.UK = SOME-REALM5.FR }