Denis Ovsienko [Fri, 3 Aug 2018 21:07:51 +0000 (22:07 +0100)]
Add a test for the RX/AFS protocol.
The .pcap file is from https://wiki.wireshark.org/SampleCaptures, as
suggested on the openafs-devel mailing list.
Guy Harris [Sun, 29 Jul 2018 22:08:16 +0000 (15:08 -0700)]
Include pcap-missing.h, to get the declaration of pcap_datalink_val_to_name().
If we're building with an old version of libpcap that doesn't include
pcap_datalink_val_to_name(), we incorporate our own version, but we need
to have it declared before we use it.
The first two ("bulk stat" and "callback"; lines 1023 and 1282) should
not fall through, but the other two ("list entry" in various forms,
lines 1741 and 1790) should fall through. The answer lies in the RPC-L
files at [1], [2] and [3] (also line 396), looking at the appropriate IN
or OUT arguments.
In the last couple years it had been proved that any decoder can
potentially have buffer overflows, hence let's not emphasize one of them
more than the others.
Try the pcap_dump_ftell() check after pcap-config.
Apparently, the test for pcap_dump_ftell() cannot succeed if the test
program isn't linked with libpcap, this depending on the output of
pcap-config. That's why all pcap_* function checks come after the
pcap-config check.
This explains why in my working copy a ./configure build of the previous
commit tree with the master branch of libpcap found that the function
was "missing" and tried to substitute it with the local implementation
and eventually failed trying to link with libpcap that actually had the
function.
However, this does not explain why all 32 Travis CI builds of the same
tree passed, including the builds that used autotools.
It looks like CMake after commit 3e9e2b6 started to use the newly added
missing/pcap_dump_ftell.c to make pcap_dump_ftell() available in tcpdump
if libpcap does not have it. However, autotools continued to use the
previously existing ./pcap_dump_ftell.c for the same purpose. Remove the
previously existing file and amend autotools files to cover
pcap_dump_ftell() the same way as the other functions in the missing/
directory files.
Amend missing/pcap_dump_ftell.c not to use pcap_dump_file(), as it may be
unavailable.
This change addresses one of the warnings listed in the bug report.
./print-isoclns.c: In function ‘clnp_print’:
./print-isoclns.c:1054:16: warning: this statement may fall through [-Wimplicit-fallthrough=]
if (EXTRACT_U_1(pptr) == NLPID_CLNP) {
^
./print-isoclns.c:1061:9: note: here
case CLNP_PDU_DT:
^~~~
./print-sll.c: In function ‘sll2_if_print’:
./print-sll.c:419:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
uint32_t index = EXTRACT_BE_U_4(sllp->sll2_if_index);
^~~~~~~~
Do not decrease font size for CLI output examples.
Three CLI output examples in the tcpdump man page used to request a
smaller font size since the beginning of the version control history.
That had no effect on the plain text format, and made the examples
difficult to read in the HTML format, so get rid of it.
Found with -Wunreachable-code-return clang compiler option.
The errors were:
./print-esp.c:317:10: warning: 'return' will never be executed
[-Wunreachable-code-return]
return 0;
^
./print-esp.c:552:4: warning: 'return' will never be executed
[-Wunreachable-code-return]
return;
^~~~~~
From Linux manual page of capng_change_id():
Note: the only safe action to do upon failure of this function is to
probably exit. This is because you are likely in a situation with par-
tial permissions and not what you intended.
Guy Harris [Tue, 10 Jul 2018 08:01:15 +0000 (01:01 -0700)]
Clean up handling of different packet types.
Only look at the header for control packets; we don't know what the
payload contains for other packet types.
This fixes some cases where we fail to check whether we have a full
header before fetching from the header - we only need to fetch from the
header for control packets, so we now only need to check that we have it
for control packets; make sure we *don't* look at the header for other
packet types.
Update the v0 code to match draft-ietf-bfd-base-01, which was the last
draft that discussed v0.
Guy Harris [Mon, 9 Jul 2018 16:42:17 +0000 (09:42 -0700)]
Clean up dissection.
Don't use pointers to anything other than octets; there is no guarantee
that the L2TP packet is aligned on a 2-byte or 4-byte boundary, and
there is no need to pretend that we have pointers to aligned values -
we're using the EXTRACT_ macros, which will fetch multi-byte integral
values regardless of the alignment of the pointer.
This also fixes some cases where we were advancing 2 bytes after
processing a 1-byte field - we were incrementing a uint16_t * by 1,
which means advancing it by 2 bytes, and we're now incrementing the
uint8_t * by 1.
Don't cast a 4-byte integer to u_long - EXTRACT_BE_U_4() is guaranteed
to return something printable with %u.
Don't fetch fields dividded into "high" and "low" portions 2 bytes at a
time and reassemble them; the only reason they're divided into "high"
and "low" partitions in the ASCII-art diagrams in RFC 2661 is that those
diagrams tend to show packets in the form of 32-bit words, and those
fields aren't aligned on 32-bit word boundaries, so we can just fetch
those fields with EXTRACT_BE_U_4().
Don't print a sequence of AVPs by recursion; iterate instead.
Found with -Wunreachable-code clang compiler option.
The errors were:
./print-esp.c:263:3: warning: code will never be executed
[-Wunreachable-code]
free(input_buffer);
^~~~
./print-esp.c:246:3: warning: code will never be executed
[-Wunreachable-code]
EVP_CIPHER_CTX_free(ctx);
^~~~~~~~~~~~~~~~~~~
./print-esp.c:843:5: warning: code will never be executed
[-Wunreachable-code]
free(input_buffer);
^~~~
./print-esp.c:826:5: warning: code will never be executed
[-Wunreachable-code]
EVP_CIPHER_CTX_free(ctx);
^~~~~~~~~~~~~~~~~~~
Guy Harris [Fri, 22 Jun 2018 22:28:10 +0000 (15:28 -0700)]
Clean up processing of RPC request header.
Don't just blast through it and do a single check at the end to make
sure we didn't run past the end of the packet; check for the
fixed-length part of the credentials, then check for the variable-length
part of the credentials, and then do the same two steps for the
verifier.
Fix the checks against the on-the-network length while we're at it.
Francois-Xavier Le Bail [Tue, 5 Jun 2018 12:19:33 +0000 (14:19 +0200)]
Include conditionally <config.h> in netdissect-alloc.c
This should suppress the warning reported by Gisle Vanem:
In file included from netdissect-alloc.c:18:
In file included from ./netdissect-alloc.h:22:
./netdissect.h(131,14): warning: '_strdup' redeclared without 'dllimport'
attribute: previous 'dllimport' ignored [-Winconsistent-dllimport]
extern char *strdup (const char *str);
^
./netdissect-stdinc.h(219,18): note: expanded from macro 'strdup'
#define strdup _strdup
^
Guy Harris [Thu, 24 May 2018 19:11:09 +0000 (12:11 -0700)]
Cast dport and sport to u_int before shifting them.
The result of the expression is ultimately going to be put into a u_int;
cast them to u_int so that we'll be shifting unsigned values left rather
than int values, to avoid undefined behavior.
Guy Harris [Wed, 23 May 2018 21:43:47 +0000 (14:43 -0700)]
Declare the NFLOG pseudo-header ourselves.
It's not specified by a libpcap header that might have a different
layout in different pcap releases, it's specified on the list of
link-layer header types and must remain the same forever (except for
getting additional bits defined), so we don't need to pick it up from
libpcap.
This means we get to use tcpdump's nd_ types; do so.