From ad426d038576d05a6d1f4420978862802f79dbfd Mon Sep 17 00:00:00 2001 From: Luke Howard Date: Thu, 3 Jan 2019 09:06:46 +1100 Subject: [PATCH] gssapi: import bugfixes from Apple Heimdal-520 * check `ctx->gc_ctx` in `gss_inquire_context()` * check `gm_inquire_cred != NULL` in `gss_inquire_context()` * check `min_lifetime` in `gss_inquire_cred()` * check `gm_inquire_cred_by_mech != NULL` in `gss_inquire_cred_by_mech()` * set mech error in `gss_inquire_cred_by_oid()` * don't clobber error in `gss_inquire_cred_by_oid()` * don't pass NULL minor_status to `gss_krb5_free_lucid_sec_context()` * allow NULL ccache in `gss_krb5_ccache_name()` * NULL names OK in `_gss_find_mn()` * allow empty names in `gss_import_name()` (removes `input_name_buffer` length check). to support ANONYMOUS. in `gss_import_name()`, ignore mech name import failure as long as it's possible to import the name in some other mechanism * better argument validation in `gss_export_sec_context()` * in `gss_compare_name()`, check `mn2 != NULL` * check `gss_add_oid_set_member()` return code in `gss_indicate_mechs()` * in `gss_destroy_cred()`, set output cred handle to `GSS_C_NO_CREDENTIAL` * cast size_t to OM_uint32 where required --- lib/gssapi/mech/gss_compare_name.c | 2 +- lib/gssapi/mech/gss_cred.c | 2 +- lib/gssapi/mech/gss_destroy_cred.c | 1 + lib/gssapi/mech/gss_export_sec_context.c | 18 +++++++++++++++++- lib/gssapi/mech/gss_import_name.c | 19 ++++++++++++++----- lib/gssapi/mech/gss_indicate_mechs.c | 19 ++++++++++++++----- lib/gssapi/mech/gss_inquire_context.c | 9 ++++++++- lib/gssapi/mech/gss_inquire_cred.c | 11 ++++++++++- lib/gssapi/mech/gss_inquire_cred_by_mech.c | 2 +- lib/gssapi/mech/gss_inquire_cred_by_oid.c | 10 +++++++--- lib/gssapi/mech/gss_krb5.c | 13 +++++++++---- lib/gssapi/mech/gss_unwrap.c | 13 ++++++++++++- lib/gssapi/mech/gss_utils.c | 5 +++-- 13 files changed, 98 insertions(+), 26 deletions(-) diff --git a/lib/gssapi/mech/gss_compare_name.c b/lib/gssapi/mech/gss_compare_name.c index fa2b08226..3fa5f24d9 100644 --- a/lib/gssapi/mech/gss_compare_name.c +++ b/lib/gssapi/mech/gss_compare_name.c @@ -60,7 +60,7 @@ gss_compare_name(OM_uint32 *minor_status, major_status = _gss_find_mn(minor_status, name2, mn1->gmn_mech_oid, &mn2); - if (major_status == GSS_S_COMPLETE) { + if (major_status == GSS_S_COMPLETE && mn2) { return (mn1->gmn_mech->gm_compare_name( minor_status, mn1->gmn_name, diff --git a/lib/gssapi/mech/gss_cred.c b/lib/gssapi/mech/gss_cred.c index f2cfb79b9..cc7d2ccaa 100644 --- a/lib/gssapi/mech/gss_cred.c +++ b/lib/gssapi/mech/gss_cred.c @@ -158,7 +158,7 @@ gss_import_cred(OM_uint32 * minor_status, goto out; } oid.elements = data.data; - oid.length = data.length; + oid.length = (OM_uint32)data.length; m = __gss_get_mechanism(&oid); krb5_data_free(&data); diff --git a/lib/gssapi/mech/gss_destroy_cred.c b/lib/gssapi/mech/gss_destroy_cred.c index 100271ad5..03f1df3b7 100644 --- a/lib/gssapi/mech/gss_destroy_cred.c +++ b/lib/gssapi/mech/gss_destroy_cred.c @@ -43,6 +43,7 @@ gss_destroy_cred(void *status, return GSS_S_COMPLETE; cred = (struct _gss_cred *)*cred_handle; + *cred_handle = GSS_C_NO_CREDENTIAL; while (HEIM_SLIST_FIRST(&cred->gc_mc)) { mc = HEIM_SLIST_FIRST(&cred->gc_mc); diff --git a/lib/gssapi/mech/gss_export_sec_context.c b/lib/gssapi/mech/gss_export_sec_context.c index 369f3a225..1718ceb79 100644 --- a/lib/gssapi/mech/gss_export_sec_context.c +++ b/lib/gssapi/mech/gss_export_sec_context.c @@ -38,7 +38,23 @@ gss_export_sec_context(OM_uint32 *minor_status, gssapi_mech_interface m = ctx->gc_mech; gss_buffer_desc buf; - _mg_buffer_zero(interprocess_token); + *minor_status = 0; + + if (interprocess_token) + _mg_buffer_zero(interprocess_token); + else + return GSS_S_CALL_INACCESSIBLE_READ; + + if (context_handle == NULL) + return GSS_S_NO_CONTEXT; + + ctx = (struct _gss_context *) *context_handle; + if (ctx == NULL || ctx->gc_ctx == NULL) { + *minor_status = 0; + return GSS_S_NO_CONTEXT; + } + + m = ctx->gc_mech; major_status = m->gm_export_sec_context(minor_status, &ctx->gc_ctx, &buf); diff --git a/lib/gssapi/mech/gss_import_name.c b/lib/gssapi/mech/gss_import_name.c index 121b242fe..df2deba31 100644 --- a/lib/gssapi/mech/gss_import_name.c +++ b/lib/gssapi/mech/gss_import_name.c @@ -186,12 +186,14 @@ gss_import_name(OM_uint32 *minor_status, struct _gss_mech_switch *m; gss_name_t rname; + if (input_name_buffer == GSS_C_NO_BUFFER) + return GSS_S_CALL_INACCESSIBLE_READ; + if (output_name == NULL) + return GSS_S_CALL_INACCESSIBLE_WRITE; + *output_name = GSS_C_NO_NAME; - if (input_name_buffer->length == 0) { - *minor_status = 0; - return (GSS_S_BAD_NAME); - } + /* Allow empty names since that's valid (ANONYMOUS for example) */ _gss_load_mech(); @@ -261,7 +263,14 @@ gss_import_name(OM_uint32 *minor_status, if (major_status != GSS_S_COMPLETE) { _gss_mg_error(&m->gm_mech, major_status, *minor_status); free(mn); - goto out; + /** + * If we failed to import the name in a mechanism, it + * will be ignored as long as its possible to import + * name in some other mechanism. We will catch the + * failure later though in gss_init_sec_context() or + * another function. + */ + continue; } mn->gmn_mech = &m->gm_mech; diff --git a/lib/gssapi/mech/gss_indicate_mechs.c b/lib/gssapi/mech/gss_indicate_mechs.c index c38a5864c..22493e57e 100644 --- a/lib/gssapi/mech/gss_indicate_mechs.c +++ b/lib/gssapi/mech/gss_indicate_mechs.c @@ -33,7 +33,7 @@ gss_indicate_mechs(OM_uint32 *minor_status, gss_OID_set *mech_set) { struct _gss_mech_switch *m; - OM_uint32 major_status; + OM_uint32 major_status, junk; gss_OID_set set; size_t i; @@ -50,16 +50,25 @@ gss_indicate_mechs(OM_uint32 *minor_status, minor_status, &set); if (major_status) continue; - for (i = 0; i < set->count; i++) - gss_add_oid_set_member( + major_status = GSS_S_COMPLETE; + for (i = 0; i < set->count; i++) { + major_status = gss_add_oid_set_member( minor_status, &set->elements[i], mech_set); + if (major_status) + break; + } gss_release_oid_set(minor_status, &set); } else { - gss_add_oid_set_member( + major_status = gss_add_oid_set_member( minor_status, m->gm_mech_oid, mech_set); } + if (major_status) + break; } + if (major_status) + gss_release_oid_set(&junk, mech_set); + *minor_status = 0; - return (GSS_S_COMPLETE); + return major_status; } diff --git a/lib/gssapi/mech/gss_inquire_context.c b/lib/gssapi/mech/gss_inquire_context.c index aedaa6cb9..63fae9180 100644 --- a/lib/gssapi/mech/gss_inquire_context.c +++ b/lib/gssapi/mech/gss_inquire_context.c @@ -41,7 +41,7 @@ gss_inquire_context(OM_uint32 *minor_status, { OM_uint32 major_status; struct _gss_context *ctx = (struct _gss_context *) context_handle; - gssapi_mech_interface m = ctx->gc_mech; + gssapi_mech_interface m; struct _gss_name *name; gss_name_t src_mn, targ_mn; @@ -60,6 +60,13 @@ gss_inquire_context(OM_uint32 *minor_status, *mech_type = GSS_C_NO_OID; src_mn = targ_mn = GSS_C_NO_NAME; + if (ctx == NULL || ctx->gc_ctx == NULL) { + *minor_status = 0; + return GSS_S_NO_CONTEXT; + } + + m = ctx->gc_mech; + major_status = m->gm_inquire_context(minor_status, ctx->gc_ctx, src_name ? &src_mn : NULL, diff --git a/lib/gssapi/mech/gss_inquire_cred.c b/lib/gssapi/mech/gss_inquire_cred.c index 1b0204e59..e8222c445 100644 --- a/lib/gssapi/mech/gss_inquire_cred.c +++ b/lib/gssapi/mech/gss_inquire_cred.c @@ -100,6 +100,9 @@ gss_inquire_cred(OM_uint32 *minor_status, gss_name_t mc_name; OM_uint32 mc_lifetime; + if (mc->gmc_mech->gm_inquire_cred == NULL) + continue; + major_status = mc->gmc_mech->gm_inquire_cred(minor_status, mc->gmc_cred, &mc_name, &mc_lifetime, &usage, NULL); if (major_status) @@ -135,6 +138,9 @@ gss_inquire_cred(OM_uint32 *minor_status, gss_name_t mc_name; OM_uint32 mc_lifetime; + if (m->gm_mech.gm_inquire_cred == NULL) + continue; + major_status = m->gm_mech.gm_inquire_cred(minor_status, GSS_C_NO_CREDENTIAL, &mc_name, &mc_lifetime, &usage, NULL); @@ -169,12 +175,15 @@ gss_inquire_cred(OM_uint32 *minor_status, } } - if (found == 0) { + if (found == 0 || min_lifetime == 0) { gss_name_t n = (gss_name_t)name; if (n) gss_release_name(minor_status, &n); gss_release_oid_set(minor_status, mechanisms); *minor_status = 0; + if (min_lifetime == 0) + return (GSS_S_CREDENTIALS_EXPIRED); + return (GSS_S_NO_CRED); } diff --git a/lib/gssapi/mech/gss_inquire_cred_by_mech.c b/lib/gssapi/mech/gss_inquire_cred_by_mech.c index 7bd0bfaad..8ff2cc43c 100644 --- a/lib/gssapi/mech/gss_inquire_cred_by_mech.c +++ b/lib/gssapi/mech/gss_inquire_cred_by_mech.c @@ -55,7 +55,7 @@ gss_inquire_cred_by_mech(OM_uint32 *minor_status, *cred_usage = 0; m = __gss_get_mechanism(mech_type); - if (!m) + if (m == NULL || m->gm_inquire_cred_by_mech == NULL) return (GSS_S_NO_CRED); if (cred_handle != GSS_C_NO_CREDENTIAL) { diff --git a/lib/gssapi/mech/gss_inquire_cred_by_oid.c b/lib/gssapi/mech/gss_inquire_cred_by_oid.c index 8836a09ff..91166139d 100644 --- a/lib/gssapi/mech/gss_inquire_cred_by_oid.c +++ b/lib/gssapi/mech/gss_inquire_cred_by_oid.c @@ -50,6 +50,8 @@ gss_inquire_cred_by_oid (OM_uint32 *minor_status, if (cred == NULL) return GSS_S_NO_CRED; + status = GSS_S_FAILURE; + HEIM_SLIST_FOREACH(mc, &cred->gc_mc, gmc_link) { gss_buffer_set_t rset = GSS_C_NO_BUFFER_SET; size_t i; @@ -66,10 +68,12 @@ gss_inquire_cred_by_oid (OM_uint32 *minor_status, status = m->gm_inquire_cred_by_oid(minor_status, mc->gmc_cred, desired_object, &rset); - if (status != GSS_S_COMPLETE) + if (status != GSS_S_COMPLETE) { + _gss_mg_error(m, status, *minor_status); continue; + } - for (i = 0; i < rset->count; i++) { + for (i = 0; rset != NULL && i < rset->count; i++) { status = gss_add_buffer_set_member(minor_status, &rset->elements[i], &set); if (status != GSS_S_COMPLETE) @@ -77,7 +81,7 @@ gss_inquire_cred_by_oid (OM_uint32 *minor_status, } gss_release_buffer_set(minor_status, &rset); } - if (set == GSS_C_NO_BUFFER_SET) + if (set == GSS_C_NO_BUFFER_SET && status == GSS_S_COMPLETE) status = GSS_S_FAILURE; *data_set = set; *minor_status = 0; diff --git a/lib/gssapi/mech/gss_krb5.c b/lib/gssapi/mech/gss_krb5.c index f81df431d..4f16761e8 100644 --- a/lib/gssapi/mech/gss_krb5.c +++ b/lib/gssapi/mech/gss_krb5.c @@ -390,8 +390,9 @@ out: krb5_free_context(context); if (ret) { + OM_uint32 junk; if (ctx) - gss_krb5_free_lucid_sec_context(NULL, ctx); + gss_krb5_free_lucid_sec_context(&junk, ctx); *minor_status = ret; return GSS_S_FAILURE; @@ -527,8 +528,12 @@ gss_krb5_ccache_name(OM_uint32 *minor_status, if (out_name) *out_name = NULL; - buffer.value = rk_UNCONST(name); - buffer.length = strlen(name); + if (name) { + buffer.value = rk_UNCONST(name); + buffer.length = strlen(name); + } else { + _mg_buffer_zero(&buffer); + } HEIM_SLIST_FOREACH(m, &_gss_mechs, gm_link) { if (m->gm_mech.gm_set_sec_context_option == NULL) @@ -642,7 +647,7 @@ gsskrb5_extract_authz_data_from_sec_context(OM_uint32 *minor_status, oid.components[oid.length - 1] = ad_type; - oid_flat.length = der_length_oid(&oid); + oid_flat.length = (OM_uint32)der_length_oid(&oid); oid_flat.elements = malloc(oid_flat.length); if (oid_flat.elements == NULL) { free(oid.components); diff --git a/lib/gssapi/mech/gss_unwrap.c b/lib/gssapi/mech/gss_unwrap.c index 6bf6088f3..dd6363b0f 100644 --- a/lib/gssapi/mech/gss_unwrap.c +++ b/lib/gssapi/mech/gss_unwrap.c @@ -37,7 +37,18 @@ gss_unwrap(OM_uint32 *minor_status, gss_qop_t *qop_state) { struct _gss_context *ctx = (struct _gss_context *) context_handle; - gssapi_mech_interface m = ctx->gc_mech; + gssapi_mech_interface m; + + if (conf_state) + *conf_state = 0; + if (qop_state) + *qop_state = 0; + + if (ctx == NULL) { + *minor_status = 0; + return GSS_S_NO_CONTEXT; + } + m = ctx->gc_mech; return (m->gm_unwrap(minor_status, ctx->gc_ctx, input_message_buffer, output_message_buffer, diff --git a/lib/gssapi/mech/gss_utils.c b/lib/gssapi/mech/gss_utils.c index a13133ab1..571382b78 100644 --- a/lib/gssapi/mech/gss_utils.c +++ b/lib/gssapi/mech/gss_utils.c @@ -30,7 +30,8 @@ static OM_uint32 _gss_copy_oid(OM_uint32 *minor_status, - gss_const_OID from_oid, gss_OID to_oid) + gss_const_OID from_oid, + gss_OID to_oid) { size_t len = from_oid->length; @@ -41,7 +42,7 @@ _gss_copy_oid(OM_uint32 *minor_status, *minor_status = ENOMEM; return GSS_S_FAILURE; } - to_oid->length = len; + to_oid->length = (OM_uint32)len; memcpy(to_oid->elements, from_oid->elements, len); return (GSS_S_COMPLETE); }