On Sat, Aug 23, 2014 at 9:02 PM, Krishna Chaitanya <chaitanya.mgit@xxxxxxxxx> wrote: > On Fri, Aug 22, 2014 at 2:44 PM, Jouni Malinen <j@xxxxx> wrote: >> >> On Tue, Aug 12, 2014 at 12:21:54AM +0530, chaitanya.mgit@xxxxxxxxx wrote: >> > Enforce the check for protected field for all unicast >> > robust management frames. >> >> Why? This function is supposed to indicate whether the frame is a robust >> action frame and as such, has to have Protected bit set to one. If the >> sender (attacker) tries to send the frame unprotected, it will still >> need to be caught here. >> >> Rather than enforcing anything, this would add a significant security >> vulnerability by breaking PMF more or less completely. > > I agree jouni, we were using this API to figure out the length of the > crypto header (IV) to pass it to the HW crypto, so even for > unencrypted frames during the initial connection we were treating as > robust mgmt frames causing us trouble. >> >> >> > This removed the dependency on the driver to check for protected >> > bit, especially for those drivers who believed the API :-). >> >> Huh.. What is this driver referring to or what do you think the API is >> supposed to be doing? ieee80211_is_unicast_robust_mgmt_frame() is a >> static function within net/mac80211/rx.c and has only a single caller, >> so it cannot really be used by any driver.. > > Sorry, i overlooked the static in git, we have a custom kernel without > the static. > >> >> > diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c >> > @@ -569,6 +569,9 @@ static int ieee80211_is_unicast_robust_mgmt_frame(struct sk_buff *skb) >> > if (is_multicast_ether_addr(hdr->addr1)) >> > return 0; >> > >> > + if (!ieee80211_has_protected(hdr->frame_control)) >> > + return 0; >> > + >> > return ieee80211_is_robust_mgmt_frame(skb); >> >> This looks very incorrect. This would completely break >> ieee80211_drop_unencrypted_mgmt() and allow unprotected robust >> management frames to be processed. > > Ok i see it. robust mgmt and protected robust mgmt checks are > independently handled, but as the name suggested unicast robust mgmt > isn't it better to club those 2 checks together? > Johannes, Please drop this patch, i did not see that the function is static. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
