From f070e2db6db608c61a9178082fa83cf04aeb201a Mon Sep 17 00:00:00 2001 From: Kapil Gupta Date: Tue, 26 Apr 2022 15:23:43 +0530 Subject: [PATCH] wpa_supplicant: Fix issues reported by coverity --- .../esp_supplicant/src/esp_wps.c | 4 ++ components/wpa_supplicant/src/ap/ap_config.h | 1 - components/wpa_supplicant/src/ap/wpa_auth.c | 63 ++++--------------- components/wpa_supplicant/src/ap/wpa_auth.h | 1 - components/wpa_supplicant/src/ap/wpa_auth_i.h | 11 ---- .../wpa_supplicant/src/ap/wpa_auth_ie.c | 32 ---------- .../wpa_supplicant/src/ap/wpa_auth_ie.h | 10 --- components/wpa_supplicant/src/common/sae.c | 5 +- .../wpa_supplicant/src/common/wpa_common.c | 4 -- .../wpa_supplicant/src/common/wpa_common.h | 23 ------- .../src/crypto/crypto_mbedtls.c | 50 +++++++++------ .../src/eap_peer/eap_fast_pac.c | 1 + .../wpa_supplicant/src/rsn_supp/wpa_ie.h | 10 --- 13 files changed, 49 insertions(+), 166 deletions(-) diff --git a/components/wpa_supplicant/esp_supplicant/src/esp_wps.c b/components/wpa_supplicant/esp_supplicant/src/esp_wps.c index 7141e6c735..68502972d4 100644 --- a/components/wpa_supplicant/esp_supplicant/src/esp_wps.c +++ b/components/wpa_supplicant/esp_supplicant/src/esp_wps.c @@ -808,6 +808,10 @@ int wps_process_wps_mX_req(u8 *ubuf, int len, enum wps_process_res *res) } if ((flag & WPS_MSG_FLAG_MORE) || wps_buf != NULL) {//frag msg + if (tlen > 50000) { + wpa_printf(MSG_ERROR, "EAP-WSC: Invalid Message Length"); + return ESP_FAIL; + } wpa_printf(MSG_DEBUG, "rx frag msg id:%d, flag:%d, frag_len: %d, tot_len: %d, be_tot_len:%d", sm->current_identifier, flag, frag_len, tlen, be_tot_len); if (ESP_OK != wps_enrollee_process_msg_frag(&wps_buf, tlen, tbuf, frag_len, flag)) { if (wps_buf) { diff --git a/components/wpa_supplicant/src/ap/ap_config.h b/components/wpa_supplicant/src/ap/ap_config.h index 1700d26747..71d9be9091 100644 --- a/components/wpa_supplicant/src/ap/ap_config.h +++ b/components/wpa_supplicant/src/ap/ap_config.h @@ -219,7 +219,6 @@ struct hostapd_bss_config { int rsn_pairwise; int rsn_preauth; char *rsn_preauth_interfaces; - int peerkey; #ifdef CONFIG_IEEE80211R /* IEEE 802.11r - Fast BSS Transition */ diff --git a/components/wpa_supplicant/src/ap/wpa_auth.c b/components/wpa_supplicant/src/ap/wpa_auth.c index dd31225a83..2031448eee 100644 --- a/components/wpa_supplicant/src/ap/wpa_auth.c +++ b/components/wpa_supplicant/src/ap/wpa_auth.c @@ -585,8 +585,7 @@ void wpa_receive(struct wpa_authenticator *wpa_auth, struct wpa_state_machine *s struct ieee802_1x_hdr *hdr; struct wpa_eapol_key *key; u16 key_info, key_data_length; - enum { PAIRWISE_2, PAIRWISE_4, GROUP_2, REQUEST, - SMK_M1, SMK_M3, SMK_ERROR } msg; + enum { PAIRWISE_2, PAIRWISE_4, GROUP_2, REQUEST } msg; struct wpa_eapol_ie_parse kde; int ft; const u8 *eapol_key_ie; @@ -645,16 +644,12 @@ void wpa_receive(struct wpa_authenticator *wpa_auth, struct wpa_state_machine *s /* FIX: verify that the EAPOL-Key frame was encrypted if pairwise keys * are set */ - if ((key_info & (WPA_KEY_INFO_SMK_MESSAGE | WPA_KEY_INFO_REQUEST)) == - (WPA_KEY_INFO_SMK_MESSAGE | WPA_KEY_INFO_REQUEST)) { - if (key_info & WPA_KEY_INFO_ERROR) { - msg = SMK_ERROR; - } else { - msg = SMK_M1; - } - } else if (key_info & WPA_KEY_INFO_SMK_MESSAGE) { - msg = SMK_M3; - } else if (key_info & WPA_KEY_INFO_REQUEST) { + if (key_info & WPA_KEY_INFO_SMK_MESSAGE) { + wpa_printf(MSG_DEBUG, "WPA: Ignore SMK message"); + return; + } + + if (key_info & WPA_KEY_INFO_REQUEST) { msg = REQUEST; } else if (!(key_info & WPA_KEY_INFO_KEY_TYPE)) { msg = GROUP_2; @@ -664,7 +659,6 @@ void wpa_receive(struct wpa_authenticator *wpa_auth, struct wpa_state_machine *s msg = PAIRWISE_2; } - /* TODO: key_info type validation for PeerKey */ if (msg == REQUEST || msg == PAIRWISE_2 || msg == PAIRWISE_4 || msg == GROUP_2) { u16 ver = key_info & WPA_KEY_INFO_TYPE_MASK; @@ -805,25 +799,6 @@ continue_processing: return; } break; -#ifdef CONFIG_PEERKEY - case SMK_M1: - case SMK_M3: - case SMK_ERROR: - if (!wpa_auth->conf.peerkey) { - wpa_printf( MSG_DEBUG, "RSN: SMK M1/M3/Error, but " - "PeerKey use disabled - ignoring message"); - return; - } - if (!sm->PTK_valid) { - return; - } - break; -#else /* CONFIG_PEERKEY */ - case SMK_M1: - case SMK_M3: - case SMK_ERROR: - return; /* STSL disabled - ignore SMK messages */ -#endif /* CONFIG_PEERKEY */ case REQUEST: break; } @@ -864,22 +839,13 @@ continue_processing: * even though MAC address KDE is not normally encrypted, * supplicant is allowed to encrypt it. */ - if (msg == SMK_ERROR) { -#ifdef CONFIG_PEERKEY - wpa_smk_error(wpa_auth, sm, key); -#endif /* CONFIG_PEERKEY */ - return; - } else if (key_info & WPA_KEY_INFO_ERROR) { + if (key_info & WPA_KEY_INFO_ERROR) { if (wpa_receive_error_report( wpa_auth, sm, !(key_info & WPA_KEY_INFO_KEY_TYPE)) > 0) return; /* STA entry was removed */ } else if (key_info & WPA_KEY_INFO_KEY_TYPE) { wpa_request_new_ptk(sm); -#ifdef CONFIG_PEERKEY - } else if (msg == SMK_M1) { - wpa_smk_m1(wpa_auth, sm, key); -#endif /* CONFIG_PEERKEY */ } else if (key_data_length > 0 && wpa_parse_kde_ies((const u8 *) (key + 1), key_data_length, &kde) == 0 && @@ -915,13 +881,6 @@ continue_processing: wpa_replay_counter_mark_invalid(sm->key_replay, NULL); } -#ifdef CONFIG_PEERKEY - if (msg == SMK_M3) { - wpa_smk_m3(wpa_auth, sm, key); - return; - } -#endif /* CONFIG_PEERKEY */ - wpa_printf( MSG_DEBUG, "wpa_rx: free eapol=%p\n", sm->last_rx_eapol_key); os_free(sm->last_rx_eapol_key); sm->last_rx_eapol_key = (u8 *)os_malloc(data_len); @@ -1053,11 +1012,11 @@ void __wpa_send_eapol(struct wpa_authenticator *wpa_auth, WPA_PUT_BE16(key->key_info, key_info); alg = pairwise ? sm->pairwise : wpa_auth->conf.wpa_group; - WPA_PUT_BE16(key->key_length, wpa_cipher_key_len(alg)); - if (key_info & WPA_KEY_INFO_SMK_MESSAGE) + if (sm->wpa == WPA_VERSION_WPA2 && !pairwise) WPA_PUT_BE16(key->key_length, 0); + else + WPA_PUT_BE16(key->key_length, wpa_cipher_key_len(alg)); - /* FIX: STSL: what to use as key_replay_counter? */ for (i = RSNA_MAX_EAPOL_RETRIES - 1; i > 0; i--) { sm->key_replay[i].valid = sm->key_replay[i - 1].valid; memcpy(sm->key_replay[i].counter, diff --git a/components/wpa_supplicant/src/ap/wpa_auth.h b/components/wpa_supplicant/src/ap/wpa_auth.h index 4979ec6bae..a4a040fd27 100644 --- a/components/wpa_supplicant/src/ap/wpa_auth.h +++ b/components/wpa_supplicant/src/ap/wpa_auth.h @@ -136,7 +136,6 @@ struct wpa_auth_config { int rsn_pairwise; int rsn_preauth; int eapol_version; - int peerkey; int wmm_enabled; int wmm_uapsd; int disable_pmksa_caching; diff --git a/components/wpa_supplicant/src/ap/wpa_auth_i.h b/components/wpa_supplicant/src/ap/wpa_auth_i.h index fba036732e..8b1c51cb90 100644 --- a/components/wpa_supplicant/src/ap/wpa_auth_i.h +++ b/components/wpa_supplicant/src/ap/wpa_auth_i.h @@ -183,17 +183,6 @@ int wpa_auth_for_each_auth(struct wpa_authenticator *wpa_auth, int (*cb)(struct wpa_authenticator *a, void *ctx), void *cb_ctx); -#ifdef CONFIG_PEERKEY -int wpa_stsl_remove(struct wpa_authenticator *wpa_auth, - struct wpa_stsl_negotiation *neg); -void wpa_smk_error(struct wpa_authenticator *wpa_auth, - struct wpa_state_machine *sm, struct wpa_eapol_key *key); -void wpa_smk_m1(struct wpa_authenticator *wpa_auth, - struct wpa_state_machine *sm, struct wpa_eapol_key *key); -void wpa_smk_m3(struct wpa_authenticator *wpa_auth, - struct wpa_state_machine *sm, struct wpa_eapol_key *key); -#endif /* CONFIG_PEERKEY */ - #ifdef CONFIG_IEEE80211R int wpa_write_mdie(struct wpa_auth_config *conf, u8 *buf, size_t len); int wpa_write_ftie(struct wpa_auth_config *conf, const u8 *r0kh_id, diff --git a/components/wpa_supplicant/src/ap/wpa_auth_ie.c b/components/wpa_supplicant/src/ap/wpa_auth_ie.c index 532127c8e5..6ef4bbe7e7 100644 --- a/components/wpa_supplicant/src/ap/wpa_auth_ie.c +++ b/components/wpa_supplicant/src/ap/wpa_auth_ie.c @@ -216,8 +216,6 @@ int wpa_write_rsn_ie(struct wpa_auth_config *conf, u8 *buf, size_t len, capab = 0; if (conf->rsn_preauth) capab |= WPA_CAPABILITY_PREAUTH; - if (conf->peerkey) - capab |= WPA_CAPABILITY_PEERKEY_ENABLED; if (conf->wmm_enabled) { /* 4 PTKSA replay counters when using WMM */ capab |= (RSN_NUM_REPLAY_COUNTERS_16 << 2); @@ -626,36 +624,6 @@ static int wpa_parse_generic(const u8 *pos, const u8 *end, return 0; } -#ifdef CONFIG_PEERKEY - if (pos[1] > RSN_SELECTOR_LEN + 2 && - RSN_SELECTOR_GET(pos + 2) == RSN_KEY_DATA_SMK) { - ie->smk = pos + 2 + RSN_SELECTOR_LEN; - ie->smk_len = pos[1] - RSN_SELECTOR_LEN; - return 0; - } - - if (pos[1] > RSN_SELECTOR_LEN + 2 && - RSN_SELECTOR_GET(pos + 2) == RSN_KEY_DATA_NONCE) { - ie->nonce = pos + 2 + RSN_SELECTOR_LEN; - ie->nonce_len = pos[1] - RSN_SELECTOR_LEN; - return 0; - } - - if (pos[1] > RSN_SELECTOR_LEN + 2 && - RSN_SELECTOR_GET(pos + 2) == RSN_KEY_DATA_LIFETIME) { - ie->lifetime = pos + 2 + RSN_SELECTOR_LEN; - ie->lifetime_len = pos[1] - RSN_SELECTOR_LEN; - return 0; - } - - if (pos[1] > RSN_SELECTOR_LEN + 2 && - RSN_SELECTOR_GET(pos + 2) == RSN_KEY_DATA_ERROR) { - ie->error = pos + 2 + RSN_SELECTOR_LEN; - ie->error_len = pos[1] - RSN_SELECTOR_LEN; - return 0; - } -#endif /* CONFIG_PEERKEY */ - #ifdef CONFIG_IEEE80211W if (pos[1] > RSN_SELECTOR_LEN + 2 && RSN_SELECTOR_GET(pos + 2) == RSN_KEY_DATA_IGTK) { diff --git a/components/wpa_supplicant/src/ap/wpa_auth_ie.h b/components/wpa_supplicant/src/ap/wpa_auth_ie.h index 4999139510..dfcfbd301e 100644 --- a/components/wpa_supplicant/src/ap/wpa_auth_ie.h +++ b/components/wpa_supplicant/src/ap/wpa_auth_ie.h @@ -19,16 +19,6 @@ struct wpa_eapol_ie_parse { size_t gtk_len; const u8 *mac_addr; size_t mac_addr_len; -#ifdef CONFIG_PEERKEY - const u8 *smk; - size_t smk_len; - const u8 *nonce; - size_t nonce_len; - const u8 *lifetime; - size_t lifetime_len; - const u8 *error; - size_t error_len; -#endif /* CONFIG_PEERKEY */ #ifdef CONFIG_IEEE80211W const u8 *igtk; size_t igtk_len; diff --git a/components/wpa_supplicant/src/common/sae.c b/components/wpa_supplicant/src/common/sae.c index 469303cb28..af1da9d506 100644 --- a/components/wpa_supplicant/src/common/sae.c +++ b/components/wpa_supplicant/src/common/sae.c @@ -48,7 +48,6 @@ int sae_set_group(struct sae_data *sae, int group) tmp->prime_len = tmp->dh->prime_len; if (tmp->prime_len > SAE_MAX_PRIME_LEN) { sae_clear_data(sae); - os_free(tmp); return ESP_FAIL; } @@ -56,7 +55,6 @@ int sae_set_group(struct sae_data *sae, int group) tmp->prime_len); if (tmp->prime_buf == NULL) { sae_clear_data(sae); - os_free(tmp); return ESP_FAIL; } tmp->prime = tmp->prime_buf; @@ -65,7 +63,6 @@ int sae_set_group(struct sae_data *sae, int group) tmp->dh->order_len); if (tmp->order_buf == NULL) { sae_clear_data(sae); - os_free(tmp); return ESP_FAIL; } tmp->order = tmp->order_buf; @@ -846,7 +843,7 @@ fail: int sae_process_commit(struct sae_data *sae) { - u8 k[SAE_MAX_PRIME_LEN]; + u8 k[SAE_MAX_PRIME_LEN] = {0}; if (sae->tmp == NULL || (sae->tmp->ec && sae_derive_k_ecc(sae, k) < 0) || (sae->tmp->dh && sae_derive_k_ffc(sae, k) < 0) || diff --git a/components/wpa_supplicant/src/common/wpa_common.c b/components/wpa_supplicant/src/common/wpa_common.c index 070ba0887e..9842be0827 100644 --- a/components/wpa_supplicant/src/common/wpa_common.c +++ b/components/wpa_supplicant/src/common/wpa_common.c @@ -646,10 +646,6 @@ const char * wpa_cipher_txt(int cipher) * PTK = PRF-X(PMK, "Pairwise key expansion", * Min(AA, SA) || Max(AA, SA) || * Min(ANonce, SNonce) || Max(ANonce, SNonce)) - * - * STK = PRF-X(SMK, "Peer key expansion", - * Min(MAC_I, MAC_P) || Max(MAC_I, MAC_P) || - * Min(INonce, PNonce) || Max(INonce, PNonce)) */ int wpa_pmk_to_ptk(const u8 *pmk, size_t pmk_len, const char *label, const u8 *addr1, const u8 *addr2, diff --git a/components/wpa_supplicant/src/common/wpa_common.h b/components/wpa_supplicant/src/common/wpa_common.h index da8132e28c..e96afa7d95 100644 --- a/components/wpa_supplicant/src/common/wpa_common.h +++ b/components/wpa_supplicant/src/common/wpa_common.h @@ -94,12 +94,6 @@ RSN_SELECTOR(0x00, 0x0f, 0xac, 13) #define RSN_KEY_DATA_GROUPKEY RSN_SELECTOR(0x00, 0x0f, 0xac, 1) #define RSN_KEY_DATA_MAC_ADDR RSN_SELECTOR(0x00, 0x0f, 0xac, 3) #define RSN_KEY_DATA_PMKID RSN_SELECTOR(0x00, 0x0f, 0xac, 4) -#ifdef CONFIG_PEERKEY -#define RSN_KEY_DATA_SMK RSN_SELECTOR(0x00, 0x0f, 0xac, 5) -#define RSN_KEY_DATA_NONCE RSN_SELECTOR(0x00, 0x0f, 0xac, 6) -#define RSN_KEY_DATA_LIFETIME RSN_SELECTOR(0x00, 0x0f, 0xac, 7) -#define RSN_KEY_DATA_ERROR RSN_SELECTOR(0x00, 0x0f, 0xac, 8) -#endif /* CONFIG_PEERKEY */ #ifdef CONFIG_IEEE80211W #define RSN_KEY_DATA_IGTK RSN_SELECTOR(0x00, 0x0f, 0xac, 9) #endif /* CONFIG_IEEE80211W */ @@ -272,23 +266,6 @@ struct rsn_ie_hdr { u8 version[2]; /* little endian */ } STRUCT_PACKED; - -#ifdef CONFIG_PEERKEY -enum { - STK_MUI_4WAY_STA_AP = 1, - STK_MUI_4WAY_STAT_STA = 2, - STK_MUI_GTK = 3, - STK_MUI_SMK = 4 -}; - -enum { - STK_ERR_STA_NR = 1, - STK_ERR_STA_NRSN = 2, - STK_ERR_CPHR_NS = 3, - STK_ERR_NO_STSL = 4 -}; -#endif /* CONFIG_PEERKEY */ - struct rsn_error_kde { be16 mui; be16 error_type; diff --git a/components/wpa_supplicant/src/crypto/crypto_mbedtls.c b/components/wpa_supplicant/src/crypto/crypto_mbedtls.c index 85f8983e32..629e5f3245 100644 --- a/components/wpa_supplicant/src/crypto/crypto_mbedtls.c +++ b/components/wpa_supplicant/src/crypto/crypto_mbedtls.c @@ -122,7 +122,6 @@ struct crypto_hash * crypto_hash_init(enum crypto_hash_alg alg, const u8 *key, struct crypto_hash *ctx; mbedtls_md_type_t md_type; const mbedtls_md_info_t *md_info; - int ret; switch (alg) { case CRYPTO_HASH_ALG_HMAC_MD5: @@ -146,29 +145,37 @@ struct crypto_hash * crypto_hash_init(enum crypto_hash_alg alg, const u8 *key, mbedtls_md_init(&ctx->ctx); md_info = mbedtls_md_info_from_type(md_type); if (!md_info) { - os_free(ctx); - return NULL; + goto cleanup; } - ret = mbedtls_md_setup(&ctx->ctx, md_info, 1); - if (ret != 0) { - os_free(ctx); - return NULL; + if (mbedtls_md_setup(&ctx->ctx, md_info, 1) != 0) { + goto cleanup; + } + if (mbedtls_md_hmac_starts(&ctx->ctx, key, key_len) != 0) { + goto cleanup; } - mbedtls_md_hmac_starts(&ctx->ctx, key, key_len); - return ctx; +cleanup: + os_free(ctx); + return NULL; } void crypto_hash_update(struct crypto_hash *ctx, const u8 *data, size_t len) { + int ret; + if (ctx == NULL) { return; } - mbedtls_md_hmac_update(&ctx->ctx, data, len); + ret = mbedtls_md_hmac_update(&ctx->ctx, data, len); + if (ret != 0) { + wpa_printf(MSG_ERROR, "%s: mbedtls_md_hmac_update failed", __func__); + } } int crypto_hash_finish(struct crypto_hash *ctx, u8 *mac, size_t *len) { + int ret; + if (ctx == NULL) { return -2; } @@ -178,11 +185,11 @@ int crypto_hash_finish(struct crypto_hash *ctx, u8 *mac, size_t *len) bin_clear_free(ctx, sizeof(*ctx)); return 0; } - mbedtls_md_hmac_finish(&ctx->ctx, mac); + ret = mbedtls_md_hmac_finish(&ctx->ctx, mac); mbedtls_md_free(&ctx->ctx); bin_clear_free(ctx, sizeof(*ctx)); - return 0; + return ret; } static int hmac_vector(mbedtls_md_type_t md_type, @@ -207,17 +214,24 @@ static int hmac_vector(mbedtls_md_type_t md_type, return(ret); } - mbedtls_md_hmac_starts(&md_ctx, key, key_len); - - for (i = 0; i < num_elem; i++) { - mbedtls_md_hmac_update(&md_ctx, addr[i], len[i]); + ret = mbedtls_md_hmac_starts(&md_ctx, key, key_len); + if (ret != 0) { + return(ret); } - mbedtls_md_hmac_finish(&md_ctx, mac); + for (i = 0; i < num_elem; i++) { + ret = mbedtls_md_hmac_update(&md_ctx, addr[i], len[i]); + if (ret != 0) { + return(ret); + } + + } + + ret = mbedtls_md_hmac_finish(&md_ctx, mac); mbedtls_md_free(&md_ctx); - return 0; + return ret; } int hmac_sha384_vector(const u8 *key, size_t key_len, size_t num_elem, diff --git a/components/wpa_supplicant/src/eap_peer/eap_fast_pac.c b/components/wpa_supplicant/src/eap_peer/eap_fast_pac.c index 4f92f4ad3d..39821ddb1a 100644 --- a/components/wpa_supplicant/src/eap_peer/eap_fast_pac.c +++ b/components/wpa_supplicant/src/eap_peer/eap_fast_pac.c @@ -552,6 +552,7 @@ static int eap_fast_write_pac(struct eap_sm *sm, const char *pac_file, return -1; } eap_set_config_blob(sm, blob); + os_free(blob); } else { FILE *f; f = fopen(pac_file, "wb"); diff --git a/components/wpa_supplicant/src/rsn_supp/wpa_ie.h b/components/wpa_supplicant/src/rsn_supp/wpa_ie.h index c71a926f2b..98ba648794 100644 --- a/components/wpa_supplicant/src/rsn_supp/wpa_ie.h +++ b/components/wpa_supplicant/src/rsn_supp/wpa_ie.h @@ -25,16 +25,6 @@ struct wpa_eapol_ie_parse { size_t gtk_len; const u8 *mac_addr; size_t mac_addr_len; -#ifdef CONFIG_PEERKEY - const u8 *smk; - size_t smk_len; - const u8 *nonce; - size_t nonce_len; - const u8 *lifetime; - size_t lifetime_len; - const u8 *error; - size_t error_len; -#endif /* CONFIG_PEERKEY */ #ifdef CONFIG_IEEE80211W const u8 *igtk; size_t igtk_len;