remove redundant check. Found by PVS-Studio
[freeradius.git] / src / lib / radius.c
index 3e09232..245b86c 100644 (file)
@@ -36,6 +36,11 @@ RCSID("$Id$")
 #include       <freeradius-devel/udpfromto.h>
 #endif
 
+/*
+ *     Some messages get printed out only in debugging mode.
+ */
+#define FR_DEBUG_STRERROR_PRINTF if (fr_debug_lvl) fr_strerror_printf
+
 #if 0
 #define VP_TRACE printf
 
@@ -60,11 +65,6 @@ static void VP_HEXDUMP(char const *msg, uint8_t const *data, size_t len)
 
 
 /*
- *  The RFC says 4096 octets max, and most packets are less than 256.
- */
-#define MAX_PACKET_LEN 4096
-
-/*
  *     The maximum number of attributes which we allow in an incoming
  *     request.  If there are more attributes than this, the request
  *     is rejected.
@@ -152,7 +152,7 @@ void fr_printf_log(char const *fmt, ...)
        va_list ap;
 
        va_start(ap, fmt);
-       if ((fr_debug_flag == 0) || !fr_log_fp) {
+       if ((fr_debug_lvl == 0) || !fr_log_fp) {
                va_end(ap);
                return;
        }
@@ -179,13 +179,38 @@ static void print_hex_data(uint8_t const *ptr, int attrlen, int depth)
 }
 
 
-void rad_print_hex(RADIUS_PACKET *packet)
+void rad_print_hex(RADIUS_PACKET const *packet)
 {
        int i;
 
        if (!packet->data || !fr_log_fp) return;
 
-       fprintf(fr_log_fp, "  Code:\t\t%u\n", packet->data[0]);
+       fprintf(fr_log_fp, "  Socket:\t%d\n", packet->sockfd);
+#ifdef WITH_TCP
+       fprintf(fr_log_fp, "  Proto:\t%d\n", packet->proto);
+#endif
+
+       if (packet->src_ipaddr.af == AF_INET) {
+               char buffer[32];
+
+               fprintf(fr_log_fp, "  Src IP:\t%s\n",
+                       inet_ntop(packet->src_ipaddr.af,
+                                 &packet->src_ipaddr.ipaddr,
+                                 buffer, sizeof(buffer)));
+               fprintf(fr_log_fp, "    port:\t%u\n", packet->src_port);
+
+               fprintf(fr_log_fp, "  Dst IP:\t%s\n",
+                       inet_ntop(packet->dst_ipaddr.af,
+                                 &packet->dst_ipaddr.ipaddr,
+                                 buffer, sizeof(buffer)));
+               fprintf(fr_log_fp, "    port:\t%u\n", packet->dst_port);
+       }
+
+       if (packet->data[0] < FR_MAX_PACKET_CODE) {
+               fprintf(fr_log_fp, "  Code:\t\t(%d) %s\n", packet->data[0], fr_packet_codes[packet->data[0]]);
+       } else {
+               fprintf(fr_log_fp, "  Code:\t\t%u\n", packet->data[0]);
+       }
        fprintf(fr_log_fp, "  Id:\t\t%u\n", packet->data[1]);
        fprintf(fr_log_fp, "  Length:\t%u\n", ((packet->data[2] << 8) |
                                   (packet->data[3])));
@@ -313,7 +338,20 @@ void rad_recv_discard(int sockfd)
                        (struct sockaddr *)&src, &sizeof_src);
 }
 
-
+/** Basic validation of RADIUS packet header
+ *
+ * @note fr_strerror errors are only available if fr_debug_lvl > 0. This is to reduce CPU time
+ *     consumed when discarding malformed packet.
+ *
+ * @param[in] sockfd we're reading from.
+ * @param[out] src_ipaddr of the packet.
+ * @param[out] src_port of the packet.
+ * @param[out] code Pointer to where to write the packet code.
+ * @return
+ *     - -1 on failure.
+ *     - 1 on decode error.
+ *     - >= RADIUS_HDR_LEN on success. This is the packet length as specified in the header.
+ */
 ssize_t rad_recv_header(int sockfd, fr_ipaddr_t *src_ipaddr, uint16_t *src_port, int *code)
 {
        ssize_t                 data_len, packet_len;
@@ -321,54 +359,59 @@ ssize_t rad_recv_header(int sockfd, fr_ipaddr_t *src_ipaddr, uint16_t *src_port,
        struct sockaddr_storage src;
        socklen_t               sizeof_src = sizeof(src);
 
-       data_len = recvfrom(sockfd, header, sizeof(header), MSG_PEEK,
-                           (struct sockaddr *)&src, &sizeof_src);
+       data_len = recvfrom(sockfd, header, sizeof(header), MSG_PEEK, (struct sockaddr *)&src, &sizeof_src);
        if (data_len < 0) {
                if ((errno == EAGAIN) || (errno == EINTR)) return 0;
                return -1;
        }
 
        /*
-        *      Too little data is available, discard the packet.
+        *      Convert AF.  If unknown, discard packet.
         */
-       if (data_len < 4) {
+       if (!fr_sockaddr2ipaddr(&src, sizeof_src, src_ipaddr, src_port)) {
+               FR_DEBUG_STRERROR_PRINTF("Unknown address family");
                rad_recv_discard(sockfd);
 
                return 1;
+       }
 
-       } else {                /* we got 4 bytes of data. */
-               /*
-                *      See how long the packet says it is.
-                */
-               packet_len = (header[2] * 256) + header[3];
-
-               /*
-                *      The length in the packet says it's less than
-                *      a RADIUS header length: discard it.
-                */
-               if (packet_len < RADIUS_HDR_LEN) {
-                       rad_recv_discard(sockfd);
+       /*
+        *      Too little data is available, discard the packet.
+        */
+       if (data_len < 4) {
+               FR_DEBUG_STRERROR_PRINTF("Expected at least 4 bytes of header data, got %zu bytes", data_len);
+invalid:
+               FR_DEBUG_STRERROR_PRINTF("Invalid data from %s: %s",
+                                        fr_inet_ntop(src_ipaddr->af, &src_ipaddr->ipaddr),
+                                        fr_strerror());
+               rad_recv_discard(sockfd);
 
-                       return 1;
+               return 1;
+       }
 
-                       /*
-                        *      Enforce RFC requirements, for sanity.
-                        *      Anything after 4k will be discarded.
-                        */
-               } else if (packet_len > MAX_PACKET_LEN) {
-                       rad_recv_discard(sockfd);
+       /*
+        *      See how long the packet says it is.
+        */
+       packet_len = (header[2] * 256) + header[3];
 
-                       return 1;
-               }
+       /*
+        *      The length in the packet says it's less than
+        *      a RADIUS header length: discard it.
+        */
+       if (packet_len < RADIUS_HDR_LEN) {
+               FR_DEBUG_STRERROR_PRINTF("Expected at least " STRINGIFY(RADIUS_HDR_LEN)  " bytes of packet "
+                                        "data, got %zu bytes", packet_len);
+               goto invalid;
        }
 
        /*
-        *      Convert AF.  If unknown, discard packet.
+        *      Enforce RFC requirements, for sanity.
+        *      Anything after 4k will be discarded.
         */
-       if (!fr_sockaddr2ipaddr(&src, sizeof_src, src_ipaddr, src_port)) {
-               rad_recv_discard(sockfd);
-
-               return 1;
+       if (packet_len > MAX_PACKET_LEN) {
+               FR_DEBUG_STRERROR_PRINTF("Length field value too large, expected maximum of "
+                                        STRINGIFY(MAX_PACKET_LEN) " bytes, got %zu bytes", packet_len);
+               goto invalid;
        }
 
        *code = header[0];
@@ -571,62 +614,59 @@ static void make_passwd(uint8_t *output, ssize_t *outlen,
        memcpy(output, passwd, len);
 }
 
+
 static void make_tunnel_passwd(uint8_t *output, ssize_t *outlen,
                               uint8_t const *input, size_t inlen, size_t room,
                               char const *secret, uint8_t const *vector)
 {
        FR_MD5_CTX context, old;
        uint8_t digest[AUTH_VECTOR_LEN];
-       uint8_t passwd[MAX_STRING_LEN + AUTH_VECTOR_LEN];
-       int     i, n;
-       int     len;
+       size_t  i, n;
+       size_t  encrypted_len;
 
        /*
-        *      Be paranoid.
+        *      The password gets encoded with a 1-byte "length"
+        *      field.  Ensure that it doesn't overflow.
         */
        if (room > 253) room = 253;
 
        /*
-        *      Account for 2 bytes of the salt, and round the room
-        *      available down to the nearest multiple of 16.  Then,
-        *      subtract one from that to account for the length byte,
-        *      and the resulting number is the upper bound on the data
-        *      to copy.
-        *
-        *      We could short-cut this calculation just be forcing
-        *      inlen to be no more than 239.  It would work for all
-        *      VSA's, as we don't pack multiple VSA's into one
-        *      attribute.
-        *
-        *      However, this calculation is more general, if a little
-        *      complex.  And it will work in the future for all possible
-        *      kinds of weird attribute packing.
+        *      Limit the maximum size of the input password.  2 bytes
+        *      are taken up by the salt, and one by the encoded
+        *      "length" field.  Note that if we have a tag, the
+        *      "room" will be 252 octets, not 253 octets.
         */
-       room -= 2;
-       room -= (room & 0x0f);
-       room--;
-
-       if (inlen > room) inlen = room;
+       if (inlen > (room - 3)) inlen = room - 3;
 
        /*
-        *      Length of the encrypted data is password length plus
-        *      one byte for the length of the password.
+        *      Length of the encrypted data is the clear-text
+        *      password length plus one byte which encodes the length
+        *      of the password.  We round up to the nearest encoding
+        *      block.  Note that this can result in the encoding
+        *      length being more than 253 octets.
         */
-       len = inlen + 1;
-       if ((len & 0x0f) != 0) {
-               len += 0x0f;
-               len &= ~0x0f;
+       encrypted_len = inlen + 1;
+       if ((encrypted_len & 0x0f) != 0) {
+               encrypted_len += 0x0f;
+               encrypted_len &= ~0x0f;
        }
-       *outlen = len + 2;      /* account for the salt */
 
        /*
-        *      Copy the password over.
+        *      We need 2 octets for the salt, followed by the actual
+        *      encrypted data.
         */
-       memcpy(passwd + 3, input, inlen);
-       memset(passwd + 3 + inlen, 0, sizeof(passwd) - 3 - inlen);
+       if (encrypted_len > (room - 2)) encrypted_len = room - 2;
+
+       *outlen = encrypted_len + 2;    /* account for the salt */
 
        /*
-        *      Generate salt.  The RFC's say:
+        *      Copy the password over, and zero-fill the remainder.
+        */
+       memcpy(output + 3, input, inlen);
+       memset(output + 3 + inlen, 0, *outlen - 3 - inlen);
+
+       /*
+        *      Generate salt.  The RFCs say:
         *
         *      The high bit of salt[0] must be set, each salt in a
         *      packet should be unique, and they should be random
@@ -634,39 +674,42 @@ static void make_tunnel_passwd(uint8_t *output, ssize_t *outlen,
         *      So, we set the high bit, add in a counter, and then
         *      add in some CSPRNG data.  should be OK..
         */
-       passwd[0] = (0x80 | ( ((salt_offset++) & 0x0f) << 3) |
+       output[0] = (0x80 | ( ((salt_offset++) & 0x0f) << 3) |
                     (fr_rand() & 0x07));
-       passwd[1] = fr_rand();
-       passwd[2] = inlen;      /* length of the password string */
+       output[1] = fr_rand();
+       output[2] = inlen;      /* length of the password string */
 
        fr_md5_init(&context);
        fr_md5_update(&context, (uint8_t const *) secret, strlen(secret));
        old = context;
 
        fr_md5_update(&context, vector, AUTH_VECTOR_LEN);
-       fr_md5_update(&context, &passwd[0], 2);
+       fr_md5_update(&context, &output[0], 2);
+
+       for (n = 0; n < encrypted_len; n += AUTH_PASS_LEN) {
+               size_t block_len;
 
-       for (n = 0; n < len; n += AUTH_PASS_LEN) {
                if (n > 0) {
                        context = old;
                        fr_md5_update(&context,
-                                      passwd + 2 + n - AUTH_PASS_LEN,
+                                      output + 2 + n - AUTH_PASS_LEN,
                                       AUTH_PASS_LEN);
                }
 
                fr_md5_final(digest, &context);
 
-               for (i = 0; i < AUTH_PASS_LEN; i++) {
-                       passwd[i + 2 + n] ^= digest[i];
+               if ((2 + n + AUTH_PASS_LEN) < room) {
+                       block_len = AUTH_PASS_LEN;
+               } else {
+                       block_len = room - 2 - n;
+               }
+
+               for (i = 0; i < block_len; i++) {
+                       output[i + 2 + n] ^= digest[i];
                }
        }
-       memcpy(output, passwd, len + 2);
 }
 
-extern int const fr_attr_max_tlv;
-extern int const fr_attr_shift[];
-extern int const fr_attr_mask[];
-
 static int do_next_tlv(VALUE_PAIR const *vp, VALUE_PAIR const *next, int nest)
 {
        unsigned int tlv1, tlv2;
@@ -765,7 +808,7 @@ static ssize_t vp2data_tlvs(RADIUS_PACKET const *packet,
                ptr[1] += len;
 
 #ifndef NDEBUG
-               if ((fr_debug_flag > 3) && fr_log_fp) {
+               if ((fr_debug_lvl > 3) && fr_log_fp) {
                        fprintf(fr_log_fp, "\t\t%02x %02x  ", ptr[0], ptr[1]);
                        print_hex_data(ptr + 2, len, 3);
                }
@@ -779,7 +822,7 @@ static ssize_t vp2data_tlvs(RADIUS_PACKET const *packet,
        }
 
 #ifndef NDEBUG
-       if ((fr_debug_flag > 3) && fr_log_fp) {
+       if ((fr_debug_lvl > 3) && fr_log_fp) {
                DICT_ATTR const *da;
 
                da = dict_attrbyvalue(svp->da->attr & ((1 << fr_attr_shift[nest ]) - 1), svp->da->vendor);
@@ -827,17 +870,13 @@ static ssize_t vp2data_any(RADIUS_PACKET const *packet,
        /*
         *      Set up the default sources for the data.
         */
-       len = vp->length;
+       len = vp->vp_length;
 
-       switch(vp->da->type) {
+       switch (vp->da->type) {
        case PW_TYPE_STRING:
        case PW_TYPE_OCTETS:
-       case PW_TYPE_TLV:
                data = vp->data.ptr;
-               if (!data) {
-                       fr_strerror_printf("ERROR: Cannot encode NULL data");
-                       return -1;
-               }
+               if (!data) return 0;
                break;
 
        case PW_TYPE_IFID:
@@ -953,13 +992,15 @@ static ssize_t vp2data_any(RADIUS_PACKET const *packet,
                        make_tunnel_passwd(ptr + lvalue, &len, data, len,
                                           room - lvalue,
                                           secret, original->vector);
+                       len += lvalue;
                        break;
                case PW_CODE_ACCOUNTING_REQUEST:
                case PW_CODE_DISCONNECT_REQUEST:
                case PW_CODE_COA_REQUEST:
                        ptr[0] = TAG_VALID(vp->tag) ? vp->tag : TAG_NONE;
-                       make_tunnel_passwd(ptr + 1, &len, data, len - 1, room,
+                       make_tunnel_passwd(ptr + 1, &len, data, len, room - 1,
                                           secret, packet->vector);
+                       len += lvalue;
                        break;
                }
                break;
@@ -1137,7 +1178,7 @@ int rad_vp2extended(RADIUS_PACKET const *packet,
        ptr[1] += len;
 
 #ifndef NDEBUG
-       if ((fr_debug_flag > 3) && fr_log_fp) {
+       if ((fr_debug_lvl > 3) && fr_log_fp) {
                int jump = 3;
 
                fprintf(fr_log_fp, "\t\t%02x %02x  ", ptr[0], ptr[1]);
@@ -1230,7 +1271,7 @@ int rad_vp2wimax(RADIUS_PACKET const *packet,
        ptr[7] += len;
 
 #ifndef NDEBUG
-       if ((fr_debug_flag > 3) && fr_log_fp) {
+       if ((fr_debug_lvl > 3) && fr_log_fp) {
                fprintf(fr_log_fp, "\t\t%02x %02x  %02x%02x%02x%02x (%u)  %02x %02x %02x   ",
                       ptr[0], ptr[1],
                       ptr[2], ptr[3], ptr[4], ptr[5],
@@ -1261,7 +1302,7 @@ static ssize_t vp2attr_concat(UNUSED RADIUS_PACKET const *packet,
        VERIFY_VP(vp);
 
        p = vp->vp_octets;
-       len = vp->length;
+       len = vp->vp_length;
 
        while (len > 0) {
                if (room <= 2) break;
@@ -1280,7 +1321,7 @@ static ssize_t vp2attr_concat(UNUSED RADIUS_PACKET const *packet,
                memcpy(ptr + 2, p, left);
 
 #ifndef NDEBUG
-               if ((fr_debug_flag > 3) && fr_log_fp) {
+               if ((fr_debug_lvl > 3) && fr_log_fp) {
                        fprintf(fr_log_fp, "\t\t%02x %02x  ", ptr[0], ptr[1]);
                        print_hex_data(ptr + 2, len, 3);
                }
@@ -1314,15 +1355,15 @@ static ssize_t vp2attr_rfc(RADIUS_PACKET const *packet,
        ptr[0] = attribute & 0xff;
        ptr[1] = 2;
 
-       if (room > ((unsigned) 255 - ptr[1])) room = 255 - ptr[1];
+       if (room > 255) room = 255;
 
-       len = vp2data_any(packet, original, secret, 0, pvp, ptr + ptr[1], room);
+       len = vp2data_any(packet, original, secret, 0, pvp, ptr + ptr[1], room - ptr[1]);
        if (len <= 0) return len;
 
        ptr[1] += len;
 
 #ifndef NDEBUG
-       if ((fr_debug_flag > 3) && fr_log_fp) {
+       if ((fr_debug_lvl > 3) && fr_log_fp) {
                fprintf(fr_log_fp, "\t\t%02x %02x  ", ptr[0], ptr[1]);
                print_hex_data(ptr + 2, len, 3);
        }
@@ -1401,36 +1442,34 @@ static ssize_t vp2attr_vsa(RADIUS_PACKET const *packet,
 
        }
 
-       if (room > ((unsigned) 255 - (dv->type + dv->length))) {
-               room = 255 - (dv->type + dv->length);
-       }
+       if (room > 255) room = 255;
 
        len = vp2data_any(packet, original, secret, 0, pvp,
-                         ptr + dv->type + dv->length, room);
+                         ptr + dv->type + dv->length, room - (dv->type + dv->length));
        if (len <= 0) return len;
 
        if (dv->length) ptr[dv->type + dv->length - 1] += len;
 
 #ifndef NDEBUG
-       if ((fr_debug_flag > 3) && fr_log_fp) {
+       if ((fr_debug_lvl > 3) && fr_log_fp) {
                switch (dv->type) {
                default:
                        break;
 
                case 4:
-                       if ((fr_debug_flag > 3) && fr_log_fp)
+                       if ((fr_debug_lvl > 3) && fr_log_fp)
                                fprintf(fr_log_fp, "\t\t%02x%02x%02x%02x ",
                                        ptr[0], ptr[1], ptr[2], ptr[3]);
                        break;
 
                case 2:
-                       if ((fr_debug_flag > 3) && fr_log_fp)
+                       if ((fr_debug_lvl > 3) && fr_log_fp)
                                fprintf(fr_log_fp, "\t\t%02x%02x ",
                                        ptr[0], ptr[1]);
                break;
 
                case 1:
-                       if ((fr_debug_flag > 3) && fr_log_fp)
+                       if ((fr_debug_lvl > 3) && fr_log_fp)
                                fprintf(fr_log_fp, "\t\t%02x ", ptr[0]);
                        break;
                }
@@ -1474,12 +1513,17 @@ int rad_vp2vsa(RADIUS_PACKET const *packet, RADIUS_PACKET const *original,
        VALUE_PAIR const *vp = *pvp;
 
        VERIFY_VP(vp);
+
+       if (vp->da->vendor == 0) {
+               fr_strerror_printf("rad_vp2vsa called with rfc attribute");
+               return -1;
+       }
+
        /*
         *      Double-check for WiMAX format.
         */
        if (vp->da->flags.wimax) {
-               return rad_vp2wimax(packet, original, secret, pvp,
-                                   ptr, room);
+               return rad_vp2wimax(packet, original, secret, pvp, ptr, room);
        }
 
        if (vp->da->vendor > FR_MAX_VENDOR) {
@@ -1501,15 +1545,15 @@ int rad_vp2vsa(RADIUS_PACKET const *packet, RADIUS_PACKET const *original,
        lvalue = htonl(vp->da->vendor);
        memcpy(ptr + 2, &lvalue, 4);
 
-       if (room > ((unsigned) 255 - ptr[1])) room = 255 - ptr[1];
+       if (room > 255) room = 255;
 
        len = vp2attr_vsa(packet, original, secret, pvp,
                          vp->da->attr, vp->da->vendor,
-                         ptr + ptr[1], room);
+                         ptr + ptr[1], room - ptr[1]);
        if (len < 0) return len;
 
 #ifndef NDEBUG
-       if ((fr_debug_flag > 3) && fr_log_fp) {
+       if ((fr_debug_lvl > 3) && fr_log_fp) {
                fprintf(fr_log_fp, "\t\t%02x %02x  %02x%02x%02x%02x (%u)  ",
                       ptr[0], ptr[1],
                       ptr[2], ptr[3], ptr[4], ptr[5],
@@ -1550,7 +1594,7 @@ int rad_vp2rfc(RADIUS_PACKET const *packet,
         *      Only CUI is allowed to have zero length.
         *      Thank you, WiMAX!
         */
-       if ((vp->length == 0) &&
+       if ((vp->vp_length == 0) &&
            (vp->da->attr == PW_CHARGEABLE_USER_IDENTITY)) {
                ptr[0] = PW_CHARGEABLE_USER_IDENTITY;
                ptr[1] = 2;
@@ -1569,7 +1613,7 @@ int rad_vp2rfc(RADIUS_PACKET const *packet,
                ptr[1] = 18;
                memset(ptr + 2, 0, 16);
 #ifndef NDEBUG
-               if ((fr_debug_flag > 3) && fr_log_fp) {
+               if ((fr_debug_lvl > 3) && fr_log_fp) {
                        fprintf(fr_log_fp, "\t\t50 12 ...\n");
                }
 #endif
@@ -1581,7 +1625,7 @@ int rad_vp2rfc(RADIUS_PACKET const *packet,
        /*
         *      EAP-Message is special.
         */
-       if (vp->da->flags.concat && (vp->length > 253)) {
+       if (vp->da->flags.concat && (vp->vp_length > 253)) {
                return vp2attr_concat(packet, original, secret, pvp, vp->da->attr,
                                      ptr, room);
        }
@@ -1653,7 +1697,10 @@ int rad_vp2attr(RADIUS_PACKET const *packet, RADIUS_PACKET const *original,
         *      RFC format attributes take the fast path.
         */
        if (!vp->da->vendor) {
-               if (vp->da->attr > 255) return 0;
+               if (vp->da->attr > 255) {
+                       *pvp = vp->next;
+                       return 0;
+               }
 
                return rad_vp2rfc(packet, original, secret, pvp,
                                  start, room);
@@ -1678,8 +1725,7 @@ int rad_vp2attr(RADIUS_PACKET const *packet, RADIUS_PACKET const *original,
                                    start, room);
        }
 
-       return rad_vp2vsa(packet, original, secret, pvp,
-                         start, room);
+       return rad_vp2vsa(packet, original, secret, pvp, start, room);
 }
 
 
@@ -1762,7 +1808,7 @@ int rad_encode(RADIUS_PACKET *packet, RADIUS_PACKET const *original,
         */
        reply = packet->vps;
        while (reply) {
-               size_t last_len;
+               size_t last_len, room;
                char const *last_name = NULL;
 
                VERIFY_VP(reply);
@@ -1780,8 +1826,8 @@ int rad_encode(RADIUS_PACKET *packet, RADIUS_PACKET const *original,
                         *      attributes with a debug build.
                         */
                        if (reply->da->attr == PW_RAW_ATTRIBUTE) {
-                               memcpy(ptr, reply->vp_octets, reply->length);
-                               len = reply->length;
+                               memcpy(ptr, reply->vp_octets, reply->vp_length);
+                               len = reply->vp_length;
                                reply = reply->next;
                                goto next;
                        }
@@ -1791,10 +1837,24 @@ int rad_encode(RADIUS_PACKET *packet, RADIUS_PACKET const *original,
                }
 
                /*
+                *      We allow zero-length strings in "unlang", but
+                *      skip them (except for CUI, thanks WiMAX!) on
+                *      all other attributes.
+                */
+               if (reply->vp_length == 0) {
+                       if ((reply->da->vendor != 0) ||
+                           ((reply->da->attr != PW_CHARGEABLE_USER_IDENTITY) &&
+                            (reply->da->attr != PW_MESSAGE_AUTHENTICATOR))) {
+                               reply = reply->next;
+                               continue;
+                       }
+               }
+
+               /*
                 *      Set the Message-Authenticator to the correct
                 *      length and initial value.
                 */
-               if (reply->da->attr == PW_MESSAGE_AUTHENTICATOR) {
+               if (!reply->da->vendor && (reply->da->attr == PW_MESSAGE_AUTHENTICATOR)) {
                        /*
                         *      Cache the offset to the
                         *      Message-Authenticator
@@ -1802,12 +1862,15 @@ int rad_encode(RADIUS_PACKET *packet, RADIUS_PACKET const *original,
                        packet->offset = total_length;
                        last_len = 16;
                } else {
-                       last_len = reply->length;
+                       last_len = reply->vp_length;
                }
                last_name = reply->da->name;
 
-               len = rad_vp2attr(packet, original, secret, &reply, ptr,
-                                 ((uint8_t *) data) + sizeof(data) - ptr);
+               room = ((uint8_t *) data) + sizeof(data) - ptr;
+
+               if (room <= 2) break;
+
+               len = rad_vp2attr(packet, original, secret, &reply, ptr, room);
                if (len < 0) return -1;
 
                /*
@@ -1878,8 +1941,44 @@ int rad_sign(RADIUS_PACKET *packet, RADIUS_PACKET const *original,
        }
 
        /*
+        *      Set up the authentication vector with zero, or with
+        *      the original vector, prior to signing.
+        */
+       switch (packet->code) {
+       case PW_CODE_ACCOUNTING_REQUEST:
+       case PW_CODE_DISCONNECT_REQUEST:
+       case PW_CODE_COA_REQUEST:
+               memset(packet->vector, 0, AUTH_VECTOR_LEN);
+               break;
+
+       case PW_CODE_ACCESS_ACCEPT:
+       case PW_CODE_ACCESS_REJECT:
+       case PW_CODE_ACCESS_CHALLENGE:
+       case PW_CODE_ACCOUNTING_RESPONSE:
+       case PW_CODE_DISCONNECT_ACK:
+       case PW_CODE_DISCONNECT_NAK:
+       case PW_CODE_COA_ACK:
+       case PW_CODE_COA_NAK:
+               if (!original) {
+                       fr_strerror_printf("ERROR: Cannot sign response packet without a request packet");
+                       return -1;
+               }
+               memcpy(packet->vector, original->vector, AUTH_VECTOR_LEN);
+               break;
+
+       case PW_CODE_ACCESS_REQUEST:
+       case PW_CODE_STATUS_SERVER:
+       default:
+               break;          /* packet->vector is already random bytes */
+       }
+
+#ifndef NDEBUG
+       if ((fr_debug_lvl > 3) && fr_log_fp) rad_print_hex(packet);
+#endif
+
+       /*
         *      If there's a Message-Authenticator, update it
-        *      now, BEFORE updating the authentication vector.
+        *      now.
         */
        if (packet->offset > 0) {
                uint8_t calc_auth_vector[AUTH_VECTOR_LEN];
@@ -1896,6 +1995,7 @@ int rad_sign(RADIUS_PACKET *packet, RADIUS_PACKET const *original,
                case PW_CODE_DISCONNECT_NAK:
                case PW_CODE_COA_REQUEST:
                case PW_CODE_COA_ACK:
+               case PW_CODE_COA_NAK:
                        memset(hdr->vector, 0, AUTH_VECTOR_LEN);
                        break;
 
@@ -1903,17 +2003,11 @@ int rad_sign(RADIUS_PACKET *packet, RADIUS_PACKET const *original,
                case PW_CODE_ACCESS_ACCEPT:
                case PW_CODE_ACCESS_REJECT:
                case PW_CODE_ACCESS_CHALLENGE:
-                       if (!original) {
-                               fr_strerror_printf("ERROR: Cannot sign response packet without a request packet");
-                               return -1;
-                       }
-                       memcpy(hdr->vector, original->vector,
-                              AUTH_VECTOR_LEN);
+                       memcpy(hdr->vector, original->vector, AUTH_VECTOR_LEN);
                        break;
 
-               default:        /* others have vector already set to zero */
+               default:
                        break;
-
                }
 
                /*
@@ -1926,21 +2020,20 @@ int rad_sign(RADIUS_PACKET *packet, RADIUS_PACKET const *original,
                            (uint8_t const *) secret, strlen(secret));
                memcpy(packet->data + packet->offset + 2,
                       calc_auth_vector, AUTH_VECTOR_LEN);
-
-               /*
-                *      Copy the original request vector back
-                *      to the raw packet.
-                */
-               memcpy(hdr->vector, packet->vector, AUTH_VECTOR_LEN);
        }
 
        /*
+        *      Copy the request authenticator over to the packet.
+        */
+       memcpy(hdr->vector, packet->vector, AUTH_VECTOR_LEN);
+
+       /*
         *      Switch over the packet code, deciding how to
         *      sign the packet.
         */
        switch (packet->code) {
                /*
-                *      Request packets are not signed, bur
+                *      Request packets are not signed, but
                 *      have a random authentication vector.
                 */
        case PW_CODE_ACCESS_REQUEST:
@@ -2011,7 +2104,7 @@ int rad_send(RADIUS_PACKET *packet, RADIUS_PACKET const *original,
        }
 
 #ifndef NDEBUG
-       if ((fr_debug_flag > 3) && fr_log_fp) rad_print_hex(packet);
+       if ((fr_debug_lvl > 3) && fr_log_fp) rad_print_hex(packet);
 #endif
 
 #ifdef WITH_TCP
@@ -2181,7 +2274,10 @@ int rad_tlv_ok(uint8_t const *data, size_t length,
                        break;
 
                case 1:
-                       if (data[0] == 0) goto zero;
+                       /*
+                        *      Zero is allowed, because the Colubris
+                        *      people are dumb and use it.
+                        */
                        break;
 
                default:
@@ -2248,6 +2344,8 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
        bool                    seen_ma = false;
        uint32_t                num_attributes;
        decode_fail_t           failure = DECODE_FAIL_NONE;
+       bool                    eap = false;
+       bool                    non_eap = false;
 
        /*
         *      Check for packets smaller than the packet header.
@@ -2257,7 +2355,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
         *      "The minimum length is 20 ..."
         */
        if (packet->data_len < RADIUS_HDR_LEN) {
-               fr_strerror_printf("WARNING: Malformed RADIUS packet from host %s: too short (received %zu < minimum %d)",
+               FR_DEBUG_STRERROR_PRINTF("Malformed RADIUS packet from host %s: too short (received %zu < minimum %d)",
                           inet_ntop(packet->src_ipaddr.af,
                                     &packet->src_ipaddr.ipaddr,
                                     host_ipaddr, sizeof(host_ipaddr)),
@@ -2281,7 +2379,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
         */
        if ((hdr->code == 0) ||
            (hdr->code >= FR_MAX_PACKET_CODE)) {
-               fr_strerror_printf("WARNING: Bad RADIUS packet from host %s: unknown packet code %d",
+               FR_DEBUG_STRERROR_PRINTF("Bad RADIUS packet from host %s: unknown packet code %d",
                           inet_ntop(packet->src_ipaddr.af,
                                     &packet->src_ipaddr.ipaddr,
                                     host_ipaddr, sizeof(host_ipaddr)),
@@ -2313,7 +2411,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
         *      "The minimum length is 20 ..."
         */
        if (totallen < RADIUS_HDR_LEN) {
-               fr_strerror_printf("WARNING: Malformed RADIUS packet from host %s: too short (length %zu < minimum %d)",
+               FR_DEBUG_STRERROR_PRINTF("Malformed RADIUS packet from host %s: too short (length %zu < minimum %d)",
                           inet_ntop(packet->src_ipaddr.af,
                                     &packet->src_ipaddr.ipaddr,
                                     host_ipaddr, sizeof(host_ipaddr)),
@@ -2346,7 +2444,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
         *      i.e. No response to the NAS.
         */
        if (packet->data_len < totallen) {
-               fr_strerror_printf("WARNING: Malformed RADIUS packet from host %s: received %zu octets, packet length says %zu",
+               FR_DEBUG_STRERROR_PRINTF("Malformed RADIUS packet from host %s: received %zu octets, packet length says %zu",
                           inet_ntop(packet->src_ipaddr.af,
                                     &packet->src_ipaddr.ipaddr,
                                     host_ipaddr, sizeof(host_ipaddr)),
@@ -2392,7 +2490,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
                 *      attribute header.
                 */
                if (count < 2) {
-                       fr_strerror_printf("WARNING: Malformed RADIUS packet from host %s: attribute header overflows the packet",
+                       FR_DEBUG_STRERROR_PRINTF("Malformed RADIUS packet from host %s: attribute header overflows the packet",
                                   inet_ntop(packet->src_ipaddr.af,
                                             &packet->src_ipaddr.ipaddr,
                                             host_ipaddr, sizeof(host_ipaddr)));
@@ -2404,7 +2502,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
                 *      Attribute number zero is NOT defined.
                 */
                if (attr[0] == 0) {
-                       fr_strerror_printf("WARNING: Malformed RADIUS packet from host %s: Invalid attribute 0",
+                       FR_DEBUG_STRERROR_PRINTF("Malformed RADIUS packet from host %s: Invalid attribute 0",
                                   inet_ntop(packet->src_ipaddr.af,
                                             &packet->src_ipaddr.ipaddr,
                                             host_ipaddr, sizeof(host_ipaddr)));
@@ -2417,7 +2515,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
                 *      fields.  Anything shorter is an invalid attribute.
                 */
                if (attr[1] < 2) {
-                       fr_strerror_printf("WARNING: Malformed RADIUS packet from host %s: attribute %u too short",
+                       FR_DEBUG_STRERROR_PRINTF("Malformed RADIUS packet from host %s: attribute %u too short",
                                   inet_ntop(packet->src_ipaddr.af,
                                             &packet->src_ipaddr.ipaddr,
                                             host_ipaddr, sizeof(host_ipaddr)),
@@ -2431,7 +2529,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
                 *      attribute, it's a bad packet.
                 */
                if (count < attr[1]) {
-                       fr_strerror_printf("WARNING: Malformed RADIUS packet from host %s: attribute %u data overflows the packet",
+                       FR_DEBUG_STRERROR_PRINTF("Malformed RADIUS packet from host %s: attribute %u data overflows the packet",
                                   inet_ntop(packet->src_ipaddr.af,
                                             &packet->src_ipaddr.ipaddr,
                                             host_ipaddr, sizeof(host_ipaddr)),
@@ -2453,11 +2551,18 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
                         */
                case PW_EAP_MESSAGE:
                        require_ma = true;
+                       eap = true;
+                       break;
+
+               case PW_USER_PASSWORD:
+               case PW_CHAP_PASSWORD:
+               case PW_ARAP_PASSWORD:
+                       non_eap = true;
                        break;
 
                case PW_MESSAGE_AUTHENTICATOR:
                        if (attr[1] != 2 + AUTH_VECTOR_LEN) {
-                               fr_strerror_printf("WARNING: Malformed RADIUS packet from host %s: Message-Authenticator has invalid length %d",
+                               FR_DEBUG_STRERROR_PRINTF("Malformed RADIUS packet from host %s: Message-Authenticator has invalid length %d",
                                           inet_ntop(packet->src_ipaddr.af,
                                                     &packet->src_ipaddr.ipaddr,
                                                     host_ipaddr, sizeof(host_ipaddr)),
@@ -2486,7 +2591,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
         *      If not, we complain, and throw the packet away.
         */
        if (count != 0) {
-               fr_strerror_printf("WARNING: Malformed RADIUS packet from host %s: packet attributes do NOT exactly fill the packet",
+               FR_DEBUG_STRERROR_PRINTF("Malformed RADIUS packet from host %s: packet attributes do NOT exactly fill the packet",
                           inet_ntop(packet->src_ipaddr.af,
                                     &packet->src_ipaddr.ipaddr,
                                     host_ipaddr, sizeof(host_ipaddr)));
@@ -2501,7 +2606,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
         */
        if ((fr_max_attributes > 0) &&
            (num_attributes > fr_max_attributes)) {
-               fr_strerror_printf("WARNING: Possible DoS attack from host %s: Too many attributes in request (received %d, max %d are allowed).",
+               FR_DEBUG_STRERROR_PRINTF("Possible DoS attack from host %s: Too many attributes in request (received %d, max %d are allowed).",
                           inet_ntop(packet->src_ipaddr.af,
                                     &packet->src_ipaddr.ipaddr,
                                     host_ipaddr, sizeof(host_ipaddr)),
@@ -2522,7 +2627,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
         *      Message-Authenticator attributes.
         */
        if (require_ma && !seen_ma) {
-               fr_strerror_printf("WARNING: Insecure packet from host %s:  Packet does not contain required Message-Authenticator attribute",
+               FR_DEBUG_STRERROR_PRINTF("Insecure packet from host %s:  Packet does not contain required Message-Authenticator attribute",
                           inet_ntop(packet->src_ipaddr.af,
                                     &packet->src_ipaddr.ipaddr,
                                     host_ipaddr, sizeof(host_ipaddr)));
@@ -2530,6 +2635,15 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
                goto finish;
        }
 
+       if (eap && non_eap) {
+               FR_DEBUG_STRERROR_PRINTF("Bad packet from host %s:  Packet contains EAP-Message and non-EAP authentication attribute",
+                          inet_ntop(packet->src_ipaddr.af,
+                                    &packet->src_ipaddr.ipaddr,
+                                    host_ipaddr, sizeof(host_ipaddr)));
+               failure = DECODE_FAIL_TOO_MANY_AUTH;
+               goto finish;
+       }
+
        /*
         *      Fill RADIUS header fields
         */
@@ -2550,7 +2664,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
 /** Receive UDP client requests, and fill in the basics of a RADIUS_PACKET structure
  *
  */
-RADIUS_PACKET *rad_recv(int fd, int flags)
+RADIUS_PACKET *rad_recv(TALLOC_CTX *ctx, int fd, int flags)
 {
        int sock_flags = 0;
        ssize_t data_len;
@@ -2559,7 +2673,7 @@ RADIUS_PACKET *rad_recv(int fd, int flags)
        /*
         *      Allocate the new request data structure
         */
-       packet = rad_alloc(NULL, false);
+       packet = rad_alloc(ctx, false);
        if (!packet) {
                fr_strerror_printf("out of memory");
                return NULL;
@@ -2578,7 +2692,7 @@ RADIUS_PACKET *rad_recv(int fd, int flags)
         *      Check for socket errors.
         */
        if (data_len < 0) {
-               fr_strerror_printf("Error receiving packet: %s", fr_syserror(errno));
+               FR_DEBUG_STRERROR_PRINTF("Error receiving packet: %s", fr_syserror(errno));
                /* packet->data is NULL */
                rad_free(&packet);
                return NULL;
@@ -2591,7 +2705,7 @@ RADIUS_PACKET *rad_recv(int fd, int flags)
         *      packet.
         */
        if (packet->data_len > MAX_PACKET_LEN) {
-               fr_strerror_printf("Discarding packet: Larger than RFC limitation of 4096 bytes");
+               FR_DEBUG_STRERROR_PRINTF("Discarding packet: Larger than RFC limitation of 4096 bytes");
                /* packet->data is NULL */
                rad_free(&packet);
                return NULL;
@@ -2604,7 +2718,7 @@ RADIUS_PACKET *rad_recv(int fd, int flags)
         *      packet->data == NULL
         */
        if ((packet->data_len == 0) || !packet->data) {
-               fr_strerror_printf("Empty packet: Socket is not ready");
+               FR_DEBUG_STRERROR_PRINTF("Empty packet: Socket is not ready");
                rad_free(&packet);
                return NULL;
        }
@@ -2634,7 +2748,7 @@ RADIUS_PACKET *rad_recv(int fd, int flags)
        packet->vps = NULL;
 
 #ifndef NDEBUG
-       if ((fr_debug_flag > 3) && fr_log_fp) rad_print_hex(packet);
+       if ((fr_debug_lvl > 3) && fr_log_fp) rad_print_hex(packet);
 #endif
 
        return packet;
@@ -2644,12 +2758,13 @@ RADIUS_PACKET *rad_recv(int fd, int flags)
 /** Verify the Request/Response Authenticator (and Message-Authenticator if present) of a packet
  *
  */
-int rad_verify(RADIUS_PACKET *packet, RADIUS_PACKET *original,
-              char const *secret)
+int rad_verify(RADIUS_PACKET *packet, RADIUS_PACKET *original, char const *secret)
 {
-       uint8_t                 *ptr;
-       int                     length;
-       int                     attrlen;
+       uint8_t         *ptr;
+       int             length;
+       int             attrlen;
+       int             rcode;
+       char            buffer[32];
 
        if (!packet || !packet->data) return -1;
 
@@ -2702,7 +2817,8 @@ int rad_verify(RADIUS_PACKET *packet, RADIUS_PACKET *original,
                        case PW_CODE_COA_ACK:
                        case PW_CODE_COA_NAK:
                                if (!original) {
-                                       fr_strerror_printf("ERROR: Cannot validate Message-Authenticator in response packet without a request packet");
+                                       fr_strerror_printf("Cannot validate Message-Authenticator in response "
+                                                          "packet without a request packet");
                                        return -1;
                                }
                                memcpy(packet->data + 4, original->vector, AUTH_VECTOR_LEN);
@@ -2713,11 +2829,11 @@ int rad_verify(RADIUS_PACKET *packet, RADIUS_PACKET *original,
                                    (uint8_t const *) secret, strlen(secret));
                        if (rad_digest_cmp(calc_auth_vector, msg_auth_vector,
                                   sizeof(calc_auth_vector)) != 0) {
-                               char buffer[32];
-                               fr_strerror_printf("Received packet from %s with invalid Message-Authenticator!  (Shared secret is incorrect.)",
-                                          inet_ntop(packet->src_ipaddr.af,
-                                                    &packet->src_ipaddr.ipaddr,
-                                                    buffer, sizeof(buffer)));
+                               fr_strerror_printf("Received packet from %s with invalid Message-Authenticator!  "
+                                                  "(Shared secret is incorrect.)",
+                                                  inet_ntop(packet->src_ipaddr.af,
+                                                            &packet->src_ipaddr.ipaddr,
+                                                            buffer, sizeof(buffer)));
                                /* Silently drop packet, according to RFC 3579 */
                                return -1;
                        } /* else the message authenticator was good */
@@ -2739,14 +2855,13 @@ int rad_verify(RADIUS_PACKET *packet, RADIUS_PACKET *original,
         *      so can't validate the authenticators.
         */
        if ((packet->code == 0) || (packet->code >= FR_MAX_PACKET_CODE)) {
-               char buffer[32];
                fr_strerror_printf("Received Unknown packet code %d "
-                          "from client %s port %d: Cannot validate Request/Response Authenticator.",
-                          packet->code,
-                          inet_ntop(packet->src_ipaddr.af,
-                                    &packet->src_ipaddr.ipaddr,
-                                    buffer, sizeof(buffer)),
-                          packet->src_port);
+                                  "from client %s port %d: Cannot validate Request/Response Authenticator.",
+                                  packet->code,
+                                  inet_ntop(packet->src_ipaddr.af,
+                                            &packet->src_ipaddr.ipaddr,
+                                            buffer, sizeof(buffer)),
+                                  packet->src_port);
                return -1;
        }
 
@@ -2754,9 +2869,6 @@ int rad_verify(RADIUS_PACKET *packet, RADIUS_PACKET *original,
         *      Calculate and/or verify Request or Response Authenticator.
         */
        switch (packet->code) {
-       int rcode;
-       char buffer[32];
-
        case PW_CODE_ACCESS_REQUEST:
        case PW_CODE_STATUS_SERVER:
                /*
@@ -2770,11 +2882,12 @@ int rad_verify(RADIUS_PACKET *packet, RADIUS_PACKET *original,
        case PW_CODE_ACCOUNTING_REQUEST:
                if (calc_acctdigest(packet, secret) > 1) {
                        fr_strerror_printf("Received %s packet "
-                                  "from client %s with invalid Request Authenticator!  (Shared secret is incorrect.)",
-                                  fr_packet_codes[packet->code],
-                                  inet_ntop(packet->src_ipaddr.af,
-                                            &packet->src_ipaddr.ipaddr,
-                                            buffer, sizeof(buffer)));
+                                          "from client %s with invalid Request Authenticator!  "
+                                          "(Shared secret is incorrect.)",
+                                          fr_packet_codes[packet->code],
+                                          inet_ntop(packet->src_ipaddr.af,
+                                                    &packet->src_ipaddr.ipaddr,
+                                                    buffer, sizeof(buffer)));
                        return -1;
                }
                break;
@@ -2791,24 +2904,25 @@ int rad_verify(RADIUS_PACKET *packet, RADIUS_PACKET *original,
                rcode = calc_replydigest(packet, original, secret);
                if (rcode > 1) {
                        fr_strerror_printf("Received %s packet "
-                                  "from home server %s port %d with invalid Response Authenticator!  (Shared secret is incorrect.)",
-                                  fr_packet_codes[packet->code],
-                                  inet_ntop(packet->src_ipaddr.af,
-                                            &packet->src_ipaddr.ipaddr,
-                                            buffer, sizeof(buffer)),
-                                  packet->src_port);
+                                          "from home server %s port %d with invalid Response Authenticator!  "
+                                          "(Shared secret is incorrect.)",
+                                          fr_packet_codes[packet->code],
+                                          inet_ntop(packet->src_ipaddr.af,
+                                                    &packet->src_ipaddr.ipaddr,
+                                                    buffer, sizeof(buffer)),
+                                          packet->src_port);
                        return -1;
                }
                break;
 
        default:
                fr_strerror_printf("Received Unknown packet code %d "
-                          "from client %s port %d: Cannot validate Request/Response Authenticator",
-                          packet->code,
-                          inet_ntop(packet->src_ipaddr.af,
-                                    &packet->src_ipaddr.ipaddr,
-                                            buffer, sizeof(buffer)),
-                          packet->src_port);
+                                  "from client %s port %d: Cannot validate Request/Response Authenticator",
+                                  packet->code,
+                                  inet_ntop(packet->src_ipaddr.af,
+                                            &packet->src_ipaddr.ipaddr,
+                                            buffer, sizeof(buffer)),
+                                  packet->src_port);
                return -1;
        }
 
@@ -2848,19 +2962,19 @@ static ssize_t data2vp_concat(TALLOC_CTX *ctx,
                if (ptr[0] != attr) break;
        }
 
-       vp = pairalloc(ctx, da);
+       vp = fr_pair_afrom_da(ctx, da);
        if (!vp) return -1;
 
-       vp->length = total;
-       vp->vp_octets = p = talloc_array(vp, uint8_t, vp->length);
+       vp->vp_length = total;
+       vp->vp_octets = p = talloc_array(vp, uint8_t, vp->vp_length);
        if (!p) {
-               pairfree(&vp);
+               fr_pair_list_free(&vp);
                return -1;
        }
 
        total = 0;
        ptr = start;
-       while (total < vp->length) {
+       while (total < vp->vp_length) {
                memcpy(p, ptr + 2, ptr[1] - 2);
                p += ptr[1] - 2;
                total += ptr[1] - 2;
@@ -2875,7 +2989,7 @@ static ssize_t data2vp_concat(TALLOC_CTX *ctx,
 /** Convert TLVs to one or more VPs
  *
  */
-static ssize_t data2vp_tlvs(TALLOC_CTX *ctx,
+ssize_t rad_data2vp_tlvs(TALLOC_CTX *ctx,
                            RADIUS_PACKET *packet, RADIUS_PACKET const *original,
                            char const *secret, DICT_ATTR const *da,
                            uint8_t const *start, size_t length,
@@ -2912,13 +3026,13 @@ static ssize_t data2vp_tlvs(TALLOC_CTX *ctx,
                        my_vendor = da->vendor;
 
                        if (!dict_attr_child(da, &my_attr, &my_vendor)) {
-                               pairfree(&head);
+                               fr_pair_list_free(&head);
                                return -1;
                        }
 
                        child = dict_unknown_afrom_fields(ctx, my_attr, my_vendor);
                        if (!child) {
-                               pairfree(&head);
+                               fr_pair_list_free(&head);
                                return -1;
                        }
                }
@@ -2926,10 +3040,10 @@ static ssize_t data2vp_tlvs(TALLOC_CTX *ctx,
                tlv_len = data2vp(ctx, packet, original, secret, child,
                                  data + 2, data[1] - 2, data[1] - 2, tail);
                if (tlv_len < 0) {
-                       pairfree(&head);
+                       fr_pair_list_free(&head);
                        return -1;
                }
-               tail = &((*tail)->next);
+               if (*tail) tail = &((*tail)->next);
                data += data[1];
        }
 
@@ -3001,13 +3115,6 @@ static ssize_t data2vp_vsa(TALLOC_CTX *ctx, RADIUS_PACKET *packet,
                return -1;
        }
 
-#ifndef NDEBUG
-       if (attrlen <= (ssize_t) (dv->type + dv->length)) {
-               fr_strerror_printf("data2vp_vsa: Failure to call rad_tlv_ok");
-               return -1;
-       }
-#endif
-
        /*
         *      See if the VSA is known.
         */
@@ -3299,11 +3406,16 @@ create_attrs:
                vsa_len = data2vp_vsa(ctx, packet, original, secret, dv,
                                      data, attrlen, tail);
                if (vsa_len < 0) {
-                       pairfree(&head);
+                       fr_pair_list_free(&head);
                        fr_strerror_printf("Internal sanity check %d", __LINE__);
                        return -1;
                }
-               tail = &((*tail)->next);
+
+               /*
+                *      Vendors can send zero-length VSAs.
+                */
+               if (*tail) tail = &((*tail)->next);
+
                data += vsa_len;
                attrlen -= vsa_len;
                packetlen -= vsa_len;
@@ -3335,7 +3447,6 @@ ssize_t data2vp(TALLOC_CTX *ctx,
        ssize_t rcode;
        uint32_t vendor;
        DICT_ATTR const *child;
-       DICT_VENDOR *dv;
        VALUE_PAIR *vp;
        uint8_t const *data = start;
        char *p;
@@ -3448,7 +3559,29 @@ ssize_t data2vp(TALLOC_CTX *ctx,
                                             packet->vector);
                        }
                        buffer[253] = '\0';
-                       datalen = strlen((char *) buffer);
+
+                       /*
+                        *      MS-CHAP-MPPE-Keys are 24 octets, and
+                        *      encrypted.  Since it's binary, we can't
+                        *      look for trailing zeros.
+                        */
+                       if (da->flags.length) {
+                               if (datalen > da->flags.length) {
+                                       datalen = da->flags.length;
+                               } /* else leave datalen alone */
+                       } else {
+                               /*
+                                *      Take off trailing zeros from the END.
+                                *      This allows passwords to have zeros in
+                                *      the middle of a field.
+                                *
+                                *      However, if the password has a zero at
+                                *      the end, it will get mashed by this
+                                *      code.  There's really no way around
+                                *      that.
+                                */
+                               while ((datalen > 0) && (buffer[datalen - 1] == '\0')) datalen--;
+                       }
                        break;
 
                /*
@@ -3534,7 +3667,7 @@ ssize_t data2vp(TALLOC_CTX *ctx,
                if (datalen != 6) goto raw;
                break;
 
-       case PW_TYPE_IP_ADDR:
+       case PW_TYPE_COMBO_IP_ADDR:
                if (datalen == 4) {
                        child = dict_attrbytype(da->attr, da->vendor,
                                                PW_TYPE_IPV4_ADDR);
@@ -3628,16 +3761,19 @@ ssize_t data2vp(TALLOC_CTX *ctx,
 
                memcpy(&vendor, data, 4);
                vendor = ntohl(vendor);
-               dv = dict_vendorbyvalue(vendor);
-               if (!dv) {
-                       child = dict_unknown_afrom_fields(ctx, data[4], da->vendor | vendor);
-               } else {
-                       child = dict_attrbyparent(da, data[4], vendor);
-                       if (!child) {
-                               child = dict_unknown_afrom_fields(ctx, data[4], da->vendor | vendor);
-                       }
+               vendor |= da->vendor;
+
+               child = dict_attrbyvalue(data[4], vendor);
+               if (!child) {
+                       /*
+                        *      Create a "raw" attribute from the
+                        *      contents of the EVS VSA.
+                        */
+                       da = dict_unknown_afrom_fields(ctx, data[4], vendor);
+                       data += 5;
+                       datalen -= 5;
+                       break;
                }
-               if (!child) goto raw;
 
                rcode = data2vp(ctx, packet, original, secret, child,
                                data + 5, attrlen - 5, attrlen - 5, pvp);
@@ -3650,8 +3786,8 @@ ssize_t data2vp(TALLOC_CTX *ctx,
                 *      attribute, OR they've already been grouped
                 *      into a contiguous memory buffer.
                 */
-               rcode = data2vp_tlvs(ctx, packet, original, secret, da,
-                                    data, attrlen, pvp);
+               rcode = rad_data2vp_tlvs(ctx, packet, original, secret, da,
+                                        data, attrlen, pvp);
                if (rcode < 0) goto raw;
                return rcode;
 
@@ -3695,29 +3831,29 @@ ssize_t data2vp(TALLOC_CTX *ctx,
         *      information, decode the actual data.
         */
  alloc_cui:
-       vp = pairalloc(ctx, da);
+       vp = fr_pair_afrom_da(ctx, da);
        if (!vp) return -1;
 
-       vp->length = datalen;
+       vp->vp_length = datalen;
        vp->tag = tag;
 
        switch (da->type) {
        case PW_TYPE_STRING:
-               p = talloc_array(vp, char, vp->length + 1);
-               memcpy(p, data, vp->length);
-               p[vp->length] = '\0';
+               p = talloc_array(vp, char, vp->vp_length + 1);
+               memcpy(p, data, vp->vp_length);
+               p[vp->vp_length] = '\0';
                vp->vp_strvalue = p;
                break;
 
        case PW_TYPE_OCTETS:
-               pairmemcpy(vp, data, vp->length);
+               fr_pair_value_memcpy(vp, data, vp->vp_length);
                break;
 
        case PW_TYPE_ABINARY:
-               if (vp->length > sizeof(vp->vp_filter)) {
-                       vp->length = sizeof(vp->vp_filter);
+               if (vp->vp_length > sizeof(vp->vp_filter)) {
+                       vp->vp_length = sizeof(vp->vp_filter);
                }
-               memcpy(vp->vp_filter, data, vp->length);
+               memcpy(vp->vp_filter, data, vp->vp_length);
                break;
 
        case PW_TYPE_BYTE:
@@ -3744,7 +3880,7 @@ ssize_t data2vp(TALLOC_CTX *ctx,
                break;
 
        case PW_TYPE_ETHERNET:
-               memcpy(&vp->vp_ether, data, 6);
+               memcpy(vp->vp_ether, data, 6);
                break;
 
        case PW_TYPE_IPV4_ADDR:
@@ -3752,7 +3888,7 @@ ssize_t data2vp(TALLOC_CTX *ctx,
                break;
 
        case PW_TYPE_IFID:
-               memcpy(&vp->vp_ifid, data, 8);
+               memcpy(vp->vp_ifid, data, 8);
                break;
 
        case PW_TYPE_IPV6_ADDR:
@@ -3762,18 +3898,18 @@ ssize_t data2vp(TALLOC_CTX *ctx,
        case PW_TYPE_IPV6_PREFIX:
                /*
                 *      FIXME: double-check that
-                *      (vp->vp_octets[1] >> 3) matches vp->length + 2
+                *      (vp->vp_octets[1] >> 3) matches vp->vp_length + 2
                 */
-               memcpy(&vp->vp_ipv6prefix, data, vp->length);
-               if (vp->length < 18) {
-                       memset(((uint8_t *)vp->vp_ipv6prefix) + vp->length, 0,
-                              18 - vp->length);
+               memcpy(vp->vp_ipv6prefix, data, vp->vp_length);
+               if (vp->vp_length < 18) {
+                       memset(((uint8_t *)vp->vp_ipv6prefix) + vp->vp_length, 0,
+                              18 - vp->vp_length);
                }
                break;
 
        case PW_TYPE_IPV4_PREFIX:
                /* FIXME: do the same double-check as for IPv6Prefix */
-               memcpy(&vp->vp_ipv4prefix, data, vp->length);
+               memcpy(vp->vp_ipv4prefix, data, vp->vp_length);
 
                /*
                 *      /32 means "keep all bits".  Otherwise, mask
@@ -3799,7 +3935,7 @@ ssize_t data2vp(TALLOC_CTX *ctx,
                break;
 
        default:
-               pairfree(&vp);
+               fr_pair_list_free(&vp);
                fr_strerror_printf("Internal sanity check %d", __LINE__);
                return -1;
        }
@@ -3856,7 +3992,7 @@ ssize_t rad_attr2vp(TALLOC_CTX *ctx,
        return 2 + rcode;
 }
 
-fr_thread_local_setup(uint8_t *, rad_vp2data_buff);
+fr_thread_local_setup(uint8_t *, rad_vp2data_buff)
 
 /** Converts vp_data to network byte order
  *
@@ -3898,10 +4034,9 @@ ssize_t rad_vp2data(uint8_t const **out, VALUE_PAIR const *vp)
 
        VERIFY_VP(vp);
 
-       switch(vp->da->type) {
+       switch (vp->da->type) {
        case PW_TYPE_STRING:
        case PW_TYPE_OCTETS:
-       case PW_TYPE_TLV:
                memcpy(out, &vp->data.ptr, sizeof(*out));
                break;
 
@@ -3915,8 +4050,8 @@ ssize_t rad_vp2data(uint8_t const **out, VALUE_PAIR const *vp)
        case PW_TYPE_IPV4_PREFIX:
        case PW_TYPE_ABINARY:
        case PW_TYPE_ETHERNET:
-       case PW_TYPE_IP_ADDR:
-       case PW_TYPE_IP_PREFIX:
+       case PW_TYPE_COMBO_IP_ADDR:
+       case PW_TYPE_COMBO_IP_PREFIX:
        {
                void const *p = &vp->data;
                memcpy(out, &p, sizeof(*out));
@@ -3970,6 +4105,7 @@ ssize_t rad_vp2data(uint8_t const **out, VALUE_PAIR const *vp)
        case PW_TYPE_LONG_EXTENDED:
        case PW_TYPE_EVS:
        case PW_TYPE_VSA:
+       case PW_TYPE_TLV:
        case PW_TYPE_TIMEVAL:
        case PW_TYPE_MAX:
                fr_strerror_printf("Cannot get data for VALUE_PAIR type %i", vp->da->type);
@@ -3978,7 +4114,7 @@ ssize_t rad_vp2data(uint8_t const **out, VALUE_PAIR const *vp)
        /* Don't add default */
        }
 
-       return vp->length;
+       return vp->vp_length;
 }
 
 /** Calculate/check digest, and decode radius attributes
@@ -4017,7 +4153,7 @@ int rad_decode(RADIUS_PACKET *packet, RADIUS_PACKET *original,
                my_len = rad_attr2vp(packet, packet, original, secret,
                                     ptr, packet_length, &vp);
                if (my_len < 0) {
-                       pairfree(&head);
+                       fr_pair_list_free(&head);
                        return -1;
                }
 
@@ -4038,8 +4174,8 @@ int rad_decode(RADIUS_PACKET *packet, RADIUS_PACKET *original,
                    (num_attributes > fr_max_attributes)) {
                        char host_ipaddr[128];
 
-                       pairfree(&head);
-                       fr_strerror_printf("WARNING: Possible DoS attack from host %s: Too many attributes in request (received %d, max %d are allowed).",
+                       fr_pair_list_free(&head);
+                       fr_strerror_printf("Possible DoS attack from host %s: Too many attributes in request (received %d, max %d are allowed).",
                                   inet_ntop(packet->src_ipaddr.af,
                                             &packet->src_ipaddr.ipaddr,
                                             host_ipaddr, sizeof(host_ipaddr)),
@@ -4217,8 +4353,7 @@ int rad_pwdecode(char *passwd, size_t pwlen, char const *secret,
  * This is per RFC-2868 which adds a two char SALT to the initial intermediate
  * value MD5 hash.
  */
-int rad_tunnel_pwencode(char *passwd, size_t *pwlen, char const *secret,
-                       uint8_t const *vector)
+ssize_t rad_tunnel_pwencode(char *passwd, size_t *pwlen, char const *secret, uint8_t const *vector)
 {
        uint8_t buffer[AUTH_VECTOR_LEN + MAX_STRING_LEN + 3];
        unsigned char   digest[AUTH_VECTOR_LEN];
@@ -4231,14 +4366,15 @@ int rad_tunnel_pwencode(char *passwd, size_t *pwlen, char const *secret,
        if (len > 127) len = 127;
 
        /*
-        * Shift the password 3 positions right to place a salt and original
-        * length, tag will be added automatically on packet send
+        *      Shift the password 3 positions right to place a salt and original
+        *      length, tag will be added automatically on packet send.
         */
-       for (n=len ; n>=0 ; n--) passwd[n+3] = passwd[n];
+       for (n = len ; n >= 0 ; n--) passwd[n + 3] = passwd[n];
        salt = passwd;
        passwd += 2;
+
        /*
-        * save original password length as first password character;
+        *      save original password length as first password character;
         */
        *passwd = len;
        len += 1;
@@ -4299,20 +4435,19 @@ int rad_tunnel_pwencode(char *passwd, size_t *pwlen, char const *secret,
  * initial intermediate value, to differentiate it from the
  * above.
  */
-int rad_tunnel_pwdecode(uint8_t *passwd, size_t *pwlen, char const *secret,
-                       uint8_t const *vector)
+ssize_t rad_tunnel_pwdecode(uint8_t *passwd, size_t *pwlen, char const *secret, uint8_t const *vector)
 {
        FR_MD5_CTX  context, old;
        uint8_t         digest[AUTH_VECTOR_LEN];
        int             secretlen;
-       unsigned        i, n, len, reallen;
+       size_t          i, n, encrypted_len, reallen;
 
-       len = *pwlen;
+       encrypted_len = *pwlen;
 
        /*
         *      We need at least a salt.
         */
-       if (len < 2) {
+       if (encrypted_len < 2) {
                fr_strerror_printf("tunnel password is too short");
                return -1;
        }
@@ -4327,13 +4462,13 @@ int rad_tunnel_pwdecode(uint8_t *passwd, size_t *pwlen, char const *secret,
         *      more data.  So the 'data_len' field may be wrong,
         *      but that's ok...
         */
-       if (len <= 3) {
+       if (encrypted_len <= 3) {
                passwd[0] = 0;
                *pwlen = 0;
                return 0;
        }
 
-       len -= 2;               /* discount the salt */
+       encrypted_len -= 2;             /* discount the salt */
 
        /*
         *      Use the secret to setup the decryption digest
@@ -4353,10 +4488,20 @@ int rad_tunnel_pwdecode(uint8_t *passwd, size_t *pwlen, char const *secret,
        fr_md5_update(&context, passwd, 2);
 
        reallen = 0;
-       for (n = 0; n < len; n += AUTH_PASS_LEN) {
-               int base = 0;
+       for (n = 0; n < encrypted_len; n += AUTH_PASS_LEN) {
+               size_t base;
+               size_t block_len = AUTH_PASS_LEN;
+
+               /*
+                *      Ensure we don't overflow the input on MD5
+                */
+               if ((n + 2 + AUTH_PASS_LEN) > *pwlen) {
+                       block_len = *pwlen - n - 2;
+               }
 
                if (n == 0) {
+                       base = 1;
+
                        fr_md5_final(digest, &context);
 
                        context = old;
@@ -4367,31 +4512,27 @@ int rad_tunnel_pwdecode(uint8_t *passwd, size_t *pwlen, char const *secret,
                         *      'data_len' field.  Ensure it's sane.
                         */
                        reallen = passwd[2] ^ digest[0];
-                       if (reallen >len) {
+                       if (reallen > encrypted_len) {
                                fr_strerror_printf("tunnel password is too long for the attribute");
                                return -1;
                        }
 
-                       fr_md5_update(&context, passwd + 2, AUTH_PASS_LEN);
+                       fr_md5_update(&context, passwd + 2, block_len);
 
-                       base = 1;
                } else {
+                       base = 0;
+
                        fr_md5_final(digest, &context);
 
                        context = old;
-                       fr_md5_update(&context, passwd + n + 2, AUTH_PASS_LEN);
+                       fr_md5_update(&context, passwd + n + 2, block_len);
                }
 
-               for (i = base; i < AUTH_PASS_LEN; i++) {
+               for (i = base; i < block_len; i++) {
                        passwd[n + i - 1] = passwd[n + i + 2] ^ digest[i];
                }
        }
 
-       /*
-        *      See make_tunnel_password, above.
-        */
-       if (reallen > 239) reallen = 239;
-
        *pwlen = reallen;
        passwd[reallen] = 0;
 
@@ -4401,7 +4542,7 @@ int rad_tunnel_pwdecode(uint8_t *passwd, size_t *pwlen, char const *secret,
 /** Encode a CHAP password
  *
  * @bug FIXME: might not work with Ascend because
- * we use vp->length, and Ascend gear likes
+ * we use vp->vp_length, and Ascend gear likes
  * to send an extra '\0' in the string!
  */
 int rad_chap_encode(RADIUS_PACKET *packet, uint8_t *output, int id,
@@ -4431,18 +4572,18 @@ int rad_chap_encode(RADIUS_PACKET *packet, uint8_t *output, int id,
        *ptr++ = id;
 
        i++;
-       memcpy(ptr, password->vp_strvalue, password->length);
-       ptr += password->length;
-       i += password->length;
+       memcpy(ptr, password->vp_strvalue, password->vp_length);
+       ptr += password->vp_length;
+       i += password->vp_length;
 
        /*
         *      Use Chap-Challenge pair if present,
         *      Request Authenticator otherwise.
         */
-       challenge = pairfind(packet->vps, PW_CHAP_CHALLENGE, 0, TAG_ANY);
+       challenge = fr_pair_find_by_num(packet->vps, PW_CHAP_CHALLENGE, 0, TAG_ANY);
        if (challenge) {
-               memcpy(ptr, challenge->vp_strvalue, challenge->length);
-               i += challenge->length;
+               memcpy(ptr, challenge->vp_strvalue, challenge->vp_length);
+               i += challenge->vp_length;
        } else {
                memcpy(ptr, packet->vector, AUTH_VECTOR_LEN);
                i += AUTH_VECTOR_LEN;
@@ -4621,7 +4762,7 @@ void rad_free(RADIUS_PACKET **radius_packet_ptr)
 
        VERIFY_PACKET(radius_packet);
 
-       pairfree(&radius_packet->vps);
+       fr_pair_list_free(&radius_packet->vps);
 
        talloc_free(radius_packet);
        *radius_packet_ptr = NULL;
@@ -4654,7 +4795,7 @@ RADIUS_PACKET *rad_copy_packet(TALLOC_CTX *ctx, RADIUS_PACKET const *in)
        out->data = NULL;
        out->data_len = 0;
 
-       out->vps = paircopy(out, in->vps);
+       out->vps = fr_pair_list_copy(out, in->vps);
        out->offset = 0;
 
        return out;