Browse Source

PEAP: Verify peap_prfplus() result

This function can fail in theory since the SHA-1 functions are
allowed to return an error. While this does not really happen in
practice (we would not get this far if SHA-1 does not work), it is
cleaner to include the error handling here to keep static analyzers
happier. [Bug 421]

Signed-hostap: Jouni Malinen <j@w1.fi>
Jouni Malinen 13 years ago
parent
commit
3724ddc0c1

+ 8 - 5
src/eap_common/eap_peap_common.c

@@ -1,6 +1,6 @@
 /*
  * EAP-PEAP common routines
- * Copyright (c) 2008, Jouni Malinen <j@w1.fi>
+ * Copyright (c) 2008-2011, Jouni Malinen <j@w1.fi>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -18,9 +18,9 @@
 #include "crypto/sha1.h"
 #include "eap_peap_common.h"
 
-void peap_prfplus(int version, const u8 *key, size_t key_len,
-		  const char *label, const u8 *seed, size_t seed_len,
-		  u8 *buf, size_t buf_len)
+int peap_prfplus(int version, const u8 *key, size_t key_len,
+		 const char *label, const u8 *seed, size_t seed_len,
+		 u8 *buf, size_t buf_len)
 {
 	unsigned char counter = 0;
 	size_t pos, plen;
@@ -75,7 +75,8 @@ void peap_prfplus(int version, const u8 *key, size_t key_len,
 	while (pos < buf_len) {
 		counter++;
 		plen = buf_len - pos;
-		hmac_sha1_vector(key, key_len, 5, addr, len, hash);
+		if (hmac_sha1_vector(key, key_len, 5, addr, len, hash) < 0)
+			return -1;
 		if (plen >= SHA1_MAC_LEN) {
 			os_memcpy(&buf[pos], hash, SHA1_MAC_LEN);
 			pos += SHA1_MAC_LEN;
@@ -85,4 +86,6 @@ void peap_prfplus(int version, const u8 *key, size_t key_len,
 		}
 		len[0] = SHA1_MAC_LEN;
 	}
+
+	return 0;
 }

+ 4 - 4
src/eap_common/eap_peap_common.h

@@ -1,6 +1,6 @@
 /*
  * EAP-PEAP common routines
- * Copyright (c) 2008, Jouni Malinen <j@w1.fi>
+ * Copyright (c) 2008-2011, Jouni Malinen <j@w1.fi>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -15,8 +15,8 @@
 #ifndef EAP_PEAP_COMMON_H
 #define EAP_PEAP_COMMON_H
 
-void peap_prfplus(int version, const u8 *key, size_t key_len,
-		  const char *label, const u8 *seed, size_t seed_len,
-		  u8 *buf, size_t buf_len);
+int peap_prfplus(int version, const u8 *key, size_t key_len,
+		 const char *label, const u8 *seed, size_t seed_len,
+		 u8 *buf, size_t buf_len);
 
 #endif /* EAP_PEAP_COMMON_H */

+ 10 - 5
src/eap_peer/eap_peap.c

@@ -285,8 +285,10 @@ 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);
-	peap_prfplus(data->peap_version, tk, 40, "Inner Methods Compound Keys",
-		     isk, sizeof(isk), imck, sizeof(imck));
+	if (peap_prfplus(data->peap_version, tk, 40,
+			 "Inner Methods Compound Keys",
+			 isk, sizeof(isk), imck, sizeof(imck)) < 0)
+		return -1;
 	wpa_hexdump_key(MSG_DEBUG, "EAP-PEAP: IMCK (IPMKj)",
 			imck, sizeof(imck));
 
@@ -1247,9 +1249,12 @@ static u8 * eap_peap_getKey(struct eap_sm *sm, void *priv, size_t *len)
 		 * termination for this label while the one used for deriving
 		 * IPMK|CMK did not use null termination.
 		 */
-		peap_prfplus(data->peap_version, data->ipmk, 40,
-			     "Session Key Generating Function",
-			     (u8 *) "\00", 1, csk, sizeof(csk));
+		if (peap_prfplus(data->peap_version, data->ipmk, 40,
+				 "Session Key Generating Function",
+				 (u8 *) "\00", 1, csk, sizeof(csk)) < 0) {
+			os_free(key);
+			return NULL;
+		}
 		wpa_hexdump_key(MSG_DEBUG, "EAP-PEAP: CSK", csk, sizeof(csk));
 		os_memcpy(key, csk, EAP_TLS_KEY_LEN);
 		wpa_hexdump(MSG_DEBUG, "EAP-PEAP: Derived key",

+ 10 - 5
src/eap_server/eap_server_peap.c

@@ -351,8 +351,12 @@ 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);
-	peap_prfplus(data->peap_version, tk, 40, "Inner Methods Compound Keys",
-		     isk, sizeof(isk), imck, sizeof(imck));
+	if (peap_prfplus(data->peap_version, tk, 40,
+			 "Inner Methods Compound Keys",
+			 isk, sizeof(isk), imck, sizeof(imck)) < 0) {
+		os_free(tk);
+		return -1;
+	}
 	wpa_hexdump_key(MSG_DEBUG, "EAP-PEAP: IMCK (IPMKj)",
 			imck, sizeof(imck));
 
@@ -1320,9 +1324,10 @@ static u8 * eap_peap_getKey(struct eap_sm *sm, void *priv, size_t *len)
 		 * termination for this label while the one used for deriving
 		 * IPMK|CMK did not use null termination.
 		 */
-		peap_prfplus(data->peap_version, data->ipmk, 40,
-			     "Session Key Generating Function",
-			     (u8 *) "\00", 1, csk, sizeof(csk));
+		if (peap_prfplus(data->peap_version, data->ipmk, 40,
+				 "Session Key Generating Function",
+				 (u8 *) "\00", 1, csk, sizeof(csk)) < 0)
+			return NULL;
 		wpa_hexdump_key(MSG_DEBUG, "EAP-PEAP: CSK", csk, sizeof(csk));
 		eapKeyData = os_malloc(EAP_TLS_KEY_LEN);
 		if (eapKeyData) {