Message ID | 1326808982-11510-1-git-send-email-ordex@autistici.org |
---|---|
State | Changes Requested |
Headers | show |
On Tue, Jan 17, 2012 at 03:03:02PM +0100, Antonio Quartulli wrote: > In IBSS RSN cfg80211/mac80211 now waits for userspace to authorize new stations. > This patch makes wpa_supplicant notify the driver when a station can be > considered authorised. > diff --git a/wpa_supplicant/ibss_rsn.c b/wpa_supplicant/ibss_rsn.c > @@ -19,6 +19,7 @@ > #include "ap/wpa_auth.h" > +#include "ap/wpa_auth_i.h" This is not acceptable - the internal header files (*_i.h) are not supposed to be included outside the directory in which they are defined. > @@ -545,7 +546,7 @@ static int ibss_rsn_process_rx_eapol(struct ibss_rsn *ibss_rsn, > + /* check if the peer has been authorized */ > + if (peer->auth->wpa_ptk_state == WPA_PTK_PTKINITDONE) { This is looking at an internal state in a way that makes it more difficult to maintain the code in the future. A cleaner design is going to be needed here, e.g., with a callback function. Wouldn't the way used in AP code be suitable for this? See how hostapd_wpa_auth_set_eapol() is used to handle WPA_EAPOL_authorized changes in src/ap/wpa_auth_glue.c for an example. wpa_supplicant/ibss_rsn.c should be able to do the same.
On Sun, Jan 22, 2012 at 05:30:20PM +0200, Jouni Malinen wrote: > On Tue, Jan 17, 2012 at 03:03:02PM +0100, Antonio Quartulli wrote: > > In IBSS RSN cfg80211/mac80211 now waits for userspace to authorize new stations. > > This patch makes wpa_supplicant notify the driver when a station can be > > considered authorised. > > > diff --git a/wpa_supplicant/ibss_rsn.c b/wpa_supplicant/ibss_rsn.c > > @@ -19,6 +19,7 @@ > > #include "ap/wpa_auth.h" > > +#include "ap/wpa_auth_i.h" > > This is not acceptable - the internal header files (*_i.h) are not > supposed to be included outside the directory in which they are defined. > > > @@ -545,7 +546,7 @@ static int ibss_rsn_process_rx_eapol(struct ibss_rsn *ibss_rsn, > > + /* check if the peer has been authorized */ > > + if (peer->auth->wpa_ptk_state == WPA_PTK_PTKINITDONE) { > > This is looking at an internal state in a way that makes it more > difficult to maintain the code in the future. A cleaner design is going > to be needed here, e.g., with a callback function. Wouldn't the way used > in AP code be suitable for this? See how hostapd_wpa_auth_set_eapol() is > used to handle WPA_EAPOL_authorized changes in src/ap/wpa_auth_glue.c > for an example. wpa_supplicant/ibss_rsn.c should be able to do the same. hello, sorry for taking so much. I'll send patch v2. Thanks for the feedback > > -- > Jouni Malinen PGP id EFC895FA > _______________________________________________ > HostAP mailing list > HostAP@lists.shmoo.com > http://lists.shmoo.com/mailman/listinfo/hostap
On Sat, Jan 28, 2012 at 07:01:50 +0100, Antonio Quartulli wrote: > On Sun, Jan 22, 2012 at 05:30:20PM +0200, Jouni Malinen wrote: > > On Tue, Jan 17, 2012 at 03:03:02PM +0100, Antonio Quartulli wrote: > > > In IBSS RSN cfg80211/mac80211 now waits for userspace to authorize new stations. > > > This patch makes wpa_supplicant notify the driver when a station can be > > > considered authorised. > > > > > diff --git a/wpa_supplicant/ibss_rsn.c b/wpa_supplicant/ibss_rsn.c > > > @@ -19,6 +19,7 @@ > > > #include "ap/wpa_auth.h" > > > +#include "ap/wpa_auth_i.h" > > > > This is not acceptable - the internal header files (*_i.h) are not > > supposed to be included outside the directory in which they are defined. > > > > > @@ -545,7 +546,7 @@ static int ibss_rsn_process_rx_eapol(struct ibss_rsn *ibss_rsn, > > > + /* check if the peer has been authorized */ > > > + if (peer->auth->wpa_ptk_state == WPA_PTK_PTKINITDONE) { > > > > This is looking at an internal state in a way that makes it more > > difficult to maintain the code in the future. A cleaner design is going > > to be needed here, e.g., with a callback function. Wouldn't the way used > > in AP code be suitable for this? See how hostapd_wpa_auth_set_eapol() is > > used to handle WPA_EAPOL_authorized changes in src/ap/wpa_auth_glue.c > > for an example. wpa_supplicant/ibss_rsn.c should be able to do the same. > I saw the example. Thanks for the hint ;) But I have a question..is it better to authorize the station when receiving WPA_EAPOL_authorized or when receiving WPA_EAPOL_keyDone ? Actually I don't think it makes much difference as they are set one after the other (if I correctly got src/ap/wpa_auth.c:{2090-2107}). Thank you! Cheers,
diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c index 6af8cc9..791f15a 100644 --- a/src/drivers/driver_nl80211.c +++ b/src/drivers/driver_nl80211.c @@ -6397,6 +6397,9 @@ retry: if (ret) goto nla_put_failure; + if (params->key_mgmt_suite == KEY_MGMT_802_1X) + NLA_PUT_FLAG(msg, NL80211_ATTR_CONTROL_PORT); + if (params->wpa_ie) { wpa_hexdump(MSG_DEBUG, " * Extra IEs for Beacon/Probe Response frames", diff --git a/wpa_supplicant/ibss_rsn.c b/wpa_supplicant/ibss_rsn.c index d4fa39d..40fc7e0 100644 --- a/wpa_supplicant/ibss_rsn.c +++ b/wpa_supplicant/ibss_rsn.c @@ -19,6 +19,7 @@ #include "rsn_supp/wpa.h" #include "rsn_supp/wpa_ie.h" #include "ap/wpa_auth.h" +#include "ap/wpa_auth_i.h" #include "wpa_supplicant_i.h" #include "driver_i.h" #include "ibss_rsn.h" @@ -545,7 +546,7 @@ static int ibss_rsn_process_rx_eapol(struct ibss_rsn *ibss_rsn, struct ibss_rsn_peer *peer, const u8 *buf, size_t len) { - int supp; + int supp, res; u8 *tmp; supp = ibss_rsn_eapol_dst_supp(buf, len); @@ -562,6 +563,21 @@ static int ibss_rsn_process_rx_eapol(struct ibss_rsn *ibss_rsn, } else { wpa_printf(MSG_DEBUG, "RSN: IBSS RX EAPOL for Authenticator"); wpa_receive(ibss_rsn->auth_group, peer->auth, tmp, len); + /* check if the peer has been authorized */ + if (peer->auth->wpa_ptk_state == WPA_PTK_PTKINITDONE) { + res = wpa_drv_sta_set_flags(ibss_rsn->wpa_s, peer->addr, + WPA_STA_AUTHORIZED, + WPA_STA_AUTHORIZED, + 0xFFFFFFFF); + if (res < 0) + wpa_printf(MSG_DEBUG, "RSN: Error while " + "authorising STA " MACSTR, + MAC2STR(peer->addr)); + else + wpa_printf(MSG_DEBUG, "RSN: STA " MACSTR + " authorised.\n", + MAC2STR(peer->addr)); + } } os_free(tmp);