GTK msg 1/2 fields possibly incorrect

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:



> 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--) {

Hi Jouni,

On 5 February 2017 at 13:03, Jouni Malinen <j@w1.fi> wrote:
> On Tue, Jan 31, 2017 at 02:20:34AM +0100, Andrew Zaborowski wrote:
>> 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 */

Looks good, thanks.

>
>
>> 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);
>  }

Thanks, indeed GNonce is shown in that diagram.  In that case I guess
they're equally correct but I'd pick 0.

>
>
> 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:

After looking through the spec again it looks like either value would
be good here too.  While 802.11-2012 11.6.2 says "length in octets of
the pairwise temporal key to configure", and so this doesn't apply to
the GTK, it doesn't list 0 as a special backup value or anything
similar.  11.6.7.2 shows 0 but the "Key Length" fields in FTE GTK /
IGTK subelements are the length of the unwrapped / unpadded GTK /
IGTK.

Best regards

On Mon, Feb 06, 2017 at 12:35:58AM +0100, Andrew Zaborowski wrote:
> On 5 February 2017 at 13:03, Jouni Malinen <j@w1.fi> wrote:
> > 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:
> 
> After looking through the spec again it looks like either value would
> be good here too.  While 802.11-2012 11.6.2 says "length in octets of
> the pairwise temporal key to configure", and so this doesn't apply to
> the GTK, it doesn't list 0 as a special backup value or anything
> similar.  11.6.7.2 shows 0 but the "Key Length" fields in FTE GTK /
> IGTK subelements are the length of the unwrapped / unpadded GTK /
> IGTK.

There are actually three different values discussed here: 0, key length
of pairwise cipher key, and key length of group cipher key.
P802.11i/D3.0 (WPA) defined the Key Length field as indicating the
length of the key to be configured where that key was referring to the
pairwise key in 4-way handshake and group key in the group key
handshake. That design changed in the published IEEE Std 802.11i-2004
(WPA2/RSN) to describe the Key Length field as being specifically for
the pairwise key.

In other words, this cannot really be the same value for WPA and WPA2 if
the pairwise and group ciphers have different key lengths. It looks
cleanest to keep the current (new) behavior where the Key Length field
in group key handshake msg 1/2 is set to 0 for WPA2 and to the length of
the group key for WPA.