Verify EAP-SIM/AKA AT_MAC before processing any attributes

4 files changed, 24 insertions, 36 deletions

diff --git a/src/charon/plugins/eap_aka/eap_aka_peer.c b/src/charon/plugins/eap_aka/eap_aka_peer.c
index d1ab554a1..26546809d 100644
--- a/src/charon/plugins/eap_aka/eap_aka_peer.c
+++ b/src/charon/plugins/eap_aka/eap_aka_peer.c
@@ -327,8 +327,8 @@ static status_t process_reauthentication(private_eap_aka_peer_t *this,
 	this->crypto->derive_keys_reauth(this->crypto,
 									 chunk_create(this->mk, HASH_SIZE_SHA1));
 
-	/* parse again with decryption key */
-	if (!in->parse(in))
+	/* verify MAC and parse again with decryption key */
+	if (!in->verify(in, chunk_empty) || !in->parse(in))
 	{
 		*out = create_client_error(this, in->get_identifier(in));
 		return NEED_MORE;
@@ -366,11 +366,6 @@ static status_t process_reauthentication(private_eap_aka_peer_t *this,
 		*out = create_client_error(this, in->get_identifier(in));
 		return NEED_MORE;
 	}
-	if (!in->verify(in, chunk_empty))
-	{
-		*out = create_client_error(this, in->get_identifier(in));
-		return NEED_MORE;
-	}
 
 	message = simaka_message_create(FALSE, in->get_identifier(in), EAP_AKA,
 									AKA_REAUTHENTICATION, this->crypto);
diff --git a/src/charon/plugins/eap_aka/eap_aka_server.c b/src/charon/plugins/eap_aka/eap_aka_server.c
index acf5c632a..9baff3e23 100644
--- a/src/charon/plugins/eap_aka/eap_aka_server.c
+++ b/src/charon/plugins/eap_aka/eap_aka_server.c
@@ -370,6 +370,11 @@ static status_t process_challenge(private_eap_aka_server_t *this,
 			 simaka_subtype_names, AKA_CHALLENGE);
 		return FAILED;
 	}
+	/* verify MAC of EAP message, AT_MAC */
+	if (!in->verify(in, chunk_empty))
+	{
+		return FAILED;
+	}
 	enumerator = in->create_attribute_enumerator(in);
 	while (enumerator->enumerate(enumerator, &type, &data))
 	{
@@ -389,12 +394,6 @@ static status_t process_challenge(private_eap_aka_server_t *this,
 	}
 	enumerator->destroy(enumerator);
 
-	/* verify MAC of EAP message, AT_MAC */
-	if (!in->verify(in, chunk_empty))
-	{
-		DBG1(DBG_IKE, "AT_MAC verification failed");
-		return FAILED;
-	}
 	/* compare received RES against stored XRES */
 	if (!chunk_equals(res, this->xres))
 	{
@@ -421,6 +420,11 @@ static status_t process_reauthentication(private_eap_aka_server_t *this,
 			 simaka_subtype_names, AKA_REAUTHENTICATION);
 		return FAILED;
 	}
+	/* verify AT_MAC attribute, signature is over "EAP packet | NONCE_S"  */
+	if (!in->verify(in, this->nonce))
+	{
+		return FAILED;
+	}
 
 	enumerator = in->create_attribute_enumerator(in);
 	while (enumerator->enumerate(enumerator, &type, &data))
@@ -444,11 +448,6 @@ static status_t process_reauthentication(private_eap_aka_server_t *this,
 	}
 	enumerator->destroy(enumerator);
 
-	/* verify AT_MAC attribute, signature is over "EAP packet | NONCE_S"  */
-	if (!in->verify(in, this->nonce))
-	{
-		return FAILED;
-	}
 	if (too_small)
 	{
 		DBG1(DBG_IKE, "received %N, initiating full authentication",
diff --git a/src/charon/plugins/eap_sim/eap_sim_peer.c b/src/charon/plugins/eap_sim/eap_sim_peer.c
index 66365b303..961cfd30d 100644
--- a/src/charon/plugins/eap_sim/eap_sim_peer.c
+++ b/src/charon/plugins/eap_sim/eap_sim_peer.c
@@ -387,8 +387,8 @@ static status_t process_reauthentication(private_eap_sim_peer_t *this,
 	this->crypto->derive_keys_reauth(this->crypto,
 									 chunk_create(this->mk, HASH_SIZE_SHA1));
 
-	/* parse again with decryption key */
-	if (!in->parse(in))
+	/* verify MAC and parse again with decryption key */
+	if (!in->verify(in, chunk_empty) || !in->parse(in))
 	{
 		*out = create_client_error(this, in->get_identifier(in),
 								   SIM_UNABLE_TO_PROCESS);
@@ -429,12 +429,6 @@ static status_t process_reauthentication(private_eap_sim_peer_t *this,
 								   SIM_UNABLE_TO_PROCESS);
 		return NEED_MORE;
 	}
-	if (!in->verify(in, chunk_empty))
-	{
-		*out = create_client_error(this, in->get_identifier(in),
-								   SIM_UNABLE_TO_PROCESS);
-		return NEED_MORE;
-	}
 
 	message = simaka_message_create(FALSE, in->get_identifier(in), EAP_SIM,
 									SIM_REAUTHENTICATION, this->crypto);
diff --git a/src/charon/plugins/eap_sim/eap_sim_server.c b/src/charon/plugins/eap_sim/eap_sim_server.c
index 03b6a19d6..f6d5df09b 100644
--- a/src/charon/plugins/eap_sim/eap_sim_server.c
+++ b/src/charon/plugins/eap_sim/eap_sim_server.c
@@ -196,6 +196,11 @@ static status_t process_reauthentication(private_eap_sim_server_t *this,
 			 simaka_subtype_names, SIM_REAUTHENTICATION);
 		return FAILED;
 	}
+	/* verify AT_MAC attribute, signature is over "EAP packet | NONCE_S"  */
+	if (!in->verify(in, this->nonce))
+	{
+		return FAILED;
+	}
 
 	enumerator = in->create_attribute_enumerator(in);
 	while (enumerator->enumerate(enumerator, &type, &data))
@@ -219,11 +224,6 @@ static status_t process_reauthentication(private_eap_sim_server_t *this,
 	}
 	enumerator->destroy(enumerator);
 
-	/* verify AT_MAC attribute, signature is over "EAP packet | NONCE_S"  */
-	if (!in->verify(in, this->nonce))
-	{
-		return FAILED;
-	}
 	if (too_small)
 	{
 		DBG1(DBG_IKE, "received %N, initiating full authentication",
@@ -429,6 +429,11 @@ static status_t process_challenge(private_eap_sim_server_t *this,
 			 simaka_subtype_names, SIM_CHALLENGE);
 		return FAILED;
 	}
+	/* verify AT_MAC attribute, signature is over "EAP packet | n*SRES"  */
+	if (!in->verify(in, this->sreses))
+	{
+		return FAILED;
+	}
 
 	enumerator = in->create_attribute_enumerator(in);
 	while (enumerator->enumerate(enumerator, &type, &data))
@@ -441,11 +446,6 @@ static status_t process_challenge(private_eap_sim_server_t *this,
 	}
 	enumerator->destroy(enumerator);
 
-	/* verify AT_MAC attribute, signature is over "EAP packet | n*SRES"  */
-	if (!in->verify(in, this->sreses))
-	{
-		return FAILED;
-	}
 	return SUCCESS;
 }