From 87501b73386358df5a88f19a53f1cbd16c2df205 Mon Sep 17 00:00:00 2001 From: Jeffrey Tippet Date: Mon, 28 Aug 2023 13:41:20 -0700 Subject: [PATCH 1/5] Fix --time-stamp-precision when used with -V Capture files were opened in two different bits of code. Only the first bit of code acquired support for --time-stamp-precision, so the 2nd and subsequent files opened with -V would always have the default timestamp precision. This fix unifies file opening into a single routine, ensuring consistent behavior for each opened file. --- tcpdump.c | 110 ++++++++++++++-------------- tests/TESTLIST | 5 ++ tests/TESTrun | 9 ++- tests/multifile-timestamp.out | 2 + tests/multifile-timestamp.pcap-list | 2 + 5 files changed, 71 insertions(+), 57 deletions(-) create mode 100644 tests/multifile-timestamp.out create mode 100644 tests/multifile-timestamp.pcap-list diff --git a/tcpdump.c b/tcpdump.c index 347439664..8f344a0cc 100644 --- a/tcpdump.c +++ b/tcpdump.c @@ -895,6 +895,56 @@ get_next_file(FILE *VFile, char *ptr) return ret; } +static int +open_pcap_file(const char *path, const netdissect_options *ndo) +{ + int dlt; + const char *dlt_name; + char ebuf[PCAP_ERRBUF_SIZE]; +#ifdef DLT_LINUX_SLL2 + static int sll_warning_printed = 0; +#endif /* DLT_LINUX_SLL2 */ +#ifdef HAVE_CAPSICUM + cap_rights_t rights; +#endif /* HAVE_CAPSICUM */ + +#ifdef HAVE_PCAP_SET_TSTAMP_PRECISION + pd = pcap_open_offline_with_tstamp_precision(path, + ndo->ndo_tstamp_precision, ebuf); +#else + pd = pcap_open_offline(path, ebuf); +#endif + + if (pd == NULL) + error("%s", ebuf); +#ifdef HAVE_CAPSICUM + cap_rights_init(&rights, CAP_READ); + if (cap_rights_limit(fileno(pcap_file(pd)), &rights) < 0 && + errno != ENOSYS) { + error("unable to limit pcap descriptor"); + } +#endif + dlt = pcap_datalink(pd); + dlt_name = pcap_datalink_val_to_name(dlt); + fprintf(stderr, "reading from file %s", path); + if (dlt_name == NULL) { + fprintf(stderr, ", link-type %u", dlt); + } else { + fprintf(stderr, ", link-type %s (%s)", dlt_name, + pcap_datalink_val_to_description(dlt)); + } + fprintf(stderr, ", snapshot length %d\n", pcap_snapshot(pd)); +#ifdef DLT_LINUX_SLL2 + if (!sll_warning_printed && dlt == DLT_LINUX_SLL2) + { + fprintf(stderr, "Warning: interface names might be incorrect\n"); + sll_warning_printed = 1; + } +#endif + + return dlt; +} + #ifdef HAVE_CASPER static cap_channel_t * capdns_setup(void) @@ -1476,7 +1526,6 @@ main(int argc, char **argv) char *endp; pcap_handler callback; int dlt; - const char *dlt_name; struct bpf_program fcode; #ifndef _WIN32 void (*oldhandler)(int); @@ -2121,36 +2170,7 @@ main(int argc, char **argv) RFileName = VFileLine; } -#ifdef HAVE_PCAP_SET_TSTAMP_PRECISION - pd = pcap_open_offline_with_tstamp_precision(RFileName, - ndo->ndo_tstamp_precision, ebuf); -#else - pd = pcap_open_offline(RFileName, ebuf); -#endif - - if (pd == NULL) - error("%s", ebuf); -#ifdef HAVE_CAPSICUM - cap_rights_init(&rights, CAP_READ); - if (cap_rights_limit(fileno(pcap_file(pd)), &rights) < 0 && - errno != ENOSYS) { - error("unable to limit pcap descriptor"); - } -#endif - dlt = pcap_datalink(pd); - dlt_name = pcap_datalink_val_to_name(dlt); - fprintf(stderr, "reading from file %s", RFileName); - if (dlt_name == NULL) { - fprintf(stderr, ", link-type %u", dlt); - } else { - fprintf(stderr, ", link-type %s (%s)", dlt_name, - pcap_datalink_val_to_description(dlt)); - } - fprintf(stderr, ", snapshot length %d\n", pcap_snapshot(pd)); -#ifdef DLT_LINUX_SLL2 - if (dlt == DLT_LINUX_SLL2) - fprintf(stderr, "Warning: interface names might be incorrect\n"); -#endif + dlt = open_pcap_file(RFileName, ndo); } else if (dflag && !device) { int dump_dlt = DLT_EN10MB; /* @@ -2604,6 +2624,8 @@ DIAG_ON_ASSIGN_ENUM * to a file from the -V file). Print a message to * the standard error on UN*X. */ + const char *dlt_name; + if (!ndo->ndo_vflag && !WFileName) { (void)fprintf(stderr, "%s: verbose output suppressed, use -v[v]... for full protocol decode\n", @@ -2683,17 +2705,7 @@ DIAG_ON_ASSIGN_ENUM int new_dlt; RFileName = VFileLine; - pd = pcap_open_offline(RFileName, ebuf); - if (pd == NULL) - error("%s", ebuf); -#ifdef HAVE_CAPSICUM - cap_rights_init(&rights, CAP_READ); - if (cap_rights_limit(fileno(pcap_file(pd)), - &rights) < 0 && errno != ENOSYS) { - error("unable to limit pcap descriptor"); - } -#endif - new_dlt = pcap_datalink(pd); + new_dlt = open_pcap_file(RFileName, ndo); if (new_dlt != dlt) { /* * The new file has a different @@ -2735,20 +2747,6 @@ DIAG_ON_ASSIGN_ENUM */ if (pcap_setfilter(pd, &fcode) < 0) error("%s", pcap_geterr(pd)); - - /* - * Report the new file. - */ - dlt_name = pcap_datalink_val_to_name(dlt); - fprintf(stderr, "reading from file %s", RFileName); - if (dlt_name == NULL) { - fprintf(stderr, ", link-type %u", dlt); - } else { - fprintf(stderr, ", link-type %s (%s)", - dlt_name, - pcap_datalink_val_to_description(dlt)); - } - fprintf(stderr, ", snapshot length %d\n", pcap_snapshot(pd)); } } } diff --git a/tests/TESTLIST b/tests/TESTLIST index a6698fcae..92d7cb209 100644 --- a/tests/TESTLIST +++ b/tests/TESTLIST @@ -933,3 +933,8 @@ ip6-snmp-oid-unsigned ip6-snmp-oid-unsigned.pcap ip6-snmp-oid-unsigned.out lwres-pointer-arithmetic-ub lwres-pointer-arithmetic-ub.pcap lwres-pointer-arithmetic-ub.out ospf-signed-integer-ubsan ospf-signed-integer-ubsan.pcap ospf-signed-integer-ubsan.out -vv bgp-ub bgp-ub.pcap bgp-ub.out -v + +# Multi-file tests +# Note the input is not a pcap file, but a file that contains a list of pcap files +multifile-timestamp multifile-timestamp.pcap-list multifile-timestamp.out -V --nano + diff --git a/tests/TESTrun b/tests/TESTrun index 1843bc582..99f94d5ed 100755 --- a/tests/TESTrun +++ b/tests/TESTrun @@ -101,13 +101,20 @@ sub runtest { my $stderrlog = "tests/NEW/${outputbase}.stderr"; my $diffstat = 0; my $errdiffstat = 0; + my $inputswitch = '-r'; + + if ($options =~ s/(^|\s+)-V\b//) { + # If the options contains '-V', remove it from there and place it + # before the input file name. + $inputswitch = '-V'; + } # we used to do this as a nice pipeline, but the problem is that $r fails to # to be set properly if the tcpdump core dumps. # # Furthermore, on Windows, fc can't read the standard input, so we # can't do it as a pipeline in any case. - $r = system "$TCPDUMP -# -n -r $input $options >tests/NEW/${outputbase} 2>${rawstderrlog}"; + $r = system "$TCPDUMP -# -n $inputswitch $input $options >tests/NEW/${outputbase} 2>${rawstderrlog}"; if($r != 0) { # # Something other than "tcpdump opened the file, read it, and diff --git a/tests/multifile-timestamp.out b/tests/multifile-timestamp.out new file mode 100644 index 000000000..d31ae6770 --- /dev/null +++ b/tests/multifile-timestamp.out @@ -0,0 +1,2 @@ + 1 06:15:22.659099 1.0 Mb/s 2412 MHz 11b 100 sq antenna 0 57dB signal DeAuthentication (00:0d:93:82:36:3a): Reserved + 2 06:15:22.659099 1.0 Mb/s 2412 MHz 11b 100 sq antenna 0 57dB signal DeAuthentication (00:0d:93:82:36:3a): Reserved diff --git a/tests/multifile-timestamp.pcap-list b/tests/multifile-timestamp.pcap-list new file mode 100644 index 000000000..e04ee4acd --- /dev/null +++ b/tests/multifile-timestamp.pcap-list @@ -0,0 +1,2 @@ +tests/reason_code-0.pcap +tests/reason_code-0.pcap From 3c5926e7a190ce28c8ef284820a7313cc585d88b Mon Sep 17 00:00:00 2001 From: Jeffrey Tippet Date: Mon, 28 Aug 2023 14:27:15 -0700 Subject: [PATCH 2/5] silence unused-parameter --- tcpdump.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tcpdump.c b/tcpdump.c index 8f344a0cc..abf67221b 100644 --- a/tcpdump.c +++ b/tcpdump.c @@ -896,7 +896,7 @@ get_next_file(FILE *VFile, char *ptr) } static int -open_pcap_file(const char *path, const netdissect_options *ndo) +open_pcap_file(const char *path, const netdissect_options *ndo _U_) { int dlt; const char *dlt_name; From 904ef27ef7ffe7010970aaee66da6520978b4dff Mon Sep 17 00:00:00 2001 From: Jeffrey Tippet Date: Mon, 28 Aug 2023 14:58:00 -0700 Subject: [PATCH 3/5] Fix update-test.sh to know about -V tricks --- tests/multifile-timestamp.out | 4 ++-- update-test.sh | 12 ++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/multifile-timestamp.out b/tests/multifile-timestamp.out index d31ae6770..d636f8b99 100644 --- a/tests/multifile-timestamp.out +++ b/tests/multifile-timestamp.out @@ -1,2 +1,2 @@ - 1 06:15:22.659099 1.0 Mb/s 2412 MHz 11b 100 sq antenna 0 57dB signal DeAuthentication (00:0d:93:82:36:3a): Reserved - 2 06:15:22.659099 1.0 Mb/s 2412 MHz 11b 100 sq antenna 0 57dB signal DeAuthentication (00:0d:93:82:36:3a): Reserved + 1 06:15:22.659099000 1.0 Mb/s 2412 MHz 11b 100 sq antenna 0 57dB signal DeAuthentication (00:0d:93:82:36:3a): Reserved + 2 06:15:22.659099000 1.0 Mb/s 2412 MHz 11b 100 sq antenna 0 57dB signal DeAuthentication (00:0d:93:82:36:3a): Reserved diff --git a/update-test.sh b/update-test.sh index ad2c7ecad..6b24042be 100755 --- a/update-test.sh +++ b/update-test.sh @@ -7,16 +7,24 @@ TZ=GMT0; export TZ for TEST in "$@"; do PREFIX=tests MATCH=0 - while read -r name input output options + while read -r name input output raw_options do [ -z "$name" ] && continue # ignore empty lines [ "${name#\#}" != "$name" ] && continue # ignore comment lines [ "$name" != "$TEST" ] && continue # not the requested test [ -z "$output" ] && continue # ignore incomplete lines MATCH=1 + options=$(echo "$raw_options" | sed "s/\(^\|\s\+\)-V\b//") + if [ "$options" = "$raw_options" ] + then + inputswitch=-r + else + inputswitch=-V + fi + # Word splitting is intentional for $options. # shellcheck disable=SC2086 - ./tcpdump -# -n -r "$PREFIX/$input" $options >"$PREFIX/$output" + ./tcpdump -# -n $inputswitch "$PREFIX/$input" $options >"$PREFIX/$output" done < $PREFIX/TESTLIST [ $MATCH = 0 ] && echo "test $TEST not found" >&2 done From ec98c309ba7dd12d93f36fc326a658c26dd6822b Mon Sep 17 00:00:00 2001 From: Jeffrey Tippet Date: Mon, 28 Aug 2023 15:01:54 -0700 Subject: [PATCH 4/5] silence unused unused parameter annotation --- tcpdump.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tcpdump.c b/tcpdump.c index abf67221b..7a1d8b6cb 100644 --- a/tcpdump.c +++ b/tcpdump.c @@ -896,7 +896,7 @@ get_next_file(FILE *VFile, char *ptr) } static int -open_pcap_file(const char *path, const netdissect_options *ndo _U_) +open_pcap_file(const char *path, const netdissect_options *ndo) { int dlt; const char *dlt_name; @@ -913,6 +913,7 @@ open_pcap_file(const char *path, const netdissect_options *ndo _U_) ndo->ndo_tstamp_precision, ebuf); #else pd = pcap_open_offline(path, ebuf); + (void)ndo; /* placate -Wunused-parameter */ #endif if (pd == NULL) From 3409c1696e27c39c58a540ea89df91beb9fa19ca Mon Sep 17 00:00:00 2001 From: Jeffrey Tippet Date: Mon, 28 Aug 2023 17:13:31 -0700 Subject: [PATCH 5/5] Update test harness to rewrite -V files to use absolute paths --- tests/TESTrun | 44 ++++++++++++++++++----------- tests/multifile-timestamp.out | 4 +-- tests/multifile-timestamp.pcap-list | 4 +-- 3 files changed, 31 insertions(+), 21 deletions(-) diff --git a/tests/TESTrun b/tests/TESTrun index 99f94d5ed..9b8de6ac0 100755 --- a/tests/TESTrun +++ b/tests/TESTrun @@ -90,10 +90,10 @@ sub showfile { } sub runtest { - local($name, $input, $output, $options) = @_; + local($name, $input, $outputbase, $options) = @_; my $r; - $outputbase = basename($output); + my $output = "$testsdir/$outputbase"; my $coredump = false; my $status = 0; my $linecount = 0; @@ -103,18 +103,31 @@ sub runtest { my $errdiffstat = 0; my $inputswitch = '-r'; + # EXPAND any occurrences of @TESTDIR@ to $testsdir + $options =~ s/\@TESTDIR\@/$testsdir/; + if ($options =~ s/(^|\s+)-V\b//) { - # If the options contains '-V', remove it from there and place it - # before the input file name. - $inputswitch = '-V'; + # If the options contains '-V', the $input file is a file that itself + # contains a list of pcap file paths. We need to process that here to + # rewrite each relative path into an absolute path into $testsdir. + open(my $pcaplist, '<', "$testsdir/$input") or die "Can't read input: $!"; + my @lines = <$pcaplist>; + chomp(@lines); + my $filenames = join("\\n", map { "$testsdir/$_" } @lines); + + # We don't pass the original input file directly; instead we write the + # munged version to stdin, and use '-V -' to read it. + $r = system "printf \"$filenames\" | $TCPDUMP -# -n -V - $options >tests/NEW/${outputbase} 2>${rawstderrlog}"; + } else { + # Otherwise, $input is the pcap file to read with '-r'. + # + # we used to do this as a nice pipeline, but the problem is that $r fails to + # to be set properly if the tcpdump core dumps. + # + # Furthermore, on Windows, fc can't read the standard input, so we + # can't do it as a pipeline in any case. + $r = system "$TCPDUMP -# -n -r $testsdir/$input $options >tests/NEW/${outputbase} 2>${rawstderrlog}"; } - - # we used to do this as a nice pipeline, but the problem is that $r fails to - # to be set properly if the tcpdump core dumps. - # - # Furthermore, on Windows, fc can't read the standard input, so we - # can't do it as a pipeline in any case. - $r = system "$TCPDUMP -# -n $inputswitch $input $options >tests/NEW/${outputbase} 2>${rawstderrlog}"; if($r != 0) { # # Something other than "tcpdump opened the file, read it, and @@ -424,12 +437,9 @@ sub runOneComplexTest { #use Data::Dumper; #print Dumper($testconfig); - # EXPAND any occurrences of @TESTDIR@ to $testsdir - $options =~ s/\@TESTDIR\@/$testsdir/; - my $result = runtest($name, - $testsdir . "/" . $input, - $testsdir . "/" . $output, + $input, + $output, $options); if($result == 0) { diff --git a/tests/multifile-timestamp.out b/tests/multifile-timestamp.out index d636f8b99..3b9c7cf4a 100644 --- a/tests/multifile-timestamp.out +++ b/tests/multifile-timestamp.out @@ -1,2 +1,2 @@ - 1 06:15:22.659099000 1.0 Mb/s 2412 MHz 11b 100 sq antenna 0 57dB signal DeAuthentication (00:0d:93:82:36:3a): Reserved - 2 06:15:22.659099000 1.0 Mb/s 2412 MHz 11b 100 sq antenna 0 57dB signal DeAuthentication (00:0d:93:82:36:3a): Reserved + 1 01:10:59.680304000 IP 48.48.48.48.69 > 48.48.48.48.12336: TFTP, length 12308, RRQ "00" [|tftp] + 2 01:10:59.680304000 IP 48.48.48.48.69 > 48.48.48.48.12336: TFTP, length 12308, RRQ "00" [|tftp] diff --git a/tests/multifile-timestamp.pcap-list b/tests/multifile-timestamp.pcap-list index e04ee4acd..38aa757fe 100644 --- a/tests/multifile-timestamp.pcap-list +++ b/tests/multifile-timestamp.pcap-list @@ -1,2 +1,2 @@ -tests/reason_code-0.pcap -tests/reason_code-0.pcap +tftp-heapoverflow.pcap +tftp-heapoverflow.pcap