Hi Stefan,
I'm not yet ready to test new diffs, but I was going through old
wireless stack related emails and was wondering did below patch got
committed? Per my git / cvs search I don't think so. Is below diff
still relevant or can below change be ignored?
On Thu, Jul 01, 2021 at 12:23:33PM +0200, Stefan Sperling wrote:
> Some wifi drivers defer installation of keys into a process context.
> There was a noticable race in the past which was fixed thanks to krw@
> where link state was set to UP before key installation had completed in
> hardware. This race could result in dhclient failing to get a lease.
>
> Other races remain. These concern state which controls encryption of
> frames in the output path of net80211 (TXRXPROT flag), and marks the
> pairwise key as installed (PTKDONE state) which in turn affects our
> acceptance of group key updates sent by the AP. While either of these
> flags is set and hardware is not configured for encryption we are running
> in an inconsistent state. I don't know if this causes problems in practice
> but we should be avoiding such races. The fix is fairly mechanical and
> essentially defers modification of relevant flags from net80211 into the
> affected drivers.
>
> Affected drivers are: iwx, bfwm, athn (USB only!), urtwn, run, rsu, otus.
> I have tested on iwx, athn, urtwn, and run. More tests are welcome.
>
> ok?
>
> diff 86b3b873864f7e98e688c12d8e455803ff30eadb
> 27f93922f2b250b31e9534b457665df3dcf58794
> blob - ea245574f624ada1b8d0c9e8e8a0653fb2c3adc4
> blob + 0bac2cda2290c4a44de8f5b8110a4399f75de88d
> --- sys/dev/ic/bwfm.c
> +++ sys/dev/ic/bwfm.c
> @@ -2714,7 +2714,13 @@ bwfm_set_key_cb(struct bwfm_softc *sc, void *arg)
> wsec |= wsec_enable;
> bwfm_fwvar_var_set_int(sc, "wsec", wsec);
>
> + if ((k->k_flags & IEEE80211_KEY_GROUP) == 0 &&
> + ni->ni_rsn_supp_state == RSNA_SUPP_PTKNEGOTIATING)
> + ni->ni_rsn_supp_state = RSNA_SUPP_PTKDONE;
> +
> if (sc->sc_key_tasks == 0) {
> + if (k->k_flags & IEEE80211_KEY_SECURE)
> + ni->ni_flags |= IEEE80211_NODE_TXRXPROT;
> DPRINTF(("%s: marking port %s valid\n", DEVNAME(sc),
> ether_sprintf(cmd->ni->ni_macaddr)));
> cmd->ni->ni_port_valid = 1;
> blob - bdf8ce3e1afa332f698e3dc56af77e6acb4f8689
> blob + 5be1d92ee862f584df5901fe54ebe30e0c937d36
> --- sys/dev/pci/if_iwx.c
> +++ sys/dev/pci/if_iwx.c
> @@ -6747,12 +6747,17 @@ iwx_add_sta_key(struct iwx_softc *sc, int sta_id, stru
> return err;
> }
>
> - if (k->k_flags & IEEE80211_KEY_GROUP)
> + if (k->k_flags & IEEE80211_KEY_GROUP) {
> in->in_flags |= IWX_NODE_FLAG_HAVE_GROUP_KEY;
> - else
> + } else {
> in->in_flags |= IWX_NODE_FLAG_HAVE_PAIRWISE_KEY;
> + if (ni->ni_rsn_supp_state == RSNA_SUPP_PTKNEGOTIATING)
> + ni->ni_rsn_supp_state = RSNA_SUPP_PTKDONE;
> + }
>
> if ((in->in_flags & want_keymask) == want_keymask) {
> + if (k->k_flags & IEEE80211_KEY_SECURE)
> + ni->ni_flags |= IEEE80211_NODE_TXRXPROT;
> DPRINTF(("marking port %s valid\n",
> ether_sprintf(ni->ni_macaddr)));
> ni->ni_port_valid = 1;
> blob - 08ffda8c9aaede9c901d97a43bf2873591e8df19
> blob + 0c141fc5df7ba1a774af98a59a2b718bdcc721bb
> --- sys/dev/usb/if_athn_usb.c
> +++ sys/dev/usb/if_athn_usb.c
> @@ -1662,7 +1662,12 @@ athn_usb_set_key_cb(struct athn_usb_softc *usc, void *
> s = splnet();
> athn_usb_write_barrier(&usc->sc_sc);
> athn_set_key(ic, cmd->ni, cmd->key);
> + if ((cmd->key->k_flags & IEEE80211_KEY_GROUP) == 0 &&
> + cmd->ni->ni_rsn_supp_state == RSNA_SUPP_PTKNEGOTIATING)
> + cmd->ni->ni_rsn_supp_state = RSNA_SUPP_PTKDONE;
> if (usc->sc_key_tasks == 0) {
> + if (cmd->key->k_flags & IEEE80211_KEY_SECURE)
> + cmd->ni->ni_flags |= IEEE80211_NODE_TXRXPROT;
> DPRINTF(("marking port %s valid\n",
> ether_sprintf(cmd->ni->ni_macaddr)));
> cmd->ni->ni_port_valid = 1;
> blob - bb220831ffc852347afdcda18c9bca0589d6d939
> blob + 8e426e9cab0b389f2e1260139ad451b30e10287e
> --- sys/dev/usb/if_otus.c
> +++ sys/dev/usb/if_otus.c
> @@ -2116,7 +2116,12 @@ otus_set_key_cb(struct otus_softc *sc, void *arg)
> memcpy(key.key, k->k_key + 16, 16);
> (void)otus_cmd(sc, AR_CMD_EKEY, &key, sizeof key, NULL);
>
> + if ((k->k_flags & IEEE80211_KEY_GROUP) == 0 &&
> + cmd->ni->ni_rsn_supp_state == RSNA_SUPP_PTKNEGOTIATING)
> + cmd->ni->ni_rsn_supp_state = RSNA_SUPP_PTKDONE;
> if (sc->sc_key_tasks == 0) {
> + if (k->k_flags & IEEE80211_KEY_SECURE)
> + cmd->ni->ni_flags |= IEEE80211_NODE_TXRXPROT;
> DPRINTF(("marking port %s valid\n",
> ether_sprintf(cmd->ni->ni_macaddr)));
> cmd->ni->ni_port_valid = 1;
> blob - 929d8f161d50cd294569dd74e0a4e2980e5607ca
> blob + 5e30f1b680b1b58c020ba9627aa5a8776aa1b3a4
> --- sys/dev/usb/if_rsu.c
> +++ sys/dev/usb/if_rsu.c
> @@ -940,7 +940,12 @@ rsu_set_key_cb(struct rsu_softc *sc, void *arg)
> memcpy(key.key, k->k_key, MIN(k->k_len, sizeof(key.key)));
> (void)rsu_fw_cmd(sc, R92S_CMD_SET_KEY, &key, sizeof(key));
>
> + if ((k->k_flags & IEEE80211_KEY_GROUP) == 0 &&
> + cmd->ni->ni_rsn_supp_state == RSNA_SUPP_PTKNEGOTIATING)
> + cmd->ni->ni_rsn_supp_state = RSNA_SUPP_PTKDONE;
> if (sc->sc_key_tasks == 0) {
> + if (k->k_flags & IEEE80211_KEY_SECURE)
> + cmd->ni->ni_flags |= IEEE80211_NODE_TXRXPROT;
> DPRINTF(("marking port %s valid\n",
> ether_sprintf(cmd->ni->ni_macaddr)));
> cmd->ni->ni_port_valid = 1;
> blob - 7759d6e0cea12b48a1373d90942b20a8d55bd7fb
> blob + 70acde2498e9adcadac12fcf3c6886ca7a753ef9
> --- sys/dev/usb/if_run.c
> +++ sys/dev/usb/if_run.c
> @@ -2017,7 +2017,12 @@ run_set_key_cb(struct run_softc *sc, void *arg)
> run_write(sc, RT2860_WCID_ATTR(wcid), attr);
> }
>
> + if ((k->k_flags & IEEE80211_KEY_GROUP) == 0 &&
> + cmd->ni->ni_rsn_supp_state == RSNA_SUPP_PTKNEGOTIATING)
> + cmd->ni->ni_rsn_supp_state = RSNA_SUPP_PTKDONE;
> if (sc->sc_key_tasks == 0) {
> + if (k->k_flags & IEEE80211_KEY_SECURE)
> + cmd->ni->ni_flags |= IEEE80211_NODE_TXRXPROT;
> cmd->ni->ni_port_valid = 1;
> ieee80211_set_link_state(ic, LINK_STATE_UP);
> }
> blob - c2133041168e826909415759d2009bd74dd48aee
> blob + 82c02d91a36e5ce5d91f78f230789f348c09f5dd
> --- sys/dev/usb/if_urtwn.c
> +++ sys/dev/usb/if_urtwn.c
> @@ -1062,7 +1062,12 @@ urtwn_set_key_cb(struct urtwn_softc *sc, void *arg)
> sc->sc_key_tasks--;
>
> if (rtwn_set_key(ic, cmd->ni, &cmd->key) == 0) {
> + if ((cmd->key.k_flags & IEEE80211_KEY_GROUP) == 0 &&
> + cmd->ni->ni_rsn_supp_state == RSNA_SUPP_PTKNEGOTIATING)
> + cmd->ni->ni_rsn_supp_state = RSNA_SUPP_PTKDONE;
> if (sc->sc_key_tasks == 0) {
> + if (cmd->key.k_flags & IEEE80211_KEY_SECURE)
> + cmd->ni->ni_flags |= IEEE80211_NODE_TXRXPROT;
> DPRINTF(("marking port %s valid\n",
> ether_sprintf(cmd->ni->ni_macaddr)));
> cmd->ni->ni_port_valid = 1;
> blob - 748b44b3f8c309dfd40b347ac408f234588b404b
> blob + e14b46a0ff9c9b824b59a93c866b955ce3338c24
> --- sys/net80211/ieee80211_crypto.h
> +++ sys/net80211/ieee80211_crypto.h
> @@ -79,6 +79,7 @@ struct ieee80211_key {
> #define IEEE80211_KEY_TX 0x00000002 /* Tx+Rx */
> #define IEEE80211_KEY_IGTK 0x00000004 /* integrity group key */
> #define IEEE80211_KEY_SWCRYPTO 0x00000080 /* loaded for software
> crypto */
> +#define IEEE80211_KEY_SECURE 0x00000100 /* initial key exchange done */
>
> u_int k_len;
> u_int64_t k_rsc[IEEE80211_NUM_TID];
> blob - daf550f591fdf675e073f82a968efbdbb49fdb46
> blob + 19413bef0b9c3e5df41bb3b8bcc17c88ae331e47
> --- sys/net80211/ieee80211_pae_input.c
> +++ sys/net80211/ieee80211_pae_input.c
> @@ -564,9 +564,12 @@ ieee80211_recv_4way_msg3(struct ieee80211com *ic,
> k->k_rsc[0] = prsc;
> k->k_len = keylen;
> memcpy(k->k_key, ni->ni_ptk.tk, k->k_len);
> + if (info & (EAPOL_KEY_INSTALL | EAPOL_KEY_SECURE))
> + k->k_flags |= IEEE80211_KEY_SECURE;
> /* install the PTK */
> switch ((*ic->ic_set_key)(ic, ni, k)) {
> case 0:
> + ni->ni_rsn_supp_state = RSNA_SUPP_PTKDONE;
> break;
> case EBUSY:
> deferlink = 1;
> @@ -576,8 +579,9 @@ ieee80211_recv_4way_msg3(struct ieee80211com *ic,
> goto deauth;
> }
> ni->ni_flags &= ~IEEE80211_NODE_RSN_NEW_PTK;
> - ni->ni_flags &= ~IEEE80211_NODE_TXRXPROT;
> - ni->ni_flags |= IEEE80211_NODE_RXPROT;
> + ni->ni_flags &= ~IEEE80211_NODE_TXPROT;
> + if (deferlink == 0)
> + ni->ni_flags |= IEEE80211_NODE_RXPROT;
> } else if (ni->ni_rsncipher != IEEE80211_CIPHER_USEGROUP)
> printf("%s: unexpected pairwise key update received from %s\n",
> ic->ic_if.if_xname, ether_sprintf(ni->ni_macaddr));
> @@ -604,6 +608,8 @@ ieee80211_recv_4way_msg3(struct ieee80211com *ic,
> k->k_rsc[0] = LE_READ_6(key->rsc);
> k->k_len = keylen;
> memcpy(k->k_key, >k[8], k->k_len);
> + if (info & EAPOL_KEY_SECURE)
> + k->k_flags |= IEEE80211_KEY_SECURE;
> /* install the GTK */
> switch ((*ic->ic_set_key)(ic, ni, k)) {
> case 0:
> @@ -641,6 +647,8 @@ ieee80211_recv_4way_msg3(struct ieee80211com *ic,
> k->k_mgmt_rsc = LE_READ_6(&igtk[8]); /* IPN */
> k->k_len = 16;
> memcpy(k->k_key, &igtk[14], k->k_len);
> + if (info & EAPOL_KEY_SECURE)
> + k->k_flags |= IEEE80211_KEY_SECURE;
> /* install the IGTK */
> switch ((*ic->ic_set_key)(ic, ni, k)) {
> case 0:
> @@ -654,11 +662,9 @@ ieee80211_recv_4way_msg3(struct ieee80211com *ic,
> }
> }
> }
> - if (info & EAPOL_KEY_INSTALL)
> + if (deferlink == 0 && (info & (EAPOL_KEY_INSTALL | EAPOL_KEY_SECURE)))
> ni->ni_flags |= IEEE80211_NODE_TXRXPROT;
> -
> if (info & EAPOL_KEY_SECURE) {
> - ni->ni_flags |= IEEE80211_NODE_TXRXPROT;
> #ifndef IEEE80211_STA_ONLY
> if (ic->ic_opmode != IEEE80211_M_IBSS ||
> ++ni->ni_key_count == 2)
> @@ -814,7 +820,7 @@ ieee80211_recv_rsn_group_msg1(struct ieee80211com *ic,
> const u_int8_t *frm, *efrm;
> const u_int8_t *gtk, *igtk;
> u_int16_t info, kid, reason = 0;
> - int keylen;
> + int keylen, deferlink = 0;
>
> #ifndef IEEE80211_STA_ONLY
> if (ic->ic_opmode != IEEE80211_M_STA &&
> @@ -897,10 +903,14 @@ ieee80211_recv_rsn_group_msg1(struct ieee80211com *ic,
> k->k_rsc[0] = LE_READ_6(key->rsc);
> k->k_len = keylen;
> memcpy(k->k_key, >k[8], k->k_len);
> + if (info & EAPOL_KEY_SECURE)
> + k->k_flags |= IEEE80211_KEY_SECURE;
> /* install the GTK */
> switch ((*ic->ic_set_key)(ic, ni, k)) {
> case 0:
> + break;
> case EBUSY:
> + deferlink = 1;
> break;
> default:
> reason = IEEE80211_REASON_AUTH_LEAVE;
> @@ -929,10 +939,14 @@ ieee80211_recv_rsn_group_msg1(struct ieee80211com *ic,
> k->k_mgmt_rsc = LE_READ_6(&igtk[8]); /* IPN */
> k->k_len = 16;
> memcpy(k->k_key, &igtk[14], k->k_len);
> + if (info & EAPOL_KEY_SECURE)
> + k->k_flags |= IEEE80211_KEY_SECURE;
> /* install the IGTK */
> switch ((*ic->ic_set_key)(ic, ni, k)) {
> case 0:
> + break;
> case EBUSY:
> + deferlink = 1;
> break;
> default:
> reason = IEEE80211_REASON_AUTH_LEAVE;
> @@ -946,10 +960,12 @@ ieee80211_recv_rsn_group_msg1(struct ieee80211com *ic,
> ++ni->ni_key_count == 2)
> #endif
> {
> - DPRINTF(("marking port %s valid\n",
> - ether_sprintf(ni->ni_macaddr)));
> - ni->ni_port_valid = 1;
> - ieee80211_set_link_state(ic, LINK_STATE_UP);
> + if (deferlink == 0) {
> + DPRINTF(("marking port %s valid\n",
> + ether_sprintf(ni->ni_macaddr)));
> + ni->ni_port_valid = 1;
> + ieee80211_set_link_state(ic, LINK_STATE_UP);
> + }
> ni->ni_assoc_fail = 0;
> }
> }
> @@ -979,7 +995,7 @@ ieee80211_recv_wpa_group_msg1(struct ieee80211com *ic,
> struct ieee80211_key *k;
> u_int16_t info;
> u_int8_t kid;
> - int keylen;
> + int keylen, deferlink = 0;
> const uint8_t *gtk;
>
> #ifndef IEEE80211_STA_ONLY
> @@ -1037,10 +1053,14 @@ ieee80211_recv_wpa_group_msg1(struct ieee80211com *ic,
> k->k_rsc[0] = LE_READ_6(key->rsc);
> k->k_len = keylen;
> memcpy(k->k_key, gtk, k->k_len);
> + if (info & EAPOL_KEY_SECURE)
> + k->k_flags |= IEEE80211_KEY_SECURE;
> /* install the GTK */
> switch ((*ic->ic_set_key)(ic, ni, k)) {
> case 0:
> + break;
> case EBUSY:
> + deferlink = 1;
> break;
> default:
> IEEE80211_SEND_MGMT(ic, ni,
> IEEE80211_FC0_SUBTYPE_DEAUTH,
> @@ -1055,10 +1075,12 @@ ieee80211_recv_wpa_group_msg1(struct ieee80211com *ic,
> ++ni->ni_key_count == 2)
> #endif
> {
> - DPRINTF(("marking port %s valid\n",
> - ether_sprintf(ni->ni_macaddr)));
> - ni->ni_port_valid = 1;
> - ieee80211_set_link_state(ic, LINK_STATE_UP);
> + if (deferlink == 0) {
> + DPRINTF(("marking port %s valid\n",
> + ether_sprintf(ni->ni_macaddr)));
> + ni->ni_port_valid = 1;
> + ieee80211_set_link_state(ic, LINK_STATE_UP);
> + }
> ni->ni_assoc_fail = 0;
> }
> }
> blob - 35a00ff8b6b7315718558ad26178d8646308c6dc
> blob + 2a48158c2c93da086e7860a69a01e944991797c9
> --- sys/net80211/ieee80211_pae_output.c
> +++ sys/net80211/ieee80211_pae_output.c
> @@ -444,7 +444,6 @@ ieee80211_send_4way_msg4(struct ieee80211com *ic, stru
> struct mbuf *m;
> u_int16_t info;
>
> - ni->ni_rsn_supp_state = RSNA_SUPP_PTKDONE;
> m = ieee80211_get_eapol_key(M_DONTWAIT, MT_DATA, 0);
> if (m == NULL)
> return ENOMEM;
>
--
Regards,
Mikolaj
