From 06232dfccee57aa0f49ca94c6e0428ec94ef15ca Mon Sep 17 00:00:00 2001 From: Luke Howard Date: Sun, 8 Aug 2021 14:45:13 +1000 Subject: [PATCH] gss: fix import/export of accumulating contexts gss_{import,export}_sec_context did not work with partially accumulating contexts, where the initial context token had not been completely accumulated, Further, in gss_import_sec_context(), ctx->gc_input.value was not allocated to a buffer sufficiently large to accumulate the target length. --- lib/gssapi/mech/gss_export_sec_context.c | 95 +++++++++--------------- lib/gssapi/mech/gss_import_sec_context.c | 30 +++++--- lib/gssapi/test_context.c | 11 ++- tests/gss/check-spnego.in | 2 + 4 files changed, 64 insertions(+), 74 deletions(-) diff --git a/lib/gssapi/mech/gss_export_sec_context.c b/lib/gssapi/mech/gss_export_sec_context.c index 461ba0229..b3e85bbd1 100644 --- a/lib/gssapi/mech/gss_export_sec_context.c +++ b/lib/gssapi/mech/gss_export_sec_context.c @@ -33,13 +33,13 @@ gss_export_sec_context(OM_uint32 *minor_status, gss_ctx_id_t *context_handle, gss_buffer_t interprocess_token) { - OM_uint32 major_status = GSS_S_FAILURE; + OM_uint32 major_status = GSS_S_FAILURE, tmp_minor; krb5_storage *sp; krb5_data data; krb5_error_code kret; struct _gss_context *ctx; gssapi_mech_interface m; - gss_buffer_desc buf; + gss_buffer_desc buf = GSS_C_EMPTY_BUFFER; unsigned char verflags; *minor_status = 0; @@ -56,16 +56,10 @@ gss_export_sec_context(OM_uint32 *minor_status, if (ctx == NULL) return GSS_S_NO_CONTEXT; - if (!ctx->gc_ctx && ctx->gc_target_len) { - free(ctx); - *context_handle = GSS_C_NO_CONTEXT; - return GSS_S_NO_CONTEXT; - } - sp = krb5_storage_emem(); if (sp == NULL) { *minor_status = ENOMEM; - return GSS_S_FAILURE; + goto failure; } krb5_storage_set_byteorder(sp, KRB5_STORAGE_BYTEORDER_PACKED); @@ -98,37 +92,36 @@ gss_export_sec_context(OM_uint32 *minor_status, *minor_status = kret; goto failure; } + } else if (ctx->gc_ctx == GSS_C_NO_CONTEXT) { + gss_delete_sec_context(&tmp_minor, context_handle, + GSS_C_NO_BUFFER); + return GSS_S_NO_CONTEXT; } - if (!ctx->gc_ctx) { - free(ctx); - *context_handle = GSS_C_NO_CONTEXT; - return GSS_S_COMPLETE; - } + if (ctx->gc_ctx) { + m = ctx->gc_mech; - m = ctx->gc_mech; + major_status = m->gm_export_sec_context(minor_status, + &ctx->gc_ctx, &buf); - major_status = m->gm_export_sec_context(minor_status, - &ctx->gc_ctx, &buf); + if (major_status != GSS_S_COMPLETE) { + _gss_mg_error(m, *minor_status); + goto failure; + } - if (major_status != GSS_S_COMPLETE) { - _gss_mg_error(m, *minor_status); - goto failure; - } + kret = krb5_store_datalen(sp, m->gm_mech_oid.elements, + m->gm_mech_oid.length); + if (kret) { + *minor_status = kret; + goto failure; + } - kret = krb5_store_datalen(sp, m->gm_mech_oid.elements, - m->gm_mech_oid.length); - if (kret) { - *minor_status = kret; - goto failure; - } - - kret = krb5_store_datalen(sp, buf.value, buf.length); - if (kret) { - *minor_status = kret; - goto failure; - } - _gss_secure_release_buffer(minor_status, &buf); + kret = krb5_store_datalen(sp, buf.value, buf.length); + if (kret) { + *minor_status = kret; + goto failure; + } + } kret = krb5_storage_to_data(sp, &data); if (kret) { @@ -139,34 +132,18 @@ gss_export_sec_context(OM_uint32 *minor_status, interprocess_token->length = data.length; interprocess_token->value = data.data; + major_status = GSS_S_COMPLETE; + _gss_mg_log(1, "gss-esc: token length %zu", data.length); - free(ctx); - *context_handle = GSS_C_NO_CONTEXT; - -#if 0 - if (!interprocess_token->value) { - /* - * We are in trouble here - the context is - * already gone. This is allowed as long as we - * set the caller's context_handle to - * GSS_C_NO_CONTEXT, which we did above. - * Return GSS_S_FAILURE. - */ - _mg_buffer_zero(interprocess_token); - *minor_status = ENOMEM; - return (GSS_S_FAILURE); - } - p = interprocess_token->value; - p[0] = m->gm_mech_oid.length >> 8; - p[1] = m->gm_mech_oid.length; - memcpy(p + 2, m->gm_mech_oid.elements, m->gm_mech_oid.length); - memcpy(p + 2 + m->gm_mech_oid.length, buf.value, buf.length); - } else { - } -#endif - failure: + if (major_status == GSS_S_COMPLETE && *minor_status == 0) + gss_delete_sec_context(&tmp_minor, context_handle, + GSS_C_NO_BUFFER); + else if (*minor_status) + major_status = GSS_S_FAILURE; + + _gss_secure_release_buffer(minor_status, &buf); krb5_storage_free(sp); return major_status; } diff --git a/lib/gssapi/mech/gss_import_sec_context.c b/lib/gssapi/mech/gss_import_sec_context.c index 298eaeb09..502db9bf5 100644 --- a/lib/gssapi/mech/gss_import_sec_context.c +++ b/lib/gssapi/mech/gss_import_sec_context.c @@ -33,7 +33,7 @@ gss_import_sec_context(OM_uint32 *minor_status, const gss_buffer_t interprocess_token, gss_ctx_id_t *context_handle) { - OM_uint32 ret = GSS_S_FAILURE; + OM_uint32 ret = GSS_S_FAILURE, tmp_minor; krb5_storage *sp; krb5_data data; gssapi_mech_interface m; @@ -88,10 +88,15 @@ gss_import_sec_context(OM_uint32 *minor_status, if (krb5_ret_data(sp, &data)) goto failure; + ctx->gc_input.value = calloc(target_len, 1); + if (ctx->gc_input.value == NULL) + goto failure; + ctx->gc_target_len = target_len; - ctx->gc_input.value = data.data; ctx->gc_input.length = data.length; - /* Don't need to free data because we gave it to gc_input */ + memcpy(ctx->gc_input.value, data.data, data.length); + + krb5_data_free(&data); } if (verflags & EXPORT_CONTEXT_FLAG_MECH_CTX) { @@ -102,8 +107,10 @@ gss_import_sec_context(OM_uint32 *minor_status, mech_oid.elements = data.data; m = __gss_get_mechanism(&mech_oid); krb5_data_free(&data); - if (!m) - return GSS_S_DEFECTIVE_TOKEN; + if (m == NULL) { + ret = GSS_S_DEFECTIVE_TOKEN; + goto failure; + } ctx->gc_mech = m; if (krb5_ret_data(sp, &data)) @@ -113,16 +120,17 @@ gss_import_sec_context(OM_uint32 *minor_status, buf.value = data.data; ret = m->gm_import_sec_context(minor_status, &buf, &ctx->gc_ctx); + _gss_secure_release_buffer(&tmp_minor, &buf); if (ret != GSS_S_COMPLETE) { _gss_mg_error(m, *minor_status); - free(ctx); - } else { - *context_handle = (gss_ctx_id_t) ctx; - } + goto failure; + } } - krb5_storage_free(sp); - return (ret); + *context_handle = (gss_ctx_id_t) ctx; + ctx = NULL; + + ret = GSS_S_COMPLETE; failure: free(ctx); diff --git a/lib/gssapi/test_context.c b/lib/gssapi/test_context.c index bacf912e7..d7b6090b8 100644 --- a/lib/gssapi/test_context.c +++ b/lib/gssapi/test_context.c @@ -279,10 +279,13 @@ loop(gss_OID mechoid, gsskrb5_get_time_offset(&client_time_offset); gsskrb5_set_time_offset(server_time_offset); - tmp.length = output_token.length - offset; - if (token_split && tmp.length > token_split) - tmp.length = token_split; - tmp.value = (char *)output_token.value + offset; + if (output_token.length && ((uint8_t *)output_token.value)[0] == 0x60) { + tmp.length = output_token.length - offset; + if (token_split && tmp.length > token_split) + tmp.length = token_split; + tmp.value = (char *)output_token.value + offset; + } else + tmp = output_token; if (verbose_flag) printf("loop #%d: accept offset=%zu len=%zu\n", num_loops, diff --git a/tests/gss/check-spnego.in b/tests/gss/check-spnego.in index fe643eebb..3cf560896 100644 --- a/tests/gss/check-spnego.in +++ b/tests/gss/check-spnego.in @@ -135,6 +135,7 @@ ${context} \ --mech-type=spnego \ --ret-mech-type=krb5 \ --name-type=hostbased-service \ + --export-import-context \ host@host.test.h5l.se || \ { exitcode=1 ; echo test failed; } @@ -145,6 +146,7 @@ ${context} \ --mech-type=spnego \ --ret-mech-type=krb5 \ --name-type=hostbased-service \ + --export-import-context \ host@host.test.h5l.se || \ { exitcode=1 ; echo test failed; }