Skip to content

Commit

Permalink
Fix transit path validation
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Viktor Dukhovni authored and nicowilliams committed Aug 8, 2016
1 parent 0561396 commit 1501740
Show file tree
Hide file tree
Showing 5 changed files with 224 additions and 21 deletions.
3 changes: 2 additions & 1 deletion lib/krb5/libkrb5-exports.def.in
Expand Up @@ -797,4 +797,5 @@ EXPORTS
_krb5_plugin_find;
_krb5_plugin_free;
_krb5_expand_path_tokensv;

_krb5_find_capath;
_krb5_free_capath;
235 changes: 215 additions & 20 deletions lib/krb5/transited.c
Expand Up @@ -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,
Expand All @@ -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;
}

Expand Down Expand Up @@ -487,4 +683,3 @@ main(int argc, char **argv)
return 0;
}
#endif

2 changes: 2 additions & 0 deletions lib/krb5/verify_krb5_conf.c
Expand Up @@ -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 },
Expand Down Expand Up @@ -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 },
Expand Down
4 changes: 4 additions & 0 deletions lib/krb5/version-script.map
Expand Up @@ -784,6 +784,10 @@ HEIMDAL_KRB5_2.0 {
_krb5_fast_cf2;
_krb5_fast_armor_key;

# TGS
_krb5_find_capath;
_krb5_free_capath;

local:
*;
};
1 change: 1 addition & 0 deletions tests/kdc/krb5.conf.in
Expand Up @@ -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
}

0 comments on commit 1501740

Please sign in to comment.