| Message ID | 1375968116-24331-1-git-send-email-ja@anyfi.net (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Thu, 2013-08-08 at 15:21 +0200, Johan Almbladh wrote: > This patch enables power save processing for encrypted frames even if the > encryption key is not set. This is a requirement when implementing split-MAC > systems like Anyfi.net [1] and CAPWAP [2] on mac80211 using monitor frame > injection and reception. I have no idea what these are, nor do I actually want to care much... You presumably use Felix's active monitor mode? > The mac80211 RX handlers are reordered slightly so > that the power save handler is invoked before the decryption handler. > > The patch is minimal in the sense that it provides the required functionality > with a minimal change, but I am open to suggestions if this change is too > intrusive. Please let me know what you think. I think you should ask yourself if this makes sense in the normal wifi context... :-) It actually seems like it *does* make sense, so it should have an appropriate description for that, but I'm a bit worried about IBSS in sta_process. Also I've tried to keep the code in the file sequential, so this patch should be moving ieee80211_rx_h_decrypt() itself as well. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 9, 2013 at 3:07 PM, Johannes Berg <johannes@sipsolutions.net> wrote: > > On Thu, 2013-08-08 at 15:21 +0200, Johan Almbladh wrote: > > This patch enables power save processing for encrypted frames even if the > > encryption key is not set. This is a requirement when implementing split-MAC > > systems like Anyfi.net [1] and CAPWAP [2] on mac80211 using monitor frame > > injection and reception. > > I have no idea what these are, nor do I actually want to care much... Anyfi.net is a dynamic split-mac system where the security part of the 802.11 stack is located in your home router and the realtime part is handled by any router or AP that happens to be near your current location. The two parts are connected dynamically via a UDP tunnel that carries raw encrypted 802.11 frames, forming a complete 802.11 stack that provides your home Wi-Fi wherever you are. In a community Wi-Fi deployment, the users get secure Wi-Fi access with automatic sign-on via their home Wi-Fi network which is simply available everywhere. Should you find the concept interesting there is quite extensive technical documentation at http://anyfi.net/documentation#architecture :-) > You presumably use Felix's active monitor mode? I use a monitor socket in a userspace daemon. The daemon receives encrypted 802.11 frames with radiotap encapsulation on this monitor socket. It also injects encrypted 802.11 frames with radiotap encapsulation by transmitting them on the same socket. I believe this is the way hostapd used to handle transmission of management and EAPOL frames before they switched to nl80211. > > The mac80211 RX handlers are reordered slightly so > > that the power save handler is invoked before the decryption handler. > > > > The patch is minimal in the sense that it provides the required functionality > > with a minimal change, but I am open to suggestions if this change is too > > intrusive. Please let me know what you think. > > I think you should ask yourself if this makes sense in the normal wifi > context... :-) You are right about that, but I think this little feature can be added without affecting the normal operation :-) To be honest, mac80211 has all the interfaces required for any split-mac implementation, thanks to the mac80211/hostapd partitioning. The *only* thing missing is the ability to handle AP power save processing without handling the encryption... > It actually seems like it *does* make sense, so it should have an > appropriate description for that, but I'm a bit worried about IBSS in > sta_process. The patch enables power save processing even if there is no unicast key set, but *also* if key is set but the decryption failed. This is what I meant with "intrusive". The IBSS updating in sta_process will also run in this case, but the STA is still required to be authorized. It's possible to narrow it down to only affect the case where no encryption key is set: * Keep the RX handlers in their original order * Don't drop frames where rx->key is NULL in ieee80211_rx_h_decrypt. Instead, mark the frame with a flag * Drop any marked frames at the end of ieee80211_rx_h_sta_process with RX_DROP_MONITOR I can prepare a new patch if you prefer this solution. > Also I've tried to keep the code in the file sequential, so this patch > should be moving ieee80211_rx_h_decrypt() itself as well. I'll make sure to put them in the right order. Johan -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
I would prefer my original solution that puts the decryption handler after the sta_process handler. The code is cleaner since we avoid the extra flag and the coupling between decrypt and sta_process. My conclusion is that the change is correct, see below. ieee80211_rx_h_check_more_data, ieee80211_rx_h_uapsd_and_pspoll: The MOREDATA and PM bits are not protected by the encryption MIC. It should be valid to process those bits regardless of the decryption outcome. ieee80211_rx_h_sta_process: The updating of last_rx for an IBSS STA is conditional on the STA being AUTHORIZED. That state is the same regardless of whether the updating is done before or after decryption. The main argument that the MOREDATA and PM bits are not protected by the encryption and are therefore independent. You can always send a spoofed NULLFUNC frame to an RSN AP or STA and have the PM and MOREDATA bits processed accordingly. My change allows data frames that could not be decrypted to be processed similar to NULLFUNC frames. I have run mac80211 in STA and AP mode with this change for quite some time now without any problems. I will run it in IBSS mode also as you suggest. Provided that my IBSS tests pass, should I send you a final patch that changes the RX handler order and their locations in the file? Johan On Sat, Aug 10, 2013 at 1:31 PM, Johan Almbladh <ja@anyfi.net> wrote: > On Fri, Aug 9, 2013 at 3:07 PM, Johannes Berg <johannes@sipsolutions.net> wrote: >> >> On Thu, 2013-08-08 at 15:21 +0200, Johan Almbladh wrote: >> > This patch enables power save processing for encrypted frames even if the >> > encryption key is not set. This is a requirement when implementing split-MAC >> > systems like Anyfi.net [1] and CAPWAP [2] on mac80211 using monitor frame >> > injection and reception. >> >> I have no idea what these are, nor do I actually want to care much... > > Anyfi.net is a dynamic split-mac system where the security part of the > 802.11 stack is located in your home router and the realtime part is > handled by any router or AP that happens to be near your current > location. The two parts are connected dynamically via a UDP tunnel > that carries raw encrypted 802.11 frames, forming a complete 802.11 > stack that provides your home Wi-Fi wherever you are. In a community > Wi-Fi deployment, the users get secure Wi-Fi access with automatic > sign-on via their home Wi-Fi network which is simply available > everywhere. > > Should you find the concept interesting there is quite extensive > technical documentation at http://anyfi.net/documentation#architecture > :-) > > >> You presumably use Felix's active monitor mode? > > I use a monitor socket in a userspace daemon. The daemon receives > encrypted 802.11 frames with radiotap encapsulation on this monitor > socket. It also injects encrypted 802.11 frames with radiotap > encapsulation by transmitting them on the same socket. I believe this > is the way hostapd used to handle transmission of management and EAPOL > frames before they switched to nl80211. > > >> > The mac80211 RX handlers are reordered slightly so >> > that the power save handler is invoked before the decryption handler. >> > >> > The patch is minimal in the sense that it provides the required functionality >> > with a minimal change, but I am open to suggestions if this change is too >> > intrusive. Please let me know what you think. >> >> I think you should ask yourself if this makes sense in the normal wifi >> context... :-) > > You are right about that, but I think this little feature can be added > without affecting the normal operation :-) To be honest, mac80211 has > all the interfaces required for any split-mac implementation, thanks > to the mac80211/hostapd partitioning. The *only* thing missing is the > ability to handle AP power save processing without handling the > encryption... > > >> It actually seems like it *does* make sense, so it should have an >> appropriate description for that, but I'm a bit worried about IBSS in >> sta_process. > > The patch enables power save processing even if there is no unicast > key set, but *also* if key is set but the decryption failed. This is > what I meant with "intrusive". The IBSS updating in sta_process will > also run in this case, but the STA is still required to be authorized. > > It's possible to narrow it down to only affect the case where no > encryption key is set: > > * Keep the RX handlers in their original order > * Don't drop frames where rx->key is NULL in ieee80211_rx_h_decrypt. > Instead, mark the frame with a flag > * Drop any marked frames at the end of ieee80211_rx_h_sta_process with > RX_DROP_MONITOR > > I can prepare a new patch if you prefer this solution. > >> Also I've tried to keep the code in the file sequential, so this patch >> should be moving ieee80211_rx_h_decrypt() itself as well. > > I'll make sure to put them in the right order. > > Johan -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 2013-08-11 at 22:19 +0200, Johan Almbladh wrote: > I would prefer my original solution that puts the decryption handler > after the sta_process handler. The code is cleaner since we avoid the > extra flag and the coupling between decrypt and sta_process. My > conclusion is that the change is correct, see below. I actually agree and think that it makes our implementation of 802.11 more robust, but I think that should be said in the changelog and be carefully analysed (as I guess you've done below) > ieee80211_rx_h_check_more_data, ieee80211_rx_h_uapsd_and_pspoll: > The MOREDATA and PM bits are not protected by the encryption MIC. It > should be valid to process those bits regardless of the decryption > outcome. Agree, those seem pretty clear. > ieee80211_rx_h_sta_process: > The updating of last_rx for an IBSS STA is conditional on the STA > being AUTHORIZED. That state is the same regardless of whether the > updating is done before or after decryption. That's true, but my concern here was about the reboot problem: * two stations join a secure mesh and communicate * one of them reboots, but quickly rejoins the mesh In this case sometimes it gets hard to detect the situation and keeping the other station alive. OTOH, it already happens for beacon frames which aren't encrypted anyway, so I think it shouldn't matter. Antonio? > The main argument that the MOREDATA and PM bits are not protected by > the encryption and are therefore independent. You can always send a > spoofed NULLFUNC frame to an RSN AP or STA and have the PM and > MOREDATA bits processed accordingly. My change allows data frames that > could not be decrypted to be processed similar to NULLFUNC frames. Right, of course. > I have run mac80211 in STA and AP mode with this change for quite some > time now without any problems. I will run it in IBSS mode also as you > suggest. > > Provided that my IBSS tests pass, should I send you a final patch that > changes the RX handler order and their locations in the file? I think the IBSS test would be rather difficult, but yes, I think you've convinced me that the patch is fine - please do resend it. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 12, 2013 at 06:01:15PM +0200, Johannes Berg wrote: > On Sun, 2013-08-11 at 22:19 +0200, Johan Almbladh wrote: > > ieee80211_rx_h_sta_process: > > The updating of last_rx for an IBSS STA is conditional on the STA > > being AUTHORIZED. That state is the same regardless of whether the > > updating is done before or after decryption. > > That's true, but my concern here was about the reboot problem: > * two stations join a secure mesh and communicate > * one of them reboots, but quickly rejoins the mesh > In this case sometimes it gets hard to detect the situation and keeping > the other station alive. OTOH, it already happens for beacon frames > which aren't encrypted anyway, so I think it shouldn't matter. Antonio? For the "reboot detection" mechanism only the beacon and the AUTH frames are involved, therefore (since both are not encrypted) there should be no problem. Cheers,
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c index 6b85f95..0f0017d 100644 --- a/net/mac80211/rx.c +++ b/net/mac80211/rx.c @@ -2939,10 +2939,10 @@ static void ieee80211_rx_handlers(struct ieee80211_rx_data *rx, */ rx->skb = skb; - CALL_RXH(ieee80211_rx_h_decrypt) CALL_RXH(ieee80211_rx_h_check_more_data) CALL_RXH(ieee80211_rx_h_uapsd_and_pspoll) CALL_RXH(ieee80211_rx_h_sta_process) + CALL_RXH(ieee80211_rx_h_decrypt) CALL_RXH(ieee80211_rx_h_defragment) CALL_RXH(ieee80211_rx_h_michael_mic_verify) /* must be after MMIC verify so header is counted in MPDU mic */
This patch enables power save processing for encrypted frames even if the encryption key is not set. This is a requirement when implementing split-MAC systems like Anyfi.net [1] and CAPWAP [2] on mac80211 using monitor frame injection and reception. The mac80211 RX handlers are reordered slightly so that the power save handler is invoked before the decryption handler. The patch is minimal in the sense that it provides the required functionality with a minimal change, but I am open to suggestions if this change is too intrusive. Please let me know what you think. [1] http://anyfi.net/documentation#architecture [2] http://tools.ietf.org/html/rfc5416 Signed-off-by: Johan Almbladh <ja@anyfi.net> --- net/mac80211/rx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)