From: Francois-Xavier Le Bail Date: Fri, 22 Sep 2023 10:05:47 +0000 (+0200) Subject: EAP: Modernize packet parsing X-Git-Url: https://git.tcpdump.org/tcpdump/commitdiff_plain/4b0d808e20676e17a65cec4363a4f29ed1e5d80f EAP: Modernize packet parsing Enable ND_LONGJMP_FROM_TCHECK and remove the 'trunc' labels. Report invalid packets as invalid. Use ND_ICHECK_U() for length check. Remove a redundant ND_TCHECK_SIZE(). Add a const qualifier for the eap_print() parameter 'length'. Update a test output accordingly. --- diff --git a/netdissect.h b/netdissect.h index dcdc2548..a6e298ba 100644 --- a/netdissect.h +++ b/netdissect.h @@ -631,7 +631,7 @@ extern void domain_print(netdissect_options *, const u_char *, u_int, int, int); extern int dstopt_process(netdissect_options *, const u_char *); extern void dtp_print(netdissect_options *, const u_char *, u_int); extern void dvmrp_print(netdissect_options *, const u_char *, u_int); -extern void eap_print(netdissect_options *, const u_char *, u_int); +extern void eap_print(netdissect_options *, const u_char *, const u_int); extern void eapol_print(netdissect_options *, const u_char *); extern void egp_print(netdissect_options *, const u_char *, u_int); extern void eigrp_print(netdissect_options *, const u_char *, u_int); diff --git a/print-eap.c b/print-eap.c index e640e83d..f8e04d58 100644 --- a/print-eap.c +++ b/print-eap.c @@ -26,6 +26,7 @@ #include "netdissect-stdinc.h" +#define ND_LONGJMP_FROM_TCHECK #include "netdissect.h" #include "extract.h" @@ -149,7 +150,7 @@ static const struct tok eap_aka_subtype_values[] = { void eap_print(netdissect_options *ndo, const u_char *cp, - u_int length) + const u_int length) { u_int type, subtype, len; u_int count; @@ -158,6 +159,7 @@ eap_print(netdissect_options *ndo, ndo->ndo_protocol = "eap"; type = GET_U_1(cp); len = GET_BE_U_2(cp + 2); + ND_ICHECK_U(len, <, 4); if (len != length) { /* * Probably a fragment; in some cases the fragmentation might @@ -173,19 +175,12 @@ eap_print(netdissect_options *ndo, type, GET_U_1((cp + 1)), len); - if (len < 4) { - ND_PRINT(" (too short for EAP header)"); - return; - } ND_TCHECK_LEN(cp, len); if (type == EAP_REQUEST || type == EAP_RESPONSE) { /* RFC 3748 Section 4.1 */ - if (len < 5) { - ND_PRINT(" (too short for EAP request/response)"); - return; - } + ND_ICHECK_U(len, <, 5); subtype = GET_U_1(cp + 4); ND_PRINT("\n\t\t Type %s (%u)", tok2str(eap_type_values, "unknown", subtype), @@ -202,10 +197,7 @@ eap_print(netdissect_options *ndo, case EAP_TYPE_NOTIFICATION: /* According to RFC 3748, there must be at least one octet of message */ - if (len < 6) { - ND_PRINT(" (too short for EAP Notification request/response)"); - return; - } + ND_ICHECK_U(len, <, 6); ND_PRINT(", Notification: "); nd_printjnp(ndo, cp + 5, len - 5); break; @@ -216,10 +208,7 @@ eap_print(netdissect_options *ndo, * the desired authentication * type one octet per type */ - if (len < 6) { - ND_PRINT(" (too short for EAP Legacy NAK request/response)"); - return; - } + ND_ICHECK_U(len, <, 6); sep = ""; for (count = 5; count < len; count++) { ND_PRINT("%s %s (%u)", sep, @@ -231,10 +220,7 @@ eap_print(netdissect_options *ndo, case EAP_TYPE_TTLS: case EAP_TYPE_TLS: - if (len < 6) { - ND_PRINT(" (too short for EAP TLS/TTLS request/response)"); - return; - } + ND_ICHECK_U(len, <, 6); if (subtype == EAP_TYPE_TTLS) ND_PRINT(" TTLSv%u", EAP_TTLS_VERSION(GET_U_1((cp + 5)))); @@ -243,19 +229,13 @@ eap_print(netdissect_options *ndo, GET_U_1(cp + 5)); if (EAP_TLS_EXTRACT_BIT_L(GET_U_1(cp + 5))) { - if (len < 10) { - ND_PRINT(" (too short for EAP TLS/TTLS request/response with length)"); - return; - } + ND_ICHECK_U(len, <, 10); ND_PRINT(", len %u", GET_BE_U_4(cp + 6)); } break; case EAP_TYPE_FAST: - if (len < 6) { - ND_PRINT(" (too short for EAP FAST request/response)"); - return; - } + ND_ICHECK_U(len, <, 6); ND_PRINT(" FASTv%u", EAP_TTLS_VERSION(GET_U_1((cp + 5)))); ND_PRINT(" flags [%s] 0x%02x", @@ -263,10 +243,7 @@ eap_print(netdissect_options *ndo, GET_U_1(cp + 5)); if (EAP_TLS_EXTRACT_BIT_L(GET_U_1(cp + 5))) { - if (len < 10) { - ND_PRINT(" (too short for EAP FAST request/response with length)"); - return; - } + ND_ICHECK_U(len, <, 10); ND_PRINT(", len %u", GET_BE_U_4(cp + 6)); } @@ -275,10 +252,7 @@ eap_print(netdissect_options *ndo, case EAP_TYPE_AKA: case EAP_TYPE_SIM: - if (len < 6) { - ND_PRINT(" (too short for EAP SIM/AKA request/response)"); - return; - } + ND_ICHECK_U(len, <, 6); ND_PRINT(" subtype [%s] 0x%02x", tok2str(eap_aka_subtype_values, "unknown", GET_U_1((cp + 5))), GET_U_1(cp + 5)); @@ -296,8 +270,9 @@ eap_print(netdissect_options *ndo, } } return; -trunc: - nd_print_trunc(ndo); + +invalid: + nd_print_invalid(ndo); } void @@ -309,7 +284,6 @@ eapol_print(netdissect_options *ndo, ndo->ndo_protocol = "eap"; eap = (const struct eap_frame_t *)cp; - ND_TCHECK_SIZE(eap); eap_type = GET_U_1(eap->type); ND_PRINT("%s (%u) v%u, len %u", @@ -326,10 +300,10 @@ eapol_print(netdissect_options *ndo, switch (eap_type) { case EAP_FRAME_TYPE_PACKET: if (eap_len == 0) - goto trunc; + goto invalid; ND_PRINT(", "); eap_print(ndo, cp, eap_len); - return; + break; case EAP_FRAME_TYPE_LOGOFF: case EAP_FRAME_TYPE_ENCAP_ASF_ALERT: default: @@ -337,6 +311,6 @@ eapol_print(netdissect_options *ndo, } return; - trunc: - nd_print_trunc(ndo); +invalid: + nd_print_invalid(ndo); } diff --git a/tests/eap_extract_read2_asan.out b/tests/eap_extract_read2_asan.out index 2756d8b0..b43cd1e2 100644 --- a/tests/eap_extract_read2_asan.out +++ b/tests/eap_extract_read2_asan.out @@ -1 +1 @@ - 1 06:58:21.3759079661 EAP packet (0) v155, len 0 [|eap] + 1 06:58:21.3759079661 EAP packet (0) v155, len 0 (invalid)