[PATCH] Send Client-Error when AT_KDF attributes from the server are incorrect

[Hatano, Tomoharu (Sony Mobile)]

Hi Hostap,

I'm waiting for your feedback.
When you can start the review?

Best Regards,
Tomoharu Hatano

-----Original Message-----
From: Hatano, Tomoharu (Sony Mobile) 
Sent: Wednesday, August 16, 2017 9:52 PM
To: 'Tomoharu Hatano' <tomoharu.hatano at sony.com>; hostap at lists.infradead.org
Cc: Sogo, Shinji (Sony Mobile) <Shinji.Sogo at sony.com>; Nanbu, Tomonori (Sony Mobile) <Tomonori.Nanbu at sony.com>; Onodera, Akihiro X (Sony Mobile) <Akihiro.Onodera at sony.com>; Hatano, Tomoharu (Sony Mobile) <Tomoharu.Hatano at sony.com>
Subject: RE: [PATCH] Send Client-Error when AT_KDF attributes from the server are incorrect

Hi Hostap,

Do you have any progress about review?

Best Regards,
Tomoharu Hatano

-----Original Message-----
From: Tomoharu Hatano [mailto:tomoharu.hatano at sony.com]
Sent: Tuesday, July 25, 2017 12:26 PM
To: hostap at lists.infradead.org
Cc: Sogo, Shinji (Sony Mobile) <Shinji.Sogo at sony.com>; Nanbu, Tomonori (Sony Mobile) <Tomonori.Nanbu at sony.com>; Onodera, Akihiro X (Sony Mobile) <Akihiro.Onodera at sony.com>; Hatano, Tomoharu (Sony Mobile) <Tomoharu.Hatano at sony.com>
Subject: [PATCH] Send Client-Error when AT_KDF attributes from the server are incorrect

From: Akihiro Onodera <akihiro.onodera at sony.com>

After KDF negotiation, must check only requested change occurred in the list of AT_KDF attributes. If there are any other changes, the peer must behave like the case that AT_MAC had been incorrect and authentication is failed. These are defined in EAP-AKA' specification RFC5448.

Adds a complete check of AT_KDF attributes and sends Client-Error if a change which is not requested is included in it.

Change-Id: Ic8ac504a7ff01992e2632d35c243f53bdd27df74
Signed-off-by: Tomoharu Hatano <tomoharu.hatano at sony.com>
---
 src/eap_peer/eap_aka.c | 42 +++++++++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 15 deletions(-)

diff --git a/src/eap_peer/eap_aka.c b/src/eap_peer/eap_aka.c index 0bac62d..9a09184 100644
--- a/src/eap_peer/eap_aka.c
+++ b/src/eap_peer/eap_aka.c
@@ -53,6 +53,8 @@ struct eap_aka_data {
 	size_t network_name_len;
 	u16 kdf;
 	int kdf_negotiation;
+	u16 last_kdf_attrs[EAP_AKA_PRIME_KDF_MAX];
+	size_t last_kdf_count;
 };
 
 
@@ -817,9 +819,12 @@ static struct wpabuf * eap_aka_prime_kdf_neg(struct eap_aka_data *data,
 	size_t i;
 
 	for (i = 0; i < attr->kdf_count; i++) {
-		if (attr->kdf[i] == EAP_AKA_PRIME_KDF)
+		if (attr->kdf[i] == EAP_AKA_PRIME_KDF) {
+			os_memcpy(data->last_kdf_attrs, attr->kdf, sizeof(u16) * attr->kdf_count);
+			data->last_kdf_count = attr->kdf_count;
 			return eap_aka_prime_kdf_select(data, id,
 							EAP_AKA_PRIME_KDF);
+		}
 	}
 
 	/* No matching KDF found - fail authentication as if AUTN had been @@ -840,26 +845,30 @@ static int eap_aka_prime_kdf_valid(struct eap_aka_data *data,
 	 * of the selected KDF into the beginning of the list. */
 
 	if (data->kdf_negotiation) {
+		/* When the peer receives the new EAP-Request/AKA'-Challenge message, must check
+		 * only requested change occurred in the list of AT_KDF attributes. If there are any
+		 * other changes, the peer must behave like the case that AT_MAC had been incorrect
+		 * and authentication is failed. These are defined in EAP-AKA' specification
+		 * RFC5448. */
 		if (attr->kdf[0] != data->kdf) {
 			wpa_printf(MSG_WARNING, "EAP-AKA': The server did not "
 				   "accept the selected KDF");
-			return 0;
+			return -1;
 		}
 
-		for (i = 1; i < attr->kdf_count; i++) {
-			if (attr->kdf[i] == data->kdf)
-				break;
-		}
-		if (i == attr->kdf_count &&
-		    attr->kdf_count < EAP_AKA_PRIME_KDF_MAX) {
-			wpa_printf(MSG_WARNING, "EAP-AKA': The server did not "
-				   "duplicate the selected KDF");
-			return 0;
+		if (attr->kdf_count > EAP_AKA_PRIME_KDF_MAX ||
+		    attr->kdf_count != (data->last_kdf_count + 1)) {
+			wpa_printf(MSG_WARNING, "EAP-AKA': The length of KDF attributes is wrong");
+			return -1;
 		}
 
-		/* TODO: should check that the list is identical to the one
-		 * used in the previous Challenge message apart from the added
-		 * entry in the beginning. */
+		for (i = 1; i < attr->kdf_count; i++) {
+			if (attr->kdf[i] != data->last_kdf_attrs[i - 1]) {
+				wpa_printf(MSG_WARNING, "EAP-AKA': The KDF attributes except "
+					   "selected KDF are not same as original one.");
+				return -1;
+			}
+		}
 	}
 
 	for (i = data->kdf ? 1 : 0; i < attr->kdf_count; i++) { @@ -922,8 +931,11 @@ static struct wpabuf * eap_aka_process_challenge(struct eap_sm *sm,
 				  data->network_name, data->network_name_len);
 		/* TODO: check Network Name per 3GPP.33.402 */
 
-		if (!eap_aka_prime_kdf_valid(data, attr))
+		res = eap_aka_prime_kdf_valid(data, attr);
+		if (res == 0)
 			return eap_aka_authentication_reject(data, id);
+		else if (res == -1)
+			return eap_aka_client_error(data, id, 
+EAP_AKA_UNABLE_TO_PROCESS_PACKET);
 
 		if (attr->kdf[0] != EAP_AKA_PRIME_KDF)
 			return eap_aka_prime_kdf_neg(data, id, attr);
--
2.7.4