From 7e0ce72d965b4dd554235ff760a6b523b698c959 Mon Sep 17 00:00:00 2001 From: Luke Howard Date: Sat, 17 Sep 2011 16:24:53 +1000 Subject: [PATCH] PRF/random_to_key allocation fix MIT and Heimdal uses different allocation strategies (caller-allocates, callee-allocates) for the same functions, unfortunately. Conflicts: moonshot/mech_eap/util.h --- mech_eap/pseudo_random.c | 22 ++++++++++---- mech_eap/util.h | 25 ++++++++++++++++ mech_eap/util_krb.c | 77 +++++++++++++++++++++++++++--------------------- mech_eap/util_name.c | 2 +- 4 files changed, 85 insertions(+), 41 deletions(-) diff --git a/mech_eap/pseudo_random.c b/mech_eap/pseudo_random.c index b0ca1ea..61d1f2a 100644 --- a/mech_eap/pseudo_random.c +++ b/mech_eap/pseudo_random.c @@ -82,11 +82,8 @@ gssEapPseudoRandom(OM_uint32 *minor, GSSEAP_KRB_INIT(&krbContext); - t.length = 0; - t.data = NULL; - - ns.length = 0; - ns.data = NULL; + KRB_DATA_INIT(&t); + KRB_DATA_INIT(&ns); if (prf_key != GSS_C_PRF_KEY_PARTIAL && prf_key != GSS_C_PRF_KEY_FULL) { @@ -114,12 +111,15 @@ gssEapPseudoRandom(OM_uint32 *minor, goto cleanup; } +#ifndef HAVE_HEIMDAL_VERSION + /* Same API, but different allocation rules, unfortunately. */ t.length = prflen; t.data = GSSEAP_MALLOC(t.length); if (t.data == NULL) { code = ENOMEM; goto cleanup; } +#endif memcpy((unsigned char *)ns.data + 4, prf_in->value, prf_in->length); i = 0; @@ -141,8 +141,18 @@ gssEapPseudoRandom(OM_uint32 *minor, cleanup: if (code != 0) gss_release_buffer(&tmpMinor, prf_out); - krb5_free_data_contents(krbContext, &ns); + if (ns.data != NULL) { + memset(ns.data, 0, ns.length); + GSSEAP_FREE(ns.data); + } +#ifdef HAVE_HEIMDAL_VERSION krb5_free_data_contents(krbContext, &t); +#else + if (t.data != NULL) { + memset(t.data, 0, t.length); + GSSEAP_FREE(t.data); + } +#endif *minor = code; diff --git a/mech_eap/util.h b/mech_eap/util.h index 7fa3495..4f54d41 100644 --- a/mech_eap/util.h +++ b/mech_eap/util.h @@ -344,6 +344,21 @@ gssEapDeriveRfc3961Key(OM_uint32 *minor, krb5_keyblock *pKey); /* util_krb.c */ + +#ifndef KRB_MALLOC +/* + * If your Kerberos library uses a different allocator to your + * GSS mechanism glue, then you might wish to define these in + * config.h or elsewhere. This should eventually go away when + * we no longer need to allocate memory that is freed by the + * Kerberos library. + */ +#define KRB_CALLOC calloc +#define KRB_MALLOC malloc +#define KRB_FREE free +#define KRB_REALLOC realloc +#endif /* KRB_MALLOC */ + #ifdef HAVE_HEIMDAL_VERSION #define KRB_TIME_FOREVER ((time_t)~0L) @@ -362,6 +377,8 @@ gssEapDeriveRfc3961Key(OM_uint32 *minor, #define KRB_CRYPTO_CONTEXT(ctx) (krbCrypto) +#define KRB_DATA_INIT(d) krb5_data_zero((d)) + #else #define KRB_TIME_FOREVER KRB5_INT32_MAX @@ -380,6 +397,12 @@ gssEapDeriveRfc3961Key(OM_uint32 *minor, #define KRB_CRYPTO_CONTEXT(ctx) (&(ctx)->rfc3961Key) +#define KRB_DATA_INIT(d) do { \ + (d)->magic = KV5M_DATA; \ + (d)->length = 0; \ + (d)->data = NULL; \ + } while (0) + #endif /* HAVE_HEIMDAL_VERSION */ #define KRB_KEY_INIT(key) do { \ @@ -752,10 +775,12 @@ verifyTokenHeader(OM_uint32 *minor, /* Helper macros */ +#ifndef GSSEAP_MALLOC #define GSSEAP_CALLOC calloc #define GSSEAP_MALLOC malloc #define GSSEAP_FREE free #define GSSEAP_REALLOC realloc +#endif #ifndef GSSAPI_CALLCONV #define GSSAPI_CALLCONV KRB5_CALLCONV diff --git a/mech_eap/util_krb.c b/mech_eap/util_krb.c index 8775c83..5eaa31e 100644 --- a/mech_eap/util_krb.c +++ b/mech_eap/util_krb.c @@ -109,6 +109,9 @@ gssEapKerberosInit(OM_uint32 *minor, krb5_context *context) * Tn = pseudo-random(KMSK, n || "rfc4121-gss-eap") * L = output key size * K = truncate(L, T1 || T2 || .. || Tn) + * + * The output must be freed by krb5_free_keyblock_contents(), + * not GSSEAP_FREE(). */ OM_uint32 gssEapDeriveRfc3961Key(OM_uint32 *minor, @@ -121,41 +124,31 @@ gssEapDeriveRfc3961Key(OM_uint32 *minor, #ifndef HAVE_HEIMDAL_VERSION krb5_data data; #endif - krb5_data ns, t, prfOut; + krb5_data ns, t, derivedKeyData; krb5_keyblock kd; krb5_error_code code; size_t randomLength, keyLength, prfLength; unsigned char constant[4 + sizeof("rfc4121-gss-eap") - 1], *p; ssize_t i, remain; - GSSEAP_ASSERT(encryptionType != ENCTYPE_NULL); - - memset(pKey, 0, sizeof(*pKey)); - GSSEAP_KRB_INIT(&krbContext); + GSSEAP_ASSERT(encryptionType != ENCTYPE_NULL); + KRB_KEY_INIT(pKey); KRB_KEY_INIT(&kd); KRB_KEY_TYPE(&kd) = encryptionType; - t.data = NULL; - t.length = 0; - - prfOut.data = NULL; - prfOut.length = 0; + KRB_DATA_INIT(&ns); + KRB_DATA_INIT(&t); + KRB_DATA_INIT(&derivedKeyData); code = krb5_c_keylengths(krbContext, encryptionType, &randomLength, &keyLength); if (code != 0) goto cleanup; - KRB_KEY_DATA(&kd) = GSSEAP_MALLOC(keyLength); - if (KRB_KEY_DATA(&kd) == NULL) { - code = ENOMEM; - goto cleanup; - } - KRB_KEY_LENGTH(&kd) = keyLength; + /* Convert EAP MSK into a Kerberos key */ - /* Convert MSK into a Kerberos key */ #ifdef HAVE_HEIMDAL_VERSION code = krb5_random_to_key(krbContext, encryptionType, inputKey, MIN(inputKeyLength, randomLength), &kd); @@ -163,8 +156,15 @@ gssEapDeriveRfc3961Key(OM_uint32 *minor, data.length = MIN(inputKeyLength, randomLength); data.data = (char *)inputKey; + KRB_KEY_DATA(&kd) = KRB_MALLOC(keyLength); + if (KRB_KEY_DATA(&kd) == NULL) { + code = ENOMEM; + goto cleanup; + } + KRB_KEY_LENGTH(&kd) = keyLength; + code = krb5_c_random_to_key(krbContext, encryptionType, &data, &kd); -#endif +#endif /* HAVE_HEIMDAL_VERSION */ if (code != 0) goto cleanup; @@ -179,21 +179,24 @@ gssEapDeriveRfc3961Key(OM_uint32 *minor, if (code != 0) goto cleanup; +#ifndef HAVE_HEIMDAL_VERSION + /* Same API, but different allocation rules, unfortunately. */ t.length = prfLength; t.data = GSSEAP_MALLOC(t.length); if (t.data == NULL) { code = ENOMEM; goto cleanup; } +#endif - prfOut.length = randomLength; - prfOut.data = GSSEAP_MALLOC(prfOut.length); - if (prfOut.data == NULL) { + derivedKeyData.length = randomLength; + derivedKeyData.data = GSSEAP_MALLOC(derivedKeyData.length); + if (derivedKeyData.data == NULL) { code = ENOMEM; goto cleanup; } - for (i = 0, p = (unsigned char *)prfOut.data, remain = randomLength; + for (i = 0, p = (unsigned char *)derivedKeyData.data, remain = randomLength; remain > 0; p += t.length, remain -= t.length, i++) { @@ -208,31 +211,38 @@ gssEapDeriveRfc3961Key(OM_uint32 *minor, /* Finally, convert PRF output into a new key which we will return */ #ifdef HAVE_HEIMDAL_VERSION + krb5_free_keyblock_contents(krbContext, &kd); + KRB_KEY_INIT(&kd); + code = krb5_random_to_key(krbContext, encryptionType, - prfOut.data, prfOut.length, &kd); + derivedKeyData.data, derivedKeyData.length, &kd); #else - code = krb5_c_random_to_key(krbContext, encryptionType, &prfOut, &kd); + code = krb5_c_random_to_key(krbContext, encryptionType, + &derivedKeyData, &kd); #endif if (code != 0) goto cleanup; *pKey = kd; - KRB_KEY_DATA(&kd) = NULL; cleanup: - if (KRB_KEY_DATA(&kd) != NULL) { - memset(KRB_KEY_DATA(&kd), 0, KRB_KEY_LENGTH(&kd)); - GSSEAP_FREE(KRB_KEY_DATA(&kd)); - } + if (code != 0) + krb5_free_keyblock_contents(krbContext, &kd); +#ifdef HAVE_HEIMDAL_VERSION + krb5_free_data_contents(krbContext, &t); +#else if (t.data != NULL) { memset(t.data, 0, t.length); GSSEAP_FREE(t.data); } - if (prfOut.data != NULL) { - memset(prfOut.data, 0, prfOut.length); - GSSEAP_FREE(prfOut.data); +#endif + if (derivedKeyData.data != NULL) { + memset(derivedKeyData.data, 0, derivedKeyData.length); + GSSEAP_FREE(derivedKeyData.data); } + *minor = code; + return (code == 0) ? GSS_S_COMPLETE : GSS_S_FAILURE; } @@ -260,8 +270,7 @@ rfc3961ChecksumTypeForKey(OM_uint32 *minor, if (*minor != 0) return GSS_S_FAILURE; #else - data.length = 0; - data.data = NULL; + KRB_DATA_INIT(&data); memset(&cksum, 0, sizeof(cksum)); diff --git a/mech_eap/util_name.c b/mech_eap/util_name.c index 344c287..6045724 100644 --- a/mech_eap/util_name.c +++ b/mech_eap/util_name.c @@ -265,7 +265,7 @@ importEapNameFlags(OM_uint32 *minor, #ifdef HAVE_HEIMDAL_VERSION if (code == 0 && KRB_PRINC_REALM(krbPrinc) == NULL) { - KRB_PRINC_REALM(krbPrinc) = calloc(1, sizeof(char)); /* XXX */ + KRB_PRINC_REALM(krbPrinc) = KRB_CALLOC(1, sizeof(char)); if (KRB_PRINC_REALM(krbPrinc) == NULL) code = ENOMEM; } -- 2.1.4