Please comment on these patches to hostapd-0.7.3

[Jouni Malinen]

On Fri, Oct 14, 2011 at 09:43:26PM -0700, B. J. wrote:
> Problems using NT-hash passwords for EPA-TTLS and EAP-PEAP authentications
> - hostapd was not copying password_hash element while creating dynamic EAP
> user object for each supplicant state machine. Though the configured object
> has correct value set (password_hash=1), if we do not copy this value,
> dynamic eap_user object in supplicant state machine will loose this value
> and will treat the password as clear text password and can not authenticate
> the client due to password mismatch.
> 
> --- vendor-drop/src/src/ap/ieee802_1x.c	2010-09-07 08:43:39.000000000 -0700
> +++ mainline/src/src/ap/ieee802_1x.c	2011-10-13 11:50:25.426100260 -0700
> @@ -1536,10 +1536,11 @@ static int ieee802_1x_get_eap_user(void 
> 		user->password_len = eap_user->password_len;
> +		user->password_hash = eap_user->password_hash;

Thanks! Apparently I've been testing this only with the RADIUS server
implementation which does indeed have this entry copied, but that was
indeed missing from the case where EAP server was used without RADIUS.
Applied.

> Wireless clients being disconnected "due to inactivity" - When WPA & WPA2
> are enabled simultaneously, the key information for both WPA2 & WPA is sent
> to the client even after they have made the decision that WPA2 will be
> used. It confuses the client. As a result, the client thinks the
> authentication is not ended and waits for further GTK negotiation, while
> the AP thinks it is done. So the client will keep authenticating status and
> is disconnected after timeout.

Would you be able to identify the client implementation(s) that show
this issue? It sounds like a client side bug to me since the EAPOL-Key
message 3/4 is supposed to allow multiple IEs (or KDEs) to be included
in Key Data. The purpose of including both the WPA and RSN IE here is to
allow the station to verify that the unauthenticated information in
Beacon and Probe Response frames was correct.

> --- vendor-drop/src/src/ap/wpa_auth.c	2010-09-07 08:43:39.000000000 -0700
> +++ mainline/src/src/ap/wpa_auth.c	2011-10-13 12:00:25.536104199 -0700
> @@ -1707,10 +1707,16 @@ SM_STATE(WPA_PTK, PTKINITNEGOTIATING)
> 	    wpa_ie_len > wpa_ie[1] + 2 && wpa_ie[0] == WLAN_EID_RSN) {
> 		/* WPA-only STA, remove RSN IE */
> 		wpa_ie = wpa_ie + wpa_ie[1] + 2;
> 		wpa_ie_len = wpa_ie[1] + 2;
> 	}
> +	if (sm->wpa == WPA_VERSION_WPA2 && 
> +			(sm->wpa_auth->conf.wpa & WPA_PROTO_WPA) &&
> +			wpa_ie[0] == WLAN_EID_RSN) {
> +		/* WPA2 STA, remove WPA IE */
> +		wpa_ie_len = wpa_ie[1] + 2;
> +	}

We do have a similar workaround for WPA-only stations because there were
deployed implementations that did not handle the new RSN IE very
gracefully.. However, I'm quite disappointed if that issues shows up
with WPA2 stations, too. For the protection against downgrade attacks to
work properly, it is important to be able to include all security
related elements in this frame. As such, I don't really want to include
this change unless there is a clear indication that this affects
considerable number of deployed station devices.

> Each AP has its own group key, but ath9k driver (which is what we're using)
> only uses one. So the last AP setting the group key will be able to send
> out multicast/broadcast packets, and other APs' multicast/broadcast will
> fail.  I've updated the ath9k driver we're using since this change was
> made; I'll check into what's going on with the later ath9k driver, and in
> any case this looks like an obvious kludge, but for now I'll just throw it
> out there.

I don't understand this description. Each AP has multiple group keys
(two in most cases) of which only once is used for transmitting frames
at any given time. I'm not sure what the other APs have to do with
this.. I'm not aware of any specific limitation that would make ath9k
require some hacks in this area.

> --- vendor-drop/src/src/ap/wpa_auth.c	2010-09-07 08:43:39.000000000 -0700
> +++ mainline/src/src/ap/wpa_auth.c	2011-10-13 12:00:25.536104199 -0700
> @@ -2098,10 +2104,12 @@ static int wpa_gtk_update(struct wpa_aut
> 	os_memcpy(group->GNonce, group->Counter, WPA_NONCE_LEN);
> 	inc_byte_array(group->Counter, WPA_NONCE_LEN);
> 	wpa_gmk_to_gtk(group->GMK, wpa_auth->addr, group->GNonce,
> 		       group->GTK[group->GN - 1], group->GTK_len);
> 
> +	memset(group->GTK[group->GN - 1], 1, group->GTK_len);

Huh? That would hardcode every group key to be a fixed string of 0x01
octets.. That's pretty horrible security issue and most certainly won't
be going anywhere near the upstream repository. If you have this in any
production use, I would strongly recommend getting rid of this pretty
quickly.

> There used to be a control that allowed wireless client-to-client traffic
> to be prevented.  This was supposed to be supported through
> i802_set_internal_bridge().  However, that hook was removed in April of
> 2009.  We were using a patch that implemented that capability (AFAICT, it
> was always just a NOOP before).  So wondering what the thinking is on this,
> and whether or not it should still be possible to prevent wireless
> client-to-client traffic.

That function was never fully implemented in the hostap.git repository.
The current development branch (0.8.x) has similar functionality
implemented with ap_isolate=1 configuration in hostapd.conf.

-- 
Jouni Malinen                                            PGP id EFC895FA