[RFC] mac80211: no missing key complaint if frame may not be for us

[RFC] mac80211: no missing key complaint if frame may not be for us

[John W. Linville]

Some APs implement support for multiple SSIDs using a single BSSID.
Each SSID might implement a different protection scheme (or none
at all).

If we are associated with an unprotected SSID on an AP operating as
described above, we might receive multicast frames that are actually
intended for another SSID.  If that SSID is protected, those frames
currently cause us to log a KERN_DEBUG message: "RX protected frame
but have no key".  This can cause a lot of traffic in the logs.

Rather than complain if we receive a protected frame for which we
have no key, then let's just keep quiet unless we are in something
other than managed mode or unless the frame is unicast.

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 net/mac80211/rx.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 064924c..1cc67ef 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -509,7 +509,13 @@ ieee80211_rx_h_decrypt(struct ieee80211_txrx_data *rx)
 		rx->key->tx_rx_count++;
 		/* TODO: add threshold stuff again */
 	} else {
-		if (net_ratelimit())
+		/* if there are multiple SSIDs on this BSSID, the frame may
+		   not be for us anyway; so, check that either the frame is
+		   unicast or that we are not in managed mode before
+		   complaining about a missing key */
+		if ((!is_multicast_ether_addr(hdr->addr1) ||
+		     rx->sdata->type != IEEE80211_IF_TYPE_STA) &&
+		    net_ratelimit())
 			printk(KERN_DEBUG "%s: RX protected frame,"
 			       " but have no key\n", rx->dev->name);
 		return TXRX_DROP;
-- 
1.5.2.4

[RFC] mac80211: no decrypt failed complaint if frame may not be for us

[John W. Linville]

From: John W. Linville <linville@tuxdriver.com>

Please see changelog entry for "mac80211: no missing key complaint if
frame may not be for us" for background information.

If multiple protected SSIDs share a BSSID, then each will see the
other's encrypted multicast traffic.  Rather than complain about being
unable to decrypt multicast traffic which may not be for us, just keep
quiet.

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 net/mac80211/wep.c |   11 ++++++++++-
 net/mac80211/wpa.c |   27 +++++++++++++++++++++------
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/net/mac80211/wep.c b/net/mac80211/wep.c
index 6675261..4b75ccb 100644
--- a/net/mac80211/wep.c
+++ b/net/mac80211/wep.c
@@ -311,6 +311,8 @@ u8 * ieee80211_wep_is_weak_iv(struct sk_buff *skb, struct ieee80211_key *key)
 ieee80211_txrx_result
 ieee80211_crypto_wep_decrypt(struct ieee80211_txrx_data *rx)
 {
+	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) rx->skb->data;
+
 	if ((rx->fc & IEEE80211_FCTL_FTYPE) != IEEE80211_FTYPE_DATA &&
 	    ((rx->fc & IEEE80211_FCTL_FTYPE) != IEEE80211_FTYPE_MGMT ||
 	     (rx->fc & IEEE80211_FCTL_STYPE) != IEEE80211_STYPE_AUTH))
@@ -318,7 +320,14 @@ ieee80211_crypto_wep_decrypt(struct ieee80211_txrx_data *rx)
 
 	if (!(rx->u.rx.status->flag & RX_FLAG_DECRYPTED)) {
 		if (ieee80211_wep_decrypt(rx->local, rx->skb, rx->key)) {
-			if (net_ratelimit())
+			/* if there are multiple SSIDs on this BSSID,
+			   the frame may not be for us anyway; so, check
+			   that either the frame is unicast or that we
+			   are not in managed mode before complaining
+			   about a missing key */
+			if ((!is_multicast_ether_addr(hdr->addr1) ||
+			     rx->sdata->type != IEEE80211_IF_TYPE_STA) &&
+			    net_ratelimit())
 				printk(KERN_DEBUG "%s: RX WEP frame, decrypt "
 				       "failed\n", rx->dev->name);
 			return TXRX_DROP;
diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index 0b32ab6..2d5bf83 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -321,9 +321,16 @@ ieee80211_crypto_tkip_decrypt(struct ieee80211_txrx_data *rx)
 					  &rx->u.rx.tkip_iv32,
 					  &rx->u.rx.tkip_iv16);
 	if (res != TKIP_DECRYPT_OK || wpa_test) {
-		printk(KERN_DEBUG "%s: TKIP decrypt failed for RX frame from "
-		       MAC_FMT " (res=%d)\n",
-		       rx->dev->name, MAC_ARG(rx->sta->addr), res);
+		/* if there are multiple SSIDs on this BSSID, the
+		   frame may not be for us anyway; so, check that either
+		   the frame is unicast or that we are not in managed
+		   mode before complaining about a missing key */
+		if ((!is_multicast_ether_addr(hdr->addr1) ||
+		     rx->sdata->type != IEEE80211_IF_TYPE_STA) &&
+		    net_ratelimit())
+			printk(KERN_DEBUG "%s: TKIP decrypt failed for RX "
+			       "frame from " MAC_FMT " (res=%d)\n",
+			       rx->dev->name, MAC_ARG(rx->sta->addr), res);
 		return TXRX_DROP;
 	}
 
@@ -590,9 +597,17 @@ ieee80211_crypto_ccmp_decrypt(struct ieee80211_txrx_data *rx)
 			    skb->data + hdrlen + CCMP_HDR_LEN, data_len,
 			    skb->data + skb->len - CCMP_MIC_LEN,
 			    skb->data + hdrlen + CCMP_HDR_LEN)) {
-			printk(KERN_DEBUG "%s: CCMP decrypt failed for RX "
-			       "frame from " MAC_FMT "\n", rx->dev->name,
-			       MAC_ARG(rx->sta->addr));
+			/* if there are multiple SSIDs on this BSSID,
+			   the frame may not be for us anyway; so, check
+			   that either the frame is unicast or that we
+			   are not in managed mode before complaining
+			   about a missing key */
+			if ((!is_multicast_ether_addr(hdr->addr1) ||
+			     rx->sdata->type != IEEE80211_IF_TYPE_STA) &&
+			    net_ratelimit())
+				printk(KERN_DEBUG "%s: CCMP decrypt failed "
+				       "for RX frame from " MAC_FMT "\n",
+				       rx->dev->name, MAC_ARG(rx->sta->addr));
 			return TXRX_DROP;
 		}
 	}
-- 
1.5.2.4

Re: [RFC] mac80211: no missing key complaint if frame may not be for us

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 709 bytes --]

On Tue, 2007-10-23 at 17:46 -0400, John W. Linville wrote:

> +		/* if there are multiple SSIDs on this BSSID, the frame may
> +		   not be for us anyway; so, check that either the frame is
> +		   unicast or that we are not in managed mode before
> +		   complaining about a missing key */
> +		if ((!is_multicast_ether_addr(hdr->addr1) ||
> +		     rx->sdata->type != IEEE80211_IF_TYPE_STA) &&
> +		    net_ratelimit())

I think this and the other patch looks good, though I'm not entirely
sure about the !STA test yet. I suppose though !STA means that we're in
some way an AP and then we're the only one sending real multicast
traffic.

Is it worth keeping this message at all?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [RFC] mac80211: no missing key complaint if frame may not be for us

[John W. Linville]

On Wed, Oct 24, 2007 at 11:00:55AM +0200, Johannes Berg wrote:
> On Tue, 2007-10-23 at 17:46 -0400, John W. Linville wrote:
> 
> > +		/* if there are multiple SSIDs on this BSSID, the frame may
> > +		   not be for us anyway; so, check that either the frame is
> > +		   unicast or that we are not in managed mode before
> > +		   complaining about a missing key */
> > +		if ((!is_multicast_ether_addr(hdr->addr1) ||
> > +		     rx->sdata->type != IEEE80211_IF_TYPE_STA) &&
> > +		    net_ratelimit())
> 
> I think this and the other patch looks good, though I'm not entirely
> sure about the !STA test yet. I suppose though !STA means that we're in
> some way an AP and then we're the only one sending real multicast
> traffic.
> 
> Is it worth keeping this message at all?

I considered removing it, and that may be the best bet.  Still, I
thought it might be worth yelling about it in at least some instances.

Along those lines, the !STA check was there because that was the
only case I knew might not be worth complaining about.  For example,
an IBSS should not be reusing a BSSID for multiple SSIDs...right?
Heck, maybe that isn't any crazier than doing the same thing in an
infrastructure network...

Well, what do you think?  Leave it as-is?  Leave only the !multicast
check?  Or remove the message completely?

John
-- 
John W. Linville
linville@tuxdriver.com

Re: [RFC] mac80211: no missing key complaint if frame may not be for us

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 449 bytes --]

On Wed, 2007-10-24 at 09:48 -0400, John W. Linville wrote:

> I considered removing it, and that may be the best bet.  Still, I
> thought it might be worth yelling about it in at least some instances.

Yeah it did catch a TKIP bug Larry ran into at some point. Maybe it
should be a debug message that only shows up with debug enabled? I don't
think regular users can do anything about the message anyway, "what is
TKIP anyway?"

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]