mka: Do not ignore MKPDU parameter set decoding failures

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.

IEEE Std 802.1X-2010, Table 11-7 (MKPDU parameter sets) and 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
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 latest 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@extremenetworks.com>
This commit is contained in:
Mike Siedzik 2018-02-20 14:28:43 -05:00 committed by Jouni Malinen
parent bab1d0d359
commit db9ca18bbf

View file

@ -842,7 +842,6 @@ ieee802_1x_mka_decode_basic_body(struct ieee802_1x_kay *kay, const u8 *mka_msg,
peer->key_server_priority = body->priority; peer->key_server_priority = body->priority;
} else if (peer->mn < be_to_host32(body->actor_mn)) { } else if (peer->mn < be_to_host32(body->actor_mn)) {
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_desired = body->macsec_desired;
peer->macsec_capability = body->macsec_capability; peer->macsec_capability = body->macsec_capability;
peer->is_key_server = (Boolean) body->key_server; peer->is_key_server = (Boolean) body->key_server;
@ -1087,7 +1086,6 @@ static int ieee802_1x_mka_decode_live_peer_body(
peer = ieee802_1x_kay_get_peer(participant, peer_mi->mi); peer = ieee802_1x_kay_get_peer(participant, peer_mi->mi);
if (peer) { if (peer) {
peer->mn = peer_mn; peer->mn = peer_mn;
peer->expire = time(NULL) + MKA_LIFE_TIME / 1000;
} else if (!ieee802_1x_kay_create_potential_peer( } else if (!ieee802_1x_kay_create_potential_peer(
participant, peer_mi->mi, peer_mn)) { participant, peer_mi->mi, peer_mn)) {
return -1; return -1;
@ -1363,7 +1361,7 @@ ieee802_1x_mka_decode_sak_use_body(
} }
} }
if (!found) { if (!found) {
wpa_printf(MSG_WARNING, "KaY: Latest key is invalid"); wpa_printf(MSG_INFO, "KaY: Latest key is invalid");
return -1; return -1;
} }
if (os_memcmp(participant->lki.mi, body->lsrv_mi, if (os_memcmp(participant->lki.mi, body->lsrv_mi,
@ -3031,12 +3029,15 @@ static int ieee802_1x_kay_decode_mkpdu(struct ieee802_1x_kay *kay,
{ {
struct ieee802_1x_mka_participant *participant; struct ieee802_1x_mka_participant *participant;
struct ieee802_1x_mka_hdr *hdr; struct ieee802_1x_mka_hdr *hdr;
struct ieee802_1x_kay_peer *peer;
size_t body_len; size_t body_len;
size_t left_len; size_t left_len;
u8 body_type; u8 body_type;
int i; int i;
const u8 *pos; const u8 *pos;
Boolean handled[256]; 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)) if (ieee802_1x_kay_mkpdu_sanity_check(kay, buf, len))
return -1; return -1;
@ -3111,8 +3112,26 @@ static int ieee802_1x_kay_decode_mkpdu(struct ieee802_1x_kay *kay,
handled[body_type] = TRUE; handled[body_type] = TRUE;
if (body_type < ARRAY_SIZE(mka_body_handler) && if (body_type < ARRAY_SIZE(mka_body_handler) &&
mka_body_handler[body_type].body_rx) { mka_body_handler[body_type].body_rx) {
mka_body_handler[body_type].body_rx if (mka_body_handler[body_type].body_rx
(participant, pos, left_len); (participant, pos, left_len) != 0) {
/* Handle parameter set failure */
if (body_type != MKA_SAK_USE) {
wpa_printf(MSG_INFO,
"KaY: Discarding Rx MKPDU: decode of parameter set type (%d) failed",
body_type);
return -1;
}
/* Ideally DIST-SAK should be processed before
* SAK-USE. Unfortunately IEEE Std 802.1X-2010,
* 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 { } else {
wpa_printf(MSG_ERROR, wpa_printf(MSG_ERROR,
"The type %d is not supported in this MKA version %d", "The type %d is not supported in this MKA version %d",
@ -3120,6 +3139,20 @@ 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; kay->active = TRUE;
participant->retry_count = 0; participant->retry_count = 0;
participant->active = TRUE; participant->active = TRUE;