From: Alan T. DeKok Date: Sat, 21 Jul 2012 00:49:33 +0000 (-0400) Subject: Manually pull fixes from v2.1.x X-Git-Tag: release_3_0_0_beta0~120 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8e44869023dfd4124b69c71e2b46bf14c971825b;p=thirdparty%2Ffreeradius-server.git Manually pull fixes from v2.1.x 41bb275514c30318a6d83d34c16dc1a940418e1e 75d10dab714aa5c5c99f18b4e8d768b671771cc4 39e04c5bdd77503f09d4fdcea8f5b52d4bc69683 --- diff --git a/src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c b/src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c index 94e428b536f..65d23174997 100644 --- a/src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c +++ b/src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c @@ -48,65 +48,31 @@ static int diameter_verify(REQUEST *request, { uint32_t attr; uint32_t length; - unsigned int offset; + unsigned int hdr_len; unsigned int data_left = data_len; while (data_left > 0) { + hdr_len = 12; + if (data_len < 12) { RDEBUG2(" Diameter attribute is too small to contain a Diameter header"); return 0; } - rad_assert(data_left <= data_len); memcpy(&attr, data, sizeof(attr)); - data += 4; attr = ntohl(attr); - if (attr > 255) { - RDEBUG2(" Non-RADIUS attribute in tunneled authentication is not supported"); - return 0; - } - memcpy(&length, data , sizeof(length)); - data += 4; length = ntohl(length); - /* - * A "vendor" flag, with a vendor ID of zero, - * is equivalent to no vendor. This is stupid. - */ - offset = 8; - if ((length & (1 << 31)) != 0) { - uint32_t vendor; - DICT_ATTR *da; - - memcpy(&vendor, data, sizeof(vendor)); - vendor = ntohl(vendor); - - if (vendor > FR_MAX_VENDOR) { - RDEBUG2("Vendor codes larger than 2^24 are not supported"); - return 0; - } - - da = dict_attrbyvalue(attr, vendor); - - /* - * SHOULD check ((length & (1 << 30)) != 0) - * for the mandatory bit. - */ - if (!da) { - RDEBUG2("Fatal! Vendor %u, Attribute %u was not found in our dictionary. ", - vendor, attr); + if ((data[4] & 0x80) != 0) { + if (data_len < 16) { + RDEBUG2(" Diameter attribute is too small to contain a Diameter header with Vendor-Id"); return 0; } - data += 4; /* skip the vendor field */ - offset += 4; /* offset to value field */ + hdr_len = 16; } - /* - * Ignore the M bit. We support all RADIUS attributes... - */ - /* * Get the length. If it's too big, die. */ @@ -115,23 +81,13 @@ static int diameter_verify(REQUEST *request, /* * Too short or too long is bad. */ - if (length < offset) { - RDEBUG2("Tunneled attribute %d is too short (%d)to contain anything useful.", attr, length); - return 0; - } - - /* - * EAP Messages cane be longer than MAX_STRING_LEN. - * Other attributes cannot be. - */ - if ((attr != PW_EAP_MESSAGE) && - (length > (MAX_STRING_LEN + 8))) { - RDEBUG2("Tunneled attribute %d is too long (%d) to pack into a RADIUS attribute.", attr, length); + if (length <= (hdr_len - 4)) { + RDEBUG2("Tunneled attribute %u is too short (%u < %u) to contain anything useful.", attr, length, hdr_len); return 0; } if (length > data_left) { - RDEBUG2("Tunneled attribute %d is longer than room left in the packet (%d > %d).", attr, length, data_left); + RDEBUG2("Tunneled attribute %u is longer than room left in the packet (%u > %u).", attr, length, data_left); return 0; } @@ -171,7 +127,7 @@ static int diameter_verify(REQUEST *request, * data_left > length, continue. */ data_left -= length; - data += length - offset; + data += length; } /* @@ -217,24 +173,11 @@ static VALUE_PAIR *diameter2vp(REQUEST *request, SSL *ssl, memcpy(&vendor, data, sizeof(vendor)); vendor = ntohl(vendor); - if (vendor > FR_MAX_VENDOR) { - RDEBUG2("Cannot handle vendor Id greater than 2^&24"); - pairfree(&first); - return NULL; - } - data += 4; /* skip the vendor field, it's zero */ offset += 4; /* offset to value field */ - } - /* - * Vendor attributes can be larger than 255. - * Normal attributes cannot be. - */ - if ((attr > 255) && (vendor == 0)) { - RDEBUG2("Cannot handle Diameter attributes"); - pairfree(&first); - return NULL; + if (attr > 65535) goto next_attr; + if (vendor > FR_MAX_VENDOR) goto next_attr; } /* @@ -255,7 +198,22 @@ static VALUE_PAIR *diameter2vp(REQUEST *request, SSL *ssl, size = length - offset; /* - * Create it. + * Vendor attributes can be larger than 255. + * Normal attributes cannot be. + */ + if ((attr > 255) && (vendor == 0)) { + RDEBUG2("WARNING: Skipping Diameter attribute %u", + attr); + goto next_attr; + } + + if (size > 253) { + RDEBUG2("WARNING: diameter2vp skipping long attribute %u, attr"); + goto next_attr; + } + + /* + * Create it. If this fails, it's because we're OOM. */ vp = paircreate(attr, vendor, PW_TYPE_OCTETS); if (!vp) { @@ -272,11 +230,16 @@ static VALUE_PAIR *diameter2vp(REQUEST *request, SSL *ssl, case PW_TYPE_INTEGER: case PW_TYPE_DATE: if (size != vp->length) { - RDEBUG2("Invalid length attribute %d", - attr); - pairfree(&first); - pairfree(&vp); - return NULL; + /* + * Bad format. Create a "raw" + * attribute. + */ + raw: + vp = paircreate_raw(vp->attribute, vp->vendor, + PW_TYPE_OCTETS, vp); + vp->length = size; + memcpy(vp->vp_octets, data, vp->length); + break; } memcpy(&vp->vp_integer, data, vp->length); @@ -287,13 +250,7 @@ static VALUE_PAIR *diameter2vp(REQUEST *request, SSL *ssl, break; case PW_TYPE_INTEGER64: - if (size != vp->length) { - RDEBUG2("Invalid length attribute %d", - attr); - pairfree(&first); - pairfree(&vp); - return NULL; - } + if (size != vp->length) goto raw; memcpy(&vp->vp_integer64, data, vp->length); /* @@ -317,13 +274,36 @@ static VALUE_PAIR *diameter2vp(REQUEST *request, SSL *ssl, */ break; - /* - * String, octet, etc. Copy the data from the - * value field over verbatim. - * - * FIXME: Ipv6 attributes ? - * - */ + case PW_TYPE_BYTE: + if (size != vp->length) goto raw; + vp->vp_integer = data[0]; + break; + + case PW_TYPE_SHORT: + if (size != vp->length) goto raw; + vp->vp_integer = (data[0] * 256) + data[1]; + break; + + case PW_TYPE_SIGNED: + if (size != vp->length) goto raw; + memcpy(&vp->vp_signed, data, vp->length); + vp->vp_signed = ntohl(vp->vp_signed); + break; + + case PW_TYPE_IPV6ADDR: + if (size != vp->length) goto raw; + memcpy(&vp->vp_ipv6addr, data, vp->length); + break; + + case PW_TYPE_IPV6PREFIX: + if (size != vp->length) goto raw; + memcpy(&vp->vp_ipv6prefix, data, vp->length); + break; + + /* + * String, octet, etc. Copy the data from the + * value field over verbatim. + */ case PW_TYPE_OCTETS: if (attr == PW_EAP_MESSAGE) { const uint8_t *eap_message = data; @@ -371,66 +351,60 @@ static VALUE_PAIR *diameter2vp(REQUEST *request, SSL *ssl, * NOTE: This means that the User-Password * attribute CANNOT EVER have embedded zeros in it! */ - switch (vp->attribute) { - case PW_USER_PASSWORD: + if ((vp->vendor == 0) && (vp->attribute == PW_USER_PASSWORD)) { /* * If the password is exactly 16 octets, * it won't be zero-terminated. */ vp->vp_strvalue[vp->length] = '\0'; vp->length = strlen(vp->vp_strvalue); - break; + } + + /* + * Ensure that the client is using the + * correct challenge. This weirdness is + * to protect against against replay + * attacks, where anyone observing the + * CHAP exchange could pose as that user, + * by simply choosing to use the same + * challenge. + * + * By using a challenge based on + * information from the current session, + * we can guarantee that the client is + * not *choosing* a challenge. + * + * We're a little forgiving in that we + * have loose checks on the length, and + * we do NOT check the Id (first octet of + * the response to the challenge) + * + * But if the client gets the challenge correct, + * we're not too worried about the Id. + */ + if (((vp->vendor == 0) && (vp->attribute == PW_CHAP_CHALLENGE)) || + ((vp->vendor == VENDORPEC_MICROSOFT) && (vp->attribute == PW_MSCHAP_CHALLENGE))) { + uint8_t challenge[16]; - /* - * Ensure that the client is using the - * correct challenge. This weirdness is - * to protect against against replay - * attacks, where anyone observing the - * CHAP exchange could pose as that user, - * by simply choosing to use the same - * challenge. - * - * By using a challenge based on - * information from the current session, - * we can guarantee that the client is - * not *choosing* a challenge. - * - * We're a little forgiving in that we - * have loose checks on the length, and - * we do NOT check the Id (first octet of - * the response to the challenge) - * - * But if the client gets the challenge correct, - * we're not too worried about the Id. - */ - case PW_CHAP_CHALLENGE: - case PW_MSCHAP_CHALLENGE: if ((vp->length < 8) || (vp->length > 16)) { RDEBUG("Tunneled challenge has invalid length"); pairfree(&first); pairfree(&vp); return NULL; - - } else { - uint8_t challenge[16]; - - eapttls_gen_challenge(ssl, challenge, - sizeof(challenge)); - - if (memcmp(challenge, vp->vp_octets, - vp->length) != 0) { - RDEBUG("Tunneled challenge is incorrect"); - pairfree(&first); - pairfree(&vp); - return NULL; - } } - break; - default: - break; - } /* switch over checking/re-writing of attributes. */ + eapttls_gen_challenge(ssl, challenge, + sizeof(challenge)); + + if (memcmp(challenge, vp->vp_octets, + vp->length) != 0) { + RDEBUG("Tunneled challenge is incorrect"); + pairfree(&first); + pairfree(&vp); + return NULL; + } + } /* * Update the list.