Re: [PATCH] ath10k: rebuild crypto header in RX data frames
maybe this small patch hint here should help to make this patch better --- rx_desc.h (revision 3655) +++ rx_desc.h (working copy) @@ -239,6 +239,9 @@ HTT_RX_MPDU_ENCRYPT_WAPI = 5, HTT_RX_MPDU_ENCRYPT_AES_CCM_WPA2 = 6, HTT_RX_MPDU_ENCRYPT_NONE = 7, + HTT_RX_MPDU_ENCRYPT_AES_CCMP_256 = 8, + HTT_RX_MPDU_ENCRYPT_AES_GCMP_128 = 9, + HTT_RX_MPDU_ENCRYPT_AES_GCMP_256 = 10, Am 20.10.2017 um 18:28 schrieb Kalle Valo: From: Vasanthakumar Thiagarajan RX data frames notified through HTT_T2H_MSG_TYPE_RX_IND and HTT_T2H_MSG_TYPE_RX_FRAG_IND expect PN/TSC check to be done on host (mac80211) rather than firmware. Rebuild cipher header in every received data frames (that are notified through those HTT interfaces) from the PN/TSC and key_id information available from rx descriptor of the first msdu of each mpdu. Skip setting RX_FLAG_IV_STRIPPED flag for the packets which requires mac80211 PN/TSC check support and set appropriate RX_FLAG for stripped crypto tail. QCA988X, QCA9887, QCA99X0, QCA9984, QCA9888 and QCA4019 currently need the rebuilding of cipher header to perform PN/TSC check for replay attack. Signed-off-by: Vasanthakumar Thiagarajan Signed-off-by: Kalle Valo --- drivers/net/wireless/ath/ath10k/htt_rx.c | 120 ++ 1 file changed, 104 insertions(+), 16 deletions(-) diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c index a3f5dc78353f..9a070ad05179 100644 --- a/drivers/net/wireless/ath/ath10k/htt_rx.c +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c @@ -995,8 +995,55 @@ static int ath10k_htt_rx_nwifi_hdrlen(struct ath10k *ar, return len; } +static void ath10k_htt_rx_build_crypto_hdr(struct ath10k *ar, + struct sk_buff *msdu, + struct htt_rx_desc *rxd, + struct ieee80211_rx_status *status, + enum htt_rx_mpdu_encrypt_type type) +{ + u8 *hdr; + + if (!(status->flag & RX_FLAG_DECRYPTED) || + status->flag & RX_FLAG_IV_STRIPPED) + return; + + switch (type) { + case HTT_RX_MPDU_ENCRYPT_NONE: + return; + case HTT_RX_MPDU_ENCRYPT_WEP40: + case HTT_RX_MPDU_ENCRYPT_WEP104: + hdr = skb_push(msdu, IEEE80211_WEP_IV_LEN); + memcpy(hdr, rxd->mpdu_start.pn, IEEE80211_WEP_IV_LEN - 1); + hdr[3] = rxd->msdu_end.common.key_id_octet; + return; + case HTT_RX_MPDU_ENCRYPT_TKIP_WITHOUT_MIC: + case HTT_RX_MPDU_ENCRYPT_TKIP_WPA: + hdr = skb_push(msdu, IEEE80211_TKIP_IV_LEN); + hdr[0] = rxd->mpdu_start.pn[1]; + hdr[1] = 0; + hdr[2] = rxd->mpdu_start.pn[0]; + hdr[3] = 0x20 | (rxd->msdu_end.common.key_id_octet << 6); + memcpy(hdr + 4, rxd->mpdu_start.pn + 2, 4); + return; + case HTT_RX_MPDU_ENCRYPT_AES_CCM_WPA2: + hdr = skb_push(msdu, IEEE80211_CCMP_HDR_LEN); + memcpy(hdr, rxd->mpdu_start.pn, 2); + hdr[2] = 0; + hdr[3] = 0x20 | (rxd->msdu_end.common.key_id_octet << 6); + memcpy(hdr + 4, rxd->mpdu_start.pn + 2, 4); + return; + case HTT_RX_MPDU_ENCRYPT_WEP128: + case HTT_RX_MPDU_ENCRYPT_WAPI: + return; + default: + ath10k_warn(ar, "unsupported encryption type %d\n", type); + return; + } +} + static void ath10k_htt_rx_h_undecap_raw(struct ath10k *ar, struct sk_buff *msdu, + struct htt_rx_desc *first_rxd, struct ieee80211_rx_status *status, enum htt_rx_mpdu_encrypt_type enctype, bool is_decrypted) @@ -1050,8 +1097,14 @@ static void ath10k_htt_rx_h_undecap_raw(struct ath10k *ar, hdr = (void *)msdu->data; - /* Tail */ - if (status->flag & RX_FLAG_IV_STRIPPED) + /* MIC */ + if ((status->flag & RX_FLAG_MIC_STRIPPED) && + enctype == HTT_RX_MPDU_ENCRYPT_AES_CCM_WPA2) + skb_trim(msdu, msdu->len - 8); + + /* ICV */ + if (status->flag & RX_FLAG_ICV_STRIPPED && + enctype != HTT_RX_MPDU_ENCRYPT_AES_CCM_WPA2) skb_trim(msdu, msdu->len - ath10k_htt_rx_crypto_tail_len(ar, enctype)); @@ -1075,7 +1128,9 @@ static void ath10k_htt_rx_h_undecap_raw(struct ath10k *ar, static void ath10k_htt_rx_h_undecap_nwifi(struct ath10k *ar, struct sk_buff *msdu, struct ieee80211_rx_status *status, - const u8 fir
Re: [PATCH] ath10k: rebuild crypto header in RX data frames
i suggest the following patch on top of yours. please tell me if my thoughts are correct here. its mainly a guess --- htt_rx.c (revision 3656) +++ htt_rx.c (working copy) @@ -550,6 +550,11 @@ return IEEE80211_TKIP_IV_LEN; case HTT_RX_MPDU_ENCRYPT_AES_CCM_WPA2: return IEEE80211_CCMP_HDR_LEN; + case HTT_RX_MPDU_ENCRYPT_AES_CCMP_256: + return IEEE80211_CCMP_256_HDR_LEN; + case HTT_RX_MPDU_ENCRYPT_AES_GCMP_128: + case HTT_RX_MPDU_ENCRYPT_AES_GCMP_256: + return IEEE80211_GCMP_HDR_LEN; case HTT_RX_MPDU_ENCRYPT_WEP128: case HTT_RX_MPDU_ENCRYPT_WAPI: break; @@ -575,6 +580,11 @@ return IEEE80211_TKIP_ICV_LEN; case HTT_RX_MPDU_ENCRYPT_AES_CCM_WPA2: return IEEE80211_CCMP_MIC_LEN; + case HTT_RX_MPDU_ENCRYPT_AES_CCMP_256: + return IEEE80211_CCMP_256_MIC_LEN; + case HTT_RX_MPDU_ENCRYPT_AES_GCMP_128: + case HTT_RX_MPDU_ENCRYPT_AES_GCMP_256: + return IEEE80211_GCMP_MIC_LEN; case HTT_RX_MPDU_ENCRYPT_WEP128: case HTT_RX_MPDU_ENCRYPT_WAPI: break; @@ -1012,6 +1022,7 @@ return; case HTT_RX_MPDU_ENCRYPT_WEP40: case HTT_RX_MPDU_ENCRYPT_WEP104: + case HTT_RX_MPDU_ENCRYPT_WEP128: hdr = skb_push(msdu, IEEE80211_WEP_IV_LEN); memcpy(hdr, rxd->mpdu_start.pn, IEEE80211_WEP_IV_LEN - 1); hdr[3] = rxd->msdu_end.common.key_id_octet; @@ -1032,7 +1043,21 @@ hdr[3] = 0x20 | (rxd->msdu_end.common.key_id_octet << 6); memcpy(hdr + 4, rxd->mpdu_start.pn + 2, 4); return; - case HTT_RX_MPDU_ENCRYPT_WEP128: + case HTT_RX_MPDU_ENCRYPT_AES_CCMP_256: + hdr = skb_push(msdu, IEEE80211_CCMP_256_HDR_LEN); + memcpy(hdr, rxd->mpdu_start.pn, 2); + hdr[2] = 0; + hdr[3] = 0x20 | (rxd->msdu_end.common.key_id_octet << 6); + memcpy(hdr + 4, rxd->mpdu_start.pn + 2, 4); + return; + case HTT_RX_MPDU_ENCRYPT_AES_GCMP_128: + case HTT_RX_MPDU_ENCRYPT_AES_GCMP_256: + hdr = skb_push(msdu, IEEE80211_GCMP_HDR_LEN); + memcpy(hdr, rxd->mpdu_start.pn, 2); + hdr[2] = 0; + hdr[3] = 0x20 | (rxd->msdu_end.common.key_id_octet << 6); + memcpy(hdr + 4, rxd->mpdu_start.pn + 2, 4); + return; case HTT_RX_MPDU_ENCRYPT_WAPI: return; default: @@ -1098,16 +1123,41 @@ hdr = (void *)msdu->data; /* MIC */ - if ((status->flag & RX_FLAG_MIC_STRIPPED) && - enctype == HTT_RX_MPDU_ENCRYPT_AES_CCM_WPA2) - skb_trim(msdu, msdu->len - 8); - + if (status->flag & RX_FLAG_MIC_STRIPPED) { + switch(enctype) + { + case HTT_RX_MPDU_ENCRYPT_AES_CCM_WPA2: + skb_trim(msdu, msdu->len - IEEE80211_CCMP_MIC_LEN); + break; + case HTT_RX_MPDU_ENCRYPT_AES_CCMP_256: + skb_trim(msdu, msdu->len - IEEE80211_CCMP_256_MIC_LEN); + break; + case HTT_RX_MPDU_ENCRYPT_AES_GCMP_128: + skb_trim(msdu, msdu->len - IEEE80211_GCMP_MIC_LEN); + break; + case HTT_RX_MPDU_ENCRYPT_AES_GCMP_256: + skb_trim(msdu, msdu->len - IEEE80211_GCMP_MIC_LEN); + break; + default: + break; + } + } /* ICV */ - if (status->flag & RX_FLAG_ICV_STRIPPED && - enctype != HTT_RX_MPDU_ENCRYPT_AES_CCM_WPA2) + if (status->flag & RX_FLAG_ICV_STRIPPED) { + switch(enctype) + { + case HTT_RX_MPDU_ENCRYPT_WEP40: + case HTT_RX_MPDU_ENCRYPT_WEP104: + case HTT_RX_MPDU_ENCRYPT_TKIP_WITHOUT_MIC: + case HTT_RX_MPDU_ENCRYPT_WEP128: + case HTT_RX_MPDU_ENCRYPT_TKIP_WPA: skb_trim(msdu, msdu->len - ath10k_htt_rx_crypto_tail_len(ar, enctype)); - + break; + default: + break; + } + } /* MMIC */ if ((status->flag & RX_FLAG_MMIC_STRIPPED) && !ieee80211_has_morefrags(hdr->frame_control) && Index: rx_desc.h === --- rx_desc.h (revision 3656) +++ rx_desc.h (working copy) @@ -239,6 +239,9 @@ HTT_RX_MPDU_ENCRYPT_WAPI = 5, HTT_RX_MPDU_ENCRYPT_AES_CCM_WPA2 = 6, HTT_RX_MPDU_ENCRYPT_NONE = 7, + HTT_RX_MPDU_ENCRYPT_AES_CCMP_256 = 8, + HTT_RX_MPDU_ENCRYPT_AES_GCMP_128 = 9, + HTT_RX_MPDU_ENCRYPT_AES_GCMP_256 = 10, }; #define RX_MPDU_START_INFO0_PEER_IDX_MASK 0x07ff Am 20.10.2017 um 18:28 schrieb Kalle Valo: From: Vasanthakumar Thiagarajan RX data frames notified through HTT_T2H_MSG_TYPE_RX_IND and HTT_T2H_MS
Re: [PATCH] ath10k: rebuild crypto header in RX data frames
Jasmine Strong writes: > When we tried this patch, it completely broke all wpa2-ccmp-aes > traffic. Which patch, Vasanth's or Sebastian's? I even tested myself, with both CCMP and TKIP on both AP and client modes, and didn't see see any problems. What kind of setup you have? I tested on a x86 laptop and current ath.git master branch: ath10k_pci :02:00.0: pci irq msi oper_irq_mode 2 irq_mode 0 reset_mode 0 ath10k_pci :02:00.0: qca988x hw2.0 target 0x4100016c chip_id 0x043202ff sub : ath10k_pci :02:00.0: kconfig debug 1 debugfs 1 tracing 1 dfs 1 testmode 1 ath10k_pci :02:00.0: firmware ver 10.2.4.70.66 api 5 features no-p2p,raw-mode,mfp,allows-mesh-bcast crc32 c2dd2ad5 ath10k_pci :02:00.0: board_file api 1 bmi_id N/A crc32 bebc7c08 And my hostapd.conf: driver=nl80211 hw_mode=a channel=36 ieee80211n=1 interface=wlan0 ctrl_interface=/var/run/hostapd ctrl_interface_group=adm ssid=test-psk wpa=2 wpa_key_mgmt=WPA-PSK wpa_pairwise=CCMP wpa_passphrase=12345678 -- Kalle Valo
Re: [PATCH] ath10k: rebuild crypto header in RX data frames
even if he used my patch. my patch should have no influence to wpa2 ccmp. it just adds the new ccmp 256 + gcmp modes Am 21.10.2017 um 06:42 schrieb Kalle Valo: Jasmine Strong writes: When we tried this patch, it completely broke all wpa2-ccmp-aes traffic. Which patch, Vasanth's or Sebastian's? I even tested myself, with both CCMP and TKIP on both AP and client modes, and didn't see see any problems. What kind of setup you have? I tested on a x86 laptop and current ath.git master branch: ath10k_pci :02:00.0: pci irq msi oper_irq_mode 2 irq_mode 0 reset_mode 0 ath10k_pci :02:00.0: qca988x hw2.0 target 0x4100016c chip_id 0x043202ff sub : ath10k_pci :02:00.0: kconfig debug 1 debugfs 1 tracing 1 dfs 1 testmode 1 ath10k_pci :02:00.0: firmware ver 10.2.4.70.66 api 5 features no-p2p,raw-mode,mfp,allows-mesh-bcast crc32 c2dd2ad5 ath10k_pci :02:00.0: board_file api 1 bmi_id N/A crc32 bebc7c08 And my hostapd.conf: driver=nl80211 hw_mode=a channel=36 ieee80211n=1 interface=wlan0 ctrl_interface=/var/run/hostapd ctrl_interface_group=adm ssid=test-psk wpa=2 wpa_key_mgmt=WPA-PSK wpa_pairwise=CCMP wpa_passphrase=12345678 -- Mit freundlichen Grüssen / Regards Sebastian Gottschall / CTO NewMedia-NET GmbH - DD-WRT Firmensitz: Stubenwaldallee 21a, 64625 Bensheim Registergericht: Amtsgericht Darmstadt, HRB 25473 Geschäftsführer: Peter Steinhäuser, Christian Scheele http://www.dd-wrt.com email: s.gottsch...@dd-wrt.com Tel.: +496251-582650 / Fax: +496251-5826565
Re: [PATCH] ath10k: rebuild crypto header in RX data frames
On Saturday 21 October 2017 01:41 AM, Sebastian Gottschall wrote: > i suggest the following patch on top of yours. please tell me if my thoughts > are correct here. its mainly a guess I agree we need to take care of this for newly added ciphers as well. How about making it as a separate patch to make the patch bit smaller and to reduce the difficulties in back porting it for ciphers supported for long time? Vasanth
Re: [PATCH] ath10k: rebuild crypto header in RX data frames
Am 23.10.2017 um 16:24 schrieb Vasanthakumar Thiagarajan: On Saturday 21 October 2017 01:41 AM, Sebastian Gottschall wrote: i suggest the following patch on top of yours. please tell me if my thoughts are correct here. its mainly a guess I agree we need to take care of this for newly added ciphers as well. How about making it as a separate patch to make the patch bit smaller and to reduce the difficulties in back porting it for ciphers supported for long time? the patch for these both ciphers doesnt make it bigger. just a few lines. the main problem right now is that the original patch without my enhancements isnt working. it breaks encryption Vasanth -- Mit freundlichen Grüssen / Regards Sebastian Gottschall / CTO NewMedia-NET GmbH - DD-WRT Firmensitz: Stubenwaldallee 21a, 64625 Bensheim Registergericht: Amtsgericht Darmstadt, HRB 25473 Geschäftsführer: Peter Steinhäuser, Christian Scheele http://www.dd-wrt.com email: s.gottsch...@dd-wrt.com Tel.: +496251-582650 / Fax: +496251-5826565
Re: [PATCH] ath10k: rebuild crypto header in RX data frames
Jasmine Strong writes: > That's what I saw. A bcom client was able to associate and not pass > any traffic. This is on all three of 9882, 9888 and 4019. Thanks for the report, we'll investigate it. And I see that your email was now succesfully delivered to the list: http://lists.infradead.org/pipermail/ath10k/2017-October/010325.html -- Kalle Valo
Re: [PATCH] ath10k: rebuild crypto header in RX data frames
On 10/23/2017 09:50 PM, Kalle Valo wrote: Jasmine Strong writes: That's what I saw. A bcom client was able to associate and not pass any traffic. This is on all three of 9882, 9888 and 4019. Thanks for the report, we'll investigate it. And I see that your email was now succesfully delivered to the list: http://lists.infradead.org/pipermail/ath10k/2017-October/010325.html I'm not sure if this is helpful or not, but in the transition from 10.1 to 10.2 firmware, there was some tricky re-work of the PN counter logic in the firmware source. It had specific impact on the broadcom driver interaction. Possibly a similar issue is being seen with this driver patch? Thanks, Ben -- Ben Greear Candela Technologies Inc http://www.candelatech.com
Re: [PATCH] ath10k: rebuild crypto header in RX data frames
That can't be the case here, since we see it break the mesh too (where the clients are other QCA radios.) We're now seeing very slow mesh peering with a 9882 leaf to a 4019 gateway, with the second version of this patch. It took more than five minutes for one of the leaves to successfully peer (and the other 9882 leaf, despite achieving peering, never managed to get DHCP, implying that something is broken with data frames and this patch on qca9882.) On Tue, Oct 24, 2017 at 9:09 AM, Ben Greear wrote: > On 10/23/2017 09:50 PM, Kalle Valo wrote: >> >> Jasmine Strong writes: >> >>> That's what I saw. A bcom client was able to associate and not pass >>> any traffic. This is on all three of 9882, 9888 and 4019. >> >> >> Thanks for the report, we'll investigate it. And I see that your email >> was now succesfully delivered to the list: >> >> http://lists.infradead.org/pipermail/ath10k/2017-October/010325.html > > > > I'm not sure if this is helpful or not, but in the transition from 10.1 to > 10.2 firmware, > there was some tricky re-work of the PN counter logic in the firmware > source. It had > specific impact on the broadcom driver interaction. > > Possibly a similar issue is being seen with this driver patch? > > Thanks, > Ben > > -- > Ben Greear > Candela Technologies Inc http://www.candelatech.com >