Implemented a tls_reader class to simplify TLS data parsing

src/charon/plugins/eap_tls/Makefile.am

@@ -11,6 +11,7 @@ libstrongswan_eap_tls_la_SOURCES = eap_tls_plugin.h eap_tls_plugin.c \
 	tls/tls_compression.h tls/tls_compression.c \
 	tls/tls_fragmentation.h tls/tls_fragmentation.c \
 	tls/tls_crypto.h tls/tls_crypto.c \
+	tls/tls_reader.h tls/tls_reader.c \
 	tls/tls_peer.h tls/tls_peer.c \
 	tls/tls_server.h tls/tls_server.c \
 	tls/tls_handshake.h

src/charon/plugins/eap_tls/tls/tls_fragmentation.c

@@ -15,6 +15,8 @@
 
 #include "tls_fragmentation.h"
 
+#include "tls_reader.h"
+
 #include <daemon.h>
 
 typedef struct private_tls_fragmentation_t private_tls_fragmentation_t;
@@ -73,15 +75,17 @@ typedef union  {
  * Process TLS handshake protocol data
  */
 static status_t process_handshake(private_tls_fragmentation_t *this,
-								  chunk_t data)
+								  tls_reader_t *reader)
 {
-	while (data.len)
+	while (reader->remaining(reader))
 	{
+		tls_reader_t *msg;
+		u_int8_t type;
 		u_int32_t len;
 		status_t status;
-		tls_handshake_header_t *hdr;
+		chunk_t data;
 
-		if (data.len == 0 || data.len > MAX_TLS_FRAGMENT_LEN)
+		if (reader->remaining(reader) > MAX_TLS_FRAGMENT_LEN)
 		{
 			DBG1(DBG_IKE, "TLS fragment has invalid length");
 			return FAILED;
@@ -89,33 +93,38 @@ static status_t process_handshake(private_tls_fragmentation_t *this,
 
 		if (this->input.len == 0)
 		{	/* new handshake message */
-			if (data.len < sizeof(tls_handshake_header_t))
+			if (!reader->read_uint8(reader, &type) ||
+				!reader->read_uint24(reader, &len))
 			{
-				DBG1(DBG_IKE, "initial TLS fragment too short %B", &data);
 				return FAILED;
 			}
-			hdr = (tls_handshake_header_t*)data.ptr;
+			this->type = type;
-			len = untoh32(&hdr->length) & 0x00FFFFFF;
-			this->type = hdr->type;
 			if (len > MAX_TLS_HANDSHAKE_LEN)
 			{
 				DBG1(DBG_IKE, "TLS handshake message exceeds maximum length");
 				return FAILED;
 			}
-			this->input = len ? chunk_alloc(len) : chunk_empty;
+			chunk_free(&this->input);
 			this->inpos = 0;
-			data = chunk_skip(data, sizeof(tls_handshake_header_t));
+			if (len)
+			{
+				this->input = chunk_alloc(len);
+			}
 		}
 
-		len = min(this->input.len - this->inpos, data.len);
+		len = min(this->input.len - this->inpos, reader->remaining(reader));
+		if (!reader->read_data(reader, len, &data))
+		{
+			return FAILED;
+		}
 		memcpy(this->input.ptr + this->inpos, data.ptr, len);
 		this->inpos += len;
-		data = chunk_skip(data, len);
 
 		if (this->input.len == this->inpos)
 		{	/* message completely defragmented, process */
-			status = this->handshake->process(this->handshake,
+			msg = tls_reader_create(this->input);
-											  this->type, this->input);
+			status = this->handshake->process(this->handshake, this->type, msg);
+			msg->destroy(msg);
 			chunk_free(&this->input);
 			if (status != NEED_MORE)
 			{
@@ -129,23 +138,34 @@ static status_t process_handshake(private_tls_fragmentation_t *this,
 METHOD(tls_fragmentation_t, process, status_t,
 	private_tls_fragmentation_t *this, tls_content_type_t type, chunk_t data)
 {
+	tls_reader_t *reader;
+	status_t status;
+
+	reader = tls_reader_create(data);
 	switch (type)
 	{
 		case TLS_CHANGE_CIPHER_SPEC:
 			/* TODO: handle ChangeCipherSpec */
-			return FAILED;
+			status = FAILED;
+			break;
 		case TLS_ALERT:
 			/* TODO: handle Alert */
-			return FAILED;
+			status = FAILED;
+			break;
 		case TLS_HANDSHAKE:
-			return process_handshake(this, data);
+			status = process_handshake(this, reader);
+			break;
 		case TLS_APPLICATION_DATA:
 			/* skip application data */
-			return NEED_MORE;
+			status = NEED_MORE;
+			break;
 		default:
 			DBG1(DBG_IKE, "received unknown TLS content type %d, ignored", type);
-			return NEED_MORE;
+			status = NEED_MORE;
+			break;
 	}
+	reader->destroy(reader);
+	return status;
 }
 
 METHOD(tls_fragmentation_t, build, status_t,

src/charon/plugins/eap_tls/tls/tls_handshake.h

@@ -24,6 +24,7 @@
 typedef struct tls_handshake_t tls_handshake_t;
 
 #include "tls.h"
+#include "tls_reader.h"
 
 /**
  * TLS handshake state machine interface.
@@ -34,14 +35,14 @@ struct tls_handshake_t {
 	 * Process received TLS handshake message.
 	 *
 	 * @param type		TLS handshake message type
-	 * @param data		TLS handshake data
+	 * @param reader	TLS data buffer
 	 * @return
 	 *					- SUCCESS if handshake complete
 	 *					- FAILED if handshake failed
 	 *					- NEED_MORE if another invocation of process/build needed
 	 */
 	status_t (*process)(tls_handshake_t *this,
-						tls_handshake_type_t type, chunk_t data);
+						tls_handshake_type_t type, tls_reader_t *reader);
 
 	/**
 	 * Build TLS handshake messages to send out.

src/charon/plugins/eap_tls/tls/tls_peer.c

@@ -56,82 +56,28 @@ struct private_tls_peer_t {
 /**
  * Process a server hello message
  */
-static status_t process_server_hello(private_tls_peer_t *this, chunk_t data)
+static status_t process_server_hello(private_tls_peer_t *this,
+									 tls_reader_t *reader)
 {
-	if (data.len >= 38)
+	u_int8_t compression;
+	u_int16_t version, cipher;
+	u_int32_t gmt;
+	chunk_t random, session, ext = chunk_empty;
+
+	if (!reader->read_uint16(reader, &version) ||
+		!reader->read_uint32(reader, &gmt) ||
+		!reader->read_data(reader, 28, &random) ||
+		!reader->read_data8(reader, &session) ||
+		!reader->read_uint16(reader, &cipher) ||
+		!reader->read_uint8(reader, &compression) ||
+		(reader->remaining(reader) && !reader->read_data16(reader, &ext)))
 	{
-		tls_version_t version;
+		DBG1(DBG_IKE, "received invalid ServerHello");
-
+		return FAILED;
-		struct __attribute__((packed)) {
-			u_int16_t version;
-			struct __attribute__((packed)) {
-				u_int32_t gmt;
-				u_int8_t bytes[28];
-			} random;
-			struct __attribute__((packed)) {
-				u_int8_t len;
-				/* points to len */
-				u_int8_t id[data.ptr[34]];
-			} session;
-			u_int16_t cipher;
-			u_int8_t compression;
-			char extensions[];
-		} *hello = (void*)data.ptr;
-
-		if (sizeof(*hello) > data.len)
-		{
-			DBG1(DBG_IKE, "received invalid ServerHello");
-			return FAILED;
-		}
-
-		version = untoh16(&hello->version);
-		if (version < this->tls->get_version(this->tls))
-		{
-			this->tls->set_version(this->tls, version);
-		}
-		return NEED_MORE;
 	}
-	DBG1(DBG_IKE, "server hello has %d bytes", data.len);
+	if (version < this->tls->get_version(this->tls))
-	return FAILED;
-}
-
-/**
- * Process a Certificate message
- */
-static status_t process_certificate(private_tls_peer_t *this, chunk_t data)
-{
-	if (data.len > 3)
 	{
-		u_int32_t total;
+		this->tls->set_version(this->tls, version);
-
-		total = untoh32(data.ptr) >> 8;
-		data = chunk_skip(data, 3);
-		if (total != data.len)
-		{
-			DBG1(DBG_IKE, "certificate chain length invalid");
-			return FAILED;
-		}
-		while (data.len > 3)
-		{
-			certificate_t *cert;
-			u_int32_t len;
-
-			len = untoh32(data.ptr) >> 8;
-			data = chunk_skip(data, 3);
-			if (len > data.len)
-			{
-				DBG1(DBG_IKE, "certificate length invalid");
-				return FAILED;
-			}
-			cert = lib->creds->create(lib->creds, CRED_CERTIFICATE, CERT_X509,
-					BUILD_BLOB_ASN1_DER, chunk_create(data.ptr, len), BUILD_END);
-			if (cert)
-			{
-				DBG1(DBG_IKE, "got certificate: %Y", cert->get_subject(cert));
-				cert->destroy(cert);
-			}
-			data = chunk_skip(data, len);
-		}
 	}
 	return NEED_MORE;
 }
@@ -139,65 +85,79 @@ static status_t process_certificate(private_tls_peer_t *this, chunk_t data)
 /**
  * Process a Certificate message
  */
-static status_t process_certreq(private_tls_peer_t *this, chunk_t data)
+static status_t process_certificate(private_tls_peer_t *this,
+									tls_reader_t *reader)
 {
-	struct __attribute__((packed)) {
+	certificate_t *cert;
-		u_int8_t len;
+	tls_reader_t *certs;
-		u_int8_t types[];
+	chunk_t data;
-	} *certificate;
+
-	struct __attribute__((packed)) {
+	if (!reader->read_data24(reader, &data))
-		u_int16_t len;
+	{
-		struct __attribute__((packed)) {
+		return FAILED;
-			u_int8_t hash;
+	}
-			u_int8_t sig;
+	certs = tls_reader_create(data);
-		} types[];
+	while (certs->remaining(certs))
-	} *alg;
+	{
-	u_int16_t len;
+		if (!certs->read_data24(certs, &data))
+		{
+			certs->destroy(certs);
+			return FAILED;
+		}
+		cert = lib->creds->create(lib->creds, CRED_CERTIFICATE, CERT_X509,
+								   BUILD_BLOB_ASN1_DER, data, BUILD_END);
+		if (cert)
+		{
+			DBG1(DBG_IKE, "got certificate: %Y", cert->get_subject(cert));
+			cert->destroy(cert);
+		}
+	}
+	certs->destroy(certs);
+	return NEED_MORE;
+}
+
+/**
+ * Process a Certificate message
+ */
+static status_t process_certreq(private_tls_peer_t *this, tls_reader_t *reader)
+{
+	chunk_t types, hashsig, data;
+	tls_reader_t *authorities;
 	identification_t *id;
 
-	certificate = (void*)data.ptr;
+	if (!reader->read_data8(reader, &types))
-	data = chunk_skip(data, 1);
-	if (!data.len || certificate->len > data.len)
 	{
 		return FAILED;
 	}
-	data = chunk_skip(data, certificate->len);
-
 	if (this->tls->get_version(this->tls) >= TLS_1_2)
 	{
-		alg = (void*)data.ptr;
+		if (!reader->read_data16(reader, &hashsig))
-		data = chunk_skip(data, 2);
-		if (!data.len || untoh16(&alg->len) > data.len)
 		{
 			return FAILED;
 		}
-		data = chunk_skip(data, untoh16(&alg->len));
 	}
-	if (data.len < 2 || untoh16(data.ptr) != data.len - 2)
+	if (!reader->read_data16(reader, &data))
 	{
 		return FAILED;
 	}
-	data = chunk_skip(data, 2);
+	authorities = tls_reader_create(data);
-
+	while (authorities->remaining(authorities))
-	while (data.len >= 2)
 	{
-		len = untoh16(data.ptr);
+		if (!authorities->read_data16(authorities, &data))
-		data = chunk_skip(data, 2);
-		if (len > data.len)
 		{
+			authorities->destroy(authorities);
 			return FAILED;
 		}
-		id = identification_create_from_encoding(ID_DER_ASN1_DN,
+		id = identification_create_from_encoding(ID_DER_ASN1_DN, data);
-												 chunk_create(data.ptr, len));
 		DBG1(DBG_IKE, "received certificate request for %Y", id);
 		id->destroy(id);
-		data = chunk_skip(data, len);
 	}
+	authorities->destroy(authorities);
 	return NEED_MORE;
 }
 
 METHOD(tls_handshake_t, process, status_t,
-	private_tls_peer_t *this, tls_handshake_type_t type, chunk_t data)
+	private_tls_peer_t *this, tls_handshake_type_t type, tls_reader_t *reader)
 {
 	switch (this->state)
 	{
@@ -205,11 +165,11 @@ METHOD(tls_handshake_t, process, status_t,
 			switch (type)
 			{
 				case TLS_SERVER_HELLO:
-					return process_server_hello(this, data);
+					return process_server_hello(this, reader);
 				case TLS_CERTIFICATE:
-					return process_certificate(this, data);
+					return process_certificate(this, reader);
 				case TLS_CERTIFICATE_REQUEST:
-					return process_certreq(this, data);
+					return process_certreq(this, reader);
 				case TLS_SERVER_HELLO_DONE:
 					this->state = STATE_HELLO_DONE;
 					return NEED_MORE;

src/charon/plugins/eap_tls/tls/tls_reader.c

@@ -0,0 +1,193 @@
+/*
+ * Copyright (C) 2010 Martin Willi
+ * Hochschule fuer Technik Rapperswil
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.  See <http://www.fsf.org/copyleft/gpl.txt>.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * for more details.
+ */
+
+#include "tls_reader.h"
+
+#include <daemon.h>
+
+typedef struct private_tls_reader_t private_tls_reader_t;
+
+/**
+ * Private data of an tls_reader_t object.
+ */
+struct private_tls_reader_t {
+
+	/**
+	 * Public tls_reader_t interface.
+	 */
+	tls_reader_t public;
+
+	/**
+	 * Remaining data to process
+	 */
+	chunk_t buf;
+};
+
+METHOD(tls_reader_t, remaining, u_int32_t,
+	private_tls_reader_t *this)
+{
+	return this->buf.len;
+}
+
+METHOD(tls_reader_t, read_uint8, bool,
+	private_tls_reader_t *this, u_int8_t *res)
+{
+	if (this->buf.len < 1)
+	{
+		DBG1(DBG_IKE, "%d bytes insufficient to parse uint%d TLS data",
+			 this->buf.len, 8);
+		return FALSE;
+	}
+	*res = this->buf.ptr[0];
+	this->buf = chunk_skip(this->buf, 1);
+	return TRUE;
+}
+
+METHOD(tls_reader_t, read_uint16, bool,
+	private_tls_reader_t *this, u_int16_t *res)
+{
+	if (this->buf.len < 2)
+	{
+		DBG1(DBG_IKE, "%d bytes insufficient to parse uint%d TLS data",
+			 this->buf.len, 16);
+		return FALSE;
+	}
+	*res = untoh16(this->buf.ptr);
+	this->buf = chunk_skip(this->buf, 2);
+	return TRUE;
+}
+
+METHOD(tls_reader_t, read_uint24, bool,
+	private_tls_reader_t *this, u_int32_t *res)
+{
+	if (this->buf.len < 3)
+	{
+		DBG1(DBG_IKE, "%d bytes insufficient to parse uint%d TLS data",
+			 this->buf.len, 24);
+		return FALSE;
+	}
+	*res = untoh32(this->buf.ptr) >> 8;
+	this->buf = chunk_skip(this->buf, 3);
+	return TRUE;
+}
+
+METHOD(tls_reader_t, read_uint32, bool,
+	private_tls_reader_t *this, u_int32_t *res)
+{
+	if (this->buf.len < 4)
+	{
+		DBG1(DBG_IKE, "%d bytes insufficient to parse uint%d TLS data",
+			 this->buf.len, 32);
+		return FALSE;
+	}
+	*res = untoh32(this->buf.ptr);
+	this->buf = chunk_skip(this->buf, 4);
+	return TRUE;
+}
+
+METHOD(tls_reader_t, read_data, bool,
+	private_tls_reader_t *this, u_int32_t len, chunk_t *res)
+{
+	if (this->buf.len < len)
+	{
+		DBG1(DBG_IKE, "%d bytes insufficient to parse %d bytes TLS data",
+			 this->buf.len, len);
+		return FALSE;
+	}
+	*res = chunk_create(this->buf.ptr, len);
+	this->buf = chunk_skip(this->buf, len);
+	return TRUE;
+}
+
+METHOD(tls_reader_t, read_data8, bool,
+	private_tls_reader_t *this, chunk_t *res)
+{
+	u_int8_t len;
+
+	if (!read_uint8(this, &len))
+	{
+		return FALSE;
+	}
+	return read_data(this, len, res);
+}
+
+METHOD(tls_reader_t, read_data16, bool,
+	private_tls_reader_t *this, chunk_t *res)
+{
+	u_int16_t len;
+
+	if (!read_uint16(this, &len))
+	{
+		return FALSE;
+	}
+	return read_data(this, len, res);
+}
+
+METHOD(tls_reader_t, read_data24, bool,
+	private_tls_reader_t *this, chunk_t *res)
+{
+	u_int32_t len;
+
+	if (!read_uint24(this, &len))
+	{
+		return FALSE;
+	}
+	return read_data(this, len, res);
+}
+
+METHOD(tls_reader_t, read_data32, bool,
+	private_tls_reader_t *this, chunk_t *res)
+{
+	u_int32_t len;
+
+	if (!read_uint32(this, &len))
+	{
+		return FALSE;
+	}
+	return read_data(this, len, res);
+}
+
+METHOD(tls_reader_t, destroy, void,
+	private_tls_reader_t *this)
+{
+	free(this);
+}
+
+/**
+ * See header
+ */
+tls_reader_t *tls_reader_create(chunk_t data)
+{
+	private_tls_reader_t *this;
+
+	INIT(this,
+		.public = {
+			.remaining = _remaining,
+			.read_uint8 = _read_uint8,
+			.read_uint16 = _read_uint16,
+			.read_uint24 = _read_uint24,
+			.read_uint32 = _read_uint32,
+			.read_data = _read_data,
+			.read_data8 = _read_data8,
+			.read_data16 = _read_data16,
+			.read_data24 = _read_data24,
+			.read_data32 = _read_data32,
+			.destroy = _destroy,
+		},
+		.buf = data,
+	);
+
+	return &this->public;
+}

src/charon/plugins/eap_tls/tls/tls_reader.h

@@ -0,0 +1,124 @@
+/*
+ * Copyright (C) 2010 Martin Willi
+ * Hochschule fuer Technik Rapperswil
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.  See <http://www.fsf.org/copyleft/gpl.txt>.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * for more details.
+ */
+
+/**
+ * @defgroup tls_reader tls_reader
+ * @{ @ingroup tls
+ */
+
+#ifndef TLS_READER_H_
+#define TLS_READER_H_
+
+typedef struct tls_reader_t tls_reader_t;
+
+#include <library.h>
+
+/**
+ * TLS record parser.
+ */
+struct tls_reader_t {
+
+	/**
+	 * Get the number of remaining bytes.
+	 *
+	 * @return			number of remaining bytes in buffer
+	 */
+	u_int32_t (*remaining)(tls_reader_t *this);
+
+	/**
+	 * Read a 8-bit integer from the buffer, advance.
+	 *
+	 * @param res		pointer to result
+	 * @return			TRUE if integer read successfully
+	 */
+	bool (*read_uint8)(tls_reader_t *this, u_int8_t *res);
+
+	/**
+	 * Read a 16-bit integer from the buffer, advance.
+	 *
+	 * @param res		pointer to result
+	 * @return			TRUE if integer read successfully
+	 */
+	bool (*read_uint16)(tls_reader_t *this, u_int16_t *res);
+
+	/**
+	 * Read a 24-bit integer from the buffer, advance.
+	 *
+	 * @param res		pointer to result
+	 * @return			TRUE if integer read successfully
+	 */
+	bool (*read_uint24)(tls_reader_t *this, u_int32_t *res);
+
+	/**
+	 * Read a 32-bit integer from the buffer, advance.
+	 *
+	 * @param res		pointer to result
+	 * @return			TRUE if integer read successfully
+	 */
+	bool (*read_uint32)(tls_reader_t *this, u_int32_t *res);
+
+	/**
+	 * Read a chunk of len bytes, advance.
+	 *
+	 * @param len		number of bytes to read
+	 * @param res		pointer to result, not cloned
+	 * @return			TRUE if data read successfully
+	 */
+	bool (*read_data)(tls_reader_t *this, u_int32_t len, chunk_t *res);
+
+	/**
+	 * Read a chunk of bytes with a 8-bit length header, advance.
+	 *
+	 * @param res		pointer to result, not cloned
+	 * @return			TRUE if data read successfully
+	 */
+	bool (*read_data8)(tls_reader_t *this, chunk_t *res);
+
+	/**
+	 * Read a chunk of bytes with a 16-bit length header, advance.
+	 *
+	 * @param res		pointer to result, not cloned
+	 * @return			TRUE if data read successfully
+	 */
+	bool (*read_data16)(tls_reader_t *this, chunk_t *res);
+
+	/**
+	 * Read a chunk of bytes with a 24-bit length header, advance.
+	 *
+	 * @param res		pointer to result, not cloned
+	 * @return			TRUE if data read successfully
+	 */
+	bool (*read_data24)(tls_reader_t *this, chunk_t *res);
+
+	/**
+	 * Read a chunk of bytes with a 32-bit length header, advance.
+	 *
+	 * @param res		pointer to result, not cloned
+	 * @return			TRUE if data read successfully
+	 */
+	bool (*read_data32)(tls_reader_t *this, chunk_t *res);
+
+	/**
+	 * Destroy a tls_reader_t.
+	 */
+	void (*destroy)(tls_reader_t *this);
+};
+
+/**
+ * Create a tls_reader instance.
+ */
+tls_reader_t *tls_reader_create(chunk_t data);
+
+#endif /** tls_reader_H_ @}*/

src/charon/plugins/eap_tls/tls/tls_server.c

@@ -42,7 +42,7 @@ struct private_tls_server_t {
 
 
 METHOD(tls_handshake_t, process, status_t,
-	private_tls_server_t *this, tls_handshake_type_t type, chunk_t data)
+	private_tls_server_t *this, tls_handshake_type_t type, tls_reader_t *reader)
 {
 	return NEED_MORE;
 }