[Patch 1/1] mka : Some Bug Fixes

[Badrish Adiga H R]

On Fri, Nov 25, 2016 at 8:59 PM, Sabrina Dubroca <sd at queasysnail.net> 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