From bab1d0d3593aea1f2d9320370bc7923434e7bd3a Mon Sep 17 00:00:00 2001 From: Mike Siedzik Date: Tue, 20 Feb 2018 14:28:42 -0500 Subject: [PATCH] mka: Fix resource leak when detecting duplicated SCI If a live peer ever changes its Member Identifier (MI), the KaY correctly detects a "duplicated SCI" but then proceeds to delete the peer without deleting the peer's resources (i.e., RxSC, RxSAs, TxSAs). Note that a remote peer's MI will change if and when an ieee8021XPaePortInitialize is executed on the remote port. The solution here is to ignore all MKPDUs containing the new MI until after the peer (that corresponds to the old MI) expires and cleans up its resources. After the old peer is removed reception of the next MKPDU containing the new MI will result in the creation of a new peer with the new MI. Signed-off-by: Michael Siedzik --- src/pae/ieee802_1x_kay.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/pae/ieee802_1x_kay.c b/src/pae/ieee802_1x_kay.c index a96340c50..e2dcd186c 100644 --- a/src/pae/ieee802_1x_kay.c +++ b/src/pae/ieee802_1x_kay.c @@ -803,17 +803,31 @@ ieee802_1x_mka_decode_basic_body(struct ieee802_1x_kay *kay, const u8 *mka_msg, /* handler peer */ peer = ieee802_1x_kay_get_peer(participant, body->actor_mi); if (!peer) { - /* Check duplicated SCI */ - /* TODO: What policy should be applied to detect duplicated SCI - * is active attacker or a valid peer whose MI is be changed? + /* Check duplicated SCI + * + * A duplicated SCI indicates either an active attacker or + * a valid peer whose MI is being changed. The latter scenario + * is more likely because to have gotten this far the received + * MKPDU must have had a valid ICV, indicating the peer holds + * the same CAK as our participant. + * + * Before creating a new peer object for the new MI we must + * clean up the resources (SCs and SAs) associated with the + * old peer. An easy way to do this is to ignore MKPDUs with + * the new MI's for now and just wait for the old peer to + * time out and clean itself up (within MKA_LIFE_TIME). + * + * This method is preferable to deleting the old peer here + * and now and continuing on with processing because if this + * MKPDU is from an attacker it's better to ignore the MKPDU + * than to process it (and delete a valid peer as well). */ peer = ieee802_1x_kay_get_peer_sci(participant, &body->actor_sci); if (peer) { wpa_printf(MSG_WARNING, - "KaY: duplicated SCI detected, Maybe active attacker"); - dl_list_del(&peer->list); - os_free(peer); + "KaY: duplicated SCI detected - maybe active attacker or peer selected new MI - ignore MKPDU"); + return NULL; } peer = ieee802_1x_kay_create_potential_peer(