From: Arran Cudbard-Bell Date: Thu, 1 Apr 2021 22:10:10 +0000 (+0100) Subject: Move MAC validation into eap-sim/aka/aka' module X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=d685ae37101d813c8c05a024e2a8e0d4462a969e;p=thirdparty%2Ffreeradius-server.git Move MAC validation into eap-sim/aka/aka' module --- diff --git a/src/lib/eap_aka_sim/module.c b/src/lib/eap_aka_sim/module.c index 73071631900..a7d659e0125 100644 --- a/src/lib/eap_aka_sim/module.c +++ b/src/lib/eap_aka_sim/module.c @@ -360,8 +360,8 @@ unlang_action_t eap_aka_sim_process(rlm_rcode_t *p_result, module_ctx_t const *m } break; - case FR_SUBTYPE_VALUE_AKA_CHALLENGE: case FR_SUBTYPE_VALUE_AKA_SIM_REAUTHENTICATION: + case FR_SUBTYPE_VALUE_AKA_CHALLENGE: /* * Include our copy of the checkcode if we've been * calculating it. This is put in the control list @@ -374,7 +374,7 @@ unlang_action_t eap_aka_sim_process(rlm_rcode_t *p_result, module_ctx_t const *m uint8_t *checkcode; fr_pair_t *vp; - MEM(pair_update_control(&vp, attr_eap_aka_sim_checkcode) >= 0); + MEM(pair_append_control(&vp, attr_eap_aka_sim_checkcode) >= 0); if (fr_aka_sim_crypto_finalise_checkcode(vp, &checkcode, mod_session->checkcode_state) < 0) { RPWDEBUG("Failed calculating checkcode"); pair_delete_control(vp); @@ -382,6 +382,29 @@ unlang_action_t eap_aka_sim_process(rlm_rcode_t *p_result, module_ctx_t const *m fr_pair_value_memdup_buffer_shallow(vp, checkcode, false); /* Buffer already in the correct ctx */ } + FALL_THROUGH; + + case FR_SUBTYPE_VALUE_SIM_CHALLENGE: + { + fr_pair_t *vp; + ssize_t slen; + uint8_t *buff; + + MEM(pair_append_control(&vp, attr_eap_aka_sim_mac) >= 0); + fr_pair_value_mem_alloc(vp, &buff, AKA_SIM_MAC_DIGEST_SIZE, false); + + slen = fr_aka_sim_crypto_sign_packet(buff, eap_session->this_round->response, true, + mod_session->ctx.hmac_md, + mod_session->ctx.k_aut, + mod_session->ctx.k_aut_len, + mod_session->response_hmac_extra, + mod_session->response_hmac_extra_len); + if (slen <= 0) { + RPEDEBUG("AT_MAC calculation failed"); + pair_delete_control(vp); + RETURN_MODULE_FAIL; + } + } break; default: diff --git a/src/lib/eap_aka_sim/state_machine.c b/src/lib/eap_aka_sim/state_machine.c index c4179a6483a..97c7c376c54 100644 --- a/src/lib/eap_aka_sim/state_machine.c +++ b/src/lib/eap_aka_sim/state_machine.c @@ -529,10 +529,10 @@ static int checkcode_validate(request_t *request) peer_checkcode = fr_pair_find_by_da(&request->request_pairs, attr_eap_aka_sim_checkcode); if (peer_checkcode) { if (fr_pair_cmp(peer_checkcode, our_checkcode) == 0) { - RDEBUG2("Received checkcode matches calculated checkcode"); + RDEBUG2("Received AT_CHECKCODE matches calculated AT_CHECKCODE"); return 0; } else { - REDEBUG("Received checkcode does not match calculated checkcode"); + REDEBUG("Received AT_CHECKCODE does not match calculated AT_CHECKCODE"); RHEXDUMP_INLINE2(peer_checkcode->vp_octets, peer_checkcode->vp_length, "Received"); RHEXDUMP_INLINE2(our_checkcode->vp_octets, our_checkcode->vp_length, "Expected"); return -1; @@ -547,6 +547,52 @@ static int checkcode_validate(request_t *request) return 1; } +/** Check &control.mac matches &reply.mac + * + * @param[in] request The current request. + * @return + * - 0 if the check was successful. + * - -1 if the check failed. + */ +static int mac_validate(request_t *request) +{ + fr_pair_t *peer_mac, *our_mac; + /* + * mac validation + * + * The actual cryptographic calculations are + * done by the calling module, we just check + * the result. + */ + our_mac = fr_pair_find_by_da(&request->control_pairs, attr_eap_aka_sim_mac); + if (!our_mac) { + REDEBUG("Missing &control.%s", attr_eap_aka_sim_mac->name); + return -1; + + } + + /* + * If the peer doesn't include a mac then that + * means they don't support it, and we can't validate + * their view of the identity packets. + */ + peer_mac = fr_pair_find_by_da(&request->request_pairs, attr_eap_aka_sim_mac); + if (!peer_mac) { + REDEBUG("Peer didn't include AT_MAC"); + return -1; + } + + if (fr_pair_cmp(peer_mac, our_mac) != 0) { + REDEBUG("Received AT_MAC does not match calculated AT_MAC"); + RHEXDUMP_INLINE2(peer_mac->vp_octets, peer_mac->vp_length, "Received"); + RHEXDUMP_INLINE2(our_mac->vp_octets, our_mac->vp_length, "Expected"); + return -1; + } + + RDEBUG2("Received AT_MAC matches calculated AT_MAC"); + return 0; +} + /** Set the crypto identity from a received identity * */ @@ -1591,48 +1637,17 @@ RESUME(recv_common_client_error) */ RESUME(recv_common_reauthentication_response) { - eap_session_t *eap_session = eap_session_get(request->parent); eap_aka_sim_session_t *eap_aka_sim_session = talloc_get_type_abort(rctx, eap_aka_sim_session_t); - uint8_t calc_mac[AKA_SIM_MAC_DIGEST_SIZE]; - ssize_t slen; - fr_pair_t *mac; SECTION_RCODE_PROCESS; - mac = fr_pair_find_by_da(&request->request_pairs, attr_eap_aka_sim_mac); - if (!mac) { - REDEBUG("Missing AT_MAC attribute"); + /* + * Validate mac + */ + if (mac_validate(request) < 0) { failure: return STATE_TRANSITION(common_failure_notification); } - if (mac->vp_length != AKA_SIM_MAC_DIGEST_SIZE) { - REDEBUG("MAC has incorrect length, expected %u bytes got %zu bytes", - AKA_SIM_MAC_DIGEST_SIZE, mac->vp_length); - goto failure; - } - - // TODO: MAC validation should happen in the decoder not the module - slen = fr_aka_sim_crypto_sign_packet(calc_mac, eap_session->this_round->response, true, - eap_aka_sim_session->mac_md, - eap_aka_sim_session->keys.k_aut, eap_aka_sim_session->keys.k_aut_len, - eap_aka_sim_session->keys.reauth.nonce_s, - sizeof(eap_aka_sim_session->keys.reauth.nonce_s)); - if (slen < 0) { - RPEDEBUG("Failed calculating MAC"); - goto failure; - } else if (slen == 0) { - REDEBUG("Zero length AT_MAC attribute"); - goto failure; - } - - if (memcmp(mac->vp_octets, calc_mac, sizeof(calc_mac)) == 0) { - RDEBUG2("Received MAC matches calculated MAC"); - } else { - REDEBUG("Received MAC does not match calculated MAC"); - RHEXDUMP_INLINE2(mac->vp_octets, AKA_SIM_MAC_DIGEST_SIZE, "Received"); - RHEXDUMP_INLINE2(calc_mac, AKA_SIM_MAC_DIGEST_SIZE, "Expected"); - goto failure; - } /* * Validate the checkcode @@ -2147,48 +2162,18 @@ RESUME(recv_aka_authentication_reject) */ RESUME(recv_aka_challenge_response) { - eap_session_t *eap_session = eap_session_get(request->parent); eap_aka_sim_session_t *eap_aka_sim_session = talloc_get_type_abort(rctx, eap_aka_sim_session_t); - uint8_t calc_mac[AKA_SIM_MAC_DIGEST_SIZE]; - ssize_t slen; - fr_pair_t *vp = NULL, *mac; + fr_pair_t *vp = NULL; SECTION_RCODE_PROCESS; - - mac = fr_pair_find_by_da(&request->request_pairs, attr_eap_aka_sim_mac); - if (!mac) { - REDEBUG("Missing AT_MAC attribute"); + /* + * Validate mac + */ + if (mac_validate(request) < 0) { failure: return STATE_TRANSITION(common_failure_notification); } - if (mac->vp_length != AKA_SIM_MAC_DIGEST_SIZE) { - REDEBUG("MAC has incorrect length, expected %u bytes got %zu bytes", - AKA_SIM_MAC_DIGEST_SIZE, mac->vp_length); - goto failure; - } - - // TODO: MAC validation should happen in the decoder not the state machine - slen = fr_aka_sim_crypto_sign_packet(calc_mac, eap_session->this_round->response, true, - eap_aka_sim_session->mac_md, - eap_aka_sim_session->keys.k_aut, eap_aka_sim_session->keys.k_aut_len, - NULL, 0); - if (slen < 0) { - RPEDEBUG("Failed calculating MAC"); - goto failure; - } else if (slen == 0) { - REDEBUG("Zero length AT_MAC attribute"); - goto failure; - } - - if (memcmp(mac->vp_octets, calc_mac, sizeof(calc_mac)) == 0) { - RDEBUG2("Received MAC matches calculated MAC"); - } else { - REDEBUG("Received MAC does not match calculated MAC"); - RHEXDUMP_INLINE2(mac->vp_octets, AKA_SIM_MAC_DIGEST_SIZE, "Received"); - RHEXDUMP_INLINE2(calc_mac, AKA_SIM_MAC_DIGEST_SIZE, "Expected"); - goto failure; - } /* * Validate the checkcode @@ -2556,15 +2541,10 @@ STATE_GUARD(aka_challenge) */ RESUME(recv_sim_challenge_response) { - eap_session_t *eap_session = eap_session_get(request->parent); eap_aka_sim_session_t *eap_aka_sim_session = talloc_get_type_abort(rctx, eap_aka_sim_session_t); uint8_t sres_cat[AKA_SIM_VECTOR_GSM_SRES_SIZE * 3]; uint8_t *p = sres_cat; - uint8_t calc_mac[AKA_SIM_MAC_DIGEST_SIZE]; - ssize_t slen; - fr_pair_t *mac; - SECTION_RCODE_PROCESS; memcpy(p, eap_aka_sim_session->keys.gsm.vector[0].sres, AKA_SIM_VECTOR_GSM_SRES_SIZE); @@ -2573,39 +2553,10 @@ RESUME(recv_sim_challenge_response) p += AKA_SIM_VECTOR_GSM_SRES_SIZE; memcpy(p, eap_aka_sim_session->keys.gsm.vector[2].sres, AKA_SIM_VECTOR_GSM_SRES_SIZE); - mac = fr_pair_find_by_da(&request->request_pairs, attr_eap_aka_sim_mac); - if (!mac) { - REDEBUG("Missing AT_MAC attribute"); - failure: - return STATE_TRANSITION(common_failure_notification); - } - if (mac->vp_length != AKA_SIM_MAC_DIGEST_SIZE) { - REDEBUG("MAC has incorrect length, expected %u bytes got %zu bytes", - AKA_SIM_MAC_DIGEST_SIZE, mac->vp_length); - goto failure; - } - - // TODO: MAC validation should be in the decoder - slen = fr_aka_sim_crypto_sign_packet(calc_mac, eap_session->this_round->response, true, - eap_aka_sim_session->mac_md, - eap_aka_sim_session->keys.k_aut, eap_aka_sim_session->keys.k_aut_len, - sres_cat, sizeof(sres_cat)); - if (slen < 0) { - RPEDEBUG("Failed calculating MAC"); - goto failure; - } else if (slen == 0) { - REDEBUG("Zero length AT_MAC attribute"); - goto failure; - } - - if (memcmp(mac->vp_octets, calc_mac, sizeof(calc_mac)) == 0) { - RDEBUG2("Received MAC matches calculated MAC"); - } else { - REDEBUG("Received MAC does not match calculated MAC"); - RHEXDUMP_INLINE2(mac->vp_octets, AKA_SIM_MAC_DIGEST_SIZE, "Received"); - RHEXDUMP_INLINE2(calc_mac, AKA_SIM_MAC_DIGEST_SIZE, "Expected"); - goto failure; - } + /* + * Validate mac + */ + if (mac_validate(request) < 0) return STATE_TRANSITION(common_failure_notification); eap_aka_sim_session->challenge_success = true;