X-Git-Url: http://www.project-moonshot.org/gitweb/?a=blobdiff_plain;f=src%2Flib%2Fradius.c;h=439feece62feb61080bcd1ec921eec3d4a294b6a;hb=94dc4bb60ec649ce899c1d5e32b575d9523a48f2;hp=7447f3f7e41ccf37710197d6f41ddb3ab60d8a09;hpb=5dcd6495fcd56a199cbc42c3ab99874e993ff2c2;p=freeradius.git diff --git a/src/lib/radius.c b/src/lib/radius.c index 7447f3f..439feec 100644 --- a/src/lib/radius.c +++ b/src/lib/radius.c @@ -233,7 +233,7 @@ static int rad_sendto(int sockfd, void *data, size_t data_len, int flags, * And if they don't specify a source IP address, don't * use udpfromto. */ - if ((dst_ipaddr->af == AF_INET) || + if ((dst_ipaddr->af == AF_INET) && (src_ipaddr->af != AF_UNSPEC)) { return sendfromto(sockfd, data, data_len, flags, (struct sockaddr *)&src, sizeof_src, @@ -816,7 +816,8 @@ static int rad_vp2rfc(const RADIUS_PACKET *packet, ptr[0] = attribute & 0xff; /* NOT vp->attribute */ ptr[1] = 2; - len = vp2data(packet, original, secret, vp, ptr + 2, room - 2); + if (room > (255 - ptr[1])) room = 255 - ptr[1]; + len = vp2data(packet, original, secret, vp, ptr + 2, room); if (len < 0) return len; ptr[1] += len; @@ -824,27 +825,34 @@ static int rad_vp2rfc(const RADIUS_PACKET *packet, return ptr[1]; } +extern int fr_wimax_max_tlv; +extern int fr_wimax_shift[]; +extern int fr_wimax_mask[]; + static int tlv2data(const RADIUS_PACKET *packet, const RADIUS_PACKET *original, - const char *secret, const VALUE_PAIR *vp, int attribute, - uint8_t *ptr, size_t room) + const char *secret, const VALUE_PAIR *vp, + uint8_t *ptr, size_t room, int nest) { int len; + if (nest > fr_wimax_max_tlv) return -1; + if (room < 2) return 0; room -= 2; - ptr[0] = attribute & 0xff; + ptr[0] = (vp->attribute >> fr_wimax_shift[nest]) & fr_wimax_mask[nest]; ptr[1] = 2; /* * No more nested TLVs: pack the data. */ - if ((attribute & ~0xff) == 0) { + if ((nest == fr_wimax_max_tlv) || + ((vp->attribute >> fr_wimax_shift[nest + 1]) == 0)) { len = vp2data(packet, original, secret, vp, ptr + 2, room); } else { - len = tlv2data(packet, original, secret, vp, attribute >> 8, - ptr + 2, room); + len = tlv2data(packet, original, secret, vp, ptr + 2, room, + nest + 1); } if (len <= 0) return len; @@ -902,8 +910,7 @@ static int wimax2data(const RADIUS_PACKET *packet, if (!vp->flags.is_tlv) { len = vp2data(packet, original, secret, vp, ptr, room); } else { - len = tlv2data(packet, original, secret, vp, vp->attribute >> 8, - ptr, room); + len = tlv2data(packet, original, secret, vp, ptr, room, 1); } if (len <= 0) return len; @@ -915,6 +922,49 @@ static int wimax2data(const RADIUS_PACKET *packet, } +static int rad_vp2extended(const RADIUS_PACKET *packet, + const RADIUS_PACKET *original, + const char *secret, const VALUE_PAIR *vp, + unsigned int attribute, uint8_t *ptr, size_t room) +{ + int len = 2; + + if (room < 3) return 0; + + ptr[0] = attribute & 0xff; /* NOT vp->attribute */ + ptr[1] = 3; + + if (vp->flags.extended) { + ptr[2] = (attribute & 0xff00) >> 8; + + } else if (vp->flags.extended_flags) { + if (room < 4) return 0; + + ptr[1] = 4; + ptr[2] = (attribute & 0xff00) >> 8; + ptr[3] = 0; + } + + /* + * For now, no extended attribute can be longer than the + * encapsulating attribute. Once we add support for the + * "M" bit, this restriction will be relaxed. + */ + if (room > (255 - ptr[1])) room = 255 - ptr[1]; + + if (!vp->flags.is_tlv) { + len = vp2data(packet, original, secret, vp, ptr + ptr[1], room); + } else { + len = tlv2data(packet, original, secret, vp, ptr + ptr[1], room, 2); + } + + if (len < 0) return len; + + ptr[1] += len; + + return ptr[1]; +} + /* * Parse a data structure into a RADIUS attribute. */ @@ -931,23 +981,13 @@ int rad_vp2attr(const RADIUS_PACKET *packet, const RADIUS_PACKET *original, * RFC format attributes take the fast path. */ if (vp->vendor == 0) { - len = rad_vp2rfc(packet, original, secret, vp, - vp->attribute, start, room); - if (len < 0) return -1; - - /* - * RFC 2865 section 5 says that zero-length - * attributes MUST NOT be sent. - * - * ... and the WiMAX forum ignores - * this... because of one vendor. Don't they - * have anything better to do with their time? - */ - if ((len == 0) && - (vp->attribute != PW_CHARGEABLE_USER_IDENTITY)) return 0; - - return len; + return rad_vp2rfc(packet, original, secret, vp, + vp->attribute, start, room); + } + if (vp->vendor == VENDORPEC_EXTENDED) { + return rad_vp2extended(packet, original, secret, vp, + vp->attribute, start, room); } /* @@ -1058,7 +1098,7 @@ static int rad_encode_wimax(const RADIUS_PACKET *packet, { int len, redo; uint32_t lvalue; - uint8_t *ptr = start, *vsa = start; + uint8_t *ptr, *vsa; uint32_t maxattr; VALUE_PAIR *vp = reply; @@ -1090,8 +1130,7 @@ redo_vsa: room -= 9; redo_tlv: - len = tlv2data(packet, original, secret, vp, vp->attribute >> 8, - ptr, room); + len = tlv2data(packet, original, secret, vp, ptr, room, 1); if (len < 0) return len; /* @@ -1228,10 +1267,12 @@ int rad_encode(RADIUS_PACKET *packet, const RADIUS_PACKET *original, */ for (reply = packet->vps; reply; reply = reply->next) { /* - * Ignore non-wire attributes + * Ignore non-wire attributes, but allow extended + * attributes. */ if ((reply->vendor == 0) && - ((reply->attribute & 0xFFFF) > 0xff)) { + ((reply->attribute & 0xFFFF) >= 256) && + !reply->flags.extended && !reply->flags.extended_flags) { #ifndef NDEBUG /* * Permit the admin to send BADLY formatted @@ -1273,7 +1314,7 @@ int rad_encode(RADIUS_PACKET *packet, const RADIUS_PACKET *original, */ if (reply->flags.encoded) continue; - if (reply->flags.is_tlv) { + if ((reply->vendor == VENDORPEC_WIMAX) && reply->flags.is_tlv) { len = rad_encode_wimax(packet, original, secret, reply, ptr, ((uint8_t *) data) + sizeof(data) - ptr); @@ -1285,6 +1326,16 @@ int rad_encode(RADIUS_PACKET *packet, const RADIUS_PACKET *original, if (len < 0) return -1; + /* + * Failed to encode the attribute, likely because + * the packet is full. + */ + if ((len == 0) && + (total_length > (sizeof(data) - 2 - reply->length))) { + DEBUG("WARNING: Attributes are too long for packet. Discarding data past %d bytes", total_length); + break; + } + next: ptr += len; total_length += len; @@ -1760,6 +1811,18 @@ int rad_packet_ok(RADIUS_PACKET *packet, int flags) while (count > 0) { /* + * We need at least 2 bytes to check the + * attribute header. + */ + if (count < 2) { + fr_strerror_printf("WARNING: 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))); + return 0; + } + + /* * Attribute number zero is NOT defined. */ if (attr[0] == 0) { @@ -1775,7 +1838,7 @@ int rad_packet_ok(RADIUS_PACKET *packet, int flags) * fields. Anything shorter is an invalid attribute. */ if (attr[1] < 2) { - fr_strerror_printf("WARNING: Malformed RADIUS packet from host %s: attribute %d too short", + fr_strerror_printf("WARNING: 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)), @@ -1784,6 +1847,19 @@ int rad_packet_ok(RADIUS_PACKET *packet, int flags) } /* + * If there are fewer bytes in the packet than in the + * 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", + inet_ntop(packet->src_ipaddr.af, + &packet->src_ipaddr.ipaddr, + host_ipaddr, sizeof(host_ipaddr)), + attr[0]); + return 0; + } + + /* * Sanity check the attributes for length. */ switch (attr[0]) { @@ -2115,7 +2191,7 @@ int rad_verify(RADIUS_PACKET *packet, RADIUS_PACKET *original, case PW_ACCOUNTING_REQUEST: if (calc_acctdigest(packet, secret) > 1) { fr_strerror_printf("Received %s packet " - "from %s with invalid signature! (Shared secret is incorrect.)", + "from client %s with invalid signature! (Shared secret is incorrect.)", fr_packet_codes[packet->code], inet_ntop(packet->src_ipaddr.af, &packet->src_ipaddr.ipaddr, @@ -2136,13 +2212,12 @@ 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 client %s port %d with invalid signature (err=%d)! (Shared secret is incorrect.)", + "from home server %s port %d with invalid signature! (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, - rcode); + packet->src_port); return -1; } break; @@ -2180,6 +2255,15 @@ static VALUE_PAIR *data2vp(const RADIUS_PACKET *packet, vp->next = NULL; /* + * It's supposed to be a fixed length, but we found + * a different length instead. Make it type "octets", + * and do no more processing on it. + */ + if ((vp->flags.length > 0) && (vp->flags.length != length)) { + goto raw; + } + + /* * Handle tags. */ if (vp->flags.has_tag) { @@ -2399,16 +2483,35 @@ static VALUE_PAIR *data2vp(const RADIUS_PACKET *packet, default: raw: - vp->type = PW_TYPE_OCTETS; - vp->length = length; - memcpy(vp->vp_octets, data, length); - - /* - * Ensure there's no encryption or tag stuff, - * we just pass the attribute as-is. + * Change the name to show the user that the + * attribute is not of the correct format. */ - memset(&vp->flags, 0, sizeof(vp->flags)); + { + int attr = vp->attribute; + int vendor = vp->vendor; + VALUE_PAIR *vp2; + + vp2 = pairalloc(NULL); + if (!vp2) { + pairfree(&vp); + return NULL; + } + pairfree(&vp); + vp = vp2; + + /* + * This sets "vp->flags" appropriately, + * and vp->type. + */ + if (!paircreate_raw(attr, vendor, PW_TYPE_OCTETS, vp)) { + return NULL; + } + + vp->length = length; + memcpy(vp->vp_octets, data, length); + } + break; } return vp; @@ -2537,6 +2640,7 @@ static uint8_t *rad_coalesce(unsigned int attribute, int vendor, return tlv_data; } + /* * Walk over Evil WIMAX TLVs, creating attributes. */ @@ -2544,14 +2648,14 @@ static VALUE_PAIR *tlv2wimax(const RADIUS_PACKET *packet, const RADIUS_PACKET *original, const char *secret, int attribute, int vendor, - uint8_t *ptr, size_t len, int shift) + uint8_t *ptr, size_t len, int nest) { VALUE_PAIR *head = NULL; VALUE_PAIR **tail = &head; VALUE_PAIR *vp; uint8_t *y; /* why do I need to do this? */ - if (shift > 24) return NULL; + if (nest > fr_wimax_max_tlv) return NULL; /* * Sanity check the attribute. @@ -2561,20 +2665,29 @@ static VALUE_PAIR *tlv2wimax(const RADIUS_PACKET *packet, (y[1] < 2) || ((y + y[1]) > (ptr + len))) { return NULL; } + + /* + * Attribute number is too large for us to + * represent it in our horrible internal + * representation. + */ + if ((ptr[0] & ~fr_wimax_mask[nest]) != 0) { + return NULL; + } } for (y = ptr; y < (ptr + len); y += y[1]) { DICT_ATTR *da; - da = dict_attrbyvalue(attribute | (ptr[0] << shift), vendor); + da = dict_attrbyvalue(attribute | (ptr[0] << fr_wimax_shift[nest]), vendor); if (da && (da->type == PW_TYPE_TLV)) { vp = tlv2wimax(packet, original, secret, - attribute | (ptr[0] << shift), + attribute | (ptr[0] << fr_wimax_shift[nest]), vendor, ptr + 2, ptr[1] - 2, - shift + 8); + nest + 1); if (!vp) goto error; } else { - vp = paircreate(attribute | (ptr[0] << shift), vendor, + vp = paircreate(attribute | (ptr[0] << fr_wimax_shift[nest]), vendor, PW_TYPE_OCTETS); if (!vp) { error: @@ -2685,15 +2798,15 @@ static VALUE_PAIR *rad_continuation2vp(const RADIUS_PACKET *packet, ptr != (tlv_data + tlv_length); ptr += ptr[1]) { - tlv_da = dict_attrbyvalue(attribute | (ptr[0] << 8), vendor); + tlv_da = dict_attrbyvalue(attribute | (ptr[0] << fr_wimax_shift[1]), vendor); if (tlv_da && (tlv_da->type == PW_TYPE_TLV)) { vp = tlv2wimax(packet, original, secret, attribute | (ptr[0] << 8), - vendor, ptr + 2, ptr[1] - 2, 16); + vendor, ptr + 2, ptr[1] - 2, 2); if (!vp) goto error; } else { - vp = paircreate(attribute | (ptr[0] << 8), vendor, + vp = paircreate(attribute | (ptr[0] << fr_wimax_shift[1]), vendor, PW_TYPE_OCTETS); if (!vp) { error: @@ -2724,6 +2837,31 @@ static VALUE_PAIR *rad_continuation2vp(const RADIUS_PACKET *packet, /* + * Extended attribute TLV to VP. + */ +static VALUE_PAIR *tlv2vp(const RADIUS_PACKET *packet, + const RADIUS_PACKET *original, + const char *secret, int attribute, + int length, const uint8_t *data) +{ + VALUE_PAIR *vp; + + if ((length < 2) || (data[1] < 2)) return NULL; + + /* + * For now, only one TLV is allowed. + */ + if (data[1] != length) return NULL; + + attribute |= (data[0] << fr_wimax_shift[2]); + + vp = paircreate(attribute, VENDORPEC_EXTENDED, PW_TYPE_OCTETS); + if (!vp) return NULL; + + return data2vp(packet, original, secret, length - 2, data + 2, vp); +} + +/* * Parse a RADIUS attribute into a data structure. */ VALUE_PAIR *rad_attr2vp(const RADIUS_PACKET *packet, @@ -2733,6 +2871,58 @@ VALUE_PAIR *rad_attr2vp(const RADIUS_PACKET *packet, { VALUE_PAIR *vp; + /* + * Hard-coded values are bad... + */ + if ((vendor == 0) && (attribute >= 241) && (attribute <= 246)) { + DICT_ATTR *da; + + da = dict_attrbyvalue(attribute, VENDORPEC_EXTENDED); + if (da) { /* flags.extended MUST be set */ + + /* + * MUST have at least an "extended type" octet. + */ + if (length == 0) return NULL; + + attribute |= (data[0] << fr_wimax_shift[1]); + vendor = VENDORPEC_EXTENDED; + + data++; + length--; + + /* + * There may be a flag octet. + */ + if (da->flags.extended_flags) { + if (length == 0) return NULL; + + /* + * If there's a flag, we can't + * handle it. + */ + if (data[0] != 0) return NULL; + data++; + length--; + } + + /* + * Now look up the extended attribute, to + * see if it's a TLV carrying more data. + * + */ + da = dict_attrbyvalue(attribute, VENDORPEC_EXTENDED); + if (da && da->flags.has_tlv) { + return tlv2vp(packet, original, secret, + attribute, length, data); + } + } + + /* + * We could avoid another dictionary lookup here + * by using pairalloc(da), but it's not serious... + */ + } vp = paircreate(attribute, vendor, PW_TYPE_OCTETS); if (!vp) return NULL; @@ -2842,13 +3032,10 @@ int rad_decode(RADIUS_PACKET *packet, RADIUS_PACKET *original, if (myvendor == 0) goto create_pair; /* - * This is an implementation issue. - * We currently pack vendor into the upper - * 16 bits of a 32-bit attribute number, - * so we can't handle vendor numbers larger - * than 16 bits. + * Allow vendors up to 2^24. Past that, + * get confused. */ - if (myvendor > 65535) goto create_pair; + if (myvendor > FR_MAX_VENDOR) goto create_pair; vsa_tlen = vsa_llen = 1; vsa_offset = 0; @@ -3515,7 +3702,7 @@ void fr_rand_seed(const void *data, size_t size) size_t total; ssize_t this; - total = this = 0; + total = 0; while (total < sizeof(fr_rand_pool.randrsl)) { this = read(fd, fr_rand_pool.randrsl, sizeof(fr_rand_pool.randrsl) - total);