Fixup eap_fast_fast2vp

- Deal with unknown attributes instead of ignoring them.
- Use the same parser style as everywhere else in the code
- Avoid unecessary memcpys
- Use generic network parsing function
- Use cursors properly
- Avoid confusing code paths

diff --git a/src/modules/rlm_eap/types/rlm_eap_fast/eap_fast.c b/src/modules/rlm_eap/types/rlm_eap_fast/eap_fast.c
index 83b7428b4d96ec5ba4c33835b20f9d1a78fb50b1..fb7383a08ae88ca33926ea354fab8ca75814685f 100644 (file)
--- a/src/modules/rlm_eap/types/rlm_eap_fast/eap_fast.c
+++ b/src/modules/rlm_eap/types/rlm_eap_fast/eap_fast.c
@@ -422,75 +422,51 @@ unexpected:
        return 1;
 }
 
-
-VALUE_PAIR *eap_fast_fast2vp(REQUEST *request, SSL *ssl, uint8_t const *data, size_t data_len,
-                             fr_dict_attr_t const *fast_da, vp_cursor_t *out)
+/**
+ *
+ * FIXME do something with mandatory
+ */
+ssize_t eap_fast_decode_pair(TALLOC_CTX *ctx, vp_cursor_t *cursor, fr_dict_attr_t const *parent,
+                            uint8_t const *data, size_t data_len,
+                            UNUSED void *decoder_ctx)
 {
-       uint16_t        attr;
-       uint16_t        length;
-       size_t          data_left = data_len;
-       VALUE_PAIR      *first = NULL;
-       fr_dict_attr_t const *da;
-
-       if (!fast_da)
-               fast_da = fr_dict_attr_by_num(NULL, 0, PW_EAP_FAST_TLV);
-       rad_assert(fast_da != NULL);
-
-       if (!out) {
-               out = talloc(request, vp_cursor_t);
-               rad_assert(out != NULL);
-               fr_pair_cursor_init(out, &first);
-       }
+       fr_dict_attr_t const    *da;
+       uint8_t const           *p = data, *end = p + data_len;
 
        /*
-        * Decode the TLVs
+        *      Decode the TLVs
         */
-       while (data_left > 0) {
-               ssize_t decoded;
-
-               /* FIXME do something with mandatory */
-
-               memcpy(&attr, data, sizeof(attr));
-               attr = ntohs(attr) & EAP_FAST_TLV_TYPE;
-
-               memcpy(&length, data + 2, sizeof(length));
-               length = ntohs(length);
-
-               data += 4;
-               data_left -= 4;
-
-               /*
-                * Look up the TLV.
-                *
-                * For now, if it doesn't exist, ignore it.
-                */
-               da = fr_dict_attr_child_by_num(fast_da, attr);
-               if (!da) goto next_attr;
-
-               if (da->type == FR_TYPE_TLV) {
-                       eap_fast_fast2vp(request, ssl, data, length, da, out);
-                       goto next_attr;
-               }
-
-               decoded = fr_radius_decode_pair_value(request, out, da, data, length, data_left, NULL);
-               if (decoded < 0) {
-                       RERROR("Failed decoding %s: %s", da->name, fr_strerror());
-                       goto next_attr;
+       while (p < end) {
+               ssize_t         ret;
+               uint16_t        attr;
+               uint16_t        len;
+               VALUE_PAIR      *vp;
+
+               attr = ntohs(*((uint16_t const *)p)) & EAP_FAST_TLV_TYPE;
+               p += 2;
+               len = ntohs(*((uint16_t const *)p));
+               p += 2;
+
+               da = fr_dict_attr_child_by_num(parent, attr);
+               if (!da) {
+                       MEM(vp = fr_pair_alloc(ctx));
+                       MEM(vp->da = fr_dict_unknown_afrom_fields(vp, parent, parent->vendor, attr));
+               } else if (da->type != FR_TYPE_TLV) {
+                       p += (size_t) eap_fast_decode_pair(ctx, cursor, parent, data, data_len, decoder_ctx);
+                       continue;
+               } else {
+                       MEM(vp = fr_pair_afrom_da(ctx, da));
                }
 
-       next_attr:
-               while (fr_pair_cursor_next(out)) {
-                       /* nothing */
+               ret = fr_value_box_from_network(vp, &vp->data, vp->vp_type, vp->da, p, len, true);
+               if (ret != len) {
+                       fr_pair_to_unknown(vp);
+                       fr_pair_value_memcpy(vp, p, len);
                }
-
-               data += length;
-               data_left -= length;
+               p += len;
        }
 
-       /*
-        * We got this far.  It looks OK.
-        */
-       return first;
+       return p - data;
 }
 
 
@@ -937,7 +913,8 @@ static PW_CODE eap_fast_process_tlvs(REQUEST *request, eap_session_t *eap_sessio
 PW_CODE eap_fast_process(eap_session_t *eap_session, tls_session_t *tls_session)
 {
        PW_CODE                 code;
-       VALUE_PAIR              *fast_vps;
+       VALUE_PAIR              *fast_vps = NULL;
+       vp_cursor_t             cursor;
        uint8_t const           *data;
        size_t                  data_len;
        eap_fast_tunnel_t       *t;
@@ -990,7 +967,9 @@ PW_CODE eap_fast_process(eap_session_t *eap_session, tls_session_t *tls_session)
                return PW_CODE_ACCESS_CHALLENGE;
        }
 
-       fast_vps = eap_fast_fast2vp(request, tls_session->ssl, data, data_len, NULL, NULL);
+       fr_pair_cursor_init(&cursor, &fast_vps);
+       if (eap_fast_decode_pair(request, &cursor, fr_dict_attr_by_num(NULL, 0, PW_EAP_FAST_TLV),
+                                data, data_len, NULL) < 0) return PW_CODE_ACCESS_REJECT;
 
        RDEBUG("Got Tunneled FAST TLVs");
        rdebug_pair_list(L_DBG_LVL_1, request, fast_vps, NULL);

diff --git a/src/modules/rlm_eap/types/rlm_eap_fast/eap_fast.h b/src/modules/rlm_eap/types/rlm_eap_fast/eap_fast.h
index 984501d62a7cc831a4b6a2b005f0e25ea1c79736..64622e18cd2aa284af9a08e3d197270d295d178e 100644 (file)
--- a/src/modules/rlm_eap/types/rlm_eap_fast/eap_fast.h
+++ b/src/modules/rlm_eap/types/rlm_eap_fast/eap_fast.h
@@ -254,7 +254,8 @@ PW_CODE eap_fast_process(eap_session_t *eap_session, tls_session_t *tls_session)
 /*
  *     A bunch of EAP-FAST helper functions.
  */
-VALUE_PAIR *eap_fast_fast2vp(REQUEST *request, UNUSED SSL *ssl, uint8_t const *data,
-                            size_t data_len, fr_dict_attr_t const *fast_da, vp_cursor_t *out);
+ssize_t                eap_fast_decode_pair(TALLOC_CTX *ctx, vp_cursor_t *cursor, fr_dict_attr_t const *parent,
+                                    uint8_t const *data, size_t data_len,
+                                    UNUSED void *decoder_ctx);
 
 #endif /* _EAP_FAST_H */

diff --git a/src/modules/rlm_eap/types/rlm_eap_fast/rlm_eap_fast.c b/src/modules/rlm_eap/types/rlm_eap_fast/rlm_eap_fast.c
index 57a674e67e175e7e4826c3f848c16fba684b32b3..857ebfb54dc2492d41442f7e86459bc1018020f6 100644 (file)
--- a/src/modules/rlm_eap/types/rlm_eap_fast/rlm_eap_fast.c
+++ b/src/modules/rlm_eap/types/rlm_eap_fast/rlm_eap_fast.c
@@ -210,7 +210,7 @@ static int _session_ticket(SSL *s, uint8_t const *data, int len, void *arg)
        tls_session_t           *tls_session = arg;
        REQUEST                 *request = (REQUEST *)SSL_get_ex_data(s, FR_TLS_EX_INDEX_REQUEST);
        eap_fast_tunnel_t       *t;
-       VALUE_PAIR              *fast_vps = NULL;
+       VALUE_PAIR              *fast_vps = NULL, *vp;
        vp_cursor_t             cursor;
        fr_dict_attr_t const    *fast_da;
        char const              *errmsg;
@@ -276,10 +276,12 @@ error:
        fast_da = fr_dict_attr_by_name(NULL, "EAP-FAST-PAC-Opaque-TLV");
        rad_assert(fast_da != NULL);
 
-       fast_vps = eap_fast_fast2vp((REQUEST *)tls_session, s, (uint8_t *)&opaque_plaintext, plen, fast_da, NULL);
-       if (!fast_vps) return 0;
+       fr_pair_cursor_init(&cursor, &fast_vps);
+       if (eap_fast_decode_pair(tls_session, &cursor, fast_da, (uint8_t *)&opaque_plaintext, plen, NULL) < 0) {
+               goto error;
+       }
 
-       for (VALUE_PAIR *vp = fr_pair_cursor_init(&cursor, &fast_vps); vp; vp = fr_pair_cursor_next(&cursor)) {
+       for (vp = fr_pair_cursor_first(&cursor); vp; vp = fr_pair_cursor_next(&cursor)) {
                char *value;
 
                switch (vp->da->attr) {
@@ -287,11 +289,13 @@ error:
                        rad_assert(t->pac.type == 0);
                        t->pac.type = vp->vp_uint32;
                        break;
+
                case PAC_INFO_PAC_LIFETIME:
                        rad_assert(t->pac.expires == 0);
                        t->pac.expires = vp->vp_uint32;
                        t->pac.expired = (vp->vp_uint32 <= time(NULL));
                        break;
+
                case PAC_INFO_PAC_KEY:
                        rad_assert(t->pac.key == NULL);
                        rad_assert(vp->vp_length == PAC_KEY_LENGTH);
@@ -299,6 +303,7 @@ error:
                        rad_assert(t->pac.key != NULL);
                        memcpy(t->pac.key, vp->vp_octets, PAC_KEY_LENGTH);
                        break;
+
                default:
                        value = fr_pair_asprint(tls_session, vp, '"');
                        RERROR("unknown TLV: %s", value);