Fix pa-type 15 request against Heimdal server which

returns a different OID.

Fix a typo in the doc pointed out by Olga Kornievskaia

Add a comment about why SILLYDECRYPT is there.

Fix some formatting to prevent line-wrapping.

Encapsulate all the work-arounds for working against a
Longhorn server within #ifdef LONGHORN_BETA_COMPAT so they
are easily recognized and can be removed when the issues
are fixed in released versions of Longhorn.

Fix a memory leak in the server.  (Change the client code
to copy OID values rather than returning pointers to static
memory so that no special cases are required when freeing
structures which reference them.)

Remove an fprintf() from the pkinit_matching code.

Change functino definitions in pkinit_lib.c to match the
coding style.

git-svn-id: svn://anonsvn.mit.edu/krb5/users/coffman/pkinit@19610 dc483132-0cff-0310-8789-dd5450dbe970

9 files changed, 357 insertions, 113 deletions

diff --git a/doc/admin.texinfo b/doc/admin.texinfo
index 7d5b4ae..79608f3 100644
--- a/doc/admin.texinfo
+++ b/doc/admin.texinfo
@@ -1168,7 +1168,7 @@ This option has context-specific behavior.
 @itemx pkinit_identities
 @i{file-name} specifies the name of a PEM-format file
 containing the user's certificate.  If @i{key-file-name} is
-not specified, the uesr's private key is expected to be
+not specified, the user's private key is expected to be
 in @i{file-name} as well.  Otherwise, @i{key-file-name}
 is the name of the file containing the private key.
 @item pkinit_anchors
diff --git a/src/plugins/preauth/pkinit/pkinit.h b/src/plugins/preauth/pkinit/pkinit.h
index 11bfda3..5964dd9 100644
--- a/src/plugins/preauth/pkinit/pkinit.h
+++ b/src/plugins/preauth/pkinit/pkinit.h
@@ -37,6 +37,20 @@
 #include <profile.h>
 #include "pkinit_accessor.h"
 
+/*
+ * It is anticipated that all the special checks currently
+ * required when talking to a Longhorn server will go away
+ * by the time it is officially released and all references
+ * to the longhorn global can be removed and any code
+ * #ifdef'd with LONGHORN_BETA_COMPAT can be removed.
+ * And this #define!
+ */
+#define LONGHORN_BETA_COMPAT 1
+#ifdef LONGHORN_BETA_COMPAT
+extern int longhorn;	    /* XXX Talking to a Longhorn server? */
+#endif
+
+
 #ifndef WITHOUT_PKCS11
 #include <opensc/pkcs11.h>
 
@@ -73,8 +87,6 @@ static inline void pkiDebug (const char *fmt, ...) { }
  * hack for now.  Fix all the uses eventually. */
 #define __FUNCTION__ __func__
 
-extern int longhorn;	    /* XXX Talking to a Longhorn server? */
-
 /* Macros to deal with converting between various data types... */
 #define PADATA_TO_KRB5DATA(pad, k5d) \
     (k5d)->length = (pad)->length; (k5d)->data = (char *)(pad)->contents;
@@ -273,13 +285,6 @@ void init_krb5_pa_pk_as_req_draft9(krb5_pa_pk_as_req_draft9 **in);
 void init_krb5_reply_key_pack(krb5_reply_key_pack **in);
 void init_krb5_reply_key_pack_draft9(krb5_reply_key_pack_draft9 **in);
 
-/*
- * a note about freeing krb5_auth_pack. the caller, if needed,
- * should separately free clientPublicValue->algorithm.algorithm.data.
- * in our implementation the client uses a static value for the
- * alg oid but on the kdc side the oid is decoded (and thus
- * allocated) from the rcvd msg
- */
 void init_krb5_auth_pack(krb5_auth_pack **in);
 void init_krb5_auth_pack_draft9(krb5_auth_pack_draft9 **in);
 void init_krb5_pa_pk_as_rep(krb5_pa_pk_as_rep **in);
@@ -298,9 +303,12 @@ void free_krb5_pa_pk_as_rep_draft9(krb5_pa_pk_as_rep_draft9 **in);
 void free_krb5_external_principal_identifier(krb5_external_principal_identifier ***in);
 void free_krb5_trusted_ca(krb5_trusted_ca ***in);
 void free_krb5_typed_data(krb5_typed_data ***in);
-void free_krb5_algorithm_identifier(krb5_algorithm_identifier ***in);
+void free_krb5_algorithm_identifiers(krb5_algorithm_identifier ***in);
+void free_krb5_algorithm_identifier(krb5_algorithm_identifier *in);
 void free_krb5_kdc_dh_key_info(krb5_kdc_dh_key_info **in);
 void free_krb5_subject_pk_info(krb5_subject_pk_info **in);
+krb5_error_code pkinit_copy_krb5_octet_data(krb5_octet_data *dst, const krb5_octet_data *src);
+
 
 /*
  * Functions in pkinit_profile.c
diff --git a/src/plugins/preauth/pkinit/pkinit_accessor.c b/src/plugins/preauth/pkinit/pkinit_accessor.c
index 2ad9c8d..e954ca3 100644
--- a/src/plugins/preauth/pkinit/pkinit_accessor.c
+++ b/src/plugins/preauth/pkinit/pkinit_accessor.c
@@ -81,7 +81,8 @@ krb5_error_code (*k5int_encode_krb5_authdata_elt)
  * Grab internal function pointers from the krb5int_accessor
  * structure and make them available
  */
-krb5_error_code pkinit_accessor_init(void)
+krb5_error_code
+pkinit_accessor_init(void)
 {
     krb5_error_code retval;
     krb5int_access k5int;
diff --git a/src/plugins/preauth/pkinit/pkinit_clnt.c b/src/plugins/preauth/pkinit/pkinit_clnt.c
index 388dc15..db19141 100644
--- a/src/plugins/preauth/pkinit/pkinit_clnt.c
+++ b/src/plugins/preauth/pkinit/pkinit_clnt.c
@@ -40,10 +40,20 @@
 
 #include "pkinit.h"
 
-/* #define DEBUG */
-/* #define DEBUG_DH */
-
-int longhorn = 0;	/* XXX Talking to a Longhorn server? */
+#ifdef LONGHORN_BETA_COMPAT
+/*
+ * It is anticipated that all the special checks currently
+ * required when talking to a Longhorn server will go away
+ * by the time it is officially released and all references
+ * to the longhorn global can be removed and any code
+ * #ifdef'd with LONGHORN_BETA_COMPAT can be removed.
+ *
+ * Current testing (20070620) is against a patched Beta 3
+ * version of Longhorn.  Most, if not all, problems should
+ * be fixed in SP1 of Longhorn.
+ */
+int longhorn = 0;	/* Talking to a Longhorn server? */
+#endif
 
 krb5_error_code pkinit_client_process
 	(krb5_context context, void *plugin_context, void *request_context,
@@ -105,12 +115,6 @@ krb5_error_code pa_pkinit_parse_rep
 static int pkinit_client_plugin_init(krb5_context context, void **blob);
 static void pkinit_client_plugin_fini(krb5_context context, void *blob);
 
-/*
-static void pkinit_client_profile
-	(krb5_context context, pkinit_context plgctx,
-	 pkinit_req_context reqctx, krb5_kdc_req *request);
-*/
-
 krb5_error_code
 pa_pkinit_gen_req(krb5_context context,
 		  pkinit_context plgctx,
@@ -213,8 +217,18 @@ pa_pkinit_gen_req(krb5_context context,
     return_pa_data[0]->length = out_data->length;
     return_pa_data[0]->contents = (krb5_octet *) out_data->data;
 
+#ifdef LONGHORN_BETA_COMPAT
+    /*
+     * LH Beta 3 requires the extra pa-data, even for RFC requests,
+     * in order to get the Checksum rather than a Nonce in the reply.
+     * This can be removed when LH SP1 is released.
+     */
     if ((return_pa_data[0]->pa_type == KRB5_PADATA_PK_AS_REP_OLD
 	&& reqctx->opts->win2k_require_cksum) || (longhorn == 1)) {
+#else
+    if ((return_pa_data[0]->pa_type == KRB5_PADATA_PK_AS_REP_OLD
+	&& reqctx->opts->win2k_require_cksum)) {
+#endif
 	return_pa_data[1]->pa_type = 132;
 	return_pa_data[1]->length = 0;
 	return_pa_data[1]->contents = NULL;
@@ -302,15 +316,11 @@ pkinit_as_req_create(krb5_context context,
 	    auth_pack->clientPublicValue = info;
 
 	    /* add List of CMS algorithms */
-#if 1
 	    retval = create_krb5_supportedCMSTypes(context, plgctx->cryptoctx,
 			reqctx->cryptoctx, reqctx->idctx, 
 			&auth_pack->supportedCMSTypes);
 	    if (retval)
 		goto cleanup;
-#else
-	    auth_pack->supportedCMSTypes = NULL;
-#endif
 	    break;
 	default:
 	    pkiDebug("as_req: unrecognized pa_type = %d\n",
@@ -322,7 +332,11 @@ pkinit_as_req_create(krb5_context context,
     switch(protocol) {
 	case DH_PROTOCOL:
 	    pkiDebug("as_req: DH key transport algorithm\n");
-	    info->algorithm.algorithm = dh_oid;
+	    retval = pkinit_copy_krb5_octet_data(&info->algorithm.algorithm, &dh_oid);
+	    if (retval) {
+		pkiDebug("failed to copy dh_oid\n");
+		goto cleanup;
+	    }
 
 	    /* create client-side DH keys */
 	    if ((retval = client_create_dh(context, plgctx->cryptoctx,
@@ -433,7 +447,7 @@ pkinit_as_req_create(krb5_context context,
 	    break;
 	case KRB5_PADATA_PK_AS_REQ_OLD:
 #if 0
-/* draft9 KDC doesn't like this draft9 structure of the trusted cas. */
+	    /* W2K3 KDC doesn't like this */
 	    retval = create_krb5_trustedCas(context, plgctx->cryptoctx,
 		reqctx->cryptoctx, reqctx->idctx, 1, &req9->trustedCertifiers);
 	    if (retval)
@@ -662,7 +676,8 @@ out:
 }
 
 /*
- * Parse PA-PK-AS-REP message. Optionally evaluates the message's certificate chain.
+ * Parse PA-PK-AS-REP message. Optionally evaluates the message's
+ * certificate chain.
  * Optionally returns various components.
  */
 krb5_error_code
@@ -804,7 +819,16 @@ pkinit_as_rep_parse(krb5_context context,
 	    if ((retval = k5int_decode_krb5_reply_key_pack(&k5data,
 		    &key_pack)) != 0) {
 		pkiDebug("failed to decode reply_key_pack\n");
+#ifdef LONGHORN_BETA_COMPAT
+    /*
+     * LH Beta 3 requires the extra pa-data, even for RFC requests,
+     * in order to get the Checksum rather than a Nonce in the reply.
+     * This can be removed when LH SP1 is released.
+     */
 		if (pa_type == KRB5_PADATA_PK_AS_REP && longhorn == 0)
+#else
+		if (pa_type == KRB5_PADATA_PK_AS_REP)
+#endif
 		    goto cleanup;
 		else {
 		    if ((retval =
@@ -824,8 +848,10 @@ pkinit_as_rep_parse(krb5_context context,
 		    break;
 		}
 	    }
-	    /* this is hack but windows sends back sha1 checksum
-	     * with checksum type of 14. mit has no 14 checksum type
+	    /*
+	     * This is hack but Windows sends back SHA1 checksum
+	     * with checksum type of 14. There is currently no
+	     * checksum type of 14 defined.
 	     */
 	    if (key_pack->asChecksum.checksum_type == 14)
 		key_pack->asChecksum.checksum_type = CKSUMTYPE_NIST_SHA;
@@ -951,11 +977,13 @@ pkinit_client_profile(krb5_context context,
 	}
 	free(eku_string);
     }
+#ifdef LONGHORN_BETA_COMPAT
     /* Temporarily just set global flag from config file */
     pkinit_libdefault_boolean(context, &request->server->realm,
 			      "pkinit_longhorn",
 			      0,
 			      &longhorn);
+#endif
 
     /* Only process anchors here if they were not specified on command line */
     if (reqctx->idopts->anchors == NULL)
@@ -1168,7 +1196,7 @@ cleanup:
 	free_krb5_typed_data(&typed_data);
 
     if (algId != NULL)
-	free_krb5_algorithm_identifier(&algId);
+	free_krb5_algorithm_identifiers(&algId);
 
     pkiDebug("pkinit_client_tryagain: returning %d (%s)\n",
 	     retval, error_message(retval));
@@ -1296,7 +1324,8 @@ pkinit_init_client_profile(krb5_context context, pkinit_context plgctx)
     return 0;
 }
 
-static int pkinit_client_plugin_init(krb5_context context, void **blob)
+static int
+pkinit_client_plugin_init(krb5_context context, void **blob)
 {
     krb5_error_code retval = ENOMEM;
     struct _pkinit_context *ctx = NULL;
diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
index e8d7ae4..71d8a63 100644
--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
+++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
@@ -36,6 +36,15 @@
 #include <unistd.h>
 #include <dirent.h>
 
+/*
+ * Q: What is this SILLYDECRYPT stuff about?
+ * A: When using the ActivCard Linux pkcs11 library (v2.0.1),
+ *    the decrypt function fails.  By inserting an extra
+ *    function call, which serves nothing but to change the
+ *    stack, we were able to work around the issue.  If the
+ *    ActivCard library is fixed in the future, this
+ *    definition and related code can be removed.
+ */
 #define SILLYDECRYPT
 
 #include "pkinit_crypto_openssl.h"
@@ -994,12 +1003,15 @@ cms_signeddata_create(krb5_context context,
 
 #ifdef DEBUG_ASN1
     if (cms_msg_type == CMS_SIGN_CLIENT) {
-	print_buffer_bin(*signed_data, *signed_data_len, "/tmp/client_pkcs7_signeddata");
+	print_buffer_bin(*signed_data, *signed_data_len,
+			 "/tmp/client_pkcs7_signeddata");
     } else {
 	if (cms_msg_type == CMS_SIGN_SERVER) {
-	    print_buffer_bin(*signed_data, *signed_data_len, "/tmp/kdc_pkcs7_signeddata");
+	    print_buffer_bin(*signed_data, *signed_data_len,
+			     "/tmp/kdc_pkcs7_signeddata");
 	} else {
-	    print_buffer_bin(*signed_data, *signed_data_len, "/tmp/draft9_pkcs7_signeddata");
+	    print_buffer_bin(*signed_data, *signed_data_len,
+			     "/tmp/draft9_pkcs7_signeddata");
 	}
     }
 #endif
@@ -1066,7 +1078,8 @@ cms_signeddata_verify(krb5_context context,
     char buf[DN_BUF_LEN];
 
 #ifdef DEBUG_ASN1
-    print_buffer_bin(signed_data, signed_data_len, "/tmp/client_received_pkcs7_signeddata");
+    print_buffer_bin(signed_data, signed_data_len,
+		     "/tmp/client_received_pkcs7_signeddata");
 #endif
 
     /* Do this early enough to create the shadow OID for pkcs7-data if needed */
@@ -1246,12 +1259,18 @@ cms_signeddata_verify(krb5_context context,
 	if (!OBJ_cmp(p7->d.sign->contents->type, oid)) 
 	    valid_oid = 1;
 	else if (cms_msg_type == CMS_SIGN_DRAFT9) {
-	    /* check to see if longhorn is sending RFC oids for pa-type 15 */
-	    ASN1_OBJECT *client_oid = NULL, *server_oid = NULL;
+	    /*
+	     * Various implementations of the pa-type 15 request use
+	     * different OIDS.  We check that the returned object
+	     * has any of the acceptable OIDs
+	     */
+	    ASN1_OBJECT *client_oid = NULL, *server_oid = NULL, *rsa_oid = NULL;
 	    client_oid = pkinit_pkcs7type2oid(plgctx, CMS_SIGN_CLIENT);
 	    server_oid = pkinit_pkcs7type2oid(plgctx, CMS_SIGN_SERVER);
+	    rsa_oid = pkinit_pkcs7type2oid(plgctx, CMS_ENVEL_SERVER);
 	    if (!OBJ_cmp(p7->d.sign->contents->type, client_oid) ||
-		!OBJ_cmp(p7->d.sign->contents->type, server_oid))
+		!OBJ_cmp(p7->d.sign->contents->type, server_oid) ||
+		!OBJ_cmp(p7->d.sign->contents->type, rsa_oid))
 		valid_oid = 1;
 	}
 
@@ -1503,7 +1522,8 @@ cms_envelopeddata_verify(krb5_context context,
     int msg_type = 0;
 
 #ifdef DEBUG_ASN1
-    print_buffer_bin(enveloped_data, enveloped_data_len, "/tmp/client_envelopeddata");
+    print_buffer_bin(enveloped_data, enveloped_data_len,
+		     "/tmp/client_envelopeddata");
 #endif
     /* decode received PKCS7 message */
     if ((p7 = d2i_PKCS7(NULL, &p, (int)enveloped_data_len)) == NULL) {
@@ -1564,13 +1584,21 @@ cms_envelopeddata_verify(krb5_context context,
 	    retval = KRB5KDC_ERR_PREAUTH_FAILED;
 	    goto cleanup;
     }
-    /* If this is the RFC style, wrap the signed data to make
+    /*
+     * If this is the RFC style, wrap the signed data to make
      * decoding easier in the verify routine.
-     * For win2k-compatible, we don't do anything because it
-     * is already wrapped.  (Except for longhorn which does
-     * things differently...)
+     * For draft9-compatible, we don't do anything because it
+     * is already wrapped.
+     */
+#ifdef LONGHORN_BETA_COMPAT
+    /*
+     * The Longhorn server returns the expected RFC-style data, but
+     * it is missing the sequence tag and length, so it requires
+     * special processing when wrapping.
+     * This will hopefully be fixed before the final release and
+     * this can all be removed.
      */
-    if (msg_type == CMS_ENVEL_SERVER || longhorn) {
+    if (msg_type == CMS_ENVEL_SERVER || longhorn == 1) {
 	retval = wrap_signeddata(tmp_buf, tmp_buf_len,
 				 &tmp_buf2, &tmp_buf2_len, longhorn);
 	if (retval) {
@@ -1584,6 +1612,22 @@ cms_envelopeddata_verify(krb5_context context,
 	vfy_buf = tmp_buf;
 	vfy_buf_len = tmp_buf_len;
     }
+#else
+    if (msg_type == CMS_ENVEL_SERVER) {
+	retval = wrap_signeddata(tmp_buf, tmp_buf_len,
+				 &tmp_buf2, &tmp_buf2_len);
+	if (retval) {
+	    pkiDebug("failed to encode signeddata\n");
+	    goto cleanup;
+	}
+	vfy_buf = tmp_buf2;
+	vfy_buf_len = tmp_buf2_len;
+
+    } else {
+	vfy_buf = tmp_buf;
+	vfy_buf_len = tmp_buf_len;
+    }
+#endif
 
 #ifdef DEBUG_ASN1
     print_buffer_bin(vfy_buf, vfy_buf_len, "/tmp/client_enc_keypack2");
@@ -2936,6 +2980,11 @@ pkinit_pkcs7type2oid(pkinit_plg_crypto_context cryptoctx, int pkcs7_type)
 
 }
 
+#ifdef LONGHORN_BETA_COMPAT
+#if 0
+/*
+ * This is a version that worked with Longhorn Beta 3.
+ */
 static int
 wrap_signeddata(unsigned char *data, unsigned int data_len,
 		unsigned char **out, unsigned int *out_len,
@@ -2946,6 +2995,9 @@ wrap_signeddata(unsigned char *data, unsigned int data_len,
     ASN1_OBJECT *oid = NULL;
     unsigned char *p = NULL;
 
+    pkiDebug("%s: This is the Longhorn version and is_longhorn_server = %d\n",
+	     __FUNCTION__, is_longhorn_server);
+
     /* Get length to wrap the original data with SEQUENCE tag */
     tot_len = orig_len = ASN1_object_size(1, (int)data_len, V_ASN1_SEQUENCE);
 
@@ -2976,6 +3028,98 @@ wrap_signeddata(unsigned char *data, unsigned int data_len,
 
     return 0;
 }
+#else
+/*
+ * This is a version that works with a patched Longhorn KDC.
+ * (Which should match SP1 ??).
+ */
+static int
+wrap_signeddata(unsigned char *data, unsigned int data_len,
+	       unsigned char **out, unsigned int *out_len,
+	       int is_longhorn_server)
+{
+
+    unsigned int oid_len = 0, tot_len = 0, wrap_len = 0, tag_len = 0;
+    ASN1_OBJECT *oid = NULL;
+    unsigned char *p = NULL;
+
+    pkiDebug("%s: This is the Longhorn version and is_longhorn_server = %d\n",
+	     __FUNCTION__, is_longhorn_server);
+
+    /* New longhorn is missing another sequence */
+    if (is_longhorn_server == 1)
+       wrap_len = ASN1_object_size(1, (int)(data_len), V_ASN1_SEQUENCE);
+    else
+       wrap_len = data_len;
+
+    /* Get length to wrap the original data with SEQUENCE tag */
+    tag_len = ASN1_object_size(1, (int)wrap_len, V_ASN1_SEQUENCE);
+
+    /* Always add oid */
+    oid = OBJ_nid2obj(NID_pkcs7_signed);
+    oid_len = i2d_ASN1_OBJECT(oid, NULL);
+    oid_len += tag_len;
+
+    tot_len = ASN1_object_size(1, (int)(oid_len), V_ASN1_SEQUENCE);
+
+    p = *out = (unsigned char *)malloc(tot_len);
+    if (p == NULL)
+       return -1;
+
+    ASN1_put_object(&p, 1, (int)(oid_len),
+		    V_ASN1_SEQUENCE, V_ASN1_UNIVERSAL);
+
+    i2d_ASN1_OBJECT(oid, &p);
+
+    ASN1_put_object(&p, 1, (int)wrap_len, 0, V_ASN1_CONTEXT_SPECIFIC);
+
+    /* Wrap in extra seq tag */
+    if (is_longhorn_server == 1) {
+       ASN1_put_object(&p, 1, (int)data_len, V_ASN1_SEQUENCE, V_ASN1_UNIVERSAL);
+    }
+    memcpy(p, data, data_len);
+
+    *out_len = tot_len;
+
+    return 0;
+}
+
+#endif
+#else
+static int
+wrap_signeddata(unsigned char *data, unsigned int data_len,
+		unsigned char **out, unsigned int *out_len)
+{
+
+    unsigned int orig_len = 0, oid_len = 0, tot_len = 0;
+    ASN1_OBJECT *oid = NULL;
+    unsigned char *p = NULL;
+
+    /* Get length to wrap the original data with SEQUENCE tag */
+    tot_len = orig_len = ASN1_object_size(1, (int)data_len, V_ASN1_SEQUENCE);
+
+    /* Add the signedData OID and adjust lengths */
+    oid = OBJ_nid2obj(NID_pkcs7_signed);
+    oid_len = i2d_ASN1_OBJECT(oid, NULL);
+
+    tot_len = ASN1_object_size(1, (int)(orig_len+oid_len), V_ASN1_SEQUENCE);
+
+    p = *out = (unsigned char *)malloc(tot_len);
+    if (p == NULL) return -1;
+
+    ASN1_put_object(&p, 1, (int)(orig_len+oid_len),
+		    V_ASN1_SEQUENCE, V_ASN1_UNIVERSAL);
+
+    i2d_ASN1_OBJECT(oid, &p);
+
+    ASN1_put_object(&p, 1, (int)data_len, 0, V_ASN1_CONTEXT_SPECIFIC);
+    memcpy(p, data, data_len);
+
+    *out_len = tot_len;
+
+    return 0;
+}
+#endif
 
 static int
 prepare_enc_data(unsigned char *indata,
@@ -4844,7 +4988,9 @@ create_identifiers_from_stack(STACK_OF(X509) *sk,
 	krb5_cas[i]->issuerAndSerialNumber.magic = 0;
 	krb5_cas[i]->issuerAndSerialNumber.data = NULL;
 
+#ifdef LONGHORN_BETA_COMPAT
 if (longhorn == 0) { /* XXX Longhorn doesn't like this */
+#endif
 	is = PKCS7_ISSUER_AND_SERIAL_new();
 	X509_NAME_set(&is->issuer, X509_get_issuer_name(x));
 	M_ASN1_INTEGER_free(is->serial);
@@ -4855,14 +5001,19 @@ if (longhorn == 0) { /* XXX Longhorn doesn't like this */
 	    goto cleanup;
 	i2d_PKCS7_ISSUER_AND_SERIAL(is, &p);
 	krb5_cas[i]->issuerAndSerialNumber.length = len;
+#ifdef LONGHORN_BETA_COMPAT
 }
+#endif
 
 	/* fill-in subjectKeyIdentifier */
 	krb5_cas[i]->subjectKeyIdentifier.length = 0;
 	krb5_cas[i]->subjectKeyIdentifier.magic = 0;
 	krb5_cas[i]->subjectKeyIdentifier.data = NULL;
 
+
+#ifdef LONGHORN_BETA_COMPAT
 if (longhorn == 0) {	/* XXX Longhorn doesn't like this */
+#endif
 	if (X509_get_ext_by_NID(x, NID_subject_key_identifier, -1) >= 0) {
 	    ASN1_OCTET_STRING *ikeyid = NULL;
 
@@ -4878,7 +5029,9 @@ if (longhorn == 0) {	/* XXX Longhorn doesn't like this */
 	    if (ikeyid != NULL)
 		ASN1_OCTET_STRING_free(ikeyid);
 	}
+#ifdef LONGHORN_BETA_COMPAT
 }
+#endif
 	if (is != NULL) {
 	    if (is->issuer != NULL)
 		X509_NAME_free(is->issuer);
@@ -4944,9 +5097,16 @@ create_krb5_supportedCMSTypes(krb5_context context,
 	goto cleanup;
     loids[1] = NULL;
     loids[0] = (krb5_algorithm_identifier *)malloc(sizeof(krb5_algorithm_identifier));
-    if (loids[0] == NULL)
+    if (loids[0] == NULL) {
+	free(loids);
 	goto cleanup;
-    loids[0]->algorithm = des3oid;
+    }
+    retval = pkinit_copy_krb5_octet_data(&loids[0]->algorithm, &des3oid);
+    if (retval) {
+	free(loids[0]);
+	free(loids);
+	goto cleanup;
+    }
     loids[0]->parameters.length = 0;
     loids[0]->parameters.data = NULL;
 
diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.h b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.h
index 7bb2dbc..9c824c8 100644
--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.h
+++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.h
@@ -244,10 +244,16 @@ create_krb5_invalidCertificates(krb5_context context,
 static krb5_error_code
 create_identifiers_from_stack(STACK_OF(X509) *sk,
 			      krb5_external_principal_identifier *** ids);
+#ifdef LONGHORN_BETA_COMPAT
 static int
 wrap_signeddata(unsigned char *data, unsigned int data_len,
 		unsigned char **out, unsigned int *out_len,
 		int is_longhorn_server);
+#else
+static int
+wrap_signeddata(unsigned char *data, unsigned int data_len,
+		unsigned char **out, unsigned int *out_len);
+#endif
 
 /* This handy macro borrowed from crypto/x509v3/v3_purp.c */
 #define ku_reject(x, usage) \
diff --git a/src/plugins/preauth/pkinit/pkinit_lib.c b/src/plugins/preauth/pkinit/pkinit_lib.c
index 9745a83..f49ef5e 100644
--- a/src/plugins/preauth/pkinit/pkinit_lib.c
+++ b/src/plugins/preauth/pkinit/pkinit_lib.c
@@ -44,7 +44,8 @@ const krb5_octet_data
 	dh_oid = { 0, 7, (unsigned char *)"\x2A\x86\x48\xce\x3e\x02\x01" };
 
 
-krb5_error_code pkinit_init_req_opts(pkinit_req_opts **reqopts)
+krb5_error_code
+pkinit_init_req_opts(pkinit_req_opts **reqopts)
 {
     krb5_error_code retval = ENOMEM;
     pkinit_req_opts *opts = NULL;
@@ -68,14 +69,16 @@ krb5_error_code pkinit_init_req_opts(pkinit_req_opts **reqopts)
     return 0;
 }
 
-void pkinit_fini_req_opts(pkinit_req_opts *opts)
+void
+pkinit_fini_req_opts(pkinit_req_opts *opts)
 {
     if (opts != NULL)
 	free(opts);
     return;
 }
 
-krb5_error_code pkinit_init_plg_opts(pkinit_plg_opts **plgopts)
+krb5_error_code
+pkinit_init_plg_opts(pkinit_plg_opts **plgopts)
 {
     krb5_error_code retval = ENOMEM;
     pkinit_plg_opts *opts = NULL;
@@ -98,14 +101,16 @@ krb5_error_code pkinit_init_plg_opts(pkinit_plg_opts **plgopts)
     return 0;
 }
 
-void pkinit_fini_plg_opts(pkinit_plg_opts *opts)
+void
+pkinit_fini_plg_opts(pkinit_plg_opts *opts)
 {
     if (opts != NULL)
 	free(opts);
     return;
 }
 
-void free_krb5_pa_pk_as_req(krb5_pa_pk_as_req **in)
+void
+free_krb5_pa_pk_as_req(krb5_pa_pk_as_req **in)
 {
     if (*in == NULL) return;
     if ((*in)->signedAuthPack.data != NULL)
@@ -117,7 +122,8 @@ void free_krb5_pa_pk_as_req(krb5_pa_pk_as_req **in)
     free(*in);
 }
 
-void free_krb5_pa_pk_as_req_draft9(krb5_pa_pk_as_req_draft9 **in)
+void
+free_krb5_pa_pk_as_req_draft9(krb5_pa_pk_as_req_draft9 **in)
 {
     if (*in == NULL) return;
     if ((*in)->signedAuthPack.data != NULL)
@@ -131,7 +137,8 @@ void free_krb5_pa_pk_as_req_draft9(krb5_pa_pk_as_req_draft9 **in)
     free(*in);
 }
 
-void free_krb5_reply_key_pack(krb5_reply_key_pack **in)
+void
+free_krb5_reply_key_pack(krb5_reply_key_pack **in)
 {
     if (*in == NULL) return;
     if ((*in)->replyKey.contents != NULL)
@@ -141,7 +148,8 @@ void free_krb5_reply_key_pack(krb5_reply_key_pack **in)
     free(*in);
 }
 
-void free_krb5_reply_key_pack_draft9(krb5_reply_key_pack_draft9 **in)
+void
+free_krb5_reply_key_pack_draft9(krb5_reply_key_pack_draft9 **in)
 {
     if (*in == NULL) return;
     if ((*in)->replyKey.contents != NULL)
@@ -149,15 +157,13 @@ void free_krb5_reply_key_pack_draft9(krb5_reply_key_pack_draft9 **in)
     free(*in);
 }
 
-void free_krb5_auth_pack(krb5_auth_pack **in)
+void
+free_krb5_auth_pack(krb5_auth_pack **in)
 {
-    int i = 0;
     if ((*in) == NULL) return;
     if ((*in)->clientPublicValue != NULL) {
-    /* not freeing clientPublicValue->algorithm.algorithm.data because
-     * client has that as static memory but server allocates it therefore
-     * the server will free it outside of this function
-     */
+	if ((*in)->clientPublicValue->algorithm.algorithm.data != NULL)
+	    free((*in)->clientPublicValue->algorithm.algorithm.data);
 	if ((*in)->clientPublicValue->algorithm.parameters.data != NULL)
 	    free((*in)->clientPublicValue->algorithm.parameters.data);
 	if ((*in)->clientPublicValue->subjectPublicKey.data != NULL)
@@ -166,17 +172,13 @@ void free_krb5_auth_pack(krb5_auth_pack **in)
     }
     if ((*in)->pkAuthenticator.paChecksum.contents != NULL)
 	free((*in)->pkAuthenticator.paChecksum.contents);
-    if ((*in)->supportedCMSTypes != NULL) {
-	while ((*in)->supportedCMSTypes[i] != NULL) {
-	    free((*in)->supportedCMSTypes[i]);
-	    i++;
-	}
-	free((*in)->supportedCMSTypes);
-    }
+    if ((*in)->supportedCMSTypes != NULL)
+	free_krb5_algorithm_identifiers(&((*in)->supportedCMSTypes));
     free(*in);
 }
 
-void free_krb5_auth_pack_draft9(krb5_context context,
+void
+free_krb5_auth_pack_draft9(krb5_context context,
 				krb5_auth_pack_draft9 **in)
 {
     if ((*in) == NULL) return;
@@ -184,7 +186,8 @@ void free_krb5_auth_pack_draft9(krb5_context context,
     free(*in);
 }
 
-void free_krb5_pa_pk_as_rep(krb5_pa_pk_as_rep **in)
+void
+free_krb5_pa_pk_as_rep(krb5_pa_pk_as_rep **in)
 {
     if (*in == NULL) return;
     switch ((*in)->choice) {
@@ -202,7 +205,8 @@ void free_krb5_pa_pk_as_rep(krb5_pa_pk_as_rep **in)
     free(*in);
 }
 
-void free_krb5_pa_pk_as_rep_draft9(krb5_pa_pk_as_rep_draft9 **in)
+void
+free_krb5_pa_pk_as_rep_draft9(krb5_pa_pk_as_rep_draft9 **in)
 {
     if (*in == NULL) return;
     if ((*in)->u.encKeyPack.data != NULL)
@@ -210,7 +214,8 @@ void free_krb5_pa_pk_as_rep_draft9(krb5_pa_pk_as_rep_draft9 **in)
     free(*in);
 }
 
-void free_krb5_external_principal_identifier(krb5_external_principal_identifier ***in)
+void
+free_krb5_external_principal_identifier(krb5_external_principal_identifier ***in)
 {
     int i = 0;
     if (*in == NULL) return;
@@ -227,7 +232,8 @@ void free_krb5_external_principal_identifier(krb5_external_principal_identifier
     free(*in);
 }
 
-void free_krb5_trusted_ca(krb5_trusted_ca ***in)
+void
+free_krb5_trusted_ca(krb5_trusted_ca ***in)
 {
     int i = 0;
     if (*in == NULL) return;
@@ -252,7 +258,8 @@ void free_krb5_trusted_ca(krb5_trusted_ca ***in)
     free(*in);
 }
 
-void free_krb5_typed_data(krb5_typed_data ***in)
+void
+free_krb5_typed_data(krb5_typed_data ***in)
 {
     int i = 0;
     if (*in == NULL) return;
@@ -265,17 +272,26 @@ void free_krb5_typed_data(krb5_typed_data ***in)
     free(*in);
 }
 
-void free_krb5_algorithm_identifier(krb5_algorithm_identifier ***in)
+void
+free_krb5_algorithm_identifier(krb5_algorithm_identifier *in)
 {
-    int i = 0;
-    if (*in == NULL) return;
-    while ((*in)[i] != NULL) {
-	if ((*in)[i]->algorithm.data != NULL)
-	    free((*in)[i]->algorithm.data);
-	if ((*in)[i]->parameters.data != NULL)
-	    free((*in)[i]->parameters.data);
-	free((*in)[i]);
-	i++;
+    if (in == NULL)
+	return;
+    if (in->algorithm.data != NULL)
+	free(in->algorithm.data);
+    if (in->parameters.data != NULL)
+	free(in->parameters.data);
+    free(in);
+}
+
+void
+free_krb5_algorithm_identifiers(krb5_algorithm_identifier ***in)
+{
+    int i;
+    if (in == NULL || *in == NULL)
+	return;
+    for (i = 0; (*in)[i] != NULL; i++) {
+	free_krb5_algorithm_identifier((*in)[i]);
     }
     free(*in);
 }
@@ -300,7 +316,8 @@ free_krb5_kdc_dh_key_info(krb5_kdc_dh_key_info **in)
     free(*in);
 }
 
-void init_krb5_pa_pk_as_req(krb5_pa_pk_as_req **in)
+void
+init_krb5_pa_pk_as_req(krb5_pa_pk_as_req **in)
 {
     (*in) = malloc(sizeof(krb5_pa_pk_as_req));
     if ((*in) == NULL) return;
@@ -311,7 +328,8 @@ void init_krb5_pa_pk_as_req(krb5_pa_pk_as_req **in)
     (*in)->kdcPkId.length = 0;
 }
 
-void init_krb5_pa_pk_as_req_draft9(krb5_pa_pk_as_req_draft9 **in)
+void
+init_krb5_pa_pk_as_req_draft9(krb5_pa_pk_as_req_draft9 **in)
 {
     (*in) = malloc(sizeof(krb5_pa_pk_as_req_draft9));
     if ((*in) == NULL) return;
@@ -324,7 +342,8 @@ void init_krb5_pa_pk_as_req_draft9(krb5_pa_pk_as_req_draft9 **in)
     (*in)->encryptionCert.length = 0;
 }
 
-void init_krb5_reply_key_pack(krb5_reply_key_pack **in)
+void
+init_krb5_reply_key_pack(krb5_reply_key_pack **in)
 {
     (*in) = malloc(sizeof(krb5_reply_key_pack));
     if ((*in) == NULL) return;
@@ -334,7 +353,8 @@ void init_krb5_reply_key_pack(krb5_reply_key_pack **in)
     (*in)->asChecksum.length = 0;
 }
 
-void init_krb5_reply_key_pack_draft9(krb5_reply_key_pack_draft9 **in)
+void
+init_krb5_reply_key_pack_draft9(krb5_reply_key_pack_draft9 **in)
 {
     (*in) = malloc(sizeof(krb5_reply_key_pack_draft9));
     if ((*in) == NULL) return;
@@ -342,7 +362,8 @@ void init_krb5_reply_key_pack_draft9(krb5_reply_key_pack_draft9 **in)
     (*in)->replyKey.length = 0;
 }
 
-void init_krb5_auth_pack(krb5_auth_pack **in)
+void
+init_krb5_auth_pack(krb5_auth_pack **in)
 {
     (*in) = malloc(sizeof(krb5_auth_pack));
     if ((*in) == NULL) return;
@@ -353,14 +374,16 @@ void init_krb5_auth_pack(krb5_auth_pack **in)
     (*in)->pkAuthenticator.paChecksum.contents = NULL;
 }
 
-void init_krb5_auth_pack_draft9(krb5_auth_pack_draft9 **in)
+void
+init_krb5_auth_pack_draft9(krb5_auth_pack_draft9 **in)
 {
     (*in) = malloc(sizeof(krb5_auth_pack_draft9));
     if ((*in) == NULL) return;
     (*in)->clientPublicValue = NULL;
 }
 
-void init_krb5_pa_pk_as_rep(krb5_pa_pk_as_rep **in)
+void
+init_krb5_pa_pk_as_rep(krb5_pa_pk_as_rep **in)
 {
     (*in) = malloc(sizeof(krb5_pa_pk_as_rep));
     if ((*in) == NULL) return;
@@ -372,7 +395,8 @@ void init_krb5_pa_pk_as_rep(krb5_pa_pk_as_rep **in)
     (*in)->u.encKeyPack.data = NULL;
 }
 
-void init_krb5_pa_pk_as_rep_draft9(krb5_pa_pk_as_rep_draft9 **in)
+void
+init_krb5_pa_pk_as_rep_draft9(krb5_pa_pk_as_rep_draft9 **in)
 {
     (*in) = malloc(sizeof(krb5_pa_pk_as_rep_draft9));
     if ((*in) == NULL) return;
@@ -382,7 +406,8 @@ void init_krb5_pa_pk_as_rep_draft9(krb5_pa_pk_as_rep_draft9 **in)
     (*in)->u.encKeyPack.data = NULL;
 }
 
-void init_krb5_typed_data(krb5_typed_data **in)
+void
+init_krb5_typed_data(krb5_typed_data **in)
 {
     (*in) = malloc(sizeof(krb5_typed_data));
     if ((*in) == NULL) return;
@@ -402,6 +427,24 @@ init_krb5_subject_pk_info(krb5_subject_pk_info **in)
     (*in)->subjectPublicKey.length = 0;
 }
 
+krb5_error_code
+pkinit_copy_krb5_octet_data(krb5_octet_data *dst, const krb5_octet_data *src)
+{
+    if (dst == NULL || src == NULL)
+	return EINVAL;
+    if (src->data == NULL) {
+	dst->data = NULL;
+	dst->length = 0;
+	return 0;
+    }
+    dst->data = malloc(src->length);
+    if (dst->data == NULL)
+	return ENOMEM;
+    memcpy(dst->data, src->data, src->length);
+    dst->length = src->length;
+    return 0;
+}
+
 /* debugging functions */
 void
 print_buffer(unsigned char *buf, unsigned int len)
diff --git a/src/plugins/preauth/pkinit/pkinit_matching.c b/src/plugins/preauth/pkinit/pkinit_matching.c
index f5601ab..b790c38 100644
--- a/src/plugins/preauth/pkinit/pkinit_matching.c
+++ b/src/plugins/preauth/pkinit/pkinit_matching.c
@@ -766,7 +766,8 @@ pkinit_cert_matching(krb5_context context,
 	retval = parse_rule_set(context, rules[x], &rs);
 	if (retval) {
 	    if (retval == EINVAL) {
-		fprintf(stderr, "Ignoring invalid rule pkinit_cert_match = '%s'\n", rules[x]); 
+		pkiDebug("%s: Ignoring invalid rule pkinit_cert_match = '%s'\n",
+			 __FUNCTION__, rules[x]); 
 		continue;
 	    }
 	    goto cleanup;
diff --git a/src/plugins/preauth/pkinit/pkinit_srv.c b/src/plugins/preauth/pkinit/pkinit_srv.c
index 908a37d..7b17a7b 100644
--- a/src/plugins/preauth/pkinit/pkinit_srv.c
+++ b/src/plugins/preauth/pkinit/pkinit_srv.c
@@ -330,7 +330,7 @@ pkinit_server_verify_padata(krb5_context context,
 {
     krb5_error_code retval = 0;	
     krb5_octet_data authp_data = {0, 0, NULL}, krb5_authz = {0, 0, NULL};
-    krb5_octet_data *encoded_pkinit_authz_data = NULL;
+    krb5_data *encoded_pkinit_authz_data = NULL;
     krb5_pa_pk_as_req *reqp = NULL;
     krb5_pa_pk_as_req_draft9 *reqp9 = NULL;
     krb5_auth_pack *auth_pack = NULL;
@@ -557,7 +557,6 @@ pkinit_server_verify_padata(krb5_context context,
     /* return authorization data to be included in the ticket */
     switch ((int)data->pa_type) {
 	case KRB5_PADATA_PK_AS_REQ:
-#if 1
 	    my_authz_data = malloc(2 * sizeof(*my_authz_data));
 	    if (my_authz_data == NULL) {
 		retval = ENOMEM;
@@ -594,7 +593,9 @@ pkinit_server_verify_padata(krb5_context context,
 	    retval = k5int_encode_krb5_authdata_elt(pkinit_authz_data, 
 			    &encoded_pkinit_authz_data);
 #ifdef DEBUG_ASN1
-	    print_buffer_bin(encoded_pkinit_authz_data->data, encoded_pkinit_authz_data->length, "/tmp/kdc_pkinit_authz_data");
+	    print_buffer_bin((unsigned char *)encoded_pkinit_authz_data->data,
+			     encoded_pkinit_authz_data->length,
+			     "/tmp/kdc_pkinit_authz_data");
 #endif
 	    free(pkinit_authz_data);
 	    if (retval) {
@@ -604,14 +605,14 @@ pkinit_server_verify_padata(krb5_context context,
 		goto cleanup;
 	    }
 
-	    my_authz_data[0]->contents = encoded_pkinit_authz_data->data;
+	    my_authz_data[0]->contents =
+			    (krb5_octet *) encoded_pkinit_authz_data->data;
 	    my_authz_data[0]->length = encoded_pkinit_authz_data->length;
 	    *authz_data = my_authz_data;
 	    pkiDebug("Returning %d bytes of authorization data\n", 
 		     krb5_authz.length);
 	    encoded_pkinit_authz_data->data = NULL; /* Don't free during cleanup*/
 	    free(encoded_pkinit_authz_data);
-#endif
 	    break;
 	default: 
 	    *authz_data = NULL;
@@ -649,11 +650,8 @@ pkinit_server_verify_padata(krb5_context context,
 	free(krb5_authz.data);
     if (reqctx != NULL)
 	pkinit_fini_kdc_req_context(context, reqctx);
-    if (auth_pack != NULL) {
-	if (auth_pack->clientPublicValue->algorithm.algorithm.data != NULL)
-	    free(auth_pack->clientPublicValue->algorithm.algorithm.data);
+    if (auth_pack != NULL)
 	free_krb5_auth_pack(&auth_pack);
-    }
     if (auth_pack9 != NULL)
 	free_krb5_auth_pack_draft9(context, &auth_pack9);
 
@@ -959,7 +957,8 @@ pkinit_server_return_padata(krb5_context context,
 		break;
 	}
 	if (retval) {
-	    pkiDebug("failed to create pkcs7 enveloped data: %s\n", error_message(retval));
+	    pkiDebug("failed to create pkcs7 enveloped data: %s\n",
+		     error_message(retval));
 	    goto cleanup;
 	}
 #ifdef DEBUG_ASN1
@@ -1339,8 +1338,8 @@ pkinit_server_plugin_fini(krb5_context context, void *blob)
     free(realm_contexts);
 }
 
-static krb5_error_code pkinit_init_kdc_req_context(krb5_context context,
-						   void **ctx)
+static krb5_error_code
+pkinit_init_kdc_req_context(krb5_context context, void **ctx)
 {
     krb5_error_code retval = ENOMEM;
     pkinit_kdc_req_context reqctx = NULL;
@@ -1367,7 +1366,8 @@ cleanup:
     return retval;
 }
 
-static void  pkinit_fini_kdc_req_context(krb5_context context, void *ctx)
+static void
+pkinit_fini_kdc_req_context(krb5_context context, void *ctx)
 {
     pkinit_kdc_req_context reqctx = (pkinit_kdc_req_context)ctx;
 
@@ -1378,12 +1378,8 @@ static void  pkinit_fini_kdc_req_context(krb5_context context, void *ctx)
     pkiDebug("%s: freeing   reqctx at %p\n", __FUNCTION__, reqctx);
 
     pkinit_fini_req_crypto(reqctx->cryptoctx);
-    if (reqctx->rcv_auth_pack != NULL) {
-	if (reqctx->rcv_auth_pack->clientPublicValue != NULL &&
-	    reqctx->rcv_auth_pack->clientPublicValue->algorithm.algorithm.data != NULL)
-		free(reqctx->rcv_auth_pack->clientPublicValue->algorithm.algorithm.data);
+    if (reqctx->rcv_auth_pack != NULL)
 	free_krb5_auth_pack(&reqctx->rcv_auth_pack);
-    }
     if (reqctx->rcv_auth_pack9 != NULL)
 	free_krb5_auth_pack_draft9(context, &reqctx->rcv_auth_pack9);