| Message ID | CAGNNFCbuPkyxC0ERz5SO-vo3y1u8TWLtVEpVswP7r7y7-NiuKA@mail.gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
2016-12-05, 21:47:13 +0530, Badrish Adiga H R wrote: > From: Badrish Adiga H R <badrish.adigahr@gmail.com> > > Signed-off-by: Badrish Adiga H R <badrish.adigahr@gmail.com> That's quite similar to the patch I was working on, except for some comments below. [...] > diff --git a/wpa_supplicant/config.c b/wpa_supplicant/config.c > index 2120a6e..b446f84 100644 > --- a/wpa_supplicant/config.c > +++ b/wpa_supplicant/config.c > @@ -2127,6 +2127,7 @@ static const struct parse_data ssid_fields[] = { > { INT_RANGE(macsec_policy, 0, 1) }, > { INT_RANGE(macsec_integ_only, 0, 1) }, > { INT_RANGE(macsec_port, 1, 65534) }, > + { INT_RANGE(mka_actor_priority, 0, 255) }, > { FUNC_KEY(mka_cak) }, > { FUNC_KEY(mka_ckn) }, > #endif /* CONFIG_MACSEC */ I have an extra chunk to set the default value: @@ -2617,6 +2621,9 @@ void wpa_config_set_network_defaults(struct wpa_ssid *ssid) #ifdef CONFIG_IEEE80211W ssid->ieee80211w = MGMT_FRAME_PROTECTION_DEFAULT; #endif /* CONFIG_IEEE80211W */ +#ifdef CONFIG_MACSEC + ssid->mka_priority = DEFAULT_PRIO_NOT_KEY_SERVER; +#endif /* CONFIG_MACSEC */ ssid->mac_addr = -1; } > diff --git a/wpa_supplicant/config_ssid.h b/wpa_supplicant/config_ssid.h > index fe0f7fa..ac5d28e 100644 > --- a/wpa_supplicant/config_ssid.h > +++ b/wpa_supplicant/config_ssid.h > @@ -751,6 +751,11 @@ struct wpa_ssid { > int macsec_port; > > /** > + * mka_actor_priority - Priority of MKA Actor > + */ > + u8 mka_actor_priority; That doesn't work. The way wpa_supplicant parses configuration, it will store a full int (32 bits), so it overwrites part of the CKN after it. You need to make this an 'int'. > + > + /** > * mka_ckn - MKA pre-shared CKN > */ > #define MACSEC_CKN_LEN 32 > diff --git a/wpa_supplicant/wpa_cli.c b/wpa_supplicant/wpa_cli.c > index f11028a..d0b584c 100644 > --- a/wpa_supplicant/wpa_cli.c > +++ b/wpa_supplicant/wpa_cli.c > @@ -1392,6 +1392,7 @@ static const char *network_fields[] = { > "macsec_policy", > "macsec_integ_only", > "macsec_port", > + "mka_actor_priority", > #endif /* CONFIG_MACSEC */ > #ifdef CONFIG_HS20 > "update_identifier", > diff --git a/wpa_supplicant/wpa_supplicant.conf > b/wpa_supplicant/wpa_supplicant.conf > index edb230d..3ad2c44 100644 > --- a/wpa_supplicant/wpa_supplicant.conf > +++ b/wpa_supplicant/wpa_supplicant.conf > @@ -901,13 +901,16 @@ fast_reauth=1 > # Port component of the SCI > # Range: 1-65534 (default: 1) > # > -# mka_cak and mka_ckn: IEEE 802.1X/MACsec pre-shared authentication mode > -# This allows to configure MACsec with a pre-shared key using a (CAK,CKN) pair. > -# In this mode, instances of wpa_supplicant can act as peers, one of > -# which will become the key server and start distributing SAKs. > +# mka_cak, mka_ckn and mka_actor_priority: IEEE 802.1X/MACsec pre-shared I wouldn't mention 'mka_actor_priority' here. You're in the "pre-shared auth" section, actor priority doesn't have anything to do with that. > +# authentication mode; This allows to configure MACsec with a pre-shared key > +# using a (CAK,CKN) pair. In this mode, instances of wpa_supplicant can act as > +# MACsec peers. The peer with lower actor priority will become the key server > +# and start distributing SAKs. > # mka_cak (CAK = Secure Connectivity Association Key) takes a > 16-bytes (128 bit) > # hex-string (32 hex-digits) > # mka_ckn (CKN = CAK Name) takes a 32-bytes (256 bit) hex-string (64 > hex-digits) > +# mka_actor_priority (Priority of MKA Actor) is in 1..255 range with 255 being > +# default priority. You defined the valid range as 0..255. FWIW, here's the doc I have (I called it mka_priority instead of mka_actor_priority in my version, I'm fine either way): # mka_priority: IEEE 802.1X/MACsec priority # This allows to configure the priority of the wpa_supplicant instance # in the key server election process. Lower values indicate higher # priority, i.e. more likely to be elected. # Recommended ranges are specified in Table 9-2 (Key Server Priority # values) of the IEEE 802.1X-2010 standard. # Range: 0-255 (default: 255) > # > # mixed_cell: This option can be used to configure whether so called mixed > # cells, i.e., networks that use both plaintext and encryption in the same > diff --git a/wpa_supplicant/wpas_kay.c b/wpa_supplicant/wpas_kay.c > index d3fefda..cc41c74 100644 > --- a/wpa_supplicant/wpas_kay.c > +++ b/wpa_supplicant/wpas_kay.c > @@ -233,7 +233,8 @@ int ieee802_1x_alloc_kay_sm(struct wpa_supplicant > *wpa_s, struct wpa_ssid *ssid) > kay_ctx->disable_transmit_sa = wpas_disable_transmit_sa; > > res = ieee802_1x_kay_init(kay_ctx, policy, ssid->macsec_port, > - wpa_s->ifname, wpa_s->own_addr); > + ssid->mka_actor_priority, wpa_s->ifname, > + wpa_s->own_addr); > if (res == NULL) { > os_free(kay_ctx); > return -1;
On Tue, Dec 13, 2016 at 4:03 PM, Sabrina Dubroca <sd@queasysnail.net> wrote: > 2016-12-05, 21:47:13 +0530, Badrish Adiga H R wrote: >> From: Badrish Adiga H R <badrish.adigahr@gmail.com> >> >> Signed-off-by: Badrish Adiga H R <badrish.adigahr@gmail.com> > > That's quite similar to the patch I was working on, except for some comments below. > > > [...] >> diff --git a/wpa_supplicant/config.c b/wpa_supplicant/config.c >> index 2120a6e..b446f84 100644 >> --- a/wpa_supplicant/config.c >> +++ b/wpa_supplicant/config.c >> @@ -2127,6 +2127,7 @@ static const struct parse_data ssid_fields[] = { >> { INT_RANGE(macsec_policy, 0, 1) }, >> { INT_RANGE(macsec_integ_only, 0, 1) }, >> { INT_RANGE(macsec_port, 1, 65534) }, >> + { INT_RANGE(mka_actor_priority, 0, 255) }, >> { FUNC_KEY(mka_cak) }, >> { FUNC_KEY(mka_ckn) }, >> #endif /* CONFIG_MACSEC */ > > I have an extra chunk to set the default value: > > @@ -2617,6 +2621,9 @@ void wpa_config_set_network_defaults(struct wpa_ssid *ssid) > #ifdef CONFIG_IEEE80211W > ssid->ieee80211w = MGMT_FRAME_PROTECTION_DEFAULT; > #endif /* CONFIG_IEEE80211W */ > +#ifdef CONFIG_MACSEC > + ssid->mka_priority = DEFAULT_PRIO_NOT_KEY_SERVER; > +#endif /* CONFIG_MACSEC */ > ssid->mac_addr = -1; > } > > Thanks for looking into. I am okay with this change... > >> diff --git a/wpa_supplicant/config_ssid.h b/wpa_supplicant/config_ssid.h >> index fe0f7fa..ac5d28e 100644 >> --- a/wpa_supplicant/config_ssid.h >> +++ b/wpa_supplicant/config_ssid.h >> @@ -751,6 +751,11 @@ struct wpa_ssid { >> int macsec_port; >> >> /** >> + * mka_actor_priority - Priority of MKA Actor >> + */ >> + u8 mka_actor_priority; > > That doesn't work. The way wpa_supplicant parses configuration, it > will store a full int (32 bits), so it overwrites part of the CKN > after it. You need to make this an 'int'. > A good catch and thanks again... >> + >> + /** >> * mka_ckn - MKA pre-shared CKN >> */ >> #define MACSEC_CKN_LEN 32 >> diff --git a/wpa_supplicant/wpa_cli.c b/wpa_supplicant/wpa_cli.c >> index f11028a..d0b584c 100644 >> --- a/wpa_supplicant/wpa_cli.c >> +++ b/wpa_supplicant/wpa_cli.c >> @@ -1392,6 +1392,7 @@ static const char *network_fields[] = { >> "macsec_policy", >> "macsec_integ_only", >> "macsec_port", >> + "mka_actor_priority", >> #endif /* CONFIG_MACSEC */ >> #ifdef CONFIG_HS20 >> "update_identifier", >> diff --git a/wpa_supplicant/wpa_supplicant.conf >> b/wpa_supplicant/wpa_supplicant.conf >> index edb230d..3ad2c44 100644 >> --- a/wpa_supplicant/wpa_supplicant.conf >> +++ b/wpa_supplicant/wpa_supplicant.conf >> @@ -901,13 +901,16 @@ fast_reauth=1 >> # Port component of the SCI >> # Range: 1-65534 (default: 1) >> # >> -# mka_cak and mka_ckn: IEEE 802.1X/MACsec pre-shared authentication mode >> -# This allows to configure MACsec with a pre-shared key using a (CAK,CKN) pair. >> -# In this mode, instances of wpa_supplicant can act as peers, one of >> -# which will become the key server and start distributing SAKs. >> +# mka_cak, mka_ckn and mka_actor_priority: IEEE 802.1X/MACsec pre-shared > > I wouldn't mention 'mka_actor_priority' here. You're in the > "pre-shared auth" section, actor priority doesn't have anything to do > with that. > I had put this in "pre-shared auth" section, because I felt this is applicable to PSK mode only. In EAP_EXCHANGE mode, always Authenticator should be Key Server right? >> +# authentication mode; This allows to configure MACsec with a pre-shared key >> +# using a (CAK,CKN) pair. In this mode, instances of wpa_supplicant can act as >> +# MACsec peers. The peer with lower actor priority will become the key server >> +# and start distributing SAKs. >> # mka_cak (CAK = Secure Connectivity Association Key) takes a >> 16-bytes (128 bit) >> # hex-string (32 hex-digits) >> # mka_ckn (CKN = CAK Name) takes a 32-bytes (256 bit) hex-string (64 >> hex-digits) >> +# mka_actor_priority (Priority of MKA Actor) is in 1..255 range with 255 being >> +# default priority. > > You defined the valid range as 0..255. > > FWIW, here's the doc I have (I called it mka_priority instead of > mka_actor_priority in my version, I'm fine either way): > > # mka_priority: IEEE 802.1X/MACsec priority > # This allows to configure the priority of the wpa_supplicant instance > # in the key server election process. Lower values indicate higher > # priority, i.e. more likely to be elected. > # Recommended ranges are specified in Table 9-2 (Key Server Priority > # values) of the IEEE 802.1X-2010 standard. > # Range: 0-255 (default: 255) > > >> # >> # mixed_cell: This option can be used to configure whether so called mixed >> # cells, i.e., networks that use both plaintext and encryption in the same >> diff --git a/wpa_supplicant/wpas_kay.c b/wpa_supplicant/wpas_kay.c >> index d3fefda..cc41c74 100644 >> --- a/wpa_supplicant/wpas_kay.c >> +++ b/wpa_supplicant/wpas_kay.c >> @@ -233,7 +233,8 @@ int ieee802_1x_alloc_kay_sm(struct wpa_supplicant >> *wpa_s, struct wpa_ssid *ssid) >> kay_ctx->disable_transmit_sa = wpas_disable_transmit_sa; >> >> res = ieee802_1x_kay_init(kay_ctx, policy, ssid->macsec_port, >> - wpa_s->ifname, wpa_s->own_addr); >> + ssid->mka_actor_priority, wpa_s->ifname, >> + wpa_s->own_addr); >> if (res == NULL) { >> os_free(kay_ctx); >> return -1; > > -- > Sabrina Regards, Badrish
On Tue, Dec 13, 2016 at 09:24:17PM +0530, Badrish Adiga H R wrote: > On Tue, Dec 13, 2016 at 4:03 PM, Sabrina Dubroca <sd@queasysnail.net> wrote: > > That's quite similar to the patch I was working on, except for some comments below. ... > Thanks for looking into. I am okay with this change... I dropped this patch from my queue. Please resubmit an updated version once the identified issues have been addressed.
Hi Jouni, Thanks for your reply. I have submitted updated version as new patch... Regards, Badrish On Sun, Dec 18, 2016 at 9:33 PM, Jouni Malinen <j@w1.fi> wrote: > On Tue, Dec 13, 2016 at 09:24:17PM +0530, Badrish Adiga H R wrote: >> On Tue, Dec 13, 2016 at 4:03 PM, Sabrina Dubroca <sd@queasysnail.net> wrote: >> > That's quite similar to the patch I was working on, except for some comments below. > ... >> Thanks for looking into. I am okay with this change... > > I dropped this patch from my queue. Please resubmit an updated version > once the identified issues have been addressed. > > -- > Jouni Malinen PGP id EFC895FA
diff --git a/src/pae/ieee802_1x_kay.c b/src/pae/ieee802_1x_kay.c index 1d6d9a9..819b577 100644 --- a/src/pae/ieee802_1x_kay.c +++ b/src/pae/ieee802_1x_kay.c @@ -3082,7 +3082,7 @@ static void kay_l2_receive(void *ctx, const u8 *src_addr, const u8 *buf, */ struct ieee802_1x_kay * ieee802_1x_kay_init(struct ieee802_1x_kay_ctx *ctx, enum macsec_policy policy, - u16 port, const char *ifname, const u8 *addr) + u16 port, u8 priority, const char *ifname, const u8 *addr) { struct ieee802_1x_kay *kay; @@ -3105,7 +3105,7 @@ ieee802_1x_kay_init(struct ieee802_1x_kay_ctx *ctx, enum macsec_policy policy, os_strlcpy(kay->if_name, ifname, IFNAMSIZ); os_memcpy(kay->actor_sci.addr, addr, ETH_ALEN); kay->actor_sci.port = host_to_be16(port ? port : 0x0001); - kay->actor_priority = DEFAULT_PRIO_NOT_KEY_SERVER; + kay->actor_priority = priority; /* While actor acts as a key server, shall distribute sakey */ kay->dist_kn = 1; diff --git a/src/pae/ieee802_1x_kay.h b/src/pae/ieee802_1x_kay.h index 9a92d1c..743c5f3 100644 --- a/src/pae/ieee802_1x_kay.h +++ b/src/pae/ieee802_1x_kay.h @@ -235,7 +235,7 @@ u64 mka_sci_u64(struct ieee802_1x_mka_sci *sci); struct ieee802_1x_kay * ieee802_1x_kay_init(struct ieee802_1x_kay_ctx *ctx, enum macsec_policy policy, - u16 port, const char *ifname, const u8 *addr); + u16 port, u8 priority, const char *ifname, const u8 *addr); void ieee802_1x_kay_deinit(struct ieee802_1x_kay *kay); struct ieee802_1x_mka_participant * diff --git a/wpa_supplicant/config.c b/wpa_supplicant/config.c index 2120a6e..b446f84 100644 --- a/wpa_supplicant/config.c +++ b/wpa_supplicant/config.c @@ -2127,6 +2127,7 @@ static const struct parse_data ssid_fields[] = { { INT_RANGE(macsec_policy, 0, 1) }, { INT_RANGE(macsec_integ_only, 0, 1) }, { INT_RANGE(macsec_port, 1, 65534) }, + { INT_RANGE(mka_actor_priority, 0, 255) }, { FUNC_KEY(mka_cak) }, { FUNC_KEY(mka_ckn) }, #endif /* CONFIG_MACSEC */ diff --git a/wpa_supplicant/config_file.c b/wpa_supplicant/config_file.c index 2e3d57e..fd04c25 100644 --- a/wpa_supplicant/config_file.c +++ b/wpa_supplicant/config_file.c @@ -22,6 +22,7 @@ #include "p2p/p2p.h" #include "eap_peer/eap_methods.h" #include "eap_peer/eap.h" +#include "common/ieee802_1x_defs.h" static int newline_terminated(const char *buf, size_t buflen) @@ -810,6 +811,7 @@ static void wpa_config_write_network(FILE *f, struct wpa_ssid *ssid) write_mka_ckn(f, ssid); INT(macsec_integ_only); INT(macsec_port); + INT_DEF(mka_actor_priority, DEFAULT_PRIO_NOT_KEY_SERVER); #endif /* CONFIG_MACSEC */ #ifdef CONFIG_HS20 INT(update_identifier); diff --git a/wpa_supplicant/config_ssid.h b/wpa_supplicant/config_ssid.h index fe0f7fa..ac5d28e 100644 --- a/wpa_supplicant/config_ssid.h +++ b/wpa_supplicant/config_ssid.h @@ -751,6 +751,11 @@ struct wpa_ssid { int macsec_port; /** + * mka_actor_priority - Priority of MKA Actor + */ + u8 mka_actor_priority; + + /** * mka_ckn - MKA pre-shared CKN */ #define MACSEC_CKN_LEN 32 diff --git a/wpa_supplicant/wpa_cli.c b/wpa_supplicant/wpa_cli.c index f11028a..d0b584c 100644 --- a/wpa_supplicant/wpa_cli.c +++ b/wpa_supplicant/wpa_cli.c @@ -1392,6 +1392,7 @@ static const char *network_fields[] = { "macsec_policy", "macsec_integ_only", "macsec_port", + "mka_actor_priority", #endif /* CONFIG_MACSEC */ #ifdef CONFIG_HS20 "update_identifier", diff --git a/wpa_supplicant/wpa_supplicant.conf b/wpa_supplicant/wpa_supplicant.conf index edb230d..3ad2c44 100644 --- a/wpa_supplicant/wpa_supplicant.conf +++ b/wpa_supplicant/wpa_supplicant.conf @@ -901,13 +901,16 @@ fast_reauth=1 # Port component of the SCI # Range: 1-65534 (default: 1) # -# mka_cak and mka_ckn: IEEE 802.1X/MACsec pre-shared authentication mode -# This allows to configure MACsec with a pre-shared key using a (CAK,CKN) pair. -# In this mode, instances of wpa_supplicant can act as peers, one of -# which will become the key server and start distributing SAKs. +# mka_cak, mka_ckn and mka_actor_priority: IEEE 802.1X/MACsec pre-shared +# authentication mode; This allows to configure MACsec with a pre-shared key +# using a (CAK,CKN) pair. In this mode, instances of wpa_supplicant can act as +# MACsec peers. The peer with lower actor priority will become the key server +# and start distributing SAKs. # mka_cak (CAK = Secure Connectivity Association Key) takes a 16-bytes (128 bit) # hex-string (32 hex-digits) # mka_ckn (CKN = CAK Name) takes a 32-bytes (256 bit) hex-string (64 hex-digits) +# mka_actor_priority (Priority of MKA Actor) is in 1..255 range with 255 being +# default priority. # # mixed_cell: This option can be used to configure whether so called mixed # cells, i.e., networks that use both plaintext and encryption in the same diff --git a/wpa_supplicant/wpas_kay.c b/wpa_supplicant/wpas_kay.c index d3fefda..cc41c74 100644 --- a/wpa_supplicant/wpas_kay.c +++ b/wpa_supplicant/wpas_kay.c @@ -233,7 +233,8 @@ int ieee802_1x_alloc_kay_sm(struct wpa_supplicant *wpa_s, struct wpa_ssid *ssid) kay_ctx->disable_transmit_sa = wpas_disable_transmit_sa; res = ieee802_1x_kay_init(kay_ctx, policy, ssid->macsec_port, - wpa_s->ifname, wpa_s->own_addr); + ssid->mka_actor_priority, wpa_s->ifname, + wpa_s->own_addr); if (res == NULL) { os_free(kay_ctx);