[Patch 1/1] mka : Some Bug Fixes

[Sabrina Dubroca]

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.


> 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?


> @@ -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.


>         /* 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.

>         if (res == NULL) {
>                 os_free(kay_ctx);
>                 return -1;
> 

-- 
Sabrina