| Message ID | CAGNNFCZKkPnJnGaOV5wHf6r_OEPEnYACAMq73GH=K=XKdhovYQ@mail.gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On Mon, Dec 05, 2016 at 08:56:23PM +0530, Badrish Adiga H R wrote: > 1. API ieee802_1x_mka_decode_dist_sak_body wrongly puts > participant->to_use_sak to TRUE, if invalid DIstributed SAK Parameter > Set is received > 2. when number of live peers become 0, the flags such lrx, ltx, orx, > otx etc. needs to be cleared. In MACsec PSK mode, these stale values > create problems, while re-establishing CA... These sound completely independent changes and as such, should be split into two separate patches. As far as (1) is concerned, I'm not sure I followed the previous thread regarding the change. So this does not actually fix any observable behavior? It would be good for the commit message to be clear on why this change is needed and what impact it has on behavior. For (2), could you please be more specific on "create problems"? I.e., describe what those problems are and how one would notice that in practice (i.e., what kind of incorrect behavior this fixes)?
Hi Jouni, Thanks for your reply.. Actually, I am using a patch which allows to configure MKA actor priority. I have submitted that patch also to hostap's mailing list. The test scenario is this. I have 2 peers running MACsec in PSK mode, Peer A with MAC address higher than MAC Address of peer B. 1. Peer B starts with actor_priority 255 2. Peer A starts with priority 16, becomes key server. 3. Peer A stops.. 4. Peer A restarts with priority 255, but because of the stale values participant->is_key_server(=TRUE) and participant->is_elected(=TRUE) it continues to remain as Key Server. 5. For peer B, key server election happens and since it has lower MAC address as compared to MAC address of A, it becomes the key server. Now we have 2 key servers in CA and is not correct. On Mon, Dec 12, 2016 at 12:03 AM, Jouni Malinen <j@w1.fi> wrote: > On Mon, Dec 05, 2016 at 08:56:23PM +0530, Badrish Adiga H R wrote: >> 1. API ieee802_1x_mka_decode_dist_sak_body wrongly puts >> participant->to_use_sak to TRUE, if invalid DIstributed SAK Parameter >> Set is received >> 2. when number of live peers become 0, the flags such lrx, ltx, orx, >> otx etc. needs to be cleared. In MACsec PSK mode, these stale values >> create problems, while re-establishing CA... > > These sound completely independent changes and as such, should be split > into two separate patches. As far as (1) is concerned, I'm not sure I > followed the previous thread regarding the change. So this does not > actually fix any observable behavior? It would be good for the commit > message to be clear on why this change is needed and what impact it has > on behavior. > > For (2), could you please be more specific on "create problems"? I.e., > describe what those problems are and how one would notice that in > practice (i.e., what kind of incorrect behavior this fixes)? > > -- > Jouni Malinen PGP id EFC895FA
On Mon, Dec 12, 2016 at 06:27:08PM +0530, Badrish Adiga H R wrote: > Actually, I am using a patch which allows to configure MKA actor > priority. I have submitted that patch also to hostap's mailing list. > The test scenario is this. I have 2 peers running MACsec in PSK mode, > Peer A with MAC address higher than MAC Address of peer B. > 1. Peer B starts with actor_priority 255 > 2. Peer A starts with priority 16, becomes key server. > 3. Peer A stops.. > 4. Peer A restarts with priority 255, but because of the stale values > participant->is_key_server(=TRUE) and participant->is_elected(=TRUE) > it continues to remain as Key Server. > 5. For peer B, key server election happens and since it has lower MAC > address as compared to MAC address of A, it becomes the key server. > > Now we have 2 key servers in CA and is not correct. It would be good to get such details included in the commit message. Please split the changes into separate patches that each address a single independent issue and include the relevant description in the commit message.
Sure.. will do this. Thanks, Badrish On Sat, Dec 24, 2016 at 5:02 PM, Jouni Malinen <j@w1.fi> wrote: > On Mon, Dec 12, 2016 at 06:27:08PM +0530, Badrish Adiga H R wrote: >> Actually, I am using a patch which allows to configure MKA actor >> priority. I have submitted that patch also to hostap's mailing list. >> The test scenario is this. I have 2 peers running MACsec in PSK mode, >> Peer A with MAC address higher than MAC Address of peer B. >> 1. Peer B starts with actor_priority 255 >> 2. Peer A starts with priority 16, becomes key server. >> 3. Peer A stops.. >> 4. Peer A restarts with priority 255, but because of the stale values >> participant->is_key_server(=TRUE) and participant->is_elected(=TRUE) >> it continues to remain as Key Server. >> 5. For peer B, key server election happens and since it has lower MAC >> address as compared to MAC address of A, it becomes the key server. >> >> Now we have 2 key servers in CA and is not correct. > > It would be good to get such details included in the commit message. > Please split the changes into separate patches that each address a > single independent issue and include the relevant description in the > commit message. > > -- > Jouni Malinen PGP id EFC895FA
diff --git a/src/pae/ieee802_1x_kay.c b/src/pae/ieee802_1x_kay.c index 1d6d9a9..e81ae0c 100644 --- a/src/pae/ieee802_1x_kay.c +++ b/src/pae/ieee802_1x_kay.c @@ -1559,7 +1559,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; } @@ -2377,6 +2377,12 @@ static void ieee802_1x_participant_timer(void *eloop_ctx, void *timeout_ctx) participant->advised_capability = MACSEC_CAP_NOT_IMPLEMENTED; participant->to_use_sak = FALSE; + participant->ltx = FALSE; + participant->lrx = FALSE; + participant->otx = FALSE; + participant->orx = FALSE; + participant->is_key_server = FALSE; + participant->is_elected = FALSE; kay->authenticated = TRUE; kay->secured = FALSE;