GTK msg 1/2 fields possibly incorrect

[Jouni Malinen]

On Tue, Jan 31, 2017 at 02:20:34AM +0100, Andrew Zaborowski wrote:
> i'm receiving a GTK-handshake msg 1/2 after an FT roam which our code
> can't validate.  This is confirmed by hostapd code in
> SM_STATE(WPA_PTK_GROUP, REKEYNEGOTIATING) in src/ap/wpa_auth.c.  The
> comment says hostap sends the following msg 1/2:
> 
> EAPOL(1, 1, 1, !Pair, G, RSC, GNonce, MIC(PTK), GTK[GN])
> 
> I couldn't firgure out what sm->Pair is exactly but with FT it's going
> to be false, unlike after an EAPOL 4-Way handshake and the install bit
> will be true in effect.  Is there any reason the install bit isn't
> just hardcoded to 0, as defined by 11.6.2 and illustrated by the
> message sequences in 11.6.7?

sm->Pair is actually supposed to be hardcoded to TRUE and it is just
because of the FT protocol (and FILS authentication) case having a
mechanism to skip proper state machine initialization which resulted in
this not getting set.. The !Pair case is somewhat of a theoretical one
here currently, but based on the standard, this should be used for RSN
IBSS in one direction.

I'll do following to fix the Install bit in group key handshake during
an association started with FT protocol or FILS authentication:

diff --git a/src/ap/wpa_auth.c b/src/ap/wpa_auth.c
index 66b2d50..0bd901f 100644
--- a/src/ap/wpa_auth.c
+++ b/src/ap/wpa_auth.c
@@ -615,6 +615,7 @@ int wpa_auth_sta_associated(struct wpa_authenticator *wpa_auth,
 				"start 4-way handshake");
 		/* Go to PTKINITDONE state to allow GTK rekeying */
 		sm->wpa_ptk_state = WPA_PTK_PTKINITDONE;
+		sm->Pair = TRUE;
 		return 0;
 	}
 #endif /* CONFIG_IEEE80211R_AP */
@@ -625,6 +626,7 @@ int wpa_auth_sta_associated(struct wpa_authenticator *wpa_auth,
 				"FILS authentication already completed - do not start 4-way handshake");
 		/* Go to PTKINITDONE state to allow GTK rekeying */
 		sm->wpa_ptk_state = WPA_PTK_PTKINITDONE;
+		sm->Pair = TRUE;
 		return 0;
 	}
 #endif /* CONFIG_FILS */


> Similarly it seems the nonce should just be 0 since it's neither
> ANonce or SNonce, and Key Length should be hardcoded to 0 in
> __wpa_send_eapol.  Patch for illustration.

The standard looks a bit ambiguous on the Key Nonce field.. The example
group handshake figure and the Authenticator state machine figures
describe the Send EAPOL operation as using GNonce as the nonce value
while the subclauses listing the field values for EAPOL-Key messages
show 0. The hostapd implementation followed the Authenticator state
machine description. That said, there is no use for GNonce on the
Supplicant side, so I'll clear it to zero with this:

@@ -3102,7 +3106,7 @@ SM_STATE(WPA_PTK_GROUP, REKEYNEGOTIATING)
 		       (wpa_mic_len(sm->wpa_key_mgmt) ? WPA_KEY_INFO_MIC : 0) |
 		       WPA_KEY_INFO_ACK |
 		       (!sm->Pair ? WPA_KEY_INFO_INSTALL : 0),
-		       rsc, gsm->GNonce, kde, kde_len, gsm->GN, 1);
+		       rsc, NULL, kde, kde_len, gsm->GN, 1);
 
 	os_free(kde_buf);
 }


The Key Length field has a bit more complex story.. P802.11i/D3.0
defines that field as having value 16 for EAPOL-Key group message 1/2.
That behavior needs to be maintained for WPA interoperability. That
said, it looks like it would be fine to clear this to 0 for WPA2 (RSN)
cases and hopefully that does not cause any new interop issues.. I'm
planning on doing this to address that:

@@ -1482,9 +1484,11 @@ void __wpa_send_eapol(struct wpa_authenticator *wpa_auth,
 	WPA_PUT_BE16(key->key_info, key_info);
 
 	alg = pairwise ? sm->pairwise : wpa_auth->conf.wpa_group;
-	WPA_PUT_BE16(key->key_length, wpa_cipher_key_len(alg));
-	if (key_info & WPA_KEY_INFO_SMK_MESSAGE)
+	if ((key_info & WPA_KEY_INFO_SMK_MESSAGE) ||
+	    (sm->wpa == WPA_VERSION_WPA2 && !pairwise))
 		WPA_PUT_BE16(key->key_length, 0);
+	else
+		WPA_PUT_BE16(key->key_length, wpa_cipher_key_len(alg));
 
 	/* FIX: STSL: what to use as key_replay_counter? */
 	for (i = RSNA_MAX_EAPOL_RETRIES - 1; i > 0; i--) {

-- 
Jouni Malinen                                            PGP id EFC895FA