On Wed, 2007-08-15 at 00:12 -0400, Volker Braun wrote:
> 1) Instead of hacking around ieee80211_privacy_mismatch, remove it
> completely. It serves no useful purpose.
The purpose seems to be to avoid associating to BSSes that have privacy
enabled when we don't have any keys nor a tool told us that it's ok.
This again raises the question of why wpa_supplicant doesn't set the
IW_AUTH_KEY_MGMT_802_1X flag that should get you around the mismatch
check.
> @@ -3488,7 +3488,6 @@ static ieee80211_txrx_result
> ieee80211_rx_h_check(struct ieee80211_txrx_data *rx)
> {
> struct ieee80211_hdr *hdr;
> - int always_sta_key;
> hdr = (struct ieee80211_hdr *) rx->skb->data;
>
> /* Drop duplicate 802.11 retransmissions (IEEE 802.11 Chap. 9.2.9) */
> @@ -3556,29 +3555,19 @@ ieee80211_rx_h_check(struct ieee80211_tx
> return TXRX_QUEUED;
> }
>
> - if (rx->sdata->type == IEEE80211_IF_TYPE_STA)
> - always_sta_key = 0;
> - else
> - always_sta_key = 1;
> -
> - if (rx->sta && rx->sta->key && always_sta_key) {
> - rx->key = rx->sta->key;
> - } else {
> - if (rx->sta && rx->sta->key)
> + if (rx->fc & IEEE80211_FCTL_PROTECTED) {
> + if (rx->skb->pkt_type == PACKET_HOST &&
> + rx->sta && rx->sta->key)
> rx->key = rx->sta->key;
> - else
> - rx->key = rx->sdata->default_key;
> -
> - if ((rx->local->hw.flags & IEEE80211_HW_WEP_INCLUDE_IV) &&
> - rx->fc & IEEE80211_FCTL_PROTECTED) {
> + else {
> int keyidx = ieee80211_wep_get_keyidx(rx->skb);
> + if (keyidx < 0 || keyidx >= NUM_DEFAULT_KEYS)
> + return TXRX_DROP;
> +
> + rx->key = rx->sdata->keys[keyidx];
>
> - if (keyidx >= 0 && keyidx < NUM_DEFAULT_KEYS &&
> - (!rx->sta || !rx->sta->key || keyidx > 0))
> - rx->key = rx->sdata->keys[keyidx];
> -
> - if (!rx->key) {
> - if (!rx->u.rx.ra_match)
> + if (unlikely(!rx->key)) {
> + if (!rx->u.rx.ra_match)
> return TXRX_DROP;
> printk(KERN_DEBUG "%s: RX WEP frame with "
> "unknown keyidx %d (A1=" MAC_FMT " A2="
> @@ -3587,8 +3576,10 @@ ieee80211_rx_h_check(struct ieee80211_tx
> MAC_ARG(hdr->addr1),
> MAC_ARG(hdr->addr2),
> MAC_ARG(hdr->addr3));
> - if (!rx->local->apdev)
> + if (!rx->local->apdev) {
> + rx->local->dot11WEPUndecryptableCount++;
> return TXRX_DROP;
> + }
> ieee80211_rx_mgmt(
> rx->local, rx->skb, rx->u.rx.status,
> ieee80211_msg_wep_frame_unknown_key);
All those key selection changes and the VLAN group key thing have me
wondering now. Also see Larry's mail (meaningless messages), these
things are related.
> @@ -3597,7 +3588,7 @@ ieee80211_rx_h_check(struct ieee80211_tx
> }
> }
>
> - if (rx->fc & IEEE80211_FCTL_PROTECTED && rx->key && rx->u.rx.ra_match) {
> + if (rx->key && rx->u.rx.ra_match) {
That's just an optimisation, right? (If we have a key, the frame was
encrypted)
> - if (is_broadcast_ether_addr(sta_addr)) {
> + if (idx < 0 || idx >= NUM_DEFAULT_KEYS) {
> + printk(KERN_DEBUG "%s: set_encrypt - invalid idx = %d\n",
> + dev->name, idx);
> + return -EINVAL;
> + }
> +
> + if (is_multicast_ether_addr(sta_addr)) {
I still haven't understood why you changed from broadcast to multicast
here. Nor why you moved the key index check outside the check, if it's a
not a group key then the key index is irrelevant.
> set_tx_key = 0;
> - if (idx != 0) {
> + if (idx != 0 && alg != ALG_WEP) {
> printk(KERN_DEBUG "%s: set_encrypt - non-zero idx for "
> "individual key\n", dev->name);
> return -EINVAL;
So wpa_supplicant is actually trying to set a pairwise key with a key
index that isn't zero? That's really weird and definitely against the
rules. Is that somehow required? Shouldn't the AP be able to live with
you setting the key index to zero? Could you try that by forcing the
index to zero in this case?
Actually, maybe this is some weird Cisco rule-bending as you said, but
then I'd rather suspect that it's because of interaction with pre-shared
WEP keys rather than TKIP. In any case, it seems acceptable to remove
this restriction even if we then violate the standard.
johannes
Attachment:
signature.asc
Description: This is a digitally signed message part
