[v2,1/4] eapol: implement rekey support for authenticator

Commit Message

James Prestwood Jan. 12, 2023, 7:32 p.m. UTC

The only changes required was to set the secure bit for message 1,
reset the frame retry counter, and change the 2/4 verifier to use
the rekey flag rather than ptk_complete. This is because we must
set ptk_complete false in order to detect retransmissions of the
4/4 frame.

Initiating a rekey can now be done by simply calling eapol_start().
---
 src/eapol.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Comments

Denis Kenzior Jan. 13, 2023, 3:13 p.m. UTC | #1

Hi James,

On 1/12/23 13:32, James Prestwood wrote:
> The only changes required was to set the secure bit for message 1,
> reset the frame retry counter, and change the 2/4 verifier to use
> the rekey flag rather than ptk_complete. This is because we must
> set ptk_complete false in order to detect retransmissions of the
> 4/4 frame.
> 
> Initiating a rekey can now be done by simply calling eapol_start().
> ---
>   src/eapol.c | 14 +++++++++++---
>   1 file changed, 11 insertions(+), 3 deletions(-)
> 

<snip>

> @@ -1111,6 +1109,12 @@ static void eapol_send_ptk_1_of_4(struct eapol_sm *sm)
>   
>   	eapol_key_data_append(ek, sm->mic_len, HANDSHAKE_KDE_PMKID, pmkid, 16);
>   
> +	if (sm->handshake->ptk_complete) {
> +		ek->secure = true;
> +		sm->rekey = true;
> +		sm->handshake->ptk_complete = false;
> +	}
> +

Hmm, shouldn't ek->secure always be set to sm->rekey?  I'm thinking of 
retransmissions.  Lets say we start a rekey with eapol_start().  ptk_complete is 
true, so the first transmit of the 1/4 packet will set ek->secure to true.  But 
on subsequent retransmissions, this if() statement won't be hit due to 
ptk_complete being false.  So ek->secure won't be set properly, no?

>   	ek->header.packet_len = L_CPU_TO_BE16(EAPOL_FRAME_LEN(sm->mic_len) +
>   				EAPOL_KEY_DATA_LEN(ek, sm->mic_len) - 4);
>   

Regards,
-Denis

Patch

diff --git a/src/eapol.c b/src/eapol.c
index 22b2d5d1..2048a87d 100644
--- a/src/eapol.c
+++ b/src/eapol.c
@@ -1086,8 +1086,6 @@  static void eapol_send_ptk_1_of_4(struct eapol_sm *sm)
 
 	handshake_state_new_anonce(sm->handshake);
 
-	sm->handshake->ptk_complete = false;
-
 	sm->replay_counter++;
 
 	memset(ek, 0, EAPOL_FRAME_LEN(sm->mic_len));
@@ -1111,6 +1109,12 @@  static void eapol_send_ptk_1_of_4(struct eapol_sm *sm)
 
 	eapol_key_data_append(ek, sm->mic_len, HANDSHAKE_KDE_PMKID, pmkid, 16);
 
+	if (sm->handshake->ptk_complete) {
+		ek->secure = true;
+		sm->rekey = true;
+		sm->handshake->ptk_complete = false;
+	}
+
 	ek->header.packet_len = L_CPU_TO_BE16(EAPOL_FRAME_LEN(sm->mic_len) +
 				EAPOL_KEY_DATA_LEN(ek, sm->mic_len) - 4);
 
@@ -1589,7 +1593,7 @@  static void eapol_handle_ptk_2_of_4(struct eapol_sm *sm,
 
 	l_debug("ifindex=%u", sm->handshake->ifindex);
 
-	if (!eapol_verify_ptk_2_of_4(ek, sm->handshake->ptk_complete))
+	if (!eapol_verify_ptk_2_of_4(ek, sm->rekey))
 		return;
 
 	if (L_BE64_TO_CPU(ek->key_replay_counter) != sm->replay_counter)
@@ -2482,6 +2486,8 @@  static void eapol_eap_complete_cb(enum eap_result result, void *user_data)
 
 		/* sm->mic_len will have been set in eapol_eap_results_cb */
 
+		sm->frame_retry = 0;
+
 		/* Kick off 4-Way Handshake */
 		eapol_ptk_1_of_4_retry(NULL, sm);
 	}
@@ -2873,6 +2879,8 @@  bool eapol_start(struct eapol_sm *sm)
 			if (L_WARN_ON(!sm->handshake->have_pmk))
 				return false;
 
+			sm->frame_retry = 0;
+
 			/* Kick off handshake */
 			eapol_ptk_1_of_4_retry(NULL, sm);
 		}