[06/15] mka: KaY setting Parameter Set Body Length incorrectly

Message ID 20180302201103.16264-7-msiedzik@extremenetworks.com
State New
Headers show
Series
  • MKA bugfixes and enhancements
Related show

Commit Message

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

Per IEEE802.1X-2010 Clause 11.11 EAPOL-MKA, each parameter set must be
padded to a multiple of 4 octets.  In order for the length of variable
length parameters, such as CAK Name, to be correctly decoded the
parameter set body length must not include the length of null padding
octets.

When allocating buffer space for the parameter set (e.g., wpabuf_put())
the padded length must be used.  When setting the 'Parameter set body
length' within the parameter set the unpadded length must be used.

Consider the case were the length of a PSK's CKN is not a multiple of 4
octets.  Currently ieee802_1x_mka_encode_basic_body() will correctly
reserve the padded number buffer bytes.  However it will incorrectly
set ieee8021_x_mka_hdr->length and ->length1 to the padded number of
bytes.  The receiver will not be able to recover the original CKN
length.

Note that the hostap will successfully interoperate with itself because
both sides incorrectly calculate CKN length.  The problem is only seen
when interoperating with non-hostap KaY's.

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

--
2.11.1

Patch

diff --git a/src/pae/ieee802_1x_kay.c b/src/pae/ieee802_1x_kay.c
index 17519ae69..70fda1f2d 100644
--- a/src/pae/ieee802_1x_kay.c
+++ b/src/pae/ieee802_1x_kay.c
@@ -681,7 +681,7 @@  ieee802_1x_mka_basic_body_length(struct ieee802_1x_mka_participant *participant)

        length = sizeof(struct ieee802_1x_mka_basic_body);
        length += participant->ckn.len;
-       return MKA_ALIGN_LENGTH(length);
+       return length;
 }


@@ -697,7 +697,7 @@  ieee802_1x_mka_encode_basic_body(
        struct ieee802_1x_kay *kay = participant->kay;
        unsigned int length = ieee802_1x_mka_basic_body_length(participant);

-       body = wpabuf_put(buf, length);
+       body = wpabuf_put(buf, MKA_ALIGN_LENGTH(length));

        body->version = kay->mka_version;
        body->priority = kay->actor_priority;
@@ -856,7 +856,7 @@  ieee802_1x_mka_get_live_peer_length(
                         struct ieee802_1x_kay_peer, list)
                len += sizeof(struct ieee802_1x_mka_peer_id);

-       return MKA_ALIGN_LENGTH(len);
+       return len;
 }


@@ -916,7 +916,7 @@  ieee802_1x_mka_get_potential_peer_length(
                         struct ieee802_1x_kay_peer, list)
                len += sizeof(struct ieee802_1x_mka_peer_id);

-       return MKA_ALIGN_LENGTH(len);
+       return len;
 }


@@ -1139,7 +1139,7 @@  ieee802_1x_mka_get_sak_use_length(
        if (participant->kay->macsec_desired && participant->advised_desired)
                length = sizeof(struct ieee802_1x_mka_sak_use_body);

-       return MKA_ALIGN_LENGTH(length);
+       return length;
 }


@@ -1189,7 +1189,7 @@  ieee802_1x_mka_encode_sak_use_body(
        u32 pn = 1;

        length = ieee802_1x_mka_get_sak_use_length(participant);
-       body = wpabuf_put(buf, length);
+       body = wpabuf_put(buf, MKA_ALIGN_LENGTH(length));

        body->type = MKA_SAK_USE;
        set_mka_param_body_len(body, length - MKA_HDR_LEN);
@@ -1439,7 +1439,7 @@  ieee802_1x_mka_get_dist_sak_length(
                length += cipher_suite_tbl[cs_index].sak_len + 8;
        }

-       return MKA_ALIGN_LENGTH(length);
+       return length;
 }


@@ -1458,7 +1458,7 @@  ieee802_1x_mka_encode_dist_sak_body(
        int sak_pos;

        length = ieee802_1x_mka_get_dist_sak_length(participant);
-       body = wpabuf_put(buf, length);
+       body = wpabuf_put(buf, MKA_ALIGN_LENGTH(length));
        body->type = MKA_DISTRIBUTED_SAK;
        set_mka_param_body_len(body, length - MKA_HDR_LEN);
        if (length == MKA_HDR_LEN) {
@@ -1683,7 +1683,7 @@  ieee802_1x_mka_get_icv_length(struct ieee802_1x_mka_participant *participant)
        length = sizeof(struct ieee802_1x_mka_icv_body);
        length += mka_alg_tbl[participant->kay->mka_algindex].icv_len;

-       return MKA_ALIGN_LENGTH(length);
+       return length;
 }


@@ -1713,7 +1713,7 @@  ieee802_1x_mka_encode_icv_body(struct ieee802_1x_mka_participant *participant,

        if (length != DEFAULT_ICV_LEN)
                length -= MKA_HDR_LEN;
-       os_memcpy(wpabuf_put(buf, length), cmac, length);
+       os_memcpy(wpabuf_put(buf, MKA_ALIGN_LENGTH(length - MKA_HDR_LEN)), cmac, length - MKA_HDR_LEN);

        return 0;
 }
@@ -2297,7 +2297,7 @@  ieee802_1x_participant_send_mkpdu(
        for (i = 0; i < ARRAY_SIZE(mka_body_handler); i++) {
                if (mka_body_handler[i].body_present &&
                    mka_body_handler[i].body_present(participant))
-                       length += mka_body_handler[i].body_length(participant);
+                       length += MKA_ALIGN_LENGTH(mka_body_handler[i].body_length(participant));
        }

        buf = wpabuf_alloc(length);
@@ -2931,7 +2931,7 @@  static int ieee802_1x_kay_mkpdu_sanity_check(struct ieee802_1x_kay *kay,
        ieee802_1x_mka_dump_basic_body(body);
        body_len = get_mka_param_body_len(body);
        /* EAPOL-MKA body should comprise basic parameter set and ICV */
-       if (mka_msg_len < MKA_HDR_LEN + body_len + DEFAULT_ICV_LEN) {
+       if (mka_msg_len < MKA_HDR_LEN + MKA_ALIGN_LENGTH(body_len) + DEFAULT_ICV_LEN) {
                wpa_printf(MSG_ERROR,
                           "KaY: Received EAPOL-MKA Packet Body Length (%zu bytes) is less than the Basic Parameter Set Header Length (%zu bytes) + the Basic Parameter Set Body Length (%zu bytes) + %d bytes of ICV",
                           mka_msg_len, MKA_HDR_LEN,
@@ -3020,7 +3020,7 @@  static int ieee802_1x_kay_decode_mkpdu(struct ieee802_1x_kay *kay,

        /* to skip basic parameter set */
        hdr = (struct ieee802_1x_mka_hdr *) pos;
-       body_len = get_mka_param_body_len(hdr);
+       body_len = MKA_ALIGN_LENGTH(get_mka_param_body_len(hdr));
        pos += body_len + MKA_HDR_LEN;
        left_len -= body_len + MKA_HDR_LEN;

@@ -3060,7 +3060,7 @@  static int ieee802_1x_kay_decode_mkpdu(struct ieee802_1x_kay *kay,
             pos += body_len + MKA_HDR_LEN,
                     left_len -= body_len + MKA_HDR_LEN) {
                hdr = (struct ieee802_1x_mka_hdr *) pos;
-               body_len = get_mka_param_body_len(hdr);
+               body_len = MKA_ALIGN_LENGTH(get_mka_param_body_len(hdr));
                body_type = get_mka_param_body_type(hdr);

                if (body_type == MKA_ICV_INDICATOR)