diff mbox series

[12/15] mka: resources leaked when duplicated SCI detected

Message ID 20180302201103.16264-13-msiedzik@extremenetworks.com
State Accepted
Headers show
Series MKA bugfixes and enhancements | expand

Commit Message

Michael Siedzik March 2, 2018, 8:11 p.m. UTC 
From: Mike Siedzik <msiedzik@extremenetworks.com>

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 a
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 <msiedzik@extremenetworks.com>
---
 src/pae/ieee802_1x_kay.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

--
2.11.1

Comments

Jouni Malinen Dec. 26, 2018, 11:15 p.m. UTC | #1 
On Fri, Mar 02, 2018 at 03:11:00PM -0500, msiedzik@extremenetworks.com wrote:
> 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 a
> 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.

Thanks, applied.
diff mbox series

Patch

diff --git a/src/pae/ieee802_1x_kay.c b/src/pae/ieee802_1x_kay.c
index 27022d994..4d61cb32b 100644
--- a/src/pae/ieee802_1x_kay.c
+++ b/src/pae/ieee802_1x_kay.c
@@ -792,17 +792,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 be 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 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
+                * timeout and clean itself up (within MKA_LIFE_TIME).
+                *
+                * This method is peferable 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");
+                       return NULL;
                }

                peer = ieee802_1x_kay_create_potential_peer(