diff mbox series

[01/15] mka: When matching CKNs ensure that lengths are identical

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

Commit Message

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

KaY looks up participants using CAK Name (CKN).  Per IEEE802.1X-2010
Clause 9.3.1 CAK identification, the CKN is an integral number of octets,
between 1 and 32 (inclusive).  This fix will ensure that the KaY does not
inadvertently match CKNs such as 'myCakNamedFoo' and 'myCakNamedFooBar'.

Signed-off-by: Michael Siedzik <msiedzik@extremenetworks.com>
---
 src/pae/ieee802_1x_kay.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

--
2.11.1

Comments

Jouni Malinen March 12, 2018, 11:50 p.m. UTC | #1 
On Fri, Mar 02, 2018 at 03:10:49PM -0500, msiedzik@extremenetworks.com wrote:
> KaY looks up participants using CAK Name (CKN).  Per IEEE802.1X-2010
> Clause 9.3.1 CAK identification, the CKN is an integral number of octets,
> between 1 and 32 (inclusive).  This fix will ensure that the KaY does not
> inadvertently match CKNs such as 'myCakNamedFoo' and 'myCakNamedFooBar'.

Thanks, applied.


PS.

This series on the mailing list was whitespace damaged (tabs converted
to spaces). I used an earlier version I received to my personal address
instead to avoid having to clean up that mess manually.
diff mbox series

Patch

diff --git a/src/pae/ieee802_1x_kay.c b/src/pae/ieee802_1x_kay.c
index cad0292ec..beaae58f0 100644
--- a/src/pae/ieee802_1x_kay.c
+++ b/src/pae/ieee802_1x_kay.c
@@ -245,14 +245,15 @@  ieee802_1x_mka_dump_sak_use_body(struct ieee802_1x_mka_sak_use_body *body)
  * ieee802_1x_kay_get_participant -
  */
 static struct ieee802_1x_mka_participant *
-ieee802_1x_kay_get_participant(struct ieee802_1x_kay *kay, const u8 *ckn)
+ieee802_1x_kay_get_participant(struct ieee802_1x_kay *kay, const u8 *ckn, size_t len)
 {
        struct ieee802_1x_mka_participant *participant;

        dl_list_for_each(participant, &kay->participant_list,
                         struct ieee802_1x_mka_participant, list) {
-               if (os_memcmp(participant->ckn.name, ckn,
-                             participant->ckn.len) == 0)
+               if ((participant->ckn.len == len) &&
+                   (os_memcmp(participant->ckn.name, ckn,
+                             participant->ckn.len) == 0))
                        return participant;
        }

@@ -748,6 +749,8 @@  ieee802_1x_mka_decode_basic_body(struct ieee802_1x_kay *kay, const u8 *mka_msg,
        struct ieee802_1x_mka_participant *participant;
        const struct ieee802_1x_mka_basic_body *body;
        struct ieee802_1x_kay_peer *peer;
+       size_t ckn_len;
+       size_t body_len;

        body = (const struct ieee802_1x_mka_basic_body *) mka_msg;

@@ -761,7 +764,9 @@  ieee802_1x_mka_decode_basic_body(struct ieee802_1x_kay *kay, const u8 *mka_msg,
                return NULL;
        }

-       participant = ieee802_1x_kay_get_participant(kay, body->ckn);
+       body_len = get_mka_param_body_len(body);
+       ckn_len = body_len - (sizeof(struct ieee802_1x_mka_basic_body) - MKA_HDR_LEN);
+       participant = ieee802_1x_kay_get_participant(kay, body->ckn, ckn_len);
        if (!participant) {
                wpa_printf(MSG_DEBUG, "Peer is not included in my CA");
                return NULL;
@@ -2856,6 +2861,7 @@  static int ieee802_1x_kay_mkpdu_sanity_check(struct ieee802_1x_kay *kay,
        size_t mka_msg_len;
        struct ieee802_1x_mka_participant *participant;
        size_t body_len;
+       size_t ckn_len;
        u8 icv[MAX_ICV_LEN];
        u8 *msg_icv;

@@ -2895,8 +2901,16 @@  static int ieee802_1x_kay_mkpdu_sanity_check(struct ieee802_1x_kay *kay,
                return -1;
        }

+       ckn_len = body_len - (sizeof(struct ieee802_1x_mka_basic_body) - MKA_HDR_LEN);
+       if ((ckn_len < 1) || (ckn_len > MAX_CKN_LEN)) {
+               wpa_printf(MSG_ERROR,
+                          "KaY: Received EAPOL-MKA CKN Length (%zu bytes) is out of range (<=%u bytes)",
+                          ckn_len, MAX_CKN_LEN);
+               return -1;
+       }
+
        /* CKN should be owned by I */
-       participant = ieee802_1x_kay_get_participant(kay, body->ckn);
+       participant = ieee802_1x_kay_get_participant(kay, body->ckn, ckn_len);
        if (!participant) {
                wpa_printf(MSG_DEBUG, "CKN is not included in my CA");
                return -1;
@@ -3403,7 +3417,7 @@  ieee802_1x_kay_delete_mka(struct ieee802_1x_kay *kay, struct mka_key_name *ckn)
        wpa_printf(MSG_DEBUG, "KaY: participant removed");

        /* get the participant */
-       participant = ieee802_1x_kay_get_participant(kay, ckn->name);
+       participant = ieee802_1x_kay_get_participant(kay, ckn->name, ckn->len);
        if (!participant) {
                wpa_hexdump(MSG_DEBUG, "KaY: participant is not found",
                            ckn->name, ckn->len);
@@ -3462,7 +3476,7 @@  void ieee802_1x_kay_mka_participate(struct ieee802_1x_kay *kay,
        if (!kay || !ckn)
                return;

-       participant = ieee802_1x_kay_get_participant(kay, ckn->name);
+       participant = ieee802_1x_kay_get_participant(kay, ckn->name, ckn->len);
        if (!participant)
                return;