Please comment on these patches to hostapd-0.7.3

[B. J.]

On Oct 15, 2011, at 2:35 AM, Jouni Malinen wrote:

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

Great!  Thanks for picking this up.  Do you have a commit number I should
reference?

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

[...]

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

This makes sense; I'll dig into it and see what I find.

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

Yeah, I think this one is bogus; just ignore it for now.

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

Okay.  This is the important one for us then, as we've been using this
functionality for some time.  I'll look into what would be required for us
to keep using it under 0.7.3.  If you have any thoughts, please let me
know.

Thanks Jouni!

> 
> -- 
> Jouni Malinen                                            PGP id EFC895FA
> _______________________________________________
> HostAP mailing list
> HostAP at lists.shmoo.com
> http://lists.shmoo.com/mailman/listinfo/hostap