If a dissector has to process its input - decryption, decompression,
etc. - rather than dissect the raw input, it should push the processed
input onto the buffer stack. As soon as the dissection is done, the
stack should be popped, to free the buffer into which the processing was
done, and restore the "pointer to packet data" and "pointer to end of
packet data" members of the netdissect_options structure, so the code
can go back to dissecting the original data.
The stack will get everything popped off it when dissection is done.
Use this mechanism in the ESP decryption code rather than scribbling on
top of the input packet data.
"ivoff" is a pointer to the IV, not the offset of the IV; call it ivptr.
Have a variable that points to the beginning of the ciphertext, and use
that.
Fix the check that makes sure the authentication data/integrity check
value length isn't too big - it needs to make sure that it doesn't go
before the beginning of the ciphertext, i.e. doesn't overlap with the
IV.
Don't bother with a variable pointing to the secret, just pass
sa->secret.
Fix the check that makes sure the padding length isn't too big - make
sure it, plus 2 for the padding length and next header bytes, isn't
bigger than the ciphertext length.
Update a test to reflect the stricter length checks.
Just check whether the pointer to it is within the available packet data
- and, if it's not, report truncation.
While we're at it:
Make the initialization vector length unsigned; it's either zero or a
positive number.
Rename a variable used for the ciphertext len to ctlen.
If we can't decrypt the payload, we can't dissect it, so don't try -
just give up immediately.
While we're at it:
If EVP_CIPHER_CTX_new() fails, it means a memory allocation failed;
treat that as such.
Use some of the arguments we're passed rather than re-fetching them from
the IP header.
Add some comments.
Call nd_print_trunc() for failed length sanity checks, and note that
they can fail due to the decryption being done with the wrong key.
Update one test's output; it is, I think, being decrypted with the wrong
key.
The exceptions are currently:
Some EXTRACT_ in print-juniper.c, not used on packet buffer pointer.
An EXTRACT_BE_U_3 in addrtoname.c, not always used on packet buffer
pointer.
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;
^~~~~~
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);
^~~~~~~~~~~~~~~~~~~
This can prevent bizarre failures if, for example, you've done a
configuration in the top-level source directory, leaving behind one
config.h file, and then do an out-of-tree build in another directory,
with different configuration options. This way, we always pick up the
same config.h, in the build directory.
Add some RFC numbers.
Structures with nd_ types work well if you overlay them on top of the
packet buffer, but not as well with an on-the-stack structure into which
you copy from the packet, which is ugly if you do *both* with the same
structure. Use overlaying uniformly.
Add EXTRACT_ macros as necessary for the nd_ types.
Make stuff unsigned if it's appropriate.
Use unsigned formats for unsigned values.
A number of routines are passed the length of a payload, so they don't
have to re-fetch that length themselves. That length has been
pre-checked to make sure it's big enough for the payload header; note
that in comments.
Now all the macros have a name meaning a count in bytes.
With _S_: signed, _U_: unsigned
e.g.:
EXTRACT_BE_32BITS -> EXTRACT_BE_U_4
EXTRACT_LE_32BITS -> EXTRACT_LE_U_4
...
EXTRACT_BE_INT32 -> EXTRACT_BE_S_4
and have:
EXTRACT_8BITS -> EXTRACT_U_1
EXTRACT_INT8 -> EXTRACT_S_1
Get rid of casts to (int) that aren't needed or wanted.
If a field is unsigned, use an unsigned variable for it, print it with
%u, not %d, and don't cast it to int.
Replace a static variable in print-dvmrp.c with a local variable in
dvmrp_print() and a parameter to print_neighbors2().
Attempt to allocate the buffer before doing any decryption calls and, if
the attempt fails, report an error and return an error indication.
Make sizes unsigned, as they can't be negative;
EVP_CIPHER_CTX_block_size() returns an int, but that's just because they
picked the wrong data type in OpenSSL.
No need to use calloc() for the output buffer - just use malloc().
We don't want to try to suppress warnings about discarding the const
qualifier, as that's something we need to fix. Adding the extra
variable doesn't suppress it on some platforms, so it's not useful
there, and if it does suppress it, we don't want that.
And we weren't doing it in the other case, anyway.
Comment in both cases with an XXX comment about the cast.
The EVP_Cipher function should not be called with the same buffer as
input and output. It works fine on most architectures, but fails on
PowerPC. There is also a second problem, that we write the output to a
const buffer, but this issue is not addressed with this PR.
with the tag '\summary:' for greping.
Remark: Currently some printers have no summary line.
Moreover:
Summarize all printers with a single line in INSTALL.txt
Move stuff to initialize and clean up libraries that are used by
netdissect code into nd_init() and nd_cleanup() routines in
libnetdissect; this includes Winsock (which is used on Windows by, for
example, the code to get names for IP addresses) and libsmi.
Call nd_init() when tcpdump starts up, and call nd_cleanup() when it
exits.
Move util.c routines to tcpdump.c, and make them static.
Use ndo->ndo_error to report a failure to open a file in print-esp.c.
In 1.1.x, EVP_CIPHER_CTX is an opaque structure, so we can't declare it
on the stack.
Instead, if we don't have EVP_CIPHER_CTX_new() and EVP_CIPHER_CTX_free()
in libcrypto, define our own versions, with the same signatures as the
ones in OpenSSL 1.1.x's libcrypto, and have the code use
EVP_CIPHER_CTX_new() to allocate the structure and EVP_CIPHER_CTX_free()
to free it.
Define them in netdissect.h.
Use them in ip.h, and *don't* mark the structures as UNALIGNED; that
should no longer be necessary.
Add a new nd_ipv4 type to use as an IPv4 address; it represents the 4
bytes of IPv4 address as an array of unsigned chars, so that its natural
alignment is only on a byte boundary.
Those changes found some places where we weren't using
UNALIGNED_MEMCPY() to extract IPv4 addresses from packets; use it.
Have our own routines to convert between IPv4/IPv6 addresses and
strings; that helps if, for example, we want to build binary versions of
tcpdump for Windows that can run both on NT 5 (W2K/WXP), which doesn't
have inet_ntop() or inet_pton(), and NT 6 (Vista/7/8/10), which do. It
also means that we don't require IPv6 library support on UN*X to print
addresses (if somebody wants to build tcpdump for older UN*Xes lacking
IPv6 support in the system library or in add-on libraries).
Get rid of files in the missing directory that we don't need, and
various no-longer-necessary autoconf tests.
Have the line number for the file we're opening for "file" be separate
from the line number we're passed. That avoids warnings, and makes it
clearer *which* line number we're using.
