--- Begin Message ---
Hello,
Some people ask for a new version of tcpdump.
To understand why we cannot release now, here is some information:
We are in the process to harden the tcpdump code with use of new (GET_) macros
(with setjmp/longjmp
logic) to fetch the packet data without have to do ND_TCHECK_/ND_TTEST_ macro
calls before EXTRACT_
macros calls.
They were some preliminary stages (for example: add and use nd_ types, more
EXTRACT_XXX() calls,
rename EXTRACT_XXX(), ND_TTEST_XXX() and ND_TCHECK_XXX macros, use
EXTRACT_[SU]_1() to replace
direct dereferences, etc.).
The first step was to replace almost all the EXTRACT_ calls by GET_ calls.
See:
-176e182416d822df0f9d4695410479e8b17a07b3 (Apply the first step of the new way
to fetch data with
bounds checking)
-ee68aa36460d7efeca48747f33b7f2adc0900bfb (Use the new GET_ macros instead of
the EXTRACT_ ones)
After we had to manage this warning:
After we have had this warning:
./print.c: In function 'pretty_print_packet':
./print.c:342:8: warning: variable 'hdrlen' might be clobbered by 'longjmp' or
'vfork' [-Wclobbered]
342 | u_int hdrlen = 0;
| ^~~~~~
therefore, the second step was:
-757e793ca521cbfdc4a6104c9a2a920f5538d195 (Apply the first step of the new way
to update the
link-layer header length)
- ~30 commits
-b30f3843b93c11e897e6d8888a91abf709a716ae (Apply the last step of the new way
to update the
link-layer header length)
There are now redundant bounds checks because most of the GET_.*_n(e) macros
are preceded by a
ND_TCHECK_n(e) call (same n, same e). There are also two ways to process the
truncated cases:
GET_ only (via longjmp) or ND_TCHECK_/ND_TTEST_ before GET_ going back all the
functions called.
As a consequence of these two ways to process the truncated cases, the -x/-X
output may be not the same.
Example of a Kerberos truncated packet:
A) Currently we have:
./tcpdump -#envvvvvx -r krb-truncated.pcap
reading from file krb-truncated.pcap, link-type EN10MB (Ethernet), snapshot
length 69
1 01:00:00.000000 01:01:fc:83:00:00 > 02:8e:00:50:6a:e1, ethertype IPv4
(0x0800), length
262144: (tos 0x8, ttl 64, id 55616, offset 0, flags [DF], proto UDP (17),
length 32820, bad cksum
6a70 (->3baf)!)
10.0.0.1.88 > 0.234.154.214.24074: v4 be KDC_REQUEST:
^O^O^O^O^O^U.@^O^D^O^O^O^O^O^O^O^O^O^O^O^O [|krb]
0x0000: 4508 8034 d940 4000 4011 6a70 0a00 0001
0x0010: 00ea 9ad6 0058 5e0a 0280 00b1 0402 0f0f
0x0020: 0f0f 0f15 0000 0f04 0f0f 0f0f 0f0f 0f0f
0x0030: 0f0f 0f0f 0016 88
The -x printing begins by:
4508: IPv4 with tos 0x08: OK
B) Testing a patch to remove now useless ND_TCHECK_1() checks (because of
GET_[SU]_1() uses)
$ git diff
---------------------------------------------------------------------
diff --git a/print-krb.c b/print-krb.c
index 41a6126a..c6c2fe7d 100644
--- a/print-krb.c
+++ b/print-krb.c
@@ -168,8 +168,6 @@ krb4_print(netdissect_options *ndo,
kp = (const struct krb *)cp;
- ND_TCHECK_1(kp->type);
-
type = GET_U_1(kp->type) & (0xFF << 1);
ND_PRINT(" %s %s: ",
@@ -181,7 +179,6 @@ krb4_print(netdissect_options *ndo,
if ((cp = krb4_print_hdr(ndo, cp)) == NULL)
return;
cp += 4; /* ctime */
- ND_TCHECK_1(cp);
ND_PRINT(" %umin ", GET_U_1(cp) * 5);
cp++;
PRINT;
@@ -191,14 +188,11 @@ krb4_print(netdissect_options *ndo,
case AUTH_MSG_APPL_REQUEST:
cp += 2;
- ND_TCHECK_1(cp);
ND_PRINT("v%u ", GET_U_1(cp));
cp++;
PRINT;
- ND_TCHECK_1(cp);
ND_PRINT(" (%u)", GET_U_1(cp));
cp++;
- ND_TCHECK_1(cp);
ND_PRINT(" (%u)", GET_U_1(cp));
break;
---------------------------------------------------------------------
C) The -x printing begins now by:
028e: Begin of destination MAC address, IPv4 is 14 octets after.
./tcpdump -#envvvvvx -r krb-truncated.pcap
reading from file krb-truncated.pcap, link-type EN10MB (Ethernet), snapshot
length 69
1 01:00:00.000000 01:01:fc:83:00:00 > 02:8e:00:50:6a:e1, ethertype IPv4
(0x0800), length
262144: (tos 0x8, ttl 64, id 55616, offset 0, flags [DF], proto UDP (17),
length 32820, bad cksum
6a70 (->3baf)!)
10.0.0.1.88 > 0.234.154.214.24074: v4 be KDC_REQUEST:
^O^O^O^O^O^U.@^O^D^O^O^O^O^O^O^O^O^O^O^O^O [|krb]
0x0000: 028e 0050 6ae1 0101 fc83 0now 000 0800 4508
0x0010: 8034 d940 4000 4011 6a70 0a00 0001 00ea
0x0020: 9ad6 0058 5e0a 0280 00b1 0402 0f0f 0f0f
0x0030: 0f15 0000 0f04 0f0f 0f0f 0f0f 0f0f 0f0f
0x0040: 0f0f 0016 88
We have:
1) The "old" way, before the patch, returning from all functions and at the end
returning the header
length to the link-layer dissector (xxx_if_print), updating the
'ndo->ndo_ll_hdr_len' field (output A).
b) The "longjmp" way with GET_ macros but no ND_TCHECK_() macros (and thus no
'trunc' label
code use): The 'ndo->ndo_ll_hdr_len' is not (or sometimes partially) updated
(output C).
To have a single process of the truncated cases and avoid redundant bounds
checks, we could:
1) Remove all the redundant ND_TCHECK_/ND_TTEST_ bounds checks. Consequently
remove most of 'trunc'
labels and the associated codes.
1bis) Examine if we have to remove some ND_TCHECK_SIZE/ND_TCHECK_LEN redundant
macros calls.
1ter) Examine if we have to remove some ND_TTEST_ macros calls.
[number of ND_TCHECK_ calls:
ND_TCHECK_1: 358
ND_TCHECK_2: 337
ND_TCHECK_3: 21
ND_TCHECK_4: 367
ND_TCHECK_5: 5
ND_TCHECK_6: 16
ND_TCHECK_7: 4
ND_TCHECK_8: 65
ND_TCHECK_16: 25
ND_TCHECK_LEN: 445
ND_TCHECK_SIZE: 238
Total: 1881]
[number of ND_TTEST_ calls:
ND_TTEST_1: 39
ND_TTEST_2: 17
ND_TTEST_3: 0
ND_TTEST_4: 6
ND_TTEST_5: 0
ND_TTEST_6: 1
ND_TTEST_7: 0
ND_TTEST_8: 0
ND_TTEST_16: 0
ND_TTEST_LEN: 45
ND_TTEST_SIZE: 15
Total: 123]
(A patch is almost ready to remove ~700 ND_TCHECK_n)
2) Process all the truncated cases with:
ndo->ndo_ll_hdr_len = 0;
longjmp(ndo->ndo_truncated, 1);
(With a new macro, like 'ND_TRUNCATED' or 'ND_IS_TRUNCATED')
3) Consequently replace
'longjmp(ndo->ndo_truncated, 1);'
by this new macro in all the get_XXX inline functions in addrtoname.h and
extract.h.
4) If some 'goto trunc' are still necessary, replace them by the new macro.
5) This implies that the functions which use a return value to indicate a
truncated case, like:
---------
static int
handle_deauth(netdissect_options *ndo,
const uint8_t *src, const u_char *p, u_int length)
{
[...]
return 1;
trunc:
return 0;
}
---------
should be be updated to void functions.
Also, there is an impact on the code *calling* these 'void functions' like for
this example:
mgmt_body_print(), etc.
(Many changes to do)
6) State that
-x When parsing and printing, in addition to printing the headers
of each packet, print the data of each packet (minus its link
level header) in hex. If the packet was truncated the link layer
header is printed.
-X When parsing and printing, in addition to printing the headers
of each packet, print the data of each packet (minus its link
level header) in hex and ASCII. If the packet was truncated
the link layer header is printed.
--
Francois-Xavier
--- End Message ---
