From: Francois-Xavier Le Bail Date: Fri, 22 Sep 2023 19:43:25 +0000 (+0200) Subject: RADIUS: Modernize packet parsing X-Git-Url: https://git.tcpdump.org/tcpdump/commitdiff_plain/790427e0cf2d0a9fd1d0eb6ac714e2428b903c8c RADIUS: Modernize packet parsing Add a sanity check: The maximum length is 4096. Print the protocol name with nd_print_protocol_caps() before any check. Enable ND_LONGJMP_FROM_TCHECK and remove the 'trunc' labels. Report invalid packets as invalid. Use ND_ICHECK*() for length check. Remove some redundant ND_TCHECK_*(). Fix a length check for ARAP-Challenge-Response attribute (s/ 56] (invalid); follow-up to f64a4a5f49. Update RADIUS length (from 44006 to 263) in radius_attr_asan.pcap to pass the two new tests (length <= 4096 and length <= original length). Update some test outputs accordingly. Moreover: Remove some extra blank lines. --- diff --git a/netdissect.h b/netdissect.h index a6e298ba..1ad57c90 100644 --- a/netdissect.h +++ b/netdissect.h @@ -725,7 +725,7 @@ extern const char *q922_string(netdissect_options *, const u_char *, u_int); extern void q933_print(netdissect_options *, const u_char *, u_int); extern int quic_detect(netdissect_options *, const u_char *, const u_int); extern void quic_print(netdissect_options *, const u_char *); -extern void radius_print(netdissect_options *, const u_char *, u_int); +extern void radius_print(netdissect_options *, const u_char *, const u_int); extern void resp_print(netdissect_options *, const u_char *, u_int); extern void rip_print(netdissect_options *, const u_char *, u_int); extern void ripng_print(netdissect_options *, const u_char *, unsigned int); diff --git a/print-radius.c b/print-radius.c index d2062af6..76a9923a 100644 --- a/print-radius.c +++ b/print-radius.c @@ -92,13 +92,13 @@ #include "netdissect-ctype.h" +#define ND_LONGJMP_FROM_TCHECK #include "netdissect.h" #include "addrtoname.h" #include "extract.h" #include "oui.h" #include "ntp.h" - #define TAM_SIZE(x) (sizeof(x)/sizeof(x[0]) ) #define PRINT_HEX(bytes_len, ptr_data) \ @@ -109,7 +109,6 @@ bytes_len--; \ } - /* Radius packet codes */ /* https://www.iana.org/assignments/radius-types/radius-types.xhtml#radius-types-27 */ #define RADCMD_ACCESS_REQ 1 /* Access-Request */ @@ -201,7 +200,6 @@ static const struct tok rfc4675_tagged[] = { { 0, NULL} }; - static void print_attr_string(netdissect_options *, const u_char *, u_int, u_short ); static void print_attr_num(netdissect_options *, const u_char *, u_int, u_short ); static void print_vendor_attr(netdissect_options *, const u_char *, u_int, u_short ); @@ -217,7 +215,6 @@ static void print_attr_time(netdissect_options *, const u_char *, u_int, u_short static void print_attr_vector64(netdissect_options *, const u_char *, u_int, u_short); static void print_attr_strange(netdissect_options *, const u_char *, u_int, u_short); - struct radius_hdr { nd_uint8_t code; /* Radius packet code */ nd_uint8_t id; /* Radius packet id */ nd_uint16_t len; /* Radius total length */ @@ -230,7 +227,6 @@ struct radius_attr { nd_uint8_t type; /* Attribute type */ nd_uint8_t len; /* Attribute length */ }; - /* Service-Type Attribute standard values */ /* https://www.iana.org/assignments/radius-types/radius-types.xhtml#radius-types-4 */ static const char *serv_type[]={ NULL, @@ -298,7 +294,6 @@ static const char *login_serv[]={ "Telnet", "TCP Clear Quiet", }; - /* Termination-Action Attribute standard values */ /* https://www.iana.org/assignments/radius-types/radius-types.xhtml#radius-types-9 */ static const char *term_action[]={ "Default", @@ -757,7 +752,6 @@ static const struct attrtype { /* ^ [120, 129] ^ */ }; - /*****************************/ /* Print an attribute string */ /* value pointed by 'data' */ @@ -771,12 +765,9 @@ print_attr_string(netdissect_options *ndo, { u_int i; - ND_TCHECK_LEN(data, length); - switch(attr_code) { case TUNNEL_PASS: - if (length < 3) - goto trunc; + ND_ICHECK_U(length, <, 3); if (GET_U_1(data) && (GET_U_1(data) <= 0x1F)) ND_PRINT("Tag[%u] ", GET_U_1(data)); else @@ -794,8 +785,7 @@ print_attr_string(netdissect_options *ndo, case TUNNEL_CLIENT_AUTH: case TUNNEL_SERVER_AUTH: if (GET_U_1(data) <= 0x1F) { - if (length < 1) - goto trunc; + ND_ICHECK_U(length, <, 1); if (GET_U_1(data)) ND_PRINT("Tag[%u] ", GET_U_1(data)); else @@ -805,8 +795,7 @@ print_attr_string(netdissect_options *ndo, } break; case EGRESS_VLAN_NAME: - if (length < 1) - goto trunc; + ND_ICHECK_U(length, <, 1); ND_PRINT("%s (0x%02x) ", tok2str(rfc4675_tagged,"Unknown tag",GET_U_1(data)), GET_U_1(data)); @@ -814,8 +803,7 @@ print_attr_string(netdissect_options *ndo, length--; break; case EAP_MESSAGE: - if (length < 1) - goto trunc; + ND_ICHECK_U(length, <, 1); eap_print(ndo, data, length); return; } @@ -825,8 +813,8 @@ print_attr_string(netdissect_options *ndo, return; - trunc: - nd_print_trunc(ndo); +invalid: + nd_print_invalid(ndo); } /* @@ -841,8 +829,7 @@ print_vendor_attr(netdissect_options *ndo, u_int vendor_type; u_int vendor_length; - if (length < 4) - goto trunc; + ND_ICHECK_U(length, <, 4); vendor_id = GET_BE_U_4(data); data+=4; length-=4; @@ -855,34 +842,23 @@ print_vendor_attr(netdissect_options *ndo, vendor_type = GET_U_1(data); vendor_length = GET_U_1(data + 1); - if (vendor_length < 2) { - ND_PRINT("\n\t Vendor Attribute: %u, Length: %u (bogus, must be >= 2)", - vendor_type, - vendor_length); - return; - } - if (vendor_length > length) { - ND_PRINT("\n\t Vendor Attribute: %u, Length: %u (bogus, goes past end of vendor-specific attribute)", - vendor_type, - vendor_length); - return; - } + ND_PRINT("\n\t Vendor Attribute: %u, length: %u", + vendor_type, vendor_length); + ND_ICHECKMSG_U("length", vendor_length, <, 2); + ND_ICHECKMSG_U("length", vendor_length, >, length); data+=2; vendor_length-=2; length-=2; - ND_TCHECK_LEN(data, vendor_length); - ND_PRINT("\n\t Vendor Attribute: %u, Length: %u, Value: ", - vendor_type, - vendor_length); + ND_PRINT(", value: "); for (idx = 0; idx < vendor_length ; idx++, data++) ND_PRINT("%c", ND_ASCII_ISPRINT(GET_U_1(data)) ? GET_U_1(data) : '.'); length-=vendor_length; } return; - trunc: - nd_print_trunc(ndo); +invalid: + nd_print_invalid(ndo); } /******************************/ @@ -898,10 +874,7 @@ print_attr_num(netdissect_options *ndo, { uint32_t timeout; - if (length != 4) { - ND_PRINT("ERROR: length %u != 4", length); - return; - } + ND_ICHECK_U(length, !=, 4); /* This attribute has standard values */ if (attr_type[attr_code].siz_subtypes) { @@ -992,6 +965,10 @@ print_attr_num(netdissect_options *ndo, } /* switch */ } /* if-else */ + return; + +invalid: + nd_print_invalid(ndo); } /*****************************/ @@ -1005,10 +982,7 @@ static void print_attr_address(netdissect_options *ndo, const u_char *data, u_int length, u_short attr_code) { - if (length != 4) { - ND_PRINT("ERROR: length %u != 4", length); - return; - } + ND_ICHECK_U(length, !=, 4); switch(attr_code) { case FRM_IPADDR: @@ -1026,6 +1000,10 @@ print_attr_address(netdissect_options *ndo, ND_PRINT("%s", GET_IPADDR_STRING(data)); break; } + return; + +invalid: + nd_print_invalid(ndo); } /*****************************/ @@ -1039,12 +1017,13 @@ static void print_attr_address6(netdissect_options *ndo, const u_char *data, u_int length, u_short attr_code _U_) { - if (length != 16) { - ND_PRINT("ERROR: length %u != 16", length); - return; - } + ND_ICHECK_U(length, !=, 16); ND_PRINT("%s", GET_IP6ADDR_STRING(data)); + return; + +invalid: + nd_print_invalid(ndo); } static void @@ -1052,52 +1031,42 @@ print_attr_netmask6(netdissect_options *ndo, const u_char *data, u_int length, u_short attr_code _U_) { u_char data2[16]; - - if (length < 2 || length > 18) { - ND_PRINT("ERROR: length %u not in range (2..18)", length); - return; - } - ND_TCHECK_LEN(data, length); - if (GET_U_1(data + 1) > 128) { - ND_PRINT("ERROR: netmask %u not in range (0..128)", GET_U_1(data + 1)); - return; - } - + u_int reserved_mbz, prefix_length; + + ND_ICHECK_U(length, <, 2); + ND_ICHECK_U(length, >, 18); + reserved_mbz = GET_U_1(data); + if (reserved_mbz) + ND_PRINT("[reserved-MBZ %u] ", reserved_mbz); + prefix_length = GET_U_1(data + 1); + ND_ICHECKMSG_U("prefix length", prefix_length, >, 128); memset(data2, 0, sizeof(data2)); if (length > 2) - memcpy(data2, data+2, length-2); + GET_CPY_BYTES(data2, data+2, length-2); - ND_PRINT("%s/%u", ip6addr_string(ndo, data2), GET_U_1(data + 1)); /* local buffer, not packet data; don't use GET_IP6ADDR_STRING() */ + ND_PRINT("%s/%u", ip6addr_string(ndo, data2), prefix_length); /* local buffer, not packet data; don't use GET_IP6ADDR_STRING() */ - if (GET_U_1(data + 1) > 8 * (length - 2)) - ND_PRINT(" (inconsistent prefix length)"); + ND_ICHECKMSG_U("inconsistent prefix length", prefix_length, >, 8 * (length - 2)); return; - trunc: - nd_print_trunc(ndo); +invalid: + nd_print_invalid(ndo); } static void print_attr_mip6_home_link_prefix(netdissect_options *ndo, const u_char *data, u_int length, u_short attr_code _U_) { - if (length != 17) { - ND_PRINT("ERROR: length %u != 17", length); - return; - } - ND_TCHECK_LEN(data, length); - if (GET_U_1(data) > 128) { - ND_PRINT("ERROR: netmask %u not in range (0..128)", GET_U_1(data)); - return; - } + ND_ICHECK_U(length, !=, 17); + ND_ICHECKMSG_U("prefix length", GET_U_1(data), >, 128); ND_PRINT("%s/%u", GET_IP6ADDR_STRING(data + 1), GET_U_1(data)); return; - trunc: - nd_print_trunc(ndo); +invalid: + nd_print_invalid(ndo); } static void @@ -1106,11 +1075,7 @@ print_attr_operator_name(netdissect_options *ndo, { u_int namespace_value; - ND_TCHECK_LEN(data, length); - if (length < 2) { - ND_PRINT("ERROR: length %u < 2", length); - return; - } + ND_ICHECK_U(length, <, 2); namespace_value = GET_U_1(data); data++; ND_PRINT("[%s] ", tok2str(operator_name_vector, "unknown namespace %u", namespace_value)); @@ -1119,8 +1084,8 @@ print_attr_operator_name(netdissect_options *ndo, return; - trunc: - nd_print_trunc(ndo); +invalid: + nd_print_invalid(ndo); } static void @@ -1130,11 +1095,7 @@ print_attr_location_information(netdissect_options *ndo, uint16_t index; uint8_t code, entity; - ND_TCHECK_LEN(data, length); - if (length < 21) { - ND_PRINT("ERROR: length %u < 21", length); - return; - } + ND_ICHECK_U(length, <, 21); index = GET_BE_U_2(data); data += 2; @@ -1167,8 +1128,8 @@ print_attr_location_information(netdissect_options *ndo, return; - trunc: - nd_print_trunc(ndo); +invalid: + nd_print_invalid(ndo); } static void @@ -1177,11 +1138,7 @@ print_attr_location_data(netdissect_options *ndo, { uint16_t index; - ND_TCHECK_LEN(data, length); - if (length < 3) { - ND_PRINT("ERROR: length %u < 3", length); - return; - } + ND_ICHECK_U(length, <, 3); index = GET_BE_U_2(data); data += 2; @@ -1198,8 +1155,8 @@ print_attr_location_data(netdissect_options *ndo, return; - trunc: - nd_print_trunc(ndo); +invalid: + nd_print_invalid(ndo); } static void @@ -1208,11 +1165,7 @@ print_basic_location_policy_rules(netdissect_options *ndo, { uint16_t flags; - ND_TCHECK_LEN(data, length); - if (length < 10) { - ND_PRINT("ERROR: length %u < 10", length); - return; - } + ND_ICHECK_U(length, <, 10); flags = GET_BE_U_2(data); data += 2; @@ -1230,11 +1183,10 @@ print_basic_location_policy_rules(netdissect_options *ndo, return; - trunc: - nd_print_trunc(ndo); +invalid: + nd_print_invalid(ndo); } - /*************************************/ /* Print an attribute of 'secs since */ /* January 1, 1970 00:00 UTC' value */ @@ -1250,16 +1202,17 @@ print_attr_time(netdissect_options *ndo, time_t attr_time; char string[26]; - if (length != 4) { - ND_PRINT("ERROR: length %u != 4", length); - return; - } + ND_ICHECK_U(length, !=, 4); attr_time = GET_BE_U_4(data); strlcpy(string, ctime(&attr_time), sizeof(string)); /* Get rid of the newline */ string[24] = '\0'; ND_PRINT("%.24s", string); + return; + +invalid: + nd_print_invalid(ndo); } static void @@ -1269,10 +1222,7 @@ print_attr_vector64(netdissect_options *ndo, uint64_t data_value, i; const char *sep = ""; - if (length != 8) { - ND_PRINT("ERROR: length %u != 8", length); - return; - } + ND_ICHECK_U(length, !=, 8); ND_PRINT("["); @@ -1289,6 +1239,10 @@ print_attr_vector64(netdissect_options *ndo, } ND_PRINT("]"); + return; + +invalid: + nd_print_invalid(ndo); } /***********************************/ @@ -1307,10 +1261,7 @@ print_attr_strange(netdissect_options *ndo, switch(attr_code) { case ARAP_PASS: - if (length != 16) { - ND_PRINT("ERROR: length %u != 16", length); - return; - } + ND_ICHECK_U(length, !=, 16); ND_PRINT("User_challenge ("); len_data = 8; PRINT_HEX(len_data, data); @@ -1321,10 +1272,7 @@ print_attr_strange(netdissect_options *ndo, break; case ARAP_FEATURES: - if (length != 14) { - ND_PRINT("ERROR: length %u != 14", length); - return; - } + ND_ICHECK_U(length, !=, 14); if (GET_U_1(data)) ND_PRINT("User can change password"); else @@ -1344,25 +1292,21 @@ print_attr_strange(netdissect_options *ndo, break; case ARAP_CHALLENGE_RESP: - if (length < 8) { - ND_PRINT("ERROR: length %u != 8", length); - return; - } + ND_ICHECK_U(length, !=, 8); len_data = 8; PRINT_HEX(len_data, data); break; case ERROR_CAUSE: - if (length != 4) { - ND_PRINT("Error: length %u != 4", length); - return; - } - + ND_ICHECK_U(length, !=, 4); error_cause_value = GET_BE_U_4(data); ND_PRINT("Error cause %u: %s", error_cause_value, tok2str(errorcausetype, "Error-Cause %u not known", error_cause_value)); break; } return; + +invalid: + nd_print_invalid(ndo); } static void @@ -1374,9 +1318,7 @@ radius_attrs_print(netdissect_options *ndo, uint8_t type, len; while (length > 0) { - if (length < 2) - goto trunc; - ND_TCHECK_SIZE(rad_attr); + ND_ICHECK_U(length, <, 2); type = GET_U_1(rad_attr->type); len = GET_U_1(rad_attr->len); @@ -1389,14 +1331,8 @@ radius_attrs_print(netdissect_options *ndo, attr_string, type, len); - if (len < 2) { - ND_PRINT(" (bogus, must be >= 2)"); - return; - } - if (len > length) { - ND_PRINT(" (bogus, goes past end of packet)"); - return; - } + ND_ICHECKMSG_U("length", len, <, 2); + ND_ICHECKMSG_U("length", len, >, length); ND_PRINT(", Value: "); if (type < TAM_SIZE(attr_type)) { @@ -1416,52 +1352,50 @@ radius_attrs_print(netdissect_options *ndo, } return; -trunc: - nd_print_trunc(ndo); +invalid: + nd_print_invalid(ndo); } void radius_print(netdissect_options *ndo, - const u_char *dat, u_int length) + const u_char *dat, const u_int length) { const struct radius_hdr *rad; u_int len, auth_idx; ndo->ndo_protocol = "radius"; - ND_TCHECK_LEN(dat, MIN_RADIUS_LEN); + nd_print_protocol_caps(ndo); + ND_ICHECK_U(length, <, MIN_RADIUS_LEN); rad = (const struct radius_hdr *)dat; len = GET_BE_U_2(rad->len); - if (len < MIN_RADIUS_LEN) { - nd_print_trunc(ndo); - return; - } - - if (len > length) - len = length; + ND_ICHECKMSG_U("length", len, <, MIN_RADIUS_LEN); + ND_ICHECKMSG_U("length", len, >, 4096); + ND_ICHECKMSG_U("length", len, >, length); if (ndo->ndo_vflag < 1) { - ND_PRINT("RADIUS, %s (%u), id: 0x%02x length: %u", + ND_PRINT(", %s (%u), id: 0x%02x, length: %u", tok2str(radius_command_values,"Unknown Command",GET_U_1(rad->code)), GET_U_1(rad->code), GET_U_1(rad->id), len); + ND_TCHECK_LEN(dat, MIN_RADIUS_LEN); return; } else { - ND_PRINT("RADIUS, length: %u\n\t%s (%u), id: 0x%02x, Authenticator: ", + ND_PRINT(", length: %u\n\t%s (%u), id: 0x%02x, Authenticator: ", len, tok2str(radius_command_values,"Unknown Command",GET_U_1(rad->code)), GET_U_1(rad->code), GET_U_1(rad->id)); for(auth_idx=0; auth_idx < 16; auth_idx++) - ND_PRINT("%02x", rad->auth[auth_idx]); + ND_PRINT("%02x", GET_U_1((rad->auth + auth_idx))); } if (len > MIN_RADIUS_LEN) radius_attrs_print(ndo, dat + MIN_RADIUS_LEN, len - MIN_RADIUS_LEN); return; -trunc: - nd_print_trunc(ndo); +invalid: + nd_print_invalid(ndo); } diff --git a/tests/radius-rfc3162-v.out b/tests/radius-rfc3162-v.out index 478cae20..4cb8f6ca 100644 --- a/tests/radius-rfc3162-v.out +++ b/tests/radius-rfc3162-v.out @@ -7,6 +7,6 @@ Framed-IPv6-Prefix Attribute (97), length: 20, Value: 2001:db8:a0b:12f0::/64 Framed-IPv6-Prefix Attribute (97), length: 12, Value: 2001:db8:a0b:12f0::/64 Framed-IPv6-Prefix Attribute (97), length: 4, Value: ::/0 - Framed-IPv6-Prefix Attribute (97), length: 3, Value: ERROR: length 1 not in range (2..18) - Framed-IPv6-Prefix Attribute (97), length: 21, Value: ERROR: length 19 not in range (2..18) - Framed-IPv6-Prefix Attribute (97), length: 20, Value: ERROR: netmask 129 not in range (0..128) + Framed-IPv6-Prefix Attribute (97), length: 3, Value: [length 1 < 2] (invalid) + Framed-IPv6-Prefix Attribute (97), length: 21, Value: [length 19 > 18] (invalid) + Framed-IPv6-Prefix Attribute (97), length: 20, Value: [prefix length 129 > 128] (invalid) diff --git a/tests/radius_attr_asan.out b/tests/radius_attr_asan.out index 577b2634..727f8dc0 100644 --- a/tests/radius_attr_asan.out +++ b/tests/radius_attr_asan.out @@ -1,9 +1,9 @@ 1 06:45:20.587271427 IP (tos 0x64, ttl 249, id 40192, offset 0, flags [+, DF, rsvd], proto UDP (17), length 299, options (unknown 235 [bad length 252]), bad cksum 8000 (->1faa)!) 0.0.86.32.258 > 0.2.250.99.3799: RADIUS, length: 263 Unknown Command (58), id: 0x6a, Authenticator: 0901020ed7ff03edb63a0f00cb0f00cb - NAS-Port Attribute (5), length: 5, Value: ERROR: length 3 != 4 + NAS-Port Attribute (5), length: 5, Value: [length 3 != 4] (invalid) Unknown Attribute (254), length: 4, Value: - NAS-IP-Address Attribute (4), length: 4, Value: ERROR: length 2 != 4 - NAS-IP-Address Attribute (4), length: 4, Value: ERROR: length 2 != 4 - NAS-IP-Address Attribute (4), length: 4, Value: ERROR: length 2 != 4 + NAS-IP-Address Attribute (4), length: 4, Value: [length 2 != 4] (invalid) + NAS-IP-Address Attribute (4), length: 4, Value: [length 2 != 4] (invalid) + NAS-IP-Address Attribute (4), length: 4, Value: [length 2 != 4] (invalid) Callback-Id Attribute (20), length: 4, Value: .. [|radius] diff --git a/tests/radius_attr_asan.pcap b/tests/radius_attr_asan.pcap index 117bcf7d..efdff3cc 100644 Binary files a/tests/radius_attr_asan.pcap and b/tests/radius_attr_asan.pcap differ diff --git a/tests/radius_rfc5447_invalid_length-v.out b/tests/radius_rfc5447_invalid_length-v.out index a52c2455..7fb845db 100644 --- a/tests/radius_rfc5447_invalid_length-v.out +++ b/tests/radius_rfc5447_invalid_length-v.out @@ -1,6 +1,2 @@ 1 13:47:25.180847 IP (tos 0x0, ttl 64, id 47488, offset 0, flags [none], proto UDP (17), length 84) - 127.0.0.1.55520 > 127.0.0.1.1812: RADIUS, length: 56 - Access-Request (1), id: 0x4f, Authenticator: 5bec15a7f3ac1590f65629a9f979c340 - User-Name Attribute (1), length: 7, Value: luser - MIP6-Feature-Vector Attribute (124), length: 10, Value: [MIP6_INTEGRATED, IP4_HOA_SUPPORTED, LOCAL_MAG_ROUTING_SUPPORTED] - MIP6-Home-Link-Prefix Attribute (125), length: 19, Value: 2001:db8::/32 + 127.0.0.1.55520 > 127.0.0.1.1812: RADIUS [length 57 > 56] (invalid)