Hi Avesh,

> It turns out that unintialization of record type in the while loop during
> building of TLS records in tls.c is wreaking havoc on ppc64. I have come up
> with a preliminary patch for upstream review 

Thanks for your in-depth analysis and your patch. There is definitely a
bug while building those records.

I've tried to address this in a slightly different way. The upper layers
return NEED_MORE if any record has been created. So we actually should
check for that return type before querying the type output parameter.

Please try the attached patch; I don't have a PPC64 architecture at
hand, so your feedback is much appreciated.

Regards
Martin


>From f9cc7b7a90ab0bbd71a4503360fa5487a84416d8 Mon Sep 17 00:00:00 2001
From: Martin Willi <[email protected]>
Date: Thu, 8 Jan 2015 11:06:45 +0100
Subject: [PATCH] libtls: Check for CHANGE_CIPHER_SPEC type only if upper layer
 returns NEED_MORE

A type is returned only if upper layers successfully created a record, that is
return NEED_MORE. If we do not check for the return value, we might check a
previous record type and falsely reset the sequence number.
---
 src/libtls/tls_protection.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/libtls/tls_protection.c b/src/libtls/tls_protection.c
index b016db2..e73fedc 100644
--- a/src/libtls/tls_protection.c
+++ b/src/libtls/tls_protection.c
@@ -101,14 +101,13 @@ METHOD(tls_protection_t, build, status_t,
 	status_t status;
 
 	status = this->compression->build(this->compression, type, data);
-	if (*type == TLS_CHANGE_CIPHER_SPEC)
-	{
-		this->seq_out = 0;
-		return status;
-	}
-
 	if (status == NEED_MORE)
 	{
+		if (*type == TLS_CHANGE_CIPHER_SPEC)
+		{
+			this->seq_out = 0;
+			return status;
+		}
 		if (this->aead_out)
 		{
 			if (!this->aead_out->encrypt(this->aead_out, this->version,
-- 
1.9.1

_______________________________________________
Dev mailing list
[email protected]
https://lists.strongswan.org/mailman/listinfo/dev

Reply via email to