eap channel bindings fixes

[Kevin Wasserman]

The attached patches address several bugs in eap channel bindings:

Typos in calls to fr_cursor_init.
Use of free() on talloc'ed ptr.
Typo causing mis-computed buffer size.
Use of wrong type in talloc_array_length.

Kevin Wasserman
Painless Security, LLC
-------------- next part --------------
>From 4fd27ea2f91cdb1e5c44c76d8a3b5d3783b33227 Mon Sep 17 00:00:00 2001
From: Kevin Wasserman <kevin.wasserman at painless-security.com>
Date: Sat, 28 Jun 2014 05:22:25 -0400
Subject: [PATCH 1/4] Fix cursor initialization bugs in eap_chbind_vp2packet

---
 src/modules/rlm_eap/libeap/eap_chbind.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/modules/rlm_eap/libeap/eap_chbind.c b/src/modules/rlm_eap/libeap/eap_chbind.c
index 2304554..a99ebf5 100644
--- a/src/modules/rlm_eap/libeap/eap_chbind.c
+++ b/src/modules/rlm_eap/libeap/eap_chbind.c
@@ -243,7 +243,7 @@ chbind_packet_t *eap_chbind_vp2packet(TALLOC_CTX *ctx, VALUE_PAIR *vps)
 	 *	Compute the total length of the channel binding data.
 	 */
 	length = 0;
-	for (vp =fr_cursor_init(&cursor, first);
+	for (vp =fr_cursor_init(&cursor, &first);
 	     vp != NULL;
 	     vp = fr_cursor_next_by_num(&cursor, PW_UKERNA_CHBIND, VENDORPEC_UKERNA, TAG_ANY)) {
 		length += vp->length;
@@ -264,7 +264,7 @@ chbind_packet_t *eap_chbind_vp2packet(TALLOC_CTX *ctx, VALUE_PAIR *vps)
 	 *	Copy the data over to our packet.
 	 */
 	packet = (chbind_packet_t *) ptr;
-	for (vp = fr_cursor_init(&cursor, first);
+	for (vp = fr_cursor_init(&cursor, &first);
 	     vp != NULL;
 	     vp = fr_cursor_next_by_num(&cursor, PW_UKERNA_CHBIND, VENDORPEC_UKERNA, TAG_ANY)) {
 		memcpy(ptr, vp->vp_octets, vp->length);
-- 
1.7.10.4

-------------- next part --------------
>From 7d2b04cdbcfb5c3653adac443ba25358aabea814 Mon Sep 17 00:00:00 2001
From: Kevin Wasserman <kevin.wasserman at painless-security.com>
Date: Mon, 30 Jun 2014 11:41:32 -0400
Subject: [PATCH 3/4] Don't call free on talloc'ed channel bindings packet

---
 src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c b/src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c
index a3a2e4a..a4d3a57 100644
--- a/src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c
+++ b/src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c
@@ -1196,9 +1196,6 @@ int eapttls_process(eap_handler_t *handler, tls_session_t *tls_session)
 		}
 		chbind_code = chbind_process(request, req);
 
-		/* free the chbind packet; we're done with it */
-		free(chbind);
-
 		/* encapsulate response here */
 		if (req->response) {
 			RDEBUG("sending chbind response");
-- 
1.7.10.4

-------------- next part --------------
>From 46cefce284e35d714655b2fdc9a273a5434fabad Mon Sep 17 00:00:00 2001
From: Kevin Wasserman <krwasserman at painless-security.com>
Date: Wed, 2 Jul 2014 07:56:39 -0400
Subject: [PATCH 4/4] Channel bindings fixes

-fix size calculation
-skip unwanted attrs when copying
-add safety check to copy code in case size is wrong
-add cast to get correct result from talloc_array_length()
---
 src/modules/rlm_eap/libeap/eap_chbind.c |   15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/modules/rlm_eap/libeap/eap_chbind.c b/src/modules/rlm_eap/libeap/eap_chbind.c
index a99ebf5..802182b 100644
--- a/src/modules/rlm_eap/libeap/eap_chbind.c
+++ b/src/modules/rlm_eap/libeap/eap_chbind.c
@@ -45,7 +45,7 @@ static bool chbind_build_response(REQUEST *request, CHBIND_REQ *chbind)
 		if (vp->da->flags.encrypt != FLAG_ENCRYPT_NONE) continue;
 		if (!vp->da->vendor && (vp->da->attr == PW_MESSAGE_AUTHENTICATOR)) continue;
 
-		total = 2 + vp->length;
+		total += 2 + vp->length;
 	}
 
 	/*
@@ -88,8 +88,15 @@ static bool chbind_build_response(REQUEST *request, CHBIND_REQ *chbind)
 	for (vp = fr_cursor_init(&cursor, &request->reply->vps);
 	     vp != NULL;
 	     vp = fr_cursor_next(&cursor)) {
-		length = rad_vp2attr(NULL, NULL, NULL, &vp, ptr, end - ptr);
-		ptr += length;
+		/*
+		 *	Skip things which shouldn't be in channel bindings.
+		 */
+		if (vp->da->flags.encrypt != FLAG_ENCRYPT_NONE) continue;
+		if (!vp->da->vendor && (vp->da->attr == PW_MESSAGE_AUTHENTICATOR)) continue;
+		if (ptr < end) {
+			length = rad_vp2attr(NULL, NULL, NULL, &vp, ptr, end - ptr);
+			ptr += length;
+		}
 	}
 
 	return true;
@@ -282,7 +289,7 @@ VALUE_PAIR *eap_chbind_packet2vp(REQUEST *request, const chbind_packet_t *packet
 
 	vp = paircreate(request->packet, PW_UKERNA_CHBIND, VENDORPEC_UKERNA);
 	if (!vp) return NULL;
-	pairmemcpy(vp, (const uint8_t *) packet, talloc_array_length(packet));
+	pairmemcpy(vp, (const uint8_t *) packet, talloc_array_length((uint8_t *)packet));
 
 	return vp;
 }
-- 
1.7.10.4