From: Peter Hessler Subject: Re: fix qwx(4) TKIP crypto offload To: tech@openbsd.org Date: Sat, 2 Aug 2025 16:20:23 +0200 OK On 2025 Aug 02 (Sat) at 15:24:41 +0200 (+0200), Stefan Sperling wrote: :TKIP offload didn't work correctly, preventing qwx(4) from working with :APs that use tkip as the group cipher. : :This patch makes qwx(4) work in 11n mode against such APs which show up :in ifconfig qwx0 like this: : : wpaprotos wpa2 wpaakms psk wpaciphers ccmp wpagroupcipher tkip : :One bug was that key_len in the key installation command was set wrong :for tkip. It should not include the length of the Tx/Rx MICs. Fixing :the length prevents the bad tkip mic counter in netstat -W qwx0 from :increasing for every received broadcast packet, whenever the firmware :sends us a corresponding error event: : : 42 input packets with bad tkip mic : :Another problem was that the firmware did not use the installed TKIP :group key when sending frames. Setting the WMI_KEY_TX_USAGE bit in the :key flags and sending a command to set the default tx key ID fixes this. : :I have not yet tested with a pure WPA1 AP (i.e. with tkip as the pairwise :cipher). Thie may now start working with offloading, provided the WPA1 :protocol is enabled: ifconfig qwx0 wpaprotos wpa1,wpa2 : :This patch applies on top of the 802.11n support patch I sent earlier. : :M sys/dev/ic/qwx.c | 21+ 30- :M sys/dev/ic/qwxreg.h | 1+ 0- : :2 files changed, 22 insertions(+), 30 deletions(-) : :commit - 9346445d75793bfa730fc90d726f024ac0f7b74f :commit + 56a5a7a01f1b73bb178f5c78220033651b7ec167 :blob - 936d4522e0d77b745d22edb71c628d3a2f1ff7f9 :blob + d6490790d0f3e15bca76efb4ae82b2b518c517ac :--- sys/dev/ic/qwx.c :+++ sys/dev/ic/qwx.c :@@ -162,6 +162,8 @@ void qwx_setkey_clear(struct qwx_softc *); : void qwx_vif_free_all(struct qwx_softc *); : void qwx_dp_stop_shadow_timers(struct qwx_softc *); : void qwx_ce_stop_shadow_timers(struct qwx_softc *); :+int qwx_wmi_vdev_set_param_cmd(struct qwx_softc *, uint32_t, uint8_t, :+ uint32_t, uint32_t); : : int qwx_scan(struct qwx_softc *, int); : void qwx_scan_abort(struct qwx_softc *); :@@ -234,14 +236,11 @@ qwx_init(struct ifnet *ifp) : * : * 1) Broadcast/Multicast frames will only be received on : * encrypted networks if hardware crypto is used and a :- * CCMP group key is used. Otherwise such frames never :+ * CCMP/TKIP group key is used. Otherwise such frames never : * even trigger an interrupt. This breaks ARP and IPv6. : * This issue is known to affect the Linux ath11k vendor : * driver when software crypto mode is selected. : * Workaround: Use hardware crypto on WPA2 networks. :- * However, even with hardware crypto broadcast frames :- * are never received if TKIP is used as the WPA2 group :- * cipher and we have no workaround for this. : * : * 2) Adding WEP keys for hardware crypto crashes the firmware. : * Presumably, lack of WEP support is deliberate because the :@@ -249,20 +248,8 @@ qwx_init(struct ifnet *ifp) : * WEP keys to hardware. : * Workaround: Use software crypto if WEP is enabled. : * This suffers from the broadcast issues mentioned above. :- * :- * 3) A WPA1 group key handshake message from the AP is never :- * received if hardware crypto is used. :- * Workaround: Use software crypto if WPA1 is enabled. :- * This suffers from the broadcast issues mentioned above, :- * even on WPA2 networks when WPA1 and WPA2 are both enabled. :- * On OpenBSD, WPA1 is disabled by default. :- * :- * The only known fully working configurations are unencrypted :- * networks, and WPA2/CCMP-only networks provided WPA1 remains :- * disabled. : */ :- if ((ic->ic_flags & IEEE80211_F_WEPON) || :- (ic->ic_rsnprotos & IEEE80211_PROTO_WPA)) :+ if (ic->ic_flags & IEEE80211_F_WEPON) : sc->crypto_mode = ATH11K_CRYPT_MODE_SW; : else : sc->crypto_mode = ATH11K_CRYPT_MODE_HW; :@@ -759,6 +746,7 @@ qwx_add_sta_key(struct qwx_softc *sc, struct ieee80211 : struct qwx_node *nq = (struct qwx_node *)ni; : struct ath11k_peer *peer; : struct qwx_vif *arvif = TAILQ_FIRST(&sc->vif_list); /* XXX */ :+ uint8_t pdev_id = 0; /* TODO: derive pdev ID somehow? */ : int ret = 0; : uint32_t flags = 0; : const int want_keymask = (QWX_NODE_FLAG_HAVE_PAIRWISE_KEY | :@@ -775,7 +763,7 @@ qwx_add_sta_key(struct qwx_softc *sc, struct ieee80211 : qwx_peer_frags_flush(sc, peer); : : if (k->k_flags & IEEE80211_KEY_GROUP) :- flags |= WMI_KEY_GROUP; :+ flags |= WMI_KEY_GROUP | WMI_KEY_TX_USAGE; : else : flags |= WMI_KEY_PAIRWISE; : :@@ -793,9 +781,15 @@ qwx_add_sta_key(struct qwx_softc *sc, struct ieee80211 : return ret; : } : :- if (k->k_flags & IEEE80211_KEY_GROUP) :+ if (k->k_flags & IEEE80211_KEY_GROUP) { :+ ret = qwx_wmi_vdev_set_param_cmd(sc, arvif->vdev_id, pdev_id, :+ WMI_VDEV_PARAM_DEF_KEYID, k->k_id); :+ if (ret) { :+ printf("%s: failed to set vdev %d def key ID %d: %d\n", :+ sc->sc_dev.dv_xname, arvif->vdev_id, k->k_id, ret); :+ } : nq->flags |= QWX_NODE_FLAG_HAVE_GROUP_KEY; :- else :+ } else : nq->flags |= QWX_NODE_FLAG_HAVE_PAIRWISE_KEY; : : if ((nq->flags & want_keymask) == want_keymask) { :@@ -16449,6 +16443,10 @@ qwx_dp_rx_h_rxdma_err(struct qwx_softc *sc, struct qwx : ic->ic_stats.is_rx_locmicfail++; : drop = 1; : break; :+ case HAL_REO_ENTR_RING_RXDMA_ECODE_DECRYPT_ERR: :+ ic->ic_stats.is_rx_wepfail++; :+ drop = 1; :+ break; : default: : /* TODO: Review other rxdma error code to check if anything is : * worth reporting to mac80211 :@@ -17005,10 +17003,11 @@ qwx_dp_rx_h_mpdu(struct qwx_softc *sc, struct qwx_rx_m : #endif : struct rx_attention *rx_attention; : uint32_t err_bitmap; :- :+#if 0 : /* PN for multicast packets will be checked in net80211 */ : fill_crypto_hdr = qwx_dp_rx_h_attn_is_mcbc(sc, rx_desc); : msdu->is_mcbc = fill_crypto_hdr; :+#endif : #if 0 : if (rxcb->is_mcbc) { : rxcb->peer_id = ath11k_dp_rx_h_mpdu_start_peer_id(ar->ab, rx_desc); :@@ -19020,7 +19019,7 @@ qwx_wmi_vdev_install_key(struct qwx_softc *sc, : cmd->key_idx = arg->key_idx; : cmd->key_flags = arg->key_flags; : cmd->key_cipher = arg->key_cipher; :- cmd->key_len = arg->key_len; :+ cmd->key_len = arg->key_len - (arg->key_txmic_len + arg->key_rxmic_len); : cmd->key_txmic_len = arg->key_txmic_len; : cmd->key_rxmic_len = arg->key_rxmic_len; : :@@ -24762,14 +24761,6 @@ qwx_dp_peer_rx_pn_replay_config(struct qwx_softc *sc, : uint8_t tid; : int ret = 0; : :- /* :- * NOTE: Enable PN/TSC replay check offload only for unicast frames. :- * We use net80211 PN/TSC replay check functionality for bcast/mcast :- * for now. :- */ :- if (k->k_flags & IEEE80211_KEY_GROUP) :- return 0; :- : cmd.flag |= HAL_REO_CMD_FLG_NEED_STATUS; : cmd.upd0 |= HAL_REO_CMD_UPD0_PN | : HAL_REO_CMD_UPD0_PN_SIZE | :blob - 670f7f6d9acb993ade256155b02e4360dd982487 :blob + b409d9c8e2aa6fe9f0ab86ff7c92c85d2b2e58bb :--- sys/dev/ic/qwxreg.h :+++ sys/dev/ic/qwxreg.h :@@ -5208,6 +5208,7 @@ enum wmi_ap_ps_peer_param { : : #define WMI_KEY_PAIRWISE 0x00 : #define WMI_KEY_GROUP 0x01 :+#define WMI_KEY_TX_USAGE 0x02 : : #define WMI_CIPHER_NONE 0x0 /* clear key */ : #define WMI_CIPHER_WEP 0x1 : -- Fuch's Warning: If you actually look like your passport photo, you aren't well enough to travel.