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>

diff --git a/src/eap_common/eap_peap_common.c b/src/eap_common/eap_peap_common.c
index 3a64b8ecc44c33e307f6e15bb64bd809f31edd27..8a701d2e654c272c15a13affcfef607f4156a3b0 100644 (file)
--- a/src/eap_common/eap_peap_common.c
+++ b/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;
 }

diff --git a/src/eap_common/eap_peap_common.h b/src/eap_common/eap_peap_common.h
index f59afb07d098dd7aca0ea183f6ab16add5bfe6af..f1820786f92f35a66e4d2b20fe34706a708ec9f0 100644 (file)
--- a/src/eap_common/eap_peap_common.h
+++ b/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 */

diff --git a/src/eap_peer/eap_peap.c b/src/eap_peer/eap_peap.c
index 0202c9d10a40df0d55be120705ffcb94175810f5..7cb8213627a3b01a578aa6d3f29e6e8439f8cc75 100644 (file)
--- a/src/eap_peer/eap_peap.c
+++ b/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",

diff --git a/src/eap_server/eap_server_peap.c b/src/eap_server/eap_server_peap.c
index 8a7d626a63a14a0cbeb5361e4d8bc68c28a14a44..381c44a3ee40dce8468a0dfb7de84a1449ed3291 100644 (file)
--- a/src/eap_server/eap_server_peap.c
+++ b/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) {