* Management frame protection and packet injection from hostapd @ 2008-06-16 14:33 Jouni Malinen 2008-06-16 14:53 ` Johannes Berg 0 siblings, 1 reply; 7+ messages in thread From: Jouni Malinen @ 2008-06-16 14:33 UTC (permalink / raw) To: linux-wireless I'm working on IEEE 802.11w support for mac80211 and my current code is in a state that allows both unicast and multicast/broadcast management frames to be protected (with CCMP and BIP/AES-CMAC, respectively). I'll clean up the patches a bit and send the first version for commenting in the near future. However, even before the patches are available, there is one area that could benefit from comments. hostapd is currently using a monitor interface to inject management frames. This seems to work fine for unencrypted frames, but at least BIP protection of broadcast/multicast management frames does not work without some hacks.. The problem here is that the default TX key is configured for the main data interface (wlan0) while the injected frame is coming from the monitor interface (mon.wlan0). ieee80211_tx_h_select_key() does not find the default key for management frames and consequently, the frame ends up going out without BIP processing. My current workaround is to configure IGTK for both wlan0 and mon.wlan0 in hostapd. This works, but is somewhat undesirable.. Would there be a better way for configuring the default keys (this could apply also for data frames, but at least for IGTK and management frames for the time being), so that they would apply both to frames generated by MLME code in mac80211 (if any; currently there is no broadcast/multicast management frames used and actually, no management frames generated in mac80211 MLME code in AP mode) and monitor interfaces (or something that would replace them for packet injection)? PS. I'm not sure whether this is a bug somewhere in my changes or whether the configuration of default keys for monitor interfaces triggered this, but mac80211 is now leaving behind almost empty debugfs ieee80211/phy# directories. The only thing remaining in that directory is an empty netdev:mon.wlan0 subdirectory, so it looks like something fails to remove said subdirectory and then the phy# directory. -- Jouni Malinen PGP id EFC895FA ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Management frame protection and packet injection from hostapd 2008-06-16 14:33 Management frame protection and packet injection from hostapd Jouni Malinen @ 2008-06-16 14:53 ` Johannes Berg 2008-06-16 15:34 ` Jouni Malinen 0 siblings, 1 reply; 7+ messages in thread From: Johannes Berg @ 2008-06-16 14:53 UTC (permalink / raw) To: Jouni Malinen; +Cc: linux-wireless [-- Attachment #1: Type: text/plain, Size: 3095 bytes --] On Mon, 2008-06-16 at 17:33 +0300, Jouni Malinen wrote: > I'm working on IEEE 802.11w support for mac80211 and my current code is > in a state that allows both unicast and multicast/broadcast management > frames to be protected (with CCMP and BIP/AES-CMAC, respectively). I'll > clean up the patches a bit and send the first version for commenting in > the near future. However, even before the patches are available, there > is one area that could benefit from comments. Neat. > hostapd is currently using a monitor interface to inject management > frames. This seems to work fine for unencrypted frames, but at least BIP > protection of broadcast/multicast management frames does not work > without some hacks.. The problem here is that the default TX key is > configured for the main data interface (wlan0) while the injected frame > is coming from the monitor interface (mon.wlan0). > ieee80211_tx_h_select_key() does not find the default key for management > frames and consequently, the frame ends up going out without BIP > processing. So just for me, BIP processing is done with the same IGTK? Unicast frames are handled by the per-station key, and as such work correctly, I'd guess? > My current workaround is to configure IGTK for both wlan0 and mon.wlan0 > in hostapd. This works, but is somewhat undesirable.. Would there be a > better way for configuring the default keys (this could apply also for > data frames, but at least for IGTK and management frames for the time > being), so that they would apply both to frames generated by MLME code > in mac80211 (if any; currently there is no broadcast/multicast > management frames used and actually, no management frames generated in > mac80211 MLME code in AP mode) and monitor interfaces (or something that > would replace them for packet injection)? Yeah that sounds like a hack. I guess it should work just like when we submit a unicast frame via monitor with the radiotap flag to indicate that we want encryption, only we should add logic to look up the default key to use if there is no peer/unicast key by the outgoing MAC address? OTOH, that would break for multi-SSID/single-BSSID scenarios I guess, so we probably need a way to indicate "this frame belongs to interface index N"? > I'm not sure whether this is a bug somewhere in my changes or whether > the configuration of default keys for monitor interfaces triggered this, > but mac80211 is now leaving behind almost empty debugfs ieee80211/phy# > directories. The only thing remaining in that directory is an empty > netdev:mon.wlan0 subdirectory, so it looks like something fails to > remove said subdirectory and then the phy# directory. I don't see any such problems, but if I were to venture a guess it's because of configuring keys on a monitor interface. It sounds like something sticks around within the netdev:mon.wlan0 subdirectory, then the code tries to delete the directory and only afterwards is the entry removed, leaving the directory (and the parent, of course) hanging there. johannes [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Management frame protection and packet injection from hostapd 2008-06-16 14:53 ` Johannes Berg @ 2008-06-16 15:34 ` Jouni Malinen 2008-06-16 16:44 ` Johannes Berg 0 siblings, 1 reply; 7+ messages in thread From: Jouni Malinen @ 2008-06-16 15:34 UTC (permalink / raw) To: Johannes Berg; +Cc: linux-wireless On Mon, Jun 16, 2008 at 04:53:11PM +0200, Johannes Berg wrote: > So just for me, BIP processing is done with the same IGTK? Unicast > frames are handled by the per-station key, and as such work correctly, > I'd guess? Yes, as long as the STA entry is found (and it should, in this case), the pairwise CCMP key is used both for data and management frames. I haven't done full testing yet, but I would expect this to work even when the unicast management frames comes from hostapd. Broadcast/multicast management frames are protected with BIP using IGTK. This is similar to how GTK is used with TKIP/CCMK for broadcast data frames, i.e., there is a default TX key index that the AP uses for the frames. Indexes 1..3 are used for data frames and 802.11w is using 4..5 for BIP (even though the key index space is actually completely separate from the one used with data frames). > Yeah that sounds like a hack. I guess it should work just like when we > submit a unicast frame via monitor with the radiotap flag to indicate > that we want encryption, only we should add logic to look up the default > key to use if there is no peer/unicast key by the outgoing MAC address? Yes, we already look for STA entry for unicast and that should work as long as the STA table is not per netdev, i.e., both data and monitor interface end using the same STA table. If no STA entry is found (multicast/broadcast), we look for a default TX key, but this is only done for the netdev that was used to send the frame (which is different between normal data interface and packet injection via monitor interface). If the monitor interface were to look for the default key (or well, keys, since there are now two; one for data, one for mgmt) from the data interface (somehow bound to the monitor iface?), that should work here. > OTOH, that would break for multi-SSID/single-BSSID scenarios I guess, so > we probably need a way to indicate "this frame belongs to interface > index N"? Yes, either the frame or maybe more easily the monitor interface would need to be bound to the interface that is used for key configuration. > I don't see any such problems, but if I were to venture a guess it's > because of configuring keys on a monitor interface. It sounds like > something sticks around within the netdev:mon.wlan0 subdirectory, then > the code tries to delete the directory and only afterwards is the entry > removed, leaving the directory (and the parent, of course) hanging > there. Yes, that sounds likely since the changes I did for debugfs were very trivial copies from CCMP/data-default-key processing. I'll debug this more and try to figure if there is need to re-order something or make the debugfs entry removal able to handle such a case. -- Jouni Malinen PGP id EFC895FA ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Management frame protection and packet injection from hostapd 2008-06-16 15:34 ` Jouni Malinen @ 2008-06-16 16:44 ` Johannes Berg 2008-06-16 16:47 ` Johannes Berg 0 siblings, 1 reply; 7+ messages in thread From: Johannes Berg @ 2008-06-16 16:44 UTC (permalink / raw) To: Jouni Malinen; +Cc: linux-wireless [-- Attachment #1: Type: text/plain, Size: 3235 bytes --] > Yes, as long as the STA entry is found (and it should, in this case), > the pairwise CCMP key is used both for data and management frames. I > haven't done full testing yet, but I would expect this to work even when > the unicast management frames comes from hostapd. Yeah, it should. > Broadcast/multicast management frames are protected with BIP using IGTK. > This is similar to how GTK is used with TKIP/CCMK for broadcast data > frames, i.e., there is a default TX key index that the AP uses for the > frames. Indexes 1..3 are used for data frames and 802.11w is using 4..5 > for BIP (even though the key index space is actually completely separate > from the one used with data frames). Ok, thanks for the explanation. > > Yeah that sounds like a hack. I guess it should work just like when we > > submit a unicast frame via monitor with the radiotap flag to indicate > > that we want encryption, only we should add logic to look up the default > > key to use if there is no peer/unicast key by the outgoing MAC address? > > Yes, we already look for STA entry for unicast and that should work as > long as the STA table is not per netdev, i.e., both data and monitor > interface end using the same STA table. If no STA entry is found > (multicast/broadcast), we look for a default TX key, but this is only > done for the netdev that was used to send the frame (which is different > between normal data interface and packet injection via monitor > interface). Good point about the STA tables there. I guess we really do need to bind the frame to a certain outgoing device to use that sdata state struct. > If the monitor interface were to look for the default key (or well, > keys, since there are now two; one for data, one for mgmt) from the data > interface (somehow bound to the monitor iface?), that should work here. > > > OTOH, that would break for multi-SSID/single-BSSID scenarios I guess, so > > we probably need a way to indicate "this frame belongs to interface > > index N"? > > Yes, either the frame or maybe more easily the monitor interface would > need to be bound to the interface that is used for key configuration. Yeah, either would work, for per-frame we'd have to extend radiotap but it would probably be better as it would allow hostapd to use the same mon.wlan0 interface for all BSSes. > > I don't see any such problems, but if I were to venture a guess it's > > because of configuring keys on a monitor interface. It sounds like > > something sticks around within the netdev:mon.wlan0 subdirectory, then > > the code tries to delete the directory and only afterwards is the entry > > removed, leaving the directory (and the parent, of course) hanging > > there. > > Yes, that sounds likely since the changes I did for debugfs were very > trivial copies from CCMP/data-default-key processing. I'll debug this > more and try to figure if there is need to re-order something or make > the debugfs entry removal able to handle such a case. Ok. I don't know right now, and it does seem to work correctly here, but maybe it doesn't when the application doesn't explicitly remove the key or something, I'll take a look. johannes [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Management frame protection and packet injection from hostapd 2008-06-16 16:44 ` Johannes Berg @ 2008-06-16 16:47 ` Johannes Berg 2008-06-17 7:32 ` Jouni Malinen 0 siblings, 1 reply; 7+ messages in thread From: Johannes Berg @ 2008-06-16 16:47 UTC (permalink / raw) To: Jouni Malinen; +Cc: linux-wireless [-- Attachment #1: Type: text/plain, Size: 624 bytes --] > > Yes, that sounds likely since the changes I did for debugfs were very > > trivial copies from CCMP/data-default-key processing. I'll debug this > > more and try to figure if there is need to re-order something or make > > the debugfs entry removal able to handle such a case. > > Ok. I don't know right now, and it does seem to work correctly here, but > maybe it doesn't when the application doesn't explicitly remove the key > or something, I'll take a look. It's definitely done, from ieee80211_free_keys() calling ieee80211_debugfs_key_remove_default(), maybe you missed something there? johannes [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Management frame protection and packet injection from hostapd 2008-06-16 16:47 ` Johannes Berg @ 2008-06-17 7:32 ` Jouni Malinen 2008-06-17 8:50 ` Johannes Berg 0 siblings, 1 reply; 7+ messages in thread From: Jouni Malinen @ 2008-06-17 7:32 UTC (permalink / raw) To: Johannes Berg; +Cc: linux-wireless On Mon, Jun 16, 2008 at 06:47:48PM +0200, Johannes Berg wrote: > > > Yes, that sounds likely since the changes I did for debugfs were very > > > trivial copies from CCMP/data-default-key processing. I'll debug this > > > more and try to figure if there is need to re-order something or make > > > the debugfs entry removal able to handle such a case. > > > > Ok. I don't know right now, and it does seem to work correctly here, but > > maybe it doesn't when the application doesn't explicitly remove the key > > or something, I'll take a look. > > It's definitely done, from ieee80211_free_keys() calling > ieee80211_debugfs_key_remove_default(), maybe you missed something > there? I did.. I added another set of functions for default management key and did not remember to call the removal function from ieee80211_free_keys(). However, adding that call did not change anything. It looks like we end up trying to remove the netdev directory in debugfs before removing the default key symlinks. Consequently, debugfs_remove() fails since there is still a file in the directory. This is what happens when removing the monitor interface: cfg.c: ieee80211_del_iface() -> iface.c: ieee80211_if_remove() -> iface.c: __ieee80211_if_del() -> debugfs_netdev.c: ieee80211_debugfs_remove_netdev() [too early; symlink still there] unregister_netdevice(dev) -> [dev->uninit] iface.c: ieee80211_if_reinit() -> key.c: ieee80211_free_keys() -> debugfs_key.c: ieee80211_debugfs_key_remove_default() Any idea how to fix this? Why is ieee80211_debugfs_remove_netdev() call in __ieee80211_if_del()? Could it be moved into ieee80211_if_reinit(), so that it would happen only after the ieee80211_free_keys() call? Since ieee80211_if_reinit() is called from other places, too, it might be cleaner to define a new dev->uninit function that is a wrapper for call to ieee80211_if_reinit() followed by call to ieee80211_debugfs_remove_netdev().. However, since ieee80211_if_reinit() calls ieee80211_if_sdata_deinit(), it might be necessary to call ieee80211_debugfs_remove_netdev() before this call (or from it?); I did not yet look into details of what would be the required order for these. -- Jouni Malinen PGP id EFC895FA ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Management frame protection and packet injection from hostapd 2008-06-17 7:32 ` Jouni Malinen @ 2008-06-17 8:50 ` Johannes Berg 0 siblings, 0 replies; 7+ messages in thread From: Johannes Berg @ 2008-06-17 8:50 UTC (permalink / raw) To: Jouni Malinen; +Cc: linux-wireless [-- Attachment #1: Type: text/plain, Size: 2233 bytes --] > I did.. I added another set of functions for default management key and > did not remember to call the removal function from > ieee80211_free_keys(). However, adding that call did not change > anything. Ok. > It looks like we end up trying to remove the netdev directory in debugfs > before removing the default key symlinks. Consequently, debugfs_remove() > fails since there is still a file in the directory. This is what happens > when removing the monitor interface: > > cfg.c: ieee80211_del_iface() -> > iface.c: ieee80211_if_remove() -> > iface.c: __ieee80211_if_del() -> > debugfs_netdev.c: ieee80211_debugfs_remove_netdev() > [too early; symlink still there] > unregister_netdevice(dev) -> [dev->uninit] > iface.c: ieee80211_if_reinit() -> > key.c: ieee80211_free_keys() -> > debugfs_key.c: ieee80211_debugfs_key_remove_default() > > > Any idea how to fix this? Hmm, no, no idea at all. To be honest, I don't fully understand interface lifetime rules and I think many of those functions are probably misnamed. I.e. why is the _reinit function assigned to the ->uninit callback? Much of that either comes from the original Devicescape code or what Jiri did to it, and I never bothered to clean it up. > Why is ieee80211_debugfs_remove_netdev() call > in __ieee80211_if_del()? I don't really know. Probably because that's where Jiri had the sysfs code I converted to debugfs ;) > Could it be moved into ieee80211_if_reinit(), > so that it would happen only after the ieee80211_free_keys() call? That's well possible, I guess you'd have to try. > Since > ieee80211_if_reinit() is called from other places, too, it might be > cleaner to define a new dev->uninit function that is a wrapper for call > to ieee80211_if_reinit() followed by call to > ieee80211_debugfs_remove_netdev().. However, since ieee80211_if_reinit() > calls ieee80211_if_sdata_deinit(), it might be necessary to call > ieee80211_debugfs_remove_netdev() before this call (or from it?); I did > not yet look into details of what would be the required order for these. I really don't know either. If you feel motivated to clean up the interface handling I'd appreciate it. johannes [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2008-06-17 8:51 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-06-16 14:33 Management frame protection and packet injection from hostapd Jouni Malinen 2008-06-16 14:53 ` Johannes Berg 2008-06-16 15:34 ` Jouni Malinen 2008-06-16 16:44 ` Johannes Berg 2008-06-16 16:47 ` Johannes Berg 2008-06-17 7:32 ` Jouni Malinen 2008-06-17 8:50 ` Johannes Berg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox