PRF/random_to_key allocation fix
authorLuke Howard <lukeh@padl.com>
Sat, 17 Sep 2011 06:24:53 +0000 (16:24 +1000)
committerLuke Howard <lukeh@padl.com>
Sat, 17 Sep 2011 06:24:53 +0000 (16:24 +1000)
MIT and Heimdal uses different allocation strategies
(caller-allocates, callee-allocates) for the same functions,
unfortunately.

Conflicts:

moonshot/mech_eap/util.h

moonshot/mech_eap/pseudo_random.c
moonshot/mech_eap/util.h
moonshot/mech_eap/util_krb.c
moonshot/mech_eap/util_name.c

index b0ca1ea..61d1f2a 100644 (file)
@@ -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;
 
index 7fa3495..4f54d41 100644 (file)
@@ -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
index 8775c83..5eaa31e 100644 (file)
@@ -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));
 
index 344c287..6045724 100644 (file)
@@ -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;
         }