On Fri, Nov 25, 2016 at 8:59 PM, Sabrina Dubroca <sd@xxxxxxxxxxxxxxx> wrote:
> 2016-11-24, 20:46:51 +0530, Badrish Adiga H R wrote:
>> Plz Ignore the above patch....
>>
>> consider this one
>
> It doesn't apply, you have some extra '\n' characters.
>
> You're going to need to provide a commit message explaining these
> changes, and probably split them up in separate patches. You'll also
> need a sign-off line, see CONTRIBUTIONS.
>
Thanks.. I will send new patch-set then...
>
>> diff --git a/src/pae/ieee802_1x_kay.c b/src/pae/ieee802_1x_kay.c
>> index 3a495ca..0baa5d3 100644
>> --- a/src/pae/ieee802_1x_kay.c
>> +++ b/src/pae/ieee802_1x_kay.c
>> @@ -1548,7 +1548,7 @@ ieee802_1x_mka_decode_dist_sak_body(
>> ieee802_1x_cp_connect_authenticated(kay->cp);
>> ieee802_1x_cp_sm_step(kay->cp);
>> wpa_printf(MSG_WARNING, "KaY:The Key server advise no MACsec");
>> - participant->to_use_sak = TRUE;
>> + participant->to_use_sak = FALSE;
>> return 0;
>> }
>
> What incorrect behavior do you observe without this?
Nothing, it is just that value is incorrect...
>
>
>> @@ -3071,7 +3071,8 @@ 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, const char *ifname, const u8 *addr,
>> + enum mka_created_mode mode)
>> {
>> struct ieee802_1x_kay *kay;
>>
>> @@ -3094,7 +3095,12 @@ 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;
>> +
>> + if (mode == PSK) {
>> + kay->actor_priority = DEFAULT_PRIO_INFRA_PORT;
>> + } else {
>> + kay->actor_priority = DEFAULT_PRIO_NOT_KEY_SERVER;
>> + }
>
> I'm not a big fan of this change. The reason is that if I connect my
> laptop to a MACsec switch, I'd like the switch to be the key server,
> not my laptop.
>
> I thought about making the priority configurable instead, just haven't
> implemented it yet.
>
Making priority configurable is better alternative. in case laptop is
connected to MACsec switch, I would prefer to make switch as
authenticator and my laptop as Supplicant and use EAP mode MACsec
which is more secure as compared to PSK.
>
>> /* 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 ea5a0dd..c0b0ade 100644
>> --- a/src/pae/ieee802_1x_kay.h
>> +++ b/src/pae/ieee802_1x_kay.h
>> @@ -233,7 +233,8 @@ struct ieee802_1x_kay {
>>
>> 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, const char *ifname, const u8 *addr,
>> + enum mka_created_mode mode);
>> void ieee802_1x_kay_deinit(struct ieee802_1x_kay *kay);
>>
>> struct ieee802_1x_mka_participant *
>> diff --git a/wpa_supplicant/wpas_kay.c b/wpa_supplicant/wpas_kay.c
>> index d3fefda..fb0f26d 100644
>> --- a/wpa_supplicant/wpas_kay.c
>> +++ b/wpa_supplicant/wpas_kay.c
>> @@ -186,6 +186,7 @@ int ieee802_1x_alloc_kay_sm(struct wpa_supplicant
>> *wpa_s, struct wpa_ssid *ssid)
>> {
>> struct ieee802_1x_kay_ctx *kay_ctx;
>> struct ieee802_1x_kay *res = NULL;
>> + enum mka_created_mode mode;
>> enum macsec_policy policy;
>>
>> ieee802_1x_dealloc_kay_sm(wpa_s);
>> @@ -232,8 +233,11 @@ int ieee802_1x_alloc_kay_sm(struct wpa_supplicant
>> *wpa_s, struct wpa_ssid *ssid)
>> kay_ctx->enable_transmit_sa = wpas_enable_transmit_sa;
>> kay_ctx->disable_transmit_sa = wpas_disable_transmit_sa;
>>
>> + if ((ssid->mka_psk_set & MKA_PSK_SET) == MKA_PSK_SET) {
>> + mode = PSK;
>> + }
>> res = ieee802_1x_kay_init(kay_ctx, policy, ssid->macsec_port,
>> - wpa_s->ifname, wpa_s->own_addr);
>> + wpa_s->ifname, wpa_s->own_addr, mode);
>
> mode is used without being initialized here. I'm surprised gcc doesn't
> complain about that, looks like a compiler bug.
>
I uploaded wrong patch, so submitted another patch asking to ignore
previous one.
>> if (res == NULL) {
>> os_free(kay_ctx);
>> return -1;
>>
>
> --
> Sabrina
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
