Discussion:
fix iwn ccmp replay detection
Stefan Sperling
2016-02-05 14:41:17 UTC
Permalink
iwn currently flags A-MPDU subframes which arrived out of sequence as CCMP
replays and drops them. We can't rely on the LAST_RX_AMPDU flag from the
last RX PHY notification, it seems. Check for a valid BA agreement instead.

Updating the last seen packet number with every out-of-order packet isn't
quite right, either.

Also, align the sequence number replay check with what ieee80211_input() does.

Stops netstat -W iwn0 showing tons of replayed (and hence, dropped) frames
when using 11n mode on a WPA network.

Index: if_iwn.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_iwn.c,v
retrieving revision 1.160
diff -u -p -r1.160 if_iwn.c
--- if_iwn.c 4 Feb 2016 21:32:01 -0000 1.160
+++ if_iwn.c 5 Feb 2016 14:40:03 -0000
@@ -1844,6 +1844,7 @@ iwn_ccmp_decap(struct iwn_softc *sc, str
struct ieee80211com *ic = &sc->sc_ic;
struct ieee80211_key *k = &ni->ni_pairwise_key;
struct ieee80211_frame *wh;
+ struct ieee80211_rx_ba *ba;
uint64_t pn, *prsc;
uint8_t *ivp;
uint8_t tid;
@@ -1860,6 +1861,7 @@ iwn_ccmp_decap(struct iwn_softc *sc, str
}
hasqos = ieee80211_has_qos(wh);
tid = hasqos ? ieee80211_get_qos(wh) & IEEE80211_QOS_TID : 0;
+ ba = hasqos ? &ni->ni_rx_ba[tid] : NULL;
prsc = &k->k_rsc[tid];

/* Extract the 48-bit PN from the CCMP header. */
@@ -1870,7 +1872,7 @@ iwn_ccmp_decap(struct iwn_softc *sc, str
(uint64_t)ivp[6] << 32 |
(uint64_t)ivp[7] << 40;
if (pn <= *prsc) {
- if (hasqos && (sc->last_rx_valid & IWN_LAST_RX_AMPDU)) {
+ if (hasqos && ba->ba_state == IEEE80211_BA_AGREED) {
/*
* This is an A-MPDU subframe.
* Such frames may be received out of order due to
@@ -1891,20 +1893,24 @@ iwn_ccmp_decap(struct iwn_softc *sc, str
orxseq = ni->ni_qos_rxseqs[tid];
else
orxseq = ni->ni_rxseq;
- if (nrxseq < orxseq) {
- DPRINTF(("CCMP replayed (n=%d < o=%d)\n",
+ if ((wh->i_fc[1] & IEEE80211_FC1_RETRY) &&
+ nrxseq == orxseq) {
+ DPRINTF(("CCMP replayed (n=%d == o=%d)\n",
nrxseq, orxseq));
ic->ic_stats.is_ccmp_replays++;
return 1;
}
+ /* Update last seen packet number. */
+ *prsc = pn;
} else {
DPRINTF(("CCMP replayed\n"));
ic->ic_stats.is_ccmp_replays++;
return 1;
}
+ } else {
+ /* Update last seen packet number. */
+ *prsc = pn;
}
- /* Update last seen packet number. */
- *prsc = pn;

/* Clear Protected bit and strip IV. */
wh->i_fc[1] &= ~IEEE80211_FC1_PROTECTED;
Stefan Sperling
2016-02-05 17:30:45 UTC
Permalink
Post by Stefan Sperling
Also, align the sequence number replay check with what ieee80211_input() does.
On second thought, I'm retracting my change to the sequence number check.
This check is about replay detection to make attacks on the crypto harder,
not about retried frames. So I'd like to keep this check strict.

The A-MPDU subframe problem is fixed either way.

Index: if_iwn.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_iwn.c,v
retrieving revision 1.161
diff -u -p -r1.161 if_iwn.c
--- if_iwn.c 5 Feb 2016 16:09:19 -0000 1.161
+++ if_iwn.c 5 Feb 2016 17:21:09 -0000
@@ -1843,6 +1843,7 @@ iwn_ccmp_decap(struct iwn_softc *sc, str
struct ieee80211com *ic = &sc->sc_ic;
struct ieee80211_key *k = &ni->ni_pairwise_key;
struct ieee80211_frame *wh;
+ struct ieee80211_rx_ba *ba;
uint64_t pn, *prsc;
uint8_t *ivp;
uint8_t tid;
@@ -1859,6 +1860,7 @@ iwn_ccmp_decap(struct iwn_softc *sc, str
}
hasqos = ieee80211_has_qos(wh);
tid = hasqos ? ieee80211_get_qos(wh) & IEEE80211_QOS_TID : 0;
+ ba = hasqos ? &ni->ni_rx_ba[tid] : NULL;
prsc = &k->k_rsc[tid];

/* Extract the 48-bit PN from the CCMP header. */
@@ -1869,7 +1871,7 @@ iwn_ccmp_decap(struct iwn_softc *sc, str
(uint64_t)ivp[6] << 32 |
(uint64_t)ivp[7] << 40;
if (pn <= *prsc) {
- if (hasqos && (sc->last_rx_valid & IWN_LAST_RX_AMPDU)) {
+ if (hasqos && ba->ba_state == IEEE80211_BA_AGREED) {
/*
* This is an A-MPDU subframe.
* Such frames may be received out of order due to
@@ -1896,14 +1898,17 @@ iwn_ccmp_decap(struct iwn_softc *sc, str
ic->ic_stats.is_ccmp_replays++;
return 1;
}
+ /* Update last seen packet number. */
+ *prsc = pn;
} else {
DPRINTF(("CCMP replayed\n"));
ic->ic_stats.is_ccmp_replays++;
return 1;
}
+ } else {
+ /* Update last seen packet number. */
+ *prsc = pn;
}
- /* Update last seen packet number. */
- *prsc = pn;

/* Clear Protected bit and strip IV. */
wh->i_fc[1] &= ~IEEE80211_FC1_PROTECTED;

Loading...