| Message ID | 568B912F.8070100@neratec.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Johannes Berg |
| Headers | show |
On Tue, 2016-01-05 at 10:47 +0100, Matthias May wrote: > > We've encountered exactly this problem in a mix of devices where one > applies key material faster than the other. (ath9k and aquilla) > As a workaround we check on the STA if we are authorized when > updating/checking CCMP. (see attached patches) > Those don't really seem safe to use either. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/01/16 10:58, Johannes Berg wrote: > On Tue, 2016-01-05 at 10:47 +0100, Matthias May wrote: > >> >> We've encountered exactly this problem in a mix of devices where one >> applies key material faster than the other. (ath9k and aquilla) >> As a workaround we check on the STA if we are authorized when >> updating/checking CCMP. (see attached patches) >> > > Those don't really seem safe to use either. > > johannes > Not safe as in "access to stuff which has to be locked", or not safe as in "a CCMP replay attack is possible"? When changing this we argumented that since we are not really connected yet, a CCMP replay attack doesn't really make sense. Matthias -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2016-01-05 at 11:54 +0100, Matthias May wrote: > > Not safe as in "access to stuff which has to be locked", or not safe > as > in "a CCMP replay attack is possible"? > When changing this we argumented that since we are not really > connected > yet, a CCMP replay attack doesn't really make sense. > It's a bit more complicated than my first look suggested, it seems. However, I'm not sure what effect your patch is supposed to have. You're skipping CCMP replay checking and update when not authorized yet, at which point the station isn't receiving frames anyway (though they'd be checked for all this, they'd later be discarded). Once it becomes authorized, you do the checks. However, it never becomes unauthorized again, even for rekeying, so for the PTK rekeying issue at hand it's pretty much a no-op? johannes PS: the comment in your patch is also wrong: > + /* If we are a station update the ccmp counter only when we are > + * authorised. For all other modes always update. */ > + if (!rx->sta || > + (rx->sta && test_sta_flag(rx->sta, WLAN_STA_AUTHORIZED)) ) { There's no check for "if we are a station" here. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/01/16 16:50, Johannes Berg wrote: > On Tue, 2016-01-05 at 11:54 +0100, Matthias May wrote: >> >> Not safe as in "access to stuff which has to be locked", or not safe >> as >> in "a CCMP replay attack is possible"? >> When changing this we argumented that since we are not really >> connected >> yet, a CCMP replay attack doesn't really make sense. >> > > It's a bit more complicated than my first look suggested, it seems. > > However, I'm not sure what effect your patch is supposed to have. > You're skipping CCMP replay checking and update when not authorized > yet, at which point the station isn't receiving frames anyway (though > they'd be checked for all this, they'd later be discarded). > > Once it becomes authorized, you do the checks. However, it never > becomes unauthorized again, even for rekeying, so for the PTK rekeying > issue at hand it's pretty much a no-op? > afaik it solves the issue that when a STA roames from AP1 to AP2, and key material is installed at different times. We observed encrypted frames which had a wrong CCMP counter. If the STA updates it's counter with these frames then depending on the wrong CCMP value received, up to a few hundred frames were dropped. Not exactly the same as rekeying but the effect are pretty similar. > johannes > > PS: the comment in your patch is also wrong: > >> + /* If we are a station update the ccmp counter only when we are >> + * authorised. For all other modes always update. */ >> + if (!rx->sta || >> + (rx->sta && test_sta_flag(rx->sta, WLAN_STA_AUTHORIZED)) ) { > > There's no check for "if we are a station" here. > Yeah this doesn't make sense. Also the check on !rx-sta seems superfluous since it's already checked a few lines above. Regards Matthias -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2016-01-06 at 10:09 +0100, Matthias May wrote: > > afaik it solves the issue that when a STA roames from AP1 to AP2, and > key material is installed at different times. > We observed encrypted frames which had a wrong CCMP counter. If the > STA > updates it's counter with these frames then depending on the wrong > CCMP > value received, up to a few hundred frames were dropped. I don't really see how it has any effect there either, since in that case the old key material should be deleted long before the new one is installed, so the cross-over that causes the PN update problem with rekeying can't happen? > Not exactly the same as rekeying but the effect are pretty similar. Ignoring the discussion about the effect of the patch in roaming, the patch really can't do anything for rekeying since the station never goes back to !authorized in that case, so it can't really be relevant for this thread. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c index e115175..dc9c084 100644 --- a/net/mac80211/wpa.c +++ b/net/mac80211/wpa.c @@ -538,7 +538,13 @@ ieee80211_crypto_ccmp_decrypt(struct ieee80211_rx_data *rx, return RX_DROP_UNUSABLE; } - memcpy(key->u.ccmp.rx_pn[queue], pn, IEEE80211_CCMP_PN_LEN); + /* If we are a station update the ccmp counter only when we are + * authorised. For all other modes always update. */ + if (!rx->sta || + (rx->sta && test_sta_flag(rx->sta, WLAN_STA_AUTHORIZED)) ) { + memcpy(key->u.ccmp.rx_pn[queue], + pn, IEEE80211_CCMP_PN_LEN); + } } /* Remove CCMP header and MIC */