From d05dda61d8b9ce11d622f6234e32b3ad73f25c64 Mon Sep 17 00:00:00 2001 From: Jouni Malinen Date: Sat, 2 Feb 2019 00:01:29 +0200 Subject: [PATCH] PEAP: Explicitly clear temporary keys from memory when using CMK The case of PEAPv0 with crypto binding did not clear some of the temporary keys from stack/heap when those keys were not needed anymore. Clear those explicitly to avoid unnecessary caching of keying material. Signed-off-by: Jouni Malinen --- src/eap_peer/eap_peap.c | 14 +++++++++----- src/eap_server/eap_server_peap.c | 12 +++++++++--- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/eap_peer/eap_peap.c b/src/eap_peer/eap_peap.c index 9b87b6b94..0a756b1d7 100644 --- a/src/eap_peer/eap_peap.c +++ b/src/eap_peer/eap_peap.c @@ -188,7 +188,7 @@ static void eap_peap_deinit(struct eap_sm *sm, void *priv) os_free(data->session_id); wpabuf_free(data->pending_phase2_req); wpabuf_free(data->pending_resp); - os_free(data); + bin_clear_free(data, sizeof(*data)); } @@ -253,7 +253,7 @@ static int eap_peap_derive_cmk(struct eap_sm *sm, struct eap_peap_data *data) { u8 *tk; u8 isk[32], imck[60]; - int resumed; + int resumed, res; /* * Tunnel key (TK) is the first 60 octets of the key generated by @@ -292,9 +292,11 @@ static int eap_peap_derive_cmk(struct eap_sm *sm, struct eap_peap_data *data) * in the end of the label just before ISK; is that just a typo?) */ wpa_hexdump_key(MSG_DEBUG, "EAP-PEAP: TempKey", tk, 40); - if (peap_prfplus(data->peap_version, tk, 40, - "Inner Methods Compound Keys", - isk, sizeof(isk), imck, sizeof(imck)) < 0) + res = peap_prfplus(data->peap_version, tk, 40, + "Inner Methods Compound Keys", + isk, sizeof(isk), imck, sizeof(imck)); + os_memset(isk, 0, sizeof(isk)); + if (res < 0) return -1; wpa_hexdump_key(MSG_DEBUG, "EAP-PEAP: IMCK (IPMKj)", imck, sizeof(imck)); @@ -303,6 +305,7 @@ static int eap_peap_derive_cmk(struct eap_sm *sm, struct eap_peap_data *data) wpa_hexdump_key(MSG_DEBUG, "EAP-PEAP: IPMK (S-IPMKj)", data->ipmk, 40); os_memcpy(data->cmk, imck + 40, 20); wpa_hexdump_key(MSG_DEBUG, "EAP-PEAP: CMK (CMKj)", data->cmk, 20); + os_memset(imck, 0, sizeof(imck)); return 0; } @@ -1263,6 +1266,7 @@ static u8 * eap_peap_getKey(struct eap_sm *sm, void *priv, size_t *len) os_memcpy(key, csk, EAP_TLS_KEY_LEN); wpa_hexdump(MSG_DEBUG, "EAP-PEAP: Derived key", key, EAP_TLS_KEY_LEN); + os_memset(csk, 0, sizeof(csk)); } else os_memcpy(key, data->key_data, EAP_TLS_KEY_LEN); diff --git a/src/eap_server/eap_server_peap.c b/src/eap_server/eap_server_peap.c index 97dba41c8..3d334a0db 100644 --- a/src/eap_server/eap_server_peap.c +++ b/src/eap_server/eap_server_peap.c @@ -324,6 +324,7 @@ static int eap_peap_derive_cmk(struct eap_sm *sm, struct eap_peap_data *data) { u8 *tk; u8 isk[32], imck[60]; + int res; /* * Tunnel key (TK) is the first 60 octets of the key generated by @@ -358,9 +359,11 @@ static int eap_peap_derive_cmk(struct eap_sm *sm, struct eap_peap_data *data) * in the end of the label just before ISK; is that just a typo?) */ wpa_hexdump_key(MSG_DEBUG, "EAP-PEAP: TempKey", tk, 40); - if (peap_prfplus(data->peap_version, tk, 40, - "Inner Methods Compound Keys", - isk, sizeof(isk), imck, sizeof(imck)) < 0) { + res = peap_prfplus(data->peap_version, tk, 40, + "Inner Methods Compound Keys", + isk, sizeof(isk), imck, sizeof(imck)); + os_memset(isk, 0, sizeof(isk)); + if (res < 0) { os_free(tk); return -1; } @@ -373,6 +376,7 @@ static int eap_peap_derive_cmk(struct eap_sm *sm, struct eap_peap_data *data) wpa_hexdump_key(MSG_DEBUG, "EAP-PEAP: IPMK (S-IPMKj)", data->ipmk, 40); os_memcpy(data->cmk, imck + 40, 20); wpa_hexdump_key(MSG_DEBUG, "EAP-PEAP: CMK (CMKj)", data->cmk, 20); + os_memset(imck, 0, sizeof(imck)); return 0; } @@ -1322,6 +1326,8 @@ static u8 * eap_peap_getKey(struct eap_sm *sm, void *priv, size_t *len) "key"); } + os_memset(csk, 0, sizeof(csk)); + return eapKeyData; }