check before dereference. Found by PVS-Studio
[freeradius.git] / src / modules / rlm_eap / libeap / eap_tls.c
index aaff54b..a08cd50 100644 (file)
@@ -301,9 +301,11 @@ static int eaptls_send_ack(eap_handler_t *handler, int peap_flag)
 static fr_tls_status_t eaptls_verify(eap_handler_t *handler)
 {
        EAP_DS                  *eap_ds = handler->eap_ds;
+       tls_session_t           *tls_session = handler->opaque;
        EAP_DS                  *prev_eap_ds = handler->prev_eapds;
        eaptls_packet_t         *eaptls_packet, *eaptls_prev = NULL;
        REQUEST                 *request = handler->request;
+       size_t                  frag_len;
 
        /*
         *      We don't check ANY of the input parameters.  It's all
@@ -323,14 +325,15 @@ static fr_tls_status_t eaptls_verify(eap_handler_t *handler)
        if (prev_eap_ds && prev_eap_ds->response)
                eaptls_prev = (eaptls_packet_t *)prev_eap_ds->response->type.data;
 
-       /*
-        *      First output the flags (for debugging)
-        */
-       RDEBUG3("Peer sent flags %c%c%c",
-               TLS_START(eaptls_packet->flags) ? 'S' : '-',
-               TLS_MORE_FRAGMENTS(eaptls_packet->flags) ? 'M' : '-',
-               TLS_LENGTH_INCLUDED(eaptls_packet->flags) ? 'L' : '-');
-
+       if (eaptls_packet) {
+               /*
+                *      First output the flags (for debugging)
+                */
+               RDEBUG3("Peer sent flags %c%c%c",
+                       TLS_START(eaptls_packet->flags) ? 'S' : '-',
+                       TLS_MORE_FRAGMENTS(eaptls_packet->flags) ? 'M' : '-',
+                       TLS_LENGTH_INCLUDED(eaptls_packet->flags) ? 'L' : '-');
+       }
 
        /*
         *      check for ACK
@@ -361,6 +364,12 @@ static fr_tls_status_t eaptls_verify(eap_handler_t *handler)
        }
 
        /*
+        *      Calculate this fragment's length
+        */
+       frag_len = eap_ds->response->length -
+                  (EAP_HEADER_LEN + (TLS_LENGTH_INCLUDED(eaptls_packet->flags) ? 6 : 2));
+
+       /*
         *      The L bit (length included) is set to indicate the
         *      presence of the four octet TLS Message Length field,
         *      and MUST be set for the first fragment of a fragmented
@@ -374,28 +383,72 @@ static fr_tls_status_t eaptls_verify(eap_handler_t *handler)
         *      from a fragment acknowledgement.
         */
        if (TLS_LENGTH_INCLUDED(eaptls_packet->flags)) {
-               RDEBUG2("Peer indicated complete TLS record size will be %d bytes",
-                       eaptls_packet->data[2] * 256 | eaptls_packet->data[3]);
+               size_t total_len = eaptls_packet->data[2] * 256 | eaptls_packet->data[3];
+
+               if (frag_len > total_len) {
+                       RWDEBUG("TLS fragment length (%zu bytes) greater than TLS record length (%zu bytes)", frag_len,
+                               total_len);
+               }
+
+               RDEBUG2("Peer indicated complete TLS record size will be %zu bytes", total_len);
                if (TLS_MORE_FRAGMENTS(eaptls_packet->flags)) {
-                       RDEBUG2("Peer indicated TLS record needs fragmenting");
                        /*
-                        * FIRST_FRAGMENT is identified
-                        * 1. If there is no previous EAP-response received.
-                        * 2. If EAP-response received, then its M bit not set.
-                        *      (It is because Last fragment will not have M bit set)
+                        *      The supplicant is free to send fragments of wildly varying
+                        *      lengths, but the vast majority won't.
+                        *
+                        *      In this calculation we take into account the fact that the future
+                        *      fragments are likely to be 4 bytes larger than the initial one
+                        *      as they won't contain the length field.
+                        */
+                       if (frag_len + 4) {     /* check for wrap, else clang scan gets excited */
+                               RDEBUG2("Expecting %i TLS record fragments",
+                                       (int)((((total_len - frag_len) + ((frag_len + 4) - 1)) / (frag_len + 4)) + 1));
+                       }
+
+                       /*
+                        *      FIRST_FRAGMENT is identified
+                        *      1. If there is no previous EAP-response received.
+                        *      2. If EAP-response received, then its M bit not set.
+                        *         (It is because Last fragment will not have M bit set)
                         */
                        if (!prev_eap_ds || (!prev_eap_ds->response) || (!eaptls_prev) ||
                            !TLS_MORE_FRAGMENTS(eaptls_prev->flags)) {
-                               RDEBUG2("Got first TLS record fragment");
+                               RDEBUG2("Got first TLS record fragment (%zu bytes).  Peer indicated more fragments "
+                                       "to follow", frag_len);
+                               tls_session->tls_record_in_total_len = total_len;
+                               tls_session->tls_record_in_recvd_len = frag_len;
+
                                return FR_TLS_FIRST_FRAGMENT;
-                       } else {
-                               RDEBUG2("Got additional TLS record fragment (with length?)");
-                               return FR_TLS_MORE_FRAGMENTS_WITH_LENGTH;
                        }
-               } else {
-                       RDEBUG2("Got complete TLS record (no fragmentation)");
-                       return FR_TLS_LENGTH_INCLUDED;
+
+                       RDEBUG2("Got additional TLS record fragment with length (%zu bytes).  "
+                               "Peer indicated more fragments to follow", frag_len);
+
+                       /*
+                        *      Check we've not exceeded the originally indicated TLS record size.
+                        */
+                       tls_session->tls_record_in_recvd_len += frag_len;
+                       if (tls_session->tls_record_in_recvd_len > tls_session->tls_record_in_total_len) {
+                               RWDEBUG("Total received TLS record fragments (%zu bytes), exceeds "
+                                       "total TLS record length (%zu bytes)", frag_len, total_len);
+                       }
+
+                       return FR_TLS_MORE_FRAGMENTS_WITH_LENGTH;
                }
+
+               /*
+                *      If it's a complete record, our fragment size should match the
+                *      value of the four octet TLS length field.
+                */
+               if (total_len != frag_len) {
+                       RWDEBUG("Peer indicated no more fragments, but TLS record length (%zu bytes) "
+                               "does not match EAP-TLS data length (%zu bytes)", total_len, frag_len);
+               }
+
+               tls_session->tls_record_in_total_len = total_len;
+               tls_session->tls_record_in_recvd_len = frag_len;
+               RDEBUG2("Got complete TLS record (%zu bytes)", frag_len);
+               return FR_TLS_LENGTH_INCLUDED;
        }
 
        /*
@@ -403,16 +456,29 @@ static fr_tls_status_t eaptls_verify(eap_handler_t *handler)
         *      this must be the final record fragment
         */
        if ((eaptls_prev && TLS_MORE_FRAGMENTS(eaptls_prev->flags)) && !TLS_MORE_FRAGMENTS(eaptls_packet->flags)) {
-               RDEBUG2("Got final TLS record fragment");
-       } else {
-               RDEBUG2("Got additional TLS record fragment.  Peer indicated more fragments to follow");
+               RDEBUG2("Got final TLS record fragment (%zu bytes)", frag_len);
+               tls_session->tls_record_in_recvd_len += frag_len;
+               if (tls_session->tls_record_in_recvd_len != tls_session->tls_record_in_total_len) {
+                       RWDEBUG("Total received TLS record fragments (%zu bytes), does not equal indicated "
+                               "TLS record length (%zu bytes)",
+                               tls_session->tls_record_in_recvd_len, tls_session->tls_record_in_total_len);
+               }
        }
 
-       if (TLS_MORE_FRAGMENTS(eaptls_packet->flags)) return FR_TLS_MORE_FRAGMENTS;
+       if (TLS_MORE_FRAGMENTS(eaptls_packet->flags)) {
+               RDEBUG2("Got additional TLS record fragment (%zu bytes).  Peer indicated more fragments to follow",
+                       frag_len);
+               tls_session->tls_record_in_recvd_len += frag_len;
+               if (tls_session->tls_record_in_recvd_len > tls_session->tls_record_in_total_len) {
+                       RWDEBUG("Total received TLS record fragments (%zu bytes), exceeds "
+                               "indicated TLS record length (%zu bytes)",
+                               tls_session->tls_record_in_recvd_len, tls_session->tls_record_in_total_len);
+               }
+               return FR_TLS_MORE_FRAGMENTS;
+       }
 
        /*
-        *      None of the flags are set, but it's still a valid
-        *      EAPTLS packet.
+        *      None of the flags are set, but it's still a valid EAP-TLS packet.
         */
        return FR_TLS_OK;
 }
@@ -508,9 +574,6 @@ static EAPTLS_PACKET *eaptls_extract(REQUEST *request, EAP_DS *eap_ds, fr_tls_st
         *
         *      Likewise, if the EAP packet says N bytes, and the TLS
         *      packet says there's fewer bytes, it's a problem.
-        *
-        *      FIXME: Try to ensure that the claimed length is
-        *      consistent across multiple TLS fragments.
         */
        if (TLS_LENGTH_INCLUDED(tlspacket->flags)) {
                memcpy(&data_len, &eap_ds->response->type.data[1], 4);
@@ -522,16 +585,6 @@ static EAPTLS_PACKET *eaptls_extract(REQUEST *request, EAP_DS *eap_ds, fr_tls_st
                        talloc_free(tlspacket);
                        return NULL;
                }
-
-#if 0
-               DEBUG2(" TLS: %d %d\n", data_len, tlspacket->length);
-
-               if (data_len < tlspacket->length) {
-                       REDEBUG("EAP-TLS packet claims to be smaller than the encapsulating EAP packet");
-                       talloc_free(tlspacket);
-                       return NULL;
-               }
-#endif
        }
 
        switch (status) {
@@ -731,8 +784,8 @@ fr_tls_status_t eaptls_process(eap_handler_t *handler)
 
        SSL_set_ex_data(tls_session->ssl, FR_TLS_EX_INDEX_REQUEST, request);
 
-       if (handler->certs) pairadd(&request->packet->vps,
-                                   paircopy(request->packet, handler->certs));
+       if (handler->certs) fr_pair_add(&request->packet->vps,
+                                   fr_pair_list_copy(request->packet, handler->certs));
 
        /*
         *      This case is when SSL generates Alert then we
@@ -848,6 +901,59 @@ fr_tls_status_t eaptls_process(eap_handler_t *handler)
         *      Continue the handshake.
         */
        status = eaptls_operation(status, handler);
+       if (status == FR_TLS_SUCCESS) {
+#define MAX_SESSION_SIZE (256)
+               VALUE_PAIR *vps;
+               char buffer[2 * MAX_SESSION_SIZE + 1];
+
+               /*
+                *      Restore the cached VPs before processing the
+                *      application data.
+                */
+               tls_session_id(tls_session->ssl_session, buffer, MAX_SESSION_SIZE);
+
+               vps = SSL_SESSION_get_ex_data(tls_session->ssl_session, fr_tls_ex_index_vps);
+               if (!vps) {
+                       RWDEBUG("No information in cached session %s", buffer);
+               } else {
+                       vp_cursor_t cursor;
+                       VALUE_PAIR *vp;
+
+                       RDEBUG("Adding cached attributes from session %s", buffer);
+
+                       /*
+                        *      The cbtls_get_session() function doesn't have
+                        *      access to sock->certs or handler->certs, which
+                        *      is where the certificates normally live.  So
+                        *      the certs are all in the VPS list here, and
+                        *      have to be manually extracted.
+                        */
+                       RINDENT();
+                       for (vp = fr_cursor_init(&cursor, &vps);
+                            vp;
+                            vp = fr_cursor_next(&cursor)) {
+                               /*
+                                *      TLS-* attrs get added back to
+                                *      the request list.
+                                */
+                               if ((vp->da->vendor == 0) &&
+                                   (vp->da->attr >= PW_TLS_CERT_SERIAL) &&
+                                   (vp->da->attr <= PW_TLS_CLIENT_CERT_SUBJECT_ALT_NAME_UPN)) {
+                                       /*
+                                        *      Certs already exist.  Don't re-add them.
+                                        */
+                                       if (!handler->certs) {
+                                               rdebug_pair(L_DBG_LVL_2, request, vp, "request:");
+                                               fr_pair_add(&request->packet->vps, fr_pair_copy(request->packet, vp));
+                                       }
+                               } else {
+                                       rdebug_pair(L_DBG_LVL_2, request, vp, "reply:");
+                                       fr_pair_add(&request->reply->vps, fr_pair_copy(request->reply, vp));
+                               }
+                       }
+                       REXDENT();
+               }
+       }
 
  done:
        SSL_set_ex_data(tls_session->ssl, FR_TLS_EX_INDEX_REQUEST, NULL);