From: Tobias Brunner <tobias@strongswan.org>
Date: Mon, 13 Mar 2023 12:39:17 +0000 (+0100)
Subject: eap-tls: Fix server implementation with TLS 1.2 and earlier
X-Git-Tag: 5.9.11dr1~14
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=3d0d3f5d028a0c630f89cb4fec5b2cd5364f568b;p=thirdparty%2Fstrongswan.git

eap-tls: Fix server implementation with TLS 1.2 and earlier

With 5401a74d3608 ("eap-tls: Add support for TLS 1.3") a TLS application
was added to implement TLS 1.3's protected success indication.  For
earlier TLS versions, its build() method simply returned SUCCESS as
there was nothing to send.  However, that had the unintended side-effect
of also not sending the final TLS handshake messages (ChangeCipherSpec
and Finished).

The reason is that the TLS stack first checks for remaining handshake
messages but then also asks the registered application for data to
piggyback to that response (before the commit there was no application,
so that step was skipped).  The problem is that the status returned by
the application is directly forwarded through the TLS stack.  So not
returning INVALID_STATE caused the session to get concluded immediately
instead of resulting in ALREADY_DONE that would trigger sending the
final EAP message instead of an EAP-Success.

Fixes: 5401a74d3608 ("eap-tls: Add support for TLS 1.3")
---

diff --git a/src/libcharon/plugins/eap_tls/eap_tls.c b/src/libcharon/plugins/eap_tls/eap_tls.c
index 575488961e..98fde140d1 100644
--- a/src/libcharon/plugins/eap_tls/eap_tls.c
+++ b/src/libcharon/plugins/eap_tls/eap_tls.c
@@ -179,22 +179,31 @@ METHOD(tls_application_t, server_process, status_t,
 METHOD(tls_application_t, server_build, status_t,
 	eap_tls_app_t *app, bio_writer_t *writer)
 {
-	if (app->this->tls->get_version_max(app->this->tls) < TLS_1_3 ||
-		app->this->indication_sent_received)
+	if (app->this->indication_sent_received)
 	{
 		return SUCCESS;
 	}
-	/* build() is called twice when sending the indication, return the same
-	 * status but data only once */
-	if (app->indication_sent)
+	if (app->this->tls->get_version_max(app->this->tls) >= TLS_1_3)
 	{
-		app->this->indication_sent_received = TRUE;
+		/* build() is called twice when sending the indication, return the same
+		 * status but data only once */
+		if (app->indication_sent)
+		{
+			app->this->indication_sent_received = TRUE;
+		}
+		else
+		{	/* send a single 0x00 */
+			DBG2(DBG_TLS, "sending protected success indication via TLS");
+			writer->write_uint8(writer, 0);
+			app->indication_sent = TRUE;
+		}
 	}
 	else
-	{	/* send a single 0x00 */
-		DBG2(DBG_TLS, "sending protected success indication via TLS");
-		writer->write_uint8(writer, 0);
-		app->indication_sent = TRUE;
+	{
+		/* with earlier TLS versions, return INVALID_STATE without data to send
+		 * the final handshake messages (returning SUCCESS immediately would
+		 * prevent that) */
+		app->this->indication_sent_received = TRUE;
 	}
 	return INVALID_STATE;
 }