X-Git-Url: http://www.project-moonshot.org/gitweb/?a=blobdiff_plain;f=src%2Flib%2Fradius.c;h=359756a2108c15a7de309c22d61240311aefd6c0;hb=9960563934a7da222528a1d82224aecc207c8aa8;hp=3341fb6cc7b96472192e5cb7198a7347fb06078e;hpb=2bb1db205a0136d16882208063c8e0e670f34a6f;p=freeradius.git

diff --git a/src/lib/radius.c b/src/lib/radius.c
index 3341fb6..359756a 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,
@@ -626,6 +626,7 @@ static void make_tunnel_passwd(uint8_t *output, size_t *outlen,
 		}
 
 		fr_MD5Final(digest, &context);
+
 		for (i = 0; i < AUTH_PASS_LEN; i++) {
 			passwd[i + 2 + n] ^= digest[i];
 		}
@@ -636,14 +637,14 @@ static void make_tunnel_passwd(uint8_t *output, size_t *outlen,
 /*
  *	Returns the end of the data.
  */
-static uint8_t *vp2data(const RADIUS_PACKET *packet,
-			const RADIUS_PACKET *original,
-			const char *secret, const VALUE_PAIR *vp, uint8_t *ptr,
-			size_t room)
+static int vp2data(const RADIUS_PACKET *packet, const RADIUS_PACKET *original,
+		   const char *secret, const VALUE_PAIR *vp, uint8_t *start,
+		   size_t room)
 {
 	uint32_t lvalue;
 	size_t len;
 	const uint8_t *data;
+	uint8_t *ptr = start;
 	uint8_t	array[4];
 
 	/*
@@ -710,14 +711,14 @@ static uint8_t *vp2data(const RADIUS_PACKET *packet,
 		data = vp->vp_tlv;
 		if (!data) {
 			fr_strerror_printf("ERROR: Cannot encode NULL TLV");
-			return NULL;
+			return -1;
 		}
 		if (vp->length > room) return 0; /* can't chop TLVs to fit */
 		break;
 
 	default:		/* unknown type: ignore it */
 		fr_strerror_printf("ERROR: Unknown attribute type %d", vp->type);
-		return NULL;
+		return -1;
 	}
 
 	/*
@@ -738,6 +739,9 @@ static uint8_t *vp2data(const RADIUS_PACKET *packet,
 		break;
 
 	case FLAG_ENCRYPT_TUNNEL_PASSWORD:
+		lvalue = 0;
+		if (vp->flags.has_tag) lvalue = 1;
+
 		/*
 		 *	Check if there's enough room.  If there isn't,
 		 *	we discard the attribute.
@@ -745,7 +749,7 @@ static uint8_t *vp2data(const RADIUS_PACKET *packet,
 		 *	This is ONLY a problem if we have multiple VSA's
 		 *	in one Vendor-Specific, though.
 		 */
-		if (room < 18) return ptr;
+		if (room < (18 + lvalue)) return 0;
 
         	switch (packet->code) {
 	        case PW_AUTHENTICATION_ACK:
@@ -754,15 +758,19 @@ static uint8_t *vp2data(const RADIUS_PACKET *packet,
         	default:
 			if (!original) {
 				fr_strerror_printf("ERROR: No request packet, cannot encrypt %s attribute in the vp.", vp->name);
-				return NULL;
+				return -1;
 			}
-			make_tunnel_passwd(ptr, &len, data, len, room,
+
+			if (lvalue) ptr[0] = vp->flags.tag;
+			make_tunnel_passwd(ptr + lvalue, &len, data, len,
+					   room - lvalue,
 					   secret, original->vector);
                 	break;
 	        case PW_ACCOUNTING_REQUEST:
         	case PW_DISCONNECT_REQUEST:
 	        case PW_COA_REQUEST:
-			make_tunnel_passwd(ptr, &len, data, len, room,
+			ptr[0] = vp->flags.tag;
+			make_tunnel_passwd(ptr + 1, &len, data, len - 1, room,
 					   secret, packet->vector);
 	                break;
         	}
@@ -779,16 +787,186 @@ static uint8_t *vp2data(const RADIUS_PACKET *packet,
 
 
 	default:
-		/*
-		 *	Just copy the data over
-		 */
+		if (vp->flags.has_tag && TAG_VALID(vp->flags.tag)) {
+			if (vp->type == PW_TYPE_STRING) {
+				if (len > (room - 1)) len = room - 1;
+				ptr[0] = vp->flags.tag;
+				ptr++;
+			} else if (vp->type == PW_TYPE_INTEGER) {
+				array[0] = vp->flags.tag;
+			} /* else it can't be any other type */
+		}
 		memcpy(ptr, data, len);
 		break;
 	} /* switch over encryption flags */
 
-	return ptr + len;
+	return len + (ptr - start);;
+}
+
+
+static int rad_vp2rfc(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;
+
+	if (room < 2) return 0;
+
+	ptr[0] = attribute & 0xff; /* NOT vp->attribute */
+	ptr[1] = 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;
+
+	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,
+		    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] = (vp->attribute >> fr_wimax_shift[nest]) & fr_wimax_mask[nest];
+	ptr[1] = 2;
+
+	/*
+	 *	No more nested TLVs: pack the data.
+	 */
+	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, ptr + 2, room,
+			       nest + 1);
+	}
+	if (len <= 0) return len;
+
+	ptr[1] += len;
+
+	return ptr[1];
+}
+
+static int wimax2data(const RADIUS_PACKET *packet,
+		      const RADIUS_PACKET *original,
+		      const char *secret, const VALUE_PAIR *vp,
+		      uint8_t *start, size_t room, uint8_t *ptr)
+{
+	int len;
+
+	/*
+	 *	Offsets to Vendor-Specific length, and to length of
+	 *	WiMAX attribute.
+	 */
+#define VS_OFF (1)
+#define WM_OFF (7)
+
+	if (room < 1) return 0;
+	room--;
+
+	/*
+	 *	Account for continuation bytes.  The caller has
+	 *	already accounting for the continuation byte in the
+	 *	Vendor-Specific "length" field.
+	 */
+	start[WM_OFF]++;
+	*(ptr++) = 0;
+
+	/*
+	 *	Chop everything to fit in one attribute.
+	 */
+	if (room > (255 - 9)) room = (255 - 9);
+
+	/*
+	 *	The attribute contains TLVs that we have NOT decoded
+	 *	properly, OR it contains TLV that the user has encoded
+	 *	manually.  If it has no data, OR it's too long,
+	 *	discard it.  We're not going to walk through its
+	 *	contents trying to figure out how to chop it across
+	 *	multiple continuations.
+	 */
+	if (vp->flags.has_tlv && (!vp->vp_tlv || (vp->length > room))) {
+		return 0;
+	}
+
+	/*
+	 *	The attribute is a top-level integer, ipaddr, etc.
+	 *	Encode it.
+	 */
+	if (!vp->flags.is_tlv) {
+		len = vp2data(packet, original, secret, vp, ptr, room);
+	} else {
+		len = tlv2data(packet, original, secret, vp, ptr, room, 1);
+	}
+
+	if (len <= 0) return len;
+
+	start[VS_OFF] += len;
+	start[WM_OFF] += len;
+
+	return start[VS_OFF];
+}
+
+
+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;
+		len++;
+
+	} else if (vp->flags.extended_flags) {
+		if (room < 4) return 0;
+
+		ptr[1] = 4;
+		ptr[2] = (attribute & 0xff00) >> 8;
+		ptr[3] = 0;
+
+		len += 2;
+	}
+
+	/*
+	 *	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.
@@ -797,62 +975,61 @@ int rad_vp2attr(const RADIUS_PACKET *packet, const RADIUS_PACKET *original,
 		const char *secret, const VALUE_PAIR *vp, uint8_t *start,
 		size_t room)
 {
-	int		vendorcode;
-	int		len, total_length;
-	uint32_t	lvalue;
-	uint8_t		*ptr, *length_ptr, *vsa_length_ptr, *tlv_length_ptr;
-	uint8_t		*end, *sub_length_ptr; /* evil */
-
-	ptr = start;
-	vendorcode = total_length = 0;
-	length_ptr = vsa_length_ptr = tlv_length_ptr = sub_length_ptr = NULL;
+	int len;
+	uint32_t lvalue;
+	uint8_t *ptr;
+	DICT_VENDOR *dv;
 
 	/*
-	 *	For interoperability, always put vendor attributes
-	 *	into their own VSA.
+	 *	RFC format attributes take the fast path.
 	 */
-	if ((vendorcode = vp->vendor) == 0) {
-		if (room < 2) return 0;
-		room -= 2;
+	if (vp->vendor == 0) {
+		return rad_vp2rfc(packet, original, secret, vp,
+				  vp->attribute, start, room);
+	}
 
-		*(ptr++) = vp->attribute & 0xff;
-		length_ptr = ptr;
-		*(ptr++) = 2;
-		total_length += 2;
+	if (vp->vendor == VENDORPEC_EXTENDED) {
+		return rad_vp2extended(packet, original, secret, vp,
+				       vp->attribute, start, room);
+	}
 
-	} else {
-		int vsa_tlen = 1;
-		int vsa_llen = 1;
-		int vsa_offset = 0;
-		DICT_VENDOR *dv = dict_vendorbyvalue(vendorcode);
+	/*
+	 *	Not enough room for:
+	 *		attr, len, vendor-id, vsa, vsalen
+	 */
+	if (room < 8) return 0;
 
-		/*
-		 *	This must be an RFC-format attribute.  If it
-		 *	wasn't, then the "decode" function would have
-		 *	made a Vendor-Specific attribute (i.e. type
-		 *	26), and we would have "vendorcode == 0" here.
-		 */
-		if (dv) {
-			vsa_tlen = dv->type;
-			vsa_llen = dv->length;
-			if (dv->flags) vsa_offset = 1;
-		}
+	/*
+	 *	Build the Vendor-Specific header
+	 */
+	ptr = start;
+	*ptr++ = PW_VENDOR_SPECIFIC;
+	*ptr++ = 6;
+	room -= 6;
+	lvalue = htonl(vp->vendor);
+	memcpy(ptr, &lvalue, 4);
+	ptr += 4;
 
-		if (room < (6 + vsa_tlen + vsa_llen + vsa_offset)) return 0;
-		room -= 6 + vsa_tlen + vsa_llen + vsa_offset;
+	/*
+	 *	Unknown vendors, and type=1,length=1,no-continuation
+	 *	are RFC format attributes.
+	 */
+	dv = dict_vendorbyvalue(vp->vendor);
+	if (!dv ||
+	    ((dv->type == 1) && (dv->length = 1) && !dv->flags)) {
+		len = rad_vp2rfc(packet, original, secret, vp,
+				 vp->attribute, ptr, room);
+		if (len <= 0) return len;
 
-		/*
-		 *	Build a VSA header.
-		 */
-		*ptr++ = PW_VENDOR_SPECIFIC;
-		vsa_length_ptr = ptr;
-		*ptr++ = 6;
-		lvalue = htonl(vendorcode);
-		memcpy(ptr, &lvalue, 4);
-		ptr += 4;
-		total_length += 6;
+		start[1] += len;
+		return start[1];
+	}
 
-		switch (vsa_tlen) {
+	if (room < (dv->type + dv->length + dv->flags)) return 0;
+	room -= (dv->type + dv->length + dv->flags);
+	start[1] += (dv->type + dv->length + dv->flags);
+
+	switch (dv->type) {
 		case 1:
 			ptr[0] = (vp->attribute & 0xFF);
 			break;
@@ -864,156 +1041,67 @@ int rad_vp2attr(const RADIUS_PACKET *packet, const RADIUS_PACKET *original,
 
 		case 4:
 			ptr[0] = 0;
-			ptr[1] = 0;
+			ptr[1] = ((vp->attribute >> 16) & 0xFF);
 			ptr[2] = ((vp->attribute >> 8) & 0xFF);
 			ptr[3] = (vp->attribute & 0xFF);
 			break;
 
 		default:
 			return 0; /* silently discard it */
-		}
-		ptr += vsa_tlen;
-
-		switch (vsa_llen) {
-		case 0:
-			length_ptr = vsa_length_ptr;
-			vsa_length_ptr = NULL;
-			break;
-		case 1:
-			ptr[0] = 0;
-			length_ptr = ptr;
-			break;
-		case 2:
-			ptr[0] = 0;
-			ptr[1] = 0;
-			length_ptr = ptr + 1;
-			break;
-
-		default:
-			return 0; /* silently discard it */
-		}
-		ptr += vsa_llen;
-
-		/*
-		 *	Allow for some continuation.
-		 */
-		if (vsa_offset) {
-			ptr[0] = 0x00;
-			ptr++;
-
-			/*
-			 *	Ignore TLVs that don't have data, OR
-			 *	have too much data to fit in the
-			 *	packet, OR have too much data to fit
-			 *	in the attribute
-			 *
-			 *	This shouldn't happen in normal
-			 *	operation, as the code assumes that
-			 *	the "tlv" type shouldn't be used.
-			 */
-			if (vp->flags.has_tlv &&
-			    (!vp->vp_tlv || (vp->length > room) ||
-
-			     /*
-			      *		6 + 1 (vsa_tlen) + 1 (vsa_llen)
-			      *		+ 1 (vsa_offset).
-			      */
-			     (vp->length > (255 - 9)))) return 0;
-
-
-			/*
-			 *	sub-TLV's can only be in one format.
-			 */
-			if (vp->flags.is_tlv) {
-				if (room < 2) return 0;
-				room -= 2;
-
-				*(ptr++) = (vp->attribute & 0xff00) >> 8;
-				tlv_length_ptr = ptr;
-				*(ptr++) = 2;
-				vsa_offset += 2;
-
-				/*
-				 *	WiMAX is like sticking knitting
-				 *	needles up your nose, and claiming
-				 *	you like it.
-				 */
-				if ((vp->attribute & 0xff0000) != 0) {
-					*(ptr++) = (vp->attribute >> 16) & 0xff;
-					sub_length_ptr = ptr;
-					*(ptr++) = 2;
-					vsa_offset += 2;
-					*tlv_length_ptr += 2;
-				}
-			}
-		}
-
-		total_length += vsa_tlen + vsa_llen + vsa_offset;
-		if (vsa_length_ptr) *vsa_length_ptr += vsa_tlen + vsa_llen + vsa_offset;
-		*length_ptr += vsa_tlen + vsa_llen + vsa_offset;
 	}
+	ptr += dv->type;
 
-	/*
-	 *	Insert tags for string attributes.  They go BEFORE
-	 *	the string.
-	 */
-	if (vp->flags.has_tag && (vp->type == PW_TYPE_STRING) &&
-	    (TAG_VALID(vp->flags.tag) ||
-	     (vp->flags.encrypt == FLAG_ENCRYPT_TUNNEL_PASSWORD))) {
-		if (room < (1 + vp->length)) return 0;
+	switch (dv->length) {
+	case 0:
+		break;
+	case 1:
+		ptr[0] = dv->type + 1;
+		break;
+	case 2:
+		ptr[0] = 0;
+		ptr[1] = dv->type + 2;
+		break;
 
-		ptr[0] = vp->flags.tag;
-		end = vp2data(packet, original, secret, vp, ptr + 1,
-			      (end - ptr) - 1);
-	} else {
-		if (room < vp->length) return 0;
-		end = vp2data(packet, original, secret, vp, ptr,
-			      (end - ptr));
+	default:
+		return 0; /* silently discard it */
 	}
-	if (!end) return -1;
+	ptr += dv->length;
 
 	/*
-	 *	Insert tags for integer attributes.  They go at the START
-	 *	of the integer, and over-write the first byte.
+	 *	WiMAX attributes take their own path through the
+	 *	system.
 	 */
-	if (vp->flags.has_tag && (vp->type == PW_TYPE_INTEGER)) {
-		ptr[0] = vp->flags.tag;
-	}
+	if (dv->flags) return wimax2data(packet, original, secret, vp,
+					 start, room, ptr);
 
-	/*
-	 *	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 ((end == ptr) &&
-	    (vp->attribute != PW_CHARGEABLE_USER_IDENTITY)) return 0;
+	len = vp2data(packet, original, secret, vp, ptr, room);
+	if (len <= 0) return len;
 
-	len = (end - ptr);
+	if (dv->length != 0) ptr[-1] += len;
 
-	/*
-	 *	Update the various lengths.
-	 */
-	*length_ptr += len;
-	if (vsa_length_ptr) *vsa_length_ptr += len;
-	if (tlv_length_ptr) *tlv_length_ptr += len;
-	if (sub_length_ptr) *sub_length_ptr += len;
-	ptr += len;
-	total_length += len;
+	start[1] += len;
 
-	return total_length;	/* of attribute */
+	return start[1];
 }
 
+/*
+ *  Swap 123a -> 0321
+ */
+#define REORDER(x) ((x & 0xff00) << 8) | ((x & 0xff0000) >> 8) | ((x & 0xff000000 >> 24))
+
+
+/*
+ *	Encode a WiMAX sub-TLV.  It must NOT be called for WiMAX
+ *	attributes that are of type integer, string, etc.
+ */
 static int rad_encode_wimax(const RADIUS_PACKET *packet,
 			    const RADIUS_PACKET *original,
 			    const char *secret, VALUE_PAIR *reply,
 			    uint8_t *start, size_t room)
 {
-	int len, total_len = 0;
-	uint8_t *wimax = NULL;
-	uint8_t *ptr = start;
+	int len, redo;
+	uint32_t lvalue;
+	uint8_t *ptr = start, *vsa = start;
 	uint32_t maxattr;
 	VALUE_PAIR *vp = reply;
 
@@ -1021,42 +1109,48 @@ static int rad_encode_wimax(const RADIUS_PACKET *packet,
 	 *	Swap the order of the WiMAX hacks, to make later
 	 *	comparisons easier.
 	 */
-	maxattr = (vp->attribute & 0xff00) | ((vp->attribute >> 16) & 0xff);
-
-redo:
-	len = rad_vp2attr(packet, original, secret, vp, ptr,
-			  (start + room) - ptr);
-	if (len <= 0) return total_len;
+	maxattr = REORDER(vp->attribute);
 
 	/*
-	 *	After adding an attribute with the simplest encoding,
-	 *	check to see if we can append it to the previous one.
+	 *	Build the Vendor-Specific header
 	 */
-	if (wimax) {
-		if ((wimax[1] + (ptr[1] - 6)) <= 255) {
-			unsigned int hack;
+	ptr = start;
+	redo = 0;
 
-			hack = ptr[7] - 3;
-			memmove(ptr, ptr + 9, hack);
-			wimax[1] += hack;
-			wimax[7] += hack;
-			len -= 9;
+redo_vsa:
+	vsa = ptr;
 
-			/*
-			 *	See if we can nest sub-TLVs, too, in
-			 *	order to shorten the encoding.
-			 */
+	if (room < 9) return 0;
+	*ptr++ = PW_VENDOR_SPECIFIC;
+	*ptr++ = 9;
+	room -= 9;
+	lvalue = htonl(vp->vendor);
+	memcpy(ptr, &lvalue, 4);
+	ptr += 4;
+	*(ptr++) = vp->attribute & 0xff;
+	*(ptr++) = 3;
+	*(ptr++) = 0;		/* continuation */
+	room -= 9;
 
-		} else {
-			wimax[8] = 0x80; /* set continuation */
-			wimax = ptr;
-		}
-	} else {
-		wimax = ptr;
+redo_tlv:
+	len = tlv2data(packet, original, secret, vp, ptr, room, 1);
+	if (len < 0) return len;
+
+	/*
+	 *	Not enough room.  Do a continuation.
+	 */
+	if ((len == 0) || ((vsa[VS_OFF] + len) > 255)) {
+		if (redo) return (start - vsa);
+
+		vsa[8] = 0x80;
+		redo = 1;
+		goto redo_vsa;
 	}
+	redo = 0;
 
-	total_len += len;
 	ptr += len;
+	vsa[VS_OFF] += len;
+	vsa[WM_OFF] += len;
 
 	vp->flags.encoded = 1;
 	vp = vp->next;
@@ -1071,14 +1165,14 @@ redo:
 	    ((reply->attribute & 0xff) == (vp->attribute & 0xff))) {
 		uint32_t attr;
 
-		attr = (vp->attribute & 0xff00) | ((vp->attribute >> 16) & 0xff);
+		attr = REORDER(vp->attribute);
 		if (attr >= maxattr) {
 			maxattr = attr;
-			goto redo;
+			goto redo_tlv;
 		}
 	}
 
-	return total_len;
+	return ptr - start;
 }
 
 
@@ -1176,10 +1270,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
@@ -1221,7 +1317,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);
@@ -1233,6 +1329,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;
@@ -1708,6 +1814,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) {
@@ -1723,7 +1841,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)),
@@ -1732,6 +1850,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]) {
@@ -2063,7 +2194,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,
@@ -2084,13 +2215,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;
@@ -2128,6 +2258,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) {
@@ -2140,7 +2279,10 @@ static VALUE_PAIR *data2vp(const RADIUS_PACKET *packet,
 			vp->flags.tag = data[0];
 
 			if ((vp->type == PW_TYPE_STRING) ||
-			    (vp->type == PW_TYPE_OCTETS)) offset = 1;
+			    (vp->type == PW_TYPE_OCTETS)) {
+				if (length == 0) goto raw;
+				offset = 1;
+			}
 		}
 	}
 
@@ -2344,16 +2486,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;
@@ -2482,22 +2643,23 @@ static uint8_t *rad_coalesce(unsigned int attribute, int vendor,
 	return tlv_data;
 }
 
+
 /*
- *	Walk over Evil WIMAX Hell, creating attributes.
- *
- *	Won't someone think of the children?  What if they read this code?
+ *	Walk over Evil WIMAX TLVs, creating attributes.
  */
-static VALUE_PAIR *recurse_evil(const RADIUS_PACKET *packet,
-				const RADIUS_PACKET *original,
-				const char *secret,
-				int attribute, int vendor,
-				uint8_t *ptr, size_t len)
+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 nest)
 {
 	VALUE_PAIR *head = NULL;
 	VALUE_PAIR **tail = &head;
 	VALUE_PAIR *vp;
 	uint8_t *y;		/* why do I need to do this? */
 
+	if (nest > fr_wimax_max_tlv) return NULL;
+
 	/*
 	 *	Sanity check the attribute.
 	 */
@@ -2506,24 +2668,44 @@ static VALUE_PAIR *recurse_evil(const RADIUS_PACKET *packet,
 		    (y[1] < 2) || ((y + y[1]) > (ptr + len))) {
 			return NULL;
 		}
-	}
 
-	for (y = ptr; y < (ptr + len); y += y[1]) {
-		vp = paircreate(attribute | (ptr[0] << 16), vendor,
-				PW_TYPE_OCTETS);
-		if (!vp) {
-		error:
-			pairfree(&head);
+		/*
+		 *	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;
 		}
+	}
 
-		if (!data2vp(packet, original, secret,
-			     y[1] - 2, y + 2, vp)) {
-			goto error;
+	for (y = ptr; y < (ptr + len); y += y[1]) {
+		DICT_ATTR *da;
+
+		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] << fr_wimax_shift[nest]),
+				       vendor, ptr + 2, ptr[1] - 2,
+				       nest + 1);
+			if (!vp) goto error;
+		} else {
+			vp = paircreate(attribute | (ptr[0] << fr_wimax_shift[nest]), vendor,
+					PW_TYPE_OCTETS);
+			if (!vp) {
+			error:
+				pairfree(&head);
+				return NULL;
+			}
+
+			if (!data2vp(packet, original, secret,
+				     y[1] - 2, y + 2, vp)) {
+				goto error;
+			}
 		}
 
 		*tail = vp;
-		tail = &(vp->next);
+		while (*tail) tail = &((*tail)->next);
 	}
 
 	return head;
@@ -2619,28 +2801,25 @@ 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 = recurse_evil(packet, original, secret,
-					  attribute | (ptr[0] << 8),
-					  vendor, ptr + 2, ptr[1] - 2);
+			vp = tlv2wimax(packet, original, secret,
+				       attribute | (ptr[0] << 8),
+				       vendor, ptr + 2, ptr[1] - 2, 2);
 
-			if (!vp) {
-				pairfree(&head);
-				goto not_well_formed;
-			}
+			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:
 				pairfree(&head);
 				goto not_well_formed;
 			}
 
 			if (!data2vp(packet, original, secret,
 				     ptr[1] - 2, ptr + 2, vp)) {
-				pairfree(&head);
-				goto not_well_formed;
+				goto error;
 			}
 		}
 
@@ -2661,6 +2840,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,
@@ -2670,6 +2874,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;
 
@@ -2779,13 +3035,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;