diff mbox

wpa_supplicant: support IBSS RSN STA authorization

Message ID 1326808982-11510-1-git-send-email-ordex@autistici.org
State Changes Requested
Headers show

Commit Message

Antonio Quartulli Jan. 17, 2012, 2:03 p.m. UTC 
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.

Signed-hostap: Antonio Quartulli <ordex@autistici.org>
---

This patch depends on the not yet committed one:
"cfg80211/mac80211: userspace peer authorization in IBSS"
which is pending on the linux-wireless mailing-list



 src/drivers/driver_nl80211.c |    3 +++
 wpa_supplicant/ibss_rsn.c    |   18 +++++++++++++++++-
 2 files changed, 20 insertions(+), 1 deletions(-)

Comments

Jouni Malinen Jan. 22, 2012, 3:30 p.m. UTC | #1 
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.
Antonio Quartulli Jan. 28, 2012, 6:01 p.m. UTC | #2 
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
Antonio Quartulli Jan. 29, 2012, 2:14 a.m. UTC | #3 
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 mbox

Patch

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);