[PATCH 13/15] mka: do not ignore MKPDU parameter set decoding failures

msiedzik at extremenetworks.com msiedzik at extremenetworks.com
Fri Mar 2 12:11:01 PST 2018


From: Mike Siedzik <msiedzik at extremenetworks.com>

The status values returned by mka_param_body_handler.body_rx functions
are currently ignored by ieee802_1x_kay_decode_mkpdu().  If a failure is
detected the KaY should (a) stop processing the MKDPU and (b) do not
update the associated peer's liveliness.

IEEE802.1X-2010's Table 11-7 MKPDU Parameter sets and Clause 11.11.3
Encoding MKPDUs dictate that MKA_SAK_USE (set type 3) will always be
encoded before MKA_DISTRIBUTED_SAK (set type 4) in MKPDUs.  Due to
hostap's implementation of mka_param_body_handler, the code will always
decode MKA_SAK_USE before MKA_DISTRIBUTED_SAK.  When MKA_DISTRUBUTED_SAK
contains a new SAK the code should decode MKA_DISTRUBUTED_SAK first so
that the lastest SAK is in known before decoding MKA_SAK_USE.

The ideal solution would be to make two passes at MKDPU decoding: the
first pass decodes MKA_DISTRIBUTED_SAK, the second pass decodes all
other parameter sets.

A simpler and less risky solution is presented here: ignore MKA_SAK_USE
failures if MKA_DISTRIBUTED_SAK is also present.  The new SAK will be
saved so that the next MKPDU's MKA_SAK_USE can be properly decoded.
This is basically what the code prior to this commit was doing (by
ignoring all errors).

Also, the only real recourse the KaY has when detecting any bad
parameter set is to ignore the MKPDU by not updating the corresponding
peer's liveliness timer, 'peer->expire'.

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

diff --git a/src/pae/ieee802_1x_kay.c b/src/pae/ieee802_1x_kay.c
index 4d61cb32b..7945cc898 100644
--- a/src/pae/ieee802_1x_kay.c
+++ b/src/pae/ieee802_1x_kay.c
@@ -831,7 +831,6 @@ ieee802_1x_mka_decode_basic_body(struct ieee802_1x_kay *kay, const u8 *mka_msg,
                peer->key_server_priority = body->priority;
        } else if (peer->mn < be_to_host32(body->actor_mn)) {
                peer->mn = be_to_host32(body->actor_mn);
-               peer->expire = time(NULL) + MKA_LIFE_TIME / 1000;
                peer->macsec_desired = body->macsec_desired;
                peer->macsec_capability = body->macsec_capability;
                peer->is_key_server = (Boolean) body->key_server;
@@ -1076,7 +1075,6 @@ static int ieee802_1x_mka_decode_live_peer_body(
                peer = ieee802_1x_kay_get_peer(participant, peer_mi->mi);
                if (peer) {
                        peer->mn = peer_mn;
-                       peer->expire = time(NULL) + MKA_LIFE_TIME / 1000;
                } else if (!ieee802_1x_kay_create_potential_peer(
                                participant, peer_mi->mi, peer_mn)) {
                        return -1;
@@ -1350,7 +1348,7 @@ ieee802_1x_mka_decode_sak_use_body(
                        }
                }
                if (!found) {
-                       wpa_printf(MSG_WARNING, "KaY: Latest key is invalid");
+                       wpa_printf(MSG_INFO, "KaY: Latest key is invalid");
                        return -1;
                }
                if (os_memcmp(participant->lki.mi, body->lsrv_mi,
@@ -3041,12 +3039,14 @@ static int ieee802_1x_kay_decode_mkpdu(struct ieee802_1x_kay *kay,
 {
        struct ieee802_1x_mka_participant *participant;
        struct ieee802_1x_mka_hdr *hdr;
+       struct ieee802_1x_kay_peer *peer;
        size_t body_len;
        size_t left_len;
        u8 body_type;
        int i;
        const u8 *pos;
        Boolean handled[256];
+       Boolean bad_sak_use = FALSE; /* Error detected while processing SAK Use parameter set */

        if (ieee802_1x_kay_mkpdu_sanity_check(kay, buf, len))
                return -1;
@@ -3121,8 +3121,26 @@ static int ieee802_1x_kay_decode_mkpdu(struct ieee802_1x_kay *kay,
                handled[body_type] = TRUE;
                if (body_type < ARRAY_SIZE(mka_body_handler) &&
                    mka_body_handler[body_type].body_rx) {
-                       mka_body_handler[body_type].body_rx
-                               (participant, pos, left_len);
+                       if (mka_body_handler[body_type].body_rx
+                               (participant, pos, left_len) != 0) {
+                               /* Handle parameter set failure */
+                               if (body_type == MKA_SAK_USE) {
+                                       /* Ideally DIST-SAK should be processed before
+                                        * SAK-USE. Unfortunately IEEE8021X-2010 Clause
+                                        * 11.11.3 Encoding MKPDUs states SAK-USE(3)
+                                        * must always be encoded before DIST-SAK(4).
+                                        * Rather than redesigning mka_body_handler so
+                                        * that it somehow processes DIST-SAK before
+                                        * SAK-USE, just ignore SAK-USE failures if
+                                        * DIST-SAK is also present in this MKPDU. */
+                                       bad_sak_use = TRUE;
+                               } else {
+                                       wpa_printf(MSG_INFO,
+                                                  "KaY: Discarding Rx MKPDU: decode of parameter set type (%d) failed",
+                                                  body_type);
+                                       return -1;
+                               }
+                       }
                } else {
                        wpa_printf(MSG_ERROR,
                                   "The type %d is not supported in this MKA version %d",
@@ -3130,6 +3148,18 @@ static int ieee802_1x_kay_decode_mkpdu(struct ieee802_1x_kay *kay,
                }
        }

+       if (bad_sak_use && !handled[MKA_DISTRIBUTED_SAK]) {
+               wpa_printf(MSG_INFO,
+                          "KaY: Discarding Rx MKPDU: decode of parameter set type (%d) failed",
+                          MKA_SAK_USE);
+               return -1;
+       }
+
+       /* Only update live peer watchdog after successful decode of all parameter sets */
+       peer = ieee802_1x_kay_get_peer(participant, participant->current_peer_id.mi);
+       if (peer)
+               peer->expire = time(NULL) + MKA_LIFE_TIME / 1000;
+
        kay->active = TRUE;
        participant->retry_count = 0;
        participant->active = TRUE;
--
2.11.1


________________________________

DISCLAIMER:
This e-mail and any attachments to it may contain confidential and proprietary material and is solely for the use of the intended recipient. Any review, use, disclosure, distribution or copying of this transmittal is prohibited except by or on behalf of the intended recipient. If you have received this transmittal in error, please notify the sender and destroy this e-mail and any attachments and all copies, whether electronic or printed.




More information about the Hostap mailing list