RADIUS: Check MD5 processing result

The MD5 functions may fail, e.g., if the used crypto library is
configured to reject deprecated old algorithms. Check for this more
consistently in RADIUS routines and make it obvious in the debug log if
this is causing operations to fail instead of trying to proceed and hide
the issue.

Signed-off-by: Jouni Malinen <j@w1.fi>

diff --git a/src/radius/radius.c b/src/radius/radius.c
index 37aa216b170b777bbb2fc0801a919346e120974a..029e6223ce1496d123ea920de123819c0e19b0de 100644 (file)
--- a/src/radius/radius.c
+++ b/src/radius/radius.c
@@ -469,8 +469,10 @@ int radius_msg_finish(struct radius_msg *msg, const u8 *secret,
                        return -1;
                msg->hdr->length = host_to_be16(wpabuf_len(msg->buf));
                if (hmac_md5(secret, secret_len, wpabuf_head(msg->buf),
-                            wpabuf_len(msg->buf), pos) < 0)
+                            wpabuf_len(msg->buf), pos) < 0) {
+                       wpa_printf(MSG_INFO, "RADIUS: MD5 not available");
                        return -1;
+               }
        } else
                msg->hdr->length = host_to_be16(wpabuf_len(msg->buf));
 
@@ -497,8 +499,10 @@ int radius_msg_finish_srv(struct radius_msg *msg, const u8 *secret,
        os_memcpy(msg->hdr->authenticator, req_authenticator,
                  sizeof(msg->hdr->authenticator));
        if (hmac_md5(secret, secret_len, wpabuf_head(msg->buf),
-                    wpabuf_len(msg->buf), pos) < 0)
+                    wpabuf_len(msg->buf), pos) < 0) {
+               wpa_printf(MSG_INFO, "RADIUS: MD5 not available");
                return -1;
+       }
 
        /* ResponseAuth = MD5(Code+ID+Length+RequestAuth+Attributes+Secret) */
        addr[0] = (u8 *) msg->hdr;
@@ -509,7 +513,10 @@ int radius_msg_finish_srv(struct radius_msg *msg, const u8 *secret,
        len[2] = wpabuf_len(msg->buf) - sizeof(struct radius_hdr);
        addr[3] = secret;
        len[3] = secret_len;
-       md5_vector(4, addr, len, msg->hdr->authenticator);
+       if (md5_vector(4, addr, len, msg->hdr->authenticator) < 0) {
+               wpa_printf(MSG_INFO, "RADIUS: MD5 not available");
+               return -1;
+       }
 
        if (wpabuf_len(msg->buf) > 0xffff) {
                wpa_printf(MSG_WARNING, "RADIUS: Too long message (%lu)",
@@ -535,16 +542,20 @@ int radius_msg_finish_das_resp(struct radius_msg *msg, const u8 *secret,
        msg->hdr->length = host_to_be16(wpabuf_len(msg->buf));
        os_memcpy(msg->hdr->authenticator, req_hdr->authenticator, 16);
        if (hmac_md5(secret, secret_len, wpabuf_head(msg->buf),
-                    wpabuf_len(msg->buf), pos) < 0)
+                    wpabuf_len(msg->buf), pos) < 0) {
+               wpa_printf(MSG_INFO, "RADIUS: MD5 not available");
                return -1;
+       }
 
        /* ResponseAuth = MD5(Code+ID+Length+RequestAuth+Attributes+Secret) */
        addr[0] = wpabuf_head_u8(msg->buf);
        len[0] = wpabuf_len(msg->buf);
        addr[1] = secret;
        len[1] = secret_len;
-       if (md5_vector(2, addr, len, msg->hdr->authenticator) < 0)
+       if (md5_vector(2, addr, len, msg->hdr->authenticator) < 0) {
+               wpa_printf(MSG_INFO, "RADIUS: MD5 not available");
                return -1;
+       }
 
        if (wpabuf_len(msg->buf) > 0xffff) {
                wpa_printf(MSG_WARNING, "RADIUS: Too long message (%lu)",
@@ -555,8 +566,8 @@ int radius_msg_finish_das_resp(struct radius_msg *msg, const u8 *secret,
 }
 
 
-void radius_msg_finish_acct(struct radius_msg *msg, const u8 *secret,
-                           size_t secret_len)
+int radius_msg_finish_acct(struct radius_msg *msg, const u8 *secret,
+                          size_t secret_len)
 {
        const u8 *addr[2];
        size_t len[2];
@@ -567,17 +578,22 @@ void radius_msg_finish_acct(struct radius_msg *msg, const u8 *secret,
        len[0] = wpabuf_len(msg->buf);
        addr[1] = secret;
        len[1] = secret_len;
-       md5_vector(2, addr, len, msg->hdr->authenticator);
+       if (md5_vector(2, addr, len, msg->hdr->authenticator) < 0) {
+               wpa_printf(MSG_INFO, "RADIUS: MD5 not available");
+               return -1;
+       }
 
        if (wpabuf_len(msg->buf) > 0xffff) {
                wpa_printf(MSG_WARNING, "RADIUS: Too long messages (%lu)",
                           (unsigned long) wpabuf_len(msg->buf));
+               return -1;
        }
+       return 0;
 }
 
 
-void radius_msg_finish_acct_resp(struct radius_msg *msg, const u8 *secret,
-                                size_t secret_len, const u8 *req_authenticator)
+int radius_msg_finish_acct_resp(struct radius_msg *msg, const u8 *secret,
+                               size_t secret_len, const u8 *req_authenticator)
 {
        const u8 *addr[2];
        size_t len[2];
@@ -588,12 +604,17 @@ void radius_msg_finish_acct_resp(struct radius_msg *msg, const u8 *secret,
        len[0] = wpabuf_len(msg->buf);
        addr[1] = secret;
        len[1] = secret_len;
-       md5_vector(2, addr, len, msg->hdr->authenticator);
+       if (md5_vector(2, addr, len, msg->hdr->authenticator) < 0) {
+               wpa_printf(MSG_INFO, "RADIUS: MD5 not available");
+               return -1;
+       }
 
        if (wpabuf_len(msg->buf) > 0xffff) {
                wpa_printf(MSG_WARNING, "RADIUS: Too long messages (%lu)",
                           (unsigned long) wpabuf_len(msg->buf));
+               return -1;
        }
+       return 0;
 }
 
 
@@ -614,7 +635,10 @@ int radius_msg_verify_acct_req(struct radius_msg *msg, const u8 *secret,
        len[2] = wpabuf_len(msg->buf) - sizeof(struct radius_hdr);
        addr[3] = secret;
        len[3] = secret_len;
-       md5_vector(4, addr, len, hash);
+       if (md5_vector(4, addr, len, hash) < 0) {
+               wpa_printf(MSG_INFO, "RADIUS: MD5 not available");
+               return 1;
+       }
        return os_memcmp_const(msg->hdr->authenticator, hash, MD5_MAC_LEN) != 0;
 }
 
@@ -642,7 +666,10 @@ int radius_msg_verify_das_req(struct radius_msg *msg, const u8 *secret,
        len[2] = wpabuf_len(msg->buf) - sizeof(struct radius_hdr);
        addr[3] = secret;
        len[3] = secret_len;
-       md5_vector(4, addr, len, hash);
+       if (md5_vector(4, addr, len, hash) < 0) {
+               wpa_printf(MSG_INFO, "RADIUS: MD5 not available");
+               return 1;
+       }
        if (os_memcmp_const(msg->hdr->authenticator, hash, MD5_MAC_LEN) != 0)
                return 1;
 
@@ -674,8 +701,11 @@ int radius_msg_verify_das_req(struct radius_msg *msg, const u8 *secret,
                  sizeof(orig_authenticator));
        os_memset(msg->hdr->authenticator, 0,
                  sizeof(msg->hdr->authenticator));
-       hmac_md5(secret, secret_len, wpabuf_head(msg->buf),
-                wpabuf_len(msg->buf), auth);
+       if (hmac_md5(secret, secret_len, wpabuf_head(msg->buf),
+                    wpabuf_len(msg->buf), auth) < 0) {
+               wpa_printf(MSG_INFO, "RADIUS: MD5 not available");
+               return 1;
+       }
        os_memcpy(attr + 1, orig, MD5_MAC_LEN);
        os_memcpy(msg->hdr->authenticator, orig_authenticator,
                  sizeof(orig_authenticator));
@@ -972,8 +1002,10 @@ int radius_msg_verify_msg_auth(struct radius_msg *msg, const u8 *secret,
                          sizeof(msg->hdr->authenticator));
        }
        if (hmac_md5(secret, secret_len, wpabuf_head(msg->buf),
-                    wpabuf_len(msg->buf), auth) < 0)
+                    wpabuf_len(msg->buf), auth) < 0) {
+               wpa_printf(MSG_INFO, "RADIUS: MD5 not available");
                return 1;
+       }
        os_memcpy(attr + 1, orig, MD5_MAC_LEN);
        if (req_auth) {
                os_memcpy(msg->hdr->authenticator, orig_authenticator,
@@ -1185,6 +1217,7 @@ static u8 * decrypt_ms_key(const u8 *key, size_t len,
                        elen[1] = MD5_MAC_LEN;
                }
                if (md5_vector(first ? 3 : 2, addr, elen, hash) < 0) {
+                       wpa_printf(MSG_INFO, "RADIUS: MD5 not available");
                        os_free(plain);
                        return NULL;
                }
@@ -1213,10 +1246,10 @@ static u8 * decrypt_ms_key(const u8 *key, size_t len,
 }
 
 
-static void encrypt_ms_key(const u8 *key, size_t key_len, u16 salt,
-                          const u8 *req_authenticator,
-                          const u8 *secret, size_t secret_len,
-                          u8 *ebuf, size_t *elen)
+static int encrypt_ms_key(const u8 *key, size_t key_len, u16 salt,
+                         const u8 *req_authenticator,
+                         const u8 *secret, size_t secret_len,
+                         u8 *ebuf, size_t *elen)
 {
        int i, len, first = 1;
        u8 hash[MD5_MAC_LEN], saltbuf[2], *pos;
@@ -1250,7 +1283,10 @@ static void encrypt_ms_key(const u8 *key, size_t key_len, u16 salt,
                        addr[1] = pos - MD5_MAC_LEN;
                        _len[1] = MD5_MAC_LEN;
                }
-               md5_vector(first ? 3 : 2, addr, _len, hash);
+               if (md5_vector(first ? 3 : 2, addr, _len, hash) < 0) {
+                       wpa_printf(MSG_INFO, "RADIUS: MD5 not available");
+                       return -1;
+               }
                first = 0;
 
                for (i = 0; i < MD5_MAC_LEN; i++)
@@ -1258,6 +1294,8 @@ static void encrypt_ms_key(const u8 *key, size_t key_len, u16 salt,
 
                len -= MD5_MAC_LEN;
        }
+
+       return 0;
 }
 
 
@@ -1375,8 +1413,9 @@ int radius_msg_add_mppe_keys(struct radius_msg *msg,
        salt |= 0x8000;
        WPA_PUT_BE16(pos, salt);
        pos += 2;
-       encrypt_ms_key(send_key, send_key_len, salt, req_authenticator, secret,
-                      secret_len, pos, &elen);
+       if (encrypt_ms_key(send_key, send_key_len, salt, req_authenticator,
+                          secret, secret_len, pos, &elen) < 0)
+               return 0;
        vhdr->vendor_length = hlen + elen - sizeof(vendor_id);
 
        attr = radius_msg_add_attr(msg, RADIUS_ATTR_VENDOR_SPECIFIC,
@@ -1400,8 +1439,9 @@ int radius_msg_add_mppe_keys(struct radius_msg *msg,
        salt ^= 1;
        WPA_PUT_BE16(pos, salt);
        pos += 2;
-       encrypt_ms_key(recv_key, recv_key_len, salt, req_authenticator, secret,
-                      secret_len, pos, &elen);
+       if (encrypt_ms_key(recv_key, recv_key_len, salt, req_authenticator,
+                          secret, secret_len, pos, &elen) < 0)
+               return 0;
        vhdr->vendor_length = hlen + elen - sizeof(vendor_id);
 
        attr = radius_msg_add_attr(msg, RADIUS_ATTR_VENDOR_SPECIFIC,
@@ -1492,7 +1532,10 @@ int radius_user_password_hide(struct radius_msg *msg,
        len[0] = secret_len;
        addr[1] = msg->hdr->authenticator;
        len[1] = 16;
-       md5_vector(2, addr, len, hash);
+       if (md5_vector(2, addr, len, hash) < 0) {
+               wpa_printf(MSG_INFO, "RADIUS: MD5 not available");
+               return -1;
+       }
 
        for (i = 0; i < 16; i++)
                buf[i] ^= hash[i];
@@ -1503,7 +1546,10 @@ int radius_user_password_hide(struct radius_msg *msg,
                len[0] = secret_len;
                addr[1] = &buf[pos - 16];
                len[1] = 16;
-               md5_vector(2, addr, len, hash);
+               if (md5_vector(2, addr, len, hash) < 0) {
+                       wpa_printf(MSG_INFO, "RADIUS: MD5 not available");
+                       return -1;
+               }
 
                for (i = 0; i < 16; i++)
                        buf[pos + i] ^= hash[i];
@@ -1792,7 +1838,10 @@ char * radius_msg_get_tunnel_password(struct radius_msg *msg, int *keylen,
                len[0] = secret_len;
                addr[1] = pos - 16;
                len[1] = 16;
-               md5_vector(2, addr, len, hash);
+               if (md5_vector(2, addr, len, hash) < 0) {
+                       wpa_printf(MSG_INFO, "RADIUS: MD5 not available");
+                       goto out;
+               }
 
                for (i = 0; i < 16; i++)
                        pos[i] ^= hash[i];
@@ -1809,7 +1858,10 @@ char * radius_msg_get_tunnel_password(struct radius_msg *msg, int *keylen,
        len[1] = 16;
        addr[2] = salt;
        len[2] = 2;
-       md5_vector(3, addr, len, hash);
+       if (md5_vector(3, addr, len, hash) < 0) {
+               wpa_printf(MSG_INFO, "RADIUS: MD5 not available");
+               goto out;
+       }
 
        for (i = 0; i < 16; i++)
                pos[i] ^= hash[i];

diff --git a/src/radius/radius.h b/src/radius/radius.h
index 278e9e087bd095edca2e4769e39f1bc1f190e2a2..09d3591c535ef47ac44cb952c88828012e5d461a 100644 (file)
--- a/src/radius/radius.h
+++ b/src/radius/radius.h
@@ -275,11 +275,10 @@ int radius_msg_finish_srv(struct radius_msg *msg, const u8 *secret,
 int radius_msg_finish_das_resp(struct radius_msg *msg, const u8 *secret,
                               size_t secret_len,
                               const struct radius_hdr *req_hdr);
-void radius_msg_finish_acct(struct radius_msg *msg, const u8 *secret,
-                           size_t secret_len);
-void radius_msg_finish_acct_resp(struct radius_msg *msg, const u8 *secret,
-                                size_t secret_len,
-                                const u8 *req_authenticator);
+int radius_msg_finish_acct(struct radius_msg *msg, const u8 *secret,
+                          size_t secret_len);
+int radius_msg_finish_acct_resp(struct radius_msg *msg, const u8 *secret,
+                               size_t secret_len, const u8 *req_authenticator);
 int radius_msg_verify_acct_req(struct radius_msg *msg, const u8 *secret,
                               size_t secret_len);
 int radius_msg_verify_das_req(struct radius_msg *msg, const u8 *secret,

diff --git a/src/radius/radius_client.c b/src/radius/radius_client.c
index d4faa79367ec222c26a1f0545ff9b95212d82683..705aaef662599ee9b53a66071cc7917b653fa303 100644 (file)
--- a/src/radius/radius_client.c
+++ b/src/radius/radius_client.c
@@ -482,8 +482,11 @@ static int radius_client_retransmit(struct radius_client_data *radius,
                wpa_printf(MSG_DEBUG,
                           "RADIUS: Updated Acct-Delay-Time to %u for retransmission",
                           delay_time);
-               radius_msg_finish_acct(entry->msg, entry->shared_secret,
-                                      entry->shared_secret_len);
+               if (radius_msg_finish_acct(entry->msg, entry->shared_secret,
+                                          entry->shared_secret_len) < 0) {
+                       wpa_printf(MSG_INFO, "Failed to build RADIUS message");
+                       return -1;
+               }
                if (radius->conf->msg_dumps)
                        radius_msg_dump(entry->msg);
        }
@@ -878,7 +881,14 @@ int radius_client_send(struct radius_client_data *radius,
                }
                shared_secret = conf->acct_server->shared_secret;
                shared_secret_len = conf->acct_server->shared_secret_len;
-               radius_msg_finish_acct(msg, shared_secret, shared_secret_len);
+               if (radius_msg_finish_acct(msg, shared_secret,
+                                          shared_secret_len) < 0) {
+                       hostapd_logger(radius->ctx, NULL,
+                                      HOSTAPD_MODULE_RADIUS,
+                                      HOSTAPD_LEVEL_INFO,
+                                      "Failed to build RADIUS accounting message");
+                       return -1;
+               }
                name = "accounting";
                s = radius->acct_sock;
                conf->acct_server->requests++;
@@ -900,7 +910,14 @@ int radius_client_send(struct radius_client_data *radius,
                }
                shared_secret = conf->auth_server->shared_secret;
                shared_secret_len = conf->auth_server->shared_secret_len;
-               radius_msg_finish(msg, shared_secret, shared_secret_len);
+               if (radius_msg_finish(msg, shared_secret, shared_secret_len) <
+                   0) {
+                       hostapd_logger(radius->ctx, NULL,
+                                      HOSTAPD_MODULE_RADIUS,
+                                      HOSTAPD_LEVEL_INFO,
+                                      "Failed to build RADIUS authentication message");
+                       return -1;
+               }
                name = "authentication";
                s = radius->auth_sock;
                conf->auth_server->requests++;
@@ -1512,8 +1529,10 @@ static void radius_client_update_acct_msgs(struct radius_client_data *radius,
                if (entry->msg_type == RADIUS_ACCT) {
                        entry->shared_secret = shared_secret;
                        entry->shared_secret_len = shared_secret_len;
-                       radius_msg_finish_acct(entry->msg, shared_secret,
-                                              shared_secret_len);
+                       if (radius_msg_finish_acct(entry->msg, shared_secret,
+                                                  shared_secret_len) < 0)
+                               wpa_printf(MSG_INFO,
+                                          "RADIUS: Failed to update accounting message");
                }
        }
 }

diff --git a/src/radius/radius_server.c b/src/radius/radius_server.c
index 715b6d928806b8fd9e9777b45fbc2b0b51a783b4..5899c04371f17d0140ccce42fd2579e509251522 100644 (file)
--- a/src/radius/radius_server.c
+++ b/src/radius/radius_server.c
@@ -926,6 +926,8 @@ radius_server_encapsulate_eap(struct radius_server_data *data,
                                  client->shared_secret_len,
                                  hdr->authenticator) < 0) {
                RADIUS_DEBUG("Failed to add Message-Authenticator attribute");
+               radius_msg_free(msg);
+               return NULL;
        }
 
        if (code == RADIUS_CODE_ACCESS_ACCEPT)
@@ -1015,6 +1017,8 @@ radius_server_macacl(struct radius_server_data *data,
                                  client->shared_secret_len,
                                  hdr->authenticator) < 0) {
                RADIUS_DEBUG("Failed to add Message-Authenticator attribute");
+               radius_msg_free(msg);
+               return NULL;
        }
 
        return msg;
@@ -1066,6 +1070,8 @@ static int radius_server_reject(struct radius_server_data *data,
                                  hdr->authenticator) <
            0) {
                RADIUS_DEBUG("Failed to add Message-Authenticator attribute");
+               radius_msg_free(msg);
+               return -1;
        }
 
        if (wpa_debug_level <= MSG_MSGDUMP) {
@@ -1682,9 +1688,12 @@ static void radius_server_receive_acct(int sock, void *eloop_ctx,
        if (resp == NULL)
                goto fail;
 
-       radius_msg_finish_acct_resp(resp, (u8 *) client->shared_secret,
-                                   client->shared_secret_len,
-                                   hdr->authenticator);
+       if (radius_msg_finish_acct_resp(resp, (u8 *) client->shared_secret,
+                                       client->shared_secret_len,
+                                       hdr->authenticator) < 0) {
+               RADIUS_ERROR("Failed to add Message-Authenticator attribute");
+               goto fail;
+       }
 
        RADIUS_DEBUG("Reply to %s:%d", abuf, from_port);
        if (wpa_debug_level <= MSG_MSGDUMP) {
@@ -2588,8 +2597,12 @@ int radius_server_dac_request(struct radius_server_data *data, const char *req)
                return -1;
        }
 
-       radius_msg_finish_acct(msg, (u8 *) client->shared_secret,
-                              client->shared_secret_len);
+       if (radius_msg_finish_acct(msg, (u8 *) client->shared_secret,
+                                  client->shared_secret_len) < 0) {
+               RADIUS_ERROR("Failed to add Message-Authenticator attribute");
+               radius_msg_free(msg);
+               return -1;
+       }
 
        if (wpa_debug_level <= MSG_MSGDUMP)
                radius_msg_dump(msg);