| Message ID | 20220324004816.6202-1-pkshih@realtek.com |
|---|---|
| State | New |
| Headers | show |
| Series | mac80211: consider Order bit to fill CCMP AAD | expand |
> -----Original Message----- > From: Johannes Berg <johannes@sipsolutions.net> > Sent: Tuesday, April 12, 2022 1:47 PM > To: Pkshih <pkshih@realtek.com> > Cc: linux-wireless@vger.kernel.org > Subject: Re: [PATCH] mac80211: consider Order bit to fill CCMP AAD > > On Tue, 2022-04-12 at 00:37 +0000, Pkshih wrote: > > After I fix my driver, I don't need this, but I think it is worth to > > have > > this patch. > > OK. But I guess that means no hurry. Yes. > > Yeah, thinking about that, I guess you're right. Maybe we can express > the 22 a bit better (some headerlen - 2 with a comment?), but I can look > at that when I apply the patch. How about replacing 22 by 2 + 6 + 6 + 6 + 2 because of "FC | A1 | A2 | A3 | SC | [A4] | [QC]" ? Ping-Ke
On Thu, Mar 24, 2022 at 08:48:16AM +0800, Ping-Ke Shih wrote: > Follow IEEE 802.11-21 that HTC subfield masked to 0 for all data frames > containing a QoS Control field. It also defines the AAD length depends on > QC and A4 fields, so change logic to determine length accordingly. > diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c > @@ -317,13 +317,12 @@ static void ccmp_special_blocks(struct sk_buff *skb, u8 *pn, u8 *b_0, u8 *aad) > /* > * Mask FC: zero subtype b4 b5 b6 (if not mgmt) > - * Retry, PwrMgt, MoreData; set Protected > + * Retry, PwrMgt, MoreData, Order (if Qos Data); set Protected > */ ... For completeness, we should really do the same got GCMP AAD which is identical to the CCMP AAD. In other words, these changes should be done in gcmp_special_blocks() as well. Those functions should really have next to identical implementation for the AAD part (nonce construction is different, though). There were already some differences in the design before.. Maybe all this AAD stuff should really be moved into a separate helper function that both CCMP and GCMP could use.
diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c index 7ed0d268aff2f..cd35ae76d5b78 100644 --- a/net/mac80211/wpa.c +++ b/net/mac80211/wpa.c @@ -317,13 +317,12 @@ static void ccmp_special_blocks(struct sk_buff *skb, u8 *pn, u8 *b_0, u8 *aad) __le16 mask_fc; int a4_included, mgmt; u8 qos_tid; - u16 len_a; - unsigned int hdrlen; + u16 len_a = 22; struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data; /* * Mask FC: zero subtype b4 b5 b6 (if not mgmt) - * Retry, PwrMgt, MoreData; set Protected + * Retry, PwrMgt, MoreData, Order (if Qos Data); set Protected */ mgmt = ieee80211_is_mgmt(hdr->frame_control); mask_fc = hdr->frame_control; @@ -333,14 +332,17 @@ static void ccmp_special_blocks(struct sk_buff *skb, u8 *pn, u8 *b_0, u8 *aad) mask_fc &= ~cpu_to_le16(0x0070); mask_fc |= cpu_to_le16(IEEE80211_FCTL_PROTECTED); - hdrlen = ieee80211_hdrlen(hdr->frame_control); - len_a = hdrlen - 2; a4_included = ieee80211_has_a4(hdr->frame_control); + if (a4_included) + len_a += 6; - if (ieee80211_is_data_qos(hdr->frame_control)) + if (ieee80211_is_data_qos(hdr->frame_control)) { qos_tid = ieee80211_get_tid(hdr); - else + mask_fc &= ~cpu_to_le16(IEEE80211_FCTL_ORDER); + len_a += 2; + } else { qos_tid = 0; + } /* In CCM, the initial vectors (IV) used for CTR mode encryption and CBC * mode authentication are not allowed to collide, yet both are derived
Follow IEEE 802.11-21 that HTC subfield masked to 0 for all data frames containing a QoS Control field. It also defines the AAD length depends on QC and A4 fields, so change logic to determine length accordingly. Signed-off-by: Ping-Ke Shih <pkshih@realtek.com> --- net/mac80211/wpa.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)