[RFC PATCH] pmksa: don't evict active entry when adding new ones

Jouni Malinen j at w1.fi
Sun Nov 25 15:23:29 EST 2012


On Mon, Aug 13, 2012 at 12:51:24PM -0500, Dan Williams wrote:
> So one problem with the current code in my testing was that when an
> entry is removed, the check inwpa_sm_pmksa_free_cb() that determines
> whether or not to deauthenticate checks either the cache entry, or the
> PMK itself:
> 
> 	if (sm->cur_pmksa == entry ||
> 	    (sm->pmk_len == entry->pmk_len &&
> 	     os_memcmp(sm->pmk, entry->pmk, sm->pmk_len) == 0)) {

There shouldn't really be need for checking the actual PMK value, but
until we can trust on sm->cur_pmksa being always set correctly, it is
better to keep that there to make sure PTK gets expired when the PMK
does.

> 1) is it normal for multiple cache entries to have the same PMK?  It
> seems the answer is "yes", given pmksa_cache_clone_entry() and the
> opportunistic caching logic.

Yes, with OKC.

> 2) Shouldn't pmksa_cache_set_current() be honoring the BSSID that's
> passed in before falling back to just checking the PMK?  It seems to me
> the code should be searching from more specific to less specific.  If
> we're passed *both* a PMKID and a BSSID, shouldn't the code make an
> effort to find one that matches both, instead of just the PMKID?
> Otherwise we end up with sm->cur_pmksa pointing to a BSSID that we're
> not associated with.

This function is a bit ugly due to history, i.e., number of different
places were setting cur_pmksa and I just collected them into that single
function when adding some more separation between the core
wpa_supplicant code and RSN/PMKSA cache implementation. This is designed
to search mainly based on PMKID for the association case (which does not
actually even pass the BSSID in the current caller), but since PMKID is
derived based on AA (= BSSID), this will, in practice, match both.

Do you have clear cases where sm->cur_pmksa is really pointing to
another BSSID? The way the PMKID is derived should make that not happen
in practice..

> Something like this maybe?  I'd argue that if both the PMKID and BSSID
> are passed, and an exact match is not found, that's a failure.  If only
> one of the two is passed, then we can't find an exact match, so try for
> something close.  But give the BSSID priority since that's most likely
> what we're currently associated with?  I don't pretend to know much
> about OKC operationally though.
> 
> 	sm->cur_pmksa = NULL;
> 	if (pmkid && bssid) {
> 		/* exact match requested; if we cannot provide it we fail */
> 		sm->cur_pmksa = pmksa_cache_get(pmksa, bssid, pmkid,
> 						network_ctx);
> 	} else {
> 		/* Fuzzy matching; one of BSSID or PMKID not known */
> 		if (bssid)
> 			sm->cur_pmksa = pmksa_cache_get(pmksa, bssid, NULL,
> 							network_ctx);
> 		if (sm->cur_pmksa == NULL && try_opportunistic && bssid)
> 			sm->cur_pmksa = pmksa_cache_get_opportunistic(pmksa,
> 								      network_ctx,
> 								      bssid);
> 		if (sm->cur_pmksa == NULL && pmkid)
> 			sm->cur_pmksa = pmksa_cache_get(pmksa, NULL, pmkid,
> 							network_ctx);
> 	}

I'm not sure I would do this now, i.e., it would be better to go through
the callers first and make sure they pass proper information for this or
well, maybe just split pmksa_cache_set_current() into couple of
functions based on the use cases to make this somewhat cleaner.

-- 
Jouni Malinen                                            PGP id EFC895FA


More information about the HostAP mailing list